🩺 fix: Render Stopped Run Steps From Explicit Status (#14871)

* 🩺 fix: Render Stopped Run Steps From Explicit Status

Tool calls decided "still running" vs "stopped" with a whole-message
heuristic:

  const cancelled = !isSubmitting && progress < 1 && !hasError;

That inference cannot tell which step actually stopped. An aborted step
keeps spinning while `isSubmitting` is still true, and when submitting
ends, every unfinished part flips to "Cancelled" at once regardless of
which one died.

`@librechat/agents` v3.4.6+ emits `on_run_step_closed`, a terminal
per-step signal carrying `status` and timestamps — including for steps
swept at end-of-run because the caller aborted. The pinned 3.5.1 already
ships it; nothing consumed it.

- `StepEvents.ON_RUN_STEP_CLOSED` plus `RunStepClosedEvent` /
  `RunStepStatus` types mirroring the SDK payload.
- `PartMetadata.runStepStatus` — a dedicated field, since `status` is
  already claimed by activity-label and question-form parts.
- Server handler forwards the event without the visibility gating the
  other step handlers apply: a step whose open reached the client must
  get its close, or the client is left inferring again.
- `useStepHandler` writes the terminal status onto the tool call part.
- Both decision points (`ToolCall`, the shared `useToolCallState`) prefer
  explicit status, keeping the heuristic as fallback for messages saved
  before this and endpoints that do not emit the event.

Threaded through the five cards sharing `useToolCallState`.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014vLhxCFMYkCaTsoFTiAjJ5

* 🩹 fix: Address Codex Review On Run Step Closure Rendering

- Persist the terminal status server-side. The handler emitted the
  closure without folding it into `contentParts`, so the status existed
  only on the live React message: a reload or resumable reconnect
  dropped it and fell back to the very heuristic this fixes. Now stamped
  onto the aggregated tool-call part (via `stepMap`, falling back to the
  event's own index) before forwarding.

- Honor terminal status independently of output parsing. Gating on
  `hasError` meant a `failed` step with unparseable output rendered as
  "cancelled", while a `failed`/`cancelled` step whose output did parse
  as an error was not terminal at all and shimmered indefinitely when no
  completion event arrived. A closed step now forces progress complete
  and reports `failed` as an error state on its own authority.

- Pass the status to the second `BashCall` branch, which rendered the
  same updated component without it.

- Reuse `Agents.RunStepClosedStatus` in `PartMetadata` instead of
  redeclaring the union, so a future SDK status cannot diverge between
  the event and the persisted part.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014vLhxCFMYkCaTsoFTiAjJ5

*  fix: Replay Closed Status On Redis Resume, Announce Failures

- Apply closure events during Redis reconstruction. The stamp added in
  the previous commit mutates only the originating process's in-memory
  `contentParts`; a resumable reconnect landing on another replica
  rebuilds from `RedisJobStore.getContentParts`, whose allowlist omits
  `on_run_step_closed`. The status was therefore absent from the sync
  snapshot and, being snapshot-covered, never redelivered as pending —
  so multi-replica resume fell back to the whole-message heuristic.
  Handled as a host-authored event alongside `on_steer_applied` and
  `on_activity_label`, since the SDK aggregator has no notion of it.

- Announce terminal failures in the live region. Forcing terminal
  progress for a closed step meant a `failed` tool reached the
  `aria-live` region through `getFinishedText()`, which only special-
  cased cancellation and otherwise announced "completed function" —
  telling screen-reader users the opposite of what the card showed. A
  regression introduced by the previous commit; error states now
  announce failure before any completion string.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014vLhxCFMYkCaTsoFTiAjJ5

* 🎯 fix: Resolve Closed Steps By ID, Never By Index

The steer and HITL offset wrappers clone and shift only `ON_RUN_STEP`
and `ON_AGENT_UPDATE`; every other event passes through untouched. A
stored `on_run_step_closed` therefore carries the SDK's unshifted index,
while the part it belongs to was rebuilt at the shifted one. Any run
containing a steer insertion or HITL resume would stamp the status onto
an earlier tool card, or none — leaving the real card on the fallback
heuristic while mislabeling a different one.

- Redis reconstruction builds a step ID -> index map from the replayed
  `on_run_step` payloads (which carry the shifted index) and resolves
  closures against it, mirroring what the live callback does via
  `stepMap`.

- The live handler drops its `?? data.index` fallback for the same
  reason. Skipping is the safe failure: a missing status degrades to the
  old heuristic, whereas a misplaced one actively mislabels the wrong
  card.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014vLhxCFMYkCaTsoFTiAjJ5

---------

Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
Danny Avila 2026-08-15 10:14:16 -04:00 committed by GitHub
parent cd4511038d
commit 88747f0ad8
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
14 changed files with 266 additions and 17 deletions

View file

@ -4,6 +4,7 @@ const {
Tools,
StepTypes,
StepEvents,
ContentTypes,
FileContext,
ErrorTypes,
UsageEvents,
@ -427,6 +428,43 @@ function getDefaultHandlers({
}
},
},
[GraphEvents.ON_RUN_STEP_CLOSED]: {
/**
* Handle ON_RUN_STEP_CLOSED event the terminal signal for a run step.
*
* Stamped onto the aggregated part before it is forwarded. The SDK's
* `aggregateContent` has no notion of this event, so without stamping
* here the status would exist only on the live client message: a reload
* or a resumable reconnect would drop it and fall back to inferring
* "stopped" from `isSubmitting`, which is the behavior this fixes.
*
* Forwarded unconditionally, without the visibility gating the other
* step events apply a step whose `on_run_step` reached the client must
* get its closure, or the client is left inferring again.
*
* @param {string} event - The event name.
* @param {RunStepClosedEvent} data - The event data.
*/
handle: async (event, data) => {
const stepId = data?.id;
if (typeof stepId === 'string' && contentParts) {
/**
* Resolved through `stepMap` only. The event's own `index` is the
* SDK's, and the steer/HITL offset wrappers shift `ON_RUN_STEP` but
* pass closures through untouched so falling back to it would
* stamp an unrelated part in any run containing an injection.
* Skipping is the safe failure here; a missing status degrades to
* the old heuristic, a misplaced one mislabels the wrong card.
*/
const index = stepMap?.get(stepId)?.index;
const part = typeof index === 'number' ? contentParts[index] : undefined;
if (part?.type === ContentTypes.TOOL_CALL && part.tool_call) {
part.tool_call.runStepStatus = data.status;
}
}
await emitForJob({ event, data });
},
},
[GraphEvents.ON_RUN_STEP_DELTA]: {
/**
* Handle ON_RUN_STEP_DELTA event.