Repository navigation
fix(cli): wait for background jobs in opencode run - #60699
Conversation
1b8e517 to
7db0adc
Compare
64187c7 to
5089faf
Compare
b3b5fd2 to
1010b17
Compare
f2da732 to
e1f45df
Compare
There was a problem hiding this comment.
The approach looks sound: asking the server for background jobs before treating session.execution.succeeded as the end of the run, then checking active sessions and the inbox. The ordering that makes this race-free checks out in the code: the shell notifier calls sessions.synthetic before completeBackground deletes the job row. I couldn't reproduce the problem end to end. My drive script never got the simulated background shell call to reach the CLI, so this review rests on reading the code plus the tests (noninteractive.test.ts 15/15, server job.test.ts 1/1) and typecheck:tsc in packages/cli (no errors).
Public API (all additions, backwards compatible):
- New
GET /api/experimental/job(experimental.job.list) with optional query paramssessionID,kind(shell|subagent|image),status(running|completed|error|cancelled) anddescendants("true"|"false"). It returns{ data: Job.Info[] }and gives 404 for an unknown session. - New client method
client.experimental.job.list(promise and effect clients), plus theExperimentalJobList*types. - New
@opencode/schema/jobmodule (Job.Kind,Job.Status,Job.Time,Job.Shell,Job.Subagent,Job.Image,Job.Info). - Behaviour change in
opencode run: it now stays running until background jobs from the run finish, and Ctrl-C while waiting exits with 130.
Main concern: opencode run can now hang forever when a prompt starts a background job that never finishes (see the inline comment).
1010b17 to
4d3ad3a
Compare
There was a problem hiding this comment.
The new commit fixes the earlier concern: background shells now hold opencode run open for at most 10 minutes after the last reply. After that the run stops them, prints a warning and exits, and subagents and images still wait until they finish. I checked by reading the code, plus noninteractive.test.ts (16/16) and typecheck:tsc in packages/cli (no errors). I didn't run it end to end.
Public API: unchanged by this pull request now that the job-list endpoint and client are in the base. The only change is that opencode run waits for its session's background jobs (shells for at most 10 minutes).
Concerns: the 10-minute timer can run out while a woken reply is still streaming and cut it off (inline). The first new test is large and depends on how many ticks Bun.sleep(0) takes (inline).
packages/cli/src/run/noninteractive.ts:176: The 10-minute timer can run out while the model is still writing a reply.shellWaitis only reset onsession.execution.succeeded. Say one shell finishes near the 10-minute mark and wakes the model, and another shell is still running. If the timer runs out during that reply, this check sees only running shells and returns"shells"before it looks atsession.active. Theexpiredbranch then stops the other shell and returns fromconsume, so the rest of the reply is never printed. Could you checkactive/inbox before treating the session as idle with only shells left, or clear the timer when a new step starts (for example onsession.step.startedfor this session)?packages/cli/test/run/noninteractive.test.ts:695: This test is about 100 lines and builds its own event stream and mocks, which thebackgroundRunhelper used by the other new tests already covers. It also relies on twoawait Bun.sleep(0)calls being exactly enough ticks before it assertsexited === false. That timing is fragile, and the test can pass trivially if the run hasn't reached the job check yet. Could it usebackgroundRunand wait forjob.listto be called (or for another explicit signal) instead of counting ticks?
1feba95 to
b706e45
Compare
There was a problem hiding this comment.
The fix works: I ran the #48361 reproduction (two background subagents, 15s each) from source. On the base, run exited after 3s without the results. With this pull request it waited 18s, printed the parent's resumed FINAL: got TOKEN_A and TOKEN_B and exited 0. noninteractive.test.ts (15/15) and typecheck:tsc in packages/cli both pass.
The earlier point about run hanging forever on a long-lived shell stays addressed: shells are capped at 10 minutes, and run now just exits without them.
Public API: unchanged. The only change is that opencode run waits for its session's own background jobs (shells for at most 10 minutes).
Remaining concern: the shell timer can still run out during a woken reply, and it now aborts with no check (inline).
packages/cli/src/run/noninteractive.ts:586: This timer is still only cleared onsession.execution.succeeded, and it now aborts with no check at all. Say one shell finishes near the 10-minute mark and wakes the model while another shell is still running. The timer then fires in the middle of the woken reply andconsumestops.reconcilebackfills that reply's text, but nothing handles what the turn does after that: permission or question requests go unanswered, sosession.waitcould hang, and background jobs it starts are ignored. Clearing the timer when this session starts a new step (or re-checkingsession.activebefore aborting) would avoid this.
There was a problem hiding this comment.
The fix works: in the #48361 reproduction (two background subagents, 15s each, run from source), the base exited after 2s without the results. With this pull request, run waited 18s, printed the parent's resumed FINAL: got TOKEN_A and TOKEN_B and exited 0. noninteractive.test.ts (14/14) and typecheck:tsc in packages/cli both pass.
The 10-minute shell timer is gone, so the earlier problem of it running out during a woken reply can no longer happen. Without that cap, the open thread about a background shell that never exits keeping run open stays open; the description now lists it as out of scope.
Public API: unchanged. The only behaviour change is that opencode run waits for its session's own background jobs.
b706e45 to
a24cc37
Compare
There was a problem hiding this comment.
The fix still works: in the #48361 reproduction (two background subagents, 15s each, run from source), the base exited after 5s without the results. With this pull request, run waited 20s, printed the parent's resumed FINAL: got TOKEN_A and TOKEN_B and exited 0. noninteractive.test.ts (15/15) and typecheck:tsc in packages/cli both pass.
The new tree-wide check found no new problems. A finished job is only removed after its session's wake-up is queued, so checking the queued wake-ups of every session that owned a job and the active sessions under the main one covers the handoff without a gap. The new subagent-shell test is small and doesn't depend on timing.
The open thread on unbounded waits now also covers servers started by subagents, since their jobs count too.
Public API: unchanged. The only behaviour change is that opencode run waits for background jobs anywhere in its session tree.
a24cc37 to
d851928
Compare
There was a problem hiding this comment.
This push is a rebase onto newer dev, and the pull request's own commit is unchanged since my last review. On the rebased head, the #48361 reproduction still works: run waits for the background subagents, prints the resumed FINAL: got TOKEN_A and TOKEN_B and exits 0. noninteractive.test.ts (15/15) and typecheck:tsc in packages/cli both pass. No new concerns.
Public API: unchanged. The only behaviour change is that opencode run waits for background jobs anywhere in its session tree.
Problem
opencode runexits as soon as the model's first reply ends. Theshelltool tells the model to start long work withbackground: trueand end its reply because it will be woken when the command finishes. Inrunthere is nobody left to wake: training runs, test suites and evals get cut off before the model sees their results.Change
After each reply,
runchecks the whole session tree withGET /api/experimental/job?sessionID=...&descendants=true: the main session, its subagents, their subagents, and every background shell or image any of them started.Visual
Out of scope
runwaits for them until they exit.Verification
bun test test/run/noninteractive.test.tsinpackages/cli: 15 pass, and 150 pass with--rerun-each 10bun run typecheck:tscinpackages/clioxlintandprettier --checkon the changed filesAdded by the opencode bot (AI-generated).
Fixes #48361
Fixes #59223
Related: #60286
Public API: opencode run now waits for background jobs anywhere in its session tree before exiting; no new flags or endpoints