Repository navigation
fix(opencode): re-derive Working from OpenCode 2 step events - #23717
brennanb2025 wants to merge 4 commits into
Conversation
Review status: ready for human review (head 0a5fbe1)Verdict: the fix does what the issue asks. After OpenCode 2 reloads its plugins mid-turn, answering the agent's question no longer shows Done (or stays on Needs input) while the agent keeps working. Four review loops ran; the last returned clean. CI is green and Commits added during review:
Architecture check: after repeated reload variants, a separate review challenged the approach. Verdict: keep this PR's approach. The OpenCode 2 plugin API cannot list running sessions or open questions, so a fresh plugin must re-derive "working" from the next live event. Handing the old plugin copy's state to its replacement would also fix a question already on screen at reload, but it only helps when OpenCode reuses the loaded module and it changes the unload path #23700 owns, so it belongs with #23700 or a follow-up. Live QA on a remote Mac, real OpenCode 2.0.18, hidden Orca builds that both include #23700 and differ only by this PR. Prompt: ask one question, then
Known weaknesses (documented in the description, not fixable from the plugin in OpenCode 2's current API): a question already on screen at reload keeps its last status until the next model step (1.1 s live); a subagent's question raised after a reload shows Done after the answer until that subagent's next step. Open, not caused by this PR:
Not run: Windows, Linux, SSH. The change is generated plugin JavaScript with no platform code, and SSH and WSL hosts receive the same source. Marked ready for review; the PR has not been merged, and auto-merge is not enabled. |
|
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 3 remain after this review. 📝 WalkthroughWalkthroughThe lifecycle handler now records a factory as the busy owner for a root session when a tool-raised blocker has no existing owner. OpenCode2 setup translates Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to The changes target stale OpenCode 2 status reporting and add lifecycle regression coverage. No actionable merge-blocking risk is established in the supplied context. Architecture SummaryArchitecture risk: 🔵 Low · up to The change affects 1 system. Changed systems: Architecture concerns Review detailsSystems and components
Before / after behavior
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
ℹ️ No critical issues — one low-severity robustness note inline.
Reviewed changes
- Step-event remap —
status-plugin-setup-source.tsmaps OpenCode 2'ssession.step.startedtosession.next.step.started, which normalizes to a busysession.status, re-deriving Working for a plugin reloaded afterexecution.started. - Tool-raised blocker ownership —
status-plugin-lifecycle-source.tsrecords the factory as busy root owner when a root-session permission/question carries a tool marker and no owner exists, so answering returns the pane to Working until the turn's Idle. - Marker predicate —
isToolRaisedBlockerrecognisesproperties.tool,metadata.tool, andsource.type === "tool"; child sessions and unknown-lineage sessions are excluded. - Tests — three test files cover reload-before/inside/after-question, cancel+interrupt, API-created forms, and cross-factory no-takeover; digests re-pinned for the opencode and mimo variants. All 285 tests in
src/main/{opencode,opencode2,mimo}pass.
DeepSeek Flash (free via Pullfrog for OSS) | 𝕏
| '// tool, 2.x form metadata.tool or permission source) proves that turn is running;', | ||
| '// an API-created blocker can sit on an idle session and proves nothing.', | ||
| 'function isToolRaisedBlocker(properties) {', | ||
| ' return Boolean(properties?.tool || properties?.metadata?.tool || properties?.source?.type === "tool");', |
There was a problem hiding this comment.
The rule this predicate guards leans on the tool marker as proof the blocker came from a running tool, but properties.metadata is a freeform record the public POST /api/session/:sessionID/form accepts from any client, so a form can carry a client-set metadata.tool while owning an idle session. In that case the new rule latches busyRootOwnerBySessionID with no turn to emit Idle, and the pane stays Working after the reply until the next turn ends or the plugin is unloaded. The repo's own reproduction doc (docs/bug-reproductions/opencode2-form-created-kinds/README.md:22-27) notes metadata is optional and unvalidated, and the API-created-form test at hook-plugin-opencode2-setup.test.ts:594 uses a form with no metadata, so it does not cover a marker-carrying one.
Technical details
# Marker is client-spoofable, so it is not proof of a running turn
## Affected sites
- `src/main/opencode/status-plugin-ownership-source.ts:127-128` — `isToolRaisedBlocker` accepts any `metadata.tool`.
- `src/main/opencode/status-plugin-lifecycle-source.ts:102-104` — the rule sets `busyRootOwnerBySessionID` with no turn-liveness corroboration.
- `src/main/opencode2/status-plugin-setup-source.ts:47-65` — `form.created` spreads the client-supplied `form` (including metadata) into `question.asked` for any real session.
## Required outcome
- A blocker that is not backed by a running turn must not leave a busy owner that outlives the reply. Either corroborate the turn (e.g. only set the owner when a busy/step signal for the session is otherwise in flight) or bound the record so a reply with no turn clears it.
## Open questions for the human
- Is `metadata.tool` reachable in practice from anything other than the `question` tool, given `metadata` is `Record(String, Unknown)` and the form API is public? If yes, this is the same latch the PR rejected for "treat every question/permission as proof", just requiring a forged marker.0a5fbe1 to
49f9e9f
Compare
OpenCode 2 emits session.step.started, but the plugin only mapped the older session.next.step.started name, so per-step Busy never fired. A plugin instance re-created mid-turn (OpenCode reloads every plugin when the plugins dir changes) never sees execution.started, so answering a question left the pane on Needs input, or published Idle, until the turn ended. Forward session.step.started to the existing step mapping. Tool events stay unmapped: a parallel tool reports progress while a sibling question in the same session is still pending, and Busy would clear that blocker.
…lugin reload When OpenCode 2 reloads plugins inside the step that raises a question or permission, the fresh plugin instance never saw that turn start. Answering cleared its only record, so the pane showed Done until the next step began, which waits for every sibling tool in the step to finish. A blocker raised by a tool call proves its turn is running, since OpenCode runs tools only inside a turn. When such a blocker belongs to a known root session with no recorded Busy owner, record one. Nothing new is posted, because the blocker outranks Busy. The reply then shows Working, and the turn's own end or plugin disposal retires the record like any Busy. A blocker created through OpenCode's HTTP API can sit on an idle session, so blockers with no tool origin are left out. Subagent and unknown-lineage blockers are unchanged, and no tool event is mapped, so a sibling tool can never hide a pending blocker.
…er factory's Busy The blocker rule records Busy only when no owner is known. Removing that guard let a second factory claim the owner, so its disposal erased Busy for a turn the first factory was still running and the pane fell to Done. Only the pinned digest caught that ablation before this test.
49f9e9f to
7e0b915
Compare
|
Closing in favor of #23722, which merged as 3727100. This PR fixed a symptom of how the old design tracked status: after OpenCode 2 reloaded Orca's status plugin mid-turn, the fresh copy started with no memory of the running turn and had to re-learn "working" from later events. #23722 removes that cause. Each pane's own OpenCode window now reports its status, read straight from OpenCode's session data, so a plugin reload can no longer lose it. For OpenCode 2 panes, the shared server's copy of the plugin no longer reports at all. On main, the only path this PR still touches is the shared-server fallback, which runs only on an SSH host that hasn't received the per-pane plugin yet, and Orca now refreshes that plugin on connect. Reopen if a case turns up where that fallback matters. |





What this fixes
After OpenCode 2 reloads its plugins in the middle of a turn, answering the agent's question made the Orca pane show Done (or stay on Needs input) while the agent kept working.
/reload.ELI5
OpenCode 2 can throw away Orca's status plugin and load a fresh copy while the agent is still working. The fresh copy never saw "the agent started working", so when you answered the agent's question it concluded the agent was done. Now the fresh copy re-learns "working" from things that can only happen while the agent works: the start of each model step, and a question or permission prompt raised by one of the agent's tools.
What Changed
Before (user): after a mid-turn plugin reload, answering the agent's question showed Done for the rest of the turn (about 25 s in a capture), or left the pane on Needs input until the turn ended (about 27 s), while the agent kept working.
After (user):
Mechanism:
session.next.step.startedas "working", but OpenCode 2 names that eventsession.step.started, so the rule never fired. The plugin now forwards the OpenCode 2 name. OpenCode 2 publishes a step event only inside a running turn, and never in a session while that session's question is open, so the new Working cannot outlive the turn or hide a pending question.metadata.toolon the question tool's form,source.type: "tool"on tool permissions,toolon OpenCode 1.x requests). Blockers without a marker keep the old behaviour.Performance: one more event type per model step reaches the engine, and the session lookup it needs is cached. Working is re-posted at most once per model step, and only when an assistant-text preview was posted since the last status post. The text-streaming path is unchanged.
Why
session.retry.scheduled. The step that follows a retry already restores Working, and a retry could not be produced live./reloadand Orca plugin updates load a fresh copy, where this PR's step rule is still the only recovery), and it changes the unload path fix(opencode): keep OpenCode 2 panes Working across plugin reloads #23700 owns, so it belongs with fix(opencode): keep OpenCode 2 panes Working across plugin reloads #23700 or a follow-up.Linked Issue
Follow-up to Linear STA-8629 and #23700. Both PRs change the pinned plugin digests in
hook-service.test.ts(OpenCode and mimo-code), so whichever merges second needs both regenerated.Visual Proof
Real OpenCode 2.0.18 in a hidden Orca build on a remote Mac. Both builds include #23700; the only difference between them is this PR. Prompt: "Ask me one multiple-choice question with your question tool, then run: sleep 30, then reply FINISHED." Each screenshot is taken during
sleep 30, after the question was answered. Sidebar icons: yellow spinner = Working, orange question mark = Needs input, green check = Done.Plugin reload before the question. Before: Done for 33.5 s while the agent worked. After: Working until FINISHED.
Plugin reload inside the step that asks the question. Before: Done for 36.6 s. After: Working, with no Done flash (sampled every 100 ms).
Plugin reload while the question is on screen. Before: Needs input for 32.3 s after the answer, until the turn ended. After: Needs input for 1.1 s after the answer (until the next model step), then Working.
No reload (control). Identical on both: Working during the sleep, Done at FINISHED.
Testing
I manually tested these changes locally
Automated tests added/updated, or explained why not below
hook-plugin-opencode2-setup.test.tsruns each case against both theopencodeandopencode2plugin variants, with event orders taken from OpenCode 2.0.14: reload while a question is pending; question raised after a reload; question or permission raised in the step the reload joined while a sibling tool runs; cancel followed by interrupt; a form no tool raised; a pending question that keeps blocking while a sibling tool reports progress.hook-plugin-lifecycle-delivery.test.ts: a tool-raised blocker seen by a second plugin instance never takes over the first instance's running turn.An existing test now ends its cancelled tool-raised form with the turn's end, which is the order OpenCode 2 produces.
Ablations, each with the fix reverted and then restored: without the step mapping, 4 failures; without the blocker rule, 6; without the tool-marker check, 2; with the subagent exclusion removed, 6 existing subagent tests fail; without the no-takeover check, the new instance test fails with a false Done; mapping
session.tool.progressto Working fails the pending-question tests.src/main/opencode,src/main/opencode2,src/main/mimo: 285 passed, 1 skipped.pnpm tc:node, oxlint and the changed-code quality gate pass.Live QA on a remote Mac with real OpenCode 2.0.18 (see Visual Proof): all three reload cases now end on the expected status, the no-reload turn is unchanged, and a
/reloadtyped mid-turn stayed Working until the turn ended.Not run: Windows, Linux, SSH. The change is generated plugin JavaScript with no platform-specific code, and SSH and WSL hosts receive the same source.
Review
Agent skill upstream boundary
docs/reference/agent-skill-sharing-upstream-boundary.mdand copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.Notes
session.next.*names are unchanged, and there is no wire or schema change.Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)