Merged
Conversation
- opencode-server.mjs: save lastServerPid to server.json on successful spawn. add reapServerIfOurs() that sends SIGTERM after 5min idle and re-checks port ownership before declaring success. - session-lifecycle-hook.mjs: call reapServerIfOurs on SessionEnd. - opencode-companion.mjs: warn to stderr when workspace has .claude/settings.json deny rules and task is write-capable (issue #33). - fs.mjs: add readDenyRules() and denyRulesApplyToPath() helpers. - README.md: document permission boundary between Claude Code and OpenCode. Closes #19, #33
This was referenced Apr 12, 2026
There was a problem hiding this comment.
Pull request overview
Follow-up fixes after real-world verification of PR #51, focused on making OpenCode process management more robust (missing binaries / spawn failures), ensuring worktree task failures are reflected in job state, and documenting new PR #51 features in the README.
Changes:
- Add spawn-error handling for
runCommand,getOpencodeVersion, and detached spawns; add tests for missing-binary paths. - Update
ensureServerbehavior and add SessionEnd server reaping wiring. - Ensure foreground
task --worktreefailures mark the created job as failed; expand README docs for PR #51 features.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/process.test.mjs | Adds regression tests for spawn-error handling and missing-opencode version checks. |
| tests/opencode-server.test.mjs | Adds test to ensure ensureServer reports spawn failures without crashing. |
| tests/companion-cli.test.mjs | Adds test ensuring foreground worktree setup failures mark the job failed. |
| README.md | Documents PR reviews, --free, review-gate throttles, rescue worktrees, and runtime files. |
| plugins/opencode/scripts/session-lifecycle-hook.mjs | Calls reapServerIfOurs() on SessionEnd (best-effort). |
| plugins/opencode/scripts/opencode-companion.mjs | Warns on Claude Code deny rules for write-capable tasks; marks job failed if worktree creation fails. |
| plugins/opencode/scripts/lib/process.mjs | Adds spawn-error handling/settling guards to process helpers and swallows detached spawn errors. |
| plugins/opencode/scripts/lib/opencode-server.mjs | Adds server-state tracking + reaping logic and updates ensureServer startup error handling. |
| plugins/opencode/scripts/lib/fs.mjs | Adds helpers to read .claude/settings.json deny rules. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Guard serverStatePath against undefined workspacePath to prevent crypto.update() throwing when ensureServer is called without opts.cwd. - Stop clobbering plugin-owned state in the ensureServer early-return path — only clear tracked pid/startedAt when the tracked pid is dead, so reapServerIfOurs can still identify a server spawned by a prior plugin session. - Persist lastServerPid/lastServerStartedAt once the spawned server becomes healthy, and return alreadyRunning: false (with pid) so reapServerIfOurs has something to match against. - Add a 250ms delay to the readiness poll loop so it no longer spins hot hammering fetch() for up to 30 seconds. - Remove unused denyRulesApplyToPath import from opencode-companion. - README: "writeable" → "writable", "an disposable" → "a disposable".
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Follow-up from real verification of merged PR #51.
Fixes issues discovered during manual end-to-end verification:
What changed
opencode-server.mjs: savelastServerPid+lastServerStartedAttoserver.jsonon successful spawn. AddreapServerIfOurs()— sends SIGTERM after 5min idle, re-checks port ownership before declaring the reaper successful (don't kill a server the user started independently).session-lifecycle-hook.mjs: callreapServerIfOurs()on SessionEnd.opencode-companion.mjs: warn to stderr when workspace has.claude/settings.jsondeny rules and task is write-capable.fs.mjs: addreadDenyRules()anddenyRulesApplyToPath()helpers.README.md: document permission boundary between Claude Code and OpenCode.Existing changes from upstream PR #56 (pre-fix)
runCommand,getOpencodeVersion, and detached background spawns.ensureServerfail quickly and cleanly ifopencode servecannot spawn or exits before the health check becomes ready.task --worktreejob as failed if worktree creation fails beforerunTrackedJobstarts.Tests
172 tests passing (was 168 before this PR).