Skip to content

fix(opencode): close the native event stream when a snapshot shows its last pane stopped being OpenCode - #2389

Merged
devswha merged 2 commits into
mainfrom
fix/opencode-native-release-on-resync
Oct 29, 2026
Merged

devswha merged 2 commits into
mainfrom
fix/opencode-native-release-on-resync

Conversation

@devswha

@devswha devswha commented Oct 29, 2026 •

Copy link
Copy Markdown
Owner

Found by the flake-hunt lane: Integration failed once on 2026-10-28 12:54Z (job 122825435909), after #2223, at server/opencode.contract.test.ts "OpenCode v2 native service > closes the shared stream when its last verified pane ends or stops being OpenCode". The error was "the stream closing after the last verified pane stopped being OpenCode did not happen (native {"stream":true,"panes":["w8:p1"]})". A rerun passed.

Cause (product race)

Native OpenCode ownership (verifiedPanes in server/opencode-native.ts, and the shared event stream to opencode serve) was released only by onStatus naming another agent and by pane end (server/core/status.ts). The status collector (server/collector.ts) does not always call onStatus:

  • It stores a first-seen pane as a baseline without one.
  • It replays a gap by comparing status, not agent.
  • It recovers a lost subscription through onResync.

So when a pane stopped being OpenCode and the bridge learned it only from a snapshot, the stream stayed open with nothing left to release it: an event stream to opencode serve held for good. The CI failure is one such ordering.

Reproduced: with the real collector and the status hub's callbacks (server/core/status.test.ts, "native OpenCode ownership reconciliation"), a first baseline, an equal-status gap and a resync that each show the pane no longer OpenCode all leave { stream: true } on origin/main: 6 cases fail.

Change (server/core/status.ts, server/opencode-native.ts)

  • When a snapshot releases: a reconciled snapshot releases a verified pane when it names another agent, or a known status without an agent.
  • Handover kept: a pane with no agent and status unknown, as during a server handover, is kept. The exception is when pane.process_info proves the shell alone is in front: one foreground process that is the shell by name and argv, not just by pid, since exec opencode keeps the shell's pid. That read happens only for verified panes in that state, and not on Windows, which reports only the shell.
  • Older snapshots lose: a verification generation read before each snapshot keeps ownership verified since then. Only verified panes are released, so an older snapshot cancels neither a newer verification nor one still in flight. Replay releases go through the same guarded path.

Proof

  • Regression cases: 21 passed and 6 failed before the fix. All 28 pass after, including exec, handover, newer verification and in-flight verification. Each guard's cases fail when that guard is removed (checked by both reviewers).
  • Related suites: server/core/status.test.ts, server/collector.test.ts and server/opencode-native.test.ts pass 149/0.
  • Contract: bun run check run bun test --timeout 30000 ./server/opencode.contract.test.ts passed 24/24, three times in a row.
  • Local checks: bun run check fast ok.

Reviews

  • Codex Sol 6.1 (fast, xhigh):
    • Round 1: BLOCK, with two MEDIUMs.
      • The shell was proven by pid alone; after exec opencode that released a live pane.
      • An older snapshot released a newer verification.
    • Both fixed.
    • Round 2: APPROVE with no findings. It removed each guard in turn and saw its cases fail.
  • omo-native-code-reviewer:
    • REQUEST_CHANGES on one MEDIUM: snapshots also released never-verified panes, cancelling a verification still in flight and its send watches. Fixed: only verified panes are released, with a regression case.
    • Delta re-review: APPROVE.

Known limits (LOW, not fixed here)

  • Late verification: a verification that starts while the pane is labelled OpenCode, during an event gap, can still complete after a newer snapshot shows another agent. The pane then holds the stream until the next release trigger: an agent-change event, pane end, a failed re-verification, or the next snapshot. A precise fix would compare a verification's start with the snapshot's read time.
  • Live events: a live event with no agent and status unknown still releases without shell proof, as before this change.
  • Shell list: it lacks pwsh, xonsh and elvish. That fails safe: the stream stays open.

Second commit: a re-verification begun after a snapshot's read

The merge pass blocked 0c9ceca on one reproduced MEDIUM: an older status snapshot could cancel a newer in-flight re-verification of an already-verified pane. The checkpoint guarded only completed verifications, so the successful session lookup was discarded and the shared stream closed under a pane that was still OpenCode.

Change: each verification is stamped when it starts, from the same counter. releaseOpenCodePane(pane, checkpoint) stands down while the pane has a verification begun after that checkpoint; the newer attempt settles the pane itself. Releases without a checkpoint (status events, pane end) still cancel everything in flight.

Proof:

  • Regression: a new test with prior ownership in server/opencode-native.test.ts failed before the fix (the re-verification resolved null and the stream closed) and passes after.
  • Related suites: 150/0.
  • Contract: the opencode contract file passed 24/24 three times.
  • Local checks: bun run check fast ok.

Reviews:

  • Codex Sol 6.1 (fast, xhigh): APPROVE, no findings. It ran 100 repetitions in a source harness.
  • omo-native-code-reviewer: APPROVE, with LOW notes:
    • With an older and a newer verification overlapping, the early return spares the older one too. Two overlapping verifications can already race this way.
    • Two early exits of a newer attempt (process roots, a missing session id) return without releasing, so the stream waits for the next trigger.
    • No test covers a checkpoint release cancelling a verification begun before it.

…s last pane stopped being OpenCode

Native OpenCode ownership (verifiedPanes and the shared event stream to
opencode serve) was released only by a status event naming another agent and
by pane end. The collector stores a first-seen pane as a baseline without a
status event, replays a gap by status rather than agent, and recovers through
resync, so a pane that stopped being OpenCode where the bridge learned it
only from a snapshot kept the stream open for good. This was the cause of
the opencode contract flake "closes the shared stream when its last verified
pane ends or stops being OpenCode".

Reconciled snapshots now release a verified pane when they name another
agent or a known status without one. An unnamed "unknown" pane (a server
handover) is kept unless process info proves the shell alone is in front, by
its name and argv, not by pid: exec opencode keeps the shell's pid. A
verification generation read before each snapshot keeps ownership verified
since then, and only verified panes are released, so an older snapshot
cancels neither a newer verification nor one still in flight. Replay
releases go through the same guarded path.

Regression cases with the real collector: 21 passed and 6 failed before the
guards, 28 pass after (the in-flight case fails without its guard); related
suites 149/0; the opencode contract file passed 24/24 three times; bun run
check fast ok.
@coderabbitai

coderabbitai Bot commented Oct 29, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

📝 Summary

Summary by CodeRabbit

  • Bug Fixes
    • Improved OpenCode pane tracking when status updates are missing or delayed, helping keep its shared connection active during server handovers and close it when a snapshot confirms the last pane has stopped.
    • Improved handling of pane ownership changes during reconciliation, reducing the chance that active panes are released prematurely.

Snapshot reconciliation now releases verified OpenCode ownership when pane evidence confirms it is no longer active. Verification checkpoints prevent older snapshots from releasing newer verification. The change also adds reconciliation tests and a changelog entry.

Changes: OpenCode snapshot ownership release

Layer / File(s) Summary
Verification generation checkpoints
server/opencode-native.ts
Verified panes record a generation. releaseOpenCodePane preserves a pane when its generation is newer than the supplied checkpoint.
Snapshot evidence and reconciliation
server/core/status.ts
Snapshot replays defer pane release to reconciliation. Snapshot collection records checkpoints and identifies qualifying shell snapshots; reconciliation uses the recorded evidence to release panes.
Reconciliation tests and supporting changes
server/core/status.fake.ts, server/core/status.test.ts, changelog.d/opencode-native-snapshot-release.md
The fake adds collector-compatible handlers. Tests cover reconciliation evidence and verification races. The changelog describes snapshot-based closure of the shared event connection.

Priority: Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Possibly related PRs

  • devswha/ovis#1987: Adds OpenCode pane tracking and snapshot reconciliation that this change extends.
  • devswha/ovis#2039: Reverts the OpenCode status-hub and snapshot-tracking implementation; this change modifies the same reconciliation paths.
  • devswha/ovis#2200: Adds shared-server holds and final-server cleanup for OpenCode native state, which relates to the connection lifecycle addressed here.

Suggested reviewers: noobear

Merge Risk: Minimal · up to 0c9ce

The change closes the shared OpenCode event stream when a snapshot confirms the pane has stopped running OpenCode, and it protects newer verifications from older snapshots. No merge-blocking issue was found.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title clearly identifies the main fix: closing the native OpenCode event stream when a snapshot shows that the last pane is no longer OpenCode.
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.
Description check Passed The description clearly explains the problem, cause, behavior change, safeguards, known limits, and validation results. It does not use the required "## Validation" heading, but it includes the requir…
Full details: Description check

Explanation

The description clearly explains the problem, cause, behavior change, safeguards, known limits, and validation results. It does not use the required "## Validation" heading, but it includes the required checks, results, skipped-check information is not explicitly listed, and contract-test coverage is provided.

Full details: Docstring Coverage

Explanation

Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 4 functions across 4 files. (1 skipped: 1 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · 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.

…hrough that snapshot's release

The checkpoint read before a collector snapshot spared only verifications
that had completed since; one still in flight was cancelled by the older
snapshot's release, its successful session lookup was discarded, and the
shared stream closed under a pane that was still OpenCode. Stamp each
verification when it starts from the same counter, and let a release with a
checkpoint stand down while the pane has one begun after it: that newer
attempt settles the pane itself (it releases on its own if the session is
gone). Releases without a checkpoint (status events, pane end) still cancel
everything in flight.

Regression with prior ownership (server/opencode-native.test.ts): the
re-verification resolved null and the stream closed before the fix (1 fail),
and stands after it. Related suites 150/0; opencode contract 24/24 three
times; bun run check fast ok.
@devswha

devswha commented Oct 29, 2026

Copy link
Copy Markdown
Owner Author

Merging 4620a38. Native OpenCode ownership and the shared event stream to opencode serve were released only when the status named another agent or the pane ended, so a pane that stopped being OpenCode between snapshots could keep the stream open (the flaky contract case 'closes the shared stream when its last verified pane ends or stops being OpenCode'). A status snapshot that shows the last verified pane is no longer OpenCode now releases it too; a verification that completed, or began, after the snapshot was read is newer evidence and is not cancelled by it. Review by the merge pass: one reproduced MEDIUM (an older snapshot cancelling a newer in-flight re-verification and closing the stream under a pane still OpenCode) fixed in 4620a38 with a regression test; 149 unit tests, typecheck and the contract case 20/20 passed on the first round.

@devswha
devswha merged commit aa59771 into main Oct 29, 2026
20 checks passed
@devswha
devswha deleted the fix/opencode-native-release-on-resync branch October 29, 2026 22:03
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