Repository navigation
🔪 feat: Cooperative Mid-Generation Stream Preemption - #335
Conversation
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f34063e8e0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| chunk.usage_metadata = { | ||
| input_tokens: 0, | ||
| output_tokens: outputTokens, | ||
| total_tokens: outputTokens, | ||
| }; |
There was a problem hiding this comment.
Account for prompt tokens in sealed model usage
When a provider reports usage only in its terminal chunk, every mid-stream seal takes this fallback and records input_tokens: 0, even though the provider processed—and billed for—the entire prompt. ModelEndHandler consumes this synthetic metadata directly, and each resumed call sends the prompt again, so preempted runs can substantially under-report billing and usage; estimate the prompt tokens or represent them as unavailable rather than zero.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed in 35214191. The prompt was processed and billed in full before the seal, and every resume re-sends it, so input_tokens: 0 under-counted the more expensive half. Both sides are now estimated from the host counter over messagesForProvider — the exact array handed to model.stream. When no counter is configured we report nothing rather than a fabricated number.
Related, from live testing: on OpenAI, Azure and DeepSeek the provider sends no usage chunk before the seal, so this fallback is the only source; Anthropic streams usage incrementally and survives either way. RunConfig.tokenCounter is now documented as a precondition of StreamPreemption.
| !this.subagentScope && | ||
| this.preemption != null && | ||
| this.preemptSealCount < (this.preemption.maxSeals ?? DEFAULT_MAX_SEALS) && | ||
| this.preemption.shouldPreempt() === true |
There was a problem hiding this comment.
Claim a preemption request before parallel streams can seal
In a fan-out MultiAgentGraph, multiple model streams can evaluate this shared predicate while the host request remains armed; the counter is incremented only later by notePreemptSeal(). Consequently, several agents can seal for one queued steer (and even exceed maxSeals), while only one boundary drains the message and the others terminate as empty, incomplete responses. Atomically claim the request/seal budget when the predicate succeeds so only one parallel stream handles it.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed in 35214191, though not the way you suggested. Atomically claiming the budget is not sufficient: with maxSeals=8, three parallel agents each claim a distinct slot and two still take the nothing-to-inject path. The invariant needed is one in-flight seal per graph, not merely an atomic counter.
Added claimPreemptSeal(), which validates and mutates in a single synchronous body, gated on a new preemptSealInFlight flag released once the boundary resolves. shouldPreemptStream() stays as the cheap non-mutating poll, so a chunk that cannot seal never spends budget.
| super.clearHeavyState(); | ||
| this.messages = []; | ||
| this.overrideModel = undefined; | ||
| this.resetPreemptState(); |
There was a problem hiding this comment.
Preserve preemption stats through normal cleanup
After a normal processStream call, run.ts invokes clearHeavyState() by default, and this call resets both counters immediately before callers can inspect Run.getPreemptStats(). Thus the newly exposed API always reports { seals: 0, emptyBoundaries: 0 } after ordinary completed or empty-boundary runs, preventing hosts from detecting the unfinished response the API is intended to surface; retain the lightweight counters until the next run reset.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed in 35214191. Splitting the two concerns the single counter was conflating: preemptSealBudgetUsed is per-turn and clears on both reset paths (so a HITL resume keeps its remaining budget), while preemptSealCount / preemptEmptyBoundaries are lifetime totals that now survive clearHeavyState() and clear only when a genuinely new run starts.
| for (const block of content) { | ||
| if (block.type === 'server_tool_use') { | ||
| return true; | ||
| } |
There was a problem hiding this comment.
Distinguish paired Anthropic server-tool blocks
For an Anthropic web-search stream, the accumulated chunk retains the earlier server_tool_use block after its matching web_search_tool_result arrives, because chunks are concatenated. This unconditional existence check therefore remains true for the rest of the answer, so a preemption requested after the search result can never seal despite the pair being complete; match tool-use IDs against result IDs and reject only genuinely unpaired blocks.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Outcome confirmed, cause refuted — fixed differently in 35214191.
Id-pairing inside hasServerToolUseBlock would be a no-op, because that check never decides anything. _makeMessageChunkFromAnthropicEvent emits a tool_call_chunk for every server_tool_use content block, and concat keeps it on the accumulated message for the rest of the turn — so the tool_call_chunks gate above it already returns false, and the content check is unreachable.
Removed the dead check, and documented the invariant that actually holds: once a turn starts a web search it is no longer preemptible, and a queued message waits for the ordinary tool boundary. The tests now build realistic accumulated chunks carrying the tool-call chunk, so the real behavior is pinned rather than the fictional one.
| formattedMessages.push( | ||
| withMessageRole(new AIMessage({ content: '' }), 'assistant') | ||
| ); |
There was a problem hiding this comment.
Give the synthetic steer anchor a distinct message ID
When persisted assistant messages carry their normal messageId/id, the loop immediately below assigns that same ID to the partial assistant, the steer HumanMessage, and this new anchor. messagesStateReducer deduplicates right-hand messages by ID, so on graph initialization the anchor replaces the preceding two entries and the model never receives the steering instruction. Assign unique IDs to the split messages, especially this synthetic anchor, before returning them to graph state.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed by execution and fixed in 35214191 — good catch, this one was worse than the bug it was meant to fix.
The anchor was pushed before the sourceMessageId stamping loop, so it inherited the shared id, and messagesStateReducer treats a repeated id as replace-in-place. Reproduced: [AI "Partial answer.", Human(steer), AI ""] all stamped a1 reduce to just the empty anchor — the steering instruction never reaches the model. Fixed by pushing the anchor after the stamping loop so it arrives with a null id and gets a fresh uuid. Regression test added.
Worth separating out: the collision is broader than this anchor and predates the PR. Any consumer that supplies messageId already collapses AI(tool_calls) + Tool to an orphan tool result, and formatAgentMessages.test.ts currently asserts the duplicate stamping as intended behavior. That deserves its own issue rather than a fix smuggled into this PR.
| this.preemptEmptyBoundaries += 1; | ||
| this.preemptIncomplete = true; |
There was a problem hiding this comment.
Suppress natural completion for an empty preempt boundary
When the boundary returns no injected messages, this flag is set but never consulted by Run.processStream; the graph reaches END and the normal Stop hook still fires. Hosts therefore receive a natural-completion lifecycle event for an answer that this branch explicitly marks as cut short, which can trigger completion persistence or notifications incorrectly. Propagate the incomplete state to the run loop and skip or distinguish the Stop event.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed in 35214191. preemptIncomplete was write-only. Stop still fires — the run genuinely stopped — but it now carries stopReason: 'preempt_incomplete', matching the existing stopReason contract on StopHookInput, so a host can distinguish a cut-short answer from a finished one without a new lifecycle event.
| if (result == null || result.injectedMessages.length === 0) { | ||
| return []; | ||
| } | ||
| try { | ||
| return convertInjectedMessages(result.injectedMessages); |
There was a problem hiding this comment.
Inject PreemptBoundary additional context before resuming
PreemptBoundaryHookOutput aliases BaseHookOutput, whose public contract allows additionalContext, but this dispatch path only examines injectedMessages. A hook returning context alone is therefore treated as an empty boundary and truncates the answer, while a hook returning both silently loses the context before the next model call. Materialize result.additionalContexts using the same system-flavored HumanMessage convention as PostToolBatch before deciding the boundary is empty.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed in 35214191. additionalContexts is now materialized into the same system-flavored HumanMessage the tool boundary uses, and crucially before the emptiness test — previously a hook returning context alone read as "nothing to resume with" and truncated the answer.
| await safeDispatchCustomEvent( | ||
| GraphEvents.CHAT_MODEL_END, | ||
| { output: chunk }, | ||
| config | ||
| ); |
There was a problem hiding this comment.
Close the native model callback run on a seal
safeDispatchCustomEvent delegates to LangChain's dispatchCustomEvent, so naming this custom event on_chat_model_end invokes handleCustomEvent; it does not invoke the model run manager's handleLLMEnd. Because the sealed iterator intentionally exits without its native end/error callback, LangSmith, Langfuse, and other standard model callback handlers retain an unclosed model run even though the host registry receives a look-alike event. End the actual callback run, then separately notify the custom handler registry if needed.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed, and I could not fix it — details so the tradeoff is on the record.
Ending the real run needs its id, and it is not knowable here. model is a bound runnable, so the binding consumes config.runId for its own run and patchConfig hands the inner chat model a fresh one. I implemented your suggestion anyway — pinned an id at the model.stream call and reconstructed a CallbackManagerForLLMRun with the same argument list handleChatModelStart uses — and tested it live on Anthropic and OpenAI: it fails with No LLM run to end, and it also suppresses the host's model-end event, dropping modelEndEvents from 2 to 1 and losing usage entirely. That is strictly worse than an unclosed trace span, so it is reverted.
The limitation is now documented at the dispatch site. A real fix needs a seam in @langchain/core to end a run the caller did not start.
There was a problem hiding this comment.
Correction — this is fixed, in dc0a784c. My earlier reply called it unreachable, which was wrong.
The run id could not be dictated: pinning config.runId fails because the bound runnable consumes it for its own run and hands the chat model a fresh one. But it can be observed — handleChatModelStart fires with the real id before the first chunk. A capture handler on the model.stream config records it, and the seal rebuilds a CallbackManagerForLLMRun against that id to call handleLLMEnd. No upstream change needed.
The handler is installed only when RunConfig.preemption is configured, so a run that cannot seal carries no extra callback. The custom-event dispatch remains as a fallback if the id is never observed.
Measured, not assumed. The probe now counts raw handleChatModelStart / handleLLMEnd pairs — what a tracer actually sees — and I A/B tested to confirm the measurement can fail:
without the native close: starts=2 ends=1 runsLeftOpen=1
with the native close: starts=2 ends=2 runsLeftOpen=0
So the leak was exactly one unclosed span per seal. runsLeftOpen === 0 is now part of the probe pass condition.
Live provider resultsRan the full seal → 7 of 10 providers pass. No provider rejected a sealed sequence.
The three untested rows are credential/account gaps, not results — they say nothing either way about preemption on those providers. For scale: the same prompt unpreempted returns 12,526 characters on Anthropic. Sealing at 126 means the stream really did stop mid-answer rather than running to completion. Gemini's 5,014 reflects its much larger chunk granularity — 15 deltas is already 5k characters there — not a slower seal. Zero occurrences of the failure modes the seal gate exists to prevent: no 400 on message ordering, no empty-content rejection, no adjacent-user-turn error, no orphaned One real finding:
|
Round 1: 7 of 8 fixed in
|
| Finding | Verdict | Outcome |
|---|---|---|
| Prompt tokens in sealed usage (P1) | confirmed | Both sides now estimated from the host counter |
| Parallel seal race (P1) | confirmed, fix insufficient as proposed | claimPreemptSeal() + one-in-flight gate |
| Steer anchor id (P1) | confirmed by execution, root cause pre-existing | Anchor pushed after id stamping |
| Stats cleared by cleanup (P2) | confirmed | Per-turn budget split from lifetime totals |
preemptIncomplete unused (P2) |
confirmed | stopReason: 'preempt_incomplete' on Stop |
additionalContext dropped (P2) |
confirmed | Materialized before the emptiness test |
server_tool_use pairing (P2) |
outcome confirmed, cause refuted | Dead check removed, real invariant documented |
| Unclosed native model run (P2) | confirmed, not fixable here | Attempted, verified worse, reverted + documented |
Three worth expanding on.
The parallel race needed a stronger invariant than the one suggested. Atomically claiming the seal budget is not enough — with maxSeals=8, three parallel agents each claim a distinct slot and two still take the nothing-to-inject path. What is actually required is one in-flight seal per graph. claimPreemptSeal() validates and mutates in one synchronous body behind a preemptSealInFlight gate.
The server_tool_use fix would have been a no-op. Id-pairing inside hasServerToolUseBlock cannot help, because that check never decided anything: Anthropic emits a tool_call_chunk for every server_tool_use block, concat retains it, and the tool-call gate above already returns false. The check was dead code. Removed it, documented the invariant that actually holds — a turn that starts a web search is not preemptible afterwards — and rewrote the tests around realistic accumulated chunks so the real behavior is pinned.
The anchor-id finding was the most valuable of the eight, and it was worse than described: the anchor did not merely take precedence, it destroyed the steering instruction. Reproduced by execution — three messages sharing sourceMessageId reduce to just the empty anchor. Fixed and regression-tested. The underlying collision is broader than this PR: any consumer that supplies messageId already collapses AI(tool_calls) + Tool into an orphan tool result, and formatAgentMessages.test.ts currently asserts that duplicate stamping as intended. Filing separately rather than fixing it under cover of this review.
On the unclosed model run — confirmed, and I could not fix it. The run id is not knowable here: model is a bound runnable, so the binding consumes config.runId and patchConfig gives the inner chat model a fresh one. I implemented the suggestion regardless and tested it live: it fails with No LLM run to end and additionally suppresses the host's model-end event, dropping usage entirely. Strictly worse than an unclosed span, so it is reverted and documented at the dispatch site. A real fix needs a seam in @langchain/core.
Live re-verified after every change — anthropic and openAI both back to one seal, no empty boundaries, [ai, human(steer), ai], two model-end events carrying usage. Unit suites: 600 passing, no new failures.
@codex review
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3521419164
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| */ | ||
| if (i < payload.length - 1 && endsWithSteerMessage(formattedMessages)) { | ||
| formattedMessages.push( | ||
| withMessageRole(new AIMessage({ content: '' }), 'assistant') |
There was a problem hiding this comment.
Give the steer anchor non-empty content
When a persisted assistant ends with a steer and another turn follows, this inserts a non-final assistant message whose content is empty. The Anthropic conversion path preserves an empty string for an AI message without tool calls, so the next request contains an empty intermediate assistant turn and is rejected rather than repairing the adjacent user turns. Use a non-empty neutral placeholder, as the Bedrock/Anthropic conversion helpers already do for other emptied assistant turns.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed in 7f144661. Reproduced through the real converter — a string-content assistant message with no tool calls returns from _formatContent verbatim, and the empty-text repair only covers array content (message_inputs.ts:891-893) and the empty-string-with-tool-calls branch (:938-949), neither of which the anchor hits. Output was literally {"role":"assistant","content":""} at a non-final position.
You are right that it was worse than the problem it fixed: Anthropic tolerates adjacent user turns, and rejects empty assistant content outright. It also contradicted this PR's own seal gate, which refuses to seal without non-empty text for exactly that reason.
Now uses the same single-underscore placeholder the Anthropic and Bedrock converters already synthesize. Bedrock's wire payload is byte-identical either way, since it was already rewriting the empty anchor to [{text:'_'}].
Worth recording that the live provider probe could never have caught this — it never touches formatAgentMessages. The anchor is a replay-path construct. Added a regression test that runs formatAgentMessages output through _convertMessagesToAnthropicPayload and asserts both invariants: no empty assistant content, and no adjacent user turns.
| * sealing for a message it would never receive. | ||
| */ | ||
| claimPreemptSeal(): boolean { | ||
| if (!this.shouldPreemptStream()) { |
There was a problem hiding this comment.
Avoid polling the preemption callback twice
On a sealable chunk, attemptInvoke has already called shouldPreemptStream() before entering claimPreemptSeal(), but this line invokes it again and therefore calls the host's shouldPreempt callback twice for one chunk, contrary to the public once-per-chunk contract. An edge-triggered callback that clears or consumes its pending flag when returning true passes the first poll, fails this second poll, and loses the requested preemption; the atomic claim should recheck only the internal in-flight/budget state rather than polling the host again.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed in 7f144661 — a regression I introduced in round 1. Split canClaimPreemptSeal() out for the internal preconditions; claimPreemptSeal() now rechecks only those and never re-polls the host. Atomicity is unchanged: the guard and both mutations are still one synchronous body.
One correction to the rationale, so the docs do not overpromise. This does not make edge-triggered callbacks safe. A consume-on-read predicate still loses its request whenever the armed chunk fails canSealPreempt — the poll returns true, the shape gate rejects, the flag is gone, and no seal ever happens. Single-polling is correct on its own terms (the contract says once per chunk), but level-triggered remains the requirement.
| if ( | ||
| context?.shouldPreemptStream() === true && | ||
| canSealPreempt(finalChunk) && | ||
| context.claimPreemptSeal() |
There was a problem hiding this comment.
Disable sealing for every registered stream handler
When the host registers a plain or composed CHAT_MODEL_STREAM handler rather than a ChatModelStreamHandler instance, getRegisteredDefaultChatStreamHandler() returns undefined, so this branch still honors preemption. That custom handler is consuming the decoupled streamEvents feed and can lag the locally accumulated chunk, meaning the boundary can inject a steer before the host has displayed the content it is anchored after—the exact index inversion the public preemption contract says disables sealing. Check for the presence of any registered stream handler before evaluating this seal.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Refuted — keeping the gate as it is, with the reasoning on the record.
The instanceof result is not what makes this branch safe. The branch constructs its own ChatModelStreamHandler (invoke.ts:343) and awaits it per chunk (:351-356). That handler's dispatchRunStep / dispatchMessageDelta deliver ON_RUN_STEP — carrying runStep.index — and ON_MESSAGE_DELTA to the host through awaited direct registry dispatch (Graph.ts:3348, :3516), not through streamEvents. The seal check at invoke.ts:375-382 therefore runs after the host has been handed every content part in the accumulated chunk, so there is no index to invert.
A registered plain CHAT_MODEL_STREAM handler receives only the raw chunk echo from run.ts:945 and participates in no index assignment, so its lag cannot reorder anything.
Separately, "any handler registered" is the wrong proxy for "host renders off the raw feed" — LibreChat's OpenAI-compat controller registers a no-op OpenAIChatModelStreamHandler, which would trip that gate while rendering nothing.
| if ((chunk.invalid_tool_calls?.length ?? 0) > 0) { | ||
| return false; | ||
| } | ||
| return hasNonEmptyTextContent(chunk.content); |
There was a problem hiding this comment.
Reject Google server-tool blocks at the seal gate
During a Gemini/Vertex server-side tool turn, streamed toolCall and toolResponse parts live in content and are handled separately in stream.ts; they do not necessarily populate tool_calls or tool_call_chunks. If visible text was accumulated before an in-flight toolCall, all three guards above pass and this return value allows the stream to break with an unmatched server-side call, dropping its response or replaying an invalid partial turn on resume. Treat Google server-tool content as unsafe until its pairing is complete, or conservatively make the turn non-preemptible once one appears.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed in 7f144661. You are right that these never reach tool_calls or tool_call_chunks — those are derived from functionCall parts only, while a server-side invocation arrives as a toolCall content block and its result as a later toolResponse. All three tool gates passed.
Added a counting guard rather than id-matching: Google answers calls in order, and counting stays correct when a part omits its id. Sealing is refused while toolCall blocks outnumber toolResponse blocks, and permitted once they balance.
Note the live vertexai probe did not cover this — it used a tool-free prompt — so this is pinned by unit tests rather than by the live matrix.
| * in `processStream`'s `finally`, and the host reads `getPreemptStats()` | ||
| * after that returns. | ||
| */ | ||
| this.resetPreemptTurnState(); |
There was a problem hiding this comment.
Release the host preemption callback during cleanup
After a naturally completed or failed run, this cleanup resets the seal counters but leaves this.preemption holding the host's shouldPreempt closure. LangGraph's compiled runnable and async-local callback chains can keep the graph reachable after processStream, so a predicate that closes over the request's steering queue, messages, or other session state remains retained even though every other callback-style graph reference is cleared by clearHeavyState(). Clear the preemption reference on this non-resume cleanup path so per-request closures can be collected.
AGENTS.md reference: AGENTS.md:L44-L44
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Refuted — not fixing, for two reasons.
It reclaims nothing. Run.preemption (run.ts:211) holds the identical closure and the Run roots the Graph, so clearing the graph's reference leaves the closure just as reachable.
And it would introduce a correctness bug. preemption is assigned once, in the StandardGraph constructor (Graph.ts:1030), and nothing re-hydrates it — so clearing it in clearHeavyState() would silently disable an explicitly opted-in feature for every processStream after the first on a reused Run, while Run.preemption kept reserving seal headroom in the recursion limit.
It is also inconsistent with the sibling construction-time host callbacks: subagentUsageSink and tokenCounter both deliberately survive clearHeavyState(). If we want this hygiene it belongs as a re-hydration step at the top of processStream, applied to all three, not as a special case for this one field.
Round 2: 3 fixed in
|
| Finding | Verdict | Outcome |
|---|---|---|
| Empty steer anchor (P1) | confirmed | Non-empty placeholder + converter-level regression test |
| Host callback polled twice (P2) | confirmed | canClaimPreemptSeal() split out; one poll per chunk |
| Google server-tool blocks (P2) | confirmed | Counting guard on toolCall / toolResponse |
| Disable sealing for any registered handler (P1) | refuted | Gate unchanged — reasoning below |
Clear preemption in cleanup (P2) |
refuted | Would disable the feature on a reused Run |
The empty-anchor finding was correct and the fix was worse than the bug. Reproduced through the real converter: a string-content assistant message with no tool calls returns from _formatContent verbatim, so the anchor reached Anthropic as {"role":"assistant","content":""} at a non-final position. Anthropic tolerates adjacent user turns and rejects empty assistant content outright — so the anchor traded a tolerated problem for a fatal one. It also contradicted this PR's own seal gate, which refuses to seal without non-empty text for precisely that reason.
Two things worth recording about how this was missed. The live provider matrix could never have caught it: the probe never touches formatAgentMessages, because the anchor is a replay-path construct — it needs a persisted assistant turn that ends on a steer with another turn after it. And the round-1 tests actively locked the defect in, asserting expect(anchor.content).toBe(''). The new test runs formatAgentMessages output through _convertMessagesToAnthropicPayload and asserts the invariants that actually matter: no empty assistant content, no adjacent user turns.
The double-poll was mine, from round 1 — claimPreemptSeal() re-entered shouldPreemptStream(). Fixed, with one correction to the stated rationale: this does not make edge-triggered callbacks safe. A consume-on-read predicate still loses its request whenever the armed chunk fails canSealPreempt. Single-polling is right on its own terms; level-triggered remains the contract.
On the registered-handler finding — the instanceof result is not what makes that branch safe. The branch constructs its own ChatModelStreamHandler and awaits it per chunk, and that handler dispatches ON_RUN_STEP (carrying runStep.index) and ON_MESSAGE_DELTA through awaited direct registry calls, not through streamEvents. Every content part in the accumulated chunk has reached the host before the seal is evaluated, so there is no index to invert. A registered plain handler receives only the raw chunk echo and assigns no indices. Also, "any handler registered" is the wrong proxy for "host renders off the raw feed": LibreChat's OpenAI-compat controller registers a no-op handler that would trip that gate while rendering nothing.
On clearing preemption — it frees nothing (Run.preemption holds the same closure and roots the graph) and would silently disable sealing on every processStream after the first, since the field is assigned once at construction with nothing to re-hydrate it. It is also inconsistent with subagentUsageSink and tokenCounter, which deliberately survive cleanup. If the hygiene is wanted it belongs as a re-hydration step covering all three.
Live re-verified after the gate changes — anthropic and vertexai both ok:true, one seal, no empty boundaries, two model-end events with usage. 604 unit tests pass.
@codex review
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Retraction: the unclosed model run is fixed after all (
|
| starts | ends | runs left open | |
|---|---|---|---|
| without the native close | 2 | 1 | 1 per seal |
| with the native close | 2 | 2 | 0 |
So the leak was exactly one unclosed span per seal on LangSmith, Langfuse and every other callback tracer. runsLeftOpen === 0 is now part of the probe's pass condition rather than a claim in a comment.
Live verified on anthropic and openAI: ok:true, one seal, no empty boundaries, [ai, human(steer), ai], two model-end events with usage, zero runs left open. 604 unit tests pass.
Still open, stated plainly
- The parallel-agent seal race fix (
claimPreemptSeal+ in-flight gate) is correct by construction but analysis-only — no fan-outMultiAgentGraphhas been run with preemption armed. - There is no CI coverage of the seal → boundary → self-loop path. It is verified only by a live-credential probe, because jest cannot load any suite that imports
src/llm/providers(@langchain/mistralaiESM). That blocker is pre-existing and reproduces onmain; it wants its own PR. formatAgentMessagesis the only non-opt-in surface in this PR — it affects steering users onmaintoday, independent of preemption.
@codex review
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dc0a784cdc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| shouldPreemptStream(): boolean { | ||
| return ( | ||
| this.canClaimPreemptSeal() && this.preemption?.shouldPreempt() === true | ||
| ); |
There was a problem hiding this comment.
Gate seals on an active boundary hook
When preemption is configured but the run has no PreemptBoundary matcher—also possible after a once matcher has fired—this predicate still permits shouldPreempt to claim a seal. dispatchPreemptBoundary then returns no messages, so the partial answer is cut short and marked preempt_incomplete even though the public configuration contract says the boundary hook is required. Check hasHookFor('PreemptBoundary', runId) before sealing, or reject the invalid configuration before streaming.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed in ecb3c702. canClaimPreemptSeal() now requires hasHookFor('PreemptBoundary', runId), resolving the session id the same way dispatchPreemptBoundary does so a session-scoped matcher is not visible at one site and invisible at the other. It sits before the host poll, so the cheap checks still run first.
Failing closed lands exactly on the documented no-preemption behavior: the model finishes its answer and the queued message waits for the next tool boundary. That is strictly better than sealing into a boundary that provably returns nothing and marking the turn preempt_incomplete for no gain.
The once-matcher case you raised is covered by the same check, since the registry stops reporting the hook once it is consumed.
| if (outputTokens == null) { | ||
| return; | ||
| } | ||
| const inputTokens = countSealedTokens(context, metadata, prompt) ?? 0; |
There was a problem hiding this comment.
Include instruction overhead in sealed input usage
When a provider such as OpenAI is sealed before emitting usage, this counts only messagesForProvider, but createCallModel commonly wraps the model with agentContext.systemRunnable.pipe(model) and binds tool schemas after this array is formed. Consequently the synthetic input_tokens omits the system prompt, dynamic instructions, summaries, and tool-schema overhead on every affected seal, even though all of them were processed and billed again by the provider. Include the agent context's resolved instruction overhead, or synthesize from the same complete prompt-budget snapshot used by context accounting.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed in ecb3c702. messagesForProvider is formed before systemRunnable.pipe(model) and before tool binding, so the estimate missed the system prompt, dynamic instructions, summary and tool schemas — every one of them billed. Now adds resolvedInstructionOverhead ?? instructionTokens, read via the per-node getAgentContext(metadata) rather than the graph-level accessor, which is hardcoded to defaultAgentId and would report the wrong agent in a MultiAgentGraph.
One consequence worth flagging, because the fix opens it: with overhead included the synthetic sample now always clears the pruner's rawSent <= 2*overhead floor, and since it is derived from the same counter that produced the estimate it teaches a calibration ratio of exactly 1.0 — self-consistent by construction, and wrong for any provider whose real ratio differs. So the sealed chunk is marked estimated_usage and updateLastCallUsage skips it. Usage still reaches host billing; it just cannot move the EMA.
The docblock now states that the result is an estimate — host tokenizer, heuristic tool-schema multiplier — and that it over-counts on the fallback path, where tryFallbackProviders builds a bare model with no system-prompt pipe. Only the fallback-plus-seal combination is affected, and over-counting there is safer than the input_tokens: 0 it replaces.
| if (result.injectedMessages.length > 0) { | ||
| try { | ||
| injected.push(...convertInjectedMessages(result.injectedMessages)); |
There was a problem hiding this comment.
Normalize multiple injected turns before resuming
When the boundary returns more than one injectedMessage, or returns both additionalContext and an injected message, this appends consecutive HumanMessage instances and immediately self-loops into the provider. Bedrock and Mistral reject adjacent user turns—the trailing-steer replay code added in this same change explicitly inserts an assistant anchor for that reason—so a normal queue drain containing two steers can make the resumed call fail. Consolidate the boundary payload into one provider-facing user turn, or otherwise normalize role alternation before resuming.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed by execution and fixed in ecb3c702, though not at this call site and not the way proposed.
Ran the exact self-loop array through both vendored converters. Anthropic and Bedrock both emit user, assistant, user, user, user — neither merges plain adjacent user turns, because mergeMessages (message_inputs.ts:1205-1224) and Bedrock's reduce (:1006-1010) each require BOTH neighbours to be all-tool_result. Mistral has no merge step at all.
I did not consolidate the payload into one turn, because per-message identity here is load-bearing: additional_kwargs.source drives steer rendering and keys the trailing-steer anchor in formatAgentMessages, and skillName/isMeta matter downstream. Collapsing N steers would drop all of it.
Fixed instead at the last provider-facing hop — coalesceAdjacentUserTurns, gated to Bedrock/Mistral/MistralAI — so it covers every site that can emit this shape, not just mine: both hook boundaries, a multi-steer drain, and run.ts's pre-stream context push. Non-mutating, so graph state and persisted messages keep per-message identity; only the wire payload is reshaped.
One correction to the severity: this shape is not new. The unchanged PostToolBatch boundary emits the byte-identical sequence on main today (ToolNode.ts:3244-3269), as does run.ts:504-510 — I verified both by execution. What this PR changes is reachability: a two-steer queue drain is routine, whereas the tool boundary needs a hook returning 2+ messages. So the fix lands here, but it repairs a pre-existing path as well.
You were also right that my canSealPreempt docblock overclaimed — it said the gate guarantees "no adjacent user turns", when it only guarantees a non-empty assistant turn before the FIRST injected message. Corrected.
Round 3: all three confirmed, fixed in
|
| Finding | Reported | Actual | Fix |
|---|---|---|---|
| Adjacent user turns after a drain | P1 | P2 — pre-existing shape | coalesceAdjacentUserTurns at the provider-facing hop |
| Sealed input usage omits instruction overhead | P1 | P2 | Add resolved overhead + estimated_usage guard |
| Seal with no boundary hook live | P2 | P3 | hasHookFor in the claim precondition |
The adjacent-turns one was right, and I fixed it somewhere else on purpose. Executed against both vendored converters: a two-steer drain emits user, assistant, user, user, user, and neither converter merges plain adjacent user turns — mergeMessages and Bedrock's reduce each require both neighbours to be all-tool_result. Mistral has no merge step at all.
I did not consolidate the boundary payload as proposed, because per-message identity there is load-bearing: additional_kwargs.source drives steer rendering and keys the trailing-steer anchor, and skillName/isMeta matter downstream. Collapsing N steers into one message would discard all of it. The normalization went to the last provider-facing hop instead, gated to Bedrock/Mistral/MistralAI, so it covers every site that can produce this shape rather than only the new one.
Which matters for severity: this shape is not new. The unchanged PostToolBatch boundary emits the byte-identical sequence on main today, as does run.ts's pre-stream context push — both verified by execution. What this PR changes is reachability, since a two-steer queue drain is routine where the tool boundary needs a hook returning 2+ messages. The fix repairs the pre-existing path too.
The usage fix opened a channel it had to close. Adding instruction overhead means the synthetic sample always clears the pruner's floor, and being derived from the same counter that produced it, it teaches a calibration ratio of exactly 1.0 — self-consistent by construction and wrong for any provider whose real ratio differs. The sealed chunk is now marked estimated_usage and updateLastCallUsage skips it: usage still reaches host billing, it just cannot move the EMA.
I also corrected a docblock of my own that this round exposed. canSealPreempt claimed the gate guarantees "no adjacent user turns"; it guarantees a non-empty assistant turn before the first injected message and says nothing about adjacency among several.
Live verified on bedrock and anthropic — one seal, no empty boundaries, two model-end events with usage, zero runs left open. 614 unit tests pass, 10 of them new for the alternation pass.
Standing caveats, unchanged
- The parallel-agent seal race fix is analysis-only — no fan-out
MultiAgentGraphhas been run with preemption armed. - No CI coverage of the seal → boundary → self-loop path; it is verified only by a live-credential probe, because jest cannot load any suite importing
src/llm/providers. Pre-existing, reproduces onmain, wants its own PR. formatAgentMessagesand now the Bedrock/Mistral alternation pass are the non-opt-in surfaces here — they affect runs with nopreemptionconfigured.
@codex review
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ecb3c702e8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (result.injectedMessages.length > 0) { | ||
| try { | ||
| injected.push(...convertInjectedMessages(result.injectedMessages)); |
There was a problem hiding this comment.
Honor preventContinuation before resuming
When one PreemptBoundary hook returns injected content while another returns preventContinuation: true (or one hook returns both), executeHooks sets the halt signal but this method still returns the injected messages, causing createCallModel to set pendingPreemptReturn and route into another model call. The halt is only polled by the decoupled Run.processStream consumer, so that next call can start before cancellation—and direct graph consumers never poll it at all—despite BaseHookOutput.preventContinuation promising to prevent the next model turn. Check result.preventContinuation here and suppress the self-loop.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed in d83173cb. dispatchPreemptBoundary now returns preventContinuation alongside the messages, and createCallModel commits the sealed turn plus anything injected without self-looping. You are right that the registry halt is too late — processStream polls it between stream events, by which point the call it was meant to prevent has already been issued, and a direct graph consumer never polls it at all. A trailing injected HumanMessage has no tool calls, so toolsCondition routes it to END.
| this.hookRegistry?.hasHookFor( | ||
| 'PreemptBoundary', | ||
| (this.config?.configurable?.run_id as string | undefined) ?? this.runId | ||
| ) === true |
There was a problem hiding this comment.
Require a dispatchable preempt matcher
If the only registered PreemptBoundary matcher has a non-empty pattern (or an empty hooks array), hasHookFor returns true even though executeHooks receives no matchQuery and therefore dispatches nothing. A preemption request then seals the response, the boundary returns empty, and the run marks the answer incomplete—the same no-live-hook truncation this precondition is intended to prevent. Inspect the applicable matchers for at least one wildcard matcher with a callback rather than checking bucket existence alone.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed in d83173cb — this was a hole in the check I added last round, and you named it exactly. hasHookFor answers "is one registered"; what the gate needs is "will one fire", and a matcher carrying a non-empty pattern is inert for a query-less dispatch.
Added HookRegistry.hasDispatchableHookFor, requiring a wildcard pattern AND a non-empty hooks array — mirroring the two skips in executeHooks. Implemented as a non-allocating scan rather than via getMatchers, because this runs once per streamed chunk and getMatchers ends in a defensive slice() that would allocate on every delta.
| sessionId: runId, | ||
| timeoutMs: PREEMPT_BOUNDARY_HOOK_TIMEOUT_MS, |
There was a problem hiding this comment.
Propagate cancellation into the boundary hook
When the run is aborted while a PreemptBoundary drain is pending, this executeHooks call is not given config.signal, so its callbacks receive only the timeout-derived signal and processStream can remain blocked for the full 120-second boundary timeout (or a matcher-specific timeout) while the drain continues after cancellation. The model invocation already places the graph's abort signal on config; pass it as the parent signal here so aborting the run promptly releases the sealed boundary.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Refuted — no change, measured rather than reasoned.
The 120s stall is not reachable: LangGraph races config.signal against every task promise in pregel/runner.js, and measured unblock after abort was 13ms.
The premise is also inverted. config.signal inside createCallModel is LangGraph's COMPOSED signal, not the graph abort signal — it additionally fires when an unrelated sibling node in the same superstep throws, and its forwarding listeners are disposed at end of tick. Passing it would bind the drain to failures that have nothing to do with cancellation.
If we want post-cancel tidiness later — stopping a drain and its state writes from landing after the run died — the correct parameter is this.signal, the host's own AbortSignal, as one line in the options object. I have left it out deliberately: it changes hook-cancellation semantics for hosts whose drain has already popped its queue, which is exactly why the boundary timeout is generous in the first place. Happy to add it if you would rather have the tidiness.
| if ( | ||
| context?.shouldPreemptStream() === true && | ||
| canSealPreempt(finalChunk) && | ||
| context.claimPreemptSeal() | ||
| ) { |
There was a problem hiding this comment.
Disable preemption while Bedrock guardrails are active
For Bedrock runs configured with guardrailConfig, this seals immediately after a visible text chunk and cancels the stream before the final messageStop event. The Bedrock adapter deliberately suppresses its analogous eager tool-use seal in this configuration because a guardrail can return guardrail_intervened only at messageStop after content has already streamed (src/llm/bedrock/streamSealDispatch.test.ts:115-130); cooperative sealing bypasses that safeguard and can commit and resume from content the configured guardrail would have rejected. Gate preemption off whenever the serving Bedrock model has guardrails enabled.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Refuted, and I went looking hard because a guardrail bypass would be the most serious thing found on this PR.
The suppression at bedrock/index.ts:398-399 gates the streamed tool-call seal (lc_streamed_tool_call_seal), whose only consumer is eager tool execution in stream.ts. It is a "do not START a tool early" safeguard; it has nothing to do with text content.
A cooperative seal provably cannot coexist with an in-flight tool call: canSealPreempt refuses any chunk carrying tool_calls, tool_call_chunks or invalid_tool_calls, and concat accumulation makes that permanent for the rest of the turn. Verified by driving the real CustomChatBedrockConverse._streamResponseChunks with a stubbed client through text deltas, a toolUse block, and a messageStop of guardrail_intervened: canSealPreempt is true only for the pre-toolUse text chunks and false from contentBlockStart.toolUse onward, with and without guardrails. So a seal in the text window aborts before any toolUse exists, and a seal after it is refused.
There is also no retraction to bypass. guardrail_intervened appears in exactly two places in this repo, both prose comments; no code here or in @langchain/aws reads it. On the non-preempted path an intervened messageStop becomes an empty chunk nobody inspects, and the already-streamed text stays in state — identical to what a sealed turn commits.
Gating on guardrailConfig would disable preemption for every guardrailed Bedrock agent — a common enterprise default — for zero safety gain, and could not read the config soundly anyway: by attemptInvoke the model is a RunnableBinding, so model.guardrailConfig is undefined and the check would silently fail open.
The one real residue is cosmetic: a sealed turn ends before messageStop, so its final chunk lacks that metadata. Nothing consumes it. If the team wants genuine guardrail handling, the gap is on the ORDINARY path — no code surfaces an intervention today — and that deserves its own issue rather than a gate here.
Round 4: 2 fixed in
|
| Finding | Reported | Verdict |
|---|---|---|
| Disable preemption under Bedrock guardrails | P1 | refuted — wrong seal |
preventContinuation ignored at the boundary |
P2 | confirmed, fixed |
| Pass abort signal into the boundary drain | P2 | refuted — stall not reachable |
hasHookFor insufficient |
P3 | confirmed, fixed |
The P1 does not hold, and I checked it hardest because a guardrail bypass would have been the most serious thing found here. The suppression at bedrock/index.ts:398 gates the streamed tool-call seal — lc_streamed_tool_call_seal, consumed only by eager tool execution. It is a "do not start a tool early" safeguard and has nothing to do with text.
A cooperative seal provably cannot coexist with an in-flight tool call: canSealPreempt refuses any chunk carrying tool_calls / tool_call_chunks / invalid_tool_calls, and concat accumulation makes that permanent for the turn. Verified by driving the real _streamResponseChunks with a stubbed client through text deltas, a toolUse block, and a guardrail_intervened messageStop: sealable only in the pre-toolUse text window, refused from contentBlockStart.toolUse onward, with and without guardrails.
There is also no retraction to bypass — guardrail_intervened appears twice in this repo, both prose comments, and no code here or in @langchain/aws reads it. A sealed turn commits exactly what a non-sealed guardrailed turn commits. Gating on guardrailConfig would disable preemption for every guardrailed Bedrock agent for zero safety gain, and could not read the config soundly anyway — the model is a RunnableBinding by then, so the check would fail open.
The hasHookFor finding was a hole in last round's fix. It asked "is a matcher registered" when the gate needs "will one fire" — a pattern-scoped matcher is inert for a query-less dispatch. Now hasDispatchableHookFor, requiring a wildcard pattern and a non-empty hooks array, implemented as a non-allocating scan because it runs once per streamed chunk.
On the abort signal — measured, not argued: LangGraph races config.signal against every task promise, and unblock after abort was 13ms. The premise is also inverted, since config.signal inside createCallModel is LangGraph's composed signal, which fires when an unrelated sibling node throws. If post-cancel tidiness is wanted the correct parameter is this.signal; I left it out deliberately because it changes cancellation semantics for hosts whose drain has already popped its queue.
Live verified on anthropic and bedrock — one seal, no empty boundaries, two model-end events with usage, zero runs left open. 619 unit tests pass, 5 new.
Standing caveats, unchanged
- The parallel-agent seal race fix remains analysis-only.
- No CI coverage of the seal → boundary → self-loop path, blocked on the pre-existing jest/ESM issue.
formatAgentMessagesand the Bedrock/Mistral alternation pass are the non-opt-in surfaces.
@codex review
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d83173cb70
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| context?.shouldPreemptStream() === true && | ||
| canSealPreempt(finalChunk) && | ||
| context.claimPreemptSeal() |
There was a problem hiding this comment.
Disable sealing for every registered stream handler
When customHandlers[CHAT_MODEL_STREAM] is a valid handler that is not directly a ChatModelStreamHandler—notably a wrapper returned by composeEventHandlers—getRegisteredDefaultChatStreamHandler() returns undefined, so this branch still seals synchronously while the custom handler receives chunks through the lagging Run.processStream() event loop. The PreemptBoundary hook can consequently drain and inject steering before the host has processed the sealed chunk, violating the content-part ordering that this branch is intended to protect; gate sealing on the presence of any registered CHAT_MODEL_STREAM handler rather than on its concrete class.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Partially confirmed — the harm you name does not occur, but you found a real defect underneath it, and it is mine. Fixed in 13cdd63b.
What is right. StreamPreemption documents that a host registering its own CHAT_MODEL_STREAM handler opts out of sealing. The gate implemented that as instanceof ChatModelStreamHandler, which is a different test. And no composeEventHandlers call is even needed to break it: createRunHandlers (src/session/handlers.ts) installs a plain-object CHAT_MODEL_STREAM wrapper on every AgentSession run and merges it last, so a host that registered new ChatModelStreamHandler() specifically to opt out had that silently revoked. First-party and unconditional.
Fixed by replacing identity with a capability brand — SDK_STREAM_DISPATCH, a registered symbol carried by ChatModelStreamHandler and propagated by both wrappers, read via dispatchesChatModelStream().
What I did not do, deliberately. "Disable sealing for every registered stream handler" would disable preemption for every AgentSession run, since createRunHandlers always registers — and for LibreChat's OpenAIChatModelStreamHandler, which is an explicit no-op that renders nothing. Presence is not the hazard; being or wrapping the dispatcher is.
What remains refuted. The ordering harm does not happen. The sealable branch constructs its own ChatModelStreamHandler and awaits it per chunk before polling the seal, and that handler reaches the host through awaited direct registry dispatch (Graph.dispatchRunStep / dispatchMessageDelta), not streamEvents. Probing inside the host's shouldPreempt callback — the exact moment the seal is decided — host-visible text equalled the sealed content exactly. A lagging consumer draining afterwards produced newRunStepsAfterSeal: 0, so there is no index for an injection to invert.
Unrelated bug found while checking this, worth its own issue: composing two ChatModelStreamHandlers duplicates every streamed token (AABBCC... instead of ABC...), with no preemption configured at all. Pre-existing, out of scope here.
Round 5: partially confirmed, fixed in
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 13cdd63b70
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const { messages: injected, preventContinuation } = | ||
| await this.dispatchPreemptBoundary(agentId, config); |
There was a problem hiding this comment.
Commit the sealed turn before firing boundary hooks
When a PreemptBoundary hook calls run.getRunMessages() or otherwise inspects graph/checkpoint state to decide what to inject, it cannot observe the sealed assistant turn: this dispatch occurs inside createCallModel before the node returns result.messages, while the outer messages reducer is the only code that writes this.messages. This contradicts the hook's documented ordering contract and can make persistence or steering decisions use stale history; commit the sealed result before dispatching the boundary, or expose the sealed message directly in the hook input.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed in 2361877d — by correcting the claim, not the dispatch.
Verified by execution: a PreemptBoundary hook calling getRunMessages() at boundary time observes [], while the sealed turn appears only after the run. The cause is exactly as you describe — dispatch happens inside the agent node, and only the outer graph reducer writes StandardGraph.messages, which cannot run until the node returns.
The contradicted promise is mine: PreemptBoundaryHookInput JSDoc said the event "fires AFTER the sealed assistant turn is committed to graph state". That sentence was wrong from the commit that introduced it.
I did not change the ordering, for two reasons. PostToolBatch has the identical property — verified — so a tool-boundary hook cannot see the assistant turn that requested the tool either; the behavior is a property of the single-node outer graph, consistent SDK-wide, and only my doc claimed otherwise. And committing state first would mean returning from the node and re-entering, which is precisely what the self-loop design avoids, to buy an observability guarantee no other boundary offers.
The JSDoc now states the limitation, names the cause, cites the PostToolBatch precedent, and tells hosts to decide from their own queue rather than graph state.
| (handler as unknown as Record<symbol, unknown>)[SDK_STREAM_DISPATCH] === | ||
| true |
There was a problem hiding this comment.
Remove the double assertion from dispatch detection
The new capability check bypasses the EventHandler type with the explicitly prohibited as unknown as ... pattern and an opaque Record<symbol, unknown>. This weakens the type contract at the exact point used to decide whether sealing is safe; read the symbol without a cast, for example with Reflect.get(handler, SDK_STREAM_DISPATCH) === true, or introduce a properly typed branded-handler interface.
AGENTS.md reference: AGENTS.md:L46-L50
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed in 0fadbb23. You are right on both counts — it is the prohibited as unknown as pattern, and it sits at the point that decides whether sealing is safe. Now Reflect.get(handler, SDK_STREAM_DISPATCH) === true, no cast. I introduced this one commit earlier while fixing round 5; AGENTS.md forbids it explicitly and I should have caught it before pushing.
| * - Sealing is only honored on the SDK's own dispatch loop. A host that | ||
| * registers its own `CHAT_MODEL_STREAM` handler consumes chunks through | ||
| * a decoupled `streamEvents` reader that can lag the accumulated chunk, | ||
| * which would invert content-part indices; those runs never seal. |
There was a problem hiding this comment.
Document the capability-based sealing opt-out
When a host registers a plain or no-op CHAT_MODEL_STREAM handler and relies on this documented opt-out, getRegisteredDefaultChatStreamHandler still returns undefined unless the handler carries SDK_STREAM_DISPATCH, so the run remains sealable. This is the opposite of the public precondition stated here and can make hosts unexpectedly preempt a stream; narrow the contract to handlers that are or wrap the SDK dispatcher, or restore the documented presence-based behavior.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed in 0fadbb23. The doc was describing the rule as it stood BEFORE 13cdd63b — I narrowed the semantics and left the precondition asserting the old one.
Rewritten to state what the code does: detection is by capability, so a handler that merely observes the raw chunk echo stays sealable — it assigns no content-part indices, so there is nothing an injection could invert — while the dispatcher and its wrappers do disable sealing. A host that renders from the raw feed and wants the opt-out can brand its handler with SDK_STREAM_DISPATCH.
Worth noting this is the third time on this branch a doc comment of mine has outlived the code it described.
| const recursionLimit = | ||
| (callerConfig.recursionLimit ?? DEFAULT_RECURSION_LIMIT) + | ||
| (this.preemption != null | ||
| ? (this.preemption.maxSeals ?? DEFAULT_MAX_SEALS) | ||
| : 0); |
There was a problem hiding this comment.
Validate maxSeals before deriving recursion headroom
When maxSeals is a non-integer, negative, NaN, or infinite value, it is copied directly into recursionLimit while the seal gate separately interprets it with a numeric comparison. For example, 1.5 permits two seals but supplies fractional recursion headroom, NaN makes the graph's recursion limit invalid, and Infinity removes both advertised bounds. Resolve this option once to a finite nonnegative integer before using it for either the recursion limit or the seal budget.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed in 0fadbb23. The value really was read with two different semantics — a numeric comparison in the seal gate, an addition into recursionLimit in run.ts — so 1.5 permitted two seals while reserving fractional headroom, and NaN poisoned the recursion limit outright.
Both readers now go through resolveMaxSeals: floors fractions, honors 0 as a deliberate never-seal, clamps negatives to 0, and falls back to the default for non-finite input rather than silently switching the feature off. Six unit tests cover the boundary values.
Round 6: all four confirmed, fixed in
|
| Finding | Fix |
|---|---|
| Prohibited cast in dispatch detection (P1) | Reflect.get, no cast |
| Opt-out doc contradicts the code (P2) | Doc rewritten to match capability detection |
maxSeals unvalidated (P2) |
Single resolveMaxSeals for both readers |
| Boundary cannot observe the sealed turn (P2) | JSDoc corrected — the claim was wrong, not the dispatch |
Nothing was refuted this round. Three of the four are defects I introduced in the two preceding commits, and the pattern in them is worth naming rather than repeating.
Two were doc claims that outlived their code. The StreamPreemption opt-out precondition still described the pre-13cdd63b rule after I narrowed it. And PreemptBoundaryHookInput claimed the event "fires AFTER the sealed assistant turn is committed to graph state" — verified false by execution: a hook calling getRunMessages() at boundary time observes [].
That is now the fourth doc overclaim on this branch, after canSealPreempt promising "no adjacent user turns" and my "unfixable without an upstream seam". Same failure each time: I write the strong version of a claim, the code moves, and I do not re-check the sentence.
On the state-visibility one I fixed the claim, not the ordering. PostToolBatch has the identical property — verified — so a tool-boundary hook cannot see the assistant turn that requested the tool either. The behavior is a consequence of the single-node outer graph and is consistent SDK-wide; what differed is that PostToolBatchHookInput never promised otherwise. Committing state before dispatch would mean returning from the node and re-entering, which is exactly what the self-loop avoids, in exchange for an observability guarantee no other boundary offers. The JSDoc now states the limitation, names the cause, cites the precedent, and points hosts at their own queue.
The as unknown as cast was a straight violation of AGENTS.md at the point that decides whether sealing is safe, introduced one commit earlier. maxSeals genuinely was read with two different semantics — 1.5 allowed two seals while reserving fractional recursion headroom, NaN poisoned the limit.
Live verified on anthropic. 631 unit tests pass, 12 new this round.
Standing caveats, unchanged
- The parallel-agent seal race fix remains analysis-only.
- No CI coverage of the seal → boundary → self-loop path, blocked on the pre-existing jest/ESM issue. Verification this round had to stub
@langchain/mistralaiin a throwaway config to execute the graph at all. formatAgentMessagesand the Bedrock/Mistral alternation pass are the non-opt-in surfaces.
@codex review
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2361877d16
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const result = await executeHooks({ | ||
| registry: this.hookRegistry, | ||
| input: { |
There was a problem hiding this comment.
Avoid cancelling the stream before the sealed turn commits
When a PreemptBoundary hook returns preventContinuation, this executeHooks call also installs a registry halt signal. Run.processStream observes that signal on the next streamed event and breaks its for await loop, whose iterator cancellation aborts the graph. Because this hook runs before the agent node returns and before the outer reducer commits its messages, the cancellation can discard the sealed assistant turn and injected messages despite the local branch at createCallModel avoiding a self-loop. Handle this boundary's stop without leaving a halt signal that cancels the event stream, or let processStream drain through the natural END.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed by execution and fixed in 041d6847 — you raised this three ways and all three were right.
executeHooks raises a registry halt whenever a hook returns preventContinuation. The branch I added in f36a624d honored the stop locally but never cleared that halt, so Run.processStream observed it on the next stream event and broke its loop; the break cancels the iterator and aborts Pregel before the outer reducer commits StandardGraph.messages.
Reproduced deterministically, 4/4, driving the real compiled graph: the sealed assistant turn was absent from graph state, from getRunMessages(), and from the content processStream returns. The branch written to preserve the partial answer destroyed it in exactly the case it was written for.
Cleared at the site that raised it, scoped to getHaltSignal(runId)?.source === 'PreemptBoundary' so a halt raised earlier in the same run by a different hook is untouched (haltRun is first-write-wins). Nothing is lost by clearing: the local branch already declines to self-loop, and a sealed chunk provably carries no tool calls, so the turn routes to END after exactly one model call either way — verified in the same harness.
| if (strictAlternationProviders.has(agentContext.provider)) { | ||
| finalMessages = coalesceAdjacentUserTurns(finalMessages); |
There was a problem hiding this comment.
Normalize messages for the actual fallback provider
This alternation pass is selected using only the primary agentContext.provider, but tryFallbackProviders later sends the same finalMessages to each fallback. If an OpenAI/Anthropic primary fails after a boundary injected multiple human turns and a Bedrock or Mistral fallback is configured, the pass is skipped and the strict fallback receives consecutive user messages, causing the recovery request itself to be rejected. Apply provider-specific normalization when each serving provider is chosen, including inside the fallback loop.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed in 041d6847. The normalization was keyed on the primary agentContext.provider, but tryFallbackProviders re-sends the same array — so an OpenAI/Anthropic/Google primary failing after a boundary emitted consecutive human turns handed a Bedrock or Mistral fallback exactly the shape those APIs reject, and the recovery request failed for a reason unrelated to the original failure.
Now also applied inside attemptInvoke, keyed on its own provider argument. That is the documented single funnel for primary, fallback and summarization calls, so one site covers all three. The existing createCallModel call stays — coalescing is idempotent, so the primary just re-runs a no-op.
| if (result.injectedMessages.length > 0) { | ||
| try { | ||
| injected.push(...convertInjectedMessages(result.injectedMessages)); |
There was a problem hiding this comment.
Reject empty boundary messages before self-looping
If a PreemptBoundary hook returns an injectedMessages entry with content: '' or content: []—both shapes are allowed by the public InjectedMessage type—this conversion still produces a HumanMessage, so injected.length > 0 causes the agent to self-loop. The resumed provider request then ends with an empty user turn, which strict APIs can reject, and even permissive providers receive an extra model call with no steering instruction. Filter or reject content-empty entries so a boundary containing no usable message follows the existing empty-boundary stop path.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed in 041d6847. The public InjectedMessage type does admit content: '' and [], and convertInjectedMessages produced a HumanMessage for them, so the length > 0 test passed and the graph self-looped into a trailing empty user turn.
Filtered in the shared converter rather than at the boundary, so the tool boundary gets the same protection. Whitespace counts as empty — matching the standard canSealPreempt already applies to the assistant side of the same pair — and non-text blocks count as content, so a media-only steer still survives. When every entry is empty the boundary now reads as empty, so the run marks preemptIncomplete instead of resuming.
| if ((chunk.tool_calls?.length ?? 0) > 0) { | ||
| return false; | ||
| } | ||
| if ((chunk.tool_call_chunks?.length ?? 0) > 0) { |
There was a problem hiding this comment.
Drain queued steers after Anthropic server tools finish
Once an Anthropic server_tool_use block contributes a tool_call_chunk, concat retains that chunk for the rest of the turn, so this gate keeps canSealPreempt false even after the provider has emitted the matching web_search_tool_result and resumed ordinary text generation. Unlike regular tools, provider-side tools never execute through ToolNode, so no PostToolBatch boundary exists to drain the queued message; the steer is therefore ignored for the remainder of this run rather than waiting for the claimed ordinary boundary. Recognize completed server-tool pairs, as the Google path already does, or dispatch an injection boundary when the provider-side result completes.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed — settledServerToolCallIds treats an Anthropic server-tool call whose paired web_search_tool_result block has landed as settled, so the gate reopens once the provider answers (src/llm/preempt.ts, countOpenToolCalls ignores settled ids). The doc block on canSealPreempt records the reasoning; covered in preempt.test.ts.
2361877 to
a4a33ff
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a4a33ff869
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| this.preemptIncomplete = true; | ||
| this.cleanupSignalListener(); | ||
| return injected.length > 0 | ||
| ? { messages: [...(result.messages ?? []), ...injected] } | ||
| : result; |
There was a problem hiding this comment.
Clear the halt before committing the sealed turn
When a PreemptBoundary hook returns preventContinuation: true, executeHooks also records a registry halt signal. As this node returns, the next non-custom stream event makes Run.processStream observe that signal and break at run.ts:969-973; that loop explicitly documents that breaking calls the iterator's return() and aborts Pregel rather than draining it. Consequently this return can cancel the subgraph before its reducers commit the sealed assistant turn and injected messages, defeating the branch's preservation guarantee. Consume or clear the boundary's halt signal after handling the flag locally so the graph can reach END normally.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed by execution and fixed in 041d6847 — you raised this three ways and all three were right.
executeHooks raises a registry halt whenever a hook returns preventContinuation. The branch I added in f36a624d honored the stop locally but never cleared that halt, so Run.processStream observed it on the next stream event and broke its loop; the break cancels the iterator and aborts Pregel before the outer reducer commits StandardGraph.messages.
Reproduced deterministically, 4/4, driving the real compiled graph: the sealed assistant turn was absent from graph state, from getRunMessages(), and from the content processStream returns. The branch written to preserve the partial answer destroyed it in exactly the case it was written for.
Cleared at the site that raised it, scoped to getHaltSignal(runId)?.source === 'PreemptBoundary' so a halt raised earlier in the same run by a different hook is untouched (haltRun is first-write-wins). Nothing is lost by clearing: the local branch already declines to self-loop, and a sealed chunk provably carries no tool calls, so the turn routes to END after exactly one model call either way — verified in the same harness.
| if (i < payload.length - 1 && endsWithSteerMessage(formattedMessages)) { | ||
| formattedMessages.push( | ||
| withMessageRole( | ||
| new AIMessage({ content: STEER_ANCHOR_PLACEHOLDER }), | ||
| 'assistant' |
There was a problem hiding this comment.
Avoid injecting a fake assistant reply into replay history
When a persisted assistant message ends in a steer and another user turn follows, this unconditionally inserts an assistant message containing _ for every provider. The stated reason is Bedrock/Mistral alternation, but Graph.ts:2225-2227 now coalesces adjacent human turns specifically for those strict providers, while Anthropic, OpenAI, and Google already accept the adjacency. Consequently every model receives a fabricated assistant reply to the steer, which can make it appear that the steer was already answered and distort the next response. Leave the adjacent turns intact for non-strict providers and rely on the provider-facing coalescing pass, or restrict any anchor to providers that actually require it.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Both halves noted; taking them as one. (a) is real but cosmetic — a one-token _ assistant turn in replay history that no provider rejects, now that coalesceAdjacentUserTurns handles the strict providers. (b) is the substantive one: the i < payload.length - 1 lookahead treats any later payload entry as a following turn, but entries with empty content, or entries dropped by applySummaryBoundary, are skipped without emitting — so a trailing steer followed only by skipped entries gets _ as the final assistant turn, which is a prefill the model may never answer.
Not fixed in 041d6847. The correct fix is a lazy emit — record the intent and flush the anchor immediately before the next push, only when that push is a HumanMessage — which touches three push sites and the indexMapping bookkeeping. I did not want that landing unreviewed alongside a P1 data-loss fix.
Tracking it as the remaining known defect on this PR. If the anchor is better removed entirely now that coalescing exists, that is also on the table and would drop one of the two non-opt-in surfaces here.
| this.hookRegistry?.hasDispatchableHookFor( | ||
| 'PreemptBoundary', | ||
| (this.config?.configurable?.run_id as string | undefined) ?? this.runId | ||
| ) === true |
There was a problem hiding this comment.
Refuse seals when no boundary run ID exists
For direct StandardGraph consumers, StandardGraphInput.runId is optional and the invocation config may also omit configurable.run_id. In that case this check can still approve a seal from a global dispatchable matcher because hasDispatchableHookFor does not require a session ID, but dispatchPreemptBoundary immediately returns an empty result when the same resolved run ID is absent. The requested seal therefore truncates the model answer without ever invoking the registered boundary hook. Include the resolved run ID in the claim preconditions so this configuration falls back to normal, unsealed streaming.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed — canClaimPreemptSeal now resolves the runId with the same rule dispatchPreemptBoundary uses (config.configurable.run_id ?? this.runId) and refuses the seal when it is null, so a direct StandardGraph consumer without a runId can no longer spend budget on a boundary that provably returns nothing (src/graphs/Graph.ts, comment on the resolution).
|
Re-requesting: the four visible findings are from the review of Current HEAD is @codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a4a33ff869
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (preventContinuation) { | ||
| /** | ||
| * A hook halted at the boundary. Commit the sealed turn and anything | ||
| * it injected, but do NOT self-loop: `preventContinuation` promises | ||
| * no further model turn, and the run-loop poll in `processStream` | ||
| * only sees the halt AFTER the next call would already have started | ||
| * — direct graph consumers never poll it at all. A trailing injected | ||
| * HumanMessage carries no tool calls, so `toolsCondition` routes it | ||
| * to END. | ||
| */ | ||
| this.preemptIncomplete = true; |
There was a problem hiding this comment.
Clear the halt signal before returning the sealed turn
When a PreemptBoundary hook returns preventContinuation, executeHooks also raises a registry halt signal, but this branch handles the stop locally without clearing that signal. Under Run.processStream, the next inner-graph event makes the stream loop break and cancel the iterator; because the outer graph reducer is the component that commits StandardGraph.messages, cancellation can occur before the sealed turn and injected messages are written, defeating the feature's content-preservation guarantee. Clear the already-honored halt signal before returning this result, or avoid raising it for this boundary.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed by execution and fixed in 041d6847 — you raised this three ways and all three were right.
executeHooks raises a registry halt whenever a hook returns preventContinuation. The branch I added in f36a624d honored the stop locally but never cleared that halt, so Run.processStream observed it on the next stream event and broke its loop; the break cancels the iterator and aborts Pregel before the outer reducer commits StandardGraph.messages.
Reproduced deterministically, 4/4, driving the real compiled graph: the sealed assistant turn was absent from graph state, from getRunMessages(), and from the content processStream returns. The branch written to preserve the partial answer destroyed it in exactly the case it was written for.
Cleared at the site that raised it, scoped to getHaltSignal(runId)?.source === 'PreemptBoundary' so a halt raised earlier in the same run by a different hook is untouched (haltRun is first-write-wins). Nothing is lost by clearing: the local branch already declines to self-loop, and a sealed chunk provably carries no tool calls, so the turn routes to END after exactly one model call either way — verified in the same harness.
| const runManager = new CallbackManagerForLLMRun( | ||
| llmRunId, | ||
| callbackManager.handlers, | ||
| callbackManager.inheritableHandlers, |
There was a problem hiding this comment.
Close callbacks configured on the model instance
When the chat model was constructed with callbacks through BaseChatModelParams, LangChain merges those callbacks into the real model run internally, but getCallbackManagerForConfig(config) reconstructs a manager from invocation callbacks only. After a cooperative seal skips LangChain's normal handleLLMEnd, model-level handlers therefore receive start/token callbacks but never an end callback, leaving their traces or usage records open. The native close needs the actual run manager or must also preserve the model's configured callbacks.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed, and deliberately not fixed in 041d6847 — reasoning on the record.
The mechanism reproduces: getCallbackManagerForConfig(config) rebuilds from invocation callbacks only, so handlers passed via BaseChatModelParams see start and token callbacks but never an end callback after a seal. Same class of leak as the one dc0a784c fixed, for a different callback source.
Deferred because there is no first-party trigger — neither this SDK nor LibreChat constructs a chat model with instance-level callbacks — and the fix needs unwrapChatModel recursion through RunnableBinding/RunnableSequence to reach model.callbacks, since bindTools and systemRunnable.pipe both wrap it. That is real surface area to add unreviewed next to a P1 data-loss fix. Happy to take it if you would rather have it now.
| sessionId: runId, | ||
| timeoutMs: PREEMPT_BOUNDARY_HOOK_TIMEOUT_MS, |
There was a problem hiding this comment.
Propagate the run abort signal to boundary hooks
If the caller aborts while a PreemptBoundary hook is draining, this invocation does not pass config.signal to executeHooks, so its 120-second timeout race ignores the run cancellation. Because the graph node awaits this call, cancellation can be delayed for the full boundary timeout and the hook may still drain or inject messages after the run was aborted. Pass the active runnable signal into this hook execution.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Taken in 041d6847, with a correction to the mechanism. Passing config.signal would be wrong: inside createCallModel that is LangGraph's composed signal, which also fires when an unrelated sibling node in the same superstep throws. The host's own this.signal is the right parameter, and that is what I used.
The stated symptom does not occur either — LangGraph races the external signal against every task promise, and measured unblock after abort is single-digit milliseconds, not 120 seconds; nothing can be injected post-abort because the abandoned node's return value is never committed. What the change actually buys is that a drain does not keep running after the run it belongs to has died, which is worth having on its own.
| if (strictAlternationProviders.has(agentContext.provider)) { | ||
| finalMessages = coalesceAdjacentUserTurns(finalMessages); |
There was a problem hiding this comment.
Normalize messages for the provider that actually runs
This check uses the primary agent provider before the fallback chain is selected. When an OpenAI, Anthropic, or Google primary fails after a boundary has produced consecutive human turns and the configured fallback is Bedrock or Mistral, tryFallbackProviders sends the uncoalesced messages to that strict provider and it rejects the prompt. Apply this normalization inside attemptInvoke using its actual provider, or repeat it for each fallback invocation.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed in 041d6847. The normalization was keyed on the primary agentContext.provider, but tryFallbackProviders re-sends the same array — so an OpenAI/Anthropic/Google primary failing after a boundary emitted consecutive human turns handed a Bedrock or Mistral fallback exactly the shape those APIs reject, and the recovery request failed for a reason unrelated to the original failure.
Now also applied inside attemptInvoke, keyed on its own provider argument. That is the documented single funnel for primary, fallback and summarization calls, so one site covers all three. The existing createCallModel call stays — coalescing is idempotent, so the primary just re-runs a no-op.
| * fresh one. `endsWithSteerMessage` reads only `additional_kwargs.source`, | ||
| * so the reorder cannot change which messages get anchored. | ||
| */ | ||
| if (i < payload.length - 1 && endsWithSteerMessage(formattedMessages)) { |
There was a problem hiding this comment.
Check for an emitted turn before adding the steer anchor
The index test treats any later payload entry as a following turn, even though entries with empty content arrays or entries removed by applySummaryBoundary are skipped without emitting a message. If a trailing steer is followed only by such skipped entries, this appends _ as the final assistant turn instead of leaving the steer as the final human turn, so the provider receives an assistant prefill and may never answer the steer. Look ahead for a later entry that will actually emit the user turn before inserting the anchor.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Both halves noted; taking them as one. (a) is real but cosmetic — a one-token _ assistant turn in replay history that no provider rejects, now that coalesceAdjacentUserTurns handles the strict providers. (b) is the substantive one: the i < payload.length - 1 lookahead treats any later payload entry as a following turn, but entries with empty content, or entries dropped by applySummaryBoundary, are skipped without emitting — so a trailing steer followed only by skipped entries gets _ as the final assistant turn, which is a prefill the model may never answer.
Not fixed in 041d6847. The correct fix is a lazy emit — record the intent and flush the anchor immediately before the next push, only when that push is a HumanMessage — which touches three push sites and the indexMapping bookkeeping. I did not want that landing unreviewed alongside a P1 data-loss fix.
Tracking it as the remaining known defect on this PR. If the anchor is better removed entirely now that coalescing exists, that is also on the table and would drop one of the two non-opt-in surfaces here.
| ? { messages: [...(result.messages ?? []), ...injected] } | ||
| : result; | ||
| } | ||
| if (injected.length > 0) { |
There was a problem hiding this comment.
Reject empty injected turns before self-looping
The public InjectedMessage type permits content: '' and an empty content array, but this branch treats the number of converted messages as proof that there is a valid turn to resume from. If a boundary drain returns one such empty entry, the graph self-loops and sends a trailing empty HumanMessage to the provider; Anthropic and other providers reject that payload, converting a cooperative seal into a failed run. Filter or validate empty injected content and take the existing empty-boundary path when nothing substantive remains.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed in 041d6847. The public InjectedMessage type does admit content: '' and [], and convertInjectedMessages produced a HumanMessage for them, so the length > 0 test passed and the graph self-looped into a trailing empty user turn.
Filtered in the shared converter rather than at the boundary, so the tool boundary gets the same protection. Whitespace counts as empty — matching the standard canSealPreempt already applies to the assistant side of the same pair — and non-text blocks count as content, so a media-only steer still survives. When every entry is empty the boundary now reads as empty, so the run marks preemptIncomplete instead of resuming.
| const handler = context?.handlerRegistry?.getHandler( | ||
| GraphEvents.CHAT_MODEL_STREAM | ||
| ); | ||
| return handler instanceof ChatModelStreamHandler ? handler : undefined; | ||
| return dispatchesChatModelStream(handler) ? handler : undefined; |
There was a problem hiding this comment.
Do not infer a streamEvents consumer from the handler brand
When a caller runs the public compiled StandardGraph directly rather than through Run.processStream, there is no decoupled streamEvents loop to invoke registered raw-chunk handlers. A composed or session wrapper carrying the new brand is nevertheless returned here, which selects the registered-handler branch and skips the local ChatModelStreamHandler for ordinary chunks; content dispatch disappears and cooperative seals are never polled. The capability check needs to distinguish an actively owned streamEvents dispatch loop from mere handler registration.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed as a mechanism, deliberately not fixed in 041d6847.
The branch really does conflate two decisions — "who dispatches content" and "may this run seal" — and for a caller invoking the compiled StandardGraph directly, with no streamEvents loop, selecting the registered-handler branch drops content dispatch entirely.
Deferred on scope: the identical content loss already exists at HEAD~ and back to #213 for the canonical new ChatModelStreamHandler() registration, so this is the branch's pre-existing behavior rather than something the brand introduced, and it is unreachable through any documented path. The clean fix is to let the consumer declare itself — a streamEventsConsumerActive flag set by Run.processStream — rather than inferring it from the handler. That is worth doing, but as its own change against the pre-existing defect, not folded into this PR.
Rounds 7–8: 6 fixed in
|
| Finding | Fix |
|---|---|
| Boundary halt discards the sealed turn (P1, ×3) | Clear the halt at the site that raised it, scoped by source |
| Empty injected content self-loops | Filter in the shared converter, both boundaries |
| Alternation keyed on primary, not serving, provider | Also applied in attemptInvoke |
| Seal claimed with no resolvable run id | Both sites resolve and require it identically |
| Anthropic server tools block sealing all turn | Settled calls (paired result landed) no longer count as open |
| Boundary drain ignores cancellation | Pass this.signal — not LangGraph's composed config.signal |
Deferred, with reasoning on their threads
Model-instance callbacks (real, but no first-party trigger and needs RunnableBinding unwrapping) and brand-vs-streamEvents-consumer (pre-existing since #213, unreachable via any documented path).
Outstanding
The steer-anchor lookahead — a trailing steer followed only by skipped payload entries gets _ as the final assistant turn. The correct fix is a lazy emit across three push sites plus indexMapping bookkeeping; I did not want that landing unreviewed beside a P1. It is the one known defect left on this PR.
@codex review
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 041d68472d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| * fresh one. `endsWithSteerMessage` reads only `additional_kwargs.source`, | ||
| * so the reorder cannot change which messages get anchored. | ||
| */ | ||
| if (i < payload.length - 1 && endsWithSteerMessage(formattedMessages)) { |
There was a problem hiding this comment.
Delay the steer anchor until a later message is emitted
When a trailing steer is followed only by payload entries that formatting skips, such as empty content arrays, i < payload.length - 1 still inserts the _ assistant anchor even though no subsequent user message is emitted. The formatted history then ends with this synthetic assistant turn rather than the steer, so the next provider call sees a placeholder assistant prefill with no user request; determine whether a later entry actually emits before adding the anchor.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed — the lookahead is gone. The anchor intent is recorded as pendingSteerAnchor and flushed only when a later entry actually EMITS a message; entries skipped by formatting (empty content, summary-boundary drops) leave the steer as the final turn. Further hardened in 6f234bc: the flush is also suppressed when the next emitted message is assistant-role (it IS the separation), and guarded against zero-emission assistant entries. Covered by 'omits the anchor when no later payload entry emits a message' and 'suppresses the anchor when the next payload entry is an assistant turn'.
| * short. Same system-flavored `HumanMessage` convention `ToolNode` uses — | ||
| * Anthropic and Google reject a mid-conversation `SystemMessage`. | ||
| */ | ||
| if (result.additionalContexts.length > 0) { |
There was a problem hiding this comment.
Filter empty hook contexts before resuming
When a PreemptBoundary hook returns only whitespace in additionalContext, executeHooks retains it because its raw length is nonzero, so this branch creates a whitespace-only HumanMessage and the caller treats the boundary as nonempty. That bypasses the whitespace filtering applied to injectedMessages, causing a needless self-loop and potentially sending providers an effectively empty user turn instead of stopping as an empty boundary; filter or trim these contexts before deciding to resume.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed — whitespace-only additionalContexts are filtered at the boundary before the emptiness test (src/graphs/Graph.ts, contexts = result.additionalContexts.filter((c) => c.trim() !== '')), with the doc block noting they'd otherwise read as something to resume with, cost a model call, and 400 on strict providers.
| * already returns control in milliseconds without this; what it buys is | ||
| * that a drain does not keep running after the run it belongs to died. | ||
| */ | ||
| signal: this.signal, |
There was a problem hiding this comment.
Forward the caller abort signal to boundary hooks
When cancellation is supplied through processStream's callerConfig.signal, this uses only the graph-construction signal and therefore does not observe that abort; the problem is unavoidable for Run multi-agent configs because createMultiAgentGraph never supplies a signal and MultiAgentGraphConfig exposes none. A boundary hook can consequently continue draining or mutating the host queue after the run has been canceled, until it completes or reaches the 120-second timeout, so preserve and pass the caller's original signal separately from LangGraph's composed node signal.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Partially confirmed — fixed in 1914e63. Two corrections to the mechanism first: processStream already adopted the caller's signal for the case you describe (run.ts:751 in the reviewed commit) — a multi-agent run has no construction signal, so graph.signal == null and the per-call signal WAS observed at the boundary.
But the adopt-if-null rule had two residual defects your finding surfaced, both now fixed:
- Both-signals: with a construction signal present, the caller's per-call controller was ignored entirely — your scenario, for the single-agent shape.
- Staleness (worse than flagged): the adoption wrote into
graph.signal, which has other consumers (model-call config atcreateCallModel, subagentparentSignal) and is not cleared on HITL interrupts — a host aborting a finished request's controller would poison the resumed run's model calls, not just boundary drains, with an already-aborted signal.
Fix: the caller's signal now lives on its own per-call field (Graph.callerSignal), assigned unconditionally — including back to undefined — on every processStream call, and dispatchPreemptBoundary composes the channels with AbortSignal.any only when both exist and differ. graph.signal is back to pure construction state. Covered by Graph.preemptSignal.test.ts (both-signals, caller-only, construction-only, clearHeavyState).
| result.preventContinuation === true && | ||
| this.hookRegistry.getHaltSignal(runId)?.source === 'PreemptBoundary' | ||
| ) { | ||
| this.hookRegistry.clearHaltSignal(runId); |
There was a problem hiding this comment.
Preserve the halt reason when clearing the boundary signal
When a PreemptBoundary hook returns preventContinuation: true, clearing its registry signal here also discards the only copy of its stopReason. The graph stops locally, but Run.processStream can no longer populate _haltedReason, so Run.getHaltReason() returns undefined and AgentSession records/emits run.completed for an unfinished hook-halted turn; preserve the reason in non-canceling run state while suppressing only the iterator-breaking signal.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Refuted — the reason survives, by design. The reviewed commit already carries it across: this.preemptHaltReason = halt.reason is captured before clearHaltSignal (the line this thread anchors on), and processStream transfers it into _haltedReason at run.ts:1039-1041 — deliberately AFTER the Stop dispatch, so the host still gets a completion signal to persist the sealed partial turn with.
The claimed consequence doesn't occur: Run.getHaltReason() returns the hook's stopReason, and AgentSession checks it at AgentSession.ts:1041-1049 — a hook-halted sealed turn emits run.halted with the reason, not run.completed.
What IS intentional: the Stop hook fires for this path (with _haltedReason still unset at that point). That's the documented contract from the round-9 fix — the seal exists to preserve the partial answer, and a host that persists on Stop needs the event; stopReason: 'preempt_incomplete' distinguishes it from a natural finish. Suppressing the registry signal while keeping the reason is exactly the "preserve in non-canceling run state" split this finding asks for.
Adds a fourth mid-run injection semantic: seal the live model stream at a
provably-safe token boundary, dispatch a `PreemptBoundary` hook, and
self-loop the agent node inside the same pregel run. No abort, no job
status change, no checkpointer, no second `processStream` pass.
Today the only mid-run injection seam is `PostToolBatch`, which fires only
when a batch produced tool entries. During a long text answer there is no
boundary at all, so a host cannot land a steer without tearing the run down
and discarding its work.
The seal gate (`canSealPreempt`) refuses to seal unless the ACCUMULATED
chunk already carries non-whitespace text and has no tool call, no partial
tool call, no invalid tool call and no unpaired `server_tool_use`. Chunks
accumulate monotonically through `concat`, so an unsafe shape can never be
observed at a seal point. Nothing is stripped and nothing is repaired — an
unsafe shape simply doesn't seal and the injection falls through to the
existing tool boundary. That also closes the eager-prestart hazard: a
design that stripped tool calls from a sealed chunk would re-run an
already-executed tool.
Only the SDK's own dispatch-synchronous loop may seal. The registered
stream-handler loop is excluded by construction because it dispatches
through a decoupled `streamEvents` consumer that can lag the accumulated
chunk and invert host content-part indices.
`dispatchSealedModelEnd` is required rather than cosmetic: LangChain's
`_streamIterator` calls `handleLLMEnd` after its try/catch with no
`finally`, so breaking the consumer loop fires neither `handleLLMError`
nor `handleLLMEnd`. Without the synthetic event, every seal would spend an
unrecorded full prompt on resume.
Also lifts `ToolNode.convertInjectedMessages` into `@/messages/injected` so
both injection sites emit byte-identical `HumanMessage` shapes — the entire
provider-safety argument rests on that parity.
Two pre-existing bugs fixed in passing:
- `formatAgentMessages` left a trailing STEER part replaying as a bare
`HumanMessage`, so the next turn's user message produced two adjacent
user turns — a hard error on Bedrock and Mistral. Reachable today
through abort, not only through preemption.
- The run loop's halt-signal JSDoc claimed the in-flight step finishes
before halting. It does not: leaving the `for await` cancels the
reader, which langgraph's stream wrapper turns into an abort.
`interruptAndSend` semantics, the abort path, `RunControl`, `keepContent`,
ToolNode execution and every provider converter are untouched. Subagents
never seal — a steer targets the top-level conversation.
Adds `src/scripts/preempt-probe.ts`, a live-credential probe that exercises the whole seal -> PreemptBoundary -> inject -> self-loop path against a real provider and asserts the resulting message sequence, and documents the `tokenCounter` precondition it surfaced. Verified passing on anthropic, bedrock, vertexai, openAI, azureOpenAI, openrouter and deepseek: one seal, no empty boundaries, sequence [ai, human(steer), ai], generation resumed in the same run and answered the injected steer, two CHAT_MODEL_END events carrying usage, and no provider error — notably none of the message-ordering, empty-content or adjacent-user- turn 400s the seal gate exists to prevent. The probe surfaced one real contract requirement, now on `StreamPreemption`: a sealed turn ends before most providers send their usage chunk, so the synthetic CHAT_MODEL_END depends on `RunConfig.tokenCounter` to report `output_tokens`. Without one the fallback silently no-ops. Confirmed live — OpenAI, Azure and DeepSeek report no usage for the sealed segment without a counter; Anthropic streams usage incrementally and reports it either way.
Seven of eight findings survived verification against the code; two of them were right about the outcome but wrong about the cause, so the fixes differ from what was proposed. **Atomic seal claim (P1).** `shouldPreemptStream()` was a bare predicate and `notePreemptSeal()` counted only afterwards, so in a fan-out `MultiAgentGraph` — where every parallel agent shares one graph instance — several streams could each seal for a single queued host request. One boundary drained it; the rest took the nothing-to-inject path and cut their answers short, and the budget could be overrun. Adds `claimPreemptSeal()`, which validates and mutates in one synchronous body, plus a `preemptSealInFlight` gate so only one stream at a time can hold a seal. Merely making the counter atomic is not sufficient: with maxSeals=8 three agents still claim distinct slots and two still get empty boundaries. **Sealed usage under-reported prompt tokens (P1).** The synthetic usage fabricated `input_tokens: 0`, but the provider processed and billed the whole prompt, and every resume re-sends it. Now estimates both sides from the host's counter over `messagesForProvider`, the exact array handed to the provider. **Steer anchor destroyed the steer (P1, pre-existing root cause).** The empty anchor was pushed before the `sourceMessageId` stamping loop, so it inherited the shared id — and `messagesStateReducer` treats a repeated id as replace-in-place, making the anchor the sole survivor of the group. Fixed by pushing after the stamping loop so it reaches the reducer unstamped. Verified by execution. The underlying collision is broader and predates this PR: any consumer that supplies `messageId` already collapses `AI(tool_calls)+Tool` to an orphan tool result. That deserves its own issue and is not fixed here. **Stats no longer erased by cleanup (P2).** `clearHeavyState()` runs in `processStream`'s `finally`, so resetting the counters there made `getPreemptStats()` always report zeros. Splits the per-turn seal budget from the lifetime totals: the budget clears on both reset paths, the reported totals only on a genuinely new run. **Empty boundary now reaches the host (P2).** `preemptIncomplete` was write- only. `Stop` still fires — the run did stop — but now carries `stopReason: 'preempt_incomplete'` so a host can tell a cut-short answer from a finished one. **PreemptBoundary `additionalContext` no longer dropped (P2).** The output type is `BaseHookOutput`, so context is part of the contract. It is now materialized into the same system-flavored `HumanMessage` the tool boundary uses, before the emptiness test — a hook returning context alone previously read as "nothing to resume with" and truncated the answer. **Dead server_tool_use check removed (P2, cause was wrong).** The reviewer proposed id-pairing inside `hasServerToolUseBlock`. That would be a no-op: Anthropic emits a `tool_call_chunk` for every `server_tool_use`, which `concat` retains, so the tool-call gate already covers it and the content check never decided anything. Removed the dead code and documented the real invariant — a turn that starts a web search is not preemptible afterwards. Tests rewritten around accumulated chunks that carry the tool-call chunk. **Not fixed: unclosed native model run (P2, confirmed but unreachable).** Ending the real run needs its id. `model` here is a bound runnable, so the binding consumes `config.runId` for its own run and `patchConfig` hands the chat model a fresh one. Pinning an id and reconstructing a `CallbackManagerForLLMRun` was implemented and tested live: it fails with "No LLM run to end" and additionally suppresses the host's model-end event, losing usage entirely — strictly worse than an unclosed trace span. Reverted, and the limitation is now documented at the dispatch site. A real fix needs a seam in `@langchain/core`. Live re-verified on anthropic and openAI after every change: one seal, no empty boundaries, [ai, human(steer), ai], two model-end events with usage.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ee46e240e8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const messagesForProvider = strictAlternationProviders.has(provider) | ||
| ? coalesceAdjacentUserTurns(annotated) | ||
| : annotated; |
There was a problem hiding this comment.
Avoid re-coalescing primary provider messages
For a primary Bedrock or Mistral call, createCallModel has already applied coalesceAdjacentUserTurns before passing finalMessages here, but this conditional invokes the helper again. Because the helper always walks the full array and allocates a new result even when the messages are already alternating, every such invocation now makes two O(n) passes over a potentially context-sized history; preserve an already-normalized marker or arrange the origin tracking and measurement around a single funnel pass instead.
AGENTS.md reference: AGENTS.md:L40-L41
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 8c4a41e — kept the two passes (they serve different guarantees: the createCallModel one must precede the cache breakpoint, the funnel one must cover fallback and summarization sends) and removed the cost instead: the helper now returns the input array by identity when nothing merges. The normalized second pass is a walk with zero allocation, and trackProviderMessageOrigins early-exits on before === after, so the redundant invocation is effectively free. Pinned by identity assertions both ways: an already-alternating input comes back toBe-identical, and re-coalescing merged output returns the same array.
Codex round 15: the pass runs twice for a primary Bedrock/Mistral call (createCallModel before the cache breakpoint, then the attemptInvoke funnel for fallback/summarization coverage), and the helper allocated a fresh context-sized array even when the payload was already alternating. The no-merge path now returns the input array itself, so the second pass is a walk with zero allocation, origin tracking early-exits on before === after, and callers can detect 'nothing changed' by identity. Pinned by identity assertions: no-merge input, and re-coalescing merged output.
|
Round 15 addressed in 8c4a41e (identity return on the no-merge path — the deliberate two-pass structure stays, its redundant pass now costs a walk and nothing else). ee46e24's earlier CI failure was the known live-Anthropic summarization flake; the rerun passed on identical code, and the same flake also hit 1914e63 (passed everywhere in between). If it keeps recurring it deserves its own issue — the model occasionally emits a tool call whose replayed input isn't an object, unrelated to this PR. @codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8c4a41ed86
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (result.preventContinuation === true && halt?.source === 'PreemptBoundary') { | ||
| this.preemptHaltReason = halt.reason; | ||
| this.hookRegistry.clearHaltSignal(runId); |
There was a problem hiding this comment.
Stop the outer multi-agent workflow at a halting boundary
When this runs inside a MultiAgentGraph, clearing the registry halt only terminates the sealed agent's inner subgraph; after agentSubgraph.invoke() returns, MultiAgentGraph.agentWrapper can still route to its direct successors, and parallel siblings can continue as well. Thus a PreemptBoundary hook returning preventContinuation: true can still allow additional model turns, tools, and side effects before the halted reason is surfaced. Preserve the sealed commit, but propagate a terminal marker that the outer multi-agent routing honors.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed — fixed in 96c31fa. Verified with two independent adversarial passes; your mechanism holds with one sharpening: the sealed agent itself cannot hand off (a sealed chunk provably carries no tool calls, so the transfer-ToolMessage branch is unreachable) — the leak is direct-edge successors (static addEdge and Command fan-out), fan-in wrappers, and parallel siblings' subsequent inner-loop turns.
The terminal marker you asked for already existed (preemptHaltReason, committed before any successor superstep); what was missing was a consumer. It's now honored at every model node's ENTRY — a guard at the top of createCallModel declines the turn when the reason is set, so the outer workflow drains to END with the sealed commit intact. That site was chosen deliberately: a Command({goto: END}) from the sealed agent's wrapper cannot suppress statically compiled addEdge successors, and an agentWrapper-entry guard would miss parallel siblings already inside their subgraph loop; createCallModel is the one chokepoint all of them pass through. Reset per turn, so the next processStream starts clean; single-agent runs never reach the guard.
E2E detector added: two-agent direct-edge run, halting boundary in agent A — without the guard agent B takes a model turn (invocations: 2, verified by stashing the fix), with it none, and getHaltReason() carries the hook's own reason.
| if ( | ||
| context?.shouldPreemptStream() === true && | ||
| canSealPreempt(finalChunk) && | ||
| context.claimPreemptSeal() |
There was a problem hiding this comment.
Check seal safety before consuming the preempt request
If a host implements shouldPreempt as a one-shot read that clears its pending flag, the current order consumes that request on the first chunk even when canSealPreempt(finalChunk) is false—for example, an initial whitespace/reasoning chunk or an in-flight server-tool chunk. Every later poll then returns false, so the requested steer is never sealed at the next safe boundary. Evaluate the accumulated chunk's safety before invoking the host callback, or explicitly require a level-triggered predicate in the public contract.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed as a contract gap — fixed in 96c31fa, via your second alternative. The reorder would be a regression: canSealPreempt walks the accumulated content (settled-id collection, Google pairing scan, full-string trim), so evaluating it before the poll runs it on EVERY chunk instead of only while a request is pending — O(n²) over a long stream — and the 'cheap poll first, shape check second, budget claim last' order is load-bearing for the per-chunk cost discipline the contract promises. Severity note: the request isn't lost forever even for a consuming host — the queued message still lands at the ordinary tool boundary — but the seal opportunity is silently forfeited, which is exactly the ambiguity worth closing.
The public StreamPreemption contract now states the level-triggered requirement explicitly at both anchors (the preconditions block and the shouldPreempt field doc): keep returning true until the PreemptBoundary drain consumes the request, disarm there. This matches what claimPreemptSeal's internals and the probe already assumed — it was documented everywhere except the one place hosts read.
| messages.push( | ||
| withMessageRole( | ||
| new AIMessage({ content: STEER_ANCHOR_PLACEHOLDER }), | ||
| 'assistant' | ||
| ) |
There was a problem hiding this comment.
Account for synthetic steer anchors in the token map
When formatAgentMessages receives an indexTokenCountMap and a persisted trailing steer is followed by another user turn, this synthetic AIMessage has no original payload index and is never entered into updatedIndexTokenCountMap. Because pruning only fills missing counts from lastTurnStartIndex onward, historical anchors remain permanently costed as zero; conversations with repeated steers progressively understate context usage and can bypass pruning into a provider context-overflow error. Assign a token count to each generated anchor or otherwise include it in the remapping.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Refuted — with the mechanism verified in detail. The factual observation is right: the anchor gets no updatedIndexTokenCountMap entry, and the pruner costs missing indices ?? 0 outside the recount window. But the defect claim doesn't survive quantification or the design context:
- Magnitude: the anchor is a single character (
'_') — ~1 content token plus framing per anchor. Bypassing the pruning reserve (~6,400 tokens on a 128K window) would take on the order of a thousand persisted trailing-steer anchors in one conversation. - Self-correction: anchors ARE sent to the provider, so their real tokens land in
providerInputTokenswhile contributing 0 torawSentThisTurn— the calibration ratio rises and every subsequent budget decision scales by it, absorbing the undercount after the first observed response. And the pre-invokemeasureProviderPayloadrecounts origin-less messages at full raw cost regardless, which is the layer that actually gates the provider call. - Design convention: uncounted synthetic insertions are deliberate and test-asserted in this pipeline — injected skill bodies (potentially thousands of tokens, vs. one character) are excluded from the map by design (
formatAgentMessages.skills.test.tspins it). Counting anchors while skill bodies stay uncounted would be false precision.
Happy to add the nominal map entry if you see a path to overflow that survives the calibration + pre-invoke recount layers, but as verified it's cosmetic.
…ent level-triggered shouldPreempt Codex round 16, verified by adversarial fan-out (2 lenses on the P1): - P1 CONFIRMED: clearing the registry halt (which protects the sealed commit from the stream-cancel) removed the only signal that stopped the OUTER MultiAgentGraph — after the sealed agent's subgraph returned, direct-edge successors, Command fan-out, fan-in wrappers, and parallel siblings' next inner-loop turns all continued, taking model turns and tool side effects before the halt reason surfaced. Fixed with an entry guard in createCallModel: when preemptHaltReason is set, every model node declines its turn and the workflow drains to END with the sealed commit intact. One site covers all routing modes (a Command goto cannot suppress static addEdge successors, and an agentWrapper guard would miss siblings already inside their subgraph loop). E2E detector: two-agent direct-edge run, halting boundary in agent A — agent B takes a model turn (invocations=2) without the guard, none (=1) with it; getHaltReason() carries the hook's reason. - P2 consume-order: CONFIRMED as a contract gap only — the reorder the finding suggests would run canSealPreempt on every chunk (O(n^2) per stream); the poll-first order is load-bearing. Fixed by making the level-triggered (non-consuming) requirement explicit in the public StreamPreemption contract, matching what claimPreemptSeal's internals and the probe already assumed. - P2 anchor-tokenmap: REFUTED — the anchor is one character (~1 token plus framing); missing map entries cost '?? 0' but calibration absorbs the drift after the first observed response, the pre-invoke measurement recounts everything anyway, and uncounted synthetic insertions are the test-asserted design convention (skill bodies, orders of magnitude larger, are deliberately excluded).
|
Round 16 addressed in 96c31fa — the most substantive round since 11, and all three threads carry full verification detail:
@codex review |
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
npx tsx --env-file=<env> src/scripts/preempt-scenarios.ts \ --scenario multiseal|toolturn|budget|cache|ma-halt|ma-continue|ma-baseline|websearch \ --provider <key> [--arm-at N] Verified live (anthropic + bedrock unless noted): - multiseal: two seals in one turn, three balanced model runs - toolturn: gate holds through a tool-call turn — no seal until the post-tool text turn, tool executed exactly once (the eager-rerun hazard, now demonstrated live), steer lands after the tool result - budget: maxSeals=1 with a never-disarming predicate — second seal refused, resumed turn runs to completion - cache: promptCache + seal + steer, no rejections - ma-halt: the createCallModel halt guard live — one model start total, successor never runs, halt reason surfaces - websearch (--arm-at 30): seal lands AFTER a server-side search settles; sealed content [server_tool_use, web_search_tool_result, text...] replays to Anthropic and is accepted (settled-id gate live) - ma-baseline: no-preemption control for the multi-agent flow Known finding (pre-existing, documented in the PR): ma-continue on Anthropic can yield an EMPTY successor turn — direct-edge multi-agent flows hand the previous agent's trailing assistant turn to Claude as a prefill, and a steer's short complete-reading resume gives the continuation nothing to add (baseline critique is itself a continuation; OpenAI unaffected). Preemption raises the likelihood of the trigger shape but the mechanism predates it.
…ithout slicing Codex round 1 on #346, both confirmed: - The cue lived in createCallModel's transform stack, keyed on the agent's PRIMARY provider — the same fallback mistake #335's coalescer already taught: an OpenAI primary falling back to Bedrock-Claude reused the shared payload without the cue (prefill bug reproduced on the fallback), and an Anthropic primary falling back to OpenAI shipped the Claude-only synthetic turn. Moved to the attemptInvoke funnel next to the coalesce, keyed on the provider actually serving the call; the Bedrock model id is read off the serving model instance, defaulting Bedrock to Claude-like when absent (the safe direction for a cue that is inert elsewhere). - getRunMessages() sliced the whole run tail per anthropic-like model call to read one element. New non-allocating Graph.getLastRunMessage() mirrors its disposed-graph fallbacks; appendPredecessorHandoffCue now takes the tail message directly. New funnel-level tests: cue for anthropic and Bedrock-Claude serving calls, absent for Bedrock-Nova and OpenAI. E2E spec and live ma-continue acceptance unchanged and re-verified after the move.
…on Claude (#346) * fix: break the predecessor prefill for bare direct-edge successors on Claude Fixes #345. In a multi-agent direct-edge flow, the successor's payload ends with the predecessor's assistant turn. Claude surfaces (Anthropic, Bedrock-Claude) treat a trailing assistant message as a prefill and CONTINUE it — so a bare-edge successor speaks in its predecessor's voice, or returns empty content when that turn reads complete (a preemption steer's short resume; reproduced live 3/3). Handoff edges with instructions and prompt-instruction edges already break the prefill with a user turn; this closes the same gap for bare edges. Mechanics: appendPredecessorHandoffCue runs as the last provider message transform for isAnthropicLike targets, appending a bracketed user-turn cue when the payload's trailing assistant turn is run-produced — matched by id against getRunMessages()'s tail, which an agent's own self-loops never present (they re-enter on tool results or injected user turns) and host-supplied trailing assistant turns never match (deliberate single-agent prefill flows stay untouched). Wire-only: graph state and host-visible messages carry no cue; marked isMeta so the Bedrock cache anchor does not pin it. Verified live: scenario ma-continue/anthropic flips from an empty critic (3/3 before) to a real critique in the critic's own voice; ma-baseline improves from a continuation-shaped appendage to proper critic voice; ma-continue/bedrock likewise. Unit tests cover the identity gate both ways; the e2e spec asserts the cue in the anthropic and bedrock successor payloads, its absence for openAI, and that run state stays clean. * fix: key the handoff cue on the serving provider; read the run tail without slicing Codex round 1 on #346, both confirmed: - The cue lived in createCallModel's transform stack, keyed on the agent's PRIMARY provider — the same fallback mistake #335's coalescer already taught: an OpenAI primary falling back to Bedrock-Claude reused the shared payload without the cue (prefill bug reproduced on the fallback), and an Anthropic primary falling back to OpenAI shipped the Claude-only synthetic turn. Moved to the attemptInvoke funnel next to the coalesce, keyed on the provider actually serving the call; the Bedrock model id is read off the serving model instance, defaulting Bedrock to Claude-like when absent (the safe direction for a cue that is inert elsewhere). - getRunMessages() sliced the whole run tail per anthropic-like model call to read one element. New non-allocating Graph.getLastRunMessage() mirrors its disposed-graph fallbacks; appendPredecessorHandoffCue now takes the tail message directly. New funnel-level tests: cue for anthropic and Bedrock-Claude serving calls, absent for Bedrock-Nova and OpenAI. E2E spec and live ma-continue acceptance unchanged and re-verified after the move. * fix: tolerate partial invoke contexts when reading the run tail The lint-appeasing removal of the optional call broke long-standing tests that hand attemptInvoke a partial context stub: the type claims every context is a full Graph, but summarization passes none and stubs implement only what they exercise — getLastRunMessage genuinely may be absent at runtime. Read it through a deliberately widened cast so the call is stub-safe without an unnecessary-condition warning. * fix: measured cue for primaries, provenance by id set, serving-model unwrap Codex round 2 on #346, all four confirmed (one already fixed in 5fc0fce before this commit): - Measurement: the cue is now baked in createCallModel's provider transform stage for anthropic-like PRIMARIES, so the pre-invoke projection and overflow guard measure it. The attemptInvoke funnel only corrects for family-crossing fallbacks: appends for a Claude fallback behind a tolerant primary, strips (new removePredecessorHandoffCue) for a tolerant fallback behind an Anthropic primary. Both helpers are identity on their no-op paths, so the primary's funnel re-pass is free and idempotent. - Provenance: run/host discrimination no longer derives from startIndex arithmetic — summarization's remove-all compaction rewrites the live array and stales the index, silently reviving the prefill bug post-summarize. The graph now records the ids of AI turns its agent node returns (runProducedAiMessageIds, cleared per turn) and gates the cue on membership: ids survive compaction, host prefill ids are never present, O(1) per call. getLastRunMessage is gone — nothing needs the tail itself anymore. - Serving model unwrap: resolveServingModelId walks bindTools' RunnableBinding / system-runnable RunnableSequence for the .model id so a wrapped, explicitly configured Bedrock-Nova agent is not defaulted to Claude by isAnthropicLike's missing-model rule. Funnel tests now cover wrapped Claude (cue), wrapped Nova (no cue), tolerant-provider stripping of a baked cue, and idempotency over an already-baked payload. Live ma-continue acceptance re-verified. * fix: measure the Claude-fallback cue; assign ids before provenance recording Codex round 3 on #346, both confirmed: - The fallback prep closure now applies the serving-provider cue shaping (append for Claude fallbacks, strip for tolerant ones) BEFORE projecting and measuring the fallback payload, so a prompt within the cue's cost of fallbackMaxContextTokens takes the planned recovery path instead of shipping oversized. The attemptInvoke funnel pass now finds nothing to change on any measured path. - The public ChatModel contract does not require message ids, and the reducer assigns them only AFTER the node returns — too late for the provenance set. The node now assigns a reducer-style id (v4, mirrored into lc_kwargs) before recording, so custom id-less models keep cue coverage; the reducer's keep-existing-id rule makes the state message match. Detector honesty note: BaseChatModel subclasses cannot reproduce the id-less shape (LangChain stamps run ids downstream of _streamResponseChunks), so the new e2e test overrides _streamIterator — below the stamping, still a Runnable for the systemRunnable pipe. Verified counterfactually: without the fix the critic's payload tail is the writer's raw essay; with it, the cue.
Lets the steer route ask the generating replica to seal its live model stream at the next provider-safe boundary instead of waiting for a tool step. The run is never aborted, job status never changes, the partial answer is kept, and generation resumes in the same assistant message after the injected steer. Consumes the SDK seam in @librechat/agents (LibreChat-AI/agents#335, #346). Transport: IEventTransport gains a fenced emitPreempt/onPreempt pair beside abort. RedisEventTransport fans PREEMPT out on the SAME events channel and subscription (no new connection, key, or subscribe call); onPreempt returns a registration-scoped unsubscribe with the same replacement-safe state-identity guard onAbort uses. InMemory implements neither — single-process preempt lives entirely in the runtime set. Runtime state: RuntimeJobState carries the per-generation request set, createdAt-fenced and capped at STEER_QUEUE_MAX_DEPTH, plus a bounded `cleared` tombstone so a late cross-replica arm cannot resurrect a request whose steer already drained. registerPreemptSubscription mirrors the abort registration's double fence (runtime identity + generation createdAt); releaseAbortSubscription retires BOTH listeners and the armed set, so every terminal path drops preempt state for free. Public surface: requestPreempt (arm + fenced publish, never a rejection surface, never touches job status), isPreemptRequested (O(1) level-triggered poll), noteSteersRemoved (drain/cancel bookkeeping + fenced clear), clearPreemptRequests (empty-boundary disarm). One drain body, two boundaries: createSteerDrainHook (PostToolBatch) and createSteerPreemptBoundaryHook (PreemptBoundary) share drainAndBuildInjections, so the two injection sites cannot drift — the SDK's provider-safety argument rests on identical HumanMessage shapes. The shared body builds injections incrementally under a swallow-all catch (a mid-loop throw still injects what was applied — those parts are already persisted), clears preempt requests in finally, and disarms the generation when a boundary drains nothing. Request path: POST /chat/steer accepts preempt: true. The guard ladder is unchanged in order and in every status code. A preempt request is NEVER a rejection reason — without the capability the steer still enqueues and the 202 echoes preempt: false. Armed strictly after a successful enqueue; cancel disarms. The capability is read from the OWNING replica's recorded `preemptCapable` rather than the route replica's own SDK probe, so a rolling deploy cannot label a steer "interrupting" that the older owner will only inject at a tool step. Durable label: SteerQueueItem.preempt → TPendingSteer.preempt, so a parked/claimed/replayed chip keeps its wording. Run wiring: createRun registers the PreemptBoundary hook and threads RunConfig.preemption, both gated on isSteerPreemptSupported() — a separate probe from isSteeringSupported(), so the client affordance can never arm against an SDK that only injects at tool boundaries. buildSteerWiring builds both hooks from one shared closures object, so preemption survives HITL pause/resume for free. Honest finalization: an empty preempt boundary persists and emits with unfinished: true — the same contract an abort gets — re-marked explicitly because BaseClient has already saved the row as unfinished: false by that point. Not changed: no new job status, store method, Lua, SSE event type, endpoint, or authorization surface. abortJob, completeJob, transitionStatus, closeAndDrainSteers, getResumeState, emitChunk, applySteerPart and the whole abort path are untouched. Tests: 120 packages/api steering specs (preempt lifecycle, tombstone, fences, caps, terminal release, both-boundary drain parity, level-triggered poll, request/cancel arming, owner-capability degradation) plus 5 in api for buildSteerWiring gating, and 2 Redis-gated cross-replica transport specs.
Lets the steer route ask the generating replica to seal its live model stream at the next provider-safe boundary instead of waiting for a tool step. The run is never aborted, job status never changes, the partial answer is kept, and generation resumes in the same assistant message after the injected steer. Consumes the SDK seam in @librechat/agents (LibreChat-AI/agents#335, #346). Transport: IEventTransport gains a fenced emitPreempt/onPreempt pair beside abort. RedisEventTransport fans PREEMPT out on the SAME events channel and subscription (no new connection, key, or subscribe call); onPreempt returns a registration-scoped unsubscribe with the same replacement-safe state-identity guard onAbort uses. InMemory implements neither — single-process preempt lives entirely in the runtime set. Runtime state: RuntimeJobState carries the per-generation request set, createdAt-fenced and capped at STEER_QUEUE_MAX_DEPTH, plus a bounded `cleared` tombstone so a late cross-replica arm cannot resurrect a request whose steer already drained. registerPreemptSubscription mirrors the abort registration's double fence (runtime identity + generation createdAt); releaseAbortSubscription retires BOTH listeners and the armed set, so every terminal path drops preempt state for free. Public surface: requestPreempt (arm + fenced publish, never a rejection surface, never touches job status), isPreemptRequested (O(1) level-triggered poll), noteSteersRemoved (drain/cancel bookkeeping + fenced clear), clearPreemptRequests (empty-boundary disarm). One drain body, two boundaries: createSteerDrainHook (PostToolBatch) and createSteerPreemptBoundaryHook (PreemptBoundary) share drainAndBuildInjections, so the two injection sites cannot drift — the SDK's provider-safety argument rests on identical HumanMessage shapes. The shared body builds injections incrementally under a swallow-all catch (a mid-loop throw still injects what was applied — those parts are already persisted), clears preempt requests in finally, and disarms the generation when a boundary drains nothing. Request path: POST /chat/steer accepts preempt: true. The guard ladder is unchanged in order and in every status code. A preempt request is NEVER a rejection reason — without the capability the steer still enqueues and the 202 echoes preempt: false. Armed strictly after a successful enqueue; cancel disarms. The capability is read from the OWNING replica's recorded `preemptCapable` rather than the route replica's own SDK probe, so a rolling deploy cannot label a steer "interrupting" that the older owner will only inject at a tool step. Durable label: SteerQueueItem.preempt → TPendingSteer.preempt, so a parked/claimed/replayed chip keeps its wording. Run wiring: createRun registers the PreemptBoundary hook and threads RunConfig.preemption, both gated on isSteerPreemptSupported() — a separate probe from isSteeringSupported(), so the client affordance can never arm against an SDK that only injects at tool boundaries. buildSteerWiring builds both hooks from one shared closures object, so preemption survives HITL pause/resume for free. Honest finalization: an empty preempt boundary persists and emits with unfinished: true — the same contract an abort gets — re-marked explicitly because BaseClient has already saved the row as unfinished: false by that point. Not changed: no new job status, store method, Lua, SSE event type, endpoint, or authorization surface. abortJob, completeJob, transitionStatus, closeAndDrainSteers, getResumeState, emitChunk, applySteerPart and the whole abort path are untouched. Tests: 120 packages/api steering specs (preempt lifecycle, tombstone, fences, caps, terminal release, both-boundary drain parity, level-triggered poll, request/cancel arming, owner-capability degradation) plus 5 in api for buildSteerWiring gating, and 2 Redis-gated cross-replica transport specs.
* 🛑 feat: Preemptive Steer — server half (Interrupt & Steer, PR 2 of 3) Lets the steer route ask the generating replica to seal its live model stream at the next provider-safe boundary instead of waiting for a tool step. The run is never aborted, job status never changes, the partial answer is kept, and generation resumes in the same assistant message after the injected steer. Consumes the SDK seam in @librechat/agents (LibreChat-AI/agents#335, #346). Transport: IEventTransport gains a fenced emitPreempt/onPreempt pair beside abort. RedisEventTransport fans PREEMPT out on the SAME events channel and subscription (no new connection, key, or subscribe call); onPreempt returns a registration-scoped unsubscribe with the same replacement-safe state-identity guard onAbort uses. InMemory implements neither — single-process preempt lives entirely in the runtime set. Runtime state: RuntimeJobState carries the per-generation request set, createdAt-fenced and capped at STEER_QUEUE_MAX_DEPTH, plus a bounded `cleared` tombstone so a late cross-replica arm cannot resurrect a request whose steer already drained. registerPreemptSubscription mirrors the abort registration's double fence (runtime identity + generation createdAt); releaseAbortSubscription retires BOTH listeners and the armed set, so every terminal path drops preempt state for free. Public surface: requestPreempt (arm + fenced publish, never a rejection surface, never touches job status), isPreemptRequested (O(1) level-triggered poll), noteSteersRemoved (drain/cancel bookkeeping + fenced clear), clearPreemptRequests (empty-boundary disarm). One drain body, two boundaries: createSteerDrainHook (PostToolBatch) and createSteerPreemptBoundaryHook (PreemptBoundary) share drainAndBuildInjections, so the two injection sites cannot drift — the SDK's provider-safety argument rests on identical HumanMessage shapes. The shared body builds injections incrementally under a swallow-all catch (a mid-loop throw still injects what was applied — those parts are already persisted), clears preempt requests in finally, and disarms the generation when a boundary drains nothing. Request path: POST /chat/steer accepts preempt: true. The guard ladder is unchanged in order and in every status code. A preempt request is NEVER a rejection reason — without the capability the steer still enqueues and the 202 echoes preempt: false. Armed strictly after a successful enqueue; cancel disarms. The capability is read from the OWNING replica's recorded `preemptCapable` rather than the route replica's own SDK probe, so a rolling deploy cannot label a steer "interrupting" that the older owner will only inject at a tool step. Durable label: SteerQueueItem.preempt → TPendingSteer.preempt, so a parked/claimed/replayed chip keeps its wording. Run wiring: createRun registers the PreemptBoundary hook and threads RunConfig.preemption, both gated on isSteerPreemptSupported() — a separate probe from isSteeringSupported(), so the client affordance can never arm against an SDK that only injects at tool boundaries. buildSteerWiring builds both hooks from one shared closures object, so preemption survives HITL pause/resume for free. Honest finalization: an empty preempt boundary persists and emits with unfinished: true — the same contract an abort gets — re-marked explicitly because BaseClient has already saved the row as unfinished: false by that point. Not changed: no new job status, store method, Lua, SSE event type, endpoint, or authorization surface. abortJob, completeJob, transitionStatus, closeAndDrainSteers, getResumeState, emitChunk, applySteerPart and the whole abort path are untouched. Tests: 120 packages/api steering specs (preempt lifecycle, tombstone, fences, caps, terminal release, both-boundary drain parity, level-triggered poll, request/cancel arming, owner-capability degradation) plus 5 in api for buildSteerWiring gating, and 2 Redis-gated cross-replica transport specs. * 🔒 fix: Codex round 2 — evict tombstones, scope the empty-boundary disarm, honest resumes All four server findings were fresh consequences of the round-1 fixes, which is the review doing exactly what it should. - Tombstone cap refused new entries instead of evicting. Every drained or cancelled steer is tombstoned, not just preempting ones, so a generation that processed 20 steers exhausted the set and the late-arm race resurfaced silently. Now evicts oldest-first (Set iteration is insertion-ordered), with the budget named PREEMPT_TOMBSTONE_MAX rather than an inline expression. - The empty-boundary disarm I added in round 1 wiped the generation's ENTIRE armed set. A second steer can enqueue and arm between the atomic drain returning empty and the disarm running — that arm is backed by a live, uninjected queue item and must survive. The drain now snapshots the armed ids BEFORE draining (getArmedPreemptIds) and clearPreemptRequests takes an explicit id list instead of clearing everything. - HITL resume finalized with a hardcoded unfinished: false. The boundary hook is re-registered on resume via buildSteerWiring, so a resumed segment can end on an empty preempt boundary exactly like a fresh one; finalizeResumedTurn now reads getPreemptStats() and the halt reason, matching the normal request path. - Ownership moves on resume, so the job's recorded preemptCapable must describe the replica that will actually generate. Refreshed before resumeCompletion; a job created on a capable replica that resumes on an older one during a rolling deploy no longer acknowledges steers as interrupting. Tests: +3 (scoped disarm sparing a post-snapshot arm, oldest-first tombstone eviction, id-list disarm). 122 packages/api steering specs green. * 🚨 fix: Codex round 3 — deserialize preemptCapable from Redis (feature was dead under Redis) The P1 here is the most consequential defect in the whole feature, and it was introduced by round 1's own capability fix. - `RedisJobStore.serializeJob` writes booleans generically, so `preemptCapable` reached Redis — but `deserializeJob` is an EXPLICIT field map and had no line for it. Every `getJob()` therefore dropped the flag, `job.metadata.preemptCapable` was always undefined, and `handleSteerRequest` computed `preemptArmed: false` unconditionally. Interrupt & steer would have silently degraded to ordinary tool-boundary steering in EVERY Redis deployment — i.e. the feature shipping as a no-op in production while passing every in-memory test. Now deserialized, with a round-trip assertion in the metadata spec that fails (`Received: undefined`) against the unfixed store. - The resume capability refresh moved from just-before `resumeCompletion` to immediately after `approvals.resolve` claims the run. That call already flips the job back to `running`, so the steer route accepts requests from that instant; leaving the refresh 135 lines later (across the whole client reconstruction) left a real window where a steer read the PREVIOUS owner's capability. Not the fully atomic transition Codex suggested — that reaches into the approvals Lua — but it shrinks the window from seconds to one await, which is proportionate for a label-accuracy issue. Refuted: "avoid triggering preemption inside subagents". The premise — that the run-wide poll can seal a subagent stream — does not hold against the shipped SDK. Child graphs are constructed with `subagentScope: true` (SubagentExecutor) and `preemption` is NOT propagated into child inputs, while `canClaimPreemptSeal()` requires `!subagentScope && preemption != null`. Both conditions fail independently, so a subagent can never claim a seal and the boundary cannot fire with `agentId` set. The `input.agentId != null` guard in the hook is defensive depth, not the thing standing between us and the described failure. 140 packages/api specs green. * 🔁 fix: Codex round 4 — re-arm durable interrupt steers when resume moves owners - An arm lives only in the owning replica's runtime plus a transient pub/sub message, while the steer's `preempt` flag is durable on the queue item. A HITL resume landing on a different replica therefore started with an empty armed set and a poll stuck false, so an interrupt the user had already been ACKed for silently waited for an ordinary tool boundary. New `GenerationJobManager.rearmQueuedPreempts` rebuilds the armed set by peeking the durable queue (fenced on the generation) and re-arming every item flagged `preempt`; resume calls it right after claiming. Safe by construction: every item peeked is still queued, so no drained steer can be resurrected. - Capability-refresh failure now logs at error rather than warn, but deliberately does NOT fail the resume — see the reply on that thread. Tests: +2 (rebuild from queue arms only the flagged item and reports the count; a stale generation arms nothing). 124 packages/api steering specs green. * 📡 fix: Codex round 5 — acknowledge only what was actually armed - A cross-replica arm was fire-and-forget: `emitPreempt` logged its own publish failure and `requestPreempt` returned void, so the route answered `preempt: true` even when the owner never armed a poll. The steer still injected at the next tool boundary, but the chip claimed an interrupt that could not happen — and unlike HITL resume, an ordinary running generation had no durable reconciliation to recover it. `emitPreempt` now resolves to the subscriber count and rejects on failure; `requestPreempt` is async and returns whether the arm truly landed (owned locally, or delivered to at least one subscriber). The 202 reports THAT rather than what was asked for, so the chip relabels to ordinary steering exactly as it does for a capability-degraded deployment. Errors are swallowed into `false` — an unarmed interrupt is a downgrade, never a failed steer. - The owner capability is re-read immediately before enqueue rather than reused from the top of the guard ladder. `checkAgentAccess` and file resolution are awaits, so a request can span an entire HITL pause/resume that moves ownership to a replica with different capability and rewrites that very flag. Only paid for by requests that actually asked to interrupt. Tests: +3 (not-armed when the publish reaches nobody; armed when this replica owns the generation; a throwing publish downgrades instead of propagating). 127 packages/api steering specs green. * 🎯 fix: Codex round 6 — real ownership, confirmed disarms, and a CI regression of my own Three review findings plus three CI failures the round-5 commit caused. Review: - Ownership came from `runtimeState`, which a cross-replica `getJob` populates with a FACADE runtime on any replica that merely read the job. Matching `createdAt` therefore proved only "we looked at this job", so a non-owner could arm nothing and report success. Ownership now comes from `ownedJobs`, the actual owner map. - `armPreemptIds` returns how many ids it accepted, and a local arm is only reported as armed when one was. A tombstoned id (its steer drained at an ordinary boundary mid-request) no longer answers `preempt: true` for an interrupt that cannot happen. - The cancel disarm is awaited. A dropped clear is worse than a dropped arm: the owner keeps a level-triggered request for a steer that no longer exists, seals its next chunk and truncates an unrelated answer. The boundary drain's own call stays non-blocking — there the owner is local, so the disarm is already effective and awaiting the informational publish would only delay injection. - Subscriber count is NOT read as proof of owner receipt: the count includes this replica's own facade subscription. A successful publish reports armed, a rejected one does not. Documented rather than papered over — see the acknowledgement-semantics note on the PR. CI regressions from round 5, all mine: - `registerPreemptSubscription` was AWAITED at both runtime-init sites, so job creation blocked on a second Redis channel subscription and hung when that subscribe was slow. Abort is awaited because a missed abort strands a run; a missed preempt only degrades that steer to the next tool boundary, so it now registers without gating createJob. - Two api specs mocked `@librechat/api` without the newly imported `isSteerPreemptSupported`, so the call threw before createJob; and one exact-match assertion needed the new `preemptCapable` metadata field. - My own Redis integration spec asserted arm-before-clear ordering, which two publishes carry no guarantee of — the receiving tombstone exists precisely because of that. Now asserts delivery and payload fidelity, order-independent. 158 packages/api specs, 27 api specs green. * 🧭 fix: Codex round 7 — settle the acknowledgement semantics (Option A) Round 7's second finding is the incoherence I flagged on the PR: the route persisted `preempt: true` on the durable queue item while returning `preempt: false` when delivery could not be confirmed. Those two then disagreed, and `rearmQueuedPreempts` trusts the DURABLE one — so a resumed owner would honour an interrupt the client had explicitly been told degraded to ordinary steering. Rather than patch the disagreement, this settles the meaning: `preempt` in the 202 means "queued as an interrupt request", NOT "a seal is guaranteed". It mirrors `SteerQueueItem.preempt` exactly, so the response, the durable record, and the resume-time re-arm can never disagree. The gates that ARE knowable stay — the owner's recorded capability and a successful enqueue. Everything past that degrades to the documented fallback of injecting at the next tool boundary. A route cannot synchronously know whether another replica will seal: proving it needs a correlated request/response over pub-sub, and even that only proves the owner heard, not that it is still streaming when the arm lands. Four rounds of tightening this boolean each surfaced a narrower case; the sequence does not converge, so the invariant is now "the flag describes the durable decision" and an unconfirmed arm logs a warning instead of rewriting the answer. Also from this round: a failed disarm publish is retried once and its outcome reported. `handleSteerCancel` keeps `removed: true` — the steer really did leave the queue, and saying otherwise would make the client re-show a chip for a steer that can never arrive — and adds `disarmed: false` so the residual risk is visible rather than swallowed. Damage stays bounded regardless: the empty-boundary self-clear disarms the generation after a single seal. Tests: +1 pinning the response/durable-flag invariant. 159 packages/api specs green. * 🧹 fix: Codex round 8 — remove the unverifiable disarm signal Round 8 found the same over-promise on the disarm side that round 7 corrected on the arm side, so this applies the same answer rather than patching around it. The `disarmed: false` field added in round 7 was both unreliable and unused: a resolved publish is not proof the owner heard it (the delivery count includes this replica's own facade subscription), and it was never threaded into `CancelSteerResponse` or read by any client. A signal that claims a certainty the transport cannot provide is worse than no signal — it invites callers to trust it. Removed from the response. The retry stays, because it genuinely reduces the failure rate, and `noteSteersRemoved` still returns whether the publish succeeded FOR LOGGING, now documented explicitly as "published without error", not "the owner disarmed". Disarm is best effort with a bounded, self-healing failure: if the clear is lost the owner seals once, the empty-boundary self-clear disarms the generation, and the turn is persisted `unfinished: true` rather than silently truncated. Tightening that further needs a correlated request/response over pub-sub with a timeout — noted on the PR as the deliberate boundary of this design rather than an oversight. 130 packages/api steering specs green. * 🧽 fix: Codex round 9 — spend snapshot arms on nonempty drains too The round-6 scoping fix only cleared the pre-drain snapshot when the drain came back EMPTY. On a nonempty drain the `finally` cleared just the drained ids, so a stale arm — typically a cancel whose cross-replica clear was lost — survived the boundary. It would then immediately seal the continuation meant to answer the steer that had just been injected, and land on an empty boundary as `preempt_incomplete`: the interrupt appears to work, and the answer to it is truncated. A boundary that runs has spent its seal, so everything armed at snapshot time is spent whether or not it came back from the drain. The `finally` now clears the union of the snapshot and the drained ids. Arms that land AFTER the snapshot are still spared — their queue items are live and uninjected, which is the property round 6 added. Also fixes an api-workspace CI failure of mine: `resume.spec.js` stubs `GenerationJobManager` wholesale, and the round-3/4 resume work added two calls (`updateMetadata`, `rearmQueuedPreempts`) the stub did not define, so 34 specs threw. Stub extended. Tests: +2 (a nonempty drain clears a stale snapshot arm; a nonempty drain spares an arm that landed mid-drain). Counterfactually verified — the stale-arm spec fails against the unfixed drain. 132 packages/api specs, 60 resume specs green. * fix: never let a failed preempt subscription reject into the void registerPreemptSubscription is called detached at both sites, so a rejected Redis SUBSCRIBE became an unhandled rejection — process-fatal under Node's default --unhandled-rejections=throw. The comment already promised this path merely degrades steering; it now does. Swallowed and logged inside the registration rather than at each call site, so a future third caller cannot reintroduce the trap. Losing the channel costs this generation's cross-replica preempts, not the server: same-replica arming is runtime state and still works, and remote arms fall back to the next tool boundary. Verified counterfactually — the new spec surfaces SUBSCRIBE failed as an unhandled rejection against the unfixed registration. * docs: state the real blast radius of a failed preempt subscription LibreChat's own entrypoints install a global unhandledRejection handler that logs and keeps serving, so the escaping rejection this guards was never fatal to this server — only to another consumer of @librechat/api that installs no handler. The fix stands either way; the comment just should not overstate what it prevents. * test: cover the cross-replica preempt hop with two manager instances Every other preempt test runs against a single manager, so the hop that actually carries an interrupt in production had no coverage: the steer POST lands on whichever replica the balancer picks, which is usually not the one generating. Non-owner publishes, owner arms, owner's level-triggered poll flips — none of that was exercised end to end. Two GenerationJobManagerClass instances are a faithful replica pair here. runtimeState and ownedJobs are private instance fields, there is no module-level mutable state between them, and createStreamServices duplicates a dedicated subscriber connection per call, so separate OS processes would exercise the same objects over the same Redis. Both assertions verified counterfactually against real Redis: - Deleting the preemptCapable deserialization in RedisJobStore fails this with 'Expected: true, Received: undefined' — the exact P1 that shipped past every in-memory test and would have made the feature a silent no-op on every Redis deployment. - Dropping the non-owner arm publish fails it with 'Received: false'. * test: remove the fixed sleeps and vacuity from the cross-replica preempt test Codex round 11, both findings, both on the test I added last commit. P2 — the 300ms waits were load-bearing. Redis pub/sub never replays and the owner's SUBSCRIBE is detached, so on a slow CI worker the publish could land before anyone was listening and the test would fail against correct code. Now it republishes until the owner's state converges, which is safe because arms and clears are idempotent set writes keyed by steerId. Side effect: the tests got ~10x faster (85ms/57ms vs 929ms/606ms) since they finish on delivery rather than on a timer. P3 — afterEach destroyed only the transports, leaving each manager alive in its own cleanup-interval closure, still working against a dead transport. Now tracks the managers and awaits destroy(), which disposes the job store and its timer too. Matches how the rest of this file cleans up. Fixing the sleeps exposed a third problem codex did not flag: the stale-arm test could pass vacuously, because an undelivered arm and a fenced one look identical. It now brackets the stale publish between two control arms — the first proves the owner is listening before the stale one is sent, the second proves it has had its chance to arrive. Verified counterfactually against real Redis, and stable over 5 runs: - dropping the preemptCapable deserialization fails with 'Received: undefined' - dropping the non-owner arm publish times out both tests - removing the generation fence fails the stale test with ["control-before", "steer-stale", "control-after"] — which also confirms the bracketing orders as intended rather than by luck * fix: gate interrupt on the OWNER's capability alone, not the route's Codex round 12. The comment above this gate already said 'the OWNER's recorded capability, not this replica's probe' — and then the code ANDed in isSteerPreemptSupported(), which is exactly this replica's probe. The contradiction dates to the original commit; round 6 made the gate owner-scoped and wrote that comment without removing the local conjunct. The route never seals. It enqueues and publishes an arm, neither of which touches the SDK, so during a rolling deploy a steer landing on an un-upgraded replica silently lost its interrupt even though the owner could seal. When the route IS the owner the probe is redundant anyway: the flag it would consult is the one this process wrote at createJob. The real degradation path is unchanged and still tested — an owner that recorded no capability relabels to an ordinary steer. The test that pinned the local probe asserted an impossible same-replica state (capable metadata plus an incapable local SDK, when the metadata is written from that probe); it now pins the mixed-SDK direction instead, and fails with 'Expected: true, Received: false' if the probe is put back. * fix: reconcile arms at handover, and stop holding the 202 on a publish Codex round 13, two of three findings. P2 — rearmQueuedPreempts only ever ADDED. A replica that merely read the job still installs a facade runtime and subscribes, so it can accept an arm and then miss the best-effort clear that follows the drain. HITL resume promotes that facade to owner, the union keeps the orphan, and the first resumed stream seals on a steer no longer in the queue, drains nothing, and truncates the resumed answer as preempt_incomplete. acquireResumedJobOwnership only sets ownedJobs, so nothing else was clearing it. The durable queue is the sole authority at a handover: arms it does not back are now disarmed and tombstoned, so an in-flight publish cannot revive them either. Worth recording that my own independent review raised this and my verifier refuted it. Codex found it separately; two reviewers converging should have outweighed one refutation. P2 — the route awaited the arm publish before answering. The 202 reports capability, not delivery, so the await could not change the response; it only exposed the caller to Redis latency after the queue item was already durable. A client that times out and retries mints a second steer while the first stays queued, injecting the same instruction twice, whereas a lost publish merely takes the tool-boundary fallback. Detached, with both outcomes logged. All three tests verified counterfactually: union-only rearm fails the two new handover specs, and re-awaiting the publish hangs the stalled-publish spec until jest kills it. * fix: snapshot arms before reading the queue at handover Codex round 14 — a regression from my own round-13 fix, and a worse failure than the one it corrected. Round 13 read the durable queue first, then tombstoned any armed id the snapshot did not back. But approvals.resolve reopens steering before reconciliation runs, so another replica can commit a preempt steer and publish its arm while the peek is in flight. That arm is then present locally but absent from a snapshot taken before the steer existed, so a LIVE interrupt the route already acknowledged got dropped — and tombstoned, which blocks the re-arm, making it unrecoverable rather than merely late. Fixed by inverting the two reads rather than by locking or paying a second round trip. A steer is durably enqueued BEFORE its arm is published, so any id in an arms-first snapshot was already queued when it was armed, and the later peek must observe it unless it has since drained — which is exactly the orphan this reconciliation exists to drop. Arms landing after the snapshot are simply not candidates. Also re-checks runtime identity across the await, since the generation can be replaced while the queue read is in flight. New spec injects a steer + arm during the peek and verifies it survives; against the round-13 ordering it fails with Received array: []. * fix: bound the cancel disarm wait and fence enqueue to its generation Codex round 15. P2 — the cancel awaited its disarm publish unbounded. ioredis queues commands during an outage rather than rejecting, so that await could hang for the length of the outage with the steer ALREADY durably cancelled; a client that gives up then treats the cancel as failed and restores a chip for a steer that can never produce an applied event. Every successful cancel publishes, so ordinary steers were exposed too, not only preemptive ones. Now bounded at 1s, with the publish continuing behind it — its retry and logging are unchanged, it is just no longer in front of the response. This is the sibling of round 13's arm-publish finding; I fixed one path and left this one. P3 — enqueue was not fenced to the generation the capability decision was made against. The access checks, file resolution and owner re-read are all awaits, so the run can be replaced before the enqueue: the item then lands on the REPLACEMENT queue while the durable preempt flag and the arm still name the previous epoch, the arm is fenced out at the owner, and the 202 promises an interrupt that cannot happen. enqueueSteer now takes an expected generation, mirroring drain/peek, and the Redis path enforces it inside STEER_ENQUEUE_LUA so the check is atomic with the push rather than racing it. All three new specs verified counterfactually, including the Lua guard against real Redis (removing it returns 1 where -1 is required). * fix: fence the steer to its authorized generation, bound resume setup, keep preempt when Redis parks Codex round 16, all three findings. P2 — round 15 fenced the enqueue to owner.createdAt, the RE-READ job. Every guard above it (ownership, tenant, paused-state, agent ACL) ran against the job read at the top, so if the run was replaced during those awaits the fence happily accepted the steer into a generation the request was never authorized against, carrying the wrong agent's metadata. Now rejects on any mismatch between the validated job and the re-read. P2 — resume awaited its steering bookkeeping unbounded, after approvals.resolve had consumed the action and flipped the job to running, and outside the resume lifecycle's own try/finally. .catch does not fire on a promise that never settles, which is what ioredis produces during an outage, so the client times out, its retry gets a 409 for a spent action, and no cleanup runs. Bounded at 1s with the writes finishing in the background. P3 — Redis parks leftover steers inside its terminal-transition Lua, which projects item fields one by one, so preempt was silently dropped and a steer recovered from /chat/status lost its interrupting label. Added to both projections. All three verified counterfactually, two against real Redis. Worth recording that my first version of the generation-mismatch test was VACUOUS — it faked a createdAt matching no live job, so the round-15 enqueue fence rejected it for the wrong reason and the test passed with the guard removed. Rewritten to replace the run for real; it now fails with 'Expected 404, Received 202'.
* 🛑 feat: Preemptive Steer — server half (Interrupt & Steer, PR 2 of 3) Lets the steer route ask the generating replica to seal its live model stream at the next provider-safe boundary instead of waiting for a tool step. The run is never aborted, job status never changes, the partial answer is kept, and generation resumes in the same assistant message after the injected steer. Consumes the SDK seam in @librechat/agents (LibreChat-AI/agents#335, #346). Transport: IEventTransport gains a fenced emitPreempt/onPreempt pair beside abort. RedisEventTransport fans PREEMPT out on the SAME events channel and subscription (no new connection, key, or subscribe call); onPreempt returns a registration-scoped unsubscribe with the same replacement-safe state-identity guard onAbort uses. InMemory implements neither — single-process preempt lives entirely in the runtime set. Runtime state: RuntimeJobState carries the per-generation request set, createdAt-fenced and capped at STEER_QUEUE_MAX_DEPTH, plus a bounded `cleared` tombstone so a late cross-replica arm cannot resurrect a request whose steer already drained. registerPreemptSubscription mirrors the abort registration's double fence (runtime identity + generation createdAt); releaseAbortSubscription retires BOTH listeners and the armed set, so every terminal path drops preempt state for free. Public surface: requestPreempt (arm + fenced publish, never a rejection surface, never touches job status), isPreemptRequested (O(1) level-triggered poll), noteSteersRemoved (drain/cancel bookkeeping + fenced clear), clearPreemptRequests (empty-boundary disarm). One drain body, two boundaries: createSteerDrainHook (PostToolBatch) and createSteerPreemptBoundaryHook (PreemptBoundary) share drainAndBuildInjections, so the two injection sites cannot drift — the SDK's provider-safety argument rests on identical HumanMessage shapes. The shared body builds injections incrementally under a swallow-all catch (a mid-loop throw still injects what was applied — those parts are already persisted), clears preempt requests in finally, and disarms the generation when a boundary drains nothing. Request path: POST /chat/steer accepts preempt: true. The guard ladder is unchanged in order and in every status code. A preempt request is NEVER a rejection reason — without the capability the steer still enqueues and the 202 echoes preempt: false. Armed strictly after a successful enqueue; cancel disarms. The capability is read from the OWNING replica's recorded `preemptCapable` rather than the route replica's own SDK probe, so a rolling deploy cannot label a steer "interrupting" that the older owner will only inject at a tool step. Durable label: SteerQueueItem.preempt → TPendingSteer.preempt, so a parked/claimed/replayed chip keeps its wording. Run wiring: createRun registers the PreemptBoundary hook and threads RunConfig.preemption, both gated on isSteerPreemptSupported() — a separate probe from isSteeringSupported(), so the client affordance can never arm against an SDK that only injects at tool boundaries. buildSteerWiring builds both hooks from one shared closures object, so preemption survives HITL pause/resume for free. Honest finalization: an empty preempt boundary persists and emits with unfinished: true — the same contract an abort gets — re-marked explicitly because BaseClient has already saved the row as unfinished: false by that point. Not changed: no new job status, store method, Lua, SSE event type, endpoint, or authorization surface. abortJob, completeJob, transitionStatus, closeAndDrainSteers, getResumeState, emitChunk, applySteerPart and the whole abort path are untouched. Tests: 120 packages/api steering specs (preempt lifecycle, tombstone, fences, caps, terminal release, both-boundary drain parity, level-triggered poll, request/cancel arming, owner-capability degradation) plus 5 in api for buildSteerWiring gating, and 2 Redis-gated cross-replica transport specs. * 🔒 fix: Codex round 2 — evict tombstones, scope the empty-boundary disarm, honest resumes All four server findings were fresh consequences of the round-1 fixes, which is the review doing exactly what it should. - Tombstone cap refused new entries instead of evicting. Every drained or cancelled steer is tombstoned, not just preempting ones, so a generation that processed 20 steers exhausted the set and the late-arm race resurfaced silently. Now evicts oldest-first (Set iteration is insertion-ordered), with the budget named PREEMPT_TOMBSTONE_MAX rather than an inline expression. - The empty-boundary disarm I added in round 1 wiped the generation's ENTIRE armed set. A second steer can enqueue and arm between the atomic drain returning empty and the disarm running — that arm is backed by a live, uninjected queue item and must survive. The drain now snapshots the armed ids BEFORE draining (getArmedPreemptIds) and clearPreemptRequests takes an explicit id list instead of clearing everything. - HITL resume finalized with a hardcoded unfinished: false. The boundary hook is re-registered on resume via buildSteerWiring, so a resumed segment can end on an empty preempt boundary exactly like a fresh one; finalizeResumedTurn now reads getPreemptStats() and the halt reason, matching the normal request path. - Ownership moves on resume, so the job's recorded preemptCapable must describe the replica that will actually generate. Refreshed before resumeCompletion; a job created on a capable replica that resumes on an older one during a rolling deploy no longer acknowledges steers as interrupting. Tests: +3 (scoped disarm sparing a post-snapshot arm, oldest-first tombstone eviction, id-list disarm). 122 packages/api steering specs green. * 🚨 fix: Codex round 3 — deserialize preemptCapable from Redis (feature was dead under Redis) The P1 here is the most consequential defect in the whole feature, and it was introduced by round 1's own capability fix. - `RedisJobStore.serializeJob` writes booleans generically, so `preemptCapable` reached Redis — but `deserializeJob` is an EXPLICIT field map and had no line for it. Every `getJob()` therefore dropped the flag, `job.metadata.preemptCapable` was always undefined, and `handleSteerRequest` computed `preemptArmed: false` unconditionally. Interrupt & steer would have silently degraded to ordinary tool-boundary steering in EVERY Redis deployment — i.e. the feature shipping as a no-op in production while passing every in-memory test. Now deserialized, with a round-trip assertion in the metadata spec that fails (`Received: undefined`) against the unfixed store. - The resume capability refresh moved from just-before `resumeCompletion` to immediately after `approvals.resolve` claims the run. That call already flips the job back to `running`, so the steer route accepts requests from that instant; leaving the refresh 135 lines later (across the whole client reconstruction) left a real window where a steer read the PREVIOUS owner's capability. Not the fully atomic transition Codex suggested — that reaches into the approvals Lua — but it shrinks the window from seconds to one await, which is proportionate for a label-accuracy issue. Refuted: "avoid triggering preemption inside subagents". The premise — that the run-wide poll can seal a subagent stream — does not hold against the shipped SDK. Child graphs are constructed with `subagentScope: true` (SubagentExecutor) and `preemption` is NOT propagated into child inputs, while `canClaimPreemptSeal()` requires `!subagentScope && preemption != null`. Both conditions fail independently, so a subagent can never claim a seal and the boundary cannot fire with `agentId` set. The `input.agentId != null` guard in the hook is defensive depth, not the thing standing between us and the described failure. 140 packages/api specs green. * 🔁 fix: Codex round 4 — re-arm durable interrupt steers when resume moves owners - An arm lives only in the owning replica's runtime plus a transient pub/sub message, while the steer's `preempt` flag is durable on the queue item. A HITL resume landing on a different replica therefore started with an empty armed set and a poll stuck false, so an interrupt the user had already been ACKed for silently waited for an ordinary tool boundary. New `GenerationJobManager.rearmQueuedPreempts` rebuilds the armed set by peeking the durable queue (fenced on the generation) and re-arming every item flagged `preempt`; resume calls it right after claiming. Safe by construction: every item peeked is still queued, so no drained steer can be resurrected. - Capability-refresh failure now logs at error rather than warn, but deliberately does NOT fail the resume — see the reply on that thread. Tests: +2 (rebuild from queue arms only the flagged item and reports the count; a stale generation arms nothing). 124 packages/api steering specs green. * 📡 fix: Codex round 5 — acknowledge only what was actually armed - A cross-replica arm was fire-and-forget: `emitPreempt` logged its own publish failure and `requestPreempt` returned void, so the route answered `preempt: true` even when the owner never armed a poll. The steer still injected at the next tool boundary, but the chip claimed an interrupt that could not happen — and unlike HITL resume, an ordinary running generation had no durable reconciliation to recover it. `emitPreempt` now resolves to the subscriber count and rejects on failure; `requestPreempt` is async and returns whether the arm truly landed (owned locally, or delivered to at least one subscriber). The 202 reports THAT rather than what was asked for, so the chip relabels to ordinary steering exactly as it does for a capability-degraded deployment. Errors are swallowed into `false` — an unarmed interrupt is a downgrade, never a failed steer. - The owner capability is re-read immediately before enqueue rather than reused from the top of the guard ladder. `checkAgentAccess` and file resolution are awaits, so a request can span an entire HITL pause/resume that moves ownership to a replica with different capability and rewrites that very flag. Only paid for by requests that actually asked to interrupt. Tests: +3 (not-armed when the publish reaches nobody; armed when this replica owns the generation; a throwing publish downgrades instead of propagating). 127 packages/api steering specs green. * 🎯 fix: Codex round 6 — real ownership, confirmed disarms, and a CI regression of my own Three review findings plus three CI failures the round-5 commit caused. Review: - Ownership came from `runtimeState`, which a cross-replica `getJob` populates with a FACADE runtime on any replica that merely read the job. Matching `createdAt` therefore proved only "we looked at this job", so a non-owner could arm nothing and report success. Ownership now comes from `ownedJobs`, the actual owner map. - `armPreemptIds` returns how many ids it accepted, and a local arm is only reported as armed when one was. A tombstoned id (its steer drained at an ordinary boundary mid-request) no longer answers `preempt: true` for an interrupt that cannot happen. - The cancel disarm is awaited. A dropped clear is worse than a dropped arm: the owner keeps a level-triggered request for a steer that no longer exists, seals its next chunk and truncates an unrelated answer. The boundary drain's own call stays non-blocking — there the owner is local, so the disarm is already effective and awaiting the informational publish would only delay injection. - Subscriber count is NOT read as proof of owner receipt: the count includes this replica's own facade subscription. A successful publish reports armed, a rejected one does not. Documented rather than papered over — see the acknowledgement-semantics note on the PR. CI regressions from round 5, all mine: - `registerPreemptSubscription` was AWAITED at both runtime-init sites, so job creation blocked on a second Redis channel subscription and hung when that subscribe was slow. Abort is awaited because a missed abort strands a run; a missed preempt only degrades that steer to the next tool boundary, so it now registers without gating createJob. - Two api specs mocked `@librechat/api` without the newly imported `isSteerPreemptSupported`, so the call threw before createJob; and one exact-match assertion needed the new `preemptCapable` metadata field. - My own Redis integration spec asserted arm-before-clear ordering, which two publishes carry no guarantee of — the receiving tombstone exists precisely because of that. Now asserts delivery and payload fidelity, order-independent. 158 packages/api specs, 27 api specs green. * 🧭 fix: Codex round 7 — settle the acknowledgement semantics (Option A) Round 7's second finding is the incoherence I flagged on the PR: the route persisted `preempt: true` on the durable queue item while returning `preempt: false` when delivery could not be confirmed. Those two then disagreed, and `rearmQueuedPreempts` trusts the DURABLE one — so a resumed owner would honour an interrupt the client had explicitly been told degraded to ordinary steering. Rather than patch the disagreement, this settles the meaning: `preempt` in the 202 means "queued as an interrupt request", NOT "a seal is guaranteed". It mirrors `SteerQueueItem.preempt` exactly, so the response, the durable record, and the resume-time re-arm can never disagree. The gates that ARE knowable stay — the owner's recorded capability and a successful enqueue. Everything past that degrades to the documented fallback of injecting at the next tool boundary. A route cannot synchronously know whether another replica will seal: proving it needs a correlated request/response over pub-sub, and even that only proves the owner heard, not that it is still streaming when the arm lands. Four rounds of tightening this boolean each surfaced a narrower case; the sequence does not converge, so the invariant is now "the flag describes the durable decision" and an unconfirmed arm logs a warning instead of rewriting the answer. Also from this round: a failed disarm publish is retried once and its outcome reported. `handleSteerCancel` keeps `removed: true` — the steer really did leave the queue, and saying otherwise would make the client re-show a chip for a steer that can never arrive — and adds `disarmed: false` so the residual risk is visible rather than swallowed. Damage stays bounded regardless: the empty-boundary self-clear disarms the generation after a single seal. Tests: +1 pinning the response/durable-flag invariant. 159 packages/api specs green. * 🧹 fix: Codex round 8 — remove the unverifiable disarm signal Round 8 found the same over-promise on the disarm side that round 7 corrected on the arm side, so this applies the same answer rather than patching around it. The `disarmed: false` field added in round 7 was both unreliable and unused: a resolved publish is not proof the owner heard it (the delivery count includes this replica's own facade subscription), and it was never threaded into `CancelSteerResponse` or read by any client. A signal that claims a certainty the transport cannot provide is worse than no signal — it invites callers to trust it. Removed from the response. The retry stays, because it genuinely reduces the failure rate, and `noteSteersRemoved` still returns whether the publish succeeded FOR LOGGING, now documented explicitly as "published without error", not "the owner disarmed". Disarm is best effort with a bounded, self-healing failure: if the clear is lost the owner seals once, the empty-boundary self-clear disarms the generation, and the turn is persisted `unfinished: true` rather than silently truncated. Tightening that further needs a correlated request/response over pub-sub with a timeout — noted on the PR as the deliberate boundary of this design rather than an oversight. 130 packages/api steering specs green. * 🧽 fix: Codex round 9 — spend snapshot arms on nonempty drains too The round-6 scoping fix only cleared the pre-drain snapshot when the drain came back EMPTY. On a nonempty drain the `finally` cleared just the drained ids, so a stale arm — typically a cancel whose cross-replica clear was lost — survived the boundary. It would then immediately seal the continuation meant to answer the steer that had just been injected, and land on an empty boundary as `preempt_incomplete`: the interrupt appears to work, and the answer to it is truncated. A boundary that runs has spent its seal, so everything armed at snapshot time is spent whether or not it came back from the drain. The `finally` now clears the union of the snapshot and the drained ids. Arms that land AFTER the snapshot are still spared — their queue items are live and uninjected, which is the property round 6 added. Also fixes an api-workspace CI failure of mine: `resume.spec.js` stubs `GenerationJobManager` wholesale, and the round-3/4 resume work added two calls (`updateMetadata`, `rearmQueuedPreempts`) the stub did not define, so 34 specs threw. Stub extended. Tests: +2 (a nonempty drain clears a stale snapshot arm; a nonempty drain spares an arm that landed mid-drain). Counterfactually verified — the stale-arm spec fails against the unfixed drain. 132 packages/api specs, 60 resume specs green. * fix: never let a failed preempt subscription reject into the void registerPreemptSubscription is called detached at both sites, so a rejected Redis SUBSCRIBE became an unhandled rejection — process-fatal under Node's default --unhandled-rejections=throw. The comment already promised this path merely degrades steering; it now does. Swallowed and logged inside the registration rather than at each call site, so a future third caller cannot reintroduce the trap. Losing the channel costs this generation's cross-replica preempts, not the server: same-replica arming is runtime state and still works, and remote arms fall back to the next tool boundary. Verified counterfactually — the new spec surfaces SUBSCRIBE failed as an unhandled rejection against the unfixed registration. * docs: state the real blast radius of a failed preempt subscription LibreChat's own entrypoints install a global unhandledRejection handler that logs and keeps serving, so the escaping rejection this guards was never fatal to this server — only to another consumer of @librechat/api that installs no handler. The fix stands either way; the comment just should not overstate what it prevents. * test: cover the cross-replica preempt hop with two manager instances Every other preempt test runs against a single manager, so the hop that actually carries an interrupt in production had no coverage: the steer POST lands on whichever replica the balancer picks, which is usually not the one generating. Non-owner publishes, owner arms, owner's level-triggered poll flips — none of that was exercised end to end. Two GenerationJobManagerClass instances are a faithful replica pair here. runtimeState and ownedJobs are private instance fields, there is no module-level mutable state between them, and createStreamServices duplicates a dedicated subscriber connection per call, so separate OS processes would exercise the same objects over the same Redis. Both assertions verified counterfactually against real Redis: - Deleting the preemptCapable deserialization in RedisJobStore fails this with 'Expected: true, Received: undefined' — the exact P1 that shipped past every in-memory test and would have made the feature a silent no-op on every Redis deployment. - Dropping the non-owner arm publish fails it with 'Received: false'. * test: remove the fixed sleeps and vacuity from the cross-replica preempt test Codex round 11, both findings, both on the test I added last commit. P2 — the 300ms waits were load-bearing. Redis pub/sub never replays and the owner's SUBSCRIBE is detached, so on a slow CI worker the publish could land before anyone was listening and the test would fail against correct code. Now it republishes until the owner's state converges, which is safe because arms and clears are idempotent set writes keyed by steerId. Side effect: the tests got ~10x faster (85ms/57ms vs 929ms/606ms) since they finish on delivery rather than on a timer. P3 — afterEach destroyed only the transports, leaving each manager alive in its own cleanup-interval closure, still working against a dead transport. Now tracks the managers and awaits destroy(), which disposes the job store and its timer too. Matches how the rest of this file cleans up. Fixing the sleeps exposed a third problem codex did not flag: the stale-arm test could pass vacuously, because an undelivered arm and a fenced one look identical. It now brackets the stale publish between two control arms — the first proves the owner is listening before the stale one is sent, the second proves it has had its chance to arrive. Verified counterfactually against real Redis, and stable over 5 runs: - dropping the preemptCapable deserialization fails with 'Received: undefined' - dropping the non-owner arm publish times out both tests - removing the generation fence fails the stale test with ["control-before", "steer-stale", "control-after"] — which also confirms the bracketing orders as intended rather than by luck * fix: gate interrupt on the OWNER's capability alone, not the route's Codex round 12. The comment above this gate already said 'the OWNER's recorded capability, not this replica's probe' — and then the code ANDed in isSteerPreemptSupported(), which is exactly this replica's probe. The contradiction dates to the original commit; round 6 made the gate owner-scoped and wrote that comment without removing the local conjunct. The route never seals. It enqueues and publishes an arm, neither of which touches the SDK, so during a rolling deploy a steer landing on an un-upgraded replica silently lost its interrupt even though the owner could seal. When the route IS the owner the probe is redundant anyway: the flag it would consult is the one this process wrote at createJob. The real degradation path is unchanged and still tested — an owner that recorded no capability relabels to an ordinary steer. The test that pinned the local probe asserted an impossible same-replica state (capable metadata plus an incapable local SDK, when the metadata is written from that probe); it now pins the mixed-SDK direction instead, and fails with 'Expected: true, Received: false' if the probe is put back. * fix: reconcile arms at handover, and stop holding the 202 on a publish Codex round 13, two of three findings. P2 — rearmQueuedPreempts only ever ADDED. A replica that merely read the job still installs a facade runtime and subscribes, so it can accept an arm and then miss the best-effort clear that follows the drain. HITL resume promotes that facade to owner, the union keeps the orphan, and the first resumed stream seals on a steer no longer in the queue, drains nothing, and truncates the resumed answer as preempt_incomplete. acquireResumedJobOwnership only sets ownedJobs, so nothing else was clearing it. The durable queue is the sole authority at a handover: arms it does not back are now disarmed and tombstoned, so an in-flight publish cannot revive them either. Worth recording that my own independent review raised this and my verifier refuted it. Codex found it separately; two reviewers converging should have outweighed one refutation. P2 — the route awaited the arm publish before answering. The 202 reports capability, not delivery, so the await could not change the response; it only exposed the caller to Redis latency after the queue item was already durable. A client that times out and retries mints a second steer while the first stays queued, injecting the same instruction twice, whereas a lost publish merely takes the tool-boundary fallback. Detached, with both outcomes logged. All three tests verified counterfactually: union-only rearm fails the two new handover specs, and re-awaiting the publish hangs the stalled-publish spec until jest kills it. * fix: snapshot arms before reading the queue at handover Codex round 14 — a regression from my own round-13 fix, and a worse failure than the one it corrected. Round 13 read the durable queue first, then tombstoned any armed id the snapshot did not back. But approvals.resolve reopens steering before reconciliation runs, so another replica can commit a preempt steer and publish its arm while the peek is in flight. That arm is then present locally but absent from a snapshot taken before the steer existed, so a LIVE interrupt the route already acknowledged got dropped — and tombstoned, which blocks the re-arm, making it unrecoverable rather than merely late. Fixed by inverting the two reads rather than by locking or paying a second round trip. A steer is durably enqueued BEFORE its arm is published, so any id in an arms-first snapshot was already queued when it was armed, and the later peek must observe it unless it has since drained — which is exactly the orphan this reconciliation exists to drop. Arms landing after the snapshot are simply not candidates. Also re-checks runtime identity across the await, since the generation can be replaced while the queue read is in flight. New spec injects a steer + arm during the peek and verifies it survives; against the round-13 ordering it fails with Received array: []. * fix: bound the cancel disarm wait and fence enqueue to its generation Codex round 15. P2 — the cancel awaited its disarm publish unbounded. ioredis queues commands during an outage rather than rejecting, so that await could hang for the length of the outage with the steer ALREADY durably cancelled; a client that gives up then treats the cancel as failed and restores a chip for a steer that can never produce an applied event. Every successful cancel publishes, so ordinary steers were exposed too, not only preemptive ones. Now bounded at 1s, with the publish continuing behind it — its retry and logging are unchanged, it is just no longer in front of the response. This is the sibling of round 13's arm-publish finding; I fixed one path and left this one. P3 — enqueue was not fenced to the generation the capability decision was made against. The access checks, file resolution and owner re-read are all awaits, so the run can be replaced before the enqueue: the item then lands on the REPLACEMENT queue while the durable preempt flag and the arm still name the previous epoch, the arm is fenced out at the owner, and the 202 promises an interrupt that cannot happen. enqueueSteer now takes an expected generation, mirroring drain/peek, and the Redis path enforces it inside STEER_ENQUEUE_LUA so the check is atomic with the push rather than racing it. All three new specs verified counterfactually, including the Lua guard against real Redis (removing it returns 1 where -1 is required). * ⚡ feat: Interrupt & Steer — client half (PR 3 of 3) Makes preemptive steering reachable. Consumes the server contract from PR 2 (POST /chat/steer `preempt`, echoed on the 202) and the SDK seam in @librechat/agents 3.3.5. Settings shape follows the agreed correction, NOT the earlier plan draft: `steerInterruptsByDefault` is a boolean ORTHOGONAL to `duringRunDefaultAction` — that enum still chooses steer-vs-queue, the new boolean chooses how soon a steer lands. This deliberately avoids widening the enum to three values, which would have silently broken two hard-coded binary TOGGLES (`DuringRunAction.tsx`'s setter and `SteerMenu`'s `useDefaultToggleEntry`, both `prev === 'steer' ? … : …`) where a third value collapses to the wrong branch and one click erases the setting. - useSteering: `submitSteer` takes an opts bag and threads `preempt` into the POST, the optimistic chip, and the failure chip. The ACK relabels from the SERVER's echo, so a deployment that cannot seal mid-stream downgrades the chip's wording instead of erroring — the entire UX surface of capability degradation. New `interruptSteer` reuses the whole chip lifecycle and degradation ladder, and falls back to `interruptAndSend` when `!canSteer`, because steering needs a server-side job and an always-visible button would otherwise be dead for the whole first turn. `steerFromComposer` honours the new preference. - Composer: always-visible `InterruptSteerButton` with one fixed meaning (stop now, keep what's written), disabled on a paused run to pre-empt the server's 409, `type="button"` so it never steals the form's Enter submit, RTL-correct margins. A fourth hovercard row on the during-run send button, and ⌘/Ctrl+Shift+Enter routed AHEAD of the bare ⌘/Ctrl+Enter branch that would otherwise swallow it. - Chips: an in-flight preempt chip reads "Interrupting" with a ZapOff glyph; `preempt` survives reconnect through `seedSteerChips`. - `RunEnd.interruptArmed`, `drainAfterAbortByIndex`, `useQueueDrain`, `stopGenerating` and `interruptAndSend` are untouched — the preempt path deliberately shares none of the abort machinery. Tests: 7 new specs (posts preempt, turn-1 fallback, empty-text refusal, default route with and without the preference, server-echo relabel, double-click). 66 useSteering specs green; tsc and lint clean. Round-1 review fixes folded in: - P1: interrupt & steer no longer hard-aborts a run paused on tool approval. `canSteer` is false there, so the fallback was routing the keyboard and hovercard paths into `interruptAndSend` — discarding the partial answer, the exact opposite of what the action promises. The fallback is now scoped to the missing-conversation case only, and a paused run refuses outright (the standalone button was already disabled; the guard now lives where all three paths reach it). - The preference no longer leaks into the explicit Steer action. `steerFromComposer` backs both the default Enter route AND the explicit hovercard row / Ctrl+Enter alternate; applying `steerInterruptsByDefault` inside it made ordinary Steer interrupt and the two rows indistinguishable. It now takes an explicit argument that only `submitDuringRun`'s default route sets. - Retry preserves preemption: a failed interrupt-steer chip keeps `preempt: true`, and `retrySteer` now forwards it rather than silently resubmitting as an ordinary tool-boundary steer. - ⌘/Ctrl+Shift+Enter defers to a rebound submit shortcut, mirroring the bare ⌘/Ctrl+Enter branch — a user who bound submit to that chord keeps getting submit. Round-2 fix: the preempt label now survives the page-reload resume path too. `seedSteerChips` (useResumableSSE) and `restoreSteerChips` (useResumeOnLoad) are two independent TPendingSteer→PendingSteer mappers with near-identical bodies; the first carried the flag and the second silently dropped it, so an armed interrupt reverted to plain "Steering" after a reload. Swept: those are the only two in production code. The reclaim/convert paths deliberately omit it — a queued follow-up starts its own turn, so there is nothing to interrupt. * fix: yield the interrupt-steer chord only to a submit shortcut bound to it The previous guard skipped the Ctrl/Cmd+Shift+Enter branch whenever ANY submitMessage override existed. Rebinding submit to something unrelated (Ctrl+J) or unbinding it entirely then fell through to the override resolver, which returns 'none' for shifted Enter — silently removing the shortcut the hovercard still advertises. Compare the pressed chord against the configured one instead. The adjacent bare Ctrl/Cmd+Enter branch keeps its any-override guard on purpose: that chord IS the default submit chord, so once submit moves the resolver should own it. The predicate already existed inside resolveSubmitOverrideAction; pulled it out as bindingsMatch so both sites compare chords the same way. That call is behavior-preserving — eventBinding.key is 'Enter' by the early return, and equal hashes imply equal keys, so the dropped explicit key check was redundant. * fix: disable the Interrupt & steer menu row while paused on approval interruptSteer hard-refuses when the run is paused for tool approval, but the hovercard row was never gated, so it rendered enabled and clicking it did nothing at all — no chip, no queue entry, no toast — at exactly the moment a user is trying to say "stop, don't run that command". The standalone button already gates on pausedOnApproval; the row contradicted it. Gated on pausedOnApproval rather than !canSteer like the steer row above, because canSteer is also false before a conversation exists, where interruptSteer deliberately falls back to interruptAndSend and the row must stay live for the whole first turn. Tests pin both directions and were verified counterfactually: removing the gate fails the paused case, and using !canSteer fails the first-turn case. * test: render the during-run hovercard eagerly instead of driving Ariakit The new spec passed locally and failed all four cases on CI's Ubuntu and Windows shards: Ariakit's show path keys off pointer geometry, which jsdom reports as zeros, so whether a synthetic mouseEnter opens the hovercard is environment-dependent. Driving it was testing Ariakit's hover behavior, not which rows this component disables. Mocking the three Ariakit primitives renders the rows unconditionally and drops the fake timers. Both counterfactuals still fail as they should: removing the gate fails the paused case, !canSteer fails the first-turn case. * test(e2e): cover interrupt & steer sealing mid-stream The mock Playwright suite covered every sibling during-run action — steer at a tool boundary, steer degrading to a queued follow-up, queue, and interrupt & send — but not interrupt & steer, the one this stack adds. Uses E2E_SLOW_REPLY, which streams pure text with no tools, so the scenario is the same one where an ordinary steer provably degrades to a queued follow-up turn. Injecting in-thread there is something only a mid-stream seal can do, which makes the assertion discriminating rather than incidental: the steer part lands in the response, the final chunk never arrives, the text written before the seal survives, and no follow-up turn pair is created. * test(e2e): assert the run resumes after the seal, not just that it sealed The other four assertions are all satisfied by a seal that killed the run: the steer part is persisted by applySteer during the drain, before the continuation starts, so 'sealed and resumed' and 'sealed and died' were indistinguishable — and resuming is the whole difference from interrupt & send. The continuation answers the injected steer, whose text carries no fake-model marker, so getLatestUserText falls through to the default reply. That string ('E2E mock reply') is distinct from the setup turn's ('E2E reply <label>'), so seeing it proves generation restarted rather than matching text that was already on screen. The test itself is confirmed working: it ran as 104/121 in the Playwright job on 0ece357 and the run concluded success. * test(e2e): drop the resume assertion pending an unresolved question The assertion that the run visibly resumes after the seal fails deterministically in CI across all three retries. Every other assertion in the test passes, so the chord, the server preempt, the seal, and the in-thread injection all work; what fails is only the continuation's reply becoming visible. I could not determine from CI logs whether that is the mock harness not surfacing a continuation in a no-tool scenario or generation genuinely stopping after the seal, and I am not willing to weaken it into something that passes either way — that would convert a real question into false assurance. Reverted to the four assertions that hold, with the open question recorded in the test and raised on the PR. * fix: yield the interrupt chord to any bound shortcut, and move the Enter hint Codex round 1 on this PR (its first — #14519's rounds predate these files). P2 — the chord yielded only to a rebound submitMessage. This composer handler runs before the document-level one in useKeyboardShortcuts, and that one does not check defaultPrevented, so binding any composer-allowed action (focusChat, focusSearch, showShortcuts) to Ctrl/Cmd+Shift+Enter fired BOTH: the run was interrupted and the bound action ran. Now yields to any chord the user has bound. No default binding uses this chord, so it only ever yields to a deliberate rebinding. P2 — with steerInterruptsByDefault on, plain Enter routes through submitDuringRun and preempts, but the hovercard still put the ⏎ hint on the ordinary Steer row, whose click deliberately does NOT preempt. The same row advertised a key that did something else. The hint now follows the preference: ⏎ moves to Interrupt & steer, and the Steer row shows none because no key reaches it in that mode. I had declined this twice on the grounds that the behaviour split is deliberate — it is, and it is unchanged; the finding was about the label, which was a different claim and a correct one. Lint also caught a real bug in the first fix: boundShortcutChords was missing from the handler's dependency array, so a rebinding would not have taken effect until the callback was recreated for another reason. Both verified counterfactually; 179 client specs green. * fix: resolve every composer Enter chord through one decision table Codex round 2: yielding the preempt branch to a bound chord dropped execution into the bare Ctrl/Cmd branch below it, which ignores Shift, and past that into the submit tail, where isCtrlEnter is true for the chord. So rebinding focusChat, focusSearch, or showShortcuts to Ctrl/Cmd+Shift+Enter fired the alternate action or a submit AND the document-level shortcut, consuming the draft. Two rounds in a row landed in this handler because the guard chain decided "whose chord is this?" piecemeal inside individual branches, with fall-through between them. This extracts the entire pipeline into resolveComposerKeyDown (utils/shortcuts, beside resolveSubmitOverrideAction, which it absorbs as a step): one pure decision table where every verdict is terminal, so the fall-through class is gone rather than patched around. The yield rule is now the first gate before all branches: an Enter chord bound to any shortcut the document handler runs while typing (EDITING_ALLOWED_SHORTCUTS, hoisted out of the handler and shared by both dispatchers) is left entirely to that handler. This also closes the identical latent holes in the branches codex did not flag: bound Alt+Enter and Ctrl/Cmd+Enter chords double-fired the same way during a run, and the idle submit tail consumed bound chords too. A chord bound to a shortcut the document handler does NOT run while typing keeps its composer meaning; there is nothing to collide with, and yielding would just make the chord dead. Also merged the spec file round 0 added at utils/__tests__/ into the pre-existing utils/shortcuts.spec.ts (duplicate coverage at a second path) and dropped the isNonShiftEnter+filesLoading preventDefault, which was subsumed by the unconditional one below it. The table has a spec locking every verdict. Counterfactually verified: removing the yield gate fails exactly the three yield tests. Full client suite green apart from six suites that fail identically without this change (local data-provider dist drift in unrelated areas). * fix: yield Alt+Enter to a rebound submit, derive hovercard hints from the decision table Codex round 3, both findings. P2, Alt+Enter submit rebinding. The interrupt branch reserved Alt+Enter unconditionally, so rebinding submitMessage to Alt+Enter meant the user's own submit chord aborted the run and consumed the draft as interrupt & send. Now guarded with the same bindingsMatch yield the preempt branch already had; the chord falls through to the submit override resolution and submits the default action. P2, hovercard hints. The rows advertised hardcoded chords, so a chord rebound to an editing-allowed global shortcut (yielded by the decision table) or claimed by a rebound submit still appeared as Steer, Queue, Interrupt & steer, or Interrupt & send. The hovercard now asks the same decision table the composer executes what each canonical chord does and only labels a row with a chord that still triggers it. That also fixes two dishonest hints codex did not flag: with Enter-to-send off, plain Enter inserts a newline during a run (the primary row advertised it anyway) and Ctrl/Cmd+Enter submits the default action (the alternate row claimed it). The default-action hint now moves to Ctrl/Cmd+Enter in that mode. The effective bindings (submitOverride plus yielded chords) moved from useTextarea into a shared useComposerBindings hook so the handler and the hints read the same source. resolveComposerKeyDown now takes a KeyChordSource pick of the event fields it reads, letting the hovercard pass synthetic chords. Counterfactually verified: reverting the Alt guard fails the new resolver test and the hint suppression test. 353 tests across the affected suites green; the two failing hook suites (useVisibleTools, useResumableSSE) fail identically without these changes (local data-provider dist drift). * fix: never advertise a chord on a disabled hovercard row Codex round 4, one P2. The Interrupt & steer row kept showing its chord while disabled for tool approval, but pressing it reaches interruptSteer's pausedOnApproval guard and no-ops. Fixed as a render rule rather than a per-row patch: a disabled row never shows its kbd, since its action refuses the chord by the same guard that disabled it. That also covers the disabled Steer row, whose alternate-action hint had the identical hole through steerFromComposer's canSteer refusal. Counterfactually verified: reverting the render guard fails the new test.
) * 🛑 feat: Preemptive Steer — server half (Interrupt & Steer, PR 2 of 3) Lets the steer route ask the generating replica to seal its live model stream at the next provider-safe boundary instead of waiting for a tool step. The run is never aborted, job status never changes, the partial answer is kept, and generation resumes in the same assistant message after the injected steer. Consumes the SDK seam in @librechat/agents (LibreChat-AI/agents#335, LibreChat-AI#346). Transport: IEventTransport gains a fenced emitPreempt/onPreempt pair beside abort. RedisEventTransport fans PREEMPT out on the SAME events channel and subscription (no new connection, key, or subscribe call); onPreempt returns a registration-scoped unsubscribe with the same replacement-safe state-identity guard onAbort uses. InMemory implements neither — single-process preempt lives entirely in the runtime set. Runtime state: RuntimeJobState carries the per-generation request set, createdAt-fenced and capped at STEER_QUEUE_MAX_DEPTH, plus a bounded `cleared` tombstone so a late cross-replica arm cannot resurrect a request whose steer already drained. registerPreemptSubscription mirrors the abort registration's double fence (runtime identity + generation createdAt); releaseAbortSubscription retires BOTH listeners and the armed set, so every terminal path drops preempt state for free. Public surface: requestPreempt (arm + fenced publish, never a rejection surface, never touches job status), isPreemptRequested (O(1) level-triggered poll), noteSteersRemoved (drain/cancel bookkeeping + fenced clear), clearPreemptRequests (empty-boundary disarm). One drain body, two boundaries: createSteerDrainHook (PostToolBatch) and createSteerPreemptBoundaryHook (PreemptBoundary) share drainAndBuildInjections, so the two injection sites cannot drift — the SDK's provider-safety argument rests on identical HumanMessage shapes. The shared body builds injections incrementally under a swallow-all catch (a mid-loop throw still injects what was applied — those parts are already persisted), clears preempt requests in finally, and disarms the generation when a boundary drains nothing. Request path: POST /chat/steer accepts preempt: true. The guard ladder is unchanged in order and in every status code. A preempt request is NEVER a rejection reason — without the capability the steer still enqueues and the 202 echoes preempt: false. Armed strictly after a successful enqueue; cancel disarms. The capability is read from the OWNING replica's recorded `preemptCapable` rather than the route replica's own SDK probe, so a rolling deploy cannot label a steer "interrupting" that the older owner will only inject at a tool step. Durable label: SteerQueueItem.preempt → TPendingSteer.preempt, so a parked/claimed/replayed chip keeps its wording. Run wiring: createRun registers the PreemptBoundary hook and threads RunConfig.preemption, both gated on isSteerPreemptSupported() — a separate probe from isSteeringSupported(), so the client affordance can never arm against an SDK that only injects at tool boundaries. buildSteerWiring builds both hooks from one shared closures object, so preemption survives HITL pause/resume for free. Honest finalization: an empty preempt boundary persists and emits with unfinished: true — the same contract an abort gets — re-marked explicitly because BaseClient has already saved the row as unfinished: false by that point. Not changed: no new job status, store method, Lua, SSE event type, endpoint, or authorization surface. abortJob, completeJob, transitionStatus, closeAndDrainSteers, getResumeState, emitChunk, applySteerPart and the whole abort path are untouched. Tests: 120 packages/api steering specs (preempt lifecycle, tombstone, fences, caps, terminal release, both-boundary drain parity, level-triggered poll, request/cancel arming, owner-capability degradation) plus 5 in api for buildSteerWiring gating, and 2 Redis-gated cross-replica transport specs. * 🔒 fix: Codex round 2 — evict tombstones, scope the empty-boundary disarm, honest resumes All four server findings were fresh consequences of the round-1 fixes, which is the review doing exactly what it should. - Tombstone cap refused new entries instead of evicting. Every drained or cancelled steer is tombstoned, not just preempting ones, so a generation that processed 20 steers exhausted the set and the late-arm race resurfaced silently. Now evicts oldest-first (Set iteration is insertion-ordered), with the budget named PREEMPT_TOMBSTONE_MAX rather than an inline expression. - The empty-boundary disarm I added in round 1 wiped the generation's ENTIRE armed set. A second steer can enqueue and arm between the atomic drain returning empty and the disarm running — that arm is backed by a live, uninjected queue item and must survive. The drain now snapshots the armed ids BEFORE draining (getArmedPreemptIds) and clearPreemptRequests takes an explicit id list instead of clearing everything. - HITL resume finalized with a hardcoded unfinished: false. The boundary hook is re-registered on resume via buildSteerWiring, so a resumed segment can end on an empty preempt boundary exactly like a fresh one; finalizeResumedTurn now reads getPreemptStats() and the halt reason, matching the normal request path. - Ownership moves on resume, so the job's recorded preemptCapable must describe the replica that will actually generate. Refreshed before resumeCompletion; a job created on a capable replica that resumes on an older one during a rolling deploy no longer acknowledges steers as interrupting. Tests: +3 (scoped disarm sparing a post-snapshot arm, oldest-first tombstone eviction, id-list disarm). 122 packages/api steering specs green. * 🚨 fix: Codex round 3 — deserialize preemptCapable from Redis (feature was dead under Redis) The P1 here is the most consequential defect in the whole feature, and it was introduced by round 1's own capability fix. - `RedisJobStore.serializeJob` writes booleans generically, so `preemptCapable` reached Redis — but `deserializeJob` is an EXPLICIT field map and had no line for it. Every `getJob()` therefore dropped the flag, `job.metadata.preemptCapable` was always undefined, and `handleSteerRequest` computed `preemptArmed: false` unconditionally. Interrupt & steer would have silently degraded to ordinary tool-boundary steering in EVERY Redis deployment — i.e. the feature shipping as a no-op in production while passing every in-memory test. Now deserialized, with a round-trip assertion in the metadata spec that fails (`Received: undefined`) against the unfixed store. - The resume capability refresh moved from just-before `resumeCompletion` to immediately after `approvals.resolve` claims the run. That call already flips the job back to `running`, so the steer route accepts requests from that instant; leaving the refresh 135 lines later (across the whole client reconstruction) left a real window where a steer read the PREVIOUS owner's capability. Not the fully atomic transition Codex suggested — that reaches into the approvals Lua — but it shrinks the window from seconds to one await, which is proportionate for a label-accuracy issue. Refuted: "avoid triggering preemption inside subagents". The premise — that the run-wide poll can seal a subagent stream — does not hold against the shipped SDK. Child graphs are constructed with `subagentScope: true` (SubagentExecutor) and `preemption` is NOT propagated into child inputs, while `canClaimPreemptSeal()` requires `!subagentScope && preemption != null`. Both conditions fail independently, so a subagent can never claim a seal and the boundary cannot fire with `agentId` set. The `input.agentId != null` guard in the hook is defensive depth, not the thing standing between us and the described failure. 140 packages/api specs green. * 🔁 fix: Codex round 4 — re-arm durable interrupt steers when resume moves owners - An arm lives only in the owning replica's runtime plus a transient pub/sub message, while the steer's `preempt` flag is durable on the queue item. A HITL resume landing on a different replica therefore started with an empty armed set and a poll stuck false, so an interrupt the user had already been ACKed for silently waited for an ordinary tool boundary. New `GenerationJobManager.rearmQueuedPreempts` rebuilds the armed set by peeking the durable queue (fenced on the generation) and re-arming every item flagged `preempt`; resume calls it right after claiming. Safe by construction: every item peeked is still queued, so no drained steer can be resurrected. - Capability-refresh failure now logs at error rather than warn, but deliberately does NOT fail the resume — see the reply on that thread. Tests: +2 (rebuild from queue arms only the flagged item and reports the count; a stale generation arms nothing). 124 packages/api steering specs green. * 📡 fix: Codex round 5 — acknowledge only what was actually armed - A cross-replica arm was fire-and-forget: `emitPreempt` logged its own publish failure and `requestPreempt` returned void, so the route answered `preempt: true` even when the owner never armed a poll. The steer still injected at the next tool boundary, but the chip claimed an interrupt that could not happen — and unlike HITL resume, an ordinary running generation had no durable reconciliation to recover it. `emitPreempt` now resolves to the subscriber count and rejects on failure; `requestPreempt` is async and returns whether the arm truly landed (owned locally, or delivered to at least one subscriber). The 202 reports THAT rather than what was asked for, so the chip relabels to ordinary steering exactly as it does for a capability-degraded deployment. Errors are swallowed into `false` — an unarmed interrupt is a downgrade, never a failed steer. - The owner capability is re-read immediately before enqueue rather than reused from the top of the guard ladder. `checkAgentAccess` and file resolution are awaits, so a request can span an entire HITL pause/resume that moves ownership to a replica with different capability and rewrites that very flag. Only paid for by requests that actually asked to interrupt. Tests: +3 (not-armed when the publish reaches nobody; armed when this replica owns the generation; a throwing publish downgrades instead of propagating). 127 packages/api steering specs green. * 🎯 fix: Codex round 6 — real ownership, confirmed disarms, and a CI regression of my own Three review findings plus three CI failures the round-5 commit caused. Review: - Ownership came from `runtimeState`, which a cross-replica `getJob` populates with a FACADE runtime on any replica that merely read the job. Matching `createdAt` therefore proved only "we looked at this job", so a non-owner could arm nothing and report success. Ownership now comes from `ownedJobs`, the actual owner map. - `armPreemptIds` returns how many ids it accepted, and a local arm is only reported as armed when one was. A tombstoned id (its steer drained at an ordinary boundary mid-request) no longer answers `preempt: true` for an interrupt that cannot happen. - The cancel disarm is awaited. A dropped clear is worse than a dropped arm: the owner keeps a level-triggered request for a steer that no longer exists, seals its next chunk and truncates an unrelated answer. The boundary drain's own call stays non-blocking — there the owner is local, so the disarm is already effective and awaiting the informational publish would only delay injection. - Subscriber count is NOT read as proof of owner receipt: the count includes this replica's own facade subscription. A successful publish reports armed, a rejected one does not. Documented rather than papered over — see the acknowledgement-semantics note on the PR. CI regressions from round 5, all mine: - `registerPreemptSubscription` was AWAITED at both runtime-init sites, so job creation blocked on a second Redis channel subscription and hung when that subscribe was slow. Abort is awaited because a missed abort strands a run; a missed preempt only degrades that steer to the next tool boundary, so it now registers without gating createJob. - Two api specs mocked `@librechat/api` without the newly imported `isSteerPreemptSupported`, so the call threw before createJob; and one exact-match assertion needed the new `preemptCapable` metadata field. - My own Redis integration spec asserted arm-before-clear ordering, which two publishes carry no guarantee of — the receiving tombstone exists precisely because of that. Now asserts delivery and payload fidelity, order-independent. 158 packages/api specs, 27 api specs green. * 🧭 fix: Codex round 7 — settle the acknowledgement semantics (Option A) Round 7's second finding is the incoherence I flagged on the PR: the route persisted `preempt: true` on the durable queue item while returning `preempt: false` when delivery could not be confirmed. Those two then disagreed, and `rearmQueuedPreempts` trusts the DURABLE one — so a resumed owner would honour an interrupt the client had explicitly been told degraded to ordinary steering. Rather than patch the disagreement, this settles the meaning: `preempt` in the 202 means "queued as an interrupt request", NOT "a seal is guaranteed". It mirrors `SteerQueueItem.preempt` exactly, so the response, the durable record, and the resume-time re-arm can never disagree. The gates that ARE knowable stay — the owner's recorded capability and a successful enqueue. Everything past that degrades to the documented fallback of injecting at the next tool boundary. A route cannot synchronously know whether another replica will seal: proving it needs a correlated request/response over pub-sub, and even that only proves the owner heard, not that it is still streaming when the arm lands. Four rounds of tightening this boolean each surfaced a narrower case; the sequence does not converge, so the invariant is now "the flag describes the durable decision" and an unconfirmed arm logs a warning instead of rewriting the answer. Also from this round: a failed disarm publish is retried once and its outcome reported. `handleSteerCancel` keeps `removed: true` — the steer really did leave the queue, and saying otherwise would make the client re-show a chip for a steer that can never arrive — and adds `disarmed: false` so the residual risk is visible rather than swallowed. Damage stays bounded regardless: the empty-boundary self-clear disarms the generation after a single seal. Tests: +1 pinning the response/durable-flag invariant. 159 packages/api specs green. * 🧹 fix: Codex round 8 — remove the unverifiable disarm signal Round 8 found the same over-promise on the disarm side that round 7 corrected on the arm side, so this applies the same answer rather than patching around it. The `disarmed: false` field added in round 7 was both unreliable and unused: a resolved publish is not proof the owner heard it (the delivery count includes this replica's own facade subscription), and it was never threaded into `CancelSteerResponse` or read by any client. A signal that claims a certainty the transport cannot provide is worse than no signal — it invites callers to trust it. Removed from the response. The retry stays, because it genuinely reduces the failure rate, and `noteSteersRemoved` still returns whether the publish succeeded FOR LOGGING, now documented explicitly as "published without error", not "the owner disarmed". Disarm is best effort with a bounded, self-healing failure: if the clear is lost the owner seals once, the empty-boundary self-clear disarms the generation, and the turn is persisted `unfinished: true` rather than silently truncated. Tightening that further needs a correlated request/response over pub-sub with a timeout — noted on the PR as the deliberate boundary of this design rather than an oversight. 130 packages/api steering specs green. * 🧽 fix: Codex round 9 — spend snapshot arms on nonempty drains too The round-6 scoping fix only cleared the pre-drain snapshot when the drain came back EMPTY. On a nonempty drain the `finally` cleared just the drained ids, so a stale arm — typically a cancel whose cross-replica clear was lost — survived the boundary. It would then immediately seal the continuation meant to answer the steer that had just been injected, and land on an empty boundary as `preempt_incomplete`: the interrupt appears to work, and the answer to it is truncated. A boundary that runs has spent its seal, so everything armed at snapshot time is spent whether or not it came back from the drain. The `finally` now clears the union of the snapshot and the drained ids. Arms that land AFTER the snapshot are still spared — their queue items are live and uninjected, which is the property round 6 added. Also fixes an api-workspace CI failure of mine: `resume.spec.js` stubs `GenerationJobManager` wholesale, and the round-3/4 resume work added two calls (`updateMetadata`, `rearmQueuedPreempts`) the stub did not define, so 34 specs threw. Stub extended. Tests: +2 (a nonempty drain clears a stale snapshot arm; a nonempty drain spares an arm that landed mid-drain). Counterfactually verified — the stale-arm spec fails against the unfixed drain. 132 packages/api specs, 60 resume specs green. * fix: never let a failed preempt subscription reject into the void registerPreemptSubscription is called detached at both sites, so a rejected Redis SUBSCRIBE became an unhandled rejection — process-fatal under Node's default --unhandled-rejections=throw. The comment already promised this path merely degrades steering; it now does. Swallowed and logged inside the registration rather than at each call site, so a future third caller cannot reintroduce the trap. Losing the channel costs this generation's cross-replica preempts, not the server: same-replica arming is runtime state and still works, and remote arms fall back to the next tool boundary. Verified counterfactually — the new spec surfaces SUBSCRIBE failed as an unhandled rejection against the unfixed registration. * docs: state the real blast radius of a failed preempt subscription LibreChat's own entrypoints install a global unhandledRejection handler that logs and keeps serving, so the escaping rejection this guards was never fatal to this server — only to another consumer of @librechat/api that installs no handler. The fix stands either way; the comment just should not overstate what it prevents. * test: cover the cross-replica preempt hop with two manager instances Every other preempt test runs against a single manager, so the hop that actually carries an interrupt in production had no coverage: the steer POST lands on whichever replica the balancer picks, which is usually not the one generating. Non-owner publishes, owner arms, owner's level-triggered poll flips — none of that was exercised end to end. Two GenerationJobManagerClass instances are a faithful replica pair here. runtimeState and ownedJobs are private instance fields, there is no module-level mutable state between them, and createStreamServices duplicates a dedicated subscriber connection per call, so separate OS processes would exercise the same objects over the same Redis. Both assertions verified counterfactually against real Redis: - Deleting the preemptCapable deserialization in RedisJobStore fails this with 'Expected: true, Received: undefined' — the exact P1 that shipped past every in-memory test and would have made the feature a silent no-op on every Redis deployment. - Dropping the non-owner arm publish fails it with 'Received: false'. * test: remove the fixed sleeps and vacuity from the cross-replica preempt test Codex round 11, both findings, both on the test I added last commit. P2 — the 300ms waits were load-bearing. Redis pub/sub never replays and the owner's SUBSCRIBE is detached, so on a slow CI worker the publish could land before anyone was listening and the test would fail against correct code. Now it republishes until the owner's state converges, which is safe because arms and clears are idempotent set writes keyed by steerId. Side effect: the tests got ~10x faster (85ms/57ms vs 929ms/606ms) since they finish on delivery rather than on a timer. P3 — afterEach destroyed only the transports, leaving each manager alive in its own cleanup-interval closure, still working against a dead transport. Now tracks the managers and awaits destroy(), which disposes the job store and its timer too. Matches how the rest of this file cleans up. Fixing the sleeps exposed a third problem codex did not flag: the stale-arm test could pass vacuously, because an undelivered arm and a fenced one look identical. It now brackets the stale publish between two control arms — the first proves the owner is listening before the stale one is sent, the second proves it has had its chance to arrive. Verified counterfactually against real Redis, and stable over 5 runs: - dropping the preemptCapable deserialization fails with 'Received: undefined' - dropping the non-owner arm publish times out both tests - removing the generation fence fails the stale test with ["control-before", "steer-stale", "control-after"] — which also confirms the bracketing orders as intended rather than by luck * fix: gate interrupt on the OWNER's capability alone, not the route's Codex round 12. The comment above this gate already said 'the OWNER's recorded capability, not this replica's probe' — and then the code ANDed in isSteerPreemptSupported(), which is exactly this replica's probe. The contradiction dates to the original commit; round 6 made the gate owner-scoped and wrote that comment without removing the local conjunct. The route never seals. It enqueues and publishes an arm, neither of which touches the SDK, so during a rolling deploy a steer landing on an un-upgraded replica silently lost its interrupt even though the owner could seal. When the route IS the owner the probe is redundant anyway: the flag it would consult is the one this process wrote at createJob. The real degradation path is unchanged and still tested — an owner that recorded no capability relabels to an ordinary steer. The test that pinned the local probe asserted an impossible same-replica state (capable metadata plus an incapable local SDK, when the metadata is written from that probe); it now pins the mixed-SDK direction instead, and fails with 'Expected: true, Received: false' if the probe is put back. * fix: reconcile arms at handover, and stop holding the 202 on a publish Codex round 13, two of three findings. P2 — rearmQueuedPreempts only ever ADDED. A replica that merely read the job still installs a facade runtime and subscribes, so it can accept an arm and then miss the best-effort clear that follows the drain. HITL resume promotes that facade to owner, the union keeps the orphan, and the first resumed stream seals on a steer no longer in the queue, drains nothing, and truncates the resumed answer as preempt_incomplete. acquireResumedJobOwnership only sets ownedJobs, so nothing else was clearing it. The durable queue is the sole authority at a handover: arms it does not back are now disarmed and tombstoned, so an in-flight publish cannot revive them either. Worth recording that my own independent review raised this and my verifier refuted it. Codex found it separately; two reviewers converging should have outweighed one refutation. P2 — the route awaited the arm publish before answering. The 202 reports capability, not delivery, so the await could not change the response; it only exposed the caller to Redis latency after the queue item was already durable. A client that times out and retries mints a second steer while the first stays queued, injecting the same instruction twice, whereas a lost publish merely takes the tool-boundary fallback. Detached, with both outcomes logged. All three tests verified counterfactually: union-only rearm fails the two new handover specs, and re-awaiting the publish hangs the stalled-publish spec until jest kills it. * fix: snapshot arms before reading the queue at handover Codex round 14 — a regression from my own round-13 fix, and a worse failure than the one it corrected. Round 13 read the durable queue first, then tombstoned any armed id the snapshot did not back. But approvals.resolve reopens steering before reconciliation runs, so another replica can commit a preempt steer and publish its arm while the peek is in flight. That arm is then present locally but absent from a snapshot taken before the steer existed, so a LIVE interrupt the route already acknowledged got dropped — and tombstoned, which blocks the re-arm, making it unrecoverable rather than merely late. Fixed by inverting the two reads rather than by locking or paying a second round trip. A steer is durably enqueued BEFORE its arm is published, so any id in an arms-first snapshot was already queued when it was armed, and the later peek must observe it unless it has since drained — which is exactly the orphan this reconciliation exists to drop. Arms landing after the snapshot are simply not candidates. Also re-checks runtime identity across the await, since the generation can be replaced while the queue read is in flight. New spec injects a steer + arm during the peek and verifies it survives; against the round-13 ordering it fails with Received array: []. * fix: bound the cancel disarm wait and fence enqueue to its generation Codex round 15. P2 — the cancel awaited its disarm publish unbounded. ioredis queues commands during an outage rather than rejecting, so that await could hang for the length of the outage with the steer ALREADY durably cancelled; a client that gives up then treats the cancel as failed and restores a chip for a steer that can never produce an applied event. Every successful cancel publishes, so ordinary steers were exposed too, not only preemptive ones. Now bounded at 1s, with the publish continuing behind it — its retry and logging are unchanged, it is just no longer in front of the response. This is the sibling of round 13's arm-publish finding; I fixed one path and left this one. P3 — enqueue was not fenced to the generation the capability decision was made against. The access checks, file resolution and owner re-read are all awaits, so the run can be replaced before the enqueue: the item then lands on the REPLACEMENT queue while the durable preempt flag and the arm still name the previous epoch, the arm is fenced out at the owner, and the 202 promises an interrupt that cannot happen. enqueueSteer now takes an expected generation, mirroring drain/peek, and the Redis path enforces it inside STEER_ENQUEUE_LUA so the check is atomic with the push rather than racing it. All three new specs verified counterfactually, including the Lua guard against real Redis (removing it returns 1 where -1 is required). * fix: fence the steer to its authorized generation, bound resume setup, keep preempt when Redis parks Codex round 16, all three findings. P2 — round 15 fenced the enqueue to owner.createdAt, the RE-READ job. Every guard above it (ownership, tenant, paused-state, agent ACL) ran against the job read at the top, so if the run was replaced during those awaits the fence happily accepted the steer into a generation the request was never authorized against, carrying the wrong agent's metadata. Now rejects on any mismatch between the validated job and the re-read. P2 — resume awaited its steering bookkeeping unbounded, after approvals.resolve had consumed the action and flipped the job to running, and outside the resume lifecycle's own try/finally. .catch does not fire on a promise that never settles, which is what ioredis produces during an outage, so the client times out, its retry gets a 409 for a spent action, and no cleanup runs. Bounded at 1s with the writes finishing in the background. P3 — Redis parks leftover steers inside its terminal-transition Lua, which projects item fields one by one, so preempt was silently dropped and a steer recovered from /chat/status lost its interrupting label. Added to both projections. All three verified counterfactually, two against real Redis. Worth recording that my first version of the generation-mismatch test was VACUOUS — it faked a createdAt matching no live job, so the round-15 enqueue fence rejected it for the wrong reason and the test passed with the guard removed. Rewritten to replace the run for real; it now fails with 'Expected 404, Received 202'.
* 🛑 feat: Preemptive Steer — server half (Interrupt & Steer, PR 2 of 3) Lets the steer route ask the generating replica to seal its live model stream at the next provider-safe boundary instead of waiting for a tool step. The run is never aborted, job status never changes, the partial answer is kept, and generation resumes in the same assistant message after the injected steer. Consumes the SDK seam in @librechat/agents (LibreChat-AI/agents#335, LibreChat-AI#346). Transport: IEventTransport gains a fenced emitPreempt/onPreempt pair beside abort. RedisEventTransport fans PREEMPT out on the SAME events channel and subscription (no new connection, key, or subscribe call); onPreempt returns a registration-scoped unsubscribe with the same replacement-safe state-identity guard onAbort uses. InMemory implements neither — single-process preempt lives entirely in the runtime set. Runtime state: RuntimeJobState carries the per-generation request set, createdAt-fenced and capped at STEER_QUEUE_MAX_DEPTH, plus a bounded `cleared` tombstone so a late cross-replica arm cannot resurrect a request whose steer already drained. registerPreemptSubscription mirrors the abort registration's double fence (runtime identity + generation createdAt); releaseAbortSubscription retires BOTH listeners and the armed set, so every terminal path drops preempt state for free. Public surface: requestPreempt (arm + fenced publish, never a rejection surface, never touches job status), isPreemptRequested (O(1) level-triggered poll), noteSteersRemoved (drain/cancel bookkeeping + fenced clear), clearPreemptRequests (empty-boundary disarm). One drain body, two boundaries: createSteerDrainHook (PostToolBatch) and createSteerPreemptBoundaryHook (PreemptBoundary) share drainAndBuildInjections, so the two injection sites cannot drift — the SDK's provider-safety argument rests on identical HumanMessage shapes. The shared body builds injections incrementally under a swallow-all catch (a mid-loop throw still injects what was applied — those parts are already persisted), clears preempt requests in finally, and disarms the generation when a boundary drains nothing. Request path: POST /chat/steer accepts preempt: true. The guard ladder is unchanged in order and in every status code. A preempt request is NEVER a rejection reason — without the capability the steer still enqueues and the 202 echoes preempt: false. Armed strictly after a successful enqueue; cancel disarms. The capability is read from the OWNING replica's recorded `preemptCapable` rather than the route replica's own SDK probe, so a rolling deploy cannot label a steer "interrupting" that the older owner will only inject at a tool step. Durable label: SteerQueueItem.preempt → TPendingSteer.preempt, so a parked/claimed/replayed chip keeps its wording. Run wiring: createRun registers the PreemptBoundary hook and threads RunConfig.preemption, both gated on isSteerPreemptSupported() — a separate probe from isSteeringSupported(), so the client affordance can never arm against an SDK that only injects at tool boundaries. buildSteerWiring builds both hooks from one shared closures object, so preemption survives HITL pause/resume for free. Honest finalization: an empty preempt boundary persists and emits with unfinished: true — the same contract an abort gets — re-marked explicitly because BaseClient has already saved the row as unfinished: false by that point. Not changed: no new job status, store method, Lua, SSE event type, endpoint, or authorization surface. abortJob, completeJob, transitionStatus, closeAndDrainSteers, getResumeState, emitChunk, applySteerPart and the whole abort path are untouched. Tests: 120 packages/api steering specs (preempt lifecycle, tombstone, fences, caps, terminal release, both-boundary drain parity, level-triggered poll, request/cancel arming, owner-capability degradation) plus 5 in api for buildSteerWiring gating, and 2 Redis-gated cross-replica transport specs. * 🔒 fix: Codex round 2 — evict tombstones, scope the empty-boundary disarm, honest resumes All four server findings were fresh consequences of the round-1 fixes, which is the review doing exactly what it should. - Tombstone cap refused new entries instead of evicting. Every drained or cancelled steer is tombstoned, not just preempting ones, so a generation that processed 20 steers exhausted the set and the late-arm race resurfaced silently. Now evicts oldest-first (Set iteration is insertion-ordered), with the budget named PREEMPT_TOMBSTONE_MAX rather than an inline expression. - The empty-boundary disarm I added in round 1 wiped the generation's ENTIRE armed set. A second steer can enqueue and arm between the atomic drain returning empty and the disarm running — that arm is backed by a live, uninjected queue item and must survive. The drain now snapshots the armed ids BEFORE draining (getArmedPreemptIds) and clearPreemptRequests takes an explicit id list instead of clearing everything. - HITL resume finalized with a hardcoded unfinished: false. The boundary hook is re-registered on resume via buildSteerWiring, so a resumed segment can end on an empty preempt boundary exactly like a fresh one; finalizeResumedTurn now reads getPreemptStats() and the halt reason, matching the normal request path. - Ownership moves on resume, so the job's recorded preemptCapable must describe the replica that will actually generate. Refreshed before resumeCompletion; a job created on a capable replica that resumes on an older one during a rolling deploy no longer acknowledges steers as interrupting. Tests: +3 (scoped disarm sparing a post-snapshot arm, oldest-first tombstone eviction, id-list disarm). 122 packages/api steering specs green. * 🚨 fix: Codex round 3 — deserialize preemptCapable from Redis (feature was dead under Redis) The P1 here is the most consequential defect in the whole feature, and it was introduced by round 1's own capability fix. - `RedisJobStore.serializeJob` writes booleans generically, so `preemptCapable` reached Redis — but `deserializeJob` is an EXPLICIT field map and had no line for it. Every `getJob()` therefore dropped the flag, `job.metadata.preemptCapable` was always undefined, and `handleSteerRequest` computed `preemptArmed: false` unconditionally. Interrupt & steer would have silently degraded to ordinary tool-boundary steering in EVERY Redis deployment — i.e. the feature shipping as a no-op in production while passing every in-memory test. Now deserialized, with a round-trip assertion in the metadata spec that fails (`Received: undefined`) against the unfixed store. - The resume capability refresh moved from just-before `resumeCompletion` to immediately after `approvals.resolve` claims the run. That call already flips the job back to `running`, so the steer route accepts requests from that instant; leaving the refresh 135 lines later (across the whole client reconstruction) left a real window where a steer read the PREVIOUS owner's capability. Not the fully atomic transition Codex suggested — that reaches into the approvals Lua — but it shrinks the window from seconds to one await, which is proportionate for a label-accuracy issue. Refuted: "avoid triggering preemption inside subagents". The premise — that the run-wide poll can seal a subagent stream — does not hold against the shipped SDK. Child graphs are constructed with `subagentScope: true` (SubagentExecutor) and `preemption` is NOT propagated into child inputs, while `canClaimPreemptSeal()` requires `!subagentScope && preemption != null`. Both conditions fail independently, so a subagent can never claim a seal and the boundary cannot fire with `agentId` set. The `input.agentId != null` guard in the hook is defensive depth, not the thing standing between us and the described failure. 140 packages/api specs green. * 🔁 fix: Codex round 4 — re-arm durable interrupt steers when resume moves owners - An arm lives only in the owning replica's runtime plus a transient pub/sub message, while the steer's `preempt` flag is durable on the queue item. A HITL resume landing on a different replica therefore started with an empty armed set and a poll stuck false, so an interrupt the user had already been ACKed for silently waited for an ordinary tool boundary. New `GenerationJobManager.rearmQueuedPreempts` rebuilds the armed set by peeking the durable queue (fenced on the generation) and re-arming every item flagged `preempt`; resume calls it right after claiming. Safe by construction: every item peeked is still queued, so no drained steer can be resurrected. - Capability-refresh failure now logs at error rather than warn, but deliberately does NOT fail the resume — see the reply on that thread. Tests: +2 (rebuild from queue arms only the flagged item and reports the count; a stale generation arms nothing). 124 packages/api steering specs green. * 📡 fix: Codex round 5 — acknowledge only what was actually armed - A cross-replica arm was fire-and-forget: `emitPreempt` logged its own publish failure and `requestPreempt` returned void, so the route answered `preempt: true` even when the owner never armed a poll. The steer still injected at the next tool boundary, but the chip claimed an interrupt that could not happen — and unlike HITL resume, an ordinary running generation had no durable reconciliation to recover it. `emitPreempt` now resolves to the subscriber count and rejects on failure; `requestPreempt` is async and returns whether the arm truly landed (owned locally, or delivered to at least one subscriber). The 202 reports THAT rather than what was asked for, so the chip relabels to ordinary steering exactly as it does for a capability-degraded deployment. Errors are swallowed into `false` — an unarmed interrupt is a downgrade, never a failed steer. - The owner capability is re-read immediately before enqueue rather than reused from the top of the guard ladder. `checkAgentAccess` and file resolution are awaits, so a request can span an entire HITL pause/resume that moves ownership to a replica with different capability and rewrites that very flag. Only paid for by requests that actually asked to interrupt. Tests: +3 (not-armed when the publish reaches nobody; armed when this replica owns the generation; a throwing publish downgrades instead of propagating). 127 packages/api steering specs green. * 🎯 fix: Codex round 6 — real ownership, confirmed disarms, and a CI regression of my own Three review findings plus three CI failures the round-5 commit caused. Review: - Ownership came from `runtimeState`, which a cross-replica `getJob` populates with a FACADE runtime on any replica that merely read the job. Matching `createdAt` therefore proved only "we looked at this job", so a non-owner could arm nothing and report success. Ownership now comes from `ownedJobs`, the actual owner map. - `armPreemptIds` returns how many ids it accepted, and a local arm is only reported as armed when one was. A tombstoned id (its steer drained at an ordinary boundary mid-request) no longer answers `preempt: true` for an interrupt that cannot happen. - The cancel disarm is awaited. A dropped clear is worse than a dropped arm: the owner keeps a level-triggered request for a steer that no longer exists, seals its next chunk and truncates an unrelated answer. The boundary drain's own call stays non-blocking — there the owner is local, so the disarm is already effective and awaiting the informational publish would only delay injection. - Subscriber count is NOT read as proof of owner receipt: the count includes this replica's own facade subscription. A successful publish reports armed, a rejected one does not. Documented rather than papered over — see the acknowledgement-semantics note on the PR. CI regressions from round 5, all mine: - `registerPreemptSubscription` was AWAITED at both runtime-init sites, so job creation blocked on a second Redis channel subscription and hung when that subscribe was slow. Abort is awaited because a missed abort strands a run; a missed preempt only degrades that steer to the next tool boundary, so it now registers without gating createJob. - Two api specs mocked `@librechat/api` without the newly imported `isSteerPreemptSupported`, so the call threw before createJob; and one exact-match assertion needed the new `preemptCapable` metadata field. - My own Redis integration spec asserted arm-before-clear ordering, which two publishes carry no guarantee of — the receiving tombstone exists precisely because of that. Now asserts delivery and payload fidelity, order-independent. 158 packages/api specs, 27 api specs green. * 🧭 fix: Codex round 7 — settle the acknowledgement semantics (Option A) Round 7's second finding is the incoherence I flagged on the PR: the route persisted `preempt: true` on the durable queue item while returning `preempt: false` when delivery could not be confirmed. Those two then disagreed, and `rearmQueuedPreempts` trusts the DURABLE one — so a resumed owner would honour an interrupt the client had explicitly been told degraded to ordinary steering. Rather than patch the disagreement, this settles the meaning: `preempt` in the 202 means "queued as an interrupt request", NOT "a seal is guaranteed". It mirrors `SteerQueueItem.preempt` exactly, so the response, the durable record, and the resume-time re-arm can never disagree. The gates that ARE knowable stay — the owner's recorded capability and a successful enqueue. Everything past that degrades to the documented fallback of injecting at the next tool boundary. A route cannot synchronously know whether another replica will seal: proving it needs a correlated request/response over pub-sub, and even that only proves the owner heard, not that it is still streaming when the arm lands. Four rounds of tightening this boolean each surfaced a narrower case; the sequence does not converge, so the invariant is now "the flag describes the durable decision" and an unconfirmed arm logs a warning instead of rewriting the answer. Also from this round: a failed disarm publish is retried once and its outcome reported. `handleSteerCancel` keeps `removed: true` — the steer really did leave the queue, and saying otherwise would make the client re-show a chip for a steer that can never arrive — and adds `disarmed: false` so the residual risk is visible rather than swallowed. Damage stays bounded regardless: the empty-boundary self-clear disarms the generation after a single seal. Tests: +1 pinning the response/durable-flag invariant. 159 packages/api specs green. * 🧹 fix: Codex round 8 — remove the unverifiable disarm signal Round 8 found the same over-promise on the disarm side that round 7 corrected on the arm side, so this applies the same answer rather than patching around it. The `disarmed: false` field added in round 7 was both unreliable and unused: a resolved publish is not proof the owner heard it (the delivery count includes this replica's own facade subscription), and it was never threaded into `CancelSteerResponse` or read by any client. A signal that claims a certainty the transport cannot provide is worse than no signal — it invites callers to trust it. Removed from the response. The retry stays, because it genuinely reduces the failure rate, and `noteSteersRemoved` still returns whether the publish succeeded FOR LOGGING, now documented explicitly as "published without error", not "the owner disarmed". Disarm is best effort with a bounded, self-healing failure: if the clear is lost the owner seals once, the empty-boundary self-clear disarms the generation, and the turn is persisted `unfinished: true` rather than silently truncated. Tightening that further needs a correlated request/response over pub-sub with a timeout — noted on the PR as the deliberate boundary of this design rather than an oversight. 130 packages/api steering specs green. * 🧽 fix: Codex round 9 — spend snapshot arms on nonempty drains too The round-6 scoping fix only cleared the pre-drain snapshot when the drain came back EMPTY. On a nonempty drain the `finally` cleared just the drained ids, so a stale arm — typically a cancel whose cross-replica clear was lost — survived the boundary. It would then immediately seal the continuation meant to answer the steer that had just been injected, and land on an empty boundary as `preempt_incomplete`: the interrupt appears to work, and the answer to it is truncated. A boundary that runs has spent its seal, so everything armed at snapshot time is spent whether or not it came back from the drain. The `finally` now clears the union of the snapshot and the drained ids. Arms that land AFTER the snapshot are still spared — their queue items are live and uninjected, which is the property round 6 added. Also fixes an api-workspace CI failure of mine: `resume.spec.js` stubs `GenerationJobManager` wholesale, and the round-3/4 resume work added two calls (`updateMetadata`, `rearmQueuedPreempts`) the stub did not define, so 34 specs threw. Stub extended. Tests: +2 (a nonempty drain clears a stale snapshot arm; a nonempty drain spares an arm that landed mid-drain). Counterfactually verified — the stale-arm spec fails against the unfixed drain. 132 packages/api specs, 60 resume specs green. * fix: never let a failed preempt subscription reject into the void registerPreemptSubscription is called detached at both sites, so a rejected Redis SUBSCRIBE became an unhandled rejection — process-fatal under Node's default --unhandled-rejections=throw. The comment already promised this path merely degrades steering; it now does. Swallowed and logged inside the registration rather than at each call site, so a future third caller cannot reintroduce the trap. Losing the channel costs this generation's cross-replica preempts, not the server: same-replica arming is runtime state and still works, and remote arms fall back to the next tool boundary. Verified counterfactually — the new spec surfaces SUBSCRIBE failed as an unhandled rejection against the unfixed registration. * docs: state the real blast radius of a failed preempt subscription LibreChat's own entrypoints install a global unhandledRejection handler that logs and keeps serving, so the escaping rejection this guards was never fatal to this server — only to another consumer of @librechat/api that installs no handler. The fix stands either way; the comment just should not overstate what it prevents. * test: cover the cross-replica preempt hop with two manager instances Every other preempt test runs against a single manager, so the hop that actually carries an interrupt in production had no coverage: the steer POST lands on whichever replica the balancer picks, which is usually not the one generating. Non-owner publishes, owner arms, owner's level-triggered poll flips — none of that was exercised end to end. Two GenerationJobManagerClass instances are a faithful replica pair here. runtimeState and ownedJobs are private instance fields, there is no module-level mutable state between them, and createStreamServices duplicates a dedicated subscriber connection per call, so separate OS processes would exercise the same objects over the same Redis. Both assertions verified counterfactually against real Redis: - Deleting the preemptCapable deserialization in RedisJobStore fails this with 'Expected: true, Received: undefined' — the exact P1 that shipped past every in-memory test and would have made the feature a silent no-op on every Redis deployment. - Dropping the non-owner arm publish fails it with 'Received: false'. * test: remove the fixed sleeps and vacuity from the cross-replica preempt test Codex round 11, both findings, both on the test I added last commit. P2 — the 300ms waits were load-bearing. Redis pub/sub never replays and the owner's SUBSCRIBE is detached, so on a slow CI worker the publish could land before anyone was listening and the test would fail against correct code. Now it republishes until the owner's state converges, which is safe because arms and clears are idempotent set writes keyed by steerId. Side effect: the tests got ~10x faster (85ms/57ms vs 929ms/606ms) since they finish on delivery rather than on a timer. P3 — afterEach destroyed only the transports, leaving each manager alive in its own cleanup-interval closure, still working against a dead transport. Now tracks the managers and awaits destroy(), which disposes the job store and its timer too. Matches how the rest of this file cleans up. Fixing the sleeps exposed a third problem codex did not flag: the stale-arm test could pass vacuously, because an undelivered arm and a fenced one look identical. It now brackets the stale publish between two control arms — the first proves the owner is listening before the stale one is sent, the second proves it has had its chance to arrive. Verified counterfactually against real Redis, and stable over 5 runs: - dropping the preemptCapable deserialization fails with 'Received: undefined' - dropping the non-owner arm publish times out both tests - removing the generation fence fails the stale test with ["control-before", "steer-stale", "control-after"] — which also confirms the bracketing orders as intended rather than by luck * fix: gate interrupt on the OWNER's capability alone, not the route's Codex round 12. The comment above this gate already said 'the OWNER's recorded capability, not this replica's probe' — and then the code ANDed in isSteerPreemptSupported(), which is exactly this replica's probe. The contradiction dates to the original commit; round 6 made the gate owner-scoped and wrote that comment without removing the local conjunct. The route never seals. It enqueues and publishes an arm, neither of which touches the SDK, so during a rolling deploy a steer landing on an un-upgraded replica silently lost its interrupt even though the owner could seal. When the route IS the owner the probe is redundant anyway: the flag it would consult is the one this process wrote at createJob. The real degradation path is unchanged and still tested — an owner that recorded no capability relabels to an ordinary steer. The test that pinned the local probe asserted an impossible same-replica state (capable metadata plus an incapable local SDK, when the metadata is written from that probe); it now pins the mixed-SDK direction instead, and fails with 'Expected: true, Received: false' if the probe is put back. * fix: reconcile arms at handover, and stop holding the 202 on a publish Codex round 13, two of three findings. P2 — rearmQueuedPreempts only ever ADDED. A replica that merely read the job still installs a facade runtime and subscribes, so it can accept an arm and then miss the best-effort clear that follows the drain. HITL resume promotes that facade to owner, the union keeps the orphan, and the first resumed stream seals on a steer no longer in the queue, drains nothing, and truncates the resumed answer as preempt_incomplete. acquireResumedJobOwnership only sets ownedJobs, so nothing else was clearing it. The durable queue is the sole authority at a handover: arms it does not back are now disarmed and tombstoned, so an in-flight publish cannot revive them either. Worth recording that my own independent review raised this and my verifier refuted it. Codex found it separately; two reviewers converging should have outweighed one refutation. P2 — the route awaited the arm publish before answering. The 202 reports capability, not delivery, so the await could not change the response; it only exposed the caller to Redis latency after the queue item was already durable. A client that times out and retries mints a second steer while the first stays queued, injecting the same instruction twice, whereas a lost publish merely takes the tool-boundary fallback. Detached, with both outcomes logged. All three tests verified counterfactually: union-only rearm fails the two new handover specs, and re-awaiting the publish hangs the stalled-publish spec until jest kills it. * fix: snapshot arms before reading the queue at handover Codex round 14 — a regression from my own round-13 fix, and a worse failure than the one it corrected. Round 13 read the durable queue first, then tombstoned any armed id the snapshot did not back. But approvals.resolve reopens steering before reconciliation runs, so another replica can commit a preempt steer and publish its arm while the peek is in flight. That arm is then present locally but absent from a snapshot taken before the steer existed, so a LIVE interrupt the route already acknowledged got dropped — and tombstoned, which blocks the re-arm, making it unrecoverable rather than merely late. Fixed by inverting the two reads rather than by locking or paying a second round trip. A steer is durably enqueued BEFORE its arm is published, so any id in an arms-first snapshot was already queued when it was armed, and the later peek must observe it unless it has since drained — which is exactly the orphan this reconciliation exists to drop. Arms landing after the snapshot are simply not candidates. Also re-checks runtime identity across the await, since the generation can be replaced while the queue read is in flight. New spec injects a steer + arm during the peek and verifies it survives; against the round-13 ordering it fails with Received array: []. * fix: bound the cancel disarm wait and fence enqueue to its generation Codex round 15. P2 — the cancel awaited its disarm publish unbounded. ioredis queues commands during an outage rather than rejecting, so that await could hang for the length of the outage with the steer ALREADY durably cancelled; a client that gives up then treats the cancel as failed and restores a chip for a steer that can never produce an applied event. Every successful cancel publishes, so ordinary steers were exposed too, not only preemptive ones. Now bounded at 1s, with the publish continuing behind it — its retry and logging are unchanged, it is just no longer in front of the response. This is the sibling of round 13's arm-publish finding; I fixed one path and left this one. P3 — enqueue was not fenced to the generation the capability decision was made against. The access checks, file resolution and owner re-read are all awaits, so the run can be replaced before the enqueue: the item then lands on the REPLACEMENT queue while the durable preempt flag and the arm still name the previous epoch, the arm is fenced out at the owner, and the 202 promises an interrupt that cannot happen. enqueueSteer now takes an expected generation, mirroring drain/peek, and the Redis path enforces it inside STEER_ENQUEUE_LUA so the check is atomic with the push rather than racing it. All three new specs verified counterfactually, including the Lua guard against real Redis (removing it returns 1 where -1 is required). * ⚡ feat: Interrupt & Steer — client half (PR 3 of 3) Makes preemptive steering reachable. Consumes the server contract from PR 2 (POST /chat/steer `preempt`, echoed on the 202) and the SDK seam in @librechat/agents 3.3.5. Settings shape follows the agreed correction, NOT the earlier plan draft: `steerInterruptsByDefault` is a boolean ORTHOGONAL to `duringRunDefaultAction` — that enum still chooses steer-vs-queue, the new boolean chooses how soon a steer lands. This deliberately avoids widening the enum to three values, which would have silently broken two hard-coded binary TOGGLES (`DuringRunAction.tsx`'s setter and `SteerMenu`'s `useDefaultToggleEntry`, both `prev === 'steer' ? … : …`) where a third value collapses to the wrong branch and one click erases the setting. - useSteering: `submitSteer` takes an opts bag and threads `preempt` into the POST, the optimistic chip, and the failure chip. The ACK relabels from the SERVER's echo, so a deployment that cannot seal mid-stream downgrades the chip's wording instead of erroring — the entire UX surface of capability degradation. New `interruptSteer` reuses the whole chip lifecycle and degradation ladder, and falls back to `interruptAndSend` when `!canSteer`, because steering needs a server-side job and an always-visible button would otherwise be dead for the whole first turn. `steerFromComposer` honours the new preference. - Composer: always-visible `InterruptSteerButton` with one fixed meaning (stop now, keep what's written), disabled on a paused run to pre-empt the server's 409, `type="button"` so it never steals the form's Enter submit, RTL-correct margins. A fourth hovercard row on the during-run send button, and ⌘/Ctrl+Shift+Enter routed AHEAD of the bare ⌘/Ctrl+Enter branch that would otherwise swallow it. - Chips: an in-flight preempt chip reads "Interrupting" with a ZapOff glyph; `preempt` survives reconnect through `seedSteerChips`. - `RunEnd.interruptArmed`, `drainAfterAbortByIndex`, `useQueueDrain`, `stopGenerating` and `interruptAndSend` are untouched — the preempt path deliberately shares none of the abort machinery. Tests: 7 new specs (posts preempt, turn-1 fallback, empty-text refusal, default route with and without the preference, server-echo relabel, double-click). 66 useSteering specs green; tsc and lint clean. Round-1 review fixes folded in: - P1: interrupt & steer no longer hard-aborts a run paused on tool approval. `canSteer` is false there, so the fallback was routing the keyboard and hovercard paths into `interruptAndSend` — discarding the partial answer, the exact opposite of what the action promises. The fallback is now scoped to the missing-conversation case only, and a paused run refuses outright (the standalone button was already disabled; the guard now lives where all three paths reach it). - The preference no longer leaks into the explicit Steer action. `steerFromComposer` backs both the default Enter route AND the explicit hovercard row / Ctrl+Enter alternate; applying `steerInterruptsByDefault` inside it made ordinary Steer interrupt and the two rows indistinguishable. It now takes an explicit argument that only `submitDuringRun`'s default route sets. - Retry preserves preemption: a failed interrupt-steer chip keeps `preempt: true`, and `retrySteer` now forwards it rather than silently resubmitting as an ordinary tool-boundary steer. - ⌘/Ctrl+Shift+Enter defers to a rebound submit shortcut, mirroring the bare ⌘/Ctrl+Enter branch — a user who bound submit to that chord keeps getting submit. Round-2 fix: the preempt label now survives the page-reload resume path too. `seedSteerChips` (useResumableSSE) and `restoreSteerChips` (useResumeOnLoad) are two independent TPendingSteer→PendingSteer mappers with near-identical bodies; the first carried the flag and the second silently dropped it, so an armed interrupt reverted to plain "Steering" after a reload. Swept: those are the only two in production code. The reclaim/convert paths deliberately omit it — a queued follow-up starts its own turn, so there is nothing to interrupt. * fix: yield the interrupt-steer chord only to a submit shortcut bound to it The previous guard skipped the Ctrl/Cmd+Shift+Enter branch whenever ANY submitMessage override existed. Rebinding submit to something unrelated (Ctrl+J) or unbinding it entirely then fell through to the override resolver, which returns 'none' for shifted Enter — silently removing the shortcut the hovercard still advertises. Compare the pressed chord against the configured one instead. The adjacent bare Ctrl/Cmd+Enter branch keeps its any-override guard on purpose: that chord IS the default submit chord, so once submit moves the resolver should own it. The predicate already existed inside resolveSubmitOverrideAction; pulled it out as bindingsMatch so both sites compare chords the same way. That call is behavior-preserving — eventBinding.key is 'Enter' by the early return, and equal hashes imply equal keys, so the dropped explicit key check was redundant. * fix: disable the Interrupt & steer menu row while paused on approval interruptSteer hard-refuses when the run is paused for tool approval, but the hovercard row was never gated, so it rendered enabled and clicking it did nothing at all — no chip, no queue entry, no toast — at exactly the moment a user is trying to say "stop, don't run that command". The standalone button already gates on pausedOnApproval; the row contradicted it. Gated on pausedOnApproval rather than !canSteer like the steer row above, because canSteer is also false before a conversation exists, where interruptSteer deliberately falls back to interruptAndSend and the row must stay live for the whole first turn. Tests pin both directions and were verified counterfactually: removing the gate fails the paused case, and using !canSteer fails the first-turn case. * test: render the during-run hovercard eagerly instead of driving Ariakit The new spec passed locally and failed all four cases on CI's Ubuntu and Windows shards: Ariakit's show path keys off pointer geometry, which jsdom reports as zeros, so whether a synthetic mouseEnter opens the hovercard is environment-dependent. Driving it was testing Ariakit's hover behavior, not which rows this component disables. Mocking the three Ariakit primitives renders the rows unconditionally and drops the fake timers. Both counterfactuals still fail as they should: removing the gate fails the paused case, !canSteer fails the first-turn case. * test(e2e): cover interrupt & steer sealing mid-stream The mock Playwright suite covered every sibling during-run action — steer at a tool boundary, steer degrading to a queued follow-up, queue, and interrupt & send — but not interrupt & steer, the one this stack adds. Uses E2E_SLOW_REPLY, which streams pure text with no tools, so the scenario is the same one where an ordinary steer provably degrades to a queued follow-up turn. Injecting in-thread there is something only a mid-stream seal can do, which makes the assertion discriminating rather than incidental: the steer part lands in the response, the final chunk never arrives, the text written before the seal survives, and no follow-up turn pair is created. * test(e2e): assert the run resumes after the seal, not just that it sealed The other four assertions are all satisfied by a seal that killed the run: the steer part is persisted by applySteer during the drain, before the continuation starts, so 'sealed and resumed' and 'sealed and died' were indistinguishable — and resuming is the whole difference from interrupt & send. The continuation answers the injected steer, whose text carries no fake-model marker, so getLatestUserText falls through to the default reply. That string ('E2E mock reply') is distinct from the setup turn's ('E2E reply <label>'), so seeing it proves generation restarted rather than matching text that was already on screen. The test itself is confirmed working: it ran as 104/121 in the Playwright job on 0ece357 and the run concluded success. * test(e2e): drop the resume assertion pending an unresolved question The assertion that the run visibly resumes after the seal fails deterministically in CI across all three retries. Every other assertion in the test passes, so the chord, the server preempt, the seal, and the in-thread injection all work; what fails is only the continuation's reply becoming visible. I could not determine from CI logs whether that is the mock harness not surfacing a continuation in a no-tool scenario or generation genuinely stopping after the seal, and I am not willing to weaken it into something that passes either way — that would convert a real question into false assurance. Reverted to the four assertions that hold, with the open question recorded in the test and raised on the PR. * fix: yield the interrupt chord to any bound shortcut, and move the Enter hint Codex round 1 on this PR (its first — LibreChat-AI#14519's rounds predate these files). P2 — the chord yielded only to a rebound submitMessage. This composer handler runs before the document-level one in useKeyboardShortcuts, and that one does not check defaultPrevented, so binding any composer-allowed action (focusChat, focusSearch, showShortcuts) to Ctrl/Cmd+Shift+Enter fired BOTH: the run was interrupted and the bound action ran. Now yields to any chord the user has bound. No default binding uses this chord, so it only ever yields to a deliberate rebinding. P2 — with steerInterruptsByDefault on, plain Enter routes through submitDuringRun and preempts, but the hovercard still put the ⏎ hint on the ordinary Steer row, whose click deliberately does NOT preempt. The same row advertised a key that did something else. The hint now follows the preference: ⏎ moves to Interrupt & steer, and the Steer row shows none because no key reaches it in that mode. I had declined this twice on the grounds that the behaviour split is deliberate — it is, and it is unchanged; the finding was about the label, which was a different claim and a correct one. Lint also caught a real bug in the first fix: boundShortcutChords was missing from the handler's dependency array, so a rebinding would not have taken effect until the callback was recreated for another reason. Both verified counterfactually; 179 client specs green. * fix: resolve every composer Enter chord through one decision table Codex round 2: yielding the preempt branch to a bound chord dropped execution into the bare Ctrl/Cmd branch below it, which ignores Shift, and past that into the submit tail, where isCtrlEnter is true for the chord. So rebinding focusChat, focusSearch, or showShortcuts to Ctrl/Cmd+Shift+Enter fired the alternate action or a submit AND the document-level shortcut, consuming the draft. Two rounds in a row landed in this handler because the guard chain decided "whose chord is this?" piecemeal inside individual branches, with fall-through between them. This extracts the entire pipeline into resolveComposerKeyDown (utils/shortcuts, beside resolveSubmitOverrideAction, which it absorbs as a step): one pure decision table where every verdict is terminal, so the fall-through class is gone rather than patched around. The yield rule is now the first gate before all branches: an Enter chord bound to any shortcut the document handler runs while typing (EDITING_ALLOWED_SHORTCUTS, hoisted out of the handler and shared by both dispatchers) is left entirely to that handler. This also closes the identical latent holes in the branches codex did not flag: bound Alt+Enter and Ctrl/Cmd+Enter chords double-fired the same way during a run, and the idle submit tail consumed bound chords too. A chord bound to a shortcut the document handler does NOT run while typing keeps its composer meaning; there is nothing to collide with, and yielding would just make the chord dead. Also merged the spec file round 0 added at utils/__tests__/ into the pre-existing utils/shortcuts.spec.ts (duplicate coverage at a second path) and dropped the isNonShiftEnter+filesLoading preventDefault, which was subsumed by the unconditional one below it. The table has a spec locking every verdict. Counterfactually verified: removing the yield gate fails exactly the three yield tests. Full client suite green apart from six suites that fail identically without this change (local data-provider dist drift in unrelated areas). * fix: yield Alt+Enter to a rebound submit, derive hovercard hints from the decision table Codex round 3, both findings. P2, Alt+Enter submit rebinding. The interrupt branch reserved Alt+Enter unconditionally, so rebinding submitMessage to Alt+Enter meant the user's own submit chord aborted the run and consumed the draft as interrupt & send. Now guarded with the same bindingsMatch yield the preempt branch already had; the chord falls through to the submit override resolution and submits the default action. P2, hovercard hints. The rows advertised hardcoded chords, so a chord rebound to an editing-allowed global shortcut (yielded by the decision table) or claimed by a rebound submit still appeared as Steer, Queue, Interrupt & steer, or Interrupt & send. The hovercard now asks the same decision table the composer executes what each canonical chord does and only labels a row with a chord that still triggers it. That also fixes two dishonest hints codex did not flag: with Enter-to-send off, plain Enter inserts a newline during a run (the primary row advertised it anyway) and Ctrl/Cmd+Enter submits the default action (the alternate row claimed it). The default-action hint now moves to Ctrl/Cmd+Enter in that mode. The effective bindings (submitOverride plus yielded chords) moved from useTextarea into a shared useComposerBindings hook so the handler and the hints read the same source. resolveComposerKeyDown now takes a KeyChordSource pick of the event fields it reads, letting the hovercard pass synthetic chords. Counterfactually verified: reverting the Alt guard fails the new resolver test and the hint suppression test. 353 tests across the affected suites green; the two failing hook suites (useVisibleTools, useResumableSSE) fail identically without these changes (local data-provider dist drift). * fix: never advertise a chord on a disabled hovercard row Codex round 4, one P2. The Interrupt & steer row kept showing its chord while disabled for tool approval, but pressing it reaches interruptSteer's pausedOnApproval guard and no-ops. Fixed as a render rule rather than a per-row patch: a disabled row never shows its kbd, since its action refuses the chord by the same guard that disabled it. That also covers the disabled Steer row, whose alternate-action hint had the identical hole through steerFromComposer's canSteer refusal. Counterfactually verified: reverting the render guard fails the new test.
) * 🛑 feat: Preemptive Steer — server half (Interrupt & Steer, PR 2 of 3) Lets the steer route ask the generating replica to seal its live model stream at the next provider-safe boundary instead of waiting for a tool step. The run is never aborted, job status never changes, the partial answer is kept, and generation resumes in the same assistant message after the injected steer. Consumes the SDK seam in @librechat/agents (LibreChat-AI/agents#335, LibreChat-AI#346). Transport: IEventTransport gains a fenced emitPreempt/onPreempt pair beside abort. RedisEventTransport fans PREEMPT out on the SAME events channel and subscription (no new connection, key, or subscribe call); onPreempt returns a registration-scoped unsubscribe with the same replacement-safe state-identity guard onAbort uses. InMemory implements neither — single-process preempt lives entirely in the runtime set. Runtime state: RuntimeJobState carries the per-generation request set, createdAt-fenced and capped at STEER_QUEUE_MAX_DEPTH, plus a bounded `cleared` tombstone so a late cross-replica arm cannot resurrect a request whose steer already drained. registerPreemptSubscription mirrors the abort registration's double fence (runtime identity + generation createdAt); releaseAbortSubscription retires BOTH listeners and the armed set, so every terminal path drops preempt state for free. Public surface: requestPreempt (arm + fenced publish, never a rejection surface, never touches job status), isPreemptRequested (O(1) level-triggered poll), noteSteersRemoved (drain/cancel bookkeeping + fenced clear), clearPreemptRequests (empty-boundary disarm). One drain body, two boundaries: createSteerDrainHook (PostToolBatch) and createSteerPreemptBoundaryHook (PreemptBoundary) share drainAndBuildInjections, so the two injection sites cannot drift — the SDK's provider-safety argument rests on identical HumanMessage shapes. The shared body builds injections incrementally under a swallow-all catch (a mid-loop throw still injects what was applied — those parts are already persisted), clears preempt requests in finally, and disarms the generation when a boundary drains nothing. Request path: POST /chat/steer accepts preempt: true. The guard ladder is unchanged in order and in every status code. A preempt request is NEVER a rejection reason — without the capability the steer still enqueues and the 202 echoes preempt: false. Armed strictly after a successful enqueue; cancel disarms. The capability is read from the OWNING replica's recorded `preemptCapable` rather than the route replica's own SDK probe, so a rolling deploy cannot label a steer "interrupting" that the older owner will only inject at a tool step. Durable label: SteerQueueItem.preempt → TPendingSteer.preempt, so a parked/claimed/replayed chip keeps its wording. Run wiring: createRun registers the PreemptBoundary hook and threads RunConfig.preemption, both gated on isSteerPreemptSupported() — a separate probe from isSteeringSupported(), so the client affordance can never arm against an SDK that only injects at tool boundaries. buildSteerWiring builds both hooks from one shared closures object, so preemption survives HITL pause/resume for free. Honest finalization: an empty preempt boundary persists and emits with unfinished: true — the same contract an abort gets — re-marked explicitly because BaseClient has already saved the row as unfinished: false by that point. Not changed: no new job status, store method, Lua, SSE event type, endpoint, or authorization surface. abortJob, completeJob, transitionStatus, closeAndDrainSteers, getResumeState, emitChunk, applySteerPart and the whole abort path are untouched. Tests: 120 packages/api steering specs (preempt lifecycle, tombstone, fences, caps, terminal release, both-boundary drain parity, level-triggered poll, request/cancel arming, owner-capability degradation) plus 5 in api for buildSteerWiring gating, and 2 Redis-gated cross-replica transport specs. * 🔒 fix: Codex round 2 — evict tombstones, scope the empty-boundary disarm, honest resumes All four server findings were fresh consequences of the round-1 fixes, which is the review doing exactly what it should. - Tombstone cap refused new entries instead of evicting. Every drained or cancelled steer is tombstoned, not just preempting ones, so a generation that processed 20 steers exhausted the set and the late-arm race resurfaced silently. Now evicts oldest-first (Set iteration is insertion-ordered), with the budget named PREEMPT_TOMBSTONE_MAX rather than an inline expression. - The empty-boundary disarm I added in round 1 wiped the generation's ENTIRE armed set. A second steer can enqueue and arm between the atomic drain returning empty and the disarm running — that arm is backed by a live, uninjected queue item and must survive. The drain now snapshots the armed ids BEFORE draining (getArmedPreemptIds) and clearPreemptRequests takes an explicit id list instead of clearing everything. - HITL resume finalized with a hardcoded unfinished: false. The boundary hook is re-registered on resume via buildSteerWiring, so a resumed segment can end on an empty preempt boundary exactly like a fresh one; finalizeResumedTurn now reads getPreemptStats() and the halt reason, matching the normal request path. - Ownership moves on resume, so the job's recorded preemptCapable must describe the replica that will actually generate. Refreshed before resumeCompletion; a job created on a capable replica that resumes on an older one during a rolling deploy no longer acknowledges steers as interrupting. Tests: +3 (scoped disarm sparing a post-snapshot arm, oldest-first tombstone eviction, id-list disarm). 122 packages/api steering specs green. * 🚨 fix: Codex round 3 — deserialize preemptCapable from Redis (feature was dead under Redis) The P1 here is the most consequential defect in the whole feature, and it was introduced by round 1's own capability fix. - `RedisJobStore.serializeJob` writes booleans generically, so `preemptCapable` reached Redis — but `deserializeJob` is an EXPLICIT field map and had no line for it. Every `getJob()` therefore dropped the flag, `job.metadata.preemptCapable` was always undefined, and `handleSteerRequest` computed `preemptArmed: false` unconditionally. Interrupt & steer would have silently degraded to ordinary tool-boundary steering in EVERY Redis deployment — i.e. the feature shipping as a no-op in production while passing every in-memory test. Now deserialized, with a round-trip assertion in the metadata spec that fails (`Received: undefined`) against the unfixed store. - The resume capability refresh moved from just-before `resumeCompletion` to immediately after `approvals.resolve` claims the run. That call already flips the job back to `running`, so the steer route accepts requests from that instant; leaving the refresh 135 lines later (across the whole client reconstruction) left a real window where a steer read the PREVIOUS owner's capability. Not the fully atomic transition Codex suggested — that reaches into the approvals Lua — but it shrinks the window from seconds to one await, which is proportionate for a label-accuracy issue. Refuted: "avoid triggering preemption inside subagents". The premise — that the run-wide poll can seal a subagent stream — does not hold against the shipped SDK. Child graphs are constructed with `subagentScope: true` (SubagentExecutor) and `preemption` is NOT propagated into child inputs, while `canClaimPreemptSeal()` requires `!subagentScope && preemption != null`. Both conditions fail independently, so a subagent can never claim a seal and the boundary cannot fire with `agentId` set. The `input.agentId != null` guard in the hook is defensive depth, not the thing standing between us and the described failure. 140 packages/api specs green. * 🔁 fix: Codex round 4 — re-arm durable interrupt steers when resume moves owners - An arm lives only in the owning replica's runtime plus a transient pub/sub message, while the steer's `preempt` flag is durable on the queue item. A HITL resume landing on a different replica therefore started with an empty armed set and a poll stuck false, so an interrupt the user had already been ACKed for silently waited for an ordinary tool boundary. New `GenerationJobManager.rearmQueuedPreempts` rebuilds the armed set by peeking the durable queue (fenced on the generation) and re-arming every item flagged `preempt`; resume calls it right after claiming. Safe by construction: every item peeked is still queued, so no drained steer can be resurrected. - Capability-refresh failure now logs at error rather than warn, but deliberately does NOT fail the resume — see the reply on that thread. Tests: +2 (rebuild from queue arms only the flagged item and reports the count; a stale generation arms nothing). 124 packages/api steering specs green. * 📡 fix: Codex round 5 — acknowledge only what was actually armed - A cross-replica arm was fire-and-forget: `emitPreempt` logged its own publish failure and `requestPreempt` returned void, so the route answered `preempt: true` even when the owner never armed a poll. The steer still injected at the next tool boundary, but the chip claimed an interrupt that could not happen — and unlike HITL resume, an ordinary running generation had no durable reconciliation to recover it. `emitPreempt` now resolves to the subscriber count and rejects on failure; `requestPreempt` is async and returns whether the arm truly landed (owned locally, or delivered to at least one subscriber). The 202 reports THAT rather than what was asked for, so the chip relabels to ordinary steering exactly as it does for a capability-degraded deployment. Errors are swallowed into `false` — an unarmed interrupt is a downgrade, never a failed steer. - The owner capability is re-read immediately before enqueue rather than reused from the top of the guard ladder. `checkAgentAccess` and file resolution are awaits, so a request can span an entire HITL pause/resume that moves ownership to a replica with different capability and rewrites that very flag. Only paid for by requests that actually asked to interrupt. Tests: +3 (not-armed when the publish reaches nobody; armed when this replica owns the generation; a throwing publish downgrades instead of propagating). 127 packages/api steering specs green. * 🎯 fix: Codex round 6 — real ownership, confirmed disarms, and a CI regression of my own Three review findings plus three CI failures the round-5 commit caused. Review: - Ownership came from `runtimeState`, which a cross-replica `getJob` populates with a FACADE runtime on any replica that merely read the job. Matching `createdAt` therefore proved only "we looked at this job", so a non-owner could arm nothing and report success. Ownership now comes from `ownedJobs`, the actual owner map. - `armPreemptIds` returns how many ids it accepted, and a local arm is only reported as armed when one was. A tombstoned id (its steer drained at an ordinary boundary mid-request) no longer answers `preempt: true` for an interrupt that cannot happen. - The cancel disarm is awaited. A dropped clear is worse than a dropped arm: the owner keeps a level-triggered request for a steer that no longer exists, seals its next chunk and truncates an unrelated answer. The boundary drain's own call stays non-blocking — there the owner is local, so the disarm is already effective and awaiting the informational publish would only delay injection. - Subscriber count is NOT read as proof of owner receipt: the count includes this replica's own facade subscription. A successful publish reports armed, a rejected one does not. Documented rather than papered over — see the acknowledgement-semantics note on the PR. CI regressions from round 5, all mine: - `registerPreemptSubscription` was AWAITED at both runtime-init sites, so job creation blocked on a second Redis channel subscription and hung when that subscribe was slow. Abort is awaited because a missed abort strands a run; a missed preempt only degrades that steer to the next tool boundary, so it now registers without gating createJob. - Two api specs mocked `@librechat/api` without the newly imported `isSteerPreemptSupported`, so the call threw before createJob; and one exact-match assertion needed the new `preemptCapable` metadata field. - My own Redis integration spec asserted arm-before-clear ordering, which two publishes carry no guarantee of — the receiving tombstone exists precisely because of that. Now asserts delivery and payload fidelity, order-independent. 158 packages/api specs, 27 api specs green. * 🧭 fix: Codex round 7 — settle the acknowledgement semantics (Option A) Round 7's second finding is the incoherence I flagged on the PR: the route persisted `preempt: true` on the durable queue item while returning `preempt: false` when delivery could not be confirmed. Those two then disagreed, and `rearmQueuedPreempts` trusts the DURABLE one — so a resumed owner would honour an interrupt the client had explicitly been told degraded to ordinary steering. Rather than patch the disagreement, this settles the meaning: `preempt` in the 202 means "queued as an interrupt request", NOT "a seal is guaranteed". It mirrors `SteerQueueItem.preempt` exactly, so the response, the durable record, and the resume-time re-arm can never disagree. The gates that ARE knowable stay — the owner's recorded capability and a successful enqueue. Everything past that degrades to the documented fallback of injecting at the next tool boundary. A route cannot synchronously know whether another replica will seal: proving it needs a correlated request/response over pub-sub, and even that only proves the owner heard, not that it is still streaming when the arm lands. Four rounds of tightening this boolean each surfaced a narrower case; the sequence does not converge, so the invariant is now "the flag describes the durable decision" and an unconfirmed arm logs a warning instead of rewriting the answer. Also from this round: a failed disarm publish is retried once and its outcome reported. `handleSteerCancel` keeps `removed: true` — the steer really did leave the queue, and saying otherwise would make the client re-show a chip for a steer that can never arrive — and adds `disarmed: false` so the residual risk is visible rather than swallowed. Damage stays bounded regardless: the empty-boundary self-clear disarms the generation after a single seal. Tests: +1 pinning the response/durable-flag invariant. 159 packages/api specs green. * 🧹 fix: Codex round 8 — remove the unverifiable disarm signal Round 8 found the same over-promise on the disarm side that round 7 corrected on the arm side, so this applies the same answer rather than patching around it. The `disarmed: false` field added in round 7 was both unreliable and unused: a resolved publish is not proof the owner heard it (the delivery count includes this replica's own facade subscription), and it was never threaded into `CancelSteerResponse` or read by any client. A signal that claims a certainty the transport cannot provide is worse than no signal — it invites callers to trust it. Removed from the response. The retry stays, because it genuinely reduces the failure rate, and `noteSteersRemoved` still returns whether the publish succeeded FOR LOGGING, now documented explicitly as "published without error", not "the owner disarmed". Disarm is best effort with a bounded, self-healing failure: if the clear is lost the owner seals once, the empty-boundary self-clear disarms the generation, and the turn is persisted `unfinished: true` rather than silently truncated. Tightening that further needs a correlated request/response over pub-sub with a timeout — noted on the PR as the deliberate boundary of this design rather than an oversight. 130 packages/api steering specs green. * 🧽 fix: Codex round 9 — spend snapshot arms on nonempty drains too The round-6 scoping fix only cleared the pre-drain snapshot when the drain came back EMPTY. On a nonempty drain the `finally` cleared just the drained ids, so a stale arm — typically a cancel whose cross-replica clear was lost — survived the boundary. It would then immediately seal the continuation meant to answer the steer that had just been injected, and land on an empty boundary as `preempt_incomplete`: the interrupt appears to work, and the answer to it is truncated. A boundary that runs has spent its seal, so everything armed at snapshot time is spent whether or not it came back from the drain. The `finally` now clears the union of the snapshot and the drained ids. Arms that land AFTER the snapshot are still spared — their queue items are live and uninjected, which is the property round 6 added. Also fixes an api-workspace CI failure of mine: `resume.spec.js` stubs `GenerationJobManager` wholesale, and the round-3/4 resume work added two calls (`updateMetadata`, `rearmQueuedPreempts`) the stub did not define, so 34 specs threw. Stub extended. Tests: +2 (a nonempty drain clears a stale snapshot arm; a nonempty drain spares an arm that landed mid-drain). Counterfactually verified — the stale-arm spec fails against the unfixed drain. 132 packages/api specs, 60 resume specs green. * fix: never let a failed preempt subscription reject into the void registerPreemptSubscription is called detached at both sites, so a rejected Redis SUBSCRIBE became an unhandled rejection — process-fatal under Node's default --unhandled-rejections=throw. The comment already promised this path merely degrades steering; it now does. Swallowed and logged inside the registration rather than at each call site, so a future third caller cannot reintroduce the trap. Losing the channel costs this generation's cross-replica preempts, not the server: same-replica arming is runtime state and still works, and remote arms fall back to the next tool boundary. Verified counterfactually — the new spec surfaces SUBSCRIBE failed as an unhandled rejection against the unfixed registration. * docs: state the real blast radius of a failed preempt subscription LibreChat's own entrypoints install a global unhandledRejection handler that logs and keeps serving, so the escaping rejection this guards was never fatal to this server — only to another consumer of @librechat/api that installs no handler. The fix stands either way; the comment just should not overstate what it prevents. * test: cover the cross-replica preempt hop with two manager instances Every other preempt test runs against a single manager, so the hop that actually carries an interrupt in production had no coverage: the steer POST lands on whichever replica the balancer picks, which is usually not the one generating. Non-owner publishes, owner arms, owner's level-triggered poll flips — none of that was exercised end to end. Two GenerationJobManagerClass instances are a faithful replica pair here. runtimeState and ownedJobs are private instance fields, there is no module-level mutable state between them, and createStreamServices duplicates a dedicated subscriber connection per call, so separate OS processes would exercise the same objects over the same Redis. Both assertions verified counterfactually against real Redis: - Deleting the preemptCapable deserialization in RedisJobStore fails this with 'Expected: true, Received: undefined' — the exact P1 that shipped past every in-memory test and would have made the feature a silent no-op on every Redis deployment. - Dropping the non-owner arm publish fails it with 'Received: false'. * test: remove the fixed sleeps and vacuity from the cross-replica preempt test Codex round 11, both findings, both on the test I added last commit. P2 — the 300ms waits were load-bearing. Redis pub/sub never replays and the owner's SUBSCRIBE is detached, so on a slow CI worker the publish could land before anyone was listening and the test would fail against correct code. Now it republishes until the owner's state converges, which is safe because arms and clears are idempotent set writes keyed by steerId. Side effect: the tests got ~10x faster (85ms/57ms vs 929ms/606ms) since they finish on delivery rather than on a timer. P3 — afterEach destroyed only the transports, leaving each manager alive in its own cleanup-interval closure, still working against a dead transport. Now tracks the managers and awaits destroy(), which disposes the job store and its timer too. Matches how the rest of this file cleans up. Fixing the sleeps exposed a third problem codex did not flag: the stale-arm test could pass vacuously, because an undelivered arm and a fenced one look identical. It now brackets the stale publish between two control arms — the first proves the owner is listening before the stale one is sent, the second proves it has had its chance to arrive. Verified counterfactually against real Redis, and stable over 5 runs: - dropping the preemptCapable deserialization fails with 'Received: undefined' - dropping the non-owner arm publish times out both tests - removing the generation fence fails the stale test with ["control-before", "steer-stale", "control-after"] — which also confirms the bracketing orders as intended rather than by luck * fix: gate interrupt on the OWNER's capability alone, not the route's Codex round 12. The comment above this gate already said 'the OWNER's recorded capability, not this replica's probe' — and then the code ANDed in isSteerPreemptSupported(), which is exactly this replica's probe. The contradiction dates to the original commit; round 6 made the gate owner-scoped and wrote that comment without removing the local conjunct. The route never seals. It enqueues and publishes an arm, neither of which touches the SDK, so during a rolling deploy a steer landing on an un-upgraded replica silently lost its interrupt even though the owner could seal. When the route IS the owner the probe is redundant anyway: the flag it would consult is the one this process wrote at createJob. The real degradation path is unchanged and still tested — an owner that recorded no capability relabels to an ordinary steer. The test that pinned the local probe asserted an impossible same-replica state (capable metadata plus an incapable local SDK, when the metadata is written from that probe); it now pins the mixed-SDK direction instead, and fails with 'Expected: true, Received: false' if the probe is put back. * fix: reconcile arms at handover, and stop holding the 202 on a publish Codex round 13, two of three findings. P2 — rearmQueuedPreempts only ever ADDED. A replica that merely read the job still installs a facade runtime and subscribes, so it can accept an arm and then miss the best-effort clear that follows the drain. HITL resume promotes that facade to owner, the union keeps the orphan, and the first resumed stream seals on a steer no longer in the queue, drains nothing, and truncates the resumed answer as preempt_incomplete. acquireResumedJobOwnership only sets ownedJobs, so nothing else was clearing it. The durable queue is the sole authority at a handover: arms it does not back are now disarmed and tombstoned, so an in-flight publish cannot revive them either. Worth recording that my own independent review raised this and my verifier refuted it. Codex found it separately; two reviewers converging should have outweighed one refutation. P2 — the route awaited the arm publish before answering. The 202 reports capability, not delivery, so the await could not change the response; it only exposed the caller to Redis latency after the queue item was already durable. A client that times out and retries mints a second steer while the first stays queued, injecting the same instruction twice, whereas a lost publish merely takes the tool-boundary fallback. Detached, with both outcomes logged. All three tests verified counterfactually: union-only rearm fails the two new handover specs, and re-awaiting the publish hangs the stalled-publish spec until jest kills it. * fix: snapshot arms before reading the queue at handover Codex round 14 — a regression from my own round-13 fix, and a worse failure than the one it corrected. Round 13 read the durable queue first, then tombstoned any armed id the snapshot did not back. But approvals.resolve reopens steering before reconciliation runs, so another replica can commit a preempt steer and publish its arm while the peek is in flight. That arm is then present locally but absent from a snapshot taken before the steer existed, so a LIVE interrupt the route already acknowledged got dropped — and tombstoned, which blocks the re-arm, making it unrecoverable rather than merely late. Fixed by inverting the two reads rather than by locking or paying a second round trip. A steer is durably enqueued BEFORE its arm is published, so any id in an arms-first snapshot was already queued when it was armed, and the later peek must observe it unless it has since drained — which is exactly the orphan this reconciliation exists to drop. Arms landing after the snapshot are simply not candidates. Also re-checks runtime identity across the await, since the generation can be replaced while the queue read is in flight. New spec injects a steer + arm during the peek and verifies it survives; against the round-13 ordering it fails with Received array: []. * fix: bound the cancel disarm wait and fence enqueue to its generation Codex round 15. P2 — the cancel awaited its disarm publish unbounded. ioredis queues commands during an outage rather than rejecting, so that await could hang for the length of the outage with the steer ALREADY durably cancelled; a client that gives up then treats the cancel as failed and restores a chip for a steer that can never produce an applied event. Every successful cancel publishes, so ordinary steers were exposed too, not only preemptive ones. Now bounded at 1s, with the publish continuing behind it — its retry and logging are unchanged, it is just no longer in front of the response. This is the sibling of round 13's arm-publish finding; I fixed one path and left this one. P3 — enqueue was not fenced to the generation the capability decision was made against. The access checks, file resolution and owner re-read are all awaits, so the run can be replaced before the enqueue: the item then lands on the REPLACEMENT queue while the durable preempt flag and the arm still name the previous epoch, the arm is fenced out at the owner, and the 202 promises an interrupt that cannot happen. enqueueSteer now takes an expected generation, mirroring drain/peek, and the Redis path enforces it inside STEER_ENQUEUE_LUA so the check is atomic with the push rather than racing it. All three new specs verified counterfactually, including the Lua guard against real Redis (removing it returns 1 where -1 is required). * fix: fence the steer to its authorized generation, bound resume setup, keep preempt when Redis parks Codex round 16, all three findings. P2 — round 15 fenced the enqueue to owner.createdAt, the RE-READ job. Every guard above it (ownership, tenant, paused-state, agent ACL) ran against the job read at the top, so if the run was replaced during those awaits the fence happily accepted the steer into a generation the request was never authorized against, carrying the wrong agent's metadata. Now rejects on any mismatch between the validated job and the re-read. P2 — resume awaited its steering bookkeeping unbounded, after approvals.resolve had consumed the action and flipped the job to running, and outside the resume lifecycle's own try/finally. .catch does not fire on a promise that never settles, which is what ioredis produces during an outage, so the client times out, its retry gets a 409 for a spent action, and no cleanup runs. Bounded at 1s with the writes finishing in the background. P3 — Redis parks leftover steers inside its terminal-transition Lua, which projects item fields one by one, so preempt was silently dropped and a steer recovered from /chat/status lost its interrupting label. Added to both projections. All three verified counterfactually, two against real Redis. Worth recording that my first version of the generation-mismatch test was VACUOUS — it faked a createdAt matching no live job, so the round-15 enqueue fence rejected it for the wrong reason and the test passed with the guard removed. Rewritten to replace the run for real; it now fails with 'Expected 404, Received 202'.
* 🛑 feat: Preemptive Steer — server half (Interrupt & Steer, PR 2 of 3) Lets the steer route ask the generating replica to seal its live model stream at the next provider-safe boundary instead of waiting for a tool step. The run is never aborted, job status never changes, the partial answer is kept, and generation resumes in the same assistant message after the injected steer. Consumes the SDK seam in @librechat/agents (LibreChat-AI/agents#335, LibreChat-AI#346). Transport: IEventTransport gains a fenced emitPreempt/onPreempt pair beside abort. RedisEventTransport fans PREEMPT out on the SAME events channel and subscription (no new connection, key, or subscribe call); onPreempt returns a registration-scoped unsubscribe with the same replacement-safe state-identity guard onAbort uses. InMemory implements neither — single-process preempt lives entirely in the runtime set. Runtime state: RuntimeJobState carries the per-generation request set, createdAt-fenced and capped at STEER_QUEUE_MAX_DEPTH, plus a bounded `cleared` tombstone so a late cross-replica arm cannot resurrect a request whose steer already drained. registerPreemptSubscription mirrors the abort registration's double fence (runtime identity + generation createdAt); releaseAbortSubscription retires BOTH listeners and the armed set, so every terminal path drops preempt state for free. Public surface: requestPreempt (arm + fenced publish, never a rejection surface, never touches job status), isPreemptRequested (O(1) level-triggered poll), noteSteersRemoved (drain/cancel bookkeeping + fenced clear), clearPreemptRequests (empty-boundary disarm). One drain body, two boundaries: createSteerDrainHook (PostToolBatch) and createSteerPreemptBoundaryHook (PreemptBoundary) share drainAndBuildInjections, so the two injection sites cannot drift — the SDK's provider-safety argument rests on identical HumanMessage shapes. The shared body builds injections incrementally under a swallow-all catch (a mid-loop throw still injects what was applied — those parts are already persisted), clears preempt requests in finally, and disarms the generation when a boundary drains nothing. Request path: POST /chat/steer accepts preempt: true. The guard ladder is unchanged in order and in every status code. A preempt request is NEVER a rejection reason — without the capability the steer still enqueues and the 202 echoes preempt: false. Armed strictly after a successful enqueue; cancel disarms. The capability is read from the OWNING replica's recorded `preemptCapable` rather than the route replica's own SDK probe, so a rolling deploy cannot label a steer "interrupting" that the older owner will only inject at a tool step. Durable label: SteerQueueItem.preempt → TPendingSteer.preempt, so a parked/claimed/replayed chip keeps its wording. Run wiring: createRun registers the PreemptBoundary hook and threads RunConfig.preemption, both gated on isSteerPreemptSupported() — a separate probe from isSteeringSupported(), so the client affordance can never arm against an SDK that only injects at tool boundaries. buildSteerWiring builds both hooks from one shared closures object, so preemption survives HITL pause/resume for free. Honest finalization: an empty preempt boundary persists and emits with unfinished: true — the same contract an abort gets — re-marked explicitly because BaseClient has already saved the row as unfinished: false by that point. Not changed: no new job status, store method, Lua, SSE event type, endpoint, or authorization surface. abortJob, completeJob, transitionStatus, closeAndDrainSteers, getResumeState, emitChunk, applySteerPart and the whole abort path are untouched. Tests: 120 packages/api steering specs (preempt lifecycle, tombstone, fences, caps, terminal release, both-boundary drain parity, level-triggered poll, request/cancel arming, owner-capability degradation) plus 5 in api for buildSteerWiring gating, and 2 Redis-gated cross-replica transport specs. * 🔒 fix: Codex round 2 — evict tombstones, scope the empty-boundary disarm, honest resumes All four server findings were fresh consequences of the round-1 fixes, which is the review doing exactly what it should. - Tombstone cap refused new entries instead of evicting. Every drained or cancelled steer is tombstoned, not just preempting ones, so a generation that processed 20 steers exhausted the set and the late-arm race resurfaced silently. Now evicts oldest-first (Set iteration is insertion-ordered), with the budget named PREEMPT_TOMBSTONE_MAX rather than an inline expression. - The empty-boundary disarm I added in round 1 wiped the generation's ENTIRE armed set. A second steer can enqueue and arm between the atomic drain returning empty and the disarm running — that arm is backed by a live, uninjected queue item and must survive. The drain now snapshots the armed ids BEFORE draining (getArmedPreemptIds) and clearPreemptRequests takes an explicit id list instead of clearing everything. - HITL resume finalized with a hardcoded unfinished: false. The boundary hook is re-registered on resume via buildSteerWiring, so a resumed segment can end on an empty preempt boundary exactly like a fresh one; finalizeResumedTurn now reads getPreemptStats() and the halt reason, matching the normal request path. - Ownership moves on resume, so the job's recorded preemptCapable must describe the replica that will actually generate. Refreshed before resumeCompletion; a job created on a capable replica that resumes on an older one during a rolling deploy no longer acknowledges steers as interrupting. Tests: +3 (scoped disarm sparing a post-snapshot arm, oldest-first tombstone eviction, id-list disarm). 122 packages/api steering specs green. * 🚨 fix: Codex round 3 — deserialize preemptCapable from Redis (feature was dead under Redis) The P1 here is the most consequential defect in the whole feature, and it was introduced by round 1's own capability fix. - `RedisJobStore.serializeJob` writes booleans generically, so `preemptCapable` reached Redis — but `deserializeJob` is an EXPLICIT field map and had no line for it. Every `getJob()` therefore dropped the flag, `job.metadata.preemptCapable` was always undefined, and `handleSteerRequest` computed `preemptArmed: false` unconditionally. Interrupt & steer would have silently degraded to ordinary tool-boundary steering in EVERY Redis deployment — i.e. the feature shipping as a no-op in production while passing every in-memory test. Now deserialized, with a round-trip assertion in the metadata spec that fails (`Received: undefined`) against the unfixed store. - The resume capability refresh moved from just-before `resumeCompletion` to immediately after `approvals.resolve` claims the run. That call already flips the job back to `running`, so the steer route accepts requests from that instant; leaving the refresh 135 lines later (across the whole client reconstruction) left a real window where a steer read the PREVIOUS owner's capability. Not the fully atomic transition Codex suggested — that reaches into the approvals Lua — but it shrinks the window from seconds to one await, which is proportionate for a label-accuracy issue. Refuted: "avoid triggering preemption inside subagents". The premise — that the run-wide poll can seal a subagent stream — does not hold against the shipped SDK. Child graphs are constructed with `subagentScope: true` (SubagentExecutor) and `preemption` is NOT propagated into child inputs, while `canClaimPreemptSeal()` requires `!subagentScope && preemption != null`. Both conditions fail independently, so a subagent can never claim a seal and the boundary cannot fire with `agentId` set. The `input.agentId != null` guard in the hook is defensive depth, not the thing standing between us and the described failure. 140 packages/api specs green. * 🔁 fix: Codex round 4 — re-arm durable interrupt steers when resume moves owners - An arm lives only in the owning replica's runtime plus a transient pub/sub message, while the steer's `preempt` flag is durable on the queue item. A HITL resume landing on a different replica therefore started with an empty armed set and a poll stuck false, so an interrupt the user had already been ACKed for silently waited for an ordinary tool boundary. New `GenerationJobManager.rearmQueuedPreempts` rebuilds the armed set by peeking the durable queue (fenced on the generation) and re-arming every item flagged `preempt`; resume calls it right after claiming. Safe by construction: every item peeked is still queued, so no drained steer can be resurrected. - Capability-refresh failure now logs at error rather than warn, but deliberately does NOT fail the resume — see the reply on that thread. Tests: +2 (rebuild from queue arms only the flagged item and reports the count; a stale generation arms nothing). 124 packages/api steering specs green. * 📡 fix: Codex round 5 — acknowledge only what was actually armed - A cross-replica arm was fire-and-forget: `emitPreempt` logged its own publish failure and `requestPreempt` returned void, so the route answered `preempt: true` even when the owner never armed a poll. The steer still injected at the next tool boundary, but the chip claimed an interrupt that could not happen — and unlike HITL resume, an ordinary running generation had no durable reconciliation to recover it. `emitPreempt` now resolves to the subscriber count and rejects on failure; `requestPreempt` is async and returns whether the arm truly landed (owned locally, or delivered to at least one subscriber). The 202 reports THAT rather than what was asked for, so the chip relabels to ordinary steering exactly as it does for a capability-degraded deployment. Errors are swallowed into `false` — an unarmed interrupt is a downgrade, never a failed steer. - The owner capability is re-read immediately before enqueue rather than reused from the top of the guard ladder. `checkAgentAccess` and file resolution are awaits, so a request can span an entire HITL pause/resume that moves ownership to a replica with different capability and rewrites that very flag. Only paid for by requests that actually asked to interrupt. Tests: +3 (not-armed when the publish reaches nobody; armed when this replica owns the generation; a throwing publish downgrades instead of propagating). 127 packages/api steering specs green. * 🎯 fix: Codex round 6 — real ownership, confirmed disarms, and a CI regression of my own Three review findings plus three CI failures the round-5 commit caused. Review: - Ownership came from `runtimeState`, which a cross-replica `getJob` populates with a FACADE runtime on any replica that merely read the job. Matching `createdAt` therefore proved only "we looked at this job", so a non-owner could arm nothing and report success. Ownership now comes from `ownedJobs`, the actual owner map. - `armPreemptIds` returns how many ids it accepted, and a local arm is only reported as armed when one was. A tombstoned id (its steer drained at an ordinary boundary mid-request) no longer answers `preempt: true` for an interrupt that cannot happen. - The cancel disarm is awaited. A dropped clear is worse than a dropped arm: the owner keeps a level-triggered request for a steer that no longer exists, seals its next chunk and truncates an unrelated answer. The boundary drain's own call stays non-blocking — there the owner is local, so the disarm is already effective and awaiting the informational publish would only delay injection. - Subscriber count is NOT read as proof of owner receipt: the count includes this replica's own facade subscription. A successful publish reports armed, a rejected one does not. Documented rather than papered over — see the acknowledgement-semantics note on the PR. CI regressions from round 5, all mine: - `registerPreemptSubscription` was AWAITED at both runtime-init sites, so job creation blocked on a second Redis channel subscription and hung when that subscribe was slow. Abort is awaited because a missed abort strands a run; a missed preempt only degrades that steer to the next tool boundary, so it now registers without gating createJob. - Two api specs mocked `@librechat/api` without the newly imported `isSteerPreemptSupported`, so the call threw before createJob; and one exact-match assertion needed the new `preemptCapable` metadata field. - My own Redis integration spec asserted arm-before-clear ordering, which two publishes carry no guarantee of — the receiving tombstone exists precisely because of that. Now asserts delivery and payload fidelity, order-independent. 158 packages/api specs, 27 api specs green. * 🧭 fix: Codex round 7 — settle the acknowledgement semantics (Option A) Round 7's second finding is the incoherence I flagged on the PR: the route persisted `preempt: true` on the durable queue item while returning `preempt: false` when delivery could not be confirmed. Those two then disagreed, and `rearmQueuedPreempts` trusts the DURABLE one — so a resumed owner would honour an interrupt the client had explicitly been told degraded to ordinary steering. Rather than patch the disagreement, this settles the meaning: `preempt` in the 202 means "queued as an interrupt request", NOT "a seal is guaranteed". It mirrors `SteerQueueItem.preempt` exactly, so the response, the durable record, and the resume-time re-arm can never disagree. The gates that ARE knowable stay — the owner's recorded capability and a successful enqueue. Everything past that degrades to the documented fallback of injecting at the next tool boundary. A route cannot synchronously know whether another replica will seal: proving it needs a correlated request/response over pub-sub, and even that only proves the owner heard, not that it is still streaming when the arm lands. Four rounds of tightening this boolean each surfaced a narrower case; the sequence does not converge, so the invariant is now "the flag describes the durable decision" and an unconfirmed arm logs a warning instead of rewriting the answer. Also from this round: a failed disarm publish is retried once and its outcome reported. `handleSteerCancel` keeps `removed: true` — the steer really did leave the queue, and saying otherwise would make the client re-show a chip for a steer that can never arrive — and adds `disarmed: false` so the residual risk is visible rather than swallowed. Damage stays bounded regardless: the empty-boundary self-clear disarms the generation after a single seal. Tests: +1 pinning the response/durable-flag invariant. 159 packages/api specs green. * 🧹 fix: Codex round 8 — remove the unverifiable disarm signal Round 8 found the same over-promise on the disarm side that round 7 corrected on the arm side, so this applies the same answer rather than patching around it. The `disarmed: false` field added in round 7 was both unreliable and unused: a resolved publish is not proof the owner heard it (the delivery count includes this replica's own facade subscription), and it was never threaded into `CancelSteerResponse` or read by any client. A signal that claims a certainty the transport cannot provide is worse than no signal — it invites callers to trust it. Removed from the response. The retry stays, because it genuinely reduces the failure rate, and `noteSteersRemoved` still returns whether the publish succeeded FOR LOGGING, now documented explicitly as "published without error", not "the owner disarmed". Disarm is best effort with a bounded, self-healing failure: if the clear is lost the owner seals once, the empty-boundary self-clear disarms the generation, and the turn is persisted `unfinished: true` rather than silently truncated. Tightening that further needs a correlated request/response over pub-sub with a timeout — noted on the PR as the deliberate boundary of this design rather than an oversight. 130 packages/api steering specs green. * 🧽 fix: Codex round 9 — spend snapshot arms on nonempty drains too The round-6 scoping fix only cleared the pre-drain snapshot when the drain came back EMPTY. On a nonempty drain the `finally` cleared just the drained ids, so a stale arm — typically a cancel whose cross-replica clear was lost — survived the boundary. It would then immediately seal the continuation meant to answer the steer that had just been injected, and land on an empty boundary as `preempt_incomplete`: the interrupt appears to work, and the answer to it is truncated. A boundary that runs has spent its seal, so everything armed at snapshot time is spent whether or not it came back from the drain. The `finally` now clears the union of the snapshot and the drained ids. Arms that land AFTER the snapshot are still spared — their queue items are live and uninjected, which is the property round 6 added. Also fixes an api-workspace CI failure of mine: `resume.spec.js` stubs `GenerationJobManager` wholesale, and the round-3/4 resume work added two calls (`updateMetadata`, `rearmQueuedPreempts`) the stub did not define, so 34 specs threw. Stub extended. Tests: +2 (a nonempty drain clears a stale snapshot arm; a nonempty drain spares an arm that landed mid-drain). Counterfactually verified — the stale-arm spec fails against the unfixed drain. 132 packages/api specs, 60 resume specs green. * fix: never let a failed preempt subscription reject into the void registerPreemptSubscription is called detached at both sites, so a rejected Redis SUBSCRIBE became an unhandled rejection — process-fatal under Node's default --unhandled-rejections=throw. The comment already promised this path merely degrades steering; it now does. Swallowed and logged inside the registration rather than at each call site, so a future third caller cannot reintroduce the trap. Losing the channel costs this generation's cross-replica preempts, not the server: same-replica arming is runtime state and still works, and remote arms fall back to the next tool boundary. Verified counterfactually — the new spec surfaces SUBSCRIBE failed as an unhandled rejection against the unfixed registration. * docs: state the real blast radius of a failed preempt subscription LibreChat's own entrypoints install a global unhandledRejection handler that logs and keeps serving, so the escaping rejection this guards was never fatal to this server — only to another consumer of @librechat/api that installs no handler. The fix stands either way; the comment just should not overstate what it prevents. * test: cover the cross-replica preempt hop with two manager instances Every other preempt test runs against a single manager, so the hop that actually carries an interrupt in production had no coverage: the steer POST lands on whichever replica the balancer picks, which is usually not the one generating. Non-owner publishes, owner arms, owner's level-triggered poll flips — none of that was exercised end to end. Two GenerationJobManagerClass instances are a faithful replica pair here. runtimeState and ownedJobs are private instance fields, there is no module-level mutable state between them, and createStreamServices duplicates a dedicated subscriber connection per call, so separate OS processes would exercise the same objects over the same Redis. Both assertions verified counterfactually against real Redis: - Deleting the preemptCapable deserialization in RedisJobStore fails this with 'Expected: true, Received: undefined' — the exact P1 that shipped past every in-memory test and would have made the feature a silent no-op on every Redis deployment. - Dropping the non-owner arm publish fails it with 'Received: false'. * test: remove the fixed sleeps and vacuity from the cross-replica preempt test Codex round 11, both findings, both on the test I added last commit. P2 — the 300ms waits were load-bearing. Redis pub/sub never replays and the owner's SUBSCRIBE is detached, so on a slow CI worker the publish could land before anyone was listening and the test would fail against correct code. Now it republishes until the owner's state converges, which is safe because arms and clears are idempotent set writes keyed by steerId. Side effect: the tests got ~10x faster (85ms/57ms vs 929ms/606ms) since they finish on delivery rather than on a timer. P3 — afterEach destroyed only the transports, leaving each manager alive in its own cleanup-interval closure, still working against a dead transport. Now tracks the managers and awaits destroy(), which disposes the job store and its timer too. Matches how the rest of this file cleans up. Fixing the sleeps exposed a third problem codex did not flag: the stale-arm test could pass vacuously, because an undelivered arm and a fenced one look identical. It now brackets the stale publish between two control arms — the first proves the owner is listening before the stale one is sent, the second proves it has had its chance to arrive. Verified counterfactually against real Redis, and stable over 5 runs: - dropping the preemptCapable deserialization fails with 'Received: undefined' - dropping the non-owner arm publish times out both tests - removing the generation fence fails the stale test with ["control-before", "steer-stale", "control-after"] — which also confirms the bracketing orders as intended rather than by luck * fix: gate interrupt on the OWNER's capability alone, not the route's Codex round 12. The comment above this gate already said 'the OWNER's recorded capability, not this replica's probe' — and then the code ANDed in isSteerPreemptSupported(), which is exactly this replica's probe. The contradiction dates to the original commit; round 6 made the gate owner-scoped and wrote that comment without removing the local conjunct. The route never seals. It enqueues and publishes an arm, neither of which touches the SDK, so during a rolling deploy a steer landing on an un-upgraded replica silently lost its interrupt even though the owner could seal. When the route IS the owner the probe is redundant anyway: the flag it would consult is the one this process wrote at createJob. The real degradation path is unchanged and still tested — an owner that recorded no capability relabels to an ordinary steer. The test that pinned the local probe asserted an impossible same-replica state (capable metadata plus an incapable local SDK, when the metadata is written from that probe); it now pins the mixed-SDK direction instead, and fails with 'Expected: true, Received: false' if the probe is put back. * fix: reconcile arms at handover, and stop holding the 202 on a publish Codex round 13, two of three findings. P2 — rearmQueuedPreempts only ever ADDED. A replica that merely read the job still installs a facade runtime and subscribes, so it can accept an arm and then miss the best-effort clear that follows the drain. HITL resume promotes that facade to owner, the union keeps the orphan, and the first resumed stream seals on a steer no longer in the queue, drains nothing, and truncates the resumed answer as preempt_incomplete. acquireResumedJobOwnership only sets ownedJobs, so nothing else was clearing it. The durable queue is the sole authority at a handover: arms it does not back are now disarmed and tombstoned, so an in-flight publish cannot revive them either. Worth recording that my own independent review raised this and my verifier refuted it. Codex found it separately; two reviewers converging should have outweighed one refutation. P2 — the route awaited the arm publish before answering. The 202 reports capability, not delivery, so the await could not change the response; it only exposed the caller to Redis latency after the queue item was already durable. A client that times out and retries mints a second steer while the first stays queued, injecting the same instruction twice, whereas a lost publish merely takes the tool-boundary fallback. Detached, with both outcomes logged. All three tests verified counterfactually: union-only rearm fails the two new handover specs, and re-awaiting the publish hangs the stalled-publish spec until jest kills it. * fix: snapshot arms before reading the queue at handover Codex round 14 — a regression from my own round-13 fix, and a worse failure than the one it corrected. Round 13 read the durable queue first, then tombstoned any armed id the snapshot did not back. But approvals.resolve reopens steering before reconciliation runs, so another replica can commit a preempt steer and publish its arm while the peek is in flight. That arm is then present locally but absent from a snapshot taken before the steer existed, so a LIVE interrupt the route already acknowledged got dropped — and tombstoned, which blocks the re-arm, making it unrecoverable rather than merely late. Fixed by inverting the two reads rather than by locking or paying a second round trip. A steer is durably enqueued BEFORE its arm is published, so any id in an arms-first snapshot was already queued when it was armed, and the later peek must observe it unless it has since drained — which is exactly the orphan this reconciliation exists to drop. Arms landing after the snapshot are simply not candidates. Also re-checks runtime identity across the await, since the generation can be replaced while the queue read is in flight. New spec injects a steer + arm during the peek and verifies it survives; against the round-13 ordering it fails with Received array: []. * fix: bound the cancel disarm wait and fence enqueue to its generation Codex round 15. P2 — the cancel awaited its disarm publish unbounded. ioredis queues commands during an outage rather than rejecting, so that await could hang for the length of the outage with the steer ALREADY durably cancelled; a client that gives up then treats the cancel as failed and restores a chip for a steer that can never produce an applied event. Every successful cancel publishes, so ordinary steers were exposed too, not only preemptive ones. Now bounded at 1s, with the publish continuing behind it — its retry and logging are unchanged, it is just no longer in front of the response. This is the sibling of round 13's arm-publish finding; I fixed one path and left this one. P3 — enqueue was not fenced to the generation the capability decision was made against. The access checks, file resolution and owner re-read are all awaits, so the run can be replaced before the enqueue: the item then lands on the REPLACEMENT queue while the durable preempt flag and the arm still name the previous epoch, the arm is fenced out at the owner, and the 202 promises an interrupt that cannot happen. enqueueSteer now takes an expected generation, mirroring drain/peek, and the Redis path enforces it inside STEER_ENQUEUE_LUA so the check is atomic with the push rather than racing it. All three new specs verified counterfactually, including the Lua guard against real Redis (removing it returns 1 where -1 is required). * ⚡ feat: Interrupt & Steer — client half (PR 3 of 3) Makes preemptive steering reachable. Consumes the server contract from PR 2 (POST /chat/steer `preempt`, echoed on the 202) and the SDK seam in @librechat/agents 3.3.5. Settings shape follows the agreed correction, NOT the earlier plan draft: `steerInterruptsByDefault` is a boolean ORTHOGONAL to `duringRunDefaultAction` — that enum still chooses steer-vs-queue, the new boolean chooses how soon a steer lands. This deliberately avoids widening the enum to three values, which would have silently broken two hard-coded binary TOGGLES (`DuringRunAction.tsx`'s setter and `SteerMenu`'s `useDefaultToggleEntry`, both `prev === 'steer' ? … : …`) where a third value collapses to the wrong branch and one click erases the setting. - useSteering: `submitSteer` takes an opts bag and threads `preempt` into the POST, the optimistic chip, and the failure chip. The ACK relabels from the SERVER's echo, so a deployment that cannot seal mid-stream downgrades the chip's wording instead of erroring — the entire UX surface of capability degradation. New `interruptSteer` reuses the whole chip lifecycle and degradation ladder, and falls back to `interruptAndSend` when `!canSteer`, because steering needs a server-side job and an always-visible button would otherwise be dead for the whole first turn. `steerFromComposer` honours the new preference. - Composer: always-visible `InterruptSteerButton` with one fixed meaning (stop now, keep what's written), disabled on a paused run to pre-empt the server's 409, `type="button"` so it never steals the form's Enter submit, RTL-correct margins. A fourth hovercard row on the during-run send button, and ⌘/Ctrl+Shift+Enter routed AHEAD of the bare ⌘/Ctrl+Enter branch that would otherwise swallow it. - Chips: an in-flight preempt chip reads "Interrupting" with a ZapOff glyph; `preempt` survives reconnect through `seedSteerChips`. - `RunEnd.interruptArmed`, `drainAfterAbortByIndex`, `useQueueDrain`, `stopGenerating` and `interruptAndSend` are untouched — the preempt path deliberately shares none of the abort machinery. Tests: 7 new specs (posts preempt, turn-1 fallback, empty-text refusal, default route with and without the preference, server-echo relabel, double-click). 66 useSteering specs green; tsc and lint clean. Round-1 review fixes folded in: - P1: interrupt & steer no longer hard-aborts a run paused on tool approval. `canSteer` is false there, so the fallback was routing the keyboard and hovercard paths into `interruptAndSend` — discarding the partial answer, the exact opposite of what the action promises. The fallback is now scoped to the missing-conversation case only, and a paused run refuses outright (the standalone button was already disabled; the guard now lives where all three paths reach it). - The preference no longer leaks into the explicit Steer action. `steerFromComposer` backs both the default Enter route AND the explicit hovercard row / Ctrl+Enter alternate; applying `steerInterruptsByDefault` inside it made ordinary Steer interrupt and the two rows indistinguishable. It now takes an explicit argument that only `submitDuringRun`'s default route sets. - Retry preserves preemption: a failed interrupt-steer chip keeps `preempt: true`, and `retrySteer` now forwards it rather than silently resubmitting as an ordinary tool-boundary steer. - ⌘/Ctrl+Shift+Enter defers to a rebound submit shortcut, mirroring the bare ⌘/Ctrl+Enter branch — a user who bound submit to that chord keeps getting submit. Round-2 fix: the preempt label now survives the page-reload resume path too. `seedSteerChips` (useResumableSSE) and `restoreSteerChips` (useResumeOnLoad) are two independent TPendingSteer→PendingSteer mappers with near-identical bodies; the first carried the flag and the second silently dropped it, so an armed interrupt reverted to plain "Steering" after a reload. Swept: those are the only two in production code. The reclaim/convert paths deliberately omit it — a queued follow-up starts its own turn, so there is nothing to interrupt. * fix: yield the interrupt-steer chord only to a submit shortcut bound to it The previous guard skipped the Ctrl/Cmd+Shift+Enter branch whenever ANY submitMessage override existed. Rebinding submit to something unrelated (Ctrl+J) or unbinding it entirely then fell through to the override resolver, which returns 'none' for shifted Enter — silently removing the shortcut the hovercard still advertises. Compare the pressed chord against the configured one instead. The adjacent bare Ctrl/Cmd+Enter branch keeps its any-override guard on purpose: that chord IS the default submit chord, so once submit moves the resolver should own it. The predicate already existed inside resolveSubmitOverrideAction; pulled it out as bindingsMatch so both sites compare chords the same way. That call is behavior-preserving — eventBinding.key is 'Enter' by the early return, and equal hashes imply equal keys, so the dropped explicit key check was redundant. * fix: disable the Interrupt & steer menu row while paused on approval interruptSteer hard-refuses when the run is paused for tool approval, but the hovercard row was never gated, so it rendered enabled and clicking it did nothing at all — no chip, no queue entry, no toast — at exactly the moment a user is trying to say "stop, don't run that command". The standalone button already gates on pausedOnApproval; the row contradicted it. Gated on pausedOnApproval rather than !canSteer like the steer row above, because canSteer is also false before a conversation exists, where interruptSteer deliberately falls back to interruptAndSend and the row must stay live for the whole first turn. Tests pin both directions and were verified counterfactually: removing the gate fails the paused case, and using !canSteer fails the first-turn case. * test: render the during-run hovercard eagerly instead of driving Ariakit The new spec passed locally and failed all four cases on CI's Ubuntu and Windows shards: Ariakit's show path keys off pointer geometry, which jsdom reports as zeros, so whether a synthetic mouseEnter opens the hovercard is environment-dependent. Driving it was testing Ariakit's hover behavior, not which rows this component disables. Mocking the three Ariakit primitives renders the rows unconditionally and drops the fake timers. Both counterfactuals still fail as they should: removing the gate fails the paused case, !canSteer fails the first-turn case. * test(e2e): cover interrupt & steer sealing mid-stream The mock Playwright suite covered every sibling during-run action — steer at a tool boundary, steer degrading to a queued follow-up, queue, and interrupt & send — but not interrupt & steer, the one this stack adds. Uses E2E_SLOW_REPLY, which streams pure text with no tools, so the scenario is the same one where an ordinary steer provably degrades to a queued follow-up turn. Injecting in-thread there is something only a mid-stream seal can do, which makes the assertion discriminating rather than incidental: the steer part lands in the response, the final chunk never arrives, the text written before the seal survives, and no follow-up turn pair is created. * test(e2e): assert the run resumes after the seal, not just that it sealed The other four assertions are all satisfied by a seal that killed the run: the steer part is persisted by applySteer during the drain, before the continuation starts, so 'sealed and resumed' and 'sealed and died' were indistinguishable — and resuming is the whole difference from interrupt & send. The continuation answers the injected steer, whose text carries no fake-model marker, so getLatestUserText falls through to the default reply. That string ('E2E mock reply') is distinct from the setup turn's ('E2E reply <label>'), so seeing it proves generation restarted rather than matching text that was already on screen. The test itself is confirmed working: it ran as 104/121 in the Playwright job on 0ece357 and the run concluded success. * test(e2e): drop the resume assertion pending an unresolved question The assertion that the run visibly resumes after the seal fails deterministically in CI across all three retries. Every other assertion in the test passes, so the chord, the server preempt, the seal, and the in-thread injection all work; what fails is only the continuation's reply becoming visible. I could not determine from CI logs whether that is the mock harness not surfacing a continuation in a no-tool scenario or generation genuinely stopping after the seal, and I am not willing to weaken it into something that passes either way — that would convert a real question into false assurance. Reverted to the four assertions that hold, with the open question recorded in the test and raised on the PR. * fix: yield the interrupt chord to any bound shortcut, and move the Enter hint Codex round 1 on this PR (its first — LibreChat-AI#14519's rounds predate these files). P2 — the chord yielded only to a rebound submitMessage. This composer handler runs before the document-level one in useKeyboardShortcuts, and that one does not check defaultPrevented, so binding any composer-allowed action (focusChat, focusSearch, showShortcuts) to Ctrl/Cmd+Shift+Enter fired BOTH: the run was interrupted and the bound action ran. Now yields to any chord the user has bound. No default binding uses this chord, so it only ever yields to a deliberate rebinding. P2 — with steerInterruptsByDefault on, plain Enter routes through submitDuringRun and preempts, but the hovercard still put the ⏎ hint on the ordinary Steer row, whose click deliberately does NOT preempt. The same row advertised a key that did something else. The hint now follows the preference: ⏎ moves to Interrupt & steer, and the Steer row shows none because no key reaches it in that mode. I had declined this twice on the grounds that the behaviour split is deliberate — it is, and it is unchanged; the finding was about the label, which was a different claim and a correct one. Lint also caught a real bug in the first fix: boundShortcutChords was missing from the handler's dependency array, so a rebinding would not have taken effect until the callback was recreated for another reason. Both verified counterfactually; 179 client specs green. * fix: resolve every composer Enter chord through one decision table Codex round 2: yielding the preempt branch to a bound chord dropped execution into the bare Ctrl/Cmd branch below it, which ignores Shift, and past that into the submit tail, where isCtrlEnter is true for the chord. So rebinding focusChat, focusSearch, or showShortcuts to Ctrl/Cmd+Shift+Enter fired the alternate action or a submit AND the document-level shortcut, consuming the draft. Two rounds in a row landed in this handler because the guard chain decided "whose chord is this?" piecemeal inside individual branches, with fall-through between them. This extracts the entire pipeline into resolveComposerKeyDown (utils/shortcuts, beside resolveSubmitOverrideAction, which it absorbs as a step): one pure decision table where every verdict is terminal, so the fall-through class is gone rather than patched around. The yield rule is now the first gate before all branches: an Enter chord bound to any shortcut the document handler runs while typing (EDITING_ALLOWED_SHORTCUTS, hoisted out of the handler and shared by both dispatchers) is left entirely to that handler. This also closes the identical latent holes in the branches codex did not flag: bound Alt+Enter and Ctrl/Cmd+Enter chords double-fired the same way during a run, and the idle submit tail consumed bound chords too. A chord bound to a shortcut the document handler does NOT run while typing keeps its composer meaning; there is nothing to collide with, and yielding would just make the chord dead. Also merged the spec file round 0 added at utils/__tests__/ into the pre-existing utils/shortcuts.spec.ts (duplicate coverage at a second path) and dropped the isNonShiftEnter+filesLoading preventDefault, which was subsumed by the unconditional one below it. The table has a spec locking every verdict. Counterfactually verified: removing the yield gate fails exactly the three yield tests. Full client suite green apart from six suites that fail identically without this change (local data-provider dist drift in unrelated areas). * fix: yield Alt+Enter to a rebound submit, derive hovercard hints from the decision table Codex round 3, both findings. P2, Alt+Enter submit rebinding. The interrupt branch reserved Alt+Enter unconditionally, so rebinding submitMessage to Alt+Enter meant the user's own submit chord aborted the run and consumed the draft as interrupt & send. Now guarded with the same bindingsMatch yield the preempt branch already had; the chord falls through to the submit override resolution and submits the default action. P2, hovercard hints. The rows advertised hardcoded chords, so a chord rebound to an editing-allowed global shortcut (yielded by the decision table) or claimed by a rebound submit still appeared as Steer, Queue, Interrupt & steer, or Interrupt & send. The hovercard now asks the same decision table the composer executes what each canonical chord does and only labels a row with a chord that still triggers it. That also fixes two dishonest hints codex did not flag: with Enter-to-send off, plain Enter inserts a newline during a run (the primary row advertised it anyway) and Ctrl/Cmd+Enter submits the default action (the alternate row claimed it). The default-action hint now moves to Ctrl/Cmd+Enter in that mode. The effective bindings (submitOverride plus yielded chords) moved from useTextarea into a shared useComposerBindings hook so the handler and the hints read the same source. resolveComposerKeyDown now takes a KeyChordSource pick of the event fields it reads, letting the hovercard pass synthetic chords. Counterfactually verified: reverting the Alt guard fails the new resolver test and the hint suppression test. 353 tests across the affected suites green; the two failing hook suites (useVisibleTools, useResumableSSE) fail identically without these changes (local data-provider dist drift). * fix: never advertise a chord on a disabled hovercard row Codex round 4, one P2. The Interrupt & steer row kept showing its chord while disabled for tool approval, but pressing it reaches interruptSteer's pausedOnApproval guard and no-ops. Fixed as a render rule rather than a per-row patch: a disabled row never shows its kbd, since its action refuses the chord by the same guard that disabled it. That also covers the disabled Steer row, whose alternate-action hint had the identical hole through steerFromComposer's canSteer refusal. Counterfactually verified: reverting the render guard fails the new test.
Why
The only mid-run injection seam today is
PostToolBatch, and it fires only when a batch produced tool entries (ToolNode.ts, guarded onorderedBatchEntries.length > 0). During a long text answer there is no boundary at all. A host that wants to redirect the model mid-answer has exactly two options right now: wait for a tool step that may never come, or abort the run and throw away everything it wrote.This adds the missing third option — a cooperative seal.
What it does
Seal the live model stream at a provably-safe chunk boundary, dispatch a new
PreemptBoundaryhook, and self-loop the agent node inside the same pregel run. No abort, no job-status change, no checkpointer, no secondprocessStreampass. The partial assistant turn is kept and the model continues in the same message.PostToolBatchinjectionThe safety argument
canSealPreempt(src/llm/preempt.ts) refuses to seal unless the accumulated chunk already satisfies all of:thinking/reasoning/redacted_thinkingdo not counttool_calls/tool_call_chunks, andinvalid_tool_callsempty — with one refinement: an Anthropic server-side call whose pairedweb_search_tool_resultblock has already landed counts as settled (provider-side tools never produce aPostToolBatchboundary, so without this a turn would become permanently unsealable the moment a web search starts)tool_calls; theirtoolCall/toolResponsecontent blocks are counted pairwiseChunks accumulate monotonically through
concat, so an unsafe shape can never be observed at a seal point. Nothing is stripped and nothing is repaired — if the shape is not already safe the stream simply runs on and the injection falls through to today's tool boundary.Only the SDK's own dispatch-synchronous loop may seal. The registered-stream-handler loop is excluded by construction (brand check
SDK_STREAM_DISPATCH, robust tocomposeEventHandlers/createRunHandlerswrappers): it dispatches through a decoupledstreamEventsconsumer that can lag the accumulated chunk, which would let a host index a content part the user has not been shown.Native close of the sealed model run
@langchain/core's_streamIteratorcallshandleLLMEndafter its try/catch with nofinally, so breaking the consumer'sfor awaitfires neitherhandleLLMErrornorhandleLLMEnd— the run would stay open in every callback handler and each seal would cost one unrecorded full prompt. The seal therefore closes the real run natively: the chat model's actual runId is observed via ahandleChatModelStartcapture handler (it cannot be dictated — bound runnables consumeconfig.runId), and the close composes the per-call config callbacks with the model's own (clientOptions.callbacks) — unwrapping the production wrapper stack (bindTools'RunnableBinding, a system runnable'sRunnableSequence) to find them, the same composition the real run uses — so no handler class is left with an open span. Usage for the sealed turn is synthesized from the agent's token counter when the provider never got to send its usage chunk.Wire shaping for strict-alternation providers
Boundary injection can put two user turns next to each other (context + steer, a two-steer drain, or a steer landing right after tool results). Three layers keep the wire valid without touching graph state or host persistence:
coalesceAdjacentUserTurns(src/messages/alternation.ts) merges consecutive plain user turns at the last provider-facing hop, keyed on the provider actually serving the call (attemptInvokeis the single funnel, so fallbacks are covered — an OpenAI primary failing over to Bedrock gets the same shaping). The merged turn keeps the first turn's id (origin tracking re-attaches by key) and the last turn'sadditional_kwargs(the prompt-cache tail anchor reasons positionally). Tool-result-only turns are excluded; mixed turns merge with block order preserved. The no-merge path returns the input array by identity, so the funnel's second pass over an already-normalized payload is a walk with zero allocation.ToolMessageandHumanMessageboth convert torole: 'user', so a hook injection after tool results otherwise reaches Converse as adjacent user messages.formatAgentMessagesmarks a replayed steer as answered with a placeholder assistant turn — deferred until a later entry actually emits (never stranded as a final prefill), and suppressed when the next emitted message is itself an assistant turn (which is its own separation; emitting anyway would create adjacent assistant turns).Enforcement nuance, live-verified (2026-07-28): Mistral rejects consecutive user turns outright; Bedrock Converse documents strict alternation but claude-sonnet-4-5 on Converse currently accepts both the unmerged and merged shapes. The shaping is contract-safety across Converse model families (Nova, Llama, …), not a bet on per-family leniency.
Halt semantics and telemetry
PreemptBoundaryhook returningpreventContinuation: truehalts without destroying the sealed turn: the registry halt (whose stream-cancel would abort pregel before the outer reducer commits) is cleared at the boundary, the reason is carried on the graph, andprocessStreamtransfers it intogetHaltReason()after theStopdispatch — hosts keep their persistence signal,AgentSessionemitsrun.halted, and the hook's ownstopReasonreachesStophooks verbatim.MultiAgentGraph, clearing the registry halt removed the only signal the outer routing consulted — so every model node now carries an entry guard: when the halt reason is set,createCallModeldeclines the turn and the workflow (static direct edges, Command fan-out, fan-in wrappers, parallel siblings' next turns) drains to END with the sealed commit intact. One chokepoint covers every routing mode; aCommand({goto: END})cannot suppress statically compiled edges.getHaltReason() === 'preempt_incomplete'— a truncated answer can never be finalized as a natural completion — and counts towardgetPreemptStats().emptyBoundarieswhether or not the hook also halted.callerConfig.signal(Graph.callerSignal, assigned unconditionally per call; composed withAbortSignal.anywhen both exist and differ).graph.signalstays pure construction state — its other consumers (model-call config, subagent parentSignal) are unaffected.Scope
shouldPreemptis contractually level-triggered (non-consuming): the SDK never clears the host's request, and a true result is honored only once the accumulated chunk is provider-safe. The seal condition's poll-first order is load-bearing — evaluating the shape check before the poll would walk the accumulated content on every chunk.PreemptBoundaryis not result-altering — registering a drain hook never silently disables eager tool execution. Pinned by a test.ToolNode.convertInjectedMessagesis lifted to@/messages/injectedand shared by both injection sites — the provider-safety argument rests on the two sites emitting identicalHumanMessageshapes.buildChildInputsdoes not propagatepreemption, andshouldPreemptStream()returns false forsubagentScope.maxSeals, default 8) is per-turn; a HITL resume keeps its remaining budget. Preemption-enabled runs reserve recursion-limit headroom equal to the budget so a preempted tool-heavy agent cannot hitGraphRecursionError.MultiAgentGraphroutes parallel agents through oneStandardGraph: the seal claim is a synchronous one-in-flight slot, so concurrent streams cannot double-spend a queued message.trackProviderMessageOriginslike every other provider transform, so merged turns keep calibrated token attribution (a bare merge could flip a just-fits payload into a spurious pre-invoke overflow).RunControl,keepContent,prune.ts, ToolNode execution paths, and all other converters are untouched.Tests
95 tests across 9 PR-owned suites (plus additions to the Bedrock converter suite), all passing:
src/llm/preempt.test.ts— truth table over every seal conditionsrc/specs/preemptSeal.test.ts— fake-model end-to-end: seal → empty boundary →preempt_incompletewith partial content kept; seal → inject → self-loop resume → completion with budget spent; halting hook'sstopReasonthroughStopandgetHaltReason(); a two-agent run where a halting boundary stops the direct-edge successor (agent B takes a model turn without the guard, none with it); model-level callbacks balanced through a realRunnableBindingwrapper (each detector counterfactually verified — fails without its fix)src/graphs/__tests__/Graph.preemptSignal.test.ts— abort-signal composition (both channels, either alone, clearHeavyState)src/messages/alternation.test.ts+src/llm/invoke.alternation.test.ts— merge semantics (kwargs/id survivorship, mixed tool_result, camelCase variant) and the fallback funnel (strict vs tolerant provider, idempotency)src/messages/formatAgentMessages.steer.test.ts— anchor deferral, assistant-turn suppression, no-adjacent-user-turns invariant, Anthropic converter round-tripsrc/hooks/__tests__/preemptBoundary.test.ts,src/messages/injected.test.ts,src/stream.dispatch.test.ts— event registration/capabilities, injected-message shape parity, dispatch brandThe former "known gap" (no integration test for the seal → boundary → self-loop path) is closed: #343 unblocked jest for the
providersimport chain, andpreemptSeal.test.tsruns the full path on a fake streaming model.Live verification
Seal → boundary → inject → resume verified against real providers on the final head: anthropic, bedrock, vertexai (gemini-2.5-flash), openAI, azureOpenAI, openrouter, deepseek (deepseek-reasoner), xai (grok-3-fast) — all
ok, lifecycle counters balanced, no orphaned spans.--controlmode (no preemption) verified as the baseline. Bedrock wire probes verified the merged[toolResult, text]user message is accepted end-to-end.A live scenario matrix (
src/scripts/preempt-scenarios.ts, anthropic + bedrock) additionally verified: two seals in one turn (three balanced model runs); the gate holding through a tool-call turn with the tool executed exactly once and the steer landing after the result; budget exhaustion with a never-disarming predicate (second seal refused, turn completes); promptCache + seal + steer with no rejections; the multi-agent halt guard live (successor never starts, halt reason surfaces); and a seal landing after an Anthropic server-side web search settles — the sealed[server_tool_use, web_search_tool_result, text…]content replays and is accepted.Known interaction (pre-existing mechanism, documented not fixed): in a multi-agent direct-edge flow on Anthropic, the successor receives the previous agent's trailing assistant turn as a prefill continuation. A steer's short, complete-reading resume can leave the continuation nothing to add, yielding an empty successor turn (reproduced 3/3 live; the no-preemption baseline's successor output is itself continuation-shaped, and OpenAI is unaffected). Preemption raises the likelihood of the trigger shape but the prefill framing predates it — tracked as #345, out of scope here. Untested for lack of working credentials: mistral (401 — note Mistral is a strict-alternation provider whose converter path is unit-tested but not live-verified) and google direct (no
GOOGLE_API_KEY); Gemini itself is covered via Vertex. (src/utils/llmConfig.tspins xai togrok-2-latest, which this team can no longer access — probe ran withgrok-3-fast.)Review provenance
17 requested Codex review rounds, 42 findings — every finding either fixed with a regression test (most counterfactually verified against the pre-fix code) or refuted with executed evidence, plus a 20-agent adversarial self-review of the rebase surface. All review threads carry written dispositions.
Follow-ups
This is PR 1 of 3. LibreChat server (fenced preempt request set,
emitPreempt/onPreempttransport pair) and client (composer control,steerInterruptsByDefaultpreference) build on this and land after it.