Fix PR #51 verification follow-ups (#56)

* fix: address pr51 verification follow-ups

* fix: track plugin-spawned server PID and warn on deny rules

- 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

* fix: address Copilot PR#56 review comments

- 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".

Author: JohnnyVicious

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 <ref>`, `--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 <ref>`, `--pr <number>`, `--model <provider/model>`, `--free`, `--wait`, and `--background`.
+- `/opencode:adversarial-review` -- Steerable review that challenges implementation and design decisions. Supports `--base <ref>`, `--pr <number>`, `--model <provider/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 <n|off>` limits how many stop-time reviews can run in one Claude session.
+- `--review-gate-cooldown <minutes|off>` 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,20 +199,24 @@ 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
 │   │       ├── opencode-server.mjs       # HTTP API client
 │   │       ├── 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

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;
+}

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<boolean>} 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;
+}

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;
 }