Skip to content

fix(opencode): restarts no longer leave OpenCode servers running in their systemd scopes - #19148

Closed
nkoynov wants to merge 1 commit into
pingdotgg:mainfrom
nkoynov:fix/opencode-ledger-scoped-servers
Closed

nkoynov wants to merge 1 commit into
pingdotgg:mainfrom
nkoynov:fix/opencode-ledger-scoped-servers

Conversation

@nkoynov

@nkoynov nkoynov commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

Problem

Since agents run in their own systemd scopes (#17662), a T3 server that is SIGKILLed leaves its opencode serve running for good, and the next server starts a second one on the same OpenCode database. The OpenCode server ledger (#13879) is meant to catch exactly this, but it never stops these servers.

The ledger records the spawned process's command line right after spawn. With scopes, that pid is still the launcher at that point: /bin/sh -c … t3-agent … systemd-run --user --scope … -- opencode serve --hostname=127.0.0.1 --port=N. It then execs into opencode serve --hostname=127.0.0.1 --port=N, keeping its pid, group and start time. On the next start stopOrphan compares the full command line, sees a mismatch, and deletes the entry without signalling anything.

It happens on ordinary restarts. The service launcher SIGKILLs the server 5 s after SIGTERM, and a server with many threads spends longer than that in its shutdown recovery pass before it reaches the OpenCode finalizer. On a Linux machine with ~900 threads, a plain systemctl --user restart t3code (stop took 5.2 s) and the in-app update both left the previous opencode serve running. In one run the orphan finished the turn's shell command and kept working on the session after T3 had restarted, with nobody watching it.

Change

  • Ledger entries also record the serve arguments (args).
  • stopOrphan still requires the same pid, start time and process group, and now accepts the command line when it ends with those serve arguments, so a launcher that exec'd into the server matches. Entries written before this change (no args) match when their recorded launcher line ends with the live command line, so the first restart onto this version also stops servers left by an affected one.
  • The "another port" test now changes the port in the recorded arguments too, since those decide the match now.

Scope and approval

A small fix for an obvious regression: the reaper from #13879 silently skips every server started through an agent scope on Linux. No product behavior changes; it only makes the existing cleanup work again. The slow shutdown pass that triggers the SIGKILL is a separate issue and isn't touched here.

Verification

  • OpenCodeServerLedger.test.ts: two new cases. One records a launcher shaped like systemd-run --scope -- opencode serve … that then execs into the server under the same pid; the other is an entry from before args existed. Both fail on main (the group is never signalled) and pass with the change. The existing cases pass. The darwin-observing variants fail on my NixOS machine with or without the change (no /bin/ps).
  • A scratch test (not committed) that launches through the real agentScopeCommand with systemd-run --user --scope on systemd 261: the ledger recorded /bin/sh -c { echo 800 >/proc/self/oom_score_adj; } …, the live process became node … serve --hostname=127.0.0.1 --port=4999 in its own scope, and after killing the recorded owner the reaper on main left it running; with this change it stopped it.
  • Live, before the change, on nightly 20261013.3066 in a NixOS VM (systemd 261, OpenCode 2.0.24): after an OpenCode turn, systemctl --user restart t3code left the previous server running (reparented to the user manager, still in its t3code-opencode-*.scope) next to the new one, and its ledger entry was gone with no "Stopping an OpenCode server" log. Same with a restart where the server is stuck past the grace. A graceful t3 update that finished within 5 s stopped the server correctly. On a real host, the update at 19:38 UTC left an idle opencode serve running for over 24 h.
  • I haven't run a T3 build with this change live.
  • vp fmt, vp lint and tsc-rs --noEmit in packages/provider-opencode are clean.

Model: Claude Opus 5.5 (1M). Harness: OpenCode in T3 Code.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:S 10-29 changed lines (additions + deletions). labels Oct 13, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at cee4cd4

Macroscope's review found this PR approvable — This is a contained OpenCode cleanup bug fix with backward-compatible ledger handling and targeted regression tests. It restores orphan-process termination for launcher-to-server exec transitions while retaining the existing identity safeguards.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Oct 13, 2026

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Path: .coderabbit.config.ts
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 1aaf9282-5c15-4f8f-b080-7726620d8cfa
📥 Commits

Reviewing files that changed from the base of the PR and between c616e16 and cee4cd4.

📒 Files selected for processing: 2
  • packages/provider-opencode/src/server/OpenCodeServerLedger.test.ts
  • packages/provider-opencode/src/server/OpenCodeServerLedger.ts

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

📝 Walkthrough

The server ledger now records serve arguments and uses command matching to identify orphaned servers. Tests cover launcher-to-server command changes, legacy entries, and port mismatch handling.

Changes: OpenCode orphan reaping

Layer / File(s) Summary
Record and match server commands
packages/provider-opencode/src/server/OpenCodeServerLedger.ts, packages/provider-opencode/src/server/OpenCodeServerLedger.test.ts
Ledger entries record serve arguments. Orphan cleanup accepts exact command matches and supported launcher-to-serve command matches. Tests cover these cases and update port mismatch handling to replace every matching occurrence.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to cee4c

No actionable merge-blocking issue was established for orphan cleanup. The change is mergeable after normal checks.

Security architecture risk: 🔵 Low · up to cee4c

The restart fix improves cleanup of abandoned agent processes while retaining process and ownership checks. Remaining risk is limited to local cleanup assumptions; the reviewed change does not establish a new remotely reachable capability.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The broadened matcher affects cleanup eligibility within recorded local process groups. Its unchanged member-identity checks do not authorize arbitrary PIDs or other groups merely because their command arguments match.

Trust Boundaries and Controls

  • inferred — The consequential authority boundary remains the integrity of persisted process identities plus the runtime's ownership of the spawned group. A same-process exec with matching arguments does not itself establish a new victim outside that group. Ledger-write compromise would already allow forging an exact-match entry at the base; filesystem access controls remain unverified.

Resilience and Maintainability Implications

  • observed — Unsupported identity transitions still cause an entry to be discarded without signaling, and reaping remains asynchronous with startup. These pre-existing recovery limitations are not introduced by this PR; the new matcher addresses the supported launcher transition.
Pre-merge checks: Passed 4
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title clearly and concisely describes the primary fix: preventing OpenCode servers from remaining active in systemd scopes after restarts.
Description check Passed The description includes all required sections. It explains the problem, the implementation, scope and approval rationale, focused verification, known limitations, and the agent model and harness.
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.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

@nkoynov

nkoynov commented Oct 14, 2026

Copy link
Copy Markdown
Contributor Author

Closing this: #19291 now runs each server's agents in their own slice and stops it on the next start, which cleans up the same leftover OpenCode servers this PR targeted. I re-tested on a nightly with #19291 and the orphan is gone. Thanks!

@nkoynov nkoynov closed this Oct 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:S 10-29 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant