Repository navigation
Conversation
|
The following comment was made by an LLM, it may be inaccurate: |
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
I reproduced the problem on the base and it is gone with this change. I ran the #54658 opencode-drive script with a simulated model against both checkouts. On the base, the parent gets only cancelled while the child replies DONE. With this PR, the parent gets one completed notification containing the reply. The core session, coordinator, job and subagent tests pass, and packages/core typechecks. The new steer test fails on the base as expected.
Public API: unchanged. SessionRunCoordinator.handoff and the extra continued argument to settled are internal to packages/core, and settled has a single implementer.
The approach looks sound. join follows the successor only when the interrupted execution was handed off and its exit is interrupt-only, so other interrupts and failures behave as before. Callers that join through run (for example a foreground subagent) now also wait for the resumed steer's result, which looks like the right behaviour. The remaining race you describe, where cleanup settles before handoff runs, still yields cancelled. That is acceptable as a known gap for this PR.
Related: #50751
|
Automated PR Cleanup Thank you for contributing to opencode. Due to the high volume of PRs from users and AI agents, we periodically close older PRs using automated criteria so maintainers can focus review time on the most active and community-supported contributions. This PR was closed because it matched the following cleanup criteria:
PRs created within the last month are not affected by this cleanup. If you believe this PR was closed incorrectly, or if you are still actively working on it, please leave a comment explaining why it should be reopened. A maintainer can review and reopen it if appropriate. Thanks again for taking the time to contribute. |
Issue for this PR
Closes #54658
Type of change
What does this PR do?
When an interrupt with
resume=truecontinues a background subagent, the parent now receivesstate="completed"with the subagent's reply. Before this fix, the parent receivedstate="cancelled".Two parts of
packages/corecaused the bug:settledhook inSessionExecutioncancels the session's job after every user interrupt.SessionRunCoordinator.runreturns the exit of the interrupted execution, even when a successor execution runs the steer prompt. The job takes that exit and recordscancelled.The fix adds
SessionRunCoordinator.handoff. If the next inbox item is a steer prompt,SessionExecution.interruptcallshandoffinstead ofwake.handoffmarks the interrupted execution as continued. Thesettledhook then skipsjobs.cancel, andrunreturns the exit of the successor.This also changes what other callers of
runget. A foreground subagent job, for example, now waits for the reply to the steer prompt and completes with it, instead of ending ascancelledat the interrupt.Interrupts without
resumework as before. If only a compaction or a move waits,interruptstill callswake, and the job stayscancelled. Withhandoffin that case, the parent would get an old reply marked as completed.Known gap:
SessionExecution.interruptreads the inbox after the interrupt starts. If cleanup of the interrupted execution finishes before that read, the parent still getscancelled. Cleanup stops the running tool and writes events, so it almost always takes longer than the read. Closing the gap needs a change tointerruptin the coordinator, so I left it out of this PR.How did you verify your code works?
packages/core/test/session-execution.test.ts: "a background child job follows an interrupt with continue through the resumed steer". It fails ondevwithcancelledand passes with this fix.packages/core, I ranbun teston the session, job, and subagent tests, andbun typecheck. At the root, I ranbun run check.opencode servefrom this branch and ran the steps from the issue. With a steer prompt in the inbox, the parent receivedstate="completed"and the reply. Without one, the parent receivedstate="cancelled".Screenshots / recordings
n/a
Checklist
Added by the opencode bot (AI-generated).
Related: #50751