diff --git a/README.md b/README.md index 24ee9b1..d48ff80 100644 --- a/README.md +++ b/README.md @@ -22,7 +22,8 @@ they already have. - `/opencode:review` for a normal read-only OpenCode review - `/opencode:adversarial-review` for a steerable challenge review -- `/opencode:rescue`, `/opencode:status`, `/opencode:result`, and `/opencode:cancel` to delegate work and manage background jobs +- `/opencode:rescue`, `/opencode:status`, `/opencode:result`, and `/opencode:cancel` to delegate work and manage active jobs +- Optional rescue worktrees so OpenCode can make writable changes in an isolated git worktree before you keep or discard them ## Requirements @@ -96,17 +97,51 @@ To check your configured providers: ## Slash Commands -- `/opencode:review` -- Normal OpenCode code review (read-only). Supports `--base `, `--wait`, `--background`. -- `/opencode:adversarial-review` -- Steerable review that challenges implementation and design decisions. Accepts custom focus text. -- `/opencode:rescue` -- Delegates a task to OpenCode via the `opencode:opencode-rescue` subagent. Supports `--model`, `--agent`, `--resume`, `--fresh`, `--background`. +- `/opencode:review` -- Normal OpenCode code review (read-only). Supports `--base `, `--pr `, `--model `, `--free`, `--wait`, and `--background`. +- `/opencode:adversarial-review` -- Steerable review that challenges implementation and design decisions. Supports `--base `, `--pr `, `--model `, `--free`, `--wait`, `--background`, and custom focus text. +- `/opencode:rescue` -- Delegates a task to OpenCode via the `opencode:opencode-rescue` subagent. Supports `--model`, `--free`, `--agent`, `--resume`, `--fresh`, `--worktree`, `--wait`, and `--background`. - `/opencode:status` -- Shows running/recent OpenCode jobs for the current repo. - `/opencode:result` -- Shows final output for a finished job, including OpenCode session ID for resuming. -- `/opencode:cancel` -- Cancels an active background OpenCode job. -- `/opencode:setup` -- Checks OpenCode install/auth, can enable/disable the review gate hook. +- `/opencode:cancel` -- Cancels an active OpenCode job. +- `/opencode:setup` -- Checks OpenCode install/auth, can enable/disable the review gate hook, and can configure review-gate throttles. ## Review Gate -When enabled via `/opencode:setup --enable-review-gate`, a Stop hook runs a targeted OpenCode review on Claude's response. If issues are found, the stop is blocked so Claude can address them first. Warning: can create long-running loops and drain usage limits. +When enabled via `/opencode:setup --enable-review-gate`, a Stop hook runs a targeted OpenCode review on Claude's response. If issues are found, the stop is blocked so Claude can address them first. Warning: without limits this can create long-running loops and drain usage. + +Throttle controls: + +``` +/opencode:setup --review-gate-max 5 +/opencode:setup --review-gate-cooldown 10 +/opencode:setup --review-gate-max off +/opencode:setup --review-gate-cooldown off +``` + +- `--review-gate-max ` limits how many stop-time reviews can run in one Claude session. +- `--review-gate-cooldown ` enforces a minimum delay between stop-time reviews in the same Claude session. + +## Rescue Worktrees + +Use `/opencode:rescue --worktree ...` for write-capable tasks you want isolated from the current working tree. OpenCode runs in a disposable `.worktrees/opencode-*` git worktree on an `opencode/*` branch. When the task completes, the output includes keep/discard commands: + +- `keep` applies the worktree diff back to the main working tree as staged changes, then removes the temporary worktree and branch. +- `discard` removes the temporary worktree and branch without applying changes. + +If applying the patch fails, the worktree is preserved for manual recovery. + +## Review-to-Rescue + +After a successful `/opencode:review` or `/opencode:adversarial-review`, the rendered review is saved for the current repository. If you run `/opencode:rescue` with no task text, Claude can offer to pass the last review findings back to OpenCode as the rescue task. + +## Permission Boundary + +OpenCode runs as an independent process with its own permission system, separate from Claude Code's `.claude/settings.json` deny rules. This means: + +- A file that Claude Code cannot read or edit (due to a deny rule) **may still be accessible to OpenCode**. +- `/opencode:rescue` is write-capable by default and has full read/write access to the workspace. + +If your workspace uses deny rules, the companion will emit a warning when you start a write-capable task so you can make an informed choice. For fully isolated write operations, use `/opencode:rescue --worktree` which runs in a disposable git worktree. ## Troubleshooting @@ -164,6 +199,7 @@ opencode-plugin-cc/ │ ├── schemas/ # Output schemas │ ├── scripts/ # Node.js runtime │ │ ├── opencode-companion.mjs # CLI entry point +│ │ ├── safe-command.mjs # Safe slash-command argument bridge │ │ ├── session-lifecycle-hook.mjs │ │ ├── stop-review-gate-hook.mjs │ │ └── lib/ # Core modules @@ -171,13 +207,16 @@ opencode-plugin-cc/ │ │ ├── state.mjs # Persistent state │ │ ├── job-control.mjs # Job management │ │ ├── tracked-jobs.mjs # Job lifecycle tracking -│ │ ├── render.mjs # Output rendering -│ │ ├── prompts.mjs # Prompt construction -│ │ ├── git.mjs # Git utilities -│ │ ├── process.mjs # Process utilities -│ │ ├── args.mjs # Argument parsing -│ │ ├── fs.mjs # Filesystem utilities -│ │ └── workspace.mjs # Workspace detection +│ │ ├── worktree.mjs # Disposable worktree sessions +│ │ ├── render.mjs # Output rendering +│ │ ├── prompts.mjs # Prompt construction +│ │ ├── git.mjs # Git utilities +│ │ ├── process.mjs # Process utilities +│ │ ├── model.mjs # Model selection helpers +│ │ ├── review-agent.mjs # Review agent resolution +│ │ ├── args.mjs # Argument parsing +│ │ ├── fs.mjs # Filesystem utilities +│ │ └── workspace.mjs # Workspace detection │ └── skills/ # Internal skills ├── tests/ # Test suite ├── LICENSE # Apache License 2.0 diff --git a/plugins/opencode/scripts/lib/fs.mjs b/plugins/opencode/scripts/lib/fs.mjs index a8dda5a..a4591df 100644 --- a/plugins/opencode/scripts/lib/fs.mjs +++ b/plugins/opencode/scripts/lib/fs.mjs @@ -62,3 +62,42 @@ export function tailLines(filePath, n = 10) { return []; } } + +/** + * Read Claude Code project-level deny rules from .claude/settings.json. + * Returns an array of {path, read, edit} deny entries found, if any. + * @param {string} cwd + * @returns {{ path: string, read?: boolean, edit?: boolean }[]} + */ +export function readDenyRules(cwd) { + try { + const settingsPath = path.join(cwd, ".claude", "settings.json"); + const data = readJson(settingsPath); + if (!data || typeof data !== "object") return []; + const deny = data.deny ?? data.permission?.deny; + if (!Array.isArray(deny)) return []; + return deny.filter( + (rule) => + rule && + typeof rule === "object" && + typeof rule.path === "string" + ); + } catch { + return []; + } +} + +/** + * Check if any deny rules apply to a given path. + * @param {{ path: string, read?: boolean, edit?: boolean }[]} rules + * @param {string} targetPath + * @returns {boolean} + */ +export function denyRulesApplyToPath(rules, targetPath) { + if (!rules || rules.length === 0) return false; + for (const rule of rules) { + if (rule.path === targetPath) return true; + if (targetPath.startsWith(rule.path + path.sep)) return true; + } + return false; +} diff --git a/plugins/opencode/scripts/lib/opencode-server.mjs b/plugins/opencode/scripts/lib/opencode-server.mjs index 2748607..456d37f 100644 --- a/plugins/opencode/scripts/lib/opencode-server.mjs +++ b/plugins/opencode/scripts/lib/opencode-server.mjs @@ -19,6 +19,8 @@ // OpenCode to return plans instead of reviews. // (Apache License 2.0 §4(b) modification notice — see NOTICE.) +import crypto from "node:crypto"; +import os from "node:os"; import { spawn } from "node:child_process"; import fs from "node:fs"; import path from "node:path"; @@ -27,6 +29,42 @@ import { platformShellOption } from "./process.mjs"; const DEFAULT_PORT = 4096; const DEFAULT_HOST = "127.0.0.1"; const SERVER_START_TIMEOUT = 30_000; +const SERVER_REAP_IDLE_TIMEOUT = 5 * 60 * 1000; // 5 minutes + +function serverStatePath(workspacePath) { + const key = typeof workspacePath === "string" && workspacePath.length > 0 ? workspacePath : process.cwd(); + const hash = crypto.createHash("sha256").update(key).digest("hex").slice(0, 16); + const pluginDataDir = process.env.CLAUDE_PLUGIN_DATA || path.join(os.tmpdir(), "opencode-companion"); + return path.join(pluginDataDir, "state", hash, "server.json"); +} + +function loadServerState(workspacePath) { + try { + const p = serverStatePath(workspacePath); + return fs.existsSync(p) ? JSON.parse(fs.readFileSync(p, "utf8")) : {}; + } catch { + return {}; + } +} + +function saveServerState(workspacePath, data) { + try { + const p = serverStatePath(workspacePath); + fs.mkdirSync(path.dirname(p), { recursive: true, mode: 0o700 }); + fs.writeFileSync(p, JSON.stringify(data, null, 2), "utf8"); + } catch { + // best-effort + } +} + +function isProcessAlive(pid) { + try { + process.kill(pid, 0); + return true; + } catch { + return false; + } +} /** * Resolve the bundled opencode config directory shipped inside the plugin. @@ -75,6 +113,16 @@ export async function ensureServer(opts = {}) { const url = `http://${host}:${port}`; if (await isServerRunning(host, port)) { + // A server is already on the port. Only clear tracked state if the + // tracked pid is dead (stale from a prior run) — otherwise it may be + // a plugin-owned server from a previous session that reapServerIfOurs + // should still be able to identify on SessionEnd. + const state = loadServerState(opts.cwd); + if (state.lastServerPid && !isProcessAlive(state.lastServerPid)) { + delete state.lastServerPid; + delete state.lastServerStartedAt; + saveServerState(opts.cwd, state); + } return { url, alreadyRunning: true }; } @@ -104,15 +152,38 @@ export async function ensureServer(opts = {}) { shell: platformShellOption(), windowsHide: true, }); + let spawnError = null; + let earlyExit = null; + proc.once("error", (err) => { + spawnError = err; + }); + proc.once("exit", (code, signal) => { + earlyExit = { code, signal }; + }); proc.unref(); - // Wait for the server to become ready + // Wait for the server to become ready. Poll every 250ms rather than + // spinning hot — a tight loop would hammer fetch() and burn CPU for up + // to SERVER_START_TIMEOUT. const deadline = Date.now() + SERVER_START_TIMEOUT; while (Date.now() < deadline) { + if (spawnError) { + throw new Error(`Failed to start OpenCode server: ${spawnError.message}`); + } + if (earlyExit) { + const detail = earlyExit.signal + ? `signal ${earlyExit.signal}` + : `exit code ${earlyExit.code ?? "unknown"}`; + throw new Error(`OpenCode server process exited before becoming ready (${detail}).`); + } if (await isServerRunning(host, port)) { - return { url, pid: proc.pid, alreadyRunning: false }; + const state = loadServerState(opts.cwd); + state.lastServerPid = proc.pid; + state.lastServerStartedAt = Date.now(); + saveServerState(opts.cwd, state); + return { url, alreadyRunning: false, pid: proc.pid }; } - await new Promise((r) => setTimeout(r, 500)); + await new Promise((r) => setTimeout(r, 250)); } throw new Error(`OpenCode server failed to start within ${SERVER_START_TIMEOUT / 1000}s`); @@ -255,3 +326,56 @@ export async function connect(opts = {}) { const client = createClient(url, { directory: opts.cwd }); return { ...client, serverInfo: { url, alreadyRunning } }; } + +/** + * Reap the plugin-spawned OpenCode server on SessionEnd. + * + * Only kills what we started (tracked via server.json `lastServerPid`). + * Guards against killing a server the user started independently by + * re-checking isServerRunning after the kill attempt — if something + * else owns the port, leave it alone. + * + * Defense-in-depth: if the server has been running longer than + * SERVER_REAP_IDLE_TIMEOUT (5 min) without a successful health check + * from a plugin client, it is also considered orphanable even if the + * PID is still alive (user may have killed the Claude session while the + * server sat idle). + * + * @param {string} workspacePath + * @param {{ port?: number, host?: string }} [opts] + * @returns {Promise} true if a server was reaped + */ +export async function reapServerIfOurs(workspacePath, opts = {}) { + const host = opts.host ?? DEFAULT_HOST; + const port = opts.port ?? DEFAULT_PORT; + + const state = loadServerState(workspacePath); + if (!state.lastServerPid || !Number.isFinite(state.lastServerPid)) return false; + + const pid = state.lastServerPid; + const startedAt = state.lastServerStartedAt; + + if (!isProcessAlive(pid)) { + saveServerState(workspacePath, { lastServerPid: null, lastServerStartedAt: null }); + return false; + } + + const idleMs = startedAt ? Date.now() - startedAt : Infinity; + if (idleMs < SERVER_REAP_IDLE_TIMEOUT) return false; + + try { + process.kill(pid, "SIGTERM"); + } catch { + // best-effort — PID may have just exited + } + + await new Promise((r) => setTimeout(r, 1000)); + + const stillRunning = await isServerRunning(host, port); + if (!stillRunning) { + saveServerState(workspacePath, { lastServerPid: null, lastServerStartedAt: null }); + return true; + } + + return false; +} diff --git a/plugins/opencode/scripts/lib/process.mjs b/plugins/opencode/scripts/lib/process.mjs index 7645604..98184fe 100644 --- a/plugins/opencode/scripts/lib/process.mjs +++ b/plugins/opencode/scripts/lib/process.mjs @@ -45,13 +45,19 @@ export async function resolveOpencodeBinary() { windowsHide: true, }); let out = ""; + let settled = false; + const finish = (value) => { + if (settled) return; + settled = true; + resolve(value); + }; proc.stdout.on("data", (d) => (out += d)); - proc.on("error", () => resolve(null)); + proc.on("error", () => finish(null)); proc.on("close", (code) => { - if (code !== 0) return resolve(null); + if (code !== 0) return finish(null); // `where` returns all matches separated by CRLF; pick the first. const first = out.trim().split(/\r?\n/)[0] ?? ""; - resolve(first || null); + finish(first || null); }); }); } @@ -77,8 +83,15 @@ export async function getOpencodeVersion() { windowsHide: true, }); let out = ""; + let settled = false; + const finish = (value) => { + if (settled) return; + settled = true; + resolve(value); + }; proc.stdout.on("data", (d) => (out += d)); - proc.on("close", (code) => resolve(code === 0 ? out.trim() : null)); + proc.on("error", () => finish(null)); + proc.on("close", (code) => finish(code === 0 ? out.trim() : null)); }); } @@ -113,6 +126,12 @@ export function runCommand(cmd, args, opts = {}) { let stdoutBytes = 0; let stderr = ""; let overflowed = false; + let settled = false; + const finish = (result) => { + if (settled) return; + settled = true; + resolve(result); + }; proc.stdout.on("data", (chunk) => { if (overflowed) return; @@ -133,8 +152,16 @@ export function runCommand(cmd, args, opts = {}) { stdout += buf.toString("utf8"); }); proc.stderr.on("data", (d) => (stderr += d)); + proc.on("error", (err) => + finish({ + stdout, + stderr: stderr || err.message, + exitCode: typeof err.errno === "number" ? err.errno : 1, + overflowed, + }) + ); proc.on("close", (exitCode) => - resolve({ + finish({ stdout, stderr, // When we killed the child for overflow the exit code is not @@ -163,6 +190,7 @@ export function spawnDetached(cmd, args, opts = {}) { shell: platformShellOption(), windowsHide: true, }); + child.on("error", () => {}); child.unref(); return child; } diff --git a/plugins/opencode/scripts/opencode-companion.mjs b/plugins/opencode/scripts/opencode-companion.mjs index 223e953..ad6a998 100644 --- a/plugins/opencode/scripts/opencode-companion.mjs +++ b/plugins/opencode/scripts/opencode-companion.mjs @@ -81,7 +81,7 @@ import { diffWorktreeSession, cleanupWorktreeSession, } from "./lib/worktree.mjs"; -import { readJson } from "./lib/fs.mjs"; +import { readJson, readDenyRules } from "./lib/fs.mjs"; import { resolveReviewAgent } from "./lib/review-agent.mjs"; import { parseModelString, selectFreeModel } from "./lib/model.mjs"; @@ -465,6 +465,23 @@ async function handleTask(argv) { process.exit(1); } + // Issue #33 — warn when deny rules exist and task is write-capable. + // OpenCode runs outside Claude Code's permission system, so it may be able + // to edit files that Claude Code itself would block. Surfacing this helps + // users make an informed choice without blocking legitimate rescue work. + if (isWrite) { + const denyRules = readDenyRules(workspace); + if (denyRules.length > 0) { + process.stderr.write( + `[opencode-companion] Warning: this workspace has Claude Code deny rules (.claude/settings.json).\n` + + ` OpenCode runs with its own permission boundary and may be able to read/edit files\n` + + ` that Claude Code would block. Review the deny rules before proceeding:\n` + + denyRules.map((r) => ` - path: ${r.path} ${r.read ? "[read blocked]" : ""} ${r.edit ? "[edit blocked]" : ""}`).join("\n") + + `\n` + ); + } + } + // Check for resume let resumeSessionId = null; if (options["resume-last"]) { @@ -535,6 +552,13 @@ async function handleTask(argv) { }, }); } catch (err) { + upsertJob(workspace, { + id: job.id, + status: "failed", + phase: "failed", + completedAt: new Date().toISOString(), + errorMessage: `Failed to create worktree: ${err.message}`, + }); console.error(`Failed to create worktree: ${err.message}`); process.exit(1); } diff --git a/plugins/opencode/scripts/session-lifecycle-hook.mjs b/plugins/opencode/scripts/session-lifecycle-hook.mjs index 2ccaade..7312fd2 100644 --- a/plugins/opencode/scripts/session-lifecycle-hook.mjs +++ b/plugins/opencode/scripts/session-lifecycle-hook.mjs @@ -4,7 +4,7 @@ // Called on SessionStart and SessionEnd events to manage the OpenCode server. import process from "node:process"; -import { isServerRunning } from "./lib/opencode-server.mjs"; +import { isServerRunning, reapServerIfOurs } from "./lib/opencode-server.mjs"; import { resolveWorkspace } from "./lib/workspace.mjs"; import { loadState } from "./lib/state.mjs"; @@ -22,6 +22,13 @@ async function main() { } if (event === "SessionEnd") { + // Reap the plugin-spawned OpenCode server (if idle > 5 min). + try { + await reapServerIfOurs(workspace); + } catch { + // best-effort + } + // Clean up: check for any orphaned running jobs and mark them as failed const state = loadState(workspace); const runningJobs = (state.jobs ?? []).filter((j) => j.status === "running"); diff --git a/tests/companion-cli.test.mjs b/tests/companion-cli.test.mjs index 39f4df0..8890cfa 100644 --- a/tests/companion-cli.test.mjs +++ b/tests/companion-cli.test.mjs @@ -101,6 +101,24 @@ describe("opencode-companion CLI", () => { assert.equal(loadState(workspace).config.reviewGateMaxPerSession, undefined); }); + it("foreground worktree setup failure marks the created job failed", async () => { + const result = await runNodeScript([ + companionScript, + "task", + "--write", + "--worktree", + "change something", + ]); + + assert.equal(result.exitCode, 1); + assert.match(result.stderr, /Failed to create worktree/); + const state = loadState(workspace); + assert.equal(state.jobs.length, 1); + assert.equal(state.jobs[0].status, "failed"); + assert.equal(state.jobs[0].phase, "failed"); + assert.match(state.jobs[0].errorMessage, /Failed to create worktree/); + }); + it("safe-command forwards setup multi-token args from stdin", async () => { const result = await runNodeScript( [safeCommandScript, "setup"], diff --git a/tests/opencode-server.test.mjs b/tests/opencode-server.test.mjs index ea1eccf..640594f 100644 --- a/tests/opencode-server.test.mjs +++ b/tests/opencode-server.test.mjs @@ -31,6 +31,30 @@ const TEST_PORT = Number(process.env.OPENCODE_TEST_PORT ?? 14096); const opencodeAvailable = await isOpencodeInstalled(); const describeOrSkip = opencodeAvailable ? describe : describe.skip; +it("ensureServer reports opencode spawn failure without crashing", async (t) => { + const port = Number(process.env.OPENCODE_MISSING_BINARY_TEST_PORT ?? 24096); + if (await isServerRunning(TEST_HOST, port)) { + t.skip(`test port ${port} is already in use`); + return; + } + + const oldPath = process.env.PATH; + const oldPathUpper = process.env.Path; + process.env.PATH = ""; + if (oldPathUpper !== undefined) process.env.Path = ""; + try { + await assert.rejects( + () => ensureServer({ host: TEST_HOST, port }), + /Failed to start OpenCode server|exited before becoming ready/ + ); + } finally { + if (oldPath === undefined) delete process.env.PATH; + else process.env.PATH = oldPath; + if (oldPathUpper === undefined) delete process.env.Path; + else process.env.Path = oldPathUpper; + } +}); + describeOrSkip("opencode HTTP server (integration)", () => { let serverInfo; let client; diff --git a/tests/process.test.mjs b/tests/process.test.mjs index 4ff3f1e..08a56b2 100644 --- a/tests/process.test.mjs +++ b/tests/process.test.mjs @@ -5,6 +5,7 @@ import path from "node:path"; import { createTmpDir, cleanupTmpDir } from "./helpers.mjs"; import { runCommand, + getOpencodeVersion, findOpencodeAuthFile, getConfiguredProviders, } from "../plugins/opencode/scripts/lib/process.mjs"; @@ -48,6 +49,29 @@ describe("process", () => { assert.equal(overflowed, false); assert.equal(stdout, "hello"); }); + + it("runCommand returns a failed result when spawn emits error", async () => { + const result = await runCommand("__opencode_missing_command_for_test__", []); + assert.notEqual(result.exitCode, 0); + assert.equal(result.stdout, ""); + assert.equal(result.overflowed, false); + assert.match(result.stderr, /__opencode_missing_command_for_test__|not found|recognized/i); + }); + + it("getOpencodeVersion returns null when opencode cannot spawn", async () => { + const oldPath = process.env.PATH; + const oldPathUpper = process.env.Path; + process.env.PATH = ""; + if (oldPathUpper !== undefined) process.env.Path = ""; + try { + assert.equal(await getOpencodeVersion(), null); + } finally { + if (oldPath === undefined) delete process.env.PATH; + else process.env.PATH = oldPath; + if (oldPathUpper === undefined) delete process.env.Path; + else process.env.Path = oldPathUpper; + } + }); }); // Tests for OpenCode auth.json discovery + provider detection.