You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix(opencode): restarted server stops an orphaned OpenCode server started through a wrapper - #34959
When OpenCode is launched through a wrapper, the ledger can record the server process while it still carries the wrapper's command line. After it execs, the full command no longer matches, so a restarted T3 server treated the group as a recycled pid and left the orphaned OpenCode server running. Ledger entries now also store the serve arguments (optional field, so version-1 entries without it still decode). On reap, a group is stopped when pid, start time and pgid match and either the exact command line or the saved serve arguments (as the command's suffix) match; the exact-command fallback is kept for wrappers that append their own options and for legacy entries. Author's approach kept unchanged; main merged in cleanly (merge commit), and one follow-up commit removes the test helper's now-unused ready/printed signal, since the wrapper test now synchronizes through fifos. The new wrapper test fails on main (Linux and Darwin observers) and passes with the fix. The legacy-entry and appended-options tests pass on main too; they guard the compatibility paths the competing PR #22209 removes. Note for the PR body: legacy ledger entries are retained and reaped by exact command, not dropped.
Proposed real-app procedure, not executed: On Linux or macOS, configure an OpenCode provider instance's Binary path to a wrapper that forks a child, exits immediately, and has the child wait on a FIFO before exec'ing the real OpenCode executable with the original arguments. Start an OpenCode thread in T3 Code; while startup is waiting, confirm the server has written the group's ledger entry, then release the FIFO and let OpenCode finish starting. Record the OpenCode PID and port, forcibly terminate only the owning T3 Code server to bypass graceful cleanup, and restart T3 Code with the same state directory. On main, the old OpenCode process and listening port remain; with this change, restart cleanup stops that recorded group. The gated fixture makes the otherwise intermittent fork-to-exec race deliberate.
Evidence
Real T3 server and OpenCode 1.19.5 on Linux, fresh synthetic state on each side. Launch an OpenCode thread through a FIFO-gated forking wrapper; confirm the ledger records its child before exec; release the gate and observe the same PID/start time/group running OpenCode on its recorded port; crash only T3 and restart with the same state. Both runs use a 200ms spawn-return delay, disable systemd agent scopes, and pause T3 after ledger creation while OpenCode boots to prevent separate wrapper-exit handling from racing the crash. No application source or ledger edits.
Unpaused wrapper startup was not demonstrated: T3's separate wrapper-exit handler ran before this OpenCode executable finished booting, so both proof runs paused T3 after ledger creation until the deliberate crash.
macOS behavior was not exercised; verification ran on Linux.
A completed model response was not verified; this proof covers real OpenCode process startup, crash survival and restart cleanup.
Tests
vp test run src/server/OpenCodeServerLedger.test.ts (packages/provider-opencode): 16/16 pass. Same tests with main's OpenCodeServerLedger.ts: 2 failed ('records a group whose spawned wrapper already exited', linux and darwin observers), 14 passed. vp run check:changed: format, lint, knip, typecheck (10 packages), tests (2 files, 18 tests) all pass.
Not in this change
Runtime proof with a real OpenCode wrapper binary is not done here (left to the proof step).
The PR description's statement about legacy ledger entries contradicts the code (entries are kept and matched by exact command) and should be rewritten when the PR body is updated.
Who did the work
Written by Claude Opus 5.5, in Claude Code
Reviewed by GPT-6.1 Sol, in Codex, and by Claude Opus 5.5, each on its own
Proof captured by GPT-6.1 Sol and checked by Claude Opus 5.5
The reaper skips a wrapper's server group when the ledger recorded the
forked child before it exec'd, because the recorded command line no
longer matches. Gate the child on a fifo so the test records it pre-exec.
Fails on main; reported in #22324 item 4.
AI-assisted; verified with the tests and tools listed above.
…rted through a wrapper
The ledger recorded the member's full command line, which for a wrapper's
forked child can be the pre-exec line, and the reaper required an exact
match, so it treated the orphan as a recycled pid and left it running.
Record the serve arguments instead and match on them both when tracking
and when reaping.
Fixes#22324
AI-assisted; verified with the tests and tools listed above.
Keep the version-1 entry format with its full command line and add the
serve arguments as an optional field. The reaper stops a recorded group
when the command line still matches exactly or, for entries that carry
them, when it still ends with the serve arguments. Entries written by an
older server still decode and are reaped as before, and a wrapper that
appends its own options is still recorded and stopped by its command line.
AI-assisted; verified with the tests and tools listed above.
Macroscope's review found this PR approvable — Orphan cleanup now recognizes wrapper-started OpenCode servers after their command line changes during exec. Existing owner, process identity, and process-group checks keep cleanup confined to recorded orphaned servers.
Notes:
All code in this push has already been reviewed. Approvability was decided on eligibility alone.
You can add or adjust custom eligibility rules. Learn more.
No actionable comments were generated in the recent review.
Walkthrough
The ledger now records server arguments and uses them to match processes during tracking and orphan cleanup. Tests cover wrapper-exit timing, legacy entries without recorded arguments, appended log-level arguments, and repeated port arguments.
Changes: OpenCode server ledger
Layer / File(s)
Summary
Record and identify the server process packages/provider-opencode/src/server/OpenCodeServerLedger.ts, packages/provider-opencode/src/server/OpenCodeServerLedger.test.ts
The ledger stores server arguments and uses their command-line suffix to select a process-group member. The wrapper test uses FIFO gates to record the process group before the child continues, then waits for the ready-file write.
Match orphan processes during cleanup packages/provider-opencode/src/server/OpenCodeServerLedger.ts, packages/provider-opencode/src/server/OpenCodeServerLedger.test.ts
Orphan cleanup accepts a matching serve-argument suffix when arguments are recorded. Entries without arguments still require an exact command match. Tests cover both cases, appended --log-level=ERROR arguments, and replacing every matching port argument.
This change makes orphan cleanup of OpenCode server processes more reliable when a wrapper process changes its command line. No actionable merge-blocking risk was identified.
Pre-merge checks: 3 · 1
Failed checks: 1 inconclusive
Check name
Status
Explanation
Resolution
Linked Issues check
[#22324] The ledger child-command race is addressed. The reviewed diff in OpenCodeServerLedger.ts adds an optional args field and runsServe, a suffix check on the observed command. stopOrphan acce…
Provide evidence at the reviewed head that the non-ledger items are implemented: the merged commits or files, and the passing tests for pointer state, SSH zombie-process setup, timeline fixture wiring, favicon loading, and update.test.ts Wi…
Passed checks: 3 passed
Check name
Status
Explanation
Title check
The title names the main change: a restarted server now stops an orphaned OpenCode server that was started through a wrapper. It follows the conventional commit form and matches the ledger race fix in…
Out of Scope Changes check
All changes in the reviewed diff are in packages/provider-opencode/src/server/OpenCodeServerLedger.ts and its test file. The schema field args, runsServe, the stopOrphan condition, and the recording…
Description check
The description covers the problem, the change, the scope link, and verification in detail. It describes the root cause (wrapper command line recorded before exec), the new optional serve-arguments fi…
Full details: Title check
Explanation
The title names the main change: a restarted server now stops an orphaned OpenCode server that was started through a wrapper. It follows the conventional commit form and matches the ledger race fix in the changeset (serve-argument suffix matching for recorded process groups). The title is 91 characters, which is longer than ideal but still clear.
Full details: Out of Scope Changes check
Explanation
All changes in the reviewed diff are in packages/provider-opencode/src/server/OpenCodeServerLedger.ts and its test file. The schema field args, runsServe, the stopOrphan condition, and the recording path all serve the ledger race in #22324. The replace-to-replaceAll change in the 'leaves a serve process on another port alone' test follows from the new args field, which stores the port in both command and args. The commit that removes the unused ready signal from the test helper supports the reproduction fixture. No unrelated change was found.
Full details: Description check
Explanation
The description covers the problem, the change, the scope link, and verification in detail. It describes the root cause (wrapper command line recorded before exec), the new optional serve-arguments field with its suffix match, and the fallback kept for legacy entries and appended options. It links the fixed issue (#22324). The Verification section lists focused test results (16/16 pass, 2 failures on main) and check:changed results. Real-app before/after evidence and a proof procedure are provided, with explicit notes on what was not shown. The description also states one inconsistency: it says legacy entries are dropped, then later says they are retained and matched by exact command. The template's own header comment asks for a conventional commit title, and the PR title is not part of the supplied context, so this cannot be checked. The description also includes an attribution section naming the models used, which the template asks for. Overall the description is complete and specific, with one internal contradiction to fix.
Full details: Linked Issues check
Explanation
[#22324] The ledger child-command race is addressed. The reviewed diff in OpenCodeServerLedger.ts adds an optional args field and runsServe, a suffix check on the observed command. stopOrphan accepts either an exact command match or a serve-argument suffix match, with PID, start time, and PGID checks kept. The test change in OpenCodeServerLedger.test.ts adds two cases for legacy entries and for wrappers that append options. The author reports a 16/16 pass, but the test run is not re-verified here. [#22324] The other requested items are not shown at the reviewed head. The author states that pointer state, SSH zombie-process setup, timeline fixture wiring, favicon loading, and the Windows update.test.ts path handling were fixed by #22523, #22427, #22534, #22529, or #22557. The recent history shown at head does not list those PRs, and no diff in this PR touches those areas. The lower-confidence orchestration failure is not addressed or explained. The item on flaky release checks is a reported impact, not a code change.
Resolution
Provide evidence at the reviewed head that the non-ledger items are implemented: the merged commits or files, and the passing tests for pointer state, SSH zombie-process setup, timeline fixture wiring, favicon loading, and update.test.ts Windows path handling. Also state whether the orchestration test failure is resolved or deliberately excluded from #22324.
Finishing Touches
Generate unit tests (beta)
Commit to this branch
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 2 remain after this review.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
size:S10-29 changed lines (additions + deletions).vouch:trustedAuthor is trusted by repo permissions or the VOUCHED list.
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
github-actions
Bot
added
vouch:trusted
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
juliusmarminge
added
the
takeover:in-progress
When OpenCode is launched through a wrapper, the ledger can record the server process while it still carries the wrapper's command line. After it execs, the full command no longer matches, so a restarted T3 server treated the group as a recycled pid and left the orphaned OpenCode server running. Ledger entries now also store the serve arguments (optional field, so version-1 entries without it still decode). On reap, a group is stopped when pid, start time and pgid match and either the exact command line or the saved serve arguments (as the command's suffix) match; the exact-command fallback is kept for wrappers that append their own options and for legacy entries. Author's approach kept unchanged; main merged in cleanly (merge commit), and one follow-up commit removes the test helper's now-unused
ready/printedsignal, since the wrapper test now synchronizes through fifos. The new wrapper test fails on main (Linux and Darwin observers) and passes with the fix. The legacy-entry and appended-options tests pass on main too; they guard the compatibility paths the competing PR #22209 removes. Note for the PR body: legacy ledger entries are retained and reaped by exact command, not dropped.Fixes #22324
Reproduction on main
Proposed real-app procedure, not executed: On Linux or macOS, configure an OpenCode provider instance's Binary path to a wrapper that forks a child, exits immediately, and has the child wait on a FIFO before exec'ing the real OpenCode executable with the original arguments. Start an OpenCode thread in T3 Code; while startup is waiting, confirm the server has written the group's ledger entry, then release the FIFO and let OpenCode finish starting. Record the OpenCode PID and port, forcibly terminate only the owning T3 Code server to bypass graceful cleanup, and restart T3 Code with the same state directory. On main, the old OpenCode process and listening port remain; with this change, restart cleanup stops that recorded group. The gated fixture makes the otherwise intermittent fork-to-exec race deliberate.
Evidence
Real T3 server and OpenCode 1.19.5 on Linux, fresh synthetic state on each side. Launch an OpenCode thread through a FIFO-gated forking wrapper; confirm the ledger records its child before exec; release the gate and observe the same PID/start time/group running OpenCode on its recorded port; crash only T3 and restart with the same state. Both runs use a 200ms spawn-return delay, disable systemd agent scopes, and pause T3 after ledger creation while OpenCode boots to prevent separate wrapper-exit handling from racing the crash. No application source or ledger edits.
Driver · Complete fixture sources, original terminal recordings, transcripts and sanitized server logs. Videos are rendered from timestamped live terminal output.
Not shown:
Tests
vp test run src/server/OpenCodeServerLedger.test.ts (packages/provider-opencode): 16/16 pass. Same tests with main's OpenCodeServerLedger.ts: 2 failed ('records a group whose spawned wrapper already exited', linux and darwin observers), 14 passed. vp run check:changed: format, lint, knip, typecheck (10 packages), tests (2 files, 18 tests) all pass.
Not in this change
Who did the work
Run by the t3code factory in T3 Code.