Repository navigation
Conversation
There was a problem hiding this comment.
I reproduced the bug on the base branch and confirmed this PR fixes it. I ran OpenCode from source with a scripted simulated model: the parent runs a foreground subagent, and the child starts a real background shell, replies early, then replies again after the shell's notice.
- Base: the parent got the early reply, and the child's real final answer never reached it.
- This PR: the parent got only the final answer, after the notice was handled, with no extra model calls.
The new test fails on the base and passes here. The nearby core tests and typecheck pass.
The public API is unchanged.
One concern (inline): if a finished job's notice is never delivered, the subagent now waits forever instead of reporting late. That can already happen through the open bug #56729.
| } | ||
| const running = known.filter((info) => info.status === "running") | ||
| // Finished jobs keep their marker until their notice is admitted. | ||
| if (running.length === 0) yield* Effect.sleep("50 millis") |
There was a problem hiding this comment.
This loop has no exit if a finished job's notice never arrives. That can happen today, not just in theory: in #56729, rebuilding the shell plugin cancels notifyWhenDone. The job stays known to this process and finished, but its marker is never cleared. The subagent then scans KV every 50 ms forever and never reports, and a parent waiting in the foreground stays blocked. Before this change, the parent at least got the early reply.
Could the wait end when the notice is actually delivered rather than by polling? For example, completeBackground could signal a per-notification Deferred or PubSub. Alternatively, a finished job whose marker hasn't cleared within some limit could stop blocking the result. Either way, a dropped notice would no longer leave the subagent stuck.
bd5388d to
b834edd
Compare
|
@jlongster verified this on the branch, including a 3-level nested chain (leaf starts a 10s background
Depth-1 case verified the same way. Fix works 👍 |
…1987) ## What this adds An OpenCode 2 pane's subagents now show up the way a Claude Code pane's do: - the message box's background tasks button and the sidebar badge count the running ones; - the list shows them, with the newest ten that ended in the last day (title, agent, model, steps, tool calls, context tokens); - a turn that ends while a subagent it started still runs reads **BG**, not DONE, and its finish is alerted once (the same BackgroundWait path as Claude Code); - one left running when the pane stops running that session reads as lost; - a subagent run in the background leaves a card in the chat with its answer when it ends, instead of a plain notice, and a foreground call's answer no longer shows OpenCode's `<subagent …>` tag. Fixes #893 ## How OpenCode 2 runs each subagent in a child session of the session that started it (`session_v2.parent_id`), with its own `session_message` rows, in the store the chat already reads. Read from OpenCode's own source (`packages/core`, dev): the `subagent` tool (`tool/plugin/subagent.ts`) makes the child, `session/projector.ts` appends an `idle` row with the run's outcome (`succeeded` / `failed` / `interrupted`) when a run ends, and `session/subagent-completion.ts` wakes the parent with a `synthetic` row (`metadata.source: "subagent"`, `childID`, `agent`, `state`) when a background one ends. - `server/opencode-subagents.ts`: `opencodeSubagentState` reads a session's children. A child whose newest run row is `idle` has ended; a later prompt (the tool called again with its sessionID) starts another run. A store 1.x wrote has no idle rows, so there an answered, failed or stopped step ends the run. A run last written before the running OpenCode service started was taken along by the service before it and reads as lost. `OpencodeSubagentStatus` tracks the session herdr's OpenCode integration reports for each pane (`herdr:opencode`, never a guess from the cwd) and polls the store every 2 s, once for all panes. - `server/core/status.ts`: the tracker feeds the same count, BackgroundWait and `pane-status` frames as the Claude tracker. A turn end on an OpenCode pane polls it first, so the frame that ends the turn already says BG. Which tracker speaks for a pane follows the agent the newest snapshot shows there (`owns`): a tracker that has not read that snapshot yet can neither end the new agent's wait nor keep the old agent's wait from being dropped. A pane's first session keeps what its turn was seen do before the session was found, unless the pane ran another agent until then. - `server/tasks-routes.ts`: `GET /api/pane/omo-tasks` answers an OpenCode pane with its subagents (the session the pane runs now, if it ran Claude Code before). Each task says `detail_kind: null`: the detail sheet #1842 added opens for every running task, and for an OpenCode one it now says this PC can't show details, not that the bridge needs an update. - `server/opencode.ts`: the synthetic wake row becomes a `task_result` part (the card Claude Code's ended subagents use), and the foreground tool's output is unwrapped. - Client: the list shows "N more running" when the answer says some are left out. `BackgroundTasks`' `claude` prop is now `subagents`, set for Claude Code and OpenCode panes, so an OpenCode pane with only ended subagents still finds its list. As for a foreground Claude Code `Agent` call, a foreground OpenCode subagent's answer stays its tool call's output and gets no separate card. Known limits: ends are read from the child session's own idle row, which matches OpenCode as released (2.1.4); anomalyco/opencode#57418 (open) would answer the parent only after a child's own background work, and #1999 tracks reading ends from the parent's record. Jobs live in OpenCode's service process. If that service goes away mid-run without ending it (a crash, not a clean shutdown, which appends an interrupted idle row), the child reads as running until another service starts or the pane leaves the session. ## Verification - Unit (`server/opencode-subagents.test.ts`, `server/opencode.test.ts`, `server/tasks-routes.test.ts`; with `background-agent`, `background-wait` and `claude-subagents` 158 pass): running, continued, succeeded, failed, interrupted, never prompted, older than a day, archived and other sessions' children; lost when not live or orphaned by a service restart; turn counts by each run's own prompt; the 1.x fallback; unreadable stores; the tracker's change, home-screen, pane-leaves-OpenCode, session-switch and first-session rules; the route; `detail_kind`; the card and the unwrapped output. Mutations each fail a test: idle never ending a run, never lost, dispatch by creation time, no card, no unwrap, no `detail_kind`, a reset on every first session, no reset after another agent. - Contract (`server/opencode.contract.test.ts`, real isolated herdr, HTTP and WebSocket, its own server with an empty OpenCode state dir): a running child is counted in a `pane-status` frame, the turn that ends meanwhile gets `background_wait`, the list shows it, and when it ends and OpenCode wakes the parent the count goes and the conversation ends with its card. 21/21 on the PR head (`e9d83c227`). Removing the tracker's refresh fails the case at its count frame. - Status hub (`server/core/status.test.ts`, the real `createStatusHub` with the collector stood in for): an OpenCode turn after Claude Code's, held on its subagent before any snapshot and after one, with its finish alert called off; a snapshot read before the session was found, answered after it; a Claude Code session that follows OpenCode's. Reverting the reset that keeps OpenCode's rests, the immediate lookup, the snapshot's agent, or the read stamps each fails it. - Status hub, more cases (QA script, not in the repo: the real `createStatusHub` driven through `clientSnapshot`, with herdr's collector and snapshot, the OpenCode service and the Claude lookup stubbed): a session found only after its turn ended (with or without OpenCode's home screen first) reads BG; an OpenCode pane held on a subagent that then runs a resting Claude session with a running subagent does not inherit the hold; a Claude pane held on a subagent that then runs OpenCode drops the hold. Removing the snapshot's agent rule or resetting on every first session each fails it. The status hub has no unit harness in the repo. - Browser, the list's cut (QA script, real built client, isolated herdr, 1440x900 and 390x844): with 1004 running subagents the button counts them all, the list holds 1000, and "4 more running" shows below it. - Browser (on the PR head; real built client, real server on port 0, isolated herdr with a stand-in OpenCode pane and a fixture store; desktop 1440x900 and phone 390x844): with a running child the composer status reads BG, the button reads "1 background task", the sidebar's agent row shows 1, the list shows the child with its agent, model, steps, tool calls and tokens, and opening it says this PC can't show details for it. When it ends and the parent is woken, the chat shows the "Background task ended" card, which opens to the answer, and the badge goes. A second child left running when the pane stops running OpenCode is listed as lost. - `bun run check fast` on the PR head (`e9d83c227`): green, unit 7211 pass, 0 fail. ## After the merge pass's gate (ae1384c, fa86923) - A poll no longer waits on the event loop: - the OpenCode service's start is looked up once per service process, off the loop; - a poll's store read waits on no writer, and a busy store keeps the counts as they were; - a read takes steps of a page each (`SUBAGENT_PAGE`, 1000). A step is a page of a session's children, or a page of the rows of those it keeps (the ones still running, and the ten that ended last), or a few listed children's work. A poll takes the first two steps at once, so a session of up to 1000 children reads whole as before, and a turn's end still finds its subagents in the frame that ends it. A larger session is read a step per turn of the event loop. A pane that leaves its session meanwhile is told so at once. The longest hold of the event loop is about 12 ms on 100,000 ended children, and 13 ms on 20,000 running ones. - Every child whose run has not ended is counted, however many newer ones ended. Up to 1000 of them are listed, the newest, while the badge counts them all. The answer then says how many it leaves out (`more_running` on `GET /api/pane/omo-tasks`, optional and only when some are), and the list shows "N more running" below the running ones. The ones that ended are listed by when they ended, newest first. - The background tasks route answers for what the pane runs after its list is read. OmO, Claude Code and OpenCode are told again, and the list is given only if the pane is still bound to the session that was read; otherwise the route decides again from the current state. - An OpenCode turn that followed another agent and ended on its subagent before its session was found reads BG: - such a reset keeps the rests seen since the pane's first status event under its new agent (`BackgroundWait.resetBefore`); - a status event of an OpenCode pane whose session is not known yet looks the pane up at once, so the finish alert, which waits at least 10 s, is called off; - a snapshot read begun before that lookup no longer undoes it. ## Reviews - omo-native-code-reviewer (after the rebase onto #1848 and #1842): approve, four LOWs. Fixed: a Claude-held wait left behind on a direct Claude→OpenCode switch, the contract case reading the machine's own `service.json`, and the guide's wording about lost subagents. Declined: reading `service.json` on each poll, which `omo.ts` and `prompt.ts` do the same way. - Codex (gpt-6.1-sol, xhigh): two MEDIUMs, both reproduced and fixed: a session found after its turn ended dropped the turn's rest (DONE while a child runs), and an incoming Claude session inherited OpenCode's BG hold because the outgoing tracker's stale state vetoed Claude's reset. The shared pane mute (#1848) and the detail sheet (#1842) checked out. Codex Sol review did not run (host stall); the merge pass reviews: its re-review of these fixes stalled twice before starting. omo-native-code-reviewer re-reviewed them instead: approve. Of its LOWs, fixed: a route's Claude lookup between snapshots could still meet the old veto (a status event's agent now counts too), and a dead test line. Declined: an OpenCode turn that starts and ends after another agent with no snapshot in between loses its rest to the reset of the first session, as Claude's own reset does on main. - An earlier round before the rebase (omo and Codex) fixed: a run orphaned by a service crash read running forever, a tracker letting go of a pane released another agent's BG wait, the child cap applied before classifying, the home screen kept the binding live, one store open per pane per poll, a route test pinned to a date, contract frames matched by index. --------- Co-authored-by: mp <mp@local>
…al setting) (#3167) Fixes #3027. Refs #1999 (the reading stays opt-in until anomalyco/opencode#57418 ships). Supersedes #3022 (its branch, merged with main, plus the fixes below). ## What #3022 read an OpenCode subagent's end from its parent session's record (the `subagent` call block, or the background notice), behind **Read OpenCode subagent ends from their session (experimental)**. That reading is ready for anomalyco/opencode#57418, where a child can go idle while its parent still waits on it. It was parked because review still reproduced open cases with the setting on. This PR keeps everything from #3022 and closes those cases using only facts OpenCode records. The setting stays experimental and off by default, because #57418 is still open upstream. With the setting off, nothing reads parent rows, so behaviour matches main. Each item below has a test that fails on #3022's head and passes here. 1. **A late background notice no longer ends a newer call the parent waits on** (#3027 case 2). A background notice's row id is the job's `notificationID`. OpenCode names that id when the job goes to the background (`Job.markBackground`, then `SubagentCompletion.deliver` writes the synthetic row with `id: notificationID`, in 2.2.2 and 2.2.5). Row ids are `Identifier.ascending`: 12 hex digits of `ms * 4096 + counter`, then random characters. A `subagent` call's row id is named when its step begins. A call the parent still waits on started a job of its own: a call to a child whose job is already in the background joins that job and is answered at once (`subagent.ts`, `Job.block`). So a notice named before that call's step answers an older job, however late it was written, and it does not end the call. Every other pair keeps row order, because a call answered as backgrounded may have joined the job that an older-named notice ends. A test covers that join case. Ids of any other form (forked rows, plain ids) fall back to row order, as before. 2. **A staged `/undo` is applied before the newest record per child is picked** (#3027 case 4). The parent rows the boundary hides are not read at all, so an older call the chat still shows decides. When the boundary moves below what the log has read, the log starts over. 3. **The first read of a long parent history no longer misses the frame that ends a turn** (#3027 case 3). Reading more than a page of parent rows from the start takes more than the two synchronous steps (a pane just bound, a log started over). In that case the children's own rows are read alone as well, as with the setting off. Their counts are told at once, but only when neither count is lower than the counts that stand. So a plainly running child counts in the turn-end frame, and no count falls before the whole read says so. 4. **Turning the setting off mid-wait no longer holds BG for a minute** (#3027 case 5). The tracker tells `onChange` that a count was read under the other side of the setting. `BackgroundWait.running` then starts no grace for a count that fell that way. 5. **The `time_updated` watermark** (#3022 open item 5). A row the log took a word from is read again whenever its `time_updated` differs from the one read, older or newer. A session whose newest row is older than the log's watermark (a clock set back, a restored store) starts the read over. 6. **A wake in the same millisecond as the recorded end** (#3022 open item 6) counts as a wake: a run cannot end in the millisecond it was dispatched. ## Not changed - #3022 open item 7 (one pass over the parent's row headers on each warm poll, measured at about 90 ms on 250,000 rows) is unchanged. The new watermark check reuses the read the log already makes of the rows it relies on. - A late notice after a newer call that was answered as backgrounded keeps row order. OpenCode writes nothing in the parent's rows that tells a fresh background job from one the call joined. The `job` table's pending markers (2.2.5) could tell them apart, but this PR does not read a new table. ## Validation - Failing-first: 6 of the 7 new tests in `server/opencode-subagents.test.ts` (late notice, staged `/undo` order, same-millisecond wake, long cold read, clock set back, setting toggle) and the new `server/background-wait.test.ts` test fail on #3022's head and pass here. The seventh, the join case, passes on both and guards against a regression. Existing tracker tests now expect the new fifth `onChange` argument (`false`). - `bun test server/opencode-subagents.test.ts server/background-wait.test.ts server/core/status.test.ts`: 133 pass, 0 fail. - `bun run typecheck`: pass. - `bun run check fast`: pass on old-desktop through the heavy runner (unit 12690 pass, 0 fail, 38 skip in 766 files; typecheck, build, generated types/themes, workflow syntax ok). - Merged origin/main: the settings row moved into Appearance's Experimental fold next to **Lock pinned agents (experimental)**, the fold's keys include `opencodeParentEnds`, and the i18n parts include `settings-agents.*`. Not run: the browser regression suites (the only UI change is one row in an existing settings fold, with no selection or focus change), and a live OpenCode with #57418 (that PR has not merged).


Supersedes #57410, which was auto-closed when
v2was promoted todev.Problem
A subagent reported its final message to the parent as soon as its child session went idle, even if the child still had background work running (background shell commands, its own background subagents). When that work finished, its notice woke the child again, but the parent had already received the earlier message, and the child's real final answer was never reported.
Root cause
The subagent job's
runeffect (SubagentJob.start, and the restart-recovery equivalent inSessionRestart) was justresume(child)followed by reading the latest assistant message. It never checked the background jobs the child owned.Design
The job now calls
resume(child)once, thenSubagentCompletion.awaitBackground(jobs, execution, childID), and only then reads the final assistant message.awaitBackgroundreuses the existing durable background markers (Job.pendingBackground). It finds the markers the child owns (recovery.sessionIDfor shells,recovery.parentSessionIDfor subagents) and ignores any whose job this process doesn't know. Then it loops:jobs.waiton the owned jobs that are still running.SessionExecution.awaitIdle(child): admission callswakebefore the marker is cleared, so waiting for idle after the clear covers the drain that handles the notice.Nothing in this loop calls
resume, so there are no forced runs and no extra model calls with empty input. With no background work, step 3 returns immediately, so nothing changes for normal subagents.subagent-job.tsnow depends onSessionExecution.Service.restart.tsalready had it.Tests
New
tool-subagent.test.tscase. It uses the real session runner with scripted responses ("before notice", then "after notice"). The child holds a background job open, then a real notice is delivered the way the shell tool does (sessions.synthetic, thencompleteBackground), 200 ms after the job finishes. It asserts:The test fails on the base branch and on the earlier forced-resume version of this fix.
bun typecheckinpackages/core. Rantool-subagent,job,session-execution,subagent-lifetimeandtool-shelltests: 132 pass, 0 fail.Remaining risks
resume: false. If it lands in that window after the child's first resume, nothing wakes the child, soawaitIdlereturns and the parent gets the earlier message. Shell notices are admitted before subagent recovery starts, so the child's first resume handles them.Added by the opencode bot (AI-generated).
Fixes #48826
Related: #56729