Skip to content

fix(opencode): stop deriving opencode run status from the pane's process - #24472

Open
brennanb2025 wants to merge 6 commits into
mainfrom
brennanb2025/opencode-run-remove-detector
Open

brennanb2025 wants to merge 6 commits into
mainfrom
brennanb2025/opencode-run-remove-detector

Conversation

@brennanb2025

@brennanb2025 brennanb2025 commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 9 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​132 $\color{#cf222e}{\Huge{\mathbf{−}}}$​941 $\color{#cf222e}{\Huge{\mathbf{−}}}$​809
Prod 20 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​31 $\color{#cf222e}{\Huge{\mathbf{−}}}$​352 $\color{#cf222e}{\Huge{\mathbf{−}}}$​321

ELI5

Orca shows an agent's status (Working, Needs input, Done) from the agent's own hooks or plugins. That works the same way for every agent. #23722 added one exception: for OpenCode 2's one-shot opencode run "<prompt>" command, Orca watched the terminal's foreground process to guess when it was working. That special case also left a stale Done row after the command had exited. This PR removes it. opencode run now behaves like any other command that sends no agent hooks: no status row.

What it fixes and scope

  • Fixes: a Done row (sidebar, orca worktree ps, mobile) left behind after opencode run has exited back to the shell. Removes an agent-specific status path. Regression source: fix(opencode): report OpenCode 2 status from each pane's own TUI #23722.
  • The rule: agent status comes only from the agent's own hooks or plugins, plus the usual rule that clears a pane's row when its agent process exits. Done means a running agent window finished its turn; a command that has exited leaves no row.
  • User-visible change: an OpenCode 2 opencode run in a pane shows no status, before or after it exits. OpenCode 2 opencode mini already shows none. Both are client commands that load no Orca plugin in the pane.
  • Not affected: the full OpenCode window, which keeps reporting its own pane through its plugin (fix(opencode): report OpenCode 2 status from each pane's own TUI #23722); OpenCode 1 run, which reports through its own plugin; Claude, Codex and every other agent; and the shell's command start/finish markers, which other features still use.

What Changed

Before: on the shell's command-start marker, main read the pane's foreground process and command line. If it was opencode run, main posted Working, and on command finish it posted Done. Because the process had already exited by then, the Done either lingered (stale row) or was dropped, depending on timing.

After: that detector and everything that existed only for it are gone:

  • Removed from main:
    • the opencode run detector and its wiring on command start, command finish, the hidden-pane command-finished fact, and pane teardown;
    • the ordering queue that delayed its Done;
    • the foreground command-line readers it used (macOS/Linux and Windows).
  • Removed from the status store:
    • the "process-reported" provenance it needed, including a path that let a process-reported Working revive a retired pane past the launch-token check;
    • the rule that let a hook already reporting a command win over it.
  • Removed from the title tracker: a command-start callback whose only consumer was the detector.
  • Removed from the OpenCode 1 status plugin: a line that skipped the session-start post inside an OpenCode 1 run process. It existed only to stop the run's session-start post from blinking the pane idle between the detector's Working and the plugin's own Working. The plugin's bytes change, so installed copies are rewritten once on the next start.
  • Kept, with other users: the foreground-read timing constants (renderer agent tracker), the shell command start/finish scanner (renderer command lifecycle, terminal painting, launch-authority retirement), the "process" observation origin (renderer producers), and the opencode run command-line parser (isOpenCodeRunCommand), which OpenCode startup-prompt handling now uses (fix(opencode): submit admitted native startup briefs without overwriting input #24762).

Why

  • One status path for every agent. A per-agent detector is a second producer beside the agent's own hooks, with its own timing and ordering rules. That's where the stale Done came from. Removing it removes that class of bug, not just this instance.
  • Rejected: keep the detector and fix it (closed fix(opencode): show opencode run status when it starts in a hidden tab #24238, fix(opencode): clear an opencode run row when its process exits #24292). Those PRs made it clear its row on exit and see runs started in hidden tabs. They worked, but they kept an OpenCode-only producer alive.
  • Known gap, TEMPORARY: opencode run and opencode mini show no status. The way to cover them through the normal path would be OpenCode exposing a session's client environment (which includes Orca's pane ID) to plugins. OpenCode already receives it but doesn't expose it. That would need an upstream OpenCode change; none is filed.
  • Known gap, TEMPORARY (not changed here): the renderer keeps a status that arrives after the shell reports the command finished, so an agent's own Done that races its exit (for example OpenCode 1 run) can still be left behind. That rule predates fix(opencode): report OpenCode 2 status from each pane's own TUI #23722; a follow-up should decide whether a Done from the same turn is also dropped. This PR deletes the one test that pinned keeping it, because that test contradicts the rule above.

Linked Issue

Follow-up to Linear STA-8629 and #23722. Supersedes #24238 and #24292 (closed). Should ship with the #23722 cherry-pick (STA-9065).

Visual Proof

Live check with real OpenCode 2 on macOS, main vs this PR. A test also proves no row appears for opencode run, and that test fails on main's code.

Before (main), after opencode run has exited, the stale Done row:
main-a1-plus60.jpg

After (this PR), the same command after exit, no row:
pr-a1-plus60.jpg

After (this PR), the full opencode window still shows Done on its own pane:
pr-b-done.jpg

Testing

  • I manually tested these changes locally

  • Automated tests added/updated, or explained why not below

  • New src/main/runtime/opencode-run-pane-status.test.ts wires the runtime to a real status store, with opencode run … in the pane's foreground. It covers a visible pane and a hidden pane whose daemon reports the command finished. Neither produces a status event or row, and the command-finished fact still fires. With main's detector restored, both cases fail.

  • Related suites, 251 explicit files: runtime including orca-runtime.test.ts, all of src/main/agent-hooks, daemon transient facts, process foreground readers, OpenCode/OpenCode 2/relay plugin, shared command-marker and side-effect tests, and the renderer pty-connection, foreground-tracker and completion tests. All pass; one renderer cleanup test timed out once under load and passes on its own (it imports none of the changed code).

  • pnpm tc:node, oxlint, oxfmt and the changed-code quality check pass.

  • A renderer test that pinned "a Done arriving after the command finished survives" was deleted rather than kept, because it contradicts the rule above. That renderer behaviour itself is unchanged here.

Review

Agent skill upstream boundary

  • Not applicable, or this change follows docs/reference/agent-skill-sharing-upstream-boundary.md and copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

  • Correctness: removes a path that could revive a retired pane past the launch-token check.
  • Backwards compatibility: no wire or persisted change; the plugin file is rewritten once.
  • Performance: no more foreground command-line reads (a process-table query) on every command start.

Checklist

  • This PR is small and focused
  • I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
  • Before/after screenshots or videos attached for UI changes, or N/A with reason
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
  • pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

…ocess

Agent status comes only from the general mechanisms: an agent's own hooks
or plugins, plus the rule that clears a pane's row when its agent process
exits. Done means a live agent finished its turn; an exited process leaves
no row. #23722 broke that rule for one command: the runtime read a pane's
foreground process and argv after each OSC 133 command start and posted
Working, then Done, for an OpenCode `run`, which no hook reports.

Remove that special case and everything that existed only for it:

- the runtime producer (OpenCodeRunLifetimeStatus), its wiring into command
  start/finish, the daemon's background command-finished fact and pane
  teardown, and the after-facts queue that ordered its Done;
- the foreground argv readers it used (local, POSIX and Windows);
- the status store's process-origin terminal writes: the yield-to-hook
  window and the retired-pane revival a process Working could trigger;
- the title tracker's command-start callback, which had no other consumer
  (the OSC 133 scanner and its other start consumers are unchanged);
- the status plugin's skip of SessionStart inside an OpenCode 1 `run`
  process, which only hid an idle blink after the producer's Working.

An OpenCode 2 `opencode run` in a pane now shows no status, and its exit
clears nothing because nothing was posted. OpenCode 1 `run` reports through
its in-process plugin as before #23722. Other agents, the OpenCode 2 TUI
plugin, the server plugin's stand-down and the OpenCode 1 binder are
unchanged.
@brennanb2025

brennanb2025 commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor Author

Superseded: the current review status, including the passing live QA, is in the 2026-10-05 comment below: #24472 (comment)

Review status (head 9c32987)

The problem: #23722 added an OpenCode-2-only status path that watched the terminal's foreground process for opencode run "<prompt>". It posted Working when the command started and Done when it finished, and because the process had already exited by then, a stale Done row could stay in the sidebar, orca worktree ps and mobile after opencode run returned to the shell.

User-facing change: an OpenCode 2 opencode run in a terminal pane now shows no status row, during the run or after it exits, the same as opencode mini and any other command that sends no agent hooks. The full opencode window still reports Working then Done on its own pane through its plugin, and every other agent is unchanged.

Review result: no P0, P1 or P2 issues; no code changes were needed. Three independent reviewers (removal census, status store and OpenCode 1 plugin, renderer and tests) plus a readiness checklist found nothing blocking.

  • Removed code has no remaining callers. The runtime, foreground-process and status-store files this PR touches are byte-identical to their state before fix(opencode): report OpenCode 2 status from each pane's own TUI #23722, except where fix(opencode): report OpenCode 2 status from each pane's own TUI #23722's kept OpenCode 2 window plugin or later main commits changed them.
  • Terminal status ingest behaves exactly as before fix(opencode): report OpenCode 2 status from each pane's own TUI #23722. Without an event the status rules can never ask for a pane restart, so the simplified check drops nothing it used to accept.
  • The removed OpenCode 1 run session-start skip existed only for the detector. The session-start row it re-allows raises no notification, unread mark or completion, the same as every OpenCode 1 window's first prompt.
  • The new opencode-run-pane-status.test.ts fails 2/2 against main's code, which still has the detector, and passes on this branch. The deleted renderer late-Done test pinned behaviour that contradicts "an exited process leaves no row", and no remaining feature depends on it.
  • No per-command-start process-table read remains, and nothing in the wire format or saved data changes.

Fixed during review: PR description wording only. It now says correctly when the old OpenCode 1 skip's idle blink happened, relabels the launch-token note as correctness, and records the pre-existing late-Done gap below as TEMPORARY.

Deferred:

  • P3, caused by this PR: the new test hardcodes the renderer's foreground read timing (350 + 1200 + 6000 ms) instead of deriving it from FOREGROUND_COMMAND_READS. This is a nit.
  • P3, not caused by this PR (the rule dates from Infer interrupted agent status from terminal input #2263): the renderer keeps a status that arrives after the shell reports the command finished (interrupt-input-intent.ts:101). So an agent's own Done that races its exit, for example OpenCode 1 run, can still be left behind. This needs a follow-up to decide whether a Done from the same turn should also be dropped.
  • TEMPORARY, already in the description: OpenCode 2 run and mini show no status until OpenCode exposes a session's client environment to plugins.

Verified:

  • CI is green (17 passed, 16 skipped).
  • About 2,400 tests across the explicit related suites pass on this branch: runtime, agent-hooks, OpenCode plugins, relay overlay, foreground process, shell command markers, and renderer terminal-pane suites.
  • An ablation: the new test was run against main's code to confirm it catches the old detector.

Not verified: live Electron QA with real OpenCode 2 has not run. The remote QA machine's agent driver is out of usage, and no other allowed driver was available. A remote rig with main and this branch built side by side is set up and ready to resume. The PR stays a draft until that QA passes.

Conflict resolutions:
- status-plugin-runtime-state-source.ts: keep main's isOpenCodeCommandProcess
  (main's plugin post source now uses it for "serve"); drop the detector-only
  isOpenCodeRunProcess wrapper.
- opencode-headless-command.ts/.test.ts: keep main's versions; main uses
  isOpenCodeRunCommand/findOpenCodeRunCommand for startup-prompt handling.
  Only the comment describing the removed status detector changes.
- pty-connection-command-finished-late-done.test.ts: stays deleted; main only
  made its setup await installTerminalTestGlobals.
@brennanb2025

brennanb2025 commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor Author

Review status, 2026-10-05 (head 9405f9b, supersedes the 10-01 status)

The problem: #23722 added an OpenCode-2-only status path that watched the terminal's foreground process for opencode run "<prompt>". It posted Working when the command started and Done when it finished, but by then the process had already exited, so a stale Done row could stay in the sidebar, orca worktree ps and mobile after opencode run returned to the shell.

User-facing change: an OpenCode 2 opencode run in a terminal pane now shows no status row, during the run or after it exits, the same as opencode mini and any other command that sends no agent hooks. The full opencode window still reports Working then Done on its own pane through its plugin, and every other agent is unchanged.

Code review: no P0, P1 or P2 issues. Three independent reviewers (removal census, status store and OpenCode 1 plugin, renderer and tests) plus a readiness checklist found nothing blocking. Details are in the 10-01 status comment above; all of it still holds after the main merges below.

Changed since 10-01:

  • Merged main to clear conflicts, then again several times to pick up main's own test and type fixes (only the first merge had conflicts). The PR's own diff is unchanged: the same 29 files, +163/−1293.
  • One merge decision: main had given isOpenCodeRunCommand three new users unrelated to status (OpenCode startup-prompt handling, fix(opencode): submit admitted native startup briefs without overwriting input #24762). So src/shared/opencode-headless-command.ts and its test now stay, with only the comment that described the removed detector corrected. Main's generalized isOpenCodeCommandProcess in the OpenCode plugin stays too, because fix(opencode): keep shared-session TUI activity with its owning pane #24962 uses it; only the run wrapper and its session-start skip are removed.
  • After the merges: tc:node, tc:web and tc:cli show 0 errors. 118 related test files (2,716 tests) pass.

Live Electron QA (passed): run on a separate Mac with real OpenCode 2.0.21, comparing main (990b0ed) against this branch (c4f606f; later merges changed none of the PR's files). Each build was confirmed by string counts in its bundle: the removed detector's strings appear in main's bundle (2/3/3) and not at all in the PR's (0/0/0). An in-page sampler checked the status rows every 250 ms. All free OpenCode models were rate-limited (HTTP 429), so OpenCode talked to a local test model that replies after 20 s. OpenCode itself, its plugin and Orca were all real.

  • PR, opencode run: no row in the status store, the sidebar or worktree ps at any sample, whether in a visible pane, with a worktree ps during the run, in a hidden tab, or in a fresh pane after the full window had run.
  • PR, full opencode window: Working, then Done on its own pane, and the row cleared when the process exited.
  • Main, opencode run in a visible pane: Working, then a Done that was still showing two minutes after the command had exited. This is the bug.
  • Main also misses rows in two cases, which this PR makes consistent:
    • an opencode run in a hidden tab shows no row at all;
    • a run in a fresh pane shows Working, then nothing at exit.
  • A Claude turn looks identical on both builds. The test install of Claude had no Orca hooks, so this shows the PR leaves Claude's display unchanged, not that hook-driven status works.

Main, after the visible opencode run exited (stale Done still showing):
main-a1-plus60.jpg

This PR, during the same opencode run (no row):
pr-a1-mid.jpg

This PR, after it exited (no row):
pr-a1-plus60.jpg

This PR, three opencode run commands that ran in a background tab, opened afterwards (no row):
pr-a2-opened.jpg

This PR, full opencode window after its turn (Done on its own pane):
pr-b-done.jpg

This PR, after the full window exited (row cleared):
pr-b-exited.jpg

Deferred (unchanged from 10-01):

  • P3 nit: the new test hardcodes the renderer's foreground timing instead of using FOREGROUND_COMMAND_READS.
  • P3, not caused by this PR (rule from Infer interrupted agent status from terminal input #2263): the renderer keeps a status that arrives after the command-finished marker (interrupt-input-intent.ts:101). It needs a follow-up and is labelled TEMPORARY in the description.
  • TEMPORARY: OpenCode 2 run and mini show no status until OpenCode lets its plugins see a session's client environment.

CI (head 9405f9b): green. Typecheck, all five test shards, relay integration, the macOS and Windows packages, managed hooks on Node 18 and verify all pass. One supplementary full-unit-test check was still queued when I last checked. Earlier red runs on this branch came only from breaks on main itself, each fixed on main and merged in: a test using an outdated type shape (#24991 vs #25073, fixed by #25706, #25709, #25717 and #25728), and a prompt-carry test that broke after #23672 (fixed by #25734).

Not verified: the live QA covered macOS only (no Windows, Linux, SSH or WSL panes; the removed detector never ran in SSH or WSL panes anyway). It also didn't test a hook-driven Claude turn.

@brennanb2025
brennanb2025 marked this pull request as ready for review October 6, 2026 03:09
@coderabbitai

coderabbitai Bot commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 379dffb3-a33c-4861-8433-2120a7b719ee
📥 Commits

Reviewing files that changed from the base of the PR and between a47e0f5 and 9405f9b.

📒 Files selected for processing (29)
  • src/main/agent-hooks/server-process-lifetime-status.test.ts
  • src/main/agent-hooks/server/server-ingest-terminal.ts
  • src/main/agent-hooks/server/server-status-disposition.ts
  • src/main/opencode/hook-plugin-run-process-session-start.test.ts
  • src/main/opencode/status-plugin-factory-source.ts
  • src/main/opencode/status-plugin-runtime-state-source.ts
  • src/main/providers/agent-foreground-command-line.test.ts
  • src/main/providers/agent-foreground-process.ts
  • src/main/providers/windows-agent-foreground-command-line.test.ts
  • src/main/providers/windows-agent-foreground-command-line.ts
  • src/main/providers/windows-agent-foreground-process.ts
  • src/main/runtime/local-pty-foreground-command-line.ts
  • src/main/runtime/opencode-run-lifetime-runtime.test.ts
  • src/main/runtime/opencode-run-lifetime-status.test.ts
  • src/main/runtime/opencode-run-lifetime-status.ts
  • src/main/runtime/opencode-run-pane-status.test.ts
  • src/main/runtime/orca-runtime-apply-tracked-pty-title.ts
  • src/main/runtime/orca-runtime-create-terminal-side-effect-command-code-detector.ts
  • src/main/runtime/orca-runtime-emit-daemon-pty-transient-fact.ts
  • src/main/runtime/orca-runtime-get-unpersisted-tracked-title-for-pty.ts
  • src/main/runtime/orca-runtime-schedule-wait-blocked-check.ts
  • src/main/runtime/orca-runtime-stop-requested-pty-ids.ts
  • src/main/runtime/runtime-terminal-contracts.ts
  • src/main/runtime/runtime-terminal-state-records.ts
  • src/renderer/src/components/terminal-pane/pty-connection-command-finished-late-done.test.ts
  • src/shared/foreground-command-settle.ts
  • src/shared/opencode-headless-command.ts
  • src/shared/terminal-output-side-effects.test.ts
  • src/shared/terminal-output-side-effects.ts
💤 Files with no reviewable changes (18)
  • src/shared/terminal-output-side-effects.test.ts
  • src/main/providers/windows-agent-foreground-command-line.test.ts
  • src/main/providers/agent-foreground-command-line.test.ts
  • src/main/opencode/hook-plugin-run-process-session-start.test.ts
  • src/main/agent-hooks/server-process-lifetime-status.test.ts
  • src/renderer/src/components/terminal-pane/pty-connection-command-finished-late-done.test.ts
  • src/main/runtime/opencode-run-lifetime-runtime.test.ts
  • src/main/providers/windows-agent-foreground-command-line.ts
  • src/main/opencode/status-plugin-factory-source.ts
  • src/main/runtime/orca-runtime-stop-requested-pty-ids.ts
  • src/main/runtime/runtime-terminal-state-records.ts
  • src/main/runtime/opencode-run-lifetime-status.test.ts
  • src/main/runtime/orca-runtime-apply-tracked-pty-title.ts
  • src/main/runtime/runtime-terminal-contracts.ts
  • src/main/runtime/orca-runtime-emit-daemon-pty-transient-fact.ts
  • src/main/runtime/local-pty-foreground-command-line.ts
  • src/main/runtime/opencode-run-lifetime-status.ts
  • src/main/runtime/orca-runtime-get-unpersisted-tracked-title-for-pty.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 2 remain after this review.


📝 Walkthrough

Walkthrough

The generated OpenCode plugin now enqueues SessionStart for run processes. Terminal status ingestion applies the status disposition without process-origin overrides or provenance fields. The process-based OpenCode run status tracker and its foreground command-line helpers were removed. Terminal side-effect tracking retains command-finished reporting but no longer reports command-start events.

Priority: ➖ Normal

Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 9405f

The changed status behavior and command-completion paths show no demonstrated merge-blocking issue. Merge after normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 9405f

The inspected changes preserve identity checks and remove a status-acceptance exception. No newly introduced security weakness was identified, but incomplete coverage leaves some uncertainty.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • observed — The inspected terminal producer derives target identities from leaves associated with the emitting PTY, or its stored pane record. Accepted observations affect pane status and associated session identity; payloads do not supply an independent target pane to this producer.

Security Findings and Attack Paths

  • inferred — No new status-authority bypass was identified in the inspected paths. Removing processNewTurn eliminates an exception to launch-token validation and a separate means of reviving retired status; terminal observations now require accept rather than handling restart.

Trust Boundaries and Controls

  • observed — Generated SessionStart uses the existing loopback hook POST with a hook-token header and environment-derived pane, tab, worktree and launch identity. Hook ingress validates authentication and envelope identity before applying status; the new producer does not introduce a direct mutation path around those controls.

Resilience and Maintainability Implications

  • observed — Lifecycle delivery remains best-effort: the generated POST has a timeout, returns false on transport failure and clears its timer. Queued SessionStart checks disposal before posting. These controls contain reporting failures rather than making status delivery a prerequisite for the agent run.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 30.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 11 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely identifies the main change: removing OpenCode run status derived from the pane’s process.
Description check ✅ Passed The description follows the template and explains the change, rationale, user impact, linked issues, visual proof, testing, and known gaps. The remaining unchecked items do not make it incomplete.
Linked Issues check ✅ Passed Direct issue #24238 is closed and provides historical context only. No active directly linked issue remains, so no linked-issue coding requirements apply.
Out of Scope Changes check ✅ Passed The removed detector, process-status provenance, foreground command-line readers, and related runtime wiring support the stated removal of the OpenCode 2 opencode run status path. The test and plugi…
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant