29 commits

Author	SHA1	Message	Date
Danny Avila	e3ccaba5af	🛄 feat: Restore Compaction Guidance Across Continuations (#15356) ... * 🧭 feat: preserve compaction guidance across continuations * 🔧 fix: narrow persisted compaction fields	2026-08-30 08:13:24 -04:00
Danny Avila	773127bff2	🎠 refactor: Route Every Event Actor Turn Through One Lifecycle (#15325) ... * refactor: unify Event Actor turn lifecycle * fix: retain Event Actor fence ownership * fix: preserve mixed-version actor suspension safety	2026-08-28 17:30:23 -04:00
Danny Avila	3fa33b740b	🛫 refactor: Promote Generation Protocol V2 Automatically (#15324) ... * refactor: promote generation protocol v2 automatically * fix: remove unused protocol import	2026-08-28 17:17:09 -04:00
Danny Avila	77c2a51cf3	🪧 fix: Advertise Detached Event Actor Support from the Generation Store (#15322) ... * fix: activate detached event actions automatically * fix: shield detached terminal generations * perf: share capability availability index * style: flatten capability status selection * fix: preserve mixed-version lifecycle shells * fix: complete detached action store adapters * fix: close mixed-version capability races * fix: honor legacy capability success	2026-08-28 16:33:23 -04:00
Danny Avila	487984193a	🪂 feat: Make Event Actor Detached Actions Durable (#15307) ... * feat: make Event Actor detached actions durable * fix: align detached actor checks with merged base * fix: close detached actor lifecycle gaps * fix: break detached action outcome cycle * fix: deepen detached action ownership * fix: fence detached launch handoffs * fix: preserve detached rollout fencing * test: type detached recovery failure * fix: close detached ownership handoffs * fix: stage detached action activation * fix: persist detached terminal retries * fix: fence detached durability ownership * fix: fence trigger lane publication * fix: fence detached transition generations * fix: retire failed detached predecessors * fix: close detached rollout consumers * test: type detached retry turns * fix: close detached ownership gaps * fix: fence detached action resumes * fix: fence prior-head event actor resumes * fix: preserve legacy event actor resumes * fix: align detached resume boundaries	2026-08-28 14:21:04 -04:00
Danny Avila	c06b09c945	🤖 feat: make Event Actor HITL durable (#15305) ... * 🤖 feat: make event actor HITL durable * 🤖 fix: break event actor outcome cycle * 🤖 fix: close durable actor terminal races * fix: harden durable event actor recovery proofs * fix: close event actor resume publication races	2026-08-28 08:05:11 -04:00
Danny Avila	8ba76507ae	🧷 fix: Verify Scheduled HITL State Before Pausing (#15284) ... * fix: require durable storage for scheduled HITL * fix: verify scheduled HITL checkpoints before pause * fix: narrow scheduled HITL capability checks * fix: honor ask tool filters in schedule preflight * fix: enforce durability across scheduled HITL resumes * fix: scope scheduled HITL admission to active tools * refactor: move scheduled HITL admission to TypeScript * fix: close scheduled HITL admission gaps * fix: bind scheduled pauses to current checkpoint * fix: cap pending actions at binding deadline * fix: reject expired pending actions * fix: guard approval deadlines atomically	2026-08-27 11:16:24 -04:00
Danny Avila	68fc46a055	🪢 feat: Resume Bound Event Actors from Checkpoint Forks (#15227) ... * feat: resume event actors from checkpoint forks * fix: fence event actor checkpoint uncertainty * fix: satisfy event actor type contracts * fix: make actor reconciliation recoverable * fix: preserve event actor lifecycle transitions * fix: fence event actor lifecycle outcomes * fix: enforce event actor lifecycle ownership * fix: retain event actor settlement proof * 🔒 fix: Retain Event Actor Receipts Through Repair and Bound Their Journal Repair and compensation deleted the reconciliation row they resolved, which was the only durable proof that the invocation had already applied an external action. A delayed duplicate owner could then reacquire the same invocation id and repeat that action. Both resolutions now retire their receipt to `settled` and record how it settled, so the same-id tombstone survives; `history_repaired` and `action_compensated` still force a cold rebuild. Compensation undoes the effect without re-authorizing the delivery, so a legitimate retry must arrive under a new invocation id. A retried repair converges on its own receipt. Bound the journal so a long-lived actor cannot grow its conversation document without limit: a new fence is admitted only when no active lifecycle row exists, so a capped push can evict nothing but the oldest settled receipts. Stop shipping the unbounded source payload on every bound-child continuation. It rode the delivery body regardless of the feature flag while the sibling `fire` body deliberately sends event identity alone, so a large webhook payload could push a previously working delivery past the chat route's body limit. The actor binds an invocation from identity and never builds the prompt from it. Blank the positional token map on warm continuations. It is derived from the full DB history, while a warm run executes on checkpoint-restored state, so its indices address different messages and the pruner never recounts them — misattributing cached counts to the wrong messages in both directions. Keep the replaced-claim exit on its cleanup path when preserving reconciliation fails: the committing CAS already left a blocking row, so the failed status upgrade costs provenance, not safety. * 🧪 test: Pin the Warm Continuation's Map/Summary Asymmetry Give the warm-continuation client test a populated token map and a real cross-run summary so its assertions bite: the positional map must arrive blank (checkpoint-restored state no longer matches DB-derived indices, and the pruner never recounts a populated entry) while `initialSummary` must pass through unchanged — it rides the system tail and summarizes pre-boundary turns that were excluded from the very history the committed checkpoint was built from, so blanking it would silently drop context no warm run can recover. * ⚖️ fix: Honor Compensation in Settlement and Age-Bound the Receipt Journal A compensated receipt still tombstones its invocation id, but its external effect was explicitly undone — the terminal handler nonetheless replayed every settled lifecycle's stored action as authoritative and settled the public outcome as applied, telling an action-aware source the operation stands and suppressing the new-invocation retry compensation requires. The handler now settles a compensated invocation as failed with an explicit compensation error, overriding even fresh applied run evidence from a replayed generation; verified and repaired receipts continue to replay applied. Receipt eviction is now primarily age-based: a stale same-id owner is bounded by time, not by how many newer invocations settle, so the previous count-only slice let a high-rate actor evict a tombstone while its delayed duplicate owner could still wake and repeat the action. Admission prunes only settled receipts older than a retention window that dwarfs every generation, job, and delivery-retry lifetime, and the count cap is demoted to a raised document-size backstop. * 🔀 fix: Serialize Compensation Against Settlement and Never Evict Live Receipts The terminal handler read its lifecycle snapshot, verified history, and then settled the public outcome — so a compensation resolving the same receipt during that window lost: the handler settled applied from its stale snapshot and no retry could ever change the replay-identity-locked outcome. The receipt's status CAS is now the serialization point: verification resolves the receipt BEFORE settling, whichever transition wins determines the public outcome, and a crash between resolve and settle converges through the retained receipt's replay. The verified-replay probe requires the receipt's own resolution, so a compensated receipt can never satisfy a verification retry. This inverts the settle-before-receipt ordering deliberately: that ordering guarded proof that resolution used to delete, and the receipt now retains its full action proof through resolution. The document-size cap is no longer an eviction quota. A receipt inside its retention window is never discarded: when the journal holds a full cap of unexpired receipts, new invocations are refused fail-closed until receipts age out, making duplicate protection and document integrity simultaneous invariants instead of a rate-dependent trade. * 🎓 fix: Keep Skill-Bearing Event Actors on the Legacy Path Skill primes are spliced into the message list directly ahead of the newest message, and a warm continuation forwards only that newest message — so a checkpoint-restored actor would keep serving the prime bodies baked in at its last cold start and never observe an edited or newly attached skill. Until the actor head carries a context fingerprint that forces a cold rebuild when the agent's skill context changes, agents with always-apply or manual skill primes stay on the legacy path, which re-primes fresh bodies every turn: correct on every event, just never warm. * 📜 fix: Gate Fork Mode on the Skills Capability, Not Just Request-Time Primes History-derived re-priming was a third path into the same staleness class: an actor that previously invoked a skill carries no request-time prime arrays, yet primeInvokedSkills re-resolves that skill's current body from history each turn and the warm slice drops the reconstruction — leaving the checkpoint's old body active after edits. The fork gate now keys on the priming hook itself (present exactly when the skills capability is enabled) alongside the request-time arrays, so every skill-body path routes to the legacy rebuild until #15235's context fingerprint restores warm continuation for skill-bearing actors. * 🧾 fix: Capture Applied-Action Proof at Tool Execution, Not After sendMessage The executor read applied-action evidence from the run-step collection the instant sendMessage resolved, but that collection is populated asynchronously — an applied invocation could classify as actionless (runSteps still empty while the tool result already streamed), discarding its fork and stranding the actor cold while the terminal handler later settled the same delivery as applied from the persisted evidence. Authoritative proof is now recorded in graph context the moment the expected tool executes: the request-owned recorder observes the tool-end chain (which ToolNode dispatches synchronously with both input and output) and applies the same fences as run-step evidence — exact tool name with the MCP-suffixed form, the declared argument subset against the execution input, an error-free result, and the background non-execution receipt exclusion. readAppliedAction consults the receipt first; run-step inspection remains the fallback for paths that bypass the tool-end chain. Regression coverage reproduces the observed ordering: the real executor commits the head from the receipt while run steps are empty, warm-continues the next event, and never re-executes the action; recorder fences and the receipt-first controller wiring are covered separately. * 🎯 fix: Supply Execution Arguments to the Tool End Callback The live Vertex + MCP canary exposed a contract mismatch the synthetic fixtures hid: the ON_TOOL_EXECUTE execution path invoked its tool end callback with output only, while the action recorder must verify the declared argument subset against the execution input. The receipt never qualified, every turn fell back to cold history rebuilds, and the tournament advanced with zero actor heads and zero retained checkpoints while looking successful. The execution handler owns both halves at the same moment, so the fix is at the source rather than a correlation store: ToolEndCallbackData gains the executed call's input and every callback site passes tc.args. A handler-level regression drives the real createToolExecuteHandler and asserts the callback receives both fields; recorder regressions pin the production shapes — an output-only tool end must starve an argument-fenced receipt rather than trust an unfenced match, and still qualifies a name-only expected action. * 🕵️ fix: Mark Background Deliveries So They Cannot Impersonate Applied Actions The background-claim callback reports the ORIGINAL tool's name for artifact attribution on the poll turn that harvests a completed task. A name-only expected action could therefore be impersonated by work some earlier turn dispatched: the recorder would attribute that delivery to the current invocation and commit a head whose state never contained the invocation's own action. The run-step evidence path never had this hole — it sees the poll tool's name — so the recorder must match its provenance discipline. Delivery callbacks now carry an explicit backgroundDelivery marker set at the one site that rewrites the name, and the recorder ignores marked deliveries outright. Regressions pin both halves of the contract: the delivery callback must carry the marker with the poll call's arguments, and a marked delivery can never qualify even a name-only expected action. * 🧿 fix: Version Invalidations, Keep Evidence Ahead of Output Policy, Gate Detachable Actions Three closeout-round findings, each converted into an invariant. Every legacy-path invalidation now advances a durable epoch — including for headless and already cold-marked actors, where the marker alone leaves no CAS-visible trace — and the actor-head CAS requires the epoch observed at preparation. A concurrently prepared fork whose history predates an intervening legacy turn can no longer commit past it; the commit reports an ordinary conflict and journals. Execution identity is now emitted before post-execution output policy: when a side-effecting tool succeeds but its returned content is withheld by the output filter, the callback delivers an outputFiltered receipt with blank content — the recorder accepts it as proof (rejecting model-detached calls it cannot distinguish through the blank shape), the artifact path never sees it, and an applied action is no longer reclassified actionless and re-executed on retry. Background-capable expected actions stay off the fork path: dispatch returns a launch handle every evidence fence correctly rejects, and the completion is provenance-marked as another turn's work, so a fork would settle actionless before the external effect lands with nothing to stop a retry from dispatching it again. The gate mirrors the MCP-suffix name matching of the evidence path. * 🚧 fix: Seal the Whole Legacy Turn Behind a Second Epoch Advance The epoch fenced only the legacy turn's start: a fork preparing after the begin invalidation but before the turn's message persistence observed the new epoch and cold marker, rebuilt from history that did not yet contain the turn, and committed cleanly because nothing advanced the epoch again — making the incomplete rebuild authoritative and clearing the marker. Every legacy event turn now seals its invalidation at terminal persistence with a second epoch advance, on the success, replaced-claim, and error exits alike. Sealing deliberately carries no quiescence requirement — it must succeed while a fork fence is active, because that is exactly the mid-turn race it defeats — and a fork that already committed against the begin epoch is healed the same way: the seal re-marks the head cold, so the next event rebuilds with complete history. Seal failure never diverts the turn's own exit; the begin bump still fences everything prepared before the turn. * 🔗 fix: Replace the Best-Effort Epoch Bump With a Durable Legacy-Turn Fence The second epoch advance could not make a legacy turn atomic, and three findings shared that root cause: two conditional updates left a headless gap a fork could create the head inside; the error exit sealed before saveErrorTurn made the error history durable; and any crash or failure between persistence and sealing left an incomplete fork authoritative, because the seal was best-effort and its failure was swallowed. A legacy turn now carries one durable fence. A token is written before execution by a single update-pipeline write — no two-write gap, and the cold marker is applied only where a head exists via $cond/$$REMOVE. While the token is present no fork may prepare (the adapter refuses) or commit (the CAS requires its absence), because the turn's messages are not yet durable. One atomic write clears the exact token and advances the epoch once history is persisted — after saveErrorTurn on the error exit — and success, replacement, and error exits all route through it. Failure is now fail-closed rather than silent: a failed seal leaves the token set, which keeps blocking forks and is logged as such, and a fence abandoned by a crashed turn is reclaimed only once stale, advancing the epoch and marking any head cold so the next event rebuilds from whatever history actually survived. * fix: serialize legacy event actor turns * fix: close legacy actor fence ownership gaps * fix: preserve legacy actor persistence fences	2026-08-26 08:39:00 -04:00
Danny Avila	ac2aef00f6	🫗 fix: Drain Quoted Excerpts Into Mid-Run Steering (#15175) ... * 🧭 fix: Carry Quoted Excerpts Through Mid-Run Steering "Add to chat" quote chips were dropped by every during-run steer path: the steer POST had no quotes concept, so a composer-origin steer left the chip staged (gluing onto the NEXT send) and a queued item steered into the live run lost its quotes silently. Quotes now ride the steer protocol end to end: - POST + admission: `quotes` on the steer body, normalized like the chat route's (getReferencedQuotes caps), part of the idempotency fingerprint only when present so pre-existing receipts still replay. - Injection: merged into the model-bound turn as Markdown blockquotes at both boundaries (text-only and media paths), mirroring prependQuotes. - Persistence + replay: the STEER content part stores `quotes` separately from the typed text; stampSteerPartMedia re-merges them per turn (even with resendFiles off) via the SDK's transient media stamp, with the quote block folded into the token budget. - UI: composer steers/interrupt-steers drain the chips (skill picks stay staged — they configure a NEW turn's run); SteerPart and the in-flight bubble render the same MessageQuotes reference blocks as user bubbles; queued/failed rows show a quote count; reconnect reseeds fall back to the server item's quotes when no local chip survives. - buildMessages keeps its zero-await path to the parallel context kickoff via a synchronous stamp-target probe. * 🧭 fix: Keep Quotes in the Client-Safe Steer Projection toPendingSteer is the projection behind resume-state pendingSteers, abort responses, and terminal leftover claims — dropping quotes there would lose them on exactly the recovery paths the reconnect reseed's server fallback relies on. * 🧪 test: In-Flight Steer Bubble Renders Carried Quotes * 🔁 fix: Re-Stage Quotes When a Pre-Quotes Replica Accepts the Steer Codex flagged the rolling-deploy window: an old replica 202s a quoted steer while dropping the excerpts, so the client cleared the chips for context the model never received. The 202 (fresh and receipt replay) now echoes quotesAccepted from the DURABLE item; a missing echo on a quote-bearing composer-origin steer re-stages the excerpts as composer chips — the pre-steer behavior, so they ride the next send instead of vanishing — and strips them from the surviving chip so a later terminal conversion cannot duplicate them. Queued-origin steers keep quotes on the item, whose restore paths already return it intact. The residual cross-version lost-ACK retry stays fail-closed as a 409 idempotency conflict (failed chip with retry controls). * 🔁 fix: Close the Remaining Cross-Version Quote-Loss Windows Codex round 2: - Send now of a quoted queued item against a pre-quotes replica now re-stages the excerpts too (the row is consumed and the words inject bare, so the composer is their only remaining home); the strip clears the chip's captured origin copy so reclaims and terminal conversions cannot duplicate them. - A quoted retry whose lost first ACK was accepted by a pre-quotes replica now REPLAYS that legacy receipt instead of 409ing: the stored fingerprint matching the quote-less hash of the same words proves the cross-version case, and the replayed 202's missing echo drives the re-stage. Different quotes against a quote-bearing receipt still conflict. - TSteerAppliedEvent.part gains the quotes field (typed SSE consumers). * 🧪 test: Drop the Stale Narrow SteerDrainOutput Alias The spec's local intersection re-declared injectedMessages with content: string, predating the SDK pin that declares the field natively (content: string | MessageContentComplex[]). Under CI's clean install the hook's BaseHookOutput is no longer assignable to that narrower alias; the plain PostToolBatchHookOutput is the correct type for every drain/boundary assertion. Verified against the published 3.6.16 dist and the local one. * 🔁 fix: Honor the Generation Owner's Quote Capability End to End Codex round 4: - steerQuotesCapable rides job metadata (createJob + HITL resume rewrite), mirroring preemptCapable's owner-recorded pattern: an upgraded admission replica no longer stores quotes — or claims them accepted — for a generation whose older owning drain would silently drop them at injection. The missing echo drives the client re-stage, and a later capable handover cannot double-deliver restored context. - Applied events reconcile dropped quotes: when a quote-less applied part settles a quote-bearing chip (the lost-202 ordering the ACK-echo path cannot see), resolveSteerChip and both reconnect settle paths re-stage the chip's excerpts before removing their only copy. mergeRestagedQuotes dedupe keeps every trigger idempotent for the same excerpts. * 🔁 fix: Re-Read Quote Capability at the Last Moment and Cap Restaged Chips Codex round 5: - A HITL resume rewrites steerQuotesCapable without changing the generation's createdAt, so the enqueue fence cannot see a capable-to-legacy handover landing during admission's awaits. Re-read the owner's flag immediately before item construction (paid for only by quote-bearing requests); the residual between re-read and enqueue commit matches preemptCapable's documented race. - mergeRestagedQuotes now respects the 10-quote contract with the staged chips winning: a restored tail that cannot ride the next send is dropped explicitly instead of rendering as a chip the submission would silently discard. MAX_QUOTE_COUNT moves to utils/steer as the single client source; QuoteButton imports it. * 🔁 fix: Steer Quote Coverage for Preflights, Memory, and Single-Scan Stamping Codex round 6: - Stored-message policy inspection now extracts steer-part quotes as quote fragments (path /content/N/quotes/M), so conversation import and shared link preflights inspect the newly persisted field exactly like top-level message.quotes. - The memory copy gets its own quote-merge stamp (text only, resendFiles false): formatAgentMessages ignores part.quotes, so without it a steer whose substance lives in its excerpt reached the chat model but never memory extraction. - collectSteerStampTargets replaces the boolean probe: buildMessages collects once and hands the targets to stampSteerPartMedia, keeping the zero-await fast path without scanning the history twice. * 🔁 fix: Redis Quote Plumbing, Conversion-Race Guard, and Quote-Bound Recovery Proof Codex round 7: - RedisJobStore.deserializeJob now restores steerQuotesCapable (the explicit mapper otherwise dropped it on every read, leaving quote steering inert in Redis deployments), with the round-trip spec extended. - Both Lua parked-steer projections (terminal close + generation replacement) forward item.quotes, matching toPendingSteer — a lost final no longer strips excerpts from durable recovery in Redis mode. - The no-echo restage reads the SURVIVING chip (reclaimRejectedChipQuotes): a terminal conversion that beat the delayed 202 already moved the quotes onto the queued follow-up, and re-staging them again double-delivered. Regression-tested with the conversion-before-ACK ordering. - RecoveredSteerPayload binds normalized, order-significant quotes (builder, validator, TS matcher, and the Lua decode+matcher): a stale client presenting the same recoverySteerId with altered or missing quotes cannot consume the parked source. Quote-less sources keep matching quote-less recoveries. * 🔁 fix: Execution-Bound Quote Capability with an Atomic Enqueue Predicate Codex round 8: - steerQuotesCapable becomes a transient assertion translated (at createJob and in ApprovalLifecycle.resolve) into steerQuotesExecutionId, valid only while it equals the LIVE providerExecutionId. A legacy replica winning a HITL resume rewrites the execution id without knowing the marker, so its stale assertion self-invalidates — a bare boolean could not be cleared by code that predates it. - The fenced enqueue evaluates that equality atomically (all three Redis scripts decode-and-strip like the existing preemptCapable normalization; both InMemory sites mirror it) and returns the persisted item, so the quotesAccepted echo reflects exactly what was stored even when a handover lands between admission's read and the commit. The last-moment re-read is gone — the transaction is the authority. - Tests: capable-resume re-binding, legacy-resume omit-not-clear invalidation, the admission-vs-handover race (capability read true, then execution rewritten before enqueue), and the Redis round-trip of the marker. * 🔁 fix: Full Redis Parking Coverage and Loss-Moment Quote Restaging Codex round 9: - The two remaining Redis parking projections (terminal status CAS and stale-running cleanup) forward item.quotes — every field-picked steer projection now carries them (audited: 2 Lua 'projected' + 2 Lua 'clientItem' + toPendingSteer). - The ordinary no-echo ACK no longer re-stages: the steer has not injected yet, so the quotes stay carried on the pending chip. A quote-less applied event re-stages them at the actual loss; a terminal leftover conversion carries them onto the recovered row, whose normal send delivers quotes on any server — re-staging at the ACK let that leftover auto-send bare text while the excerpts glued onto an unrelated draft. Only the settled receipt replay (already injected, no future event) reclaims immediately. * 🔁 fix: Legacy-Replayable Receipts with Separate Quote Identity Codex round 10: an upgraded-first receipt stored a quote-inclusive fingerprint no pre-quotes replica could recompute, so a lost-ACK retry routed through one 409'd already-accepted words with duplicate-send controls. The durable fingerprint reverts to the quote-independent 3-field hash — the one shape EVERY deployed version computes, replayable across a rolling deploy in both directions — and quote identity moves beside it as requestedQuotesFingerprint (of the REQUESTED quotes, pre any capability strip, so an incapable-owner acceptance still replays its own retries). Absent records (legacy-written or quote-less) accept any same-words retry, preserving the round-5 rule; present records must match exactly, keeping different-quotes clientSteerId reuse a 409 on quote-aware readers. Under the keep-on-chip client contract a legacy replay's missing echo is harmless — the excerpts stay carried on the pending chip. * 🧪 chore: Re-Trigger CI After Dropped Workflow Events	2026-08-24 22:29:13 -04:00
Danny Avila	fc2b8584c4	📇 feat: Surface Event Child Activity Through a Bounded Parent Index (#15142) ... * feat: surface event-driven child activity * fix: keep child task aggregation documentdb-compatible * fix: address event activity review findings * test: provide markdown message context defaults * fix: report bounded child history truncation * fix: preserve current child activity state * fix: preserve durable event child activity * fix: handle missing task timestamps * fix: keep active event snapshots live * fix: preserve event activity across valid anchors * fix: close event child activity gaps * fix: preserve event activity across resume	2026-08-23 18:50:00 -04:00
Danny Avila	1de88e7e91	📨 feat: Continue Bound Child Agents from Events (#15112) ... * feat: add authenticated agent event ingress * style: sort agent ingress imports * fix: harden agent event ingress * fix: bind event provenance to API keys * fix: inspect event input with legacy PII filters * fix: scope event status reads to source keys * fix: bind event status reads to remote sources * feat: add bound event-driven child turns * fix: harden event-bound child continuations * fix: satisfy event binding type contracts * fix: close event actor lifecycle races * fix: harden event actor dispatch continuity * fix: fence event actor resume lifecycle * fix: bind event actor state to lifecycle * fix: preserve cascade write outcomes * test: type cascade failure injection * style: sort cascade test imports * fix: harden event child lifecycle boundaries * fix: make event cleanup retryable * fix: annotate event retention clock * fix: reconcile partial cascade metadata * fix: recheck event binding expiry on resume * fix: fence event actors by retention deadline * fix: close event actor lifecycle races * fix: harden event child lease acquisition * fix: lazy-load event child lease adapter	2026-08-23 01:15:57 -04:00
Danny Avila	67b7b441b2	🛂 feat: Filter Model-Bound Content by Source (#14425) ... * feat: introduce optional content protection seam * feat: enforce source-aware content filters * feat: complete source-aware content enforcement * test: activate skill file-text fail-close fixtures * fix: harden source-aware content filters * fix: harden model-bound content filtering * fix: preserve legacy filters and generated files * fix: inspect shared scalar metadata * test: align mocks with current dev dependencies * feat: add persisted content filter safeguards * feat: complete source-aware content filter enforcement * fix: move resume content preflight into TypeScript * fix: close content inspection edge cases * fix: harden content protection boundaries * fix: complete content protection safeguards * test: align persisted memory filter coverage * fix: reconcile content protection with current dev * fix: reconcile content protection with latest dev * fix: close content protection review gaps * fix: enforce source-aware provider boundaries * fix: preserve legacy PII preflight semantics * test: stabilize stored branch preflight fixture * fix: defer agent writes until protected model admission * perf: harden source-aware model-bound filtering * fix: canonicalize provider lineage before validation * fix: satisfy model-bound callback type checks * perf: Bound content protection filtering work * fix: Bound submission array traversal * fix: Stabilize bounded content snapshots * fix: Scope model-bound traversal overflows * fix: Preserve scoped content inspection * fix: Accumulate aggregate traversal scopes * fix: centralize content policy boundaries * test: align deferred tool policy context * test: align controller policy mocks * style: normalize content protection imports * fix: close content policy review gaps * fix: narrow active skill policy config * fix: address content protection review boundaries * fix: retain exact provenance overflow sentinel * fix: preserve literal and scoped provenance updates * fix: narrow persisted edit provenance * fix: isolate exact overflow attribution * fix: centralize stored prompt protection * fix: fail closed on incomplete transcript evidence * fix: align canonical transcript routing * refactor: centralize content policy preflights * fix: isolate upload policy error typing * style: sort policy preflight imports * refactor: centralize content policy boundaries	2026-08-21 22:43:32 -04:00
Danny Avila	8ae94afa91	🪡 fix: Thread Parent Message ID Through MCP Request-Scoped Bodies (#15095) ... * fix: Unify MCP request-scoped headers * fix: address request-scoped MCP review findings * test: preserve request scope on status errors * fix: treat authorized on-demand MCP servers as ready * refactor: separate MCP readiness from connection state * fix: preserve on-demand MCP readiness labels * test: satisfy OpenAI conversation ownership guard * fix: keep MCP action predicates boolean * fix: close deferred MCP request context gaps * fix: preserve on-demand MCP configuration actions * fix: fail closed on unavailable MCP parent context * test: complete MCP connecting-state mocks * fix: preserve missing MCP parent on continuations * fix: align native MCP request identities * fix: preserve edited MCP parent identity * test: use scoped Agent initializer fixture * test: expose MCP request body helper * fix: preserve MCP turn identity across resume * style: sort stream metadata imports * fix: carry normalized MCP identity to execution	2026-08-21 16:33:01 -04:00
Danny Avila	8c14f03432	🗂️ feat: Scope Scheduled Chats to Chat Projects (#15056) ... * 🗂️ feat: Scope Scheduled Chats to Chat Projects Adds an optional chat-project destination to a schedule, plus the operator config to require one — or to pin every scheduled run to a specific project. Feature: - `chatProjectId` on the schedule row, accepted on create/update, projected on the wire, and carried into the run's conversation through the durable trigger envelope's `run` context. - `interface.schedules.requireProject` refuses schedules that are not filed under a project; `interface.schedules.projectId` pins every run to one project and implies the requirement. - Dialog gains a project picker (required when configured, a read-only row when pinned); the card shows the destination and the new disabled reasons. Invariants: - ONE resolver (`resolveScheduleProjectId`) decides the destination for the write handler, the fire path, and the wire projection alike, and an operator pin OUTRANKS the stored id in all three. Tightening the config therefore redirects — or stops — existing schedules instead of grandfathering where their runs land. A pin implies `requireProject` for the same reason: without it, a row created before the pin would keep firing with no project at all. - Create/fire precheck symmetry, mirroring `resolveAgentFireAccess`: a write this handler accepts is one the next fire also accepts. Any edit leaving a schedule ENABLED re-validates its EFFECTIVE (possibly stored) project, like the existing stored-agent and cadence-floor rechecks. A DISABLING edit skips the requirement, or a schedule auto-disabled for `project_required` could never be turned off. - Fire-time enforcement auto-disables rather than filing runs loose, matching agent_deleted: new `project_required` (requirement raised after creation) and `project_deleted` (gone, or pinned to a project this owner does not have) reasons, both refused BEFORE a billed generation is dispatched, and both advancing so a schedule can never wedge on the occurrence. - `computeCreateDigest` appends the field only when present, so a payload without a project digests byte-identically to one from before this change — a create retried across the upgrade still matches its own row instead of reading as key reuse. - Project reads are scoped to the owner, so ownership and existence are the same lookup; a read ERROR propagates instead of failing closed, so a Mongo blip retries the fire rather than auto-disabling the schedule. The trigger idempotency key hashes principal/event/target and never `envelope.run`, so the added run field cannot destabilize delivery identity. * 🗜️ fix: Keep the Schedule Dialog Inside Its Height Budget The project picker landed as a new ROW in the schedule dialog, which broke the e2e edit spec: `md:overflow-visible` turns off the template's scrolling from `md` up, so the dialog's content must fit the viewport. The extra row pushed the footer's Save button below a 720x1280 window, where Playwright reported a visible, enabled button it could never click — 226 scroll-into-view retries and a 2-minute timeout, on all three attempts. Measured against `dev` at 1280x720 (Save button's bottom edge, viewport 720): dev 688 (32px slack) project row ~790 (off-screen, CI failure) 3-column row 704 (16px slack — half the budget spent) this commit 686 (34px slack, 2px better than dev) The identity row is now three columns — name, agent, project — and its caption moved out of the agent cell to sit full width beneath the row: at a third of the dialog that sentence wraps an extra line, and the row is the tallest thing competing for the budget. The caption is grouped with the row rather than left to the form's own 4-unit rhythm, which spent more height on the gap than the caption occupies. The e2e spec now asserts the button is in the viewport before clicking it, so the next field that overflows this dialog says so in one line instead of a two-minute timeout on a visible element. FOLLOWUPS.md records what the planned dialog controls (multi-day weekly, timezone, attachments) need first: give `ControlCombobox` the `portalElement` prop `Dropdown` already has, portal the popovers into the dialog content, and let the form scroll again. * 🧹 fix: Address Codex Review on Scheduled Chat Project Scope Four P2 findings, all real. Unreachable clearing path (ScheduleDialog). The picker only held live projects, so `com_ui_schedule_project_none` was a PLACEHOLDER — nothing selectable. Once a schedule had a project the owner could never take it away, leaving the server's `chatProjectId: null` path reachable only by API. The picker now carries a real "No project" option whenever a project is optional, and omits it when one is required, where there is nothing valid to select. Placeholder shown for a real project (ScheduleDialog). A stored or pinned project outside the first loaded page had no name in the paged map, and the combobox renders its placeholder for an empty display value — telling the owner a scoped schedule had no project. That one project is now read by id, with the raw id as a last resort: a poor label, but an honest one. Project policy skipped at the resume boundary (service.ts). `claimScheduleResume` re-applied the schedules gate, the revision fence, the kill switch and SCHEDULES:USE, but not the project policy this PR added. Approving a paused run whose project was deleted — or whose owner now sits under a requirement or a pin it no longer satisfies — billed a continuation the very next scheduled fire would refuse and auto-disable the schedule for. The effective-project resolution now runs there too, refused before the lease and the capacity slot so a policy refusal costs nothing and leaves no state to unwind. NOTE: agent access and balance are still not rechecked on resume; that gap predates this PR and is left alone. Per-card project derivation (ScheduleCard). Every card ran the projects hook and rebuilt the full option array, name map, and one icon element per project, to use a single name — O(schedules x projects) per render and per project-list refresh. The hook is split: `useChatProjectNames` (map only, for the panel, which resolves every card's name once and passes it down) and `useChatProjectPicker` (options and pagination, for the dialog's one combobox). The panel skips the query entirely until some schedule actually has a scope. Tests: four at the resume boundary (verified failing without the gate) and four in the dialog spec. The picker selection in one existing test now goes through the search field — the popover's VIRTUALIZED renderer sizes its window from a scroll height jsdom always reports as 0, so with three options it materialized only two. Full schedules e2e re-run green against the rebuilt client. * 🎯 fix: Settle Project-Policy Refusals and Keep Create Retries Idempotent Second Codex round, four P2s. Three were consequences of the resume gate added in the previous commit, which was half-built: it admitted where it should not and stranded the run where it refused. Project policy moves from `claimScheduleResume` into `isScheduleLive`'s `policy` branch. Both entry points consult that branch FIRST, and both already route its refusal through abort-and-settle — so a policy stop now settles the occurrence instead of answering a bare 409 while the job stays `requires_action`, the card keeps reading "Needs approval", and every retry repeats the same 409 until expiry. No change to resume.js: the existing branch does the work. The rule is deliberately NARROW. It refuses only where no valid destination is left — the requirement is on with nothing satisfying it, or the schedule's own project is gone (which also unset it on the conversation). It does NOT refuse because an operator's pin moved: the paused conversation cannot be rebound (`chatProjectId` is excluded from the resume context and the continuation reuses the same conversationId), so refusing would strand a pending approval over a destination it can never reach, for a pin that governs only where the NEXT run lands — which the fire path already redirects. Create retries are idempotent again. Project policy had been applied BEFORE the `clientRequestId` replay lookup, so a raised requirement, a deleted project, or a moved pin could answer 400 for a create that already committed — pushing the client to rotate its key and create a DUPLICATE schedule, the exact failure the key exists to prevent. Policy now applies only to a genuinely new insert, and the digest is computed from the CLIENT's payload rather than the resolved destination, so today's policy can no longer re-digest a genuine retry into a mismatch. An explicit `chatProjectId: null` under a pin is refused rather than silently resolved to the pin. The payload contract defines `null` as clearing the scope; answering 201 while filing under the pin reported success for the opposite of what was asked. Only an OMITTED field takes the pin silently. Tests: five on the policy branch (both refusals verified failing without it, plus guards that a moved pin and a live project still admit) and two on the handlers (the pinned explicit clear, and a committed create recovered by retry after the policy tightened — verified answering 400 without the reordering). * 🧭 fix: Converge the Stored Project on the Destination a Fire Resolved Third Codex round, four P2s. The root confusion behind the resume findings: an operator pin outranks the stored id at fire time, `fireSchedule` sends the pin in the trigger envelope, and the row keeps its old value. The row therefore LIED about where that occurrence's conversation went, and every later re-validation — the resume boundary above all — checked a project the conversation was never filed under. A schedule storing A, pinned to B, with B later deleted and A still live, was admitted for resume into a conversation that had just been unscoped. Fixed at the source rather than at each reader: a fire that resolves a destination different from the stored one writes it back, claim-token fenced like every other worker-side write and deliberately WITHOUT a configRevision bump — this is the server reconciling itself to policy, not an owner edit, and a bump would fence an in-flight occurrence off its own run. Written only AFTER the destination validates, so an unusable pin never lands in the row, and best-effort: the envelope already carries the right destination, so a failed write costs accuracy on a later recheck, never the run itself. The wire projection already reported the pin, so this also stops the row and the UI disagreeing. An explicit `chatProjectId: null` under a pin is now refused on the DISABLING edit path too. The pin check and the requirement are independent rules, and folding them together let `{enabled: false, chatProjectId: null}` skip the pin check entirely, unset the row, and answer with a wire projection still naming the pin. Only the requirement is waived for a disabling edit. The dialog no longer requires a project for an edit that leaves a schedule DISABLED. The server waives the requirement there precisely so a row auto-disabled for `project_required` can still be renamed or tidied up; requiring it in the form made that unreachable, and an owner with no projects could not edit the stopped schedule at all. FOLLOWUPS.md records the two residual gaps with their exact triggers: the sub-second deletion race inside the resume claim window (which needs the effective project persisted per OCCURRENCE plus a distinct policy conflict routed through abort-and-settle), and the fact that convergence happens only when a schedule fires. Tests: four on convergence (pin written, no write when unchanged, no write for a destination that failed validation, fire survives a failed write), two on the disabling-edit pin rules, two in the dialog. Schedules e2e re-run green. * 🔑 fix: Keep an Explicit Project Clear Out of an Omitted Field's Digest `computeCreateDigest` appended `chatProjectId` on `!= null`, so an OMITTED field and an explicit `null` produced the same digest. Because the replay lookup and `matchesCreateIntent` deliberately run before project policy, a request could reuse a pinned create's `clientRequestId` while explicitly sending `chatProjectId: null` and receive 201 describing the pinned row — success reported for the opposite of what it asked, and the pinned-clear refusal the normal create path applies never reached. Now `!== undefined`: an omitted field still digests byte-identically to a payload from before project scope existed, so a create in flight across the upgrade still matches its own row, while an explicit clear is a distinct intent and digests differently. A pre-scope client never sent the field at all, so nothing legacy can carry an explicit null. * 📍 fix: Validate a Paused Run Against the Project Its Own Occurrence Used The schedule-wide convergence from the previous commit was not enough, and the reason is the single-active run index: it covers `status: 'started'` only, so a PAUSED run does not block the next occurrence. While run 1 sat paused in project A, a pin move plus one later fire rewrote the schedule row to B — and the resume policy then validated B while run 1's conversation was still filed under A. Delete A and the continuation was admitted into a conversation that had just been unscoped. That window lasts as long as the pause, not the sub-second race the previous commit documented. The reservation now records the destination THIS occurrence used, and `isScheduleLive` validates that record when given the occurrence's `scheduledFor` — which `resume.js` already reads two lines above the call. No new conflict type and no settlement-path surgery: the refusal rides the abort-and-settle branch that check already has. An ABSENT record falls back to the schedule-level resolution rather than reading as unscoped. A pre-scope occurrence, or one whose row is gone, must never be treated as evidence to stop a run — the fallback keeps legacy paused runs behaving exactly as they do today. Schedule-wide convergence stays: it keeps the row honest for the UI and for every check that has no occurrence in hand. FOLLOWUPS.md now describes the one remaining gap accurately — a deletion inside the claim window, which needs a distinct policy conflict routed through abort-and-settle rather than the bare 409 an `inactive` conflict produces. Tests: three on occurrence-vs-row precedence (the decisive one verified failing without the lookup), two on what the reservation records, and the resume controller spec now pins `scheduledFor` in the policy call. Schedules e2e re-run green. * 🏷️ fix: Tell a Deliberately Unscoped Occurrence From an Unrecorded One The occurrence fallback added in the previous commit conflated two different absences. A post-upgrade run that deliberately went unscoped omitted the field exactly like a row written before the field existed, so both took the fallback — and a paused unscoped run was then validated against the schedule's CURRENT project. Under a requirement or a pin added while it sat paused, that admitted a billed continuation into a conversation satisfying no present policy. The reservation now ALWAYS records its decision, `null` for unscoped, and the read reports `recorded` from key PRESENCE rather than truthiness. Only an unknown record — a pre-scope row, or no row at all — falls back to the schedule-level resolution; a recorded null is the genuinely unscoped occurrence it says it is, and is refused once a project becomes required. The distinction rests entirely on a stored `null` surviving as a present key while a never-written field stays absent, so that is asserted against real Mongo rather than assumed: if it ever stopped holding, unscoped runs would silently start being validated against the schedule's current project again. Tests: two against mongodb-memory-server (recorded null vs never-written vs missing row), plus refusal of a recorded-unscoped occurrence under a new requirement and the preserved fallback for a pre-scope one. Schedules e2e green. * 🔁 fix: Validate an Initial Scheduled Start Against Its Own Occurrence The initial-start policy check in `request.js` called `isScheduleLive(..., { policy: true })` without `scheduledFor`, so it fell back to the schedule-level resolution even though the run row already carries the occurrence's recorded scope. An occurrence reserved unscoped, with a pin introduced while its loopback request sat queued, was therefore admitted against the new pin — producing a billed unscoped conversation under a requirement it does not satisfy, from an envelope already built without a project. `scheduledFor` was already in scope there. Passing it makes the initial start and the resume validate the same way: against the destination the occurrence itself recorded.	2026-08-21 11:09:04 -04:00
Danny Avila	c7e355b219	🛑 fix: Confirm Scheduled Stops and Separate Terminal Scheduler Failure from Startup (#15051) ... * 🛑 fix: Confirm Scheduled Stops and Separate Terminal Scheduler Failure from Startup Fixes #15042, fixes #15043. `resume.js` inferred a confirmed stop from the ABSENCE of `failureReason`, but `abortJob` had four `success: false` paths that returned no reason at all. Those settled the occurrence as `interrupted` and pruned the checkpoint on aborts that never landed — including one where a REPLACEMENT generation owned the conversation, which pruned the successor's checkpoint. Every `success: false` return now names itself (`job_not_found`, `already_settled` added alongside the existing `generation_replaced` / `job_still_active`), and a single canonical `isStopConfirmed` predicate decides whether durable state may be settled. `already_settled` confirms a stop — `awaitProviderDrain` has proven the provider segment can no longer persist — so a permanently terminal generation is not answered with a retry loop. Separately, a schedule engine that failed to arm advertised its permanent outage as a transient 503 with `Retry-After`, so a client obeying it would poll forever. Readiness is now tri-state (`starting` / `armed` / `unavailable`): the retry contract applies only while arming is genuinely pending, and a failed arm returns a terminal `SCHEDULES_UNAVAILABLE` with no `Retry-After` and an error-level log. * 🏷️ fix: Declare Schedule Write Gate Return Types `--isolatedDeclarations` requires an explicit return type on the exported factory and on the middleware it returns (TS9007). Adds a named `ScheduleWriteGate` type matching the existing `ShareMiddleware` shape.	2026-08-20 18:33:51 -04:00
Danny Avila	c5276fc63d	⏱️ feat: Run Scheduled Chats Through Durable Agent Triggers (#14939) ... * feat: Scheduled Chats — agent-centric scheduled runs creating real conversations Squash of the full review-hardened branch (PR 14540, supersedes 14373) onto latest dev, preserving the exact verified tree. History prior to this commit lived on the pre-squash branch; every invariant below survived 25 Codex review rounds plus two external audit rounds (R26) with regression tests that fail without their fixes. Feature: - Schedules CRUD + side-panel UI (cadence dialog, run cards, Run Now), roles/ permissions (SCHEDULES:USE), interface.schedules availability, per-user limits and capacity slots, timezone-aware cadence with DST-conservative floors and misfire grace. - Engine: single-process claim/fire loop with leases, loopback POST dispatch (signed schedule-fire JWT claims, per-occurrence idempotency key inside the route's clientRequestId charset), overlap/balance/capacity/duplicate skip policies with auto-disable streaks (too_many_failures, insufficient_balance), reconciliation from retained terminal-job evidence, erasure sweep. - Scheduled runs create real conversations through the resumable agents chat path: HITL pauses surface on the card (requires_action), resumes re-apply the fire boundary's admission policy (revision fence, enabled, global kill switch, SCHEDULES:USE, availability) before continuing a billed generation. Correctness invariants (the audit surface): - Settlement discipline: every persistence-producing write happens-before a run's terminal outcome write; Stop/complete/pause race through single-winner terminal CAS claims (dev's TerminalJobClaim substrate) with retained, completedAt-less evidence for scheduled fires plus an owner-intended outcome stamp (scheduleOutcome) the reconciler prefers over re-derived success — round-tripped through the Redis hash mapper. - Swallowed generation failures (client error content parts) classify to error/skipped_balance instead of success on both initial and resumed paths; stale stamps are refreshed evidence-first when persistence plus the Mongo outcome write both fail. - Abort honesty: delivery judged by generation ownership and the CAS's actual from-status; republication escalates to the transport's acknowledged variant; the Stop route settles only genuinely paused runs. - Account deletion: one-way barrier (deletionRequestedAt) with auth-cache tombstone-before-stamp, boundary rechecks across all auth strategies, quiesce of scheduled + interactive work with durable per-stream abort fences (positive-evidence acknowledgement only), owner-side finalization markers for post-terminal billed writes, deferred-deletion sweep (explicitly ensured partial index) that completes cascades autonomously. Remote OpenAI-compatible/ Responses requests are documented as outside the quiesce and tracked in issue 14594. - Store compatibility: finalization markers optional on the legacy IJobStore contract with coherent degradation (registration fails -> synchronous title fallback; count reads 0). * fix: fence terminal response persistence from deletion; retain stale-pause evidence Two blockers from the third external review round. Terminal persistence visible to account deletion: - The finalization marker was registered only when post-terminal TITLE work was possible, but every persistence-owning terminal CAS opens the same window: the claim drops the job out of the active set BEFORE the response save (and the background user-message/convo saves), so a deletion quiesce landing there saw neither an active job nor a marker and could cascade while the admitted request could still recreate messages. Both controllers now register the marker before every persistence-owning claim — the fresh-turn path and the HITL resume — and release it only after their pending saves (and any post-terminal title) have landed, on success and failure paths alike. The TTL bounds a crash. - settleAbortFence no longer clears a complete/error fence while `terminalPersistencePending` is set: terminal at the CAS is not settled while the owner is still persisting. The job facade now surfaces the flag. - The marker trio is REQUIRED by the runtime store contract (assertJobStoreV2 refuses a store without it at configure time, keeping the failure loud and deterministic) while remaining optional on the legacy public IJobStore type for source compatibility. The silent degrade path from the previous round is gone — it was not deletion-safe. Stale-pause recovery retains scheduled evidence: - Three crash/timeout recovery paths — ApprovalLifecycle.failStalePausePersistence and the InMemory/Redis stale-pause cleanups — unconditionally stamped `completedAt`, putting a scheduled fire's failed-pause error terminal on the short completed TTL. A Mongo outage longer than that TTL erased the evidence and the reconciler recovered the run as `interrupted` instead of `error`. All three now follow the controller-observed path from the previous round: scheduled jobs omit `completedAt` (retained-evidence TTL) and stamp the error outcome. The PR description now explicitly narrows the deletion guarantee for the remote OpenAI-compatible/Responses paths (tracked in issue 14594). * fix: deletion-fence marker protocol — generation-scoped, atomic, fail-closed, all terminal paths One consolidated pass over the finalization-marker mechanism, per the fourth external review round. The invariant it establishes: NO persistence-owning terminal CAS runs without a durable, generation-qualified marker covering the window it opens, and every consumer treats a pending terminal as unsettled. - Generation-scoped markers. Entries were keyed (userId, streamId), so a Stop-superseded generation finishing late could clear the marker its replacement registered on the same conversation. Marker fields are now qualified by the generation's createdAt; clears must present the same identity, and an unqualified legacy clear cannot drop a qualified entry. - Atomic Redis registration. HSET-then-EXPIRE loses the fresh marker when the user's existing hash expires between the two commands (or the process dies there); registration is now a single Lua script carrying both. - Fail closed everywhere. Registration failure (after one retry) now REFUSES the terminal CAS instead of proceeding uncovered: the completion claim throws into the error path, the error path skips completeJob and leaves the job ACTIVE — deletion-visible by itself, recovered by the stale-running reaper — and abortJob returns a new retryable `fence_unavailable` failure the Stop route answers with 503 and the deletion quiesce treats as an unacknowledged stop (fence kept). The previous round's log-and-proceed is gone. - Every terminal path enrolled. abortJob now owns its window (register before the abort CAS, clear in its finally — the Stop route's checkpoint prune and partial save run inside beforePublish, between CAS and publication); the interactive and background generation-error paths register before their completeJob; the resume controller's error finalization registers before its completeJob. A lost or thrown claim releases the marker after pending saves flush instead of holding the user's deletion behind the TTL. - Every pending terminal unsettled. settleAbortFence defers on terminalPersistencePending for ALL statuses — including `aborted`, whose route-side persistence the previous guard missed. Also: a direct Redis regression for scheduled stale-pause retention (the P2 test gap), and the PR description no longer claims to carry every commit. * fix: lease-token lifecycle fences — same-generation isolation, admission fence, undelivered-Stop retention, legacy abort enrollment Fifth external review round; four P1s, handled as the requested consolidated lifecycle-fence pass. - Lease tokens. Marker fields were (streamId, createdAt), shared by every contender on the same generation — completion and Stop, or two racing Stops — so a losing contender's cleanup erased the winner's still-live marker. Every registrant now carries a unique lease token in the field and may only ever clear its own lease; unqualified legacy clears cannot touch qualified entries. - Admission fence. Authentication can pass before the deletion barrier goes up, and the durable createJob is several async steps later — a deletion quiesce in that window saw neither an active job nor a marker and could cascade before the admitted request created its job. The controller now registers an admission lease and THEN rereads the deletion barrier: the ordering guarantees either this request observes the barrier (403, lease released, slot/claim cleanup) or the quiesce observes the lease and defers. Held until createJob is durable; released on every refusal and initialization-error path. Fail closed when the lease itself cannot be registered (503 retryable). - Undelivered-Stop retention. abortJob released its lease in a finally even when delivery AND publication had provably failed — the job reads terminal (invisible to active-set scans), a user Stop writes no durable abort fence, and the remote owner keeps generating and will persist its abort-catch writes whenever the signal finally lands. The lease is now retained in exactly that case, and each resignal attempt heartbeats a fresh lease so the fence outlives the TTL for as long as delivery is still being driven. The abort-winning turn's own loser-side pending saves are additionally fenced in the controller catch (best-effort — those writes are already in flight). - Legacy abort enrollment. abortMiddleware (assistants abort route fallback for non-assistants endpoints) awaited abortJob and then spent usage and saved the stopped response AFTER the abort's lease was released. Both writes now run inside `beforePublish`, between the abort CAS and publication, covered by the same lease as every other abort. Barrier tests, each verified to fail without its fix: same-generation lease isolation (store), racing two-Stop loser cleanup (manager, stale-read forced CAS race), undelivered-Stop lease retention, resignal heartbeat, admission refusal with lease-before-reread ordering plus release-on-durable-create, and legacy-abort persistence inside beforePublish. * fix: heartbeat-backed owner-lifecycle leases close the settlement handoff races Sixth external review round: the remaining P1 interleavings were one structural problem — lease handoffs that were not atomic — resolved as the requested consolidated lifecycle-lease pass. - Quiesce reads leases BEFORE the active-job scan. The admission-lease -> durable -job handoff is only atomic against a reader in the OPPOSITE order of the writer: writers hold the lease strictly until the job is active-set visible, so leases-first shows every interleaving either the lease or the job. Jobs-first allowed a request to create its job after the scan and release its lease before the count — hiding both, cascading, and letting the new generation persist into a deleted account. - The abort acknowledgement is fenced by the owner-lifecycle lease. Redis ACKed the moment the owner's AbortController tripped; the stopping side released its lease on that ACK while the owner's asynchronous abort-catch persistence was still ahead. The transport now awaits a manager-installed pre-ACK hook that registers a DETERMINISTIC owner lease (exactly one owner exists per generation, and determinism is what lets the signal-time registrant and the owner's catch-side release agree across processes) before the acknowledgement is persisted or published; an owned same-replica abort bridges to the same lease before tripping its local controller. Both generation-owner catches (fresh turn, resume) release it once their writes land. - A failed replacement handoff no longer orphans the predecessor. The atomic replacement removes it from active storage, and an unconfirmed handoff terminalizes the replacement too — leaving nothing a quiesce could discover while the predecessor's provider may still be generating. Its owner lease is now retained at the point the receipt fails delivery; the owner replica renews it through the pre-ACK fence when the signal finally lands. - Leases HEARTBEAT while held. The five-minute store TTL only bounds a crashed holder; live persistence — a stalled save, a long deferred title — must never outlive its own fence. holdUserFinalization registers and renews every minute until released; the controllers' completion/error/admission leases all hold. The undelivered-Stop retention moved to a deterministic `stop` lease that every resignal attempt renews and the first successful one clears (no more opaque leases accumulating to TTL), and a THROWN abort transition releases the contender lease instead of leaking it. - The user-document abort-fence mutations now invalidate the auth user-doc cache, matching every other user-doc write. Barrier tests, each verified to fail without its fix: quiesce lease-scan ordering (plus the observed-lease defer), pre-ACK fence ordering at the transport, owned-abort owner-lease bridging, replacement-handoff predecessor retention, held-lease heartbeat past the TTL, and failed-then-successful resignal reaping the retained stop lease. * fix: one manager-owned owner-lease span across every abort delivery path Seventh external review round; four lifecycle-fence gaps, closed by making the owner-lifecycle lease a single manager-owned, heartbeat-held span. - Fail-closed acknowledgements. The pre-ACK hook registered a one-shot lease and the transport ACKed even when it failed; a same-replica owned abort likewise swallowed registration failure. The hook now acquires a HELD owner lease (heartbeat until the owner's catch releases it via releaseOwnerLease) and a rejection SUPPRESSES the acknowledgement — the stopping side stays retryable behind its retention lease, and every resignal re-drives the handler. The hook also stopped gating on `job.createdAt === generationId`: during a replacement handoff the store holds the replacement while the abort targets the predecessor, and that gate silently skipped exactly the generation being acknowledged (the store job is owner identity, never a generation gate). A local owned abort acquires the same held lease before tripping its provider; post-CAS the trip cannot be withheld, so acquisition failure downgrades delivery and the retention handoff keeps the user fenced. - Committed-but-lost-reply disambiguation. A thrown abort transition released the contender lease as if nothing had happened, but a Lua CAS can commit and lose its reply — an aborted job invisible to active-set scans whose provider was never signalled, with no fence left. The throw path now re-reads the exact generation: only a job still live under the caller's identity proves no commit; committed or ambiguous outcomes hand the fence to the deterministic stop lease (kept on the contender lease if even that fails) before rethrowing. - Replacement handoff covered end to end. A LOCAL replacement abort acquires the predecessor's held owner lease before the trip (failure reports the receipt undelivered, engaging retention). Failed-handoff retention is no longer a swallowed one-shot: it heartbeats with the durable acknowledgement proof as its renewal predicate — acquisition failures keep retrying for as long as the fence is needed, and the retainer stands down (without clearing the shared field) once the remote owner ACKs and thereby holds its own lease. - A LOCAL resignal delivery hands off to the owner lease BEFORE clearing the retained stop lease, and keeps the stop lease when that handoff fails. Barrier tests: hook rejection suppressing the ACK, commit-then-lost-reply retention with its provably-uncommitted counterpart, local-resignal owner handoff ordering, pre-ACK owner lease held past the store TTL until release (and provably stopped after), and failed-handoff retention retrying on its heartbeat — verified fail-before/pass-after by stashing the fixes. * fix: finish scheduled chat lifecycle hardening * fix: close scheduled chat review follow-ups * fix: generation-fence abort recovery evidence * test: wait for settled approval tool output * test: preserve scheduled init reconciliation option * refactor: rebuild scheduled chats on durable agent triggers * test: reset MCP cache mock between cases * test: isolate scheduler startup in server specs * fix: harden scheduled run lifecycle * fix: normalize schedule capacity conflicts * test: type schedule collision fixture * fix: re-fence scheduled resume and expiry * fix: fence scheduled resume handoffs * fix: release superseded manual schedule leases * fix: release failed run-now claims * fix: release superseded engine claims * fix: repair schedule dialog interaction and rework its form The agent picker was unusable: ControlCombobox portals its popover to the body by default, which lands it outside the dialog's Radix focus trap. Clicks passed through it, it could not be tabbed into, and the trap fighting Ariakit for focus locked the page up on selection. The prop is documented for exactly this case — pass `portal={false}` and give the dialog `overflow-visible`, as ProjectButton already does. The time and day dropdowns defaulted the same way. Alongside that: - Extract the agent builder's instructions editor (special-variable menu plus expand-to-fullscreen) into a controlled `VariableEditor` and use it for the schedule prompt. Insertions now route through `onChange`, so react-hook-form sees them — the schedule PATCH is built from `dirtyFields`, and a `setValue` that skipped dirty tracking would have dropped an inserted variable silently. - Replace the hand-rolled frequency buttons with the shared `Radio`. They marked the selection with `bg-surface-hover` on an outline button whose hover is the same token, so the selected option was indistinguishable from a hovered one; `Radio` is also a real radiogroup rather than four `aria-pressed` toggles. - Wrap the fields in a real `<form>` and associate the footer button by id, so Enter submits. Group the frequency, day and time controls in fieldsets. - Add placeholders for name and prompt, match the textarea fill to the other fields, and label the hourly case as minutes past the hour. - Widen the dialog to `md:max-w-3xl` and pair name with agent so the form fits without scrolling on desktop. - Move scheduled chats below skills and above prompts in the side nav. The new dialog spec fails when the portal fix is reverted. * test: cover scheduled and subagent deletion drains * refactor: own the form-control appearance in the client primitives Addresses the codex finding on ScheduleDialog: a feature-local `FIELD_CLASS` restated the `Input` primitive's border, radius, height and background so it could be pasted onto the schedule dropdowns, leaving those controls with no connection to the primitive they were imitating. Move that appearance into `packages/client/src/components/Field.ts` as the single source `Input` and `Textarea` now compose, and give `Dropdown` and `ControlCombobox` a `variant="field"` that applies it. The schedule dialog passes the variant and carries no class strings of its own. This also repairs a break the dev merge would otherwise have introduced: the newer `Dropdown` splits `className` (wrapper) from `triggerClassName`, so the old pasted classes would have landed on the wrapper and left the triggers unstyled. The semantic-token guard now watches the shared module and asserts each primitive still composes it, which covers more than the two files it read before. * fix: keep an explicit schedules disable from becoming an opt-in `use` is two things at once for a dual-purpose runtime interface field: a permission bit, which DB overrides strip, and the runtime disable signal that `getLimits` reads. Stripping it from `{ use: false, maxPerUser: 2 }` leaves an object, and `getLimits` treats any object without `use: false` as enabled — so an admin override written to stop scheduled billing for a role or user started it instead. Collapse an explicit disable to the boolean form before the strip, on both paths that accept it: the `interface.schedules` field patch, which admitted the object wholesale because bare runtime paths deliberately bypass the permission gate, and the overrides merge, which reached the composite-field branch and kept `maxPerUser`. Objects that only narrow limits are untouched, so a principal can still be given a smaller cap. Both regressions fail without the normalizer. * fix(schedules): Wave A — null-balance CAS, atomic paused-card clear, clustered erasure sweep Slice 1 (thread r3804518381): route the existing-null balance initialization through a { user, tokenCredits: null } compare-and-set instead of a blind $set, so a concurrent initializer/charge landing between the preflight read and the write is never handed back its spent starting balance. On a CAS miss the preflight re-reads the winner. The absent-record $setOnInsert path and the credited-record refill-config sync are unchanged. Adds the initializeNullBalance adapter (no upsert) and regression coverage for winner/miss/sync cases. Slice 2 (thread r3804518388): updateScheduleById now drops a `requires_action` lastRun projection atomically with the configRevision bump. Any pause present at edit time was projected under the pre-edit revision and can never be replaced by its own revision-fenced terminal outcome — a disabling edit would strand the card on "Needs approval" forever. Implemented as classic-operator CAS branches (DocumentDB rules out a conditional pipeline $unset), fenced on the card STILL being the pause so a terminal outcome or newer occurrence that races in is preserved. Terminal history survives untouched. Slice 4 (thread r3803826204): expose initializeScheduleErasureSweep from the schedule runtime facade and start it in every clustered (experimental) worker after Mongo is up. It re-drives eraseScheduleIfDrained for soft-deleted rows so a hidden prompt cannot outlive its drain when the delete/erase-on-settle attempts miss. It arms nothing else and never infers owner death from a process-local missing job (isTopologySafeToArm gates that). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CZwZPDBgxy4mCWWg6zQSjq * fix(schedules): Wave B.1 — reversible account-deletion schedule suspension Thread r3804518383. Account-deletion quiesce marked every schedule `deleting`, disabled it, and cleared nextRunAt destructively. When a later cascade step (or the drain itself) failed, the controller cancelled the user-deletion fence — restoring the user — but the schedules stayed `deleting` and were erased by the sweep, silently losing all of a live user's scheduled prompts. Replace the destructive marking with a REVERSIBLE, token-fenced suspension: - suspendUserSchedulesForDeletion(userId, token) snapshots each schedule's prior enabled/nextRunAt under a per-attempt token, then fences firing (disable, clear nextRunAt, rotate claimToken). It never sets `deleting`, so a suspended row is not erasure-eligible. Snapshotting reads then bulkWrites (a classic update cannot copy field values under DocumentDB), fenced per row so an already- suspended/soft-deleted/edited row is left alone; idempotent per token. - restoreUserSchedulesFromDeletion(userId, token) reverses it, re-enabling and re-arming only rows still carrying the exact attempt token and not independently deleted — so an owner-deleted or newer-attempt-suspended schedule is never resurrected. - deleteUserController generates the attempt token, passes it to quiesce, and on any failure that cancels the user-deletion fence restores the suspended rows. A successful deletion hard-deletes them (and their snapshots) via the existing cascade and never restores. Adds a `deletionSuspension` embedded field (excluded from the wire schedule), data-method regression tests (suspend/restore/fence/idempotency/no-resurrect), and controller tests (restore on drain-false and post-quiesce cascade failure, no restore on success). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CZwZPDBgxy4mCWWg6zQSjq * fix(schedules): Wave B.2 — wire the interactive Stop persistence protocol Thread r3804255932. The durable Stop primitives (requestRunAbort 'stop', getScheduleRunAbortState, markRunAbortPersisted) already existed but production only used requestRunAbort(..., 'deletion'). An interactive Stop flipped the job to `aborted` and then persisted its partial message + checkpoint inside `beforePublish`, without ever stamping the schedule Stop or acknowledging it — so reconciliation, the generation owner, or a concurrent schedule/account deletion could terminalize the run and release its capacity (and erase data) mid-write. Wire the request -> persist -> acknowledge -> settle barrier through the schedule runtime (the route never touches raw Mongo): - Expose beginScheduledStop / acknowledgeScheduledStopPersistence on the service. - The abort route stamps the Stop BEFORE signalling abortJob (a serialized 'in_progress' loser returns 409 STOP_IN_PROGRESS without a second abort), acknowledges only after beforePublish persistence succeeds, and on a persistence failure leaves the barrier unresolved so the run stays preserved (client retries; stale-owner timeout is the bounded recovery). A failed abort releases the stamp it placed so a replacement/retry is never blocked through its predecessor. - recordScheduleOutcome (the owner settlement path) now waits, bounded, for the Stop acknowledgement before terminalizing; a resolved/non-stop/stale marker proceeds immediately. The paused Stop settles only after its own ack. Adds service-layer barrier tests and route-level ordering/persistence-failure/ in-progress tests; the data-layer serialization is already covered. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CZwZPDBgxy4mCWWg6zQSjq * fix(schedules): Wave C.1 — reconcile durable trigger delivery with the reservation Threads r3803826192 (manual limiter) and r3804255924 (PII/moderation), plus the independently-found long-Retry-After race. fireSchedule reserves a `started` run and a global capacity slot BEFORE the durable trigger delivery reaches the chat route, where an interactive limiter (manual Run Now), PII, or moderation can reject it before any generation job exists — dead-lettering the delivery while the run sat `started` until the 30-minute orphan sweep mislabeled it interrupted. And a valid delivery deferred by Retry-After (up to 24h) could be orphan-settled and have its capacity released, then fire anyway. Translate durable delivery state into the schedule outcome: - Store the deterministic trigger deliveryKey on the ScheduleRun reservation (computed from the envelope BEFORE enqueue, so an ambiguous commit still has it). - Add a getTriggerDelivery engine dep (wired to the merged trigger service's getDelivery) that reads the durable delivery by key. - Schedule reconciliation, for a jobless `started` run: staging/pending/leased → admission is live, never orphan; dead → record `error` from the durable lastError and release capacity promptly (no 30-minute wait), through the ordinary outcome/auto-disable path; succeeded or no record → the existing legacy orphan policy (interrupted only past the cutoff); a delivery lookup failure defers rather than orphaning a possibly-live delivery. Limiter/PII/ moderation middleware writes no schedule state. Adds reconcile state-mapping tests (dead/pending/leased/staging/succeeded/none/ lookup-failure) and a fire test that the reservation's deliveryKey equals the enqueued delivery's idempotency key. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CZwZPDBgxy4mCWWg6zQSjq * fix(stream): Wave C.2 — durable retry/ack for terminal host lifecycle actions Thread r3804518375. Approval expiry won the `requires_action → aborted` CAS and then invoked the host hook best-effort: `runApprovalExpiredHandler` swallowed a failure, and because later sweeps enumerate only `requires_action` jobs, the now- aborted job was never offered again. In the clustered entrypoint (no schedule reconciler) the ScheduleRun stayed `requires_action` and its retained job persisted indefinitely. Make the host lifecycle work durable rather than schedule-specific: - Add a generic `terminalHostActionPending` marker, set ATOMICALLY in the same terminal transition (ApprovalLifecycle.expireWithIdentity), only when a host adapter is installed. - Retain and index such jobs: both stores keep them out of terminal reaping and expose getTerminalHostActionJobs(); Redis adds a set + extended (24h-bounded) TTL, in-memory a bounded 24h retention so a permanently-failing hook cannot leak. - The manager clears the marker only after the adapter acknowledges success, fenced by generation identity (clearTerminalHostAction), so a replacement generation can neither clear nor execute its predecessor's action. - cleanup()/expireStaleApprovals() enumerates unacknowledged terminal host actions across restarts and replicas and retries the idempotent hook; the relay only re-invokes while the marker is unacknowledged, so a successful ack prevents duplicate work. Store-won expiry marks it too, so a loser-replica relay still crosses the hook. - Terminal SSE notification continues regardless of host-hook outcome. Covers in-memory behavior (retry after failure, restart/other-replica retry, ack prevents duplicates, identity fence, terminal notification on failure, no marker accumulation for non-scheduled jobs) and updates the Redis cluster-membership contract test for the new index. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CZwZPDBgxy4mCWWg6zQSjq * style(schedules): satisfy import sorting in fire.ts and fire.spec.ts CI "Static checks" failed on IMPORT_SORT for the two files Wave C.1 added imports to (the AgentTriggerEnvelope type import and getAgentTriggerIdempotencyKey). Applied scripts/sort-imports.mts to exactly those files — imports-only reordering, no behavior change. Other files reported by a repo-wide check are pre-existing on dev and deliberately left untouched so this PR is not widened. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CZwZPDBgxy4mCWWg6zQSjq * fix(schedules): repair the deletion CLI and order restore before the fence release Addresses three findings from the fresh Codex review. P1 — config/delete-user.js called methods.disableUserSchedulesForDeletion, which Wave B removed in favor of suspendUserSchedulesForDeletion. The file is @ts-nocheck and its spec mocked the removed name, so neither typecheck nor tests caught it; the real CLI would throw a TypeError before deleting anything and then only unwind the fence. The CLI now uses the tokenized protocol: it mints a suspension token, suspends with it, and restores that exact attempt's rows in its finally block when the deletion does not commit. Its spec mocks the real methods, so the breakage can no longer hide. P2 — both the HTTP controller and the CLI released the user-deletion fence BEFORE restoring schedules. That fence is what refuses new schedule writes/claims, so the gap let an owner PATCH edit a still-suspended row and have its enabled/next-run state overwritten by the older snapshot, and let a second deletion attempt re-suspend under a new token — making the first restore a no-op and stranding the disabled snapshot permanently. Restore now runs first, while writes are still fenced. Hardening for the same defect class across a crash: suspendUserSchedulesForDeletion now ADOPTS an existing suspension's snapshot when re-suspending a row abandoned by an earlier attempt, instead of re-capturing the row's current (already-suspended) state. Without this, an attempt that died before restoring would have its successor snapshot "disabled, no next run" and permanently strand the schedule. Tests: CLI restore-before-fence ordering and no-restore-on-success; the same ordering assertion on both controller post-quiesce failure paths; a data-method regression that a second attempt adopts the abandoned snapshot and restores the original enabled/next-run state. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CZwZPDBgxy4mCWWg6zQSjq * fix(schedules): converge dead deliveries in topology-safe maintenance Codex finding: the `dead` delivery mapping added in Wave C.1 lives only inside startScheduleEngine's reconciler, but the clustered entrypoint arms no engine — it runs erasure-only maintenance. A delivery queued before a restart into clustered mode and then rejected before generation creation (interactive limiter, PII, moderation) dead-letters while its ScheduleRun stays `started`, holding a global capacity slot indefinitely for an ordinary non-deleting schedule. Add a dead-delivery convergence pass to the erasure sweep, so every topology that runs schedule maintenance settles it. The pass is POSITIVE-EVIDENCE-ONLY and is therefore safe where absence-based reconciliation is not: a `dead` delivery is durable shared state proving no generation owns the reservation. It settles only when the job is confirmed absent or identity-mismatched (an identity-matched job still owns the run), defers on an unknown job lookup, on an in-flight abort, and on an in-flight resume hand-off, ignores legacy reservations with no deliveryKey, and applies a short grace so an accepted delivery still creating its generation is never settled mid-handoff. Auto-disable policy is deliberately left to the armed engine; this path records the failure and frees the slot. Deliberately does NOT touch api/server/experimental.js — the clustered entrypoint already starts this sweep, so the convergence arrives through the existing initializer and the shared-file footprint stays as-is. Tests: settles a dead delivery as error under an explicitly UNSAFE topology, leaves live deliveries alone, never settles under an identity-matched running generation (delivery is not even consulted), defers an in-flight abort, and ignores a reservation with no deliveryKey. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CZwZPDBgxy4mCWWg6zQSjq * fix(stream): refresh host-action retention on each retry attempt Codex finding: unacknowledged terminal host-action evidence was capped at the 24h pause TTL, so a host dependency (Mongo) unreachable for longer than that let the Redis key — and its pending marker — expire with no generation-fenced acknowledgement, stranding the ScheduleRun where no reconciler is armed. Measure retention from the LAST retry rather than from the terminal transition: enumerating a pending host action IS the retry attempt, so both stores refresh its retention as they hand it to the hook (Redis re-EXPIREs the job key; the in-memory store stamps terminalHostActionRefreshedAt and bounds from it). Evidence therefore survives as long as some replica is still actively retrying, while a deployment that stops sweeping entirely still lets it age out — so this does not reintroduce the unbounded leak the cap existed to prevent. Test: after a failed hook, a later cleanup pass keeps the marker pending and moves its retention basis forward. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CZwZPDBgxy4mCWWg6zQSjq * fix(schedules): defer settlement when the Stop barrier times out Codex finding: waitForStopPersistence returned after its 5s poll budget even when the Stop was still fresh and unacknowledged, and recordScheduleOutcome then went straight on to terminalize the run — releasing its capacity, deletion, and erasure barriers while beforePublish may still have been writing. Slow checkpoint cleanup is indistinguishable from a dead route on that signal, so the timeout was being treated as if the barrier had been satisfied. The poll budget now means "undecided", not "clear". On timeout with a fresh, unacknowledged Stop the barrier DEFERS: recordScheduleOutcome returns false without recording, leaving the run active/preserved. Settlement then happens either when the route acknowledges, or once the existing stale-owner cutoff (ABORT_OWNER_PRESUMED_ALIVE_MS) authorizes a later attempt — which the loop already treats as clear-to-settle. Callers with durable retry (the approval-expiry host action, reconciliation) re-drive it, so a deferral converges rather than stranding the run. Test: a fresh Stop that never acknowledges within the budget reports not-settled and records no outcome. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CZwZPDBgxy4mCWWg6zQSjq * fix(schedules): require definite delivery failure, extract its message, converge deferred Stops Third Codex round. Three findings in code from this closeout, plus one pre-existing P1 that is a one-line operator-safety fix. P1 — dead-delivery settlement demanded too little evidence. `dead` does not prove a request was rejected: the trigger host marks response timeouts and invalid success responses `certainty: 'ambiguous'`, and the engine dead-letters those once retries are exhausted. The erasure sweep treated every dead letter as positive evidence, so an ambiguous one sitting over a generation a peer had accepted could terminalize the run and release its capacity mid-flight. It now settles only on a DEFINITE rejection, unless job absence is deployment-authoritative (safe topology), where the confirmed-absent job is itself the evidence. P1 — `lastError` is an `AgentTriggerDeliveryFailure` object, not a string. A duplicated local interface declared it `string` (against CLAUDE.md's no-duplicate- types rule), so both the sweep and the engine reconciler passed the object into the String-typed run/schedule `error` fields; Mongoose would reject the cast, the per-row catch would swallow it, and the run would keep its global capacity slot. The dep type now reuses the canonical `AgentTriggerDeliveryFailure` and both call sites pass `.message`. Re-typing immediately surfaced a stale test that had asserted a string. P1 (pre-existing) — the base-config global stop is honored in `getLimits` via `isRuntimeDisabled` rather than a literal `=== false`. The stop has two shapes, and deepMerge turns base `{ use: false }` plus a principal override of `true` into `{ use: true }`, so the literal check reported the feature enabled and Run Now dispatched straight through fireSchedule, bypassing the operator's emergency stop. Now the same predicate the engine gate already uses. P2 — a Stop whose settlement DEFERRED past the poll budget had no convergence path where no reconciler is armed. `acknowledgeScheduledStopPersistence` now optionally re-drives the terminal outcome once the barrier clears; `recordRunOutcome` is match-guarded and idempotent, so an owner that already settled makes it a no-op. The abort route passes it for a running generation; a paused job still settles explicitly. Tests: ambiguous dead letters refused under unsafe topology but settled when absence is authoritative, definite rejections settled either way, the failure message carried through, and the abort route's re-drive present for running / absent for paused. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CZwZPDBgxy4mCWWg6zQSjq * fix(schedules): converge terminal runs in clustered workers, retry suspension restore Fourth Codex round on 74f7af8. Both findings are in code from this closeout. P2 - a clustered worker never settled a run whose generation finished but whose outcome write failed. `recordScheduleOutcome` retries three times and then returns false; the owner honors that by PRESERVING the terminal job as the only surviving evidence, and the armed engine's reconciler replays exactly that. The clustered entrypoint arms no engine, and its sweep only covered deleting schedules (settleAbandonedRuns) and dead deliveries - an identity-matched job was skipped outright. So an ordinary live schedule's run stayed `started`, held its GLOBAL capacity slot, and kept a preserved job that carries no `completedAt` and is therefore invisible to the store's finished-job sweep: both leaked until store expiry. The live-schedule pass now also converges from a retained terminal job, mirroring the reconciler branch it stands in for: honor the owner's stamped outcome over the generic status (so a balance refusal still walks its streak rather than resetting it), clear the reserved conversationId when the generation never emitted its created event, and delete the retained job only AFTER the outcome write is durable. This stays positive-evidence-only and safe in every topology. Presence, not absence, is the evidence: an identity-matched job is authoritative wherever it is observed - a shared store shows the real generation, a process-local store can only be showing this process's own - which is why it needs no canInferOwnerDeathFromMissingJob fence, unlike the absence-based paths. The in-flight abort/resume fences still defer, so an `aborted` job cannot settle a run whose owner may still be persisting. Both cases now share one pass over one window rather than two, and `retainedOutcome` moved to types.ts so the sweep reuses the engine's mapping instead of duplicating it (and stays independent of the engine). P2 - a failed restore stranded a live account's schedules. Cancelling an account deletion restores the suspended rows while the deletion fence is still armed and then releases that fence; nothing re-drives the restore afterwards, so one transient write failure left the user with silently disabled, next-run-less schedules. The restore is now retried at the single choke point both the HTTP controller and the CLI share. Retrying is safe because each attempt re-reads only the rows STILL carrying the token: a partially-applied unordered write converges on exactly the stragglers, and a fully-applied one finds nothing. The fence is still released when every attempt fails, deliberately: retaining it would refuse the live account's schedule writes AND make beginAgentTriggerUserDeletion report `in_progress` forever, blocking the retry that is the convergence path (a later attempt adopts this snapshot, so its cancel restores these exact rows). Both callers now log the user id and suspension token so the state stays recoverable by hand if that never happens. Tests: retained terminal job settled and its evidence released, stamped outcome preferred over the generic status, stamped failure reason carried, reserved conversationId cleared for a never-created conversation, identity-mismatched terminal job ignored, abort-in-flight deferred; restore retried past a transient write failure and converging on a partially applied restore. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CZwZPDBgxy4mCWWg6zQSjq * fix(schedules): converge unprojected pauses and release replayed bookkeeping jobs Two of the four findings I flagged as pre-existing to my closeout commits. Both are in code this PR introduces, so merging would have shipped them; both are the same capacity/evidence-leak class the last four rounds have been closing. The remaining two (slotless rows escaping the per-user cap, and the non-rotating `started` reconciliation bucket) are genuinely latent and deliberately left alone. An UNPROJECTED PAUSE held a global capacity slot forever. The pause projection is what moves a run row off `started`; `recordScheduleOutcome` already retries it, but the request controller discarded the result, so three failed attempts were dropped silently. The armed engine's reconciler replays that state, but the clustered sweep did not: a paused job is not terminal, so the retained-job path returned without settling, and the dead-delivery path never inspects an identity-matched job at all. The row stayed `started` with no cutoff that would ever clear it. The call site now surfaces the failure, and the sweep converges it, mirroring the reconciler's pause branch: project `requires_action` (which frees the slot) but do NOT release the job's evidence — unlike a terminal job it is still live, awaiting an approval. The resume hand-off fence still defers, so re-projecting cannot release a slot a continuation just claimed. The BOOKKEEPING REPLAY pass leaked its retained job. A run reaches that pass only because its owner crashed before bookkeeping — which is also before it could release the job it retained for exactly this recovery. The active-run pass clears its own; this one never did, and a preserved job is deliberately kept WITHOUT `completedAt` so the store's finished-job sweep cannot reap it early, so nothing else ever would. It now clears after `finalizeBookkeeping` succeeds — identity-guarded, a no-op when no job is retained, and skipped entirely when the replay itself failed, since the retained job is the only surviving evidence in that case. Tests: pause projected and the slot freed while the live job's evidence is kept, paused job deferred during a resume hand-off, retained job released once replayed bookkeeping is durable, and retained job kept when the replay fails. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CZwZPDBgxy4mCWWg6zQSjq * fix(schedules): fence the clustered pause replay against a concurrent resume claim Codex round 5, on code I added in f735341. The finding is correct and the exposure is one I introduced. The pause replay lives in a sweep that runs in EVERY clustered replica, so several sweepers can observe the same unprojected pause. Its guards were all derived from an in-memory row SNAPSHOT: hasResumeHandoffInFlight reads the snapshot's resumeClaimedAt, and recordRunOutcome's pause branch matched any row currently in `started`/`requires_action` with no fence of its own. The race: sweeper A projects the pause and frees the slot; the owner's approval then claims a fresh one (markRunResumeClaimed takes the row to `started` WITH resumeClaimedAt in one write); sweeper B, still holding the pre-projection snapshot, passes its hand-off check and replays — `$unset: { capacitySlot, resumeClaimedAt }` under a continuation that is already running. The run reverts to `requires_action` while its generation proceeds outside global capacity. The engine's reconciler makes the same call and has the same snapshot-derived guard, but v1 arms exactly one engine, so it has no concurrent racer; the sweep is the first thing to run this transition in parallel. Left the engine alone rather than widening the change: its `requires_action` re-affirmation is deliberate and single-writer. Fixed where the race is, in the write itself. `recordRunOutcome` takes an optional `requireNoResumeClaim`, which adds `resumeClaimedAt: { $exists: false }` to the pause filter, and the sweep sets it. Because the stamp is written in the SAME update that moves the row to `started`, its absence is atomic proof no resume owns the row. The flag is deliberately NOT set by the generation owner: its own re-pause legitimately clears the stamp as the hand-off's completion signal. The fence cannot block the recovery it exists to enable: markRunResumeClaimed only matches `requires_action`, so a genuinely stuck `started` row can carry no resume claim — it is in fact blocking its own approval until this replay frees it. Tests: a replay from a stale snapshot leaves a resume-claimed row's status, slot, and claim stamp intact, while a stuck row with no claim still recovers; and the sweep is asserted to send the fence. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CZwZPDBgxy4mCWWg6zQSjq * fix(schedules): fence stale resume claims by age, not existence Codex round 6, on the fence I added in ee00c60. Correct again, and the defect is the mirror image of the one it fixed. The fence was an EXISTENCE check (`resumeClaimedAt: { $exists: false }`) while its caller's guard is a FRESHNESS check (hasResumeHandoffInFlight, bounded by RESUME_HANDOFF_STALE_MS). They agree while a claim is fresh and disagree once it is abandoned: a worker that dies after markRunResumeClaimed takes the row to `started` and stamps resumeClaimedAt — but before the continuation resumes or releaseRunResumeClaim rolls it back — leaves the stamp set forever. Past the bound the sweep correctly stops deferring and tries to recover the row, but the write rejected it purely because the field still existed. The row stayed `started` holding its global capacity slot, and its approval was unresumable for good, since markRunResumeClaimed only matches `requires_action`. That is exactly the stuck state this replay exists to clear, so the fence had reintroduced it for the crashed-resume case. `requireNoResumeClaim: boolean` becomes `resumeClaimStaleBefore: Date`, and the filter matches a row with no claim OR a claim older than that cutoff. The sweep passes the SAME bound its in-flight check uses, so the two can no longer disagree. A genuinely racing claim is by construction fresh — it is created after the sweeper's snapshot — so the race from round 5 stays closed. Tests: a row whose resume claim was abandoned by a dead worker now recovers (status, slot and stamp all cleared), alongside the existing two — a fresh claim still repels a stale-snapshot replay, and an unclaimed stuck row still recovers. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CZwZPDBgxy4mCWWg6zQSjq --------- Co-authored-by: Claude <noreply@anthropic.com>	2026-08-20 11:51:30 -04:00
Danny Avila	aa35cd42b1	📬 feat: Add Durable Agent Trigger Delivery (#14925) ... * feat: wire trusted agent trigger dispatch * feat: add durable agent trigger delivery * fix: annotate trigger envelope byte limit * test: isolate trigger startup in server specs * fix: fence trigger delivery during account deletion * test: isolate trigger service in user controller specs * fix: close trigger deletion admission race * fix: harden account deletion fences * fix: close durable trigger review gaps * fix: require offline stale-fence recovery * fix: type trigger lane sequence ids * fix: fence admin user deletion triggers * fix: make trigger deletion recovery durable * fix: harden offline user deletion * fix: serialize trigger lane publication * style: sort trigger delivery imports * fix: recover orphaned trigger publications * fix: preserve trigger recovery ordering * fix: fence trigger publication during purge * fix: defer remote trigger deletion fences * fix: close durable delivery cleanup races * fix: drain CLI generation owners before deletion	2026-08-17 09:25:08 -04:00
Danny Avila	88e08c91e8	🧷 fix: Preserve Elicitation Answers Across Aborts (#14745) ... * fix: preserve elicitation answers across aborts * chore: sort stream type imports * fix: guard malformed resolved answers * fix: close abort answer race gaps * fix: retain exact answers across pauses * fix: retain answers across resumed pauses * test: satisfy HITL fixture types * fix: retain legacy answers through approvals * fix: preserve answers in reconnect snapshots * test: type legacy answer fixture * fix: bind legacy answers to paused content * test: guard optional resume content * fix: resume questions without streamed content * fix: Preserve legacy answers through abort filtering * test: Narrow reconstructed abort fixture * test: Type abort transform fixture explicitly * fix: Isolate answers with missing ask content * test: Type missing-content ask fixture	2026-08-12 07:32:48 -04:00
Danny Avila	7347cfc195	🍡 feat: Batched User Questions With A Single Bounded Answer Form (#14737) ... * feat: support batched user questions * test: align batched question fixtures * fix: harden batched question lifecycle * test: submit batched HITL answers in e2e * fix: address batched question review findings * fix: preserve invoke return typing	2026-08-11 01:06:16 -04:00
Danny Avila	a3cec67e08	🪆 feat: Add Parent Activity Phase Summaries (#14721) ... * feat: add activity phase summaries * fix: preserve activity phase lifecycle semantics * fix: satisfy activity phase type checks * fix: simplify activity phase status mapping * style: format activity phase changes * fix: rebase activity phase bounds after shaping * fix: link activity phase trace ancestry * fix: reconcile activity phase bounds * style: format activity phase reconciliation test * style: align activity phase assertion * fix: retain reasoning across commentary * fix: preserve activity phase boundary state * fix: detect renderable phase children * test: type parallel phase assertion * chore: bump agents SDK for activity phases * fix: retain unphased lane reasoning * fix: preserve tool group expansion across phases * style: format phase expansion regression * fix: preserve phase interaction state efficiently * perf: skip sparse phase segment holes * perf: partition phase segments with offsets * fix: preserve phase boundaries and cursor state * test: align activity phase regressions with CI * test: keep phase context mock hoist-safe	2026-08-10 13:41:37 -04:00
Danny Avila	e7f1838515	⚡ feat: Reliable Interrupt & Steer Escalation and Recovery (#14558) ... * feat: surface interrupt-steer escalation on waiting messages The interrupt & steer feature shipped reachable only through the composer chord, the send-button hovercard, and the composer button; a message already waiting (queued for after the run, or steered and parked at the next tool boundary) had no path to it. Both waiting surfaces now carry one: - Queued rows get an icon-only ZapOff escalation button beside the existing Steer primary. It routes through sendQueuedNow, which now takes a preempt option on its live-run path. The tooltip teaches the composer chord, derived through resolveComposerKeyDown so a rebound or yielded chord is never advertised. - In-flight steer bubbles get an "Interrupt now" overflow entry with the same race rules as Edit: reclaim first, and only a `reclaimed` outcome resubmits (via retrySteer with preempt, swapping the chip for an interrupting one). `applied` and run-ended-mid-reclaim outcomes stop at the existing informational toasts, so the words can never land twice. Not offered on a steer already preempting. - Every during-run overflow menu gains an "Always interrupt instead" toggle for steerInterruptsByDefault, next to the existing steer/queue default toggle. MenuEntry supports disabled for the new entries. Only one interrupt can be unresolved at a time: while one preempt is pending (or the run is paused on approval, where the server 409s), every escalation control disables instead of racing the same seal. Ten new tests across both surfaces; 381 green in the affected suites. * fix: lock escalation across its reclaim window, keep the paused control visible, label as steer Codex round 1, all three findings. P2, escalation race. The single-interrupt invariant had a window between clicking "Interrupt now" and the reclaim resolving, where no preempt chip existed for the chip-derived gate to see: two bubbles escalated back-to-back could both resubmit. A shared escalating flag (Jotai, per-conversation) now covers the window and disables every escalation control on both surfaces, and a fresh recheck before resubmitting catches an interrupt armed elsewhere meanwhile (composer chord, queued row); those words re-home to the queue with an informational toast instead of breaking the invariant. P2, unreachable paused state. canSteer is defined as hasRealConvoId && !pausedOnApproval, so gating the button on canSteer removed it exactly when it was meant to render disabled; the test only passed on an impossible stub combination. The render gate is now duringRunActive && (canSteer || pausedOnApproval), and the test uses the real invariant. P2, label semantics. "Interrupt & send now" borrowed the name of the hard-abort action; this one preserves the partial answer and steers. Renamed to "Interrupt & steer now" (com_ui_interrupt_steer_now). Both behavior fixes counterfactually verified; 384 tests green across the affected suites. * fix: disable bubble escalation while the run cannot accept a steer Codex round 2, one P2. Answer mode (ask_user_question) sets duringRunActive false while pausedOnApproval stays false, since that flag only detects approval-bearing tool calls. The bubble's escalation entry stayed enabled there, so clicking it cancelled a healthy waiting steer and the preempt resubmission bounced off RUN_PAUSED, degrading the words to the queue. The entry now also disables on !duringRunActive, matching the queued-row control's gate. Counterfactually verified: reverting the gate fails the new answer-mode test. * fix: recheck live run state after the reclaim, not just at the click Codex round 3, one P2, and it is the round-1 recheck principle applied one level deeper: the entry-time disable cannot see a run that pauses (tool approval, answer mode) while the reclaim round-trip is in flight, and the .then closure held the render's stale steering controls, so the resubmit would fire into a RUN_PAUSED rejection after the reclaim had already surrendered the steer's boundary slot. The escalation continuation now reads the LIVE controls through a latest-ref: if the run can no longer accept a steer, the words re-home to the queue with an informational toast instead of resubmitting, and the resubmit itself also goes through the live controls. Counterfactually verified: reading the stale closure instead of the ref fails the new mid-reclaim pause test. * refactor: make escalation one atomic server-side arm, in place Codex round 4: four P2s, every one an interleaving of the same window — escalation as reclaim-then-repost is a compound, non-atomic operation whose continuation must revalidate the world (FIFO position lost, ref assigned too late, no run fence, competing bubble actions). Rounds 1-3 patched that window with a lock and rechecks; round 4 shows the window itself is the defect, so this removes it instead of guarding it again. Escalation is now POST /chat/steer/arm: the server flips preempt on the EXISTING queued item in one atomic store op (new IJobStore.armSteer; a decode-patch-encode LSET Lua on Redis, an in-place mutation in memory), fenced to the validated generation and refused once the queue closes. The handler mirrors the steer POST's preempt contract exactly: durable flag gated on the owner's recorded capability, volatile requestPreempt fire-and-forget because the durable flag is the truth resume/handover re-arm from. By construction this resolves all four findings: FIFO survives (the item never moves; the whole queue still drains in instruction order at the seal), there is no continuation to hold stale controls, the store op is fenced to the original run, and a competing Edit/Queue/Cancel either beats the arm (armed:false, chip untouched) or operates on the armed item, whose cancel already disarms. The client escalation entry becomes one mutation: armed:true relabels the chip in place (same steerId, same position), PREEMPT_UNSUPPORTED and lost races toast honestly, and the round 1-3 machinery — the escalating lock atom, the latest-ref, the post-reclaim rechecks and their two toast strings — is deleted rather than extended. Verified: 7 new handler tests on the real in-memory manager (including FIFO preservation and the stale-generation fence), 2 Redis integration tests against real Redis (in-place arm keeps order and every field; missing/stale/closed all refuse), client suites 396 green. * fix: decide capability inside the atomic arm, neutralize the lost-race toast Codex round 5, both findings, both edges of the new arm design rather than its mechanism. P2, capability TOCTOU. A HITL resume on a rolling deploy rewrites preemptCapable for the SAME generation, so the handler's read could go stale between validation and the flag flip, arming a steer the live owner cannot seal. armSteer now returns armed | missing | incapable, with the owner's live capability part of the same atomic predicate as the generation fence (HGET preemptCapable inside the Lua; the flat job field, not a metadata blob — the in-memory store reads the same field). The handler's pre-check is deleted rather than kept alongside; the store predicate is the single source. New handler test rewrites the capability after queueing and expects PREEMPT_UNSUPPORTED with the item left unflagged; the Redis guards test now asserts the incapable refusal against real Redis. P2, ambiguous toast. armed:false covers injected, cancelled, re-homed, and run-over alike, so telling the user the message "already reached the agent" claimed one specific outcome. The lost-race branch now uses a neutral message (com_ui_steer_arm_lost_race) and defers to the events for what actually happened. * fix: flip the escalation lock synchronously before the arm request Codex round 6, one P2. Round 4 deleted the escalating flag along with the reclaim continuation it guarded, but that left the one-interrupt gate blind during the arm request's own round trip: the chip-derived check cannot see an arm until its response relabels the chip, so on a slow connection two bubbles could both arm before either response landed. Double-arm is harmless server-side now (the run seals once and drains the whole queue in order), but every escalation control advertises "one interrupt at a time" by disabling, and the controls must tell the truth. The per-conversation escalating flag returns as a pure UX gate: set synchronously at click, before the mutation, cleared on settlement, and folded into interruptPending on both surfaces. Unlike its round 1-3 ancestor there is no continuation behind it to guard and no recheck to pair with it. Counterfactually verified: without the synchronous set, the two-bubble race test arms twice. 207 tests green across the Chat Input suites. * test(e2e): cover escalation of waiting messages through the real seal Three mock-harness tests on E2E_SLOW_REPLY, a 160-chunk stream with no tool boundary, so an in-thread steer part can ONLY come from a genuine mid-stream seal — which makes each test a behavioral proof rather than a UI check: - Queued row escalation: the ZapOff button turns a waiting queued message into a preempt-armed steer (202 echoes preempt: true) that seals and injects, where the sibling steering.spec test proves the unescalated path waits for run end instead. - Bubble in-place arm: an ordinary steer (202 with no preempt echo) waits as a bubble, POST /chat/steer/arm answers armed: true, the bubble relabels in place (same single bubble, same text, escalation no longer offered on reopen), and the armed steer seals mid-stream. - Always-interrupt toggle: flipped from a waiting row's overflow menu, plain Enter now produces a preempt: true steer that seals in the SAME run, and the menu offers the way back. An afterEach clears the localStorage preference so a mid-test failure cannot leak preempt-by-default into the rest of the serial suite. All three verified locally through the full harness (real backend, mock LLM, seeded DB): 3 passed in 27s. * feat: dedicated escalation arrow + shortcut, menu split into actions and preferences The escalation was still half-hidden: the bubble only offered it inside the overflow menu, and the tooltip taught the composer chord, which does a different thing (interrupts with typed text, not this chip). Three changes make it a first-class command: - A shared EscalateNowButton (circular arrow, ghost-bordered like the composer's interrupt control) is always visible on BOTH surfaces: beside each queued row's Steer primary and on every waiting steer bubble next to its menu. It disappears once a steer is interrupting. - A dedicated registry shortcut, escalateSteer (Cmd/Ctrl+Shift+.), editing-allowed and rebindable like every other action. Deliberately NOT an Enter chord: the composer owns every Enter chord, and the yield design rests on no default binding using Enter besides submit. Its handler clicks the newest enabled arrow control (bubbles beat queued rows), so the shortcut can never diverge from the button, and the arrow's tooltip teaches THIS command via the registry display. - The overflow menus separate one-off actions from sticky behavior changes: Edit, Cancel, Queue, then a smaller "Preferences" section holding the queueing and always-interrupt toggles, each with the standard InfoHoverCard reusing the Settings panel's descriptions. "Interrupt & steer now" leaves the menu entirely. 386 client tests green, including a menu-structure test locking the order and the absence of the escalation entry; bubble escalation tests drive the visible arrow. The e2e spec's bubble test now clicks the arrow, and a fourth test drives the dedicated shortcut end to end through a real mid-stream seal. * style: bind the escalation arrow to its message (variant A anatomy) Two same-weight circles in a row read as one control group, leaving the arrow's ownership ambiguous, and a floating arrow stops meaning anything once several messages stack. The shared control now carries variant A's anatomy: a thin divider binds a small SOLID arrow (filled, inverted) to the message region on its left, and the menu ellipsis stays a bare glyph, so the two affordances can no longer blur together — and the divider+arrow pairing repeats cleanly per chip at N messages. * chore: drop the unused within import CI lint caught * fix: advertise the escalation shortcut only while the control is live Codex on the e2e head, one P2: the tooltip appended the chord hint even while the button was disabled, advertising a shortcut that does nothing during an approval pause. The flagged control (InterruptNowButton) was since replaced by the shared EscalateNowButton, which inherited the pattern; the successor now omits the chord whenever the control is disabled, matching the rule the during-run hovercard already follows. * fix: harden steer escalation lifecycle and recovery * test(e2e): disambiguate accessible steer preferences * test: align abort persistence coverage with prerequisites * chore(i18n): remove obsolete steer race message * chore: normalize imports across steering changes * test: exercise stream integration on Redis Cluster * test: scope HITL checkpoints to generation * test: fix cluster cleanup and locale policy * fix: keep escalation visible during ask pauses * fix: fence recovery downgrade and stale predecessors * fix: require generation owner abort acknowledgement * fix: validate delayed preempt arms * test: align final escalation fixtures * fix: preserve in-memory predecessor abort handoff * fix: restore controls for recovered queued messages * test: cover recovered queue controls * fix: close final steering review gaps	2026-07-31 20:07:56 -04:00
Danny Avila	60ca751a7f	🧠 fix: Preserve Deferred Tool Schemas Across HITL Resume (#14552) ... * 🧠 fix: Preserve deferred tool schemas across HITL resume * 🧪 test: Harden deferred tool resume regression * 📦 chore: bump @librechat/agents to v3.3.10	2026-07-31 14:06:13 -04:00
Danny Avila	1e1de6eff9	🎯 fix: Exact Ask-Question Attribution via Interrupt tool_call_id (#14539) ... The ask_user_question pause/answer stamps (server pause-time args stamp, resume-time answer stamp, and the client mirror) targeted the newest unanswered ask part by position. When a model emits several ask calls in one turn, the interrupt's question and the user's answer land on the wrong card. @librechat/agents > 3.3.8 surfaces the interrupting call's tool_call_id on the ask interrupt payload. All three stamps now target that id exactly when present, keeping the positional fallback for older payloads. The tool body passes config.toolCall.id through to askUserQuestion via a typed alias that is a no-op on the pinned SDK and lights up on the next dependency bump. Companion to danny-avila/agents#366, which also fixes the underlying dangling tool_use 400 (INVALID_TOOL_RESULTS) when one of the parallel ask calls streams malformed args.	2026-07-31 10:11:08 -04:00
Danny Avila	7bb6651883	🛑 feat: Preemptive Steer - Backend Interrupt & Steer (#14518) ... * 🛑 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 (danny-avila/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'.	2026-07-30 12:36:13 -04:00
Danny Avila	d8427ffc5e	🛂 test: Cover Tool Approval Workflows End to End (#14427) ... * test: cover tool approval workflows end to end * fix: preserve tool approval state across resume * fix: preserve agent context in mock stream responses * fix: preserve nested approvals in collapsed groups	2026-07-26 21:58:25 -04:00
Danny Avila	73699b5c25	⚡ perf: Reduce Agent Chat Startup Latency (#14423) ... * perf: reduce agent chat startup latency * test: align Redis stream readiness assertions * perf: overlap remaining agent startup work * perf: persist initial agent job metadata atomically * test: add agent startup latency benchmark * fix: harden resumable agent stream lifecycle * fix: isolate replacement stream lifecycles * fix: preserve terminal stream epochs	2026-07-25 07:58:20 -04:00
Danny Avila	9bb351ad9c	🧭 feat: Mid-Run Steering and Queued Messages for Agent Runs (#14220) ... * 🧭 feat: Mid-Run Steering and Queued Messages for Agent Runs Steering: submit a message while a run is generating; the server queues it in the job store (cross-instance) and a run-scoped PostToolBatch hook injects it into graph state at the next tool-batch boundary, records an inline 'steer' content part on the response (replayed as a user message on later turns), and streams on_steer_applied to the client. Queuing: messages composed during a run auto-send as normal follow-up turns after clean completion (one per final event, FIFO); user aborts leave them as chips unless armed by interrupt-and-send. Requires hook injectedMessages support in @librechat/agents (danny-avila/agents#299); hard-gated via a capability probe so older SDKs 501 the steer route instead of draining and dropping messages. * 🧵 fix: Harden Steering Against Finalization Races and Route Guard Gaps Addresses local Codex review findings on the steering feature: - Close-and-drain the steer queue atomically at finalization (final event, abort) so a steer POST racing teardown is rejected instead of 202-ACKed and then silently cleared; the closed flag lives on the job hash and is reset when a replacement job reuses the stream id. - Clear inherited steer queues on createJob — a job replacement must not drain the replaced run's messages. - Keep steers queued across a HITL pause instead of draining them into ephemeral client state: resumeState re-seeds chips on reload and the resumed run injects them at its first tool boundary (steers key TTL now extends to the approval window; on_steers_pending event removed). - Queue the NO_ACTIVE_RUN steer fallback while the final SSE is still settling — a direct send would be dropped by ask()'s in-flight guard. - Reconcile the 202 ACK against on_steer_applied events that beat it over the SSE, so a chip can't be re-minted after its removal event passed. - Allow the per-send Steer override when the default action is queue. - Apply the configured message rate limiters and the PII filter to POST /chat/steer — a steer is model-bound user text. * ✅ ci: Assert Steering Capability Probe Against the Installed SDK CI installs the published @librechat/agents pin (pre-injectedMessages), where isSteeringSupported() is legitimately false — the probe test now asserts it mirrors the installed SDK's capability flag instead of hardcoding the capability-bearing build's value. Verified against both the published 3.2.61 dist and the agents#299 build. * 🛟 fix: Preserve Steer Text Across Run-End, Error, and Abort Races Codex round 2 (4 P2s): - Applied-steer-id set survives run end (capped at 100) and converted ids join it, so a 202 ACK that lands after final/abort drops its chip instead of re-minting a stranded pending one. - Failed runs no longer strand acknowledged chips: both error paths convert local pending chips to queued follow-ups (chip text is client-local), and the server closes the steer queue before emitting the error so a racing steer POST gets 404 fallback instead of a 202 whose payload dies with the job. - sendQueuedNow keys on steer availability, not the default action — send-now on a queued chip is an explicit override for queue-preferring users. - Stop path consumes pendingSteers from the abort HTTP response as a fallback for the SSE final event it may close before processing; conversion is deduped so double delivery is a no-op (shared useSteerConvert hook). * 📎 feat: Carry Attachments Through During-Run Queued Messages Steering stays text-only (SDK injection, inline STEER part, and replay are all text), so a during-run submit with media now queues the whole message as one unit instead of silently stranding the files: - QueuedMessage gains `files`; composer attachments are consumed into the queued item at queue time (steerFromComposer / queueFromComposer / interruptAndSend), fixing the latent hazard where lingering composer files glued onto whatever `ask` vacuumed up next. - Enter-steer with attachments degrades to queue with an explanatory toast; the per-send menu routes through the same composer-aware wrappers. - The drain and sendQueuedNow pass the item's files as `overrideFiles`; media items never steer (send as a normal turn when idle, re-front otherwise). ask() no longer clears composer state for caller-supplied overrideFiles — only regenerate keeps that behavior. - During-run submits hold while uploads are in flight, mirroring the send button's filesLoading gate; queued chips show a paperclip count. * 🎛️ feat: Rework During-Run Chips into Action Rows Full-width rows above the composer (reference-UI parity): each queued message shows a primary Steer/Send-now action, delete, and a "…" menu with Edit message (restores text + attachments into the composer) and a Turn on queueing/steering toggle that flips the Enter default. Steer rows share the layout with status text; failed steers keep retry / edit / queue-convert. The per-send menu gains the same default toggle. Queued file refs now retain filename + bytes so edit-restore rebuilds real composer entries (draft-recovery shape). * 🖇️ feat: Steer With Attachments (Multimodal Mid-Run Injection) Steering now carries media end-to-end instead of degrading to queue: - The steer POST accepts sanitized attachment refs (cap 10; only file_id is trusted — the drain re-fetches owner-scoped and re-derives everything else). SteerQueueItem/TPendingSteer/SteerContentPart carry `files` refs; encoded data is never persisted or queued. - New api/server/services/Files/steering.js decouples attachment building from the request path: encodeSteerContent reuses the exact per-turn pipeline (addFileContextToMessage + processAttachments' single-pass categorize/encode, SDK formatMessage assembly, prependFileContext for extracted text) with zero new encoding code. buildSteerMedia feeds the drain hook's new buildMedia seam (any failure degrades that steer to text-only — words always land); stampSteerPartMedia re-encodes past steer parts per turn with ONE batched owner-scoped fetch and stamps a transient `media` array, replaced immutably so it can never leak into a save. Replay honors resendFiles like regular message media. - The SDK's formatAgentMessages (the formatter agents actually use) gained the steer replay branch on the PR branch; the local formatMessages.js branch now mirrors the media preference. - Client: steerFromComposer consumes composer files into the POST, chips/seeding/conversions carry files everywhere (retry, queue convert, abort/error recovery), queued media items steer for real, and SteerBubble renders the steered attachments inline. * 🧵 fix: Harden Steer Recovery Races and Drain Isolation Codex round 3 (7 fixes): - A 202 ACK landing after the run ended converts straight to a queued follow-up (server queue is gone; no event will ever resolve a pending chip for a finished run). Covers stream errors with in-flight POSTs. - A Stop that lands pre-completion can arrive as a final with unfinished:true and no aborted flag — runEnd now treats it as aborted so queued messages are not auto-sent against the user's Stop. - Leftover-steer conversion merges chronologically by createdAt instead of appending, preserving the order the user composed. - Auto-drained queued messages pass explicit (possibly empty) overrideFiles/overrideQuotes/overrideManualSkills: a drain can no longer vacuum up files, quotes, or skill picks staged in the composer for the user's NEXT message (ask() treats overrideFiles != null as authoritative). - Failed-steer Retry and resume-on-load chip restoration keep the steer's attachments. - The job-replacement guard moved INSIDE the store's atomic drain/close-and-drain (Lua createdAt compare; in-memory equivalent): a stale run's hook or finalization can neither consume, close, nor steal a replacement job's steer queue, and the drain hook drops its separate check-then-drain round trip. * 🧰 refactor: Typed Steer Controller, Single-Query Media Pass, Round-4 Fixes Codex round 4 + efficiency tightening in one pass: - Moved the steer guard ladder (validation, file sanitization via a shared toSteerFileRef picker, ownership/tenant checks, status-guarded enqueue) into packages/api as handleSteerRequest; api/steer.js is now a thin wrapper. Ladder covered against the REAL in-memory job manager in request.spec.ts; the api spec pins only the wrapper contract. - Folded the steer replay stamp into the turn's ONE historical-files query: collectHistoricalFileRefs also gathers steer-part refs, the owner-scoped doc map rides client state, and stampSteerPartMedia consumes it (no second round trip) while encoding parts in parallel. - Stamped steer media now counts against the run budget (existing multimodal counter over the non-text parts, folded into indexTokenCountMap/promptTokens after the stamp). - Steer route runs the PII filter BEFORE moderateText, matching chat.js so blocked sensitive text never reaches the external moderation API. - Interrupt & send survives the abort-response-beats-SSE-final race: stopGenerating writes the run-end signal itself when the one-shot interrupt flag is armed and no signal landed (double-fire safe). - Resume reconciles chips against the server's still-queued list even when EMPTY, clearing chips for steers applied while disconnected. - The local formatter's steer flush preserves non-text assistant parts (array-content AIMessage) instead of folding to text. * 🔒 fix: Replay-Aware Capability Gate and Round-5 Race Closures - isSteeringSupported now requires BOTH halves of the SDK contract: injection (HOOK_INJECTED_MESSAGES_CAPABLE) AND replay (ContentTypes.STEER, shipped in the same SDK commit as the formatAgentMessages steer branch). An SDK that can inject but not replay 501s the steer route — no release window can create steer parts that would leak into provider-facing assistant content. - The local formatter mirrors the SDK's anchor reset: a post-steer tool_call mints a fresh AIMessage instead of attaching to the pre-steer anchor (invalid provider ordering). - Queued-chip send-now and the NO_ACTIVE_RUN fallback pass explicit (possibly empty) overrideFiles so an idle send can't vacuum composer files staged for a different draft. - Redis createJob deletes the stale steer list BEFORE the replacement hash is written as running — a steer 202-accepted against the new job can never be wiped by the reset. - Resumed-turn finalization mirrors the normal path's terminal drain: createdAt-guarded close-and-drain, leftovers ride the resumed final event as pendingSteers instead of being cleared by completeJob. - buildSteerMedia restores composer order over the $in result so multi-attachment steers reach the model in the order the user saw. * ⚛️ fix: Atomic Job Replacement and Boundary-Clean Steering Module Codex round 6 (5 fixed, 1 standing deferral): - createJob resets the steer queue and writes the job hash in ONE same-slot Lua script (JOB_CREATE_LUA): a steer POST can no longer interleave between them on cluster, so a steer accepted against one run can never be drained into another. Redis-validated. - The steering media pipeline moved to packages/api (agents/steering/media.ts) with injected getFiles and a structural client interface — /api keeps zero steering logic; specs ported to the DI seam. - handleSteerRequest checks the job BEFORE the capability gate: a steer racing completion on an unsupported SDK gets 404 (send-now) instead of a 501 queue with no run-end signal left to drain it. - useQueueDrain binds to the active conversation: navigating away between the final SSE and the drain effect leaves the signal unconsumed instead of submitting A's follow-up into B; the drain fires on return. - abortJob closes and drains the steer queue BEFORE the content snapshot, so a drain-hook apply that lands pre-drain is captured inline rather than lost between the snapshot and the terminal drain. * 🚦 fix: Parked Run-End Signals, Interrupt Priority, Settled-Run Fallbacks Codex round 7 (5 fixes): - Run-end signals for a non-active conversation are PARKED per conversation instead of squatting the shared index slot: a later run finishing on the same pane can no longer overwrite them, and the parked drain fires when the user returns. - "Interrupt & send" front-inserts carry a priority flag that outranks createdAt when abort leftovers merge back chronologically — the urgent redirect drains first, not the oldest steer. - STEER_UNSUPPORTED/RUN_PAUSED/QUEUE_FULL rejections landing after the run settled mirror the NO_ACTIVE_RUN fallback and send immediately (queueing would strand the text with no run-end signal left); on the pinned SDK this is the common Enter-near-run-end path. - A failed abort (e.g. 404 when the run completed first) still signals the interrupt drain, so the queued interrupt message can't strand and the armed flag can't leak onto a later run. - Steered-image fallback alt text is localized (com_ui_attached_image). * 📌 chore: Adopt Published @librechat/agents Types Post-Bump dev's pin bump to ^3.2.62 (the release carrying injection + steer replay) landed via merge; the steering runtime now uses the SDK's real InjectedMessage/hook-output types instead of the local structural mirrors that bridged the pre-publish window. The two-half capability probe stays as the defensive gate for mismatched deployments — and the capability spec now exercises its TRUE path against the published package in CI. * 🛅 feat: Park-and-Claim Steer Recovery + Host-View Content Reads Codex round 8 (6 fixed incl. both P1s, 1 push-back): - The long-deferred no-subscriber gap is closed: every terminal drain (final, aborted-final, error, abortJob, resumed finalize) PARKS acknowledged leftovers on the job hash (unrecoveredSteers), and the status route claims them exactly once for inactive jobs — a client that closed/reloaded past the transient final event restores its steers as queued chips within the post-terminal TTL. A replacement run clears the parked copy (a live client started it). - Same-instance content reads are steer-complete: RedisJobStore now caches the HOST content array (WeakRef) via setContentParts and prefers it over the SDK graph cache, whose view never contains host-authored steer parts; the graph fallback splice-INSERTS steer chunks at their recorded host-view indices (the graph array is unshifted, so assignment would overwrite SDK parts). - Replay token accounting now counts prepended file-context text: full stamped content minus the steer body (already counted), so large steered documents hit the budget instead of bypassing pruning. - The queue drain restores an item when ask() refuses without sending (history not yet in cache after navigating back) — text is never silently dropped. - The armed interrupt flag travels WITH a parked run-end signal, so another run on the same pane can neither consume nor clear it. - parseTextParts extracts steer text (search indexing / audio). * 🎛️ refactor: Single Send Slot + In-Thread Steer Messages - Merge the during-run send affordance into the send/stop button slot: with composer text the send button replaces Stop (Enter = default action), hover reveals Steer/Queue/Interrupt rows with shortcuts; drop the separate DuringRunActionsMenu chevron - Add during-run keyboard chords: Cmd/Ctrl+Enter = non-default action, Alt+Enter = interrupt & send (plain-Enter submitters only) - Render steers as standard user messages in the thread: SteerPart (icon + author header + user text presentation) replaces the SteerBubble, and submitted steers appear immediately at the projected injection point via the PendingSteers slot on the streaming message - Keep composer rows only for recoverable states: failed steers (retry/edit/queue) and queued follow-ups * 🩹 fix: Keep the Replacement Submission Alive Across Abort Settlement The aborted run's final SSE event fires before the abort HTTP response resolves, so an armed interrupt & send drains and starts the NEXT submission while the abort POST is still in flight. The response handler's unconditional clearAllSubmissions() then reset the new submission, aborting its stream attach before the subscribe — the follow-up ran and persisted server-side but the live placeholder finalized empty (content appeared only after reload). useAbortCleanup captures the submission before the abort round-trip and both settlement paths (success and 404-catch) clear only when the captured submission is still current; a replacement stays untouched. Plain Stop behavior is unchanged. * 🧭 test: Playwright E2E for Mid-Run Steering and Queuing - Add e2e/specs/mock/steering.spec.ts: steer mid-run (202 + immediate in-thread pending part + real MCP tool boundary + words survive run end), Cmd/Ctrl+Enter queue with auto-send after clean completion, and Alt+Enter interrupt & send with the follow-up streaming into the live view - Add the E2E_STEER_TOOL_REPLY fake-model marker: slow preamble, a real remember_fact MCP tool call (PostToolBatch boundary), then a final turn - Test 1 pins the run-end degradation contract while the SDK's top-level agentId stamping bug blocks live injection; its header documents the assertions to flip once the fixed SDK is pinned * 🧷 fix: Job-Independent Steer Recovery + Expiry and Resume-Gap Parking Codex round 10: the park-and-claim recovery had lifecycle holes. - Move parked steers off the job hash onto their own bounded-TTL store key (JOB_CREATE_LUA resets it; deleteJob leaves it alone): the default completeJob path deletes the job record immediately, and the Redis read path never deserialized the old hash field — recovery previously worked only with STREAM_KEEP_COMPLETED_JOBS on the in-memory store - Carry the owner identity inside the parked payload and authorize the claim against it, so the status route recovers steers on its jobless branch too (the common reload-after-terminal case); a non-owner claim returns nothing and re-parks the payload - Park queued steers on approval expiry: snapshot the frozen queue before the requires_action→aborted CAS (whose terminal cleanup drops the steers key) and park only when the CAS wins - Mirror the terminal drain/park block in resume.js's failure path, which previously let completeJob's backstop clear 202-accepted steers - Close the Redis snapshot→subscribe resume gap: re-peek the queue after attaching and re-surface missed on_steer_applied events from the durable content view (synthesizeAppliedSteerEvents), updating resumeState.pendingSteers to the live queue * 📌 chore: Require @librechat/agents 3.2.63 + Applied-Steer E2E Contract - Bump the @librechat/agents pin to ^3.2.63 in api/ and packages/api/: it scopes the hook agentId marker to subagent child graphs, so the steering drain hook fires at top-level tool-batch boundaries and mid-run injection is active (danny-avila/agents PR 307) - Flip e2e steering test 1 from the documented degradation contract to the applied-steer contract: the optimistic in-thread part transitions to the persisted part at the tool boundary and survives inside the response after run end, with no queued follow-up turn * 🎗️ feat: Steered Messages Join the Message-Nav Ribs Steers are user messages, so they get their own clickable rib on the navigation rail, interleaved at their in-thread position inside the response that absorbed them (one DOM query in document order). SteerPart anchors itself as #steer-<id> with a steer-render marker — both the optimistic pending entry and the persisted part — and the rib carries the user role label with a preview drawn from the steer's text body, skipping the author header. * ❎ feat: Cancel a Queued Steer Before Injection + True User-Message Alignment - Add POST /chat/steer/cancel: removes ONE still-queued steer by id via an atomic list rebuild (Redis Lua preserves order and TTL), authorized against the job owner; removed:false is advisory — the cancel lost its race to the drain or the run end, never an error - Surface an × on the in-thread pending steer (server-acknowledged entries only): optimistic removal, restored if the POST fails since the server would still inject the words - Outdent SteerPart past the response's icon column so steers sit flush with top-level message rows, reading as regular user messages * 🧯 fix: Round-11 Recovery Hardening + Provider-Free Pending Slot - Reconcile the resume steer gap by steerId SETS, not queue length — a steer added in the gap (or an equal-length drain+enqueue swap) now refreshes resumeState.pendingSteers and still synthesizes the missed on_steer_applied events - Make completeJob's terminal backstop park: direct error-path callers without the controllers' close-and-park no longer silently clear 202-accepted steers (createdAt-guarded closeAndDrain + owner park before the terminal write) - Persist the steer part BEFORE media encoding in the drain hook: an abort inside the encode window can no longer lose a file-steer (the part refs come from the enqueue-sanitized item; replay re-encodes per turn unchanged) - Move the parked-claim owner check INSIDE the atomic store claim (substring gate in the Lua / in-memory equivalent): a non-owner probe can no longer transiently delete the recovery payload; the app-side parse stays authoritative - Park queued steers in BOTH stores' own requires_action expiry cleanup, which bypassed the manager-level sweep - Sweep expired parked steers from the in-memory store's periodic cleanup; restore a queued chip when send-now's submit is refused; upsert steer ACKs so an SSE reconnect reseed cannot duplicate chips - Mount the cancel mutation per steer item so the pending slot needs no QueryClient on ordinary streaming renders (fixes the CI failure in ContentParts.integration.test) - Skipped delivery-gated parking (finding 8): transport receiver counts cannot prove browser delivery, and gating the only durable copy on them trades cosmetic chip resurrection for real text loss; the window is already bounded by claim-on-read, createJob reset, and the TTL * 🩺 fix: Annotate PARKED_STEERS_TTL_MS for isolatedDeclarations tsdown's d.ts generation requires explicit types on exported consts with computed initializers; tsc --noEmit does not run that check, so the round-11 export slipped past local verification and broke Build packages (and every downstream CI job that consumes the built dist). * 🛟 fix: Round-12 Terminal-Path Recovery + Durable Steer Events - Park queued steers before the stale-running reap deletes a crashed or hung job in BOTH stores — the one terminal path with no controller finalization; requires_action expiry parking refactored onto the same snapshot/park helpers - Enqueue instead of dropping when a steer fallback send is refused: both the NO_ACTIVE_RUN branch and the settled-run rejection branch now observe sendNow's false return - Recover on the SSE reconnect-404 terminal path: convert local pending steers to queued, claim parked steers via /chat/status, and write a non-completed run-end signal so interrupt flags release without auto-sending an unknown outcome - Fall back to a positive parked-recovery TTL when completedTtl is 0 (SET EX 0 is invalid and silently killed recovery) - Make on_steer_applied durable before publish: emitChunk gains a durable option that awaits the chunk-log append (best-effort) ahead of the transport publish; the default delta path stays fire-and-forget * 🔐 fix: Round-13 Steer Authorization + Trusted File Refs - Resolve client-supplied steer file refs against the DB owner-scoped at enqueue and queue only DB-derived shapes (same filter as the injection fetch, shared via refs.ts); any unresolved id fails loud with 400 — spoofed type/filepath metadata can no longer be persisted into assistant content or rendered in chat/share views - Enforce agent authorization on /chat/steer against the ORIGINATING run's job identity: the chat path's role gate (AGENTS:USE, with the same non-agents-endpoint skip) plus the per-agent ACL check with the capability bypass — revoked access mid-run can no longer inject; cancel stays ownership-only (nothing model-bound) - Mark steered uploads used after a successful enqueue (owner-scoped, best-effort) so the upload-window TTL cannot reap a file the persisted steer part references - Consume the parked recovery copy after live delivery: converting final/abort/error pendingSteers fires one owner-gated claim-on-read, so dismissed chips can no longer resurrect on a later reload * 🎙️ fix: Round-14 Composer-Context Fidelity + TTS and Queue-State Gaps - Keep steer text out of generic assistant text extraction: parseTextParts excludes STEER parts by default with an includeSteer opt-in for the full-record surfaces (Meili indexing, aborted-response persistence) — TTS callers no longer speak the user's own mid-run words - Mark queued uploads used at enqueue time via a minimal owner-scoped POST /files/usage (fail-closed without a user; upload limiters do not apply to a metadata touch), fired once wherever composer files enter the queued state — the upload-window TTL can no longer reap a file waiting out a long run or approval pause - Carry quote chips and manual skill picks on queued items: captured and consumed from the composer at queue/interrupt time exactly like files, threaded through the drain and send-now overrides, and restored by the queued row's Edit message - Key an early-aborted FIRST turn's run-end signal to NEW_CONVO (resolveRunEndTarget) so queued follow-ups stay visible on the restored new-chat composer instead of parking under an optimistic stream id the user never sees again * 🧿 fix: Round-15 Gap Coverage + Consolidated Sweep (Share Leak, Abort Ids, Chip Hygiene) - Run the resume steer-gap check for every still-active job: an empty snapshot no longer skips the re-peek, and synthesis now keys on the FRESH content view so an applied-in-gap steer that was never snapshotted still re-surfaces (over-emission is benign — applied-id dedupe, index-stable parts) - Thread queued context through steer degradation: sendQueuedNow passes the item's quotes/skills into submitSteer, and every fallback (requeue or settled send) restores them instead of dropping to text+files - Stop shared links from leaking steer attachment refs: the share snapshot now walks content — files-excluded shares strip steer-part files entirely; files-included shares sanitize and share-route them like top-level files (copy-on-write, non-steer content by reference) - Seed pending-steer chips unconditionally on load/return so a steer applied while away cannot linger as a stale chip beside its part - Use the abort response's resolved job id: chips/drain-signal land where the user actually is (NEW_CONVO for a new-held first turn, consistent with resolveRunEndTarget) while the parked-copy claim hits the resolved id instead of a no-op /chat/status/new - Open steered documents like normal message files (FilePreviewDialog) - Cap the applied-steer id set on the live path via a shared helper; kept surviving run end deliberately (late-ACK race depends on it) and fixed the atom comment that claimed otherwise * 💡 fix: Un-light Steer Ribs When Their Node Is Replaced Two stacked gaps kept a steer rib lit after scrolling away: the pending→applied swap replaces the DOM node under the same id, which produces no IntersectionObserver exit and — because the entry list dedupes on (id, preview) — no entries change either, so the observer kept watching a detached node; and the rail's mutation filter only reacted to .message-render nodes, so steer-node swaps and removals never triggered a refresh at all. - reconcileObservedElements re-points the observer at replaced nodes from the mutation-driven refresh regardless of entries identity, dropping stale visibility until the fresh node reports (the observer fires its initial intersection immediately, so a truly visible part re-lights within a frame) - The mutation filter now recognizes steer-render nodes alongside message rows * 🪪 fix: Round-16 Recovery Owner Fields + Context Stickiness + Share Labels - Park resumed-run leftovers with the manager facade's metadata owner fields: a bare job.userId is undefined on that shape, which made every parked payload from a resumed HITL run unclaimable - Keep a queued item's quotes/skills sticky through a successful steer ACK: the pending chip carries them (client-only), reseeds preserve them across reconnects, and every terminal conversion — local or server-list, merged by steerId — restores them onto the queued item - Convert resumeState.pendingSteers on the inactive status branch (deduped against unrecoveredSteers) so steers observed in the expired-pause-before-sweeper window convert instead of vanishing until a later reload - Label shared steer parts share-safely via the existing ShareContext: a viewer's own name no longer appears on the sharer's steered messages * ✂️ fix: Carry Steer Context Through the Failed-Chip Edit Action Retry and convert-to-queue already preserve a failed steer's carried quotes/skills; Edit message dropped them on the way back to the composer. It now restores them through the same context path.	2026-07-14 10:11:10 -04:00
Danny Avila	988a14a405	🙋 feat: ask_user_question - agent-initiated questions with durable pause/resume (#14139) ... * feat: ask_user_question tool — agent-initiated questions with durable pause/resume The HITL runtime merged in #13942/#14024/#14025/#14123 already ships the full ask_user_question lifecycle (payload-agnostic handleRunInterrupt, resume validation via mapAskUserAnswer, reconnect rehydration, and the client question card) — but nothing ever raised the interrupt. This adds the producer: - packages/api/agents/hitl/askUserQuestionTool.ts: LLM-callable tool whose func calls the SDK askUserQuestion() helper (LangGraph interrupt() from the tool body); zod schema with length caps mirroring AskUserQuestionRequest, plus a JSON-schema twin for the schema-only registry - Registration: agentToolDefinitions, manifest.json (Tools dialog, admin filteredTools/includedTools kill switch), basicToolInstances, handleTools constructor branch - run.ts gating: checkpointer now attaches for hitlCapable runs whose agents carry the ask tool even with the tool-approval policy disabled (the interrupt needs only durability, not humanInTheLoop/hooks); the tool is stripped fail-closed from non-HITL callers (OpenAI-compat/Responses) and subagent child configs; excluded from eager event execution (interrupts must be raised inside the Pregel task frame) - resume.js: 16k length cap on the answer wire field - e2e (real Run + FakeChatModel + LazyMongoSaver + supertest resume): tool-body interrupt pauses durably with NO approval policy, answer round-trips as the ToolMessage content, tool body re-runs once on resume, sequential questions re-pause * fix: adversarial-review findings — in-graph execution, orphan prunes, endpoint scoping, real kill switch Pre-PR multi-agent review confirmed 5 defects in the initial commit; all fixed: 1. CRITICAL — the tool never paused on the real agents endpoint: production loads tools definitions-only, flipping the SDK ToolNode to event-driven dispatch, and the host ON_TOOL_EXECUTE handler runs outside the Pregel task frame (under runOutsideTracing), where interrupt() throws and becomes an error ToolMessage. Reworked: the ask tool never rides toolDefinitions/ toolRegistry — on HITL-capable top-level agents a real instance is supplied via AgentInputs.graphTools (agents#289, requires @librechat/agents > 3.2.57), the SDK's in-graph direct-tool seam; new production-shape e2e pins the event-driven mode end to end. 2. CRITICAL — ask-only runs left orphaned interrupted checkpoints (silent context duplication on every later turn): both orphan prunes were gated on toolApproval.enabled. The pre-turn prune now also fires for ask-capable agents (exported agentRequestsAskUserQuestion), and the abort-route prune fires when the aborted job carries a pendingAction. 3. MAJOR — self-spawned subagents bypassed the strip (self config resolves from the parent's _sourceInputs): fixed SDK-side (buildChildInputs clears graphTools) and the tool is now never present on child surfaces host-side. 4. MINOR — the manifest entry leaked into the Assistants tools dialog and the legacy plugins endpoint, where tools execute with no run to pause: new agentsOnly manifest flag, scoped out of both listings. 5. MINOR — filteredTools/includedTools only hid the tool from the dialog: now enforced at run build (strip + no checkpointer), making the admin filter a real kill switch for already-saved agents. * chore: update @librechat/agents dependency to version 3.2.58 in package-lock.json and package.json files * fix: reject agents-only tools at assistant create/update (Codex round 1) The tools-dialog scoping keeps ask_user_question out of the assistants LISTING, but the v1/v2 create/update handlers resolve arbitrary posted tool strings from the shared getCachedTools map — a REST client or stale saved payload could still attach it, and the assistants runtime executes tools with no run to pause, so every call would error. New isAgentsOnlyTool(tool) (manifest-driven, handles string and function-object shapes) drops such tools with a warn at all four resolution sites (v1+v2, create+update). * fix: offset resumed-run content indices past the pre-pause seed A resumed run rebuilds the graph from the checkpoint, and the fresh graph numbers content indices from its own empty contentData — starting at 0. The resume path seeds the (also fresh) content aggregator with the pre-pause parts at exactly those indices, so the resumed model turn collided with the seed: type-matching parts silently MERGED (post-resume text appended into a pre-pause text block), and type-mismatching parts (a reasoning/think part at index 0 — any Anthropic reasoning agent) dropped EVERY delta with 'Content type mismatch', losing the entire post-resume output from the live stream and the saved message. Latent since #13942 — tool-approval resumes corrupt content the same way (probe-verified); it surfaced now because ask_user_question makes pausing a first-class flow and reasoning models make the loss total. - createContentIndexOffsetHandlers(handlers, offset): wraps ON_RUN_STEP (the single point where a content index enters the pipeline — deltas resolve through the aggregator's stepMap) and ON_AGENT_UPDATE's inline index; every other handler passes through by reference. Probe-validated: resumed output now lands as a new part after the paused tool call. - resumeCompletion wires it with offset = seedContent.length. - logToolError: a GraphInterrupt unwinding out of a tool body is the HITL pause working as designed — no longer logged as a Tool Error. * fix: unblock live streaming of the resumed segment after an answer With resume indices now ABSOLUTE (server continues after the pre-pause parts), the synthetic ask-user-question card was squatting on exactly the index the resumed segment streams into: applyAskUserQuestion appends the card at the end of the message content, so on the answering device every incoming part at that index was blocked and nothing rendered between the answer submission and the finalize replacing the message. removeAskUserQuestionPart(message, actionId) strips the pause-scoped card on successful answer submission (useResumeSubmit onSuccess) — the durable record of the Q&A is the ask_user_question tool call itself. Pure helper + specs; same-reference no-op when nothing matches. * fix: displace the synthetic question card in the streaming content writer The store-level strip on answer submit wasn't enough: the SSE step handler keeps its own in-flight copy of the streaming message, so on the answering device the synthetic ask-user-question card still occupied the ABSOLUTE index the resumed segment streams into — every delta warned 'Content type mismatch' (existing ask_user_question vs incoming text) and nothing rendered between the pending_action and finalize. Displace the card inside updateContent when any real part claims its slot — the same displacement pattern as the OAuth prompt part directly above it. Covers the streaming handler's own copy, reconnecting tabs, and other devices; once real content streams, the pause is over by definition. Spec drives a runStep + text delta into the card's index and pins: no mismatch warn, card gone, text rendered. * feat: dedicated UI + durable data for completed ask_user_question calls The completed ask call rendered as a generic tool card labeled 'Cancelled' with raw (and empty) JSON args. Two layers fixed: Data: the saved tool_call part had args:'' and no output — streamed arg chunks carry no tool name so the aggregator drops them (normal tools recover via the completion event, which never fires for a tool that interrupts mid-execution and resumes on a rebuilt run with no step id). The resume controller now stamps the paused ask part with the pendingAction's authoritative question as args and the user's answer as output (attachAskUserQuestionAnswer — pure, targets the newest unanswered ask part, so sequential questions each keep their own answer). UI: Part.tsx routes ask_user_question tool calls to AskUserQuestionCall — a compact Q&A record ('Asked a question' header, question, description, 'You answered: <label>' preferring the picked option's label, or 'No answer was given' for an abandoned pause) instead of the generic card. New i18n keys; parseAskUserQuestionArgs degrades to null on malformed model args. * fix: single question UI per pause + immediate answer display Two live-turn issues with the new durable Q&A card: 1. Duplicate question on ask: during a live pause the message carries BOTH the ask tool_call part (now rendered by AskUserQuestionCall, showing a misleading 'No answer was given' while paused) and the synthetic interactive card. The durable card now defers while the turn is live and unanswered (isSubmitting) — the interactive card owns the question UI until it's answered; an abandoned pause still shows its no-answer state once the turn settles. 2. 'No answer was given' after answering: the server stamps the answer onto the part at resume seed, but the client only received that at finalize. No stream emission needed — the client knows the answer it just submitted: resolveAskUserQuestionPart (replacing the plain strip on submit success) removes the synthetic card AND stamps output/progress onto the newest unanswered ask tool_call, seeding args from the synthetic part's question when the streamed args were lost — mirroring the server-side attachAskUserQuestionAnswer, so the Q&A record shows the answer the moment the user submits. * fix: keep the Q&A record visible while the resumed segment streams The optimistic output stamp lives in the message store, but the SSE step handler evolves its own cached copy of the streaming message (created at turn start) — the first resumed event overwrites the store with that copy, wiping the stamp, so the Q&A card blinked out during streaming and only returned at finalize. Render-layer fallback instead of fighting the handler's copy: submitted answers are recorded by ask tool_call id when resolveAskUserQuestionPart stamps the part, and AskUserQuestionCall reads the recorded answer whenever the part's own output is missing — the record survives any message-copy churn until finalize delivers the server-stamped part. * feat: present Ask User as a native builtin in the tools dialog It ships with the app and pauses the run like a first-class feature, so it belongs with the builtins (Run Code, Web Search, Memory, ...) rather than in the third-party plugin list — while its mechanics stay exactly a plugin's: - BuiltinId += 'ask_user_question' (documented exception: a native TOOL, not a capability; selection reads agent.tools, the toggle emits tool-add/remove patches instead of a capability field) - buildCatalog surfaces it as a builtin gated on the same signals as before (tools capability on + the server lists the plugin, i.e. not admin-filtered) and skips it in the plugin loop so it never double-lists - On-theme icon: lucide MessageCircleQuestion in a teal chip via the builtin icon map, matching the other native entries; the bespoke purple SVG and the manifest icon field are gone - i18n'd name/description keys like the other builtins * feat: composer popover for answering questions (mentions-style) Answering moves to the composer, matching the existing mentions/prompts popover pattern: while an ask_user_question pause is live, a popover anchors above the textarea with the question as its header, numbered option rows (hover/click, or ↑/↓ + Enter from the empty composer), and an × to dismiss. The main textarea doubles as the free-form answer — its placeholder flips to 'Something else...' and form submit routes the text to the paused run as the answer instead of starting a new turn. Dismissing (× or Escape) restores normal sends; the inline transcript surfaces stay as before (interactive card while paused, durable Q&A record after) so the question remains visible in history. - findLiveAskUserQuestion (pure, spec'd): newest unanswered synthetic part across the conversation IS the popover signal — applied on on_pending_action, stripped on answer submit, so visibility tracks the pause lifecycle with no extra state - useLiveAskUserQuestion hook shared by the popover and ChatForm; dismissals in a recoil atom so both react - popover only mounts on the primary composer (index 0), mirroring QuoteButton * feat: number-key selection + return glyph in the question popover Pressing 1-9 in the empty composer picks the matching option directly, mirroring the numbered row chips; the highlighted row shows a return-key glyph as the Enter affordance. Same empty-composer guard as the arrow keys — typing a free-form answer is never intercepted. * refactor: first-class composer answer mode (useAskAnswerMode) Replaces the bolted-on integration (inline onSubmit interception + raw capture-phase keydown listeners on the textarea ref) with a single hook that owns the whole answer mode: live-question derivation, dismissal + highlighted option (shared recoil state), option selection, free-form submit routing (submitText returns whether it consumed the submission), and keyboard handling (handleKeyDown returns whether it consumed the key, composed ahead of the textarea's normal handler — no more addEventListener). The popover is now pure rendering off the hook; ChatForm wires placeholder, onKeyDown, and onSubmit through the same instance. Deliberately scoped to the composer rather than useSubmitMessage: starters/prompt-commands keep new-turn semantics (and the existing job-replacement behavior while paused). * fix: Codex round 2 — inline answer input, approval exemption, pause-time args F1 (composer submit unreachable while paused — isSubmitting keeps Stop shown and useTextarea eats Enter): redesigned around it, borrowing Claude Code's AskUserQuestion semantics. The popover now owns free-form input via an inline 'Other' row (numbered last, 'Something else…'), with select-then-confirm rows (click/arrows/digits highlight; Submit ↵, Enter, or double-click fires; Skip dismisses). The composer returns to being a plain composer — no placeholder swap, no submit interception; Stop keeps meaning stop. F2: ask_user_question is exempt from the tool-approval prompt unless the admin explicitly lists it (allow/ask/deny all win) — approving the right to ask a question was a pure double pause; the tool is side-effect-free. F3: the question is stamped onto the paused ask tool_call's args at PAUSE time (attachAskUserQuestionArgs in handleRunInterrupt), so abandoned/expired/ stopped turns persist with the question intact and the record card can render it — previously only the answer-resume path stamped args. * fix: fold model-supplied 'Other' options into the inline free-form row The model can generate its own catch-all option ('Other (type your own)', value 'other'), duplicating the popover's built-in free-form row — two other-ish rows, one pickable as a literal answer. Two layers: - Tool description now tells the model NOT to include catch-all options (the answer UI always offers free-form input on its own) - splitOtherOption (pure, spec'd) folds a catch-all option that arrives anyway out of the choice rows and uses its label as the inline input's placeholder — conservative match (value 'other', or a label reading as a free-form invitation), no false positives on real choices * fix: single question surface + clean free-form-only popover Two live-pause confusions: (1) the inline transcript card and the composer popover both rendered — the card now defers while the popover is up for its action, returning as the fallback surface when the user dismisses the popover (and in contexts without a ChatContext, where the popover can't exist); (2) an options-less question showed a pointless numbered '1 Something else…' row — free-form-only questions now render the inline input alone, with the 'Type your answer…' placeholder (a folded model 'Other' label still wins). * feat: the composer is the free-form answer box (like the main chat input) While a question pause is live, the main chat textarea composes the free-form answer — placeholder swaps to 'Something else…' (or a folded model 'Other' label), Enter with text submits the answer through answer-mode key handling (composed BEFORE useTextarea's submitting-lock, so the lock can't swallow it), and the Stop button swaps to Send (enabled despite isSubmitting) per the select-then-confirm design. The popover slims to the question header, numbered option rows, and Skip/Submit — its inline input is gone since the composer owns free-form now. Dismissing the popover restores normal composer semantics (Stop button, normal sends). * fix: Codex round 3 + real Skip semantics - Skip now ANSWERS instead of hiding UI (danny): it resumes the run with a decline notice ('The user chose not to answer this question.') so the model moves on — a client-side dismiss left the run paused until expiry, a hung turn. × / Escape remain pure dismiss (switch to the inline card surface). - P1 (resumed approval tool indices): resumed tool_calls steps whose tool_call id matches a seeded UNRESOLVED part now rebind to that seeded slot instead of offsetting — the original part resolves in place (output attaches) and no duplicate appears; message steps keep the offset, so the text-loss fix stands. createContentIndexOffsetHandlers now takes the seed array; resolved seeded calls are not rebind targets. - P2 (stale selection across questions): selection state resets when the live actionId changes; the vestigial inline-Other state ('other' selection + text atom) is gone — the composer owns free-form. - P2 (Redis abort path loses the args stamp): the abort route re-stamps the question onto the ask tool_call in the reconstructed abort content, so a Stop-abandoned question persists with its question intact. - P2 (malformed args crash): parseAskUserQuestionArgs normalizes untrusted shapes (options: {} / non-string entries) instead of throwing in render. * feat: free-form hint in the question popover footer Left-aligned in the footer row (opposite Skip/Submit): 'Or type your answer below' — points open-ended answering at the composer, whose placeholder already reads 'Something else…'. * feat: preserve composer drafts across the answer-mode swap The answer phase gets its own draft key (ask-answer:<actionId>), passed as a draftId override into useAutoSave — the key change itself drives the existing save/restore machinery, so the conversation draft (or mid-run PENDING draft) is stashed when a question pause takes the composer and restored once the user answers, skips, or dismisses. Ask keys are exempt from the PENDING migration branch, which would otherwise move-and-delete the stashed draft. A half-typed answer survives reload/navigation while its question stays live. Answer submission (option pick, free-form, skip) resets the composer via a new non-throwing useOptionalChatFormContext, so the swap-back restores into an empty box even outside ChatView-less render contexts (Share/search). * fix: rebind resumed steps for ALL seeded tool call ids The resume controller pre-stamps the user's answer onto the seeded ask_user_question part, so the unresolved-only rebind predicate treated it as settled and shifted the tool's re-run step to a fresh offset slot, leaving a duplicate ask record in streamed/saved content. Tool call ids are provider-minted per call: a resumed step bearing a seeded id can only be the interrupted batch re-executing, so rebinding every seeded id is always correct. * feat: popover UX round 4 — clickable hint, collapse, click-submit, multiSelect - Footer hint is a button that focuses the composer; reads 'Type your answer below' (no 'Or') when the question has no options. - Collapse (chevron) hides the popover WITHOUT closing the pause: answer mode stays live (placeholder, Enter routing, draft key), the chat card renders the question with a ChevronUp affordance to re-expand. x remains dismiss. - Single-select options submit on a single click; the Submit button renders only for multi-select. - multiSelect end-to-end: tool zod schema + JSON definition twin, wire type, client parse, popover check-chips, card toggles, record-card label mapping; answer = option values joined ', '; composer Enter and the multi Submit button both fold free-form text in with the checked values. - Hardening from adversarial review: in-flight status guard on every submit path (no duplicate resumes on double-click), popover locks while submitting, collapsed mode disarms invisible digit/arrow steering, the card shares the hook's checked state while the pause is live, the card folds catch-all 'Other' options, record mapping is all-or-nothing to avoid phantom labels, composer resets only when its text was consumed or the draft machinery will restore the stash. * feat: ask_user_question in model specs and ephemeral agents A librechat.yaml modelSpec can now equip the tool the same way it equips webSearch/executeCode/fileSearch/memory: modelSpecs: list: - name: my-spec askUserQuestion: true loadEphemeralAgent pushes the tool name when the spec flag (or the ephemeralAgent request flag, wired for parity) is set; everything downstream is the existing persisted-agent machinery — createRun's hitlCapable gating, graphTools injection, checkpointer attach, subagent strip, and the admin filteredTools/includedTools kill switch all apply unchanged. * feat: tense-aware Q&A record label (Asking / Asked) Shorten the record card header per feedback: 'Asking' while the question is still unanswered (abandoned/awaiting), 'Asked' once answered — replacing the single 'Asked a question' label. * fix: Codex round 4 — added-agent ask parity + preserve answer on failed resume F1 (added.ts): mirror loadEphemeralAgent's ask_user_question branch in the added-agent loader so a model spec's askUserQuestion flag (or the ephemeral request flag) equips added top-level agents too, matching execute_code / web_search / memory. Two load.spec cases added. F3 (composer): submitAskAnswer now takes an onSuccess callback and useAskAnswerMode defers clearing the selection/composer until the resume is accepted. A failed resume (16k answer-cap 400, expired action, network error) leaves status re-answerable, so wiping the composer up front lost the user's only copy of a free-form answer; now it survives for trim/retry. (F2 — a claimed Tools-capability bypass — was verified NOT reproducible: agentRequestsAskUserQuestion matches only loaded instances/toolDefinitions/ toolRegistry, all capability-filtered; a raw tools string has no .name and never triggers the install. Replied on-thread with the probe evidence.) * fix: Codex round 5 — expired question exits answer mode so its message shows An expired question (e.g. resume returns the stale-action 409) previously left the popover open with locked controls and no explanation, because the chat card — which carries the only 'this action expired' message — was suppressed by the popover-open guard. Treat 'expired' as no longer active: the popover closes, the composer reverts to normal, and the card becomes the sole surface and renders the expired message. 'error' stays active (retryable). * feat: group ask_user_question calls as their own category A homogeneous group of ask_user_question tool calls now reads 'Asked N questions' (present tense 'Asking N questions' while the turn streams) with a question glyph and no raw-name suffix — mirroring the subagent 'Ran N agents' category treatment, instead of 'Used N tools — ask_user_question'. Mixed groups keep 'Used N tools' but humanize the suffix to 'Question' and show a question icon for the ask entries (TOOL_FRIENDLY_NAME_KEYS + ToolIcon map). A group only forms at count >= 2, so the plural is always grammatical. Three ToolCallGroup.test cases cover homogeneous label/icon/suffix, present tense while streaming, and the mixed-group fallback. * fix: Codex round 6 — composer submit lock + abort stamp before emit F7 (composer status lock): the ask submit status lived on ApprovalContext, a React context mounted only around message content (ContentParts). The PRIMARY answer surface — the composer in ChatForm — renders outside it, so useApprovalContext returned the inert FALLBACK: status was always 'idle', setStatus a no-op. The in-flight double-submit guard (round 4) and the expired-exits-answer-mode fix (round 5) therefore never engaged for the composer. Move ask submit status to a global Recoil atom (useAskSubmitStatus) read/written by the composer, the popover, and the card alike, so a fast double-click/Enter is actually blocked and expired/error surfaces on every surface. Tool-approval status stays on the context (unchanged). F5 (abort stamp before emit): the abort route re-stamped a paused ask_user_question's args AFTER GenerationJobManager.abortJob had already emitted the final SSE from the unstamped content, so a Redis/cross-replica Stop left the live client showing an empty question until reload. abortJob now takes an optional transformAbortContent applied to the persistable content BEFORE the final event is built (and returned), so the live client and the saved message agree. New abort.spec case + updated call assertions. * feat: gate ask_user_question behind its own agent capability Add a first-class AgentCapabilities.ask_user_question (in defaultAgentCapabilities, on by default) so admins can enable/disable questions independently via endpoints.agents.capabilities, exactly like execute_code / web_search — not lumped under the generic tools capability. - ToolService: both filteredTools predicates (definitions-only and instance loaders) gate ask_user_question on checkCapability(ask_user_question) before the generic tools fallthrough. When off, the tool is dropped from toolDefinitions/toolRegistry, so run.ts's agentRequestsAskUserQuestion (which keys on the loaded surface) declines to install it and attach a checkpointer — the capability is enforced end-to-end at the loader, no run.ts change needed. - Tools dialog catalog: surface the ask builtin under its own capability rather than the generic tools one, so the UI matches the backend gate. - Tests: ToolService capability on/off filtering + defaults membership; catalog builtin visibility keyed on the dedicated capability. * style: sort imports in ToolCallGroup.test (CI import-order gate) * fix: Codex round 7 — surface ask-answer errors in the open popover A failed answer submission (16k reject, network error) sets the ask status to 'error', which — unlike 'expired' — deliberately keeps the question active and retryable. But the chat card that renders the error message is suppressed while the popover is open, so a composer/popover answer failed silently. Expose an 'errored' flag from useAskAnswerMode and render a warning line (com_ui_ask_answer_error) in the popover, so the user gets feedback and retry guidance without having to collapse/dismiss. It clears automatically on retry (status flips to 'submitting'). * fix: Codex round 8 — respect IME composition before submitting answers handleComposerKeyDown runs before useTextarea's composition guard, so with a CJK/IME keyboard the Enter that commits an in-progress composition was being intercepted and submitting the partial answer (and the composition buffer can leave value empty mid-compose, mis-triggering digit/arrow steering too). Bail at the top when composing — nativeEvent.isComposing, or key==='Process' / keyCode===229 for Safari's inconsistent reporting — mirroring the existing composer guard so the character commits normally. * chore: update `@librechat/agents` to v3.2.60 * 🔧 chore: Update @opentelemetry/core to version 2.9.0 and clean up package-lock.json * feat: digit shortcuts select options when the popover has focus Previously a number key (1..N) only selected an option from the empty composer (handleComposerKeyDown on the textarea) — if focus moved into the popover (a row/Skip/Submit button clicked or tabbed to), the number keys went dead. Add handlePopoverKeyDown, wired to the popover container's onKeyDown so it catches digits bubbling from the focused control: a digit activates its option exactly like a click (single-select submits, multi toggles). No highlight/Enter dance on this path — the options are buttons whose action is the click, and intercepting Enter would fight the focused button. Gated on active && !locked so it no-ops while a submit is in flight. * chore: update @librechat/agents to version 3.2.61 and @opentelemetry packages to latest versions	2026-07-08 15:31:05 -04:00
Danny Avila	6dbf9d5ad3	🪝 feat: Human-in-the-Loop Runtime - Tool Approval + Ask-User-Question (Slice B) (#13942) ... * chore: add @langchain/langgraph-checkpoint-mongodb for HITL durable resume * feat: HITL tool approval runtime — backend (Slice B) - endpoints.agents.checkpointer config + durable Mongo checkpointer (seam over the app connection; SDK MemorySaver fallback) with a TTL index + deleteThread pruning - HITL run wiring (PreToolUse policy hook + humanInTheLoop) attached in createRun, fully inert when toolApproval.enabled is off - interrupt gate (pause job -> requires_action + emit on_pending_action) and a resume route that rebuilds the run from the durable checkpoint and run.resume()s it - atomic single-winner resolve; agent-consistency guard; expireStaleApprovals terminal event; checkpoint pruned on every non-paused completion (thread_id == conversationId) * feat: HITL tool approval UI — frontend (Slice B) approve/reject/edit/respond + ask-user controls in the tool card (OAuth-button precedent), batch-aware single submit, live + reconnect (resumeState.pendingAction) wiring, and resume mutations posting to /agents/chat/resume. * fix(hitl): decouple ApprovalProvider from chat context ApprovalProvider is now pure state (safe to mount in provider-less / shared / test renders); the context-dependent submit moved to a useResumeSubmit hook the cards call. Part imports getAskUserQuestionPart from ~/utils/approval directly so suites that partial-mock ~/utils render Part without throwing. * fix(hitl): address Codex review — backend - P1: enforce per-tool allowed_decisions on resume (reject a crafted decision the policy disallows) via findDisallowedDecisions - prune the durable checkpoint on user-abort of a paused run, and before a fresh HITL turn, so a new turn cannot rehydrate an expired/aborted interrupt (thread_id is the stable conversationId) - persist + use isTemporary and the original parentMessageId on resume (temporary chats stay temporary; initializeAgent scopes thread files off the right parent) - generate a deferred first-turn title BEFORE completeJob so its event reaches the client and the final event carries the real title - moderateText: skip when there is no text (tool-approval resume) and moderate the ask-user answer, instead of denying on an empty input * fix(hitl): address Codex review — frontend - render ToolApproval for ANY paused agent tool card (bash/code/file/etc.), not just the generic ToolCall, by wrapping the tool-card branch in Part (moved the rendering out of ToolCall) - findPendingActionMessageIndex only matches an assistant message, never the user message (the underscore-strip could target the user bubble before the assistant placeholder exists) * fix(hitl): address Codex re-review - title eligibility checks the user message’s parent (first turn), not the response’s parent — the previous check could never be true and skipped title generation - use client.buildResponseMetadata() for the resumed message so contextUsage / thoughtSignatures survive (the abort-only helper dropped them) - moderate decisions[].responseText (the respond action’s user text) - give /chat/abort req.config (configMiddleware) so the HITL checkpoint prune on abort actually runs - read resume state BEFORE setContentParts so the in-memory store does not lose the pre-pause seed content - count resumes against LIMIT_CONCURRENT_MESSAGES (increment/decrement) so paused-then- resumed turns cannot bypass the limit - require actionId on resume so a body without it cannot resolve the current action * fix(hitl): address Codex re-review (round 3) — resume fidelity Bring the lean resume path to parity with sendMessage for things it bypassed: - carry userMCPAuthMap into the rebuilt run so approved MCP tools keep the user's creds - seed initialSessions (buildInitialToolSessions) so approved code/file/skill tools have the pre-pause uploaded-file context (esp. cross-replica / after restart) - await client.artifactPromises and persist them as response attachments (else tool artifacts created after the pause vanish on reload / for late subscribers) - merge metadata: cumulative usage (+ summary marker) from the job, contextUsage / thoughtSignatures from the client — fixes the round-2 regression that underreported post-resume cost * fix(hitl): address Codex re-review (round 4) — resume hardening - resume: require an EXACT paused agent_id match (reject omitted/ephemeral agent_id, not just a different one) and reject an endpoint mismatch, so a request can't rebuild the claimed checkpoint on a different graph - moderateText: also moderate a tool-approval decision's reject `reason` and stringified `editedArguments`, not just `responseText` - request: re-mark the paused response `unfinished:true` after BaseClient saves it as completed, so an expired / never-resumed approval doesn't leave a "finished" response in history; the resume path overwrites it on success * test(hitl): route-level integration test for the resume controller Adds api/server/controllers/agents/__tests__/resume.spec.js, a supertest integration test that drives the real ResumeAgentController over the full pause -> approve -> resume -> finalize lifecycle with the SDK run, durable checkpointer, Mongo, and concurrency cache mocked. The pure decision/liveness helpers run for real via requireActual, so the guard ladder is exercised end to end rather than stubbed. 25 cases covering: - the authorization / staleness / agent-and-endpoint / actionId guard ladder - tool_approval validation (undecided tool call, policy-disallowed decision) - ask_user_question answer requirement - the concurrency gate (429) and the atomic single-winner claim (409) - the happy path: ACK, run reconstruction, decision->SDK mapping, finalize (save the now-finished response, emit done, complete job, prune checkpoint) - first-turn title generation before stream completion - re-pause (no double finalize), abort-during-resume (no double finalize), and the resume-failure terminal path (emitError + completeJob + prune) * test(hitl): strengthen resume coverage + add approval util tests Acts on a self-audit of the new resume integration test. resume.spec.js (25 -> 32 cases): - replace the tautological emitDone assertion (it only checked the hardcoded `final: true`) with a structural check of the finalEvent payload — responseMessage content/id/unfinished, requestMessage identity, title - cover the previously-unwalked finalize branches: tool-artifact attachments (null-filtered), the aggregatedContent fallback when live content is empty, and client response-metadata attachment - add guard cases: unsupported pending-action type (400) and the pre-multi-tenancy null-tenantId pass-through (must not 403) - add error-path cases: first-turn title generation throwing must still finalize, and a completeJob failure during a resume error must force a terminal job state via the last-resort updateJob client/src/utils/approval.spec.ts (new, 15 cases): - applyPendingAction tool_approval: join by tool_call_id not position, skip completed calls, default allowed_decisions to [], referential stability when nothing changes - applyPendingAction ask_user_question: append, idempotent replace on replay, non-array content coercion - getAskUserQuestionPart type guard; findPendingActionMessageIndex assistant-only resolution (never resolves to the user bubble) * fix(hitl): address Codex re-review (round 5) Five findings verified against the code before fixing: - resume: require an EXACT endpoint match (like agent_id) — a resume that OMITS endpoint must not fall through, since the shared chat middleware treats a missing/non-agents endpoint as the ephemeral agent and could rebuild the claimed checkpoint on a different graph - resume: filter malformed content parts before saving the finished response, matching the normal AgentClient path (a resumed turn could otherwise persist an empty/invalid tool_call part that breaks reload/rendering) - resume: accumulate tool artifacts across pause segments — persist them on re-pause and MERGE (not overwrite) at finalize, so artifacts produced before a second approval pause aren't dropped by the next rebuilt client - approval (client): findPendingActionMessageIndex returns -1 when a provided responseMessageId isn't found, so the caller retries instead of attaching the prompt/approval to a prior assistant reply; fall back to the last assistant only when no responseMessageId is given - RedisJobStore: make appendChunk extend-only (XADD + EXPIRE-if-shorter via a single eval) so the on_pending_action chunk emitted after a pause can't reset the chunk-stream TTL back to the running window and evict pre-pause content before the approval is resolved Tests: +endpoint-omitted/unsupported-type/malformed-filter/attachment-merge/ re-pause-persist cases in resume.spec.js (36); ask-retry -1 semantics in approval.spec.ts (16); extend-only TTL assertion in the RedisJobStore Redis integration spec. * test(hitl): mongodb-memory-server integration test for the checkpointer seam The checkpointer unit spec covers config/selection with no DB connection; this exercises the durable Mongo seam against a real (in-memory) MongoDB — the part correctness actually depends on: - getAgentCheckpointer builds a real MongoDBSaver when Mongo is connected and setup() creates the TTL index (expireAfterSeconds) on the checkpoint collection - memory type returns undefined (SDK MemorySaver fallback) even when connected - saver is memoized per resolved config - deleteAgentCheckpoint prunes a thread's persisted checkpoint (the cross-turn isolation guarantee: turn N+1 on the same conversationId can't rehydrate it) - pruning is thread-scoped — deleting one conversation leaves others intact - undefined threadId is a no-op * fix(hitl): address Codex re-review (round 6) Four findings verified against the code before fixing: - messageFilterPii: scan the resume payload's user-authored text (ask-user `answer`, and a tool-approval decision's `respond` text, `reject` reason, and edited tool arguments) — the shared /resume route ran through the PII filter but it only inspected req.body.text, so a blocked token rode the resume payload back into the model/tool (mirrors the earlier moderateText fix) - resume: re-prime skill files invoked in the pre-pause segment before rebuilding the run, so an approved code/file-backed tool keeps the injected skill-file session refs instead of running without them (mirrors the normal path's primeInvokedSkills; the pre-pause content stands in for the message payload) - hitl: pin the graph identity. Persist a fingerprint of the graph-determining request fields (endpoint, agent_id, model, spec, ephemeralAgent — normalized) on the pending action at pause, and reject a resume whose recomputed fingerprint differs. This closes the ephemeral-agent gap, where agent_id is undefined so the id guard can't tell two ephemeral configs apart - resume: reject incomplete edit/respond decisions (findIncompleteDecisions) — an `edit` without an object editedArguments or a `respond` without non-empty responseText is 400'd before mapping, rather than defaulting to {} / '' and resuming with behavior the user never approved Tests: incomplete-decision + fingerprint match/mismatch cases in resume.spec.js (41); findIncompleteDecisions + computeAgentRequestFingerprint unit tests; and resume-field PII cases in messageFilterPii.spec.ts. * fix(hitl): address Codex re-review (round 7) Four findings verified against the code before fixing: - RedisJobStore: clear `agent_id` on createJob (add it to staleHitlFields). The job hash is keyed by conversationId and reused across turns; updateMetadata only writes agent_id when truthy, so a conversation that switched from a saved agent to an ephemeral/no-agent turn kept the old id and the resume guard rejected the valid pause as a different agent. (real correctness bug) - fingerprint: include `promptPrefix` in computeAgentRequestFingerprint, and re-send it on resume (ResumeAgentFields + buildResumeFields). Ephemeral agents derive their system instructions from promptPrefix, so a resume changing it previously passed the pin and rebuilt different instructions. (completes the round-6 fingerprint) - resume: the re-pause branch now persists the segment's accumulated CONTENT (filtered), not just artifacts, so an approval that expires/reaps without a final resume no longer loses everything streamed during the resumed segment. - request: carry `manualSkills`/`alwaysAppliedSkills` on the persisted user message so a resumed turn's reconstructed requestMessage keeps its skill pills instead of dropping them until a full reload. Deferred (narrow, no safe contained fix yet — see PR thread replies): - resume rebuild without `addedConvo` for a multi-conversation/added-agent pane - cross-replica re-prime of manually-selected (not model-invoked) skill files Tests: stale-agent createJob clearing (Redis integration), promptPrefix fingerprint match/mismatch (resume.spec.js + policy.spec.ts), re-pause content persistence (resume.spec.js). * fix(hitl): address Codex re-review (round 8) Five findings verified against the code before fixing; the headline is a durable- resume correctness fix (the fingerprint had surfaced it as a 403): - resume durability (the important one): persist the graph-determining request fields (endpoint, agent_id, model, spec, promptPrefix, ephemeralAgent) on the pending action as `resumeContext`, and REPLAY them onto the resume request via a router-level middleware that runs before buildEndpointOption. The client can't reconstruct the ephemeral-agent config after a reload/cross-session, so the round-6/7 fingerprint would 403 a valid durable resume — and even without it the rebuilt agent would lose its tools. Replaying server-side rebuilds the SAME graph regardless of client state (and a crafted resume can't swap it; the fingerprint still matches because the body is restored first). - RedisJobStore: also clear `isTemporary` on createJob (same class as agent_id): a prior temporary turn's flag would otherwise survive a reused conversation hash and a later non-temporary resume would save its response as temporary. - resume: persist `contextMeta` (context-window calibration) onto the saved response like BaseClient does, so the next turn can seed its pruner. - request: carry manualSkills/alwaysAppliedSkills into the onStart metadata update (not just the preliminary one it overwrites), so a resumed turn's requestMessage keeps its skill pills. Deferred (narrow — see thread reply): - saved-agent edited WHILE a run is paused: agent_id matches but the definition changed; needs an agent version/config hash, which is a larger change for a narrow window. Tests: resumeContext pick/apply + round-trip (policy.spec.ts), contextMeta + manualSkills-on-requestMessage (resume.spec.js), isTemporary clearing (Redis integration). * style(hitl): prettier line-wrap in policy.spec.ts (R8 lint fix) * fix(hitl): address Codex re-review (round 9) Five findings, all fixed (addedConvo — deferred in rounds 7/8 — is now trivial thanks to the round-8 replay): - replay addedConvo: add it to RESUME_CONTEXT_KEYS so the resume middleware restores the parallel/secondary-agent config from the paused request; the client can't reconstruct it, and it determines the rebuilt graph. - skill pills (the real fix this time): the round-8 onStart metadata write was overwritten by trackUserMessage (the authoritative userMessage writer). Carry manualSkills/alwaysAppliedSkills in the emitted `created` message and persist them in trackUserMessage; widen UserMessageMeta + SerializableJobData.userMessage. - execute-code files on resume: seed the paused user message's own files onto req.body.files before initializeClient — they're excluded from the parent-walk code-session rebuild, so an approved code/read-file tool would otherwise resume without them. - in-memory pending-action UI: route ApprovalEvents.ON_PENDING_ACTION in the resume replay/pending-event loops to applyPendingActionToMessages (mirror the live handler), so a pause that lands in the snapshot window still renders its approval controls instead of sitting paused with no UI. - abort isTemporary: the /chat/abort partial-save now sources isTemporary from the job metadata, not req.body (the stop button posts only conversationId), so aborting a paused temporary chat no longer persists an orphaned partial. Tests: addedConvo in pickResumeContext (policy.spec.ts), file-restore on resume (resume.spec.js), abort-from-job-isTemporary (abort.spec.js). * fix(hitl): address Codex re-review (round 10) — resume/expiry races Three concurrency/coherence findings, verified against the code before fixing: - expiry-sweep CAS scope: both stale-approval sweeps (GenerationJobManager expireStaleApprovals and the RedisJobStore requires_action cleanup) called expire()/transitionStatus WITHOUT the observed pendingAction.actionId, so the CAS only checked status===requires_action. Between the read and the CAS a user could resolve the observed action and the run re-pause on a FRESH action; the stale sweep would then abort that valid new pause. Now both pass the observed actionId as expectActionId, so the CAS only fires for the action read as stale (a re-paused action has a different id → no-op). - resume graph cache: resumeCompletion cached the rebuilt graph (created with messages:[]) via setGraph; RedisJobStore.getContentParts prefers a cached graph over reconstructing from the chunk log, so a same-replica reload/status poll mid-resume returned aggregatedContent missing the pre-pause content. Skip setGraph on resume so introspection falls back to the complete chunk reconstruction (setContentParts still seeds the in-memory store). - pending-action UI: applyPendingActionToMessages scheduled a SINGLE animation-frame retry then dropped the pending action; Recoil/React updates can take several frames under load, leaving a valid requires_action run with no approval controls. Retry across frames (bounded at 120) until the target message commits. Test: expire() with a mismatched expectedActionId no-ops while the matching id expires (pendingAction.spec.ts). * chore(deps): update @librechat/agents to version 3.2.53 and @langchain/langgraph to version 1.4.7 in package-lock.json and related package.json files * refactor(hitl): add resolveToolApprovalPolicy seam for layered policy Extract the single point where tool-approval policy is resolved for a turn (`resolveToolApprovalPolicy`) and route the run call site through it instead of reading `endpoints.agents.toolApproval` inline. Behaviour-preserving: only the `endpoint` layer is wired today, so the result is identical to reading the app policy directly. The `agent` and `skills` layers are reserved seams with documented precedence (endpoint owns the `enabled` kill switch; agent overrides mode/allow/deny/ask/reason; skills may only tighten), so future per-agent and per-skill policy plumbing lands in one function rather than at the `createRun` site. Adds focused unit tests. * fix(hitl): address Codex re-review (round 11) — resume hardening F1 (P2, security) — applyResumeContext now DELETES any RESUME_CONTEXT_KEY absent from the persisted context, so the resume body carries exactly the graph-determining fields the pause had. Previously only defined keys were overwritten, leaving a client-supplied `addedConvo` (which the request fingerprint does not cover) in place — a crafted resume could rebuild a single-agent checkpoint as a different multi-agent graph/tool set. F3 (P2) — the resume route ACKs (res.json) before initializeClient, so a post-ACK getMCPRequestContext(req, res) saw the response as finished and returned undefined, leaving the resumed run without its run-scoped MCP connection store (approved MCP / OAuth-overlay tools then ran without their request-scoped connections). Pre-seed the store with a null res + cleanupOnResponse:false before the ACK and tear it down in the finally, mirroring the normal stream path (request.js). userMCPAuthMap was already preserved separately, so credentials were not lost — only the connection store. Declined: the ApprovalContext NEW_CONVO guard (P2) is a false positive — the `created` SSE event updates the conversation atom before any pause renders, so the id is concrete by click time (details in the PR thread). Tests: policy.spec (absent-key delete) + resume.spec (MCP context pre-seed/cleanup order). * fix(hitl): address Codex re-review (round 12) — resume fidelity + multi-tool UI F4 (P2) — temporal prompt vars: resume rebuilt the agent without restoring req.conversationCreatedAt or req.body.timezone, so {{current_datetime}}-style vars compiled a different system prompt than the paused graph (resume wall-clock, unzoned). Add 'timezone' to RESUME_CONTEXT_KEYS (persisted at pause, replayed by the resume middleware) and restore conversationCreatedAt from the convo before initializeClient — mirroring the normal path's resolveConversationCreatedAt. F5 (P2) — multi-tool approval: applyPendingActionToMessages stopped retrying once ANY tool-call part was tagged, so siblings that rendered on later frames never got approval controls and the resume route 400'd the partial batch. Add countTaggedApprovalParts and keep the bounded RAF retry going until every action_request is tagged (ask_user_question unchanged — one synthetic part). F6 (P3) — Edit accepted `null`/`[]` (valid JSON, non-object), enabling Submit for a value the resume route rejects via findIncompleteDecisions. Mirror the server's plain-object check in the client (store + editIsValid) so Submit only enables for an accepted value. Tests: policy.spec (timezone round-trip), resume.spec (conversationCreatedAt restore), approval.spec (countTaggedApprovalParts). * fix(hitl): address Codex re-review (round 13) — recurse into subagent approvals F9 (P2) — a tool paused INSIDE a subagent has its tool_call_id in the parent subagent tool_call's nested `subagent_content`, not as a top-level message part. applyToolApproval and countTaggedApprovalParts only scanned top-level content, so the approval never attached and the round-12 retry loop counted 0 tagged parts and spun to its frame cap with no controls. Both now recurse into `subagent_content` (immutably, so React refs update): the nested call gets tagged and is counted, so the retry terminates. Added approval.spec cases for the nested tag + count. Note: surfacing the interactive approve/reject controls inside the subagent view is a deliberate follow-up — ToolApproval -> useResumeSubmit -> useChatContext crashes when rendered in the portaled subagent dialog (outside the chat/approval providers), so that needs the controls scoped to the in-provider inline render (or the dialog wrapped with the providers). This commit fixes the data/traversal layer only. F7 (discovered-tool history on resume) and F8 (redis chunk TTL pause race) were verified false positives — see the PR threads. * fix(hitl): address Codex re-review (round 14) — resume fidelity + expiry relay F13 (P2) — manualSkills are graph-determining (skill allowed-tools union into the tool set before tools load) but weren't replayed, so a reload lost the skill tools and a crafted resume could inject a different skill past the fingerprint. Add 'manualSkills' to RESUME_CONTEXT_KEYS (same replay-only pattern as timezone/ addedConvo; the delete-absent half blocks injection). Not alwaysAppliedSkills — that's resolved server-side from the DB, not req.body. F12 (P2) — the resume final SSE built requestMessage from job.metadata.userMessage (persisted without files), so attachments vanished from the user bubble on resume. Spread the already-restored req.body.files onto it, matching the normal path. F11 (P2) — multi-replica approval expiry: RedisJobStore.cleanupRequiresActionIndex on another replica can win the requires_action->aborted CAS (it sets the hash error but has no event transport), and the local sweep then skips because the job is no longer requires_action, so a client subscribed here never gets the terminal error until the reap path. expireStaleApprovals now relays APPROVAL_EXPIRED_ERROR for a locally-subscribed job already aborted FOR approval expiry (error-string gated, idempotent via the errorEvent flag). emitError already publishes cross-replica. Tests: policy.spec (manualSkills round-trip + inject-drop), resume.spec (final requestMessage carries restored files). * fix(hitl): render approval controls for subagent-nested tool pauses (F10) Round-13 made applyToolApproval/countTaggedApprovalParts recurse into subagent_content (data), but SubagentDialogPart rendered nested TOOL_CALL parts with <ToolCall> only and never mounted <ToolApproval>, so a tool paused inside a subagent showed no controls and the run was unresolvable. Render <ToolApproval> in SubagentDialogPart's TOOL_CALL branch when the nested tool_call carries an approval and isn't yet resolved, mirroring the top-level Part.tsx render. The subagent dialog portals (OGDialog → ReactDOM.createPortal), but React context flows through the React tree, not the DOM tree, so ToolApproval resolves ApprovalProvider/ChatContext and the controls work + submit. Also harden useResumeSubmit: read ChatContext via useContext (non-throwing) instead of the throwing useChatContext wrapper, so the cards never crash when rendered outside a ChatContext.Provider (e.g. a search/citation render that passes chat context as a prop) — they degrade to inert (buildResumeFields returns null). * style(hitl): re-sort run.ts imports after dev rebase * fix(hitl): address Codex re-review (round 15) — resume content fidelity F14 (P2) — hide_sequential_outputs was applied in chatCompletion before saving/emitting content but not on resume, so a sequential-agent chain that pauses for HITL and resumes persisted/emitted intermediate outputs the setting is meant to hide. Extracted the filter into applyHideSequentialOutputsFilter() and call it from both chatCompletion and resumeCompletion (after handleRunInterrupt, covering the finalize + re-pause reads of client.contentParts). F16 (P2) — on a reloaded HITL pause, the DB already holds the paused user row + partial assistant row; useResumeOnLoad fed those as submission.messages, then finalHandler/createdHandler appended the same pair via requestMessage/responseMessage, duplicating the turn (buildTree doesn't dedupe children by messageId). buildSubmission- FromResumeState now strips the paused user/response rows (by messageId, incl. the padded/unpadded response id) from submission.messages — they're re-supplied by the placeholders + final event. Frontend-only; live (non-reload) pause path untouched. Deferred: F15 (collapsed-card subagent approval registration/visibility) — see thread. Tests: client.test (filter keeps last + tool_call parts / no-op when off), useResumeOnLoad.spec (paused pair stripped from submission.messages). * fix(hitl): address Codex re-review (round 16) — chunk TTL, slot, job replacement F17 (P2) — chunk-stream TTL on pause-before-chunk. CHUNK_APPEND_LUA derived its ceiling only from the chunk key's current TTL, so when the chunks key didn't exist at pause (fire-and-forget append in flight, or an ask-user pause before any chunk), the on_pending_action append created the stream with only the 20m running TTL while the approval window is 24h — content evicted before resume. The Lua now also reads the job key (KEYS[2]); when status == requires_action it takes max(running, TTL(jobKey)) (the approval window transitionStatus set), else the running TTL. Extend-only preserved; gated on paused status so normal runs never inflate. Both keys share {streamId} (cluster-safe). F19 (P2) — with LIMIT_CONCURRENT_MESSAGES, the approval prompt was emitted before the original request released its slot, so a fast Approve got /resume 429'd. handleRunInterrupt now releases the slot (idempotent via pendingRequestReleased) right after the pause, before the prompt; the request.js pause branch and resume.js finally only release if it didn't (no double-release). F20 (P2) — finalizeResumedTurn never checked the job wasn't replaced before emitDone/ completeJob/saveMessage, so a stale resume could clobber a newer turn that reused the conversationId. Added the createdAt guard the normal request path uses (skip finalization when the live job's createdAt != the paused job's). Deferred: F18 (subagent_content not reconstructed on Redis resume) — joins the subagent cluster (F15). See thread. Tests: RedisJobStore integration (pause-before-chunk gets approval TTL; running stays short), resume.spec (skip finalization on replacement; no double slot release on re-pause). * 🛡️ fix: Guard HITL terminal side-effects against job replacement Jobs are keyed by streamId == conversationId, so a new request REPLACES the running one on the same conversation. The replaced generation's tail must not clobber the live generation's state. Each path now re-reads the live job and compares createdAt against the generation's captured identity before acting. - Thread the generation's createdAt onto the client (request.js + resume.js) as client.jobCreatedAt — the identity every guard compares against. - handleRunInterrupt: skip approvals.pause when this run is no longer the live job, so a stale interrupt can't flip the NEWER job to requires_action. - chatCompletion finally: skip the checkpoint prune when replaced, so an older run's late finally can't delete the newer run's resume checkpoint. - resume catch-path: gate emitError/completeJob/prune behind a stillLive check (fail-open if the read throws), mirroring finalizeResumedTurn's success guard. - Persist the turn's uploaded files on job.metadata.userMessage (authoritative trackUserMessage writer) and prefer them on resume over the user DB row, whose save can still be racing a fast /resume. Tests: 13 guard-predicate cases in jobReplacement.spec.js. * 🔁 fix: Harden HITL resume — ownership re-check, file seeding, deferred-tool replay Three follow-ups to the round-17 job-replacement guards (Codex review 4594099963): - G1 (resume.js): the success-path ownership guard runs at the START of finalizeResumedTurn, but saveMessage + first-turn title generation await long enough for a new request to replace the job on the same conversationId. Re-read the live job immediately before emitDone/completeJob/prune so the terminal writes can't tear down the REPLACEMENT job — mirrors the catch-path guard. - G2 (request.js): onStart's metadata/chunk writes that persist the turn's files are fire-and-forget, so a fast approval could read job.metadata.userMessage before files landed. Seed files into getPreliminaryUserMessage instead — that write is AWAITED before the run starts, so files are durable before any interrupt can emit. - G3 (run.ts + client.js + resume.js + IJobStore.ts): the resumed graph is rebuilt with messages: [], so createRun's tool_search-discovery scan finds nothing. A deferred tool discovered earlier in the turn (and targeted by the paused call) was therefore absent from the rebuilt schema-only toolMap — resume would throw "unknown tool" (no loadRuntimeTools fallback is wired). Capture discovered tool names at pause via extractDiscoveredToolsFromHistory(run.getRunMessages()), persist them on job.metadata.discoveredTools, and replay them into createRun's new discoveredToolNames input (merged with message-extracted names, gated on hasAnyDeferredTools — inert otherwise). A new createRun test proves the deferred tool is promoted with the replay and absent without it (reproducing the bug). Tests: real createRun deferred-replay suite (run-summarization.test.ts) + G1/G2/G3 guard predicates (jobReplacement.spec.js). Full suite green. * 🔒 fix: Close HITL resume metadata + file-substitution + pause-race gaps Four findings on the round-18 commit (Codex review 4594430222): - H1 (P1, regression in round-18 G3): the discoveredTools captured at pause never reached resume — three metadata allowlists dropped it: GenerationJobManager .updateMetadata, RedisJobStore.deserializeJob, and buildJobFacade (plus the GenerationJobMetadata type). Added discoveredTools to all four, so the deferred-tool replay actually works end-to-end (in-memory store already kept it via Object.assign). - H2 (P2, security): /resume honored a client-supplied `files` array, letting a crafted client resume an approved code/read-file tool against a DIFFERENT file set than the one approved (files aren't in the resume fingerprint/context). Resume now ALWAYS sources files from the paused job (metadata → DB row), clearing any client-supplied set. - H3 (P2, ephemeral fidelity): non-default model parameters (temperature, max tokens, custom endpoint params) were lost on resume — ephemeral agents derive them from the request body, which the resume payload omits. Capture the resolved model_parameters in resumeContext at pause and replay them onto the body on resume (excluding `model`, which is replayed via the fingerprinted RESUME_CONTEXT_KEYS path). Saved agents already source these from the DB. - H4 (P2, Redis race): a pause landing between the resume snapshot and the Pub/Sub subscription reached neither resumeState.pendingAction nor (Redis) pendingEvents, and approval events aren't persisted to replayEvents — the client attached to a paused job with no approval UI. subscribeWithResume now re-reads the live job AFTER subscribing and surfaces the pending action if the snapshot missed it (live read, no staleness). Tests: discoveredTools metadata round-trip + subscribeWithResume re-read (pendingAction .spec.ts); client-file substitution rejection (resume.spec.js); model-parameter replay predicate (jobReplacement.spec.js). * 🧹 fix: Clear stale discovered tools, release slot on claim error, extend run-step TTL Three follow-ups on the round-19 commit (Codex review 4594783691): - I1 (P2): the round-19 discoveredTools field wasn't cleared on Redis streamId reuse. HSET only overwrites listed fields and handleRunInterrupt only writes discoveredTools when THIS turn discovers a deferred tool — so a replacement turn that pauses without its own discovery inherited the prior run's tool names and force-loaded undiscovered deferred tools on resume. Added discoveredTools to createJob's staleHitlFields HDEL list (the in-memory store already builds a fresh object, so it was Redis-only). - I2 (P2): with LIMIT_CONCURRENT_MESSAGES, approvals.resolve runs after the slot increment but before the run's try/finally, so a store/Redis error there leaked the slot until the counter TTL expired (spurious 429s on retry of the still-paused approval). Wrapped the claim in try/catch that decrements the slot and returns 500. - I3 (P3): saveRunSteps did SET ... EX running unconditionally, resetting the run-steps key to the 20-min running TTL even while the job is paused for the longer approval window — a reload after that window lost the tool timeline. Now uses a paused-window TTL script mirroring the chunk-stream no-shrink behavior (extends to the approval window when the job hash is requires_action). Also fixes a latent strict-tsc cast error in the round-19 pendingAction test. Tests: claim-throws-releases-slot (resume.spec.js); discoveredTools cleared on reuse + saveRunSteps preserves the paused TTL (RedisJobStore integration, USE_REDIS). * 🛡️ fix: Guard fast-resume save race, gate HITL to resumable routes, expire on stale submit Three findings on the round-20 commit (Codex review 4595045652): - J2 (P1): a fast /resume can claim + finalize the COMPLETED response while the original request's pause branch is still awaiting `response.databasePromise`; the later unfinished-save then overwrites the completed content. Re-check the job is still paused on THIS generation's action (a claim leaves requires_action; a replacement bumps createdAt) before marking the row unfinished; fail open on a read error. - J3 (P1): the tool-approval wiring (humanInTheLoop + PreToolUse hook + checkpointer) was applied to EVERY createRun caller when toolApproval.enabled, but the OpenAI-compatible and Responses controllers never inspect run.getInterrupt() or persist a pending action — an approval-gated tool would pause there with no approval surface or resume endpoint and the route would emit a normal final response / [DONE] with the tool call dangling. Gate the wiring on a new createRun `hitlCapable` flag, set only by AgentClient (chat + resume). - J4 (P2): a stale-action 409 on submit returned without driving expiry, leaving the job requires_action with a dead action until the periodic sweeper ran — any attached SSE client got no terminal event and the stream appeared to hang. Extracted GenerationJobManager .expireApproval(streamId, actionId) (expire CAS + terminal SSE, shared with the sweeper) and call it from the resume route when the observed action is stale. J1 (nested subagent approval controls not mounting while the details dialog is closed) is a valid frontend issue in the deferred subagent-HITL path — tracked separately (replied on the thread) since the fix touches the shared dialog primitive and needs UI verification. Tests: HITL-gate both directions (run-summarization.test.ts); expire-on-stale-submit (resume.spec.js); fast-resume unfinished-save guard predicate (jobReplacement.spec.js). * 💄 style: Wrap captureAgents signature to satisfy prettier (CI lint)	2026-06-29 16:56:41 -04:00