Skip to content

fix(agent-status): keep an opencode run's Done when its command end retires the pane - #24252

Closed
brennanb2025 wants to merge 3 commits into
mainfrom
brennanb2025/opencode-run-lost-done
Closed

brennanb2025 wants to merge 3 commits into
mainfrom
brennanb2025/opencode-run-lost-done

Conversation

@brennanb2025

@brennanb2025 brennanb2025 commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 2 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​319 $\color{#cf222e}{\Huge{\mathbf{−}}}$​2 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​317
Prod 5 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​101 $\color{#cf222e}{\Huge{\mathbf{−}}}$​21 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​80

ELI5

When you run opencode run "<prompt>" in an Orca terminal, the sidebar should show Working while it runs and Done when it finishes. In practice the row usually just disappeared at the end and never showed Done. That happened once anything had asked Orca for its terminal list: orca worktree ps, orca terminal list, an agent using orchestration, the phone app refreshing, or an automation. Now the Done shows up and stays.

What it fixes and scope

  • Fixes: a finished OpenCode 2 opencode run in a terminal pane loses its Done, and its row disappears. This happens on main today, in visible tabs and in tabs hidden before the run ends. It was introduced when fix(opencode): report OpenCode 2 status from each pane's own TUI #23722 added opencode run status, and a false "restored" mark (described below) makes it frequent.
  • Surface: the sidebar, orca worktree ps and mobile status rows for opencode run panes on the local machine.
  • Not affected: full OpenCode windows, other agents, SSH and WSL panes (the opencode run detector only runs for local panes), and stale or late updates from agents that already ended. Those are still ignored exactly as before.

What Changed

Before (user): run opencode run "…" in a pane after anything has listed terminals. The row shows Working, then disappears when the run ends. Done never appears.

After (user): the row goes from Working to Done when the run ends and stays there, as it does without a terminal-list refresh. Running the next command in that pane clears it, as it already does for other agents.

Mechanism, before:

  1. Listing terminals (the refresh behind terminal list, worktree ps, orchestration, the mobile refresh and automations) marks every live terminal as having "restored launch authority" (orca-runtime-refresh-pty-worktree-records-with-controller-inventory.ts). That is Orca's record that it launched an agent in the pane. It does this even for panes it never launched anything in.
  2. When a command in such a pane ends (the shell's command-end marker), Orca retires that launch authority. Retiring closes the pane's status record and deletes its rows, which is how Orca ignores late updates from a launch that has ended.
  3. fix(opencode): report OpenCode 2 status from each pane's own TUI #23722's opencode run detector posts its Done on that same command end, after the retirement. The status store sees a closed pane and drops the Done. The sidebar then removes the Working row.

Mechanism, after:

  • When retirement deletes a pane's rows, it now remembers the run that was live there: its state, who reported it (a hook or the terminal) and when (in memory only, never persisted).
  • On a retired pane, the status store accepts the detector's Done once, to settle that remembered run. It does not if the run had already finished, or if an agent hook reported during the run (the same "a hook owns this command" rule a live pane already uses). The pane stays retired, so every other late post is still ignored, and a closed tab still ignores the Done.
  • A new run in the pane clears the remembered run. A later retirement carries it over only while the pane is still retired.
  • Files: server-authority-fences.ts, server-status-disposition.ts, server-ingest-terminal.ts, server-types.ts, server-state.ts.

Why

  • Reordering doesn't work. Posting Done before the retirement was tried in both orders against the failing test, and both still lost the Done. Retiring deletes every row for the pane, including a Done that has just landed. And the visible-pane Done is deliberately posted after the command-finished event, so the sidebar doesn't drop it.
  • Settle by identity, not by time. An earlier version of this PR compared the retirement's clock time with the run's start time. A clock change during a run (for example a network time correction) could still lose the Done, and the comparison let a Done through even when an agent hook owned the command. Remembering the exact run the retirement deleted answers "is this Done the end of that run?" with no clock involved.
  • Narrow. Only a Done from the opencode run detector, for the run the pane showed when it retired, gets through. Late hook posts and terminal status stay fenced.

Differences from the common pattern

  • Intended: retirement still deletes the ended pane's row and re-admits only this one settlement, instead of keeping ended rows. Deleting an exited agent's row is existing Orca behaviour, and this PR doesn't change it.
  • Intended: the store settles the run from the row it captured before deleting it, rather than the detector handing the store a run id. No new interface between the terminal runtime and the status store is needed, and the capture is the same "record what was live before teardown" idea.
  • Temporary: the Done must still arrive after the command-finished event, because the sidebar drops an unchanged row on command-finished. This goes away when the sidebar becomes a pure reader of the host's status store.

Known follow-ups, not in this PR

  • The false "restored" mark. The terminal-list refresh should not mark panes that were never restored. On every refresh it re-marks every pane, so every command end in that pane retires it again. That is a separate change for the orchestration owners. It may also give restored authority back to a pane whose authority was already retired; that has not been verified. This PR is still needed after that fix, because panes with genuine launch authority also retire at the end of a run.
  • Retirement sends no clear to the sidebar. Retirement deletes the store's rows but sends nothing to the sidebar, so the desktop can briefly show a row that worktree ps and mobile no longer have. This already happens on main, and the opencode run flow ends in agreement with this PR. It is left for its own change.

Linked Issue

Follow-up to Linear STA-8629 and #23722. Found by the review QA of #24238. #24238 and the restart follow-up (branch brennanb2025/opencode-run-reattach-liveness) depend on this for Done after a hidden-tab start or a restart.

Visual Proof

Real OpenCode 2 (2.0.21) in Orca terminal panes on a remote macOS machine. Each run ran orca worktree ps first, then opencode run "…" (about 35 s). Right sidebar collapsed.

Before (main): Working while it runs, then the row is gone and there is no Done.

main: Working during the run
main: row gone after the run

After (this PR, e49dfb5): Working while it runs, then Done that is still there 60 s later. orca worktree ps shows the same Done row.

PR: Working during the run
PR: Done 60 s after the run

Tab hidden before the run ends (this PR): Done is kept while hidden and after the tab is reopened.

PR: hidden tab, Done 60 s after the run

A second refresh after a kept Done, then a new run in the same pane (this PR): Working, then Done again.

PR: second refresh then a new run, Done kept

Testing

  • Manually tested on a remote macOS machine with real OpenCode 2 in Orca terminal panes (see Visual Proof)

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

  • New opencode-run-done-after-launch-authority-retirement.test.ts connects a real runtime to a real hook server, wired as in the app. It covers a terminal-list refresh followed by opencode run in the visible pane and in the hidden pane, plus the wall clock stepping back during the run. Without the fix all of them read "missing".

  • server-process-lifetime-status.test.ts adds cases for:

    • after the Done is kept, a late OpenCode hook post and terminal status are still ignored;
    • a run an agent hook owned (hook still working, or already done) stays as on main;
    • a hook post after retirement but before the Done is ignored;
    • a closed tab still ignores the Done;
    • the Done settles the run only once;
    • a run that had already ended doesn't let a Done in;
    • a pane retired again with no rows left still settles its run;
    • a pane that left retirement doesn't carry an old run over.
  • The existing test that keeps a lone process Done out of a retired pane is unchanged and passes.

  • Ablation: removing each guard (the accept, one-shot, already-ended check, hook check, new-run clearing, carry-over, still-retired check) fails exactly its test.

  • 64 related test files pass (626 tests). pnpm tc:node, oxlint and the changed-code quality check pass.

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

  • Security: stale or late posts stay ignored, and tests cover that. No new inputs.
  • Cross-platform / SSH: the change is in the host's status store; the detector only runs for local panes, so SSH and WSL behaviour is unchanged. Windows uses the same code path.
  • Backwards compatibility: no wire or persisted change; the remembered run is in memory only.
  • Performance: a few field reads per status post on a retired pane.

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)

…etires the pane

A pane that holds launch authority (including the restored receipt any
terminal list / worktree ps / mobile refresh mints) retires it at the
133;D that ends `opencode run`. Retirement closed the pane for status and
the host's own process Done for that run was then suppressed, so the row
went Working -> removed with no Done.

The status store now accepts a process-lifetime Done on a retired pane
when the run started before the retirement, without lifting the fence:
late hook and OSC posts stay suppressed, and a closed tab still wins.
…not by the clock

The previous commit admitted a process-lifetime Done on a retired pane when
the run started before the retirement, comparing two wall-clock reads. A
clock stepped backward during the run lost the Done again, and because
retirement deletes the row the hook-yield rule reads, a process Done for a
command a hook owned (OpenCode 1 `run` with its in-process plugin) was
written where a live pane would yield.

Retirement now records on its fence which run the pane showed (agent,
state, who reported it, when), carried over when a later retirement of the
same command end finds the rows already gone. A process Done on the retired
pane is accepted once, only when that run was still live and no hook
reported during it (the same rule a live pane applies). The fence is not
lifted, a new run invalidates the record, and a closed tab still wins.
… retired

A retirement that finds no rows inherited the previous fence's recorded run
even after the pane had left retirement without a new-turn restart (moved
to another tab and back). A later hook-owned run whose row was dismissed
could then have its process Done admitted with an empty prompt, where main
shows nothing. The carry-over now applies only while the pane is still
retired under that fence, the same condition the key merge uses.

Also drops the unused agent type from the recorded run and renames the
clock tests to describe behaviour.
@brennanb2025

Copy link
Copy Markdown
Contributor Author

Review status: ready (e49dfb5)

The problem. After anything lists terminals (orca worktree ps, orca terminal list, orchestration, the phone refresh, automations), every live pane gets a false "restored launch authority" mark. When an opencode run (OpenCode 2) ends, that mark makes Orca retire the pane: it closes the pane's status record and deletes its rows. The run's Done then arrives at a closed pane and is dropped, so the sidebar shows Working and then the row just disappears.

What changes for you. opencode run in a terminal pane now goes from Working to Done and stays Done, after a refresh too, in a visible tab and in a tab hidden before the run ends. orca worktree ps and mobile show the same Done. Running the next command in the pane clears it, as on main.

What the review changed. The first version accepted the Done when the run's start time came before the retirement's clock time. The review replaced that with the design in the PR body: when retirement deletes the rows, it now remembers the run that was live in the pane, and the run's Done settles that run once. Nothing compares clocks anymore.

  • Fixed (P3): a system clock change during a run could still lose the Done (2b16b5f).
  • Fixed (P3): on a retired pane the "an agent hook owns this command" rule stopped working, so an OpenCode 1 run would show an empty-prompt Done where main shows nothing (2b16b5f).
  • Fixed (P3, introduced by the review's own fix): a pane moved between tabs could carry an old remembered run into a later retirement (e49dfb5).

Deferred (P3, not fixed here).

  • If something other than the run's own end (for example a pane unmount) retires the pane mid-run, that run's Done is still shown once. The Done is truthful.
  • If more than 1024 retired pane keys pile up, an evicted retirement record makes the Done drop, as on main.
  • Retirement deletes the store's rows without telling the sidebar. This already happens on main; the opencode run flow ends with both agreeing. It needs its own change.
  • The false "restored" mark itself belongs to the orchestration owners. Every refresh re-marks every pane, so each later command end in that pane retires it again. It may also restore authority to a pane that already retired; not verified.
  • Minor test and naming nits are recorded in the review ledger.

Seen in QA, not caused by this PR (the same on main without any refresh).

Verified.

  • Live, both arms, real OpenCode 2.0.21 on a remote macOS machine. Main lost the Done after every refresh: in the logs the retirement fired 1-6 ms before the Done, which was then suppressed.
  • The PR kept the Done for 60 s and longer in a visible pane, a hidden tab, after a second refresh, and over repeated runs, with worktree ps agreeing.
  • Tests: 64 related test files pass (626 tests). Removing each guard turns exactly its own test red.
  • pnpm tc:node, oxlint, the changed-code quality check and the readiness checklist all pass. CI is green on e49dfb5.
  • One earlier CI shard failure on d88279a was an unrelated stray timer in journal-per-session-import.test.ts; it passed on rerun.

Not verified. Windows, Linux and SSH were not run live; the detector only runs for local panes, so SSH and WSL are unchanged by design. The OpenCode 2 TUI's own hook status and a Claude TUI were not exercised in the QA rig.

main: row gone after the run
PR: Done 60 s after the run
PR: hidden tab, Done 60 s after the run

@brennanb2025
brennanb2025 marked this pull request as ready for review October 1, 2026 04:22
@greptile-apps

greptile-apps Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 4/5

[Medium risk] Adjusts how agent run status settles when panes retire.

The PR appears safe to merge, with a non-blocking risk of attributing Done to a different status row under overlapping terminal observations.

Findings

  1. P2 Done can match another run ▶

Summary

The PR retains a process-run status across pane-authority retirement so OpenCode Done can land after command end.

  • Captures the status row removed by retirement and permits a one-time process Done while keeping the pane fenced.
  • Adds server and runtime tests for visible and hidden panes, hook ownership, repeated retirement, and clock changes.
Diagram
sequenceDiagram
  participant Runtime
  participant StatusStore
  Runtime->>StatusStore: Process Working
  Runtime->>StatusStore: Retire pane at command end
  StatusStore->>StatusStore: Capture row and remove status
  Runtime->>StatusStore: Process Done
  StatusStore->>StatusStore: Settle captured row once
Loading

Reviews (1) · Last reviewed commit: "fix(agent-status): carry a retired run o..."

Comment on lines +117 to +125
if (
event?.endedProcessRunStartedAt !== undefined &&
retirementFence?.retiredRun !== undefined &&
retirementFence.retiredRun.state !== 'done' &&
!this.hookOwnsProcessRun(
retirementFence.retiredRun.origin,
retirementFence.retiredRun.receivedAt,
event.endedProcessRunStartedAt
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Done can match another run If another terminal status replaces the OpenCode detector’s Working row before retirement—for example, an OSC Working row for a different agent—the fence captures that row. This branch still accepts the OpenCode process Done and publishes a completed OpenCode row that does not match the captured run. Matching the captured agent and run before admitting Done would prevent that misleading status.

Knowledge Base Used: Agent integrations and hooks

@coderabbitai

coderabbitai Bot commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

🧰 Additional context used
📚 Code guidelines (1)
AGENTS.md — auto-discovered

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: a31fb329-df78-4434-b31d-7fbf81dd7353

📥 Commits

Reviewing files that changed from the base of the PR and between 6729f1b and e49dfb5.

📒 Files selected for processing (7)
  • src/main/agent-hooks/server-process-lifetime-status.test.ts
  • src/main/agent-hooks/server/server-authority-fences.ts
  • src/main/agent-hooks/server/server-ingest-terminal.ts
  • src/main/agent-hooks/server/server-state.ts
  • src/main/agent-hooks/server/server-status-disposition.ts
  • src/main/agent-hooks/server/server-types.ts
  • src/main/runtime/opencode-run-done-after-launch-authority-retirement.test.ts

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


📝 Walkthrough

Walkthrough

Pane retirement now records a status snapshot for the retired process run. Process-origin completion events include the ended run’s start time in status disposition. A matching completion can settle the captured run once when the run was not hook-owned. Added tests cover server status behavior and OpenCode completion through visible-pane events and daemon reports.

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to e49df

The change retains process Done after pane retirement while preserving hook ownership and retirement fences. No concrete merge-blocking risk remains identified; mergeable subject to normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to e49df

The change is narrowly scoped to preserving completion status for local terminal runs. Completion does not reopen retired authority, and closed tabs and hook-owned runs remain protected. No introduced security attack path was established, but incomplete evidence prevents a minimal-risk assessment.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The inspected completion exception affects status for panes targeted by local observable PTYs. SSH and WSL PTYs are excluded from this producer, and completion settlement does not itself restore orchestration authority.

Security Findings and Attack Paths

  • inferred — A mismatched process Done could consume captured context if supplied directly to the consumer, which lacks exact run-identity comparison. That hypothetical sequence was not established through the inspected production path: the producer publishes the preceding completion before creating a new command, checks generation after asynchronous reads, and deletes command state before completion publication.

Trust Boundaries and Controls

  • observed — Process Done now avoids the live launch-token check, but its marker requires internal process provenance. Inspected HTTP, relay and spool disposition calls pass hook metadata rather than process-completion metadata. HTTP requests authenticate before body processing; pane validation and hook-ownership checks remain in terminal ingestion.

Resilience and Maintainability Implications

  • observed — Repeated retirement retains prior captured context only while the pane remains retired. A qualifying new turn clears that context, while accepted completion consumes it without lifting the fence, preserving suppression of subsequent late updates.

Hardening Proposals

  • proposed — If completion producers later become independently asynchronous or externally supplied, carry and compare an opaque run generation in retirement context so completion identity no longer depends on the current producer's ordering guarantees.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 7 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 identifies the primary fix: preserving an OpenCode run's Done status when command-end retirement removes pane authority.
Description check ✅ Passed The description is complete and focused. It includes the user impact, mechanism, rationale, scope, linked issues, visual proof, testing details, compatibility notes, deferred follow-ups, and checklist…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • 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

Autopilot is currently an internal CodeRabbit preview.


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.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ No critical issues — one coverage gap worth a look.

Reviewed changes

  • Settlement for a retired run — retirePaneAuthority captures the pane's live row as retiredRun and carries it over across repeated retirements while the pane stays retired; getAgentStatusDisposition accepts a single process-lifetime Done to settle it, then re-fences.
  • Process-lifetime event routing — processLifetimeEvent and the shared hookOwnsProcessRun helper centralize the Working/Done and hook-ownership rules in server-ingest-terminal.ts.
  • Tests — a new real-runtime integration test (opencode-run-done-after-launch-authority-retirement.test.ts) plus ten cases in server-process-lifetime-status.test.ts.

The mechanism is sound and well-scoped: I traced the accept branch against the restart branch, the closed-tab/fence.closed early return, the closed-only carry-over guard, and transfer/restore, and could not reach a state where a Done settles a run other than the one captured. The three integration tests and the key unit cases genuinely fail on base.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

Comment on lines +168 to +171
// Why: a new run supersedes the one the retirement captured.
if (retirementFence) {
retirementFence.retiredRun = undefined
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR body's ablation claim covers "new-run clearing", but removing these four lines does not fail any test: I re-ran the ablation and server-process-lifetime-status.test.ts (17/17) and opencode-run-done-after-launch-authority-retirement.test.ts (3/3) both still pass. The existing does not let a run captured before a newer run settle that newer run case does not isolate this branch because its second retirePaneAuthority already finds the owner key absent from closedAgentStatusPaneKeys (dropStatusEntry does not re-add the closed mark) and captures nothing regardless.

Technical details
# Isolate the restart-branch retiredRun clear

## Affected sites
- `src/main/agent-hooks/server/server-status-disposition.ts:168-171` — clears `retirementFence.retiredRun` when a new turn restarts a retired pane. No test fails without it.
- `src/main/agent-hooks/server/server-process-lifetime-status.test.ts:227` — the nearest case, but it passes on base and with the guard removed.

## Required outcome
- Either add a case that fails when `:168-171` is removed, or drop the "new-run clearing" guard from the ablation/complexity claims.
- The guard is only load-bearing when a restart clears `closedAgentStatusPaneKeys` for the pane/owner keys but leaves another key of the same fence closed (a multi-key/alias fence), so a later retirement via that key would otherwise carry the superseded run. A test that retires a pane spanning a legacy alias, restarts through one key, then retires the alias key would exercise it.

## Suggested approach (optional)
- Reuse the alias/transfer fixtures already used by `does not carry a run over from a retirement the pane has since left` to build a fence spanning two keys before restart.

@brennanb2025

Copy link
Copy Markdown
Contributor Author

Closing: the premise was wrong.

This PR made a finished opencode run keep a Done row after the command exits. Orca's rule for every agent is different: when the agent's process exits back to the shell, the pane's row goes away (the command-finished drop), because nothing is running there anymore. A finished opencode run should do the same. It should show Working while it runs and nothing once it has exited.

The row disappearing after a terminal-list refresh, which this PR treated as a bug, is that intended drop. The actual inconsistency is that #23722's opencode run detector posts a Done after the process exits, so without a refresh a stale Done row can stay behind. That is being fixed in a separate PR that stops the detector from posting Done on exit.

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