fix(#621): supervisor status injection during in-flight tool call wedges interactive session - #622
Conversation
…621) The supervisor batch-end epilogue appends display banners via pi.sendMessage(..., {triggerTurn:false}). In pi's sendCustomMessage, {triggerTurn:false} always takes the branch that immediately appends a `custom` entry to the session tree at the current leaf — even while the interactive agent is streaming. When a tool call is in flight (assistant tool_use appended, tool_result not yet appended), that append splices a user-role custom message between the tool_use and its tool_result. The next request then fails with an Anthropic 400 ("tool_result must have a corresponding tool_use in the previous message"), permanently wedging the session. Reproduced repeatedly, under both auto and supervised modes (the integration-skipped banner fires for either). Fix: gate the batch-end epilogue on interactive-agent idleness. - Add SupervisorNoticeGate (supervisor-dispatch.ts): runs the epilogue immediately when idle (leaf is terminal — safe, banner shows now), else defers it to the next `agent_settled` boundary, when all tool_results have been appended. Session-scoped, generation-tagged (a newer batch invalidates stale pending work), coalescing, and disabled on shutdown. deliverAs:"nextTurn" is unusable here — it neither persists nor displays the entry, so banners would be silently dropped. - Extract stopBatchMonitoring() from transitionToRoutingMode() and call it eagerly when deferring, so a heartbeat timer send can't splice during the defer window. - Wire the gate in extension.ts: one agent_settled flush (re-checks isIdle() — another extension may have started a run), one session_shutdown clear, invalidate + generation bump on new batch, and route both batch-end callbacks (doOrchStart / doOrchResume) through it via a shared runSupervisorBatchEndEpilogue() helper. - Tests: supervisor-dispatch.test.ts (idle-immediate, busy-defer, settle-flush, busy-no-flush, generation supersede, invalidate, coalesce, dispose, flush-once). Follow-up (not in this change): other background {triggerTurn:false} sends (heartbeat/status, integration progress/result) can splice via the same mechanism and should also route through the gate. Also note extension.ts still registers the (now-obsolete in current pi) `session_end` event; the gate uses `session_shutdown`.
#621) Defense-in-depth follow-up to the batch-end epilogue gate. The gate only covers the batch-end monitor callbacks, but the supervisor has many other background pi.sendMessage(..., {triggerTurn:false}) sites (integration progress/result, heartbeat, routing) that can splice a `custom` message between an assistant tool_use and its tool_result the same way. Observed in the wild: `supervisor-batch-summary` injected during an in-flight `orch_integrate` tool call (the integration-result path), which the gate does not wrap. Rather than gate every send site, repair the ORDERING of the outgoing message array on the pi `context` event, which fires before every provider request (transformContext, on the pi-internal AgentMessage[] before convertToLlm). repairToolResultOrdering() pulls each assistant's tool results to immediately follow it (in tool-call order) and relocates any spliced-in custom/user messages to after the tool-result group. The request is therefore always valid regardless of where a stray message was appended, so a mistimed injection from ANY source can no longer wedge the session. - context-repair.ts: pure repairToolResultOrdering() (returns the same array when already well-formed; never mutates input); registered as a `context` handler in extension.ts. - Only transforms the per-request context — the persisted session tree is untouched, so it is idempotent and self-correcting across reloads. - Tests: context-repair.test.ts (well-formed no-op, single/multi splice, parallel-group preservation + intra-group splice, multiple splices, unanswered tool_use left as-is, idempotency, tiny arrays). Together with the epilogue gate this closes the whole class: the gate keeps the persisted tree clean for the common path, and the context repair guarantees every outgoing request is valid regardless of send site.
Recurrence on a non-batch-end path → added a comprehensive in-memory repairAfter the epilogue gate landed, the 400 recurred with Rather than chase each send site, PR #622 now also repairs ordering on the pi Net: the epilogue gate keeps the persisted tree clean for the common path, and the |
…, changelog Addresses all findings from the Sage strategy + code reviews of the #621 two-layer fix. Fix 1 — /orch-resume supersession gap (Sage: real lifecycle gap): doOrchStart bumped batchGeneration + invalidated the notice gate on restart, but doOrchResume did not. An epilogue deferred mid-tool by the prior batch kept the same generation, so if the user resumed before agent_settled flushed it, onSettled() matched and fired the stale epilogue against the resumed batch. Both paths now call a shared supersedeDeferredEpilogue() helper (extracted per Sage follow-up so the two entry points can't drift apart again — that drift WAS this bug). Fix 2 — repairToolResultOrdering hardening (Sage: edge-case robustness): - Map<string,T[]> queue instead of last-wins single, so duplicate toolResults sharing a toolCallId are never dropped (was silent data loss). - emitted tracked by message identity (Set<T>) not by id. - New owner-index pre-pass repairs the result-before-assistant shape (a toolResult whose owning assistant appears later is held and pulled forward at the owner). - Safety-net final pass guarantees no toolResult is ever dropped. - toolUseIds() guards null/undefined. No-op reference short-circuit and idempotency preserved. Fix 3 — CHANGELOG [Unreleased] entry documenting both layers, the Sage hardening, and the behavior note that auto-integration may now start at the agent_settled boundary (bounded latency) rather than instantly. Tests: context-repair.test.ts +4 (duplicate-no-data-loss, grouped-dup no-op, result-before-assistant, compound result-before-assistant+parallel +splice). supervisor-dispatch.test.ts +2 (FAILURE MODE: resume w/o invalidate fires stale epilogue; FIXED: resume with supersede drops it). Full suite 3747 pass / 0 fail / 1 skip. typecheck + lint (baseline) + format all green. Sage code review: no blocking issues, confidence ~0.9.
Updated: Sage review remediations applied (commit 4b2cc8d)Ran a Sage strategy review + code review of the two-layer fix. Applied all findings:
Tests: +24 total across Sage code review verdict: no blocking issues, confidence ~0.9, release-ready. Validation: typecheck ✅ · lint ✅ (286/671 baseline) · format ✅ · full suite ✅ 3747 pass / 0 fail / 1 skip · CLI smoke ✅ |
Fixes #621.
Problem
The supervisor batch-end epilogue appends display banners via
pi.sendMessage(msg, { triggerTurn: false }). In pi'ssendCustomMessage,{ triggerTurn: false }always takes the branch that immediately appends acustomentry to the session tree at the current leaf — even while the interactive agent is streaming. When a tool call is in flight (assistanttool_useappended,tool_resultnot yet appended), that append splices a user-rolecustommessage between thetool_useand itstool_result. The next request then fails with:which permanently wedges the interactive session (every retry re-sends the same broken ordering). Reproduced repeatedly in a real session, under both
autoandsupervisedintegration modes — thesupervisor-integration-skippedbanner is gated on(mode === "supervised" || mode === "auto") && phase !== "completed", so autonomy level is not the lever.Fix
Gate the batch-end epilogue on interactive-agent idleness:
SupervisorNoticeGate(supervisor-dispatch.ts): runs the epilogue immediately when idle (leaf is terminal → safe, banner renders now), else defers it to the nextagent_settledboundary — the first lifecycle point at which all tool results, retries, compaction, and queued continuations have finished. Session-scoped, generation-tagged (a newer batch invalidates stale pending work), coalescing, and disabled on shutdown.deliverAs:"nextTurn"is deliberately not used: it pushes into the next turn's in-memory context only, without persisting the entry or emittingmessage_start/end, so display banners would be silently dropped.stopBatchMonitoring()extracted fromtransitionToRoutingMode()and called eagerly when deferring, so a heartbeat-timer send cannot splice during the defer window.extension.ts: oneagent_settledflush (re-checksisIdle()— another extension may have started a run), onesession_shutdownclear,invalidate()+ generation bump on new batch, and both batch-end callbacks (doOrchStart/doOrchResume) routed through a sharedrunSupervisorBatchEndEpilogue()helper.Tests
extensions/tests/supervisor-dispatch.test.ts(9 cases): idle→immediate, busy→defer, settle→flush, busy-settle→no flush, generation supersede,invalidate(), coalescing (latest wins),dispose(), flush-once.Verification
npm run typecheckcleanbiomeclean on changed/new files (pre-existing warnings elsewhere untouched)Design
Design validated with a second-opinion review:
agent_settled+ a session-scoped, generation-aware dispatcher is the safest available design given the current pi API (there is no pi-native "persist+display a custom entry only at a safe turn boundary" mode).Follow-up (not in this PR)
{ triggerTurn: false }sends can splice via the same mechanism and should also route through the gate: heartbeat/status notices, and integration progress/result messages.extension.tsstill registers thesession_endevent, which appears obsolete in current pi (onlysession_shutdownis defined); the gate usessession_shutdown. Worth reconciling the existing cleanup handler separately.