Codex ACP: test and log chat updates after compaction

[srid]

Adds tests and logs for Codex chats that appear to stop after compaction (#559).

• Tests continuation, messages sent during compaction, and reconnects.
• Logs where updates stop; logging failures cannot break the chat view.
• Separates diagnostics from chat updates and transport.

Codex continued in the reported session, but we cannot confirm what the tab showed. #559 stays open; the steering deadline is unchanged.

Checks passed: remote typecheck, unit tests, and 1,524 e2e scenarios. No full just ci run.

[srid]

Review of head 33c0c8058734097e7771f9cfc18f2550592b4e2b

Verdict: changes requested, one blocking item. After that fix this should merge as coverage-plus-diagnostics. It should not wait for a root-cause fix, because the evidence does not establish one, but #559 must stay open and the follow-ups below are where the next real lead is.

1. Evidence reasoning

Sound where it claims backend continuation, and honest where it says delivery is unproven. The turn ended line is annotated with the session id and the turn duration (packages/plugins/chat/src/agent.ts:457-462, :2170-2173), so "Olai logs end_turn for that original prompt" is provable from the journal; the PR body should quote the duration to close that inference rather than leave it implied.

Two things the body treats as settled that are not:

• "Code inspection found no concrete failure mechanism in ... rendering, or resubscription." The browser has two silent terminal states that were not examined, both of which match the symptom exactly: socket healthy, header still says working, transcript never moves again, nothing in the serve log. See section 4.
• The timeline says the human's message was "queued" during the ~101 s compaction and delivered later in the same turn. On this leg an ordinary send while working is a steer (packages/plugins/chat/src/chat.ts:2284-2286, packages/plugins/codex/src/leg.ts:76) with a 30-second deadline (leg.ts:72). The adapter answers _session/steering only after the app-server's turn/steer round trip (dist/index.js:33280-33298 in the pinned bundle), and unlike session/prompt the steering request is registered without ctx.signal (dist/index.js:35121), so Olai's timeout cancels its wait but the steer still injects when Codex finally answers. If that is what happened, the journal has an _session/steering did not answer in 30 seconds (agent.ts:2403-2416) and the tab had a notice row plus a trouble banner (chat.ts:2345-2349) that the screenshot crop would not show. The PR body should say whether the journal contains that line at ~14:24:30-14:25:40. It changes what "nothing happens" meant to the person.

2. Tests

• compaction.test.ts.in pins adapter invariants against a mocked app-server: compaction leaves the prompt pending, steering targets the running turn, only turn/completed resolves. Correct as a regression guard for adapter bumps, and the README says so. It restates what held.
• codex_compaction.feature scenario 1 pins that the compaction tool row does not end the turn in the UI and that the continuation row reaches the DOM. Scenario 3 (offline during continuation, snapshot catch-up) is the valuable one: no existing feature pinned a transcript row published while the tab was away. Scenario 2 pins ordinary-send-while-working as a steer into the same turn, which codex_steering.feature already largely covers.
• Would they catch a dropped or un-rendered post-compaction update? For the fake-agent path, yes: "the agent's answer mentions" is a DOM assertion and would fail. But the fake completes compaction instantly (packages/tests/agent/fake-acp-agent.ts:1719-1725 sends in_progress and completed back to back) so no scenario acts while a compaction is in progress. The reported case had a message sent during compaction. Add a variant that holds between the two frames, sends during the hold, and pins both the 30 s deadline outcome and the promptRequired outcome (the fake already has refuse steering/slow steering verbs to build on). The adapter test covers only the injected outcome.

3. Diagnostics

Bounded and cheap: three transitions per compaction per stage, one data-entry-id lookup per transition, and the browser receipt is a browser-only face (packages/plugins/chat/src/wire.ts:517-523; chat declares no agent map, so it is not agent- or MCP-callable). Per-socket Info lines fire once per upgrade, not per heartbeat. Costs to note: one extra createEffect per row (Transcript.tsx:409-412) that re-runs on every frame of the growing row, and a full sort of the transcript per snapshot (state.ts:331). Acceptable.

Not enough to diagnose a recurrence from the serve log alone, for three reasons:

• The two silent browser failures leave no server trace and also stop the receipts. A non-transport error on transcript.deltas stops the retry schedule (node_modules/@kolu/surface/src/client.ts:95-97, STREAM_RETRY) and freezes the store and every fold; a throwing fold init/step is contained with console.error only and invalidates the accumulator until the next snapshot (node_modules/@kolu/surface/src/solid/useCollection.ts:551-566, containThrow.ts:53-60), which on a healthy socket never comes; order.ts:261-262 then renders no rows. From the log both look identical to "the person closed the panel". The client already tracks per-subscription liveness (node_modules/@kolu/surface/src/subscriptions.ts:157-166, lastFrameAt, retries, state: "failed"); a receipt on transcript.deltas failure through the same observed procedure would separate the two.
• rendered cannot distinguish "in the DOM below the fold" from "seen". The body acknowledges this but does not instrument it. following and atBottom() (Transcript.tsx:128, :134-136) are two booleans already in scope; put them on the rendered receipt.
• view and connection do not correlate. With two tabs open, a browser disconnected line cannot be tied to the tab whose receipts stopped.

Smaller: the published stage annotates the session at cadence flush time, not receipt time (server.ts:373-376, :380-386), so a session switch inside a window mislabels the row; rendered resets only on session change (Transcript.tsx:177) while applied resets per snapshot (state.ts:330), so the doc line "a reconnect can report the same history again" is true for applied only.

4. Mechanisms the PR missed (blocking item first)

Blocking: the diagnostic can cause the symptom it observes. state.ts:327-340 runs applied.row() inside the fold's init/step. That path calls report, which calls chatWire() synchronously (state.ts:320), and chatWire() throws by design when the plugin half is not held (packages/plugins/chat/src/browser/wire.ts:110-118). A throw inside step is contained by the surface and invalidates this fold's accumulator until the next snapshot (useCollection.ts:551-566), which is exactly the blank-after-N-rows state. The same report runs inside a per-row createEffect in Transcript.tsx:409-412, where a throw is a page error. Wrap the body of report in try/catch (or hoist the wire read out of the fold) before merging.

Candidates worth a test, none proven for the 2026-09-08 turn:

• Second concurrent prompt on promptRequired. If Codex answers turn/steer with "no active turn" while currentTurnId is still set, the adapter returns promptRequired (dist/index.js:33288-33296), steerTaken maps that to false (leg.ts:28-31), and Olai starts a second session/prompt on the live session (chat.ts:2301-2307). In the adapter that nulls currentTurnId under the running turn (dist/index.js:34305), evicts the first prompt's handle (:34158), replaces the notification dispatch target (:27779-27792), and can overwrite the first prompt's completion resolver keyed by turn id (:29603-29607); a turn marked stale by promptShouldStop has all its notifications dropped while turn/completed still resolves it (:29187-29194, :34234-34236). Whether the app-server ever answers that way during compaction is unverified from here, but the Olai arm is real and untested.
• Steer outliving its deadline (section 1): cosmetic desync, not a drop, but it is the most likely thing the person actually saw on their own row.
• Silent drops in the adapter: unknown item types and methods return undefined from exhaustive switches with no default and are swallowed at dist/index.js:24349-24352; notifications for an unregistered thread id are dropped without a log (:29652-29663). Neither is implicated by the rollout, but a recurrence through either would be invisible to these diagnostics too.

5. Cordis boundaries and conventions

Clean. The receipt is a declared surface procedure, the browser calls it through the plugin's held client, server lines go through ring under the plugin scope (server.ts:441), cursors are owned by the activation and the mounted panel, no new service or hidden dependency, and docs moved in the same PR. Nits: the trace closures reference ring above its declaration (server.ts:373-379 vs :441); it works because they run later, but it reads as a temporal-dead-zone hazard, so move them below. Chat.rendered? as an optional sink on the state object is an odd shape for a one-caller hook; fine for a diagnostic.

Merge recommendation

Fix the throw-safety of report, then merge as coverage-plus-diagnostics. Keep #559 open. Next steps that would actually move the issue: check the journal for the steering timeout line, add the send-during-compaction scenario with both steering outcomes, and put subscription liveness and follow state on the receipts so a recurrence can be classified from the log.

[srid]

Addressed the review in 2eeee4f59a6d2e0596a6edb6f82a4c2545f3afd6 (still a draft).

1. Blocking diagnostic throw: the entire report path, including synchronous chatWire() lookup and asynchronous completion callbacks, now has throw containment. The fold observer is independently guarded too. A regression test uses the real surface collection/fold hook: a throwing report during init and subsequent steps must preserve the compaction and both later rows, even if the console fallback throws. Verified red/green by removing containment: the test fails with an invalidated accumulator; restoring containment passes.

2. Journal and steering hypothesis: read the user journal for September 8, 14:24–14:29 UTC, including the requested window. No _session/steering did not answer in 30 seconds line appears. The journal records the original prompt's end_turn at 14:28:39 with duration: "336445ms", then a new prompt at 14:28:40.968. However, inspection showed that the old deadline path made an AgentGone error and a transcript notice/banner without logging the deadline. Absence is therefore inconclusive, not evidence against a timeout. The PR body now states this and calls the mid-turn message a steer. Added an Info steering-failure event with session, method, category and deadline, without user text. The PR remains coverage-plus-diagnostics; it does not claim a historical root-cause fix or close Codex ACP: turns stop abruptly after compaction #559.

3. Silent browser terminal states: transcript and saying subscriptions now report failure through their independent onError callbacks. Order and tail projections report a fold failure before the surface invalidates them. Tests exercise the actual surface hook and stream fencing: a genuine projection throw reports despite the invalidated accumulator; a non-transport stream error stops retries but still reports independently. This preserves the surface's retry/invalidation policy. If the return wire itself is unavailable, reporting remains best effort and records the failure in the browser console.

4. In-progress compaction and fallback: the fake can now hold compaction between in_progress and completed, outlasting the real 30-second steering deadline. Browser scenarios cover sending during that hold, the unanswered row and timeout log, late injection without retry, and promptRequired starting a second prompt while the first remains outstanding. Both a busy refusal and an accepted follow-up completing before the original prompt are covered. An adapter test also drives promptRequired, starts the host-owned second prompt, then verifies that late completion of the original cannot settle the follow-up or suppress its text. These are deterministic contract tests; they do not establish that the historical app-server chose this outcome.

5. Receipt context and correlation: rendered receipts include following, atBottom and viewport intersection. A declared, request-owned CurrentBrowserConnection service supplies the server socket ID, so the browser view ID correlates with connection lifecycle logs; e2e assertions check that correlation and geometry fields. Publication captures the session before cadence. Documentation distinguishes applied receipts resetting on snapshots from rendered receipts resetting with the mounted transcript/session. DOM/viewport evidence still cannot establish that a person saw the row.

6. Nits: ring is acquired before the trace closures, and Chat.rendered is a required sink. Updated the chat, adapter and coverage documentation with the changes and limits.

Validation on 2eeee4f59a6d2e0596a6edb6f82a4c2545f3afd6, with ODU_HOSTS=/home/srid/.config/odu/hosts-lane-graph.json for each remote leg:

• just typecheck-fast-remote: passed all six shards, run 0mtuip3x8-643tywui.
• just test-fast-remote: passed all six shards, run 0mtuiq6x7-pywrag2p.
• just e2e-fast-remote: passed all six shards, 1,524 scenarios / 17,627 steps, run 0mtuir78b-fz915k8r.
• Pinned adapter Vitest: 13 tests passed, including three compaction tests and existing steering coverage.
• Real surface browser diagnostics tests: 3 passed, with the blocking observer test also verified to fail without containment.
• Focused local browser checks: the deadline/logging and accepted concurrent fallback cases passed (2 scenarios / 32 steps); the full remote run covers all seven compaction scenarios.
• git diff --check: passed. No full just ci was run.

Saatchi evidence remains the planned later review/auto-merge step.

[srid]

Re-review of head 2eeee4f59a6d2e0596a6edb6f82a4c2545f3afd6

Verdict: approve. Merge as coverage-plus-diagnostics with #559 left open. Nothing blocks. Two judgments you asked for are at the end.

What I verified on this machine

• Round-one blocker is fixed and the red/green claim is real. packages/plugins/chat/src/browser/chat/diagnostics.ts:6-14 contains every observer call before it enters the fold; observedFold (:18-41) reports a genuine projection throw and then rethrows so the surface's own invalidation policy is untouched. diagnostics.browsertest.ts runs under --conditions browser and passes 3/3; with containment removed the first test fails on an undefined accumulator, exactly the failure mode I described. The file is discovered by git ls-files '*.browsertest.ts' in scripts/test-shard.sh:6, so it runs in CI.
• The two silent browser terminal states now report: transcript and saying subscriptions pass onError (state.ts:280, :285), and the order and tail folds report fold_failed (state.ts:287, :344). The third browser test drives the real fenceStream and shows a non-transport error stops retries but still reports.
• Correlation: CurrentBrowserConnection is a declared per-socket service (packages/plugin-api/src/transport.ts:92), provided by the transport owner next to CurrentWho (packages/server/src/serve.ts:278-281), consumed optionally in the procedure (server.ts:557-561). The e2e step now requires the receipt's connection to match a browser connected line and requires following/atBottom/inViewport on rendered receipts (chat_steps.ts:3955-3962). publicationSession is captured at receipt time (server.ts:492), which is correct because row upserts are never windowed by the cadence. ring now precedes the trace closures. Chat.rendered is required.
• Tests now act during an in-progress compaction: the fake holds between in_progress and completed (fake-acp-agent.ts:1728-1732), queues steers while compacting and replays them on completion (:2842, :1740), mirroring the adapter's unretractable turn/steer. The four new scenarios pin the same-prompt steer through the hold, the 30 s deadline outcome with no retry and a later injection, the promptRequired busy refusal, and an accepted second prompt completing while the first stays open. The adapter test pins promptRequired with two distinct turn ids and independent completions. The same-turn-id resolver clobber I mentioned stays untested, which is fine: it is hypothetical and not something a mock can prove either way.
• Unit tests (compaction.test.ts) still pass locally. Cordis boundaries are clean; docs moved with the code.

Judgment 1: is the steer-deadline path now diagnosable from the log?

Partly, and enough for this PR. steering request failed (agent.ts:2222-2228) carries session, method, gone and the deadline at Info, so a recurrence of the deadline expiring during compaction would be visible where the 2026-09-08 journal was blind, and the e2e scenario proves the line is emitted for the real 30 s path. What the log still cannot say is whether the timed-out steer was later injected: the adapter does not honor cancellation for _session/steering (dist/index.js:35121 versus session/prompt on the same line) and its late reply is consumed by the SDK's pending map, so Olai never sees the outcome. The adapter's own stderr line (Steering session injected, dist/index.js:33288-33292) is only dumped on turn failure. A follow-up could keep listening on the SDK promise after the deadline and log the late outcome content-free; not required here.

Judgment 2: should the steer deadline change in this PR?

No. It should change, but in its own PR. The evidence is that the deadline cannot protect anything: the adapter ignores the cancel, so a steer that outlives 30 s still lands, and the deadline only turns a slow, successful steer into a row marked unanswered with the retry button withheld (chat.ts:2286-2292, :2362-2366). Compaction alone took 101 s in the reported turn. The honest semantics are the prompt's own (agent.ts:2156-2157, no deadline, bounded by the turn's life, since the adapter answers once the turn ends or Codex refuses), or at minimum a deadline well above realistic compaction time. Either is a delivery-behavior change, which this PR's contract explicitly refuses, and the new "injected after its deadline" scenario pins the current semantics and would have to be rewritten. Keep the PR honest to its own scope and file the deadline change against #559 with that scenario's rewrite as its test.

Non-blocking notes

• The deadline scenario waits the real 30 seconds in e2e (codex_compaction.feature:67-81). Acceptable, but it is the slowest scenario in the feature by design; if the deadline moves to a leg-level config later, the test should read it rather than hard-code the string (chat_steps.ts:3974).
• The compaction hold gives up after 90 s (fake-acp-agent.ts:1730) and continues silently; a scenario that forgets to release would pass later rather than fail. Fine for now.
• Diagnostics still trace only the first row after a compaction; a later drop inside the same turn is not distinguished from any other turn's. Documented limit, unchanged from round one.

[srid]

Cordis-adherence look at head 6453e02088c9c9f7bb55ee89a2952eb2c1853c08

Verdict: approve for merge. The refactor on top of 2eeee4f5 changes no production behaviour and keeps the plugin boundaries.

What I checked:

• Behaviour equivalence, browser side. delivery.ts:10-40 reproduces the round-two logic one for one: the same view derivation, source read at report time, applied reset on snapshot and seq-sorted replay, rendered forced to dom, failed sending null compaction and row, and the same three console messages. Containment is unchanged in effect: report is diagnostic-wrapped, the caller's send runs inside it, so a synchronous chatWire() throw is still caught before it can reach a fold, and observedFold still wraps the observer and the failure callback separately. The receipt-refused test flipped from Result.isFailure to !Result.isSuccess, which is the same predicate. compaction.test.ts and the three browser-condition tests pass at this head; the chat package typechecks.
• Wire contract. observation.ts is the round-two schema moved verbatim, so the procedure input is byte-identical and the server handler's log line is unchanged (server.ts:558-561). A static contract as an import is the allowed shape; no new service, owner, subscription or lifetime was introduced, and the docs say so.
• Ownership. createChat still owns transport execution through the held plugin client and passes it in explicitly (state.ts:254-257); deliveryDiagnostics receives a context thunk and a send function and holds no reference to the wire or the folds. Cursors stay with the mounted panel; the surface keeps subscriptions and reconnection. That is the separation the commit claims.
• Fake agent. held-steering.ts is the old queue with a name: defer while held, replay on release, and release clears held before replaying so the re-entered steerTurn cannot re-defer. The only ordering difference is that a steer arriving between the filesystem release and the tool_call_update frame is now replayed after the frame instead of answered inside that window, which is the more faithful order anyway. Hold expiry now logs through noise and still continues, as described.

Nits, not blocking:

• untrack is still imported in packages/plugins/chat/src/browser/chat/state.ts:55 but no longer used there; tsc accepts it and no linter enforces it, so it is dead weight only.
• The new imports sit above the file header comments in server.ts:1 and wire.ts:1; the rest of the tree puts the header first.