feat(opencode): track native shell jobs across turns#34141

🟢 Low

https://github.com/pingdotgg/t3code/blob/044ef53a5dce3a1cc9085b998658146ec10ab64d/packages/provider-opencode/src/server/v2/adapter.ts#L1207

Failed shell completions lose their exitCode in the turn item because emitTool forwards result.metadata only for completed status. Preserve metadata for shell results when the status is failed too.

Fixed. Native shell terminal metadata now reaches the command item for failed outcomes as well. The regression asserts a failed command retains exit code 7.

🟡 Medium v2/adapter.ts:2712

When a shell produces more than 256 KiB since the last poll, its final output is truncated in the activity item, so the command’s final diagnostics can be lost. The shell.exited handler calls readShellOutput only once before deleting the shell, but that helper reads at most four 65,536-byte pages per call; drain the remaining pages before deleting the shell.

Fixed terminal reads to reach the final retained tail instead of stopping at the first four pages. A five-page burst regression now verifies that the final diagnostic is retained.

🟠 High v2/adapter.ts:3802

A shell-removal error aborts stopBackground, so interruptTurn never attempts to interrupt the parent session or stop the remaining background work. Because stopShell(id) is awaited without per-shell error handling, catch and log its failure so the loop continues.

Fixed. Each shell stop now takes the adapter lock and logs its own failure, so remaining shells, subagents, and the parent are still interrupted. The regression covers a failed removal and retry without falsely clearing the live shell.

🟠 High v2/adapter.ts:1583

After a cold resumeThread, recovered shell output and completion are never projected to a turn item, so the UI shows missing or stale command Details and can leave the activity item running. recoverShells creates entries without turn, toolId, or tool, causing emitShell to return at its guard; restore the projection identity or create a command item during recovery.

Fixed cold recovery to publish a shell-owned command item when no in-memory turn exists. Output and terminal updates share its stable shell identity. Recovery tests now assert retained output and the same item identity through Stop.

The adapter now emits the item, but it has a null runId, so server event routing drops it before persistence. The recovered output and status still won't reach the UI.

Confirmed. The adapter-only assertion missed the server routing guard. I added that guard to the recovery regression and reproduced the rejection. Recovered shells now retain output and settlement until they can attach to a real turn, or attach immediately when recovery finds an existing active turn. Both output and terminal events carry that run owner. The regression covers running and already-settled jobs and checks the actual server routing decision; shared routing rules are unchanged.

The updates now route once a turn binds them, but after a cold resume without an active turn, output and settlement remain unprojected until another turn starts. The original UI issue therefore persists in that case.

Confirmed: the previous patch still delayed idle recovery until a new turn. Fixed in dfc152bd4a. Recovered command activity and explicitly session-scoped idle roster snapshots now persist through the existing session event pump, restricted to attached threads. Commands retain a stable persisted run anchor when available, so timeline ordering also works before another turn starts. The persistence regression failed before the fix with a missing recovered item; it now checks retained output, settlement, an empty roster, and exclusion of unattached threads without starting a turn. Adapter fixtures cover idle natural completion, Stop, already-settled recovery, older inventory fallback, and persisted run anchors. vp run check:changed passed with 220 tests across four files.

External image Minor · Stability & Availability · Quick win

Add a timeout to the shell output read.

client.shell.output has no timeout. handleEvent calls readShellOutput while follow holds the global lock. The call sites are at Lines 2443, 2616 and 2712. If the server stalls on this request, event processing for all threads stops until it returns.

Effect.ignore({ log: true }) handles errors only, not a hang. Nearby code bounds comparable calls, for example REQUEST_REPLY_TIMEOUT and INVENTORY_TIMEOUT. Put the timeout inside readShellOutput so every caller is covered.

✅ Confirmed as addressed by @Yash-Singh1

Fixed. The complete shell-output read has the existing inventory timeout, including event-handler and cancellation callers. Output-read failures do not prevent terminal state projection.

@Yash-Singh1 Thanks for fixing this. readShellOutput now applies INVENTORY_TIMEOUT to the complete read. The shell event and stop paths handle read failures before projecting the terminal state.

Review thread resolved.

_{You are interacting with an AI system.}

External image Minor · Stability & Availability · Quick win

Bound shell.remove.

If client.shell.remove does not respond, stopTask can hold the session-wide lock and block event processing.

✅ Confirmed as addressed by @Yash-Singh1

Fixed. Native shell removal now has the inventory timeout and resets the stopping state on failure. A failed request leaves the shell pending rather than claiming it stopped.

@Yash-Singh1 Thanks for the fix. shell.remove now has a timeout, and a failed request leaves the shell pending.

Review thread resolved.

_{You are interacting with an AI system.}

External image Minor · Stability & Availability · Quick win

Run recoverShells under the adapter lock.

recoverShells mutates shared session state: the shells map, shell.output, shell.cursor, and the roster through syncRosterAncestors. Neither call site holds lock. In reconnect, the call runs before lock.withPermit(reconcile()). In resumeThread, it runs under the session gate only.

The 2-second polling task and handleEvent take lock and also read and mutate the same shells. Two concurrent readShellOutput calls can start from the same cursor and append the same output twice. An exit event processed mid-recovery can also leave shells and the roster inconsistent.

Take the lock around the whole recovery. Put the timeout inside the permit. Confirm that no caller of resumeThread already holds lock, because Semaphore is not reentrant.

Also applies to: 4085-4089

Source: Learnings

✅ Confirmed as addressed by @Yash-Singh1

Fixed. Recovery acquires the same adapter lock as polling, events, and individual Stop. Its timeout and final roster synchronization run inside the permit. Neither resumeThread nor reconnect enters this helper while holding the permit.

@Yash-Singh1 Thanks. recoverShells now holds the adapter lock through recovery and roster synchronization. Its timeout runs inside the permit, and both call sites invoke it outside the permit.

Review thread resolved.

_{You are interacting with an AI system.}

External image Minor · Stability & Availability · Quick win

Do not let one failed shell lookup abort recovery.

In recoverShells, client.shell.get fails the whole effect on any error except ShellNotFoundError. yield* readShellOutput(id) also fails it. Every other call site ignores output-read errors.

If one recovered shell hits a transient error, the loop stops. Later shells are not checked. The final syncRosterAncestors(state) at Line 1616 is skipped. Entries already added to shells keep running: true, so owesWork is true while the published roster lacks them.

Ignore and log the output-read error, as the other call sites do. Make sure the roster sync still runs when a lookup fails.

✅ Confirmed as addressed by @Yash-Singh1

Fixed. Recovery handles lookup and output-read failures per shell and publishes the roster in an ensuring block. Tests cover an early lookup failure and an output failure while later shells remain recoverable and stoppable.

@Yash-Singh1 Thanks for the fix. The recovery loop now catches lookup errors per shell, logs output-read errors, and syncs the roster in an ensuring block.

Review thread resolved.

_{You are interacting with an AI system.}

External image Minor · Stability & Availability · Quick win

Run stopShell under lock and tolerate its failure in stopBackground.

stopTask runs stopShell inside lock.withPermit. stopBackground does not. It runs from interruptTurn outside the lock. stopShell reads output, calls the server, mutates shells, and emits events. Meanwhile handleEvent can process shell.exited for the same shell. stopping is set only after the first output read, so that exit can complete and delete the entry first. stopShell then calls remove on a gone shell.

An error from stopShell also propagates out of stopBackground. Subagent children are then not interrupted, and interruptTurn fails. Take the lock and log the error so the subagent stop still runs.

Source: Learnings

✅ Confirmed as addressed by @Yash-Singh1

Fixed. Thread Stop acquires the same lock for each native shell stop and catches each failure before continuing to other work. The regression verifies remaining shell, subagent, and parent interrupts despite a shell-removal error.

@Yash-Singh1 Thanks for fixing the locking and failure handling, and for adding the regression test.

Review thread resolved.

_{You are interacting with an AI system.}

🟡 Medium v2/adapter.ts:3498

The shell-output poll holds the adapter-wide lock during sequential network reads, so six stalled reads can block stream-event processing, turn setup, and Stop for roughly 30 seconds on every poll. Each read has its own INVENTORY_TIMEOUT, and withPermit retains the permit until the wrapped effect finishes; move the reads outside the lock and acquire it only for any state updates that need protection.

Fixed in dfc152bd4a. The poll takes a short snapshot under the lock, performs bounded output reads outside it with concurrency four, and reacquires it only to apply results. Results are discarded if the shell identity or cursor changed, or if it stopped while the read was in flight. Extended the existing shell lifecycle case with a hung poll response: the root turn still completes and explicit Stop settles the shell without advancing the poll timeout. vp run check:changed passed with 220 tests across four files.

External image Minor · Stability & Availability · Quick win

Emit recovered shells before publishing the roster.

If the recovery timeout fires during a shell read, that shell and later inserted shells can remain running without a turn item. The roster still lists them, but their command output panels are unavailable. A shell with no unread output remains without an item until another event emits it.

Ensure interrupted recovery emits the missing running items before syncing the roster. Keep the recovery work bounded, including the inventory read and per-shell reads.

✅ Confirmed as addressed by @Yash-Singh1

Confirmed and fixed in b368b819c9. Recovery now records whether a shell item has been published. Its existing finalizer emits any missing running items before synchronizing the roster, without adding network reads or duplicating successful projections. Extended the recovery/Stop failure matrix with a hung first lookup and a second discovered shell. The regression failed before the fix with no command items for either roster entry; it now verifies both items appear before the roster, even though neither has unread output. Inventory and shell reads retain the existing timeout. vp run check:changed passed with 222 tests across four files.

@Yash-Singh1 Thanks for the fix and the regression coverage. The supplied code shows that recovery emits unpublished running shell items before syncing the roster. Recovery reads remain timeout-bounded.

Review thread resolved.

_{You are interacting with an AI system.}

External image Minor · Data Integrity & Integration · Quick win

Reset session-scoped routing for child status updates.

If recovery first finds a shell for an idle child, it can leave sessionScopedBackground set on that child. Later active and idle updates can then be dropped when the child is not attached.

Reset the flag for child status transitions, including the update in attachChild. Preserve session-scoped routing for idle recovery.

✅ Addressed in commit b368b81

Confirmed and fixed in b368b819c9. Attaching a child, starting its turn, and settling its call now clear stale session-scoped background routing. Idle recovery keeps its session-scoped path. Extended the existing failed-background-subagent case with a child first resumed through shell inventory recovery; it failed before the fix because the reattached child active/idle snapshots remained session-scoped. Both variants now pass, and vp run check:changed passed with 222 tests across four files.

@Yash-Singh1 Thanks for the fix and regression coverage. The supplied code clears sessionScopedBackground during the child transitions and preserves it for idle recovery. I did not run the tests.

Review thread resolved.

_{You are interacting with an AI system.}