Repository navigation
fix(opencode): emit a done record at the end of JSON runs - #50341
oldantique wants to merge 1 commit into
Conversation
|
The following comment was made by an LLM, it may be inaccurate: |
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The implementation cleanly gates done emission to JSON mode and idle completion, and the added subprocess tests cover the new terminal record and continuation edge cases.
Review effort: Lite
Findings: None
What changed in this PR
This PR improves the machine-readable contract of opencode run --format json by emitting a final terminal "done" record once the primary session reaches idle and the prompt/command request has completed, allowing consumers to reliably distinguish “completed” runs from truncated streams and to see any CLI auto-rejected permissions.
Changes:
- Add tracking of auto-rejected permissions during the run event loop and emit a final JSON
"done"record on idle completion. - Update JSON subprocess integration tests to assert the
"done"record, validatetimestamp/sessionIDinvariants, and cover continuation-after-stopwith tool calls. - Extend interrupt coverage to ensure
"done"is not emitted when the run is interrupted.
| File | Description |
|---|---|
| packages/opencode/src/cli/cmd/run.ts | Track auto-rejected permissions and emit a final done JSON record after the run completes and the session is idle. |
| packages/opencode/test/cli/run/run-process.test.ts | Add/adjust subprocess tests to assert the new done record and cover key JSON-stream edge cases. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
This branch has been upgraded to V2. We're no longer taking PRs for V1. If you think this is still relevant, please port it over and open it for V2. — from 𝕺𝖕𝖊𝖓𝕮𝖔𝖉𝖊 |
Issue for this PR
Closes #36413
Type of change
What does this PR do?
opencode run --format jsonhas no record that marks the end of a run, so a consumer can't tell a finished run from one that died after its last event. And an auto-rejected permission leaves nothing in the JSON stream (the warning only goes to stderr).This adds one final record, written once the primary session goes idle and the prompt/command request has returned:
{"type":"done","timestamp":…,"sessionID":"ses_…","reason":"idle","rejected":["bash"]}rejectedlists the permissions the CLI auto-rejected (deduped,[]if none).donemeans the run completed, not that it succeeded: earliererrorrecords and the exit code keep their meaning.It's written in
finish()rather than at the idle break, because the request can still fail after idle is seen, and thendonewould not be the last line. If the event stream ends without idle, or the loop throws, there's nodone. Exit codes, text output and--attachare unchanged (attach returns before the loop result today; I left that alone).On #44966: ending the loop on
step_finishwithreason: "stop"isn't safe, because some providers returnstoptogether with tool calls andSessionPromptkeeps going (see "loop continues when finish is stop but assistant has tool parts" in prompt.test.ts). The new test "waits for continuation after stop with tool calls" covers this. With #44966's change applied, the run stops after the firststep_finishand the continuation text is lost.How did you verify your code works?
From
packages/opencode:bun test test/cli/run/run-process.test.ts: 16 pass (13 before, plus 3 new tests: stop followed by a tool continuation, emptyinitcommand output, auto-rejected permission listed indone). The interrupt test now also checks that nodoneis written.run.tschange reverted, the 6 JSON-format tests fail, so the tests depend on the change.bun test test/cli: 371 pass, 0 fail.bun typecheck: clean.Screenshots / recordings
N/A
Checklist