diff --git a/plugins/opencode/agents/opencode-rescue.md b/plugins/opencode/agents/opencode-rescue.md index 2e8ccc3..a4701a2 100644 --- a/plugins/opencode/agents/opencode-rescue.md +++ b/plugins/opencode/agents/opencode-rescue.md @@ -1,6 +1,7 @@ --- name: opencode-rescue description: Proactively use when Claude Code is stuck, wants a second implementation or diagnosis pass, needs a deeper root-cause investigation, or should hand a substantial coding task to OpenCode through the shared runtime +model: sonnet tools: Bash skills: - opencode-runtime @@ -28,6 +29,7 @@ Forwarding rules: - Leave `--agent` unset unless the user explicitly requests a specific agent (build or plan). - Leave model unset by default. Only add `--model` or `--free` when the user explicitly asks for a specific model or a free-tier pick. `--free` and `--model` are mutually exclusive. - Treat `--agent `, `--model `, and `--free` as runtime controls and do not include them in the task text you pass through. +- If the request includes `--worktree`, pass `--worktree` through to `task`. This runs OpenCode in an isolated git worktree instead of editing the working directory in-place. - Default to a write-capable OpenCode run by adding `--write` unless the user explicitly asks for read-only behavior or only wants review, diagnosis, or research without edits. - Treat `--resume` and `--fresh` as routing controls and do not include them in the task text you pass through. - `--resume` means add `--resume-last`. diff --git a/plugins/opencode/commands/cancel.md b/plugins/opencode/commands/cancel.md index a114ba3..7f8b8b7 100644 --- a/plugins/opencode/commands/cancel.md +++ b/plugins/opencode/commands/cancel.md @@ -8,7 +8,9 @@ allowed-tools: Bash(node:*) Run the cancel command and return output verbatim. ```bash -node "${CLAUDE_PLUGIN_ROOT}/scripts/opencode-companion.mjs" cancel $ARGUMENTS +node "${CLAUDE_PLUGIN_ROOT}/scripts/safe-command.mjs" cancel <<'OPENCODE_ARGS' +$ARGUMENTS +OPENCODE_ARGS ``` - Return the command stdout verbatim, exactly as-is. diff --git a/plugins/opencode/commands/rescue.md b/plugins/opencode/commands/rescue.md index 1a74d7f..2545680 100644 --- a/plugins/opencode/commands/rescue.md +++ b/plugins/opencode/commands/rescue.md @@ -1,8 +1,8 @@ --- description: Delegate investigation, an explicit fix request, or follow-up rescue work to the OpenCode rescue subagent -argument-hint: "[--background|--wait] [--resume|--fresh] [--model | --free] [--agent ] [what OpenCode should investigate, solve, or continue]" +argument-hint: "[--background|--wait] [--worktree] [--resume|--fresh] [--model | --free] [--agent ] [what OpenCode should investigate, solve, or continue]" context: fork -allowed-tools: Bash(node:*) +allowed-tools: Bash(node:*), AskUserQuestion --- Route this request to the `opencode:opencode-rescue` subagent. @@ -18,6 +18,7 @@ Execution mode: - If neither flag is present, default to foreground. - `--background` and `--wait` are execution flags for Claude Code. Do not forward them to `task`, and do not treat them as part of the natural-language task text. - `--model`, `--free`, and `--agent` are runtime-selection flags. Preserve them for the forwarded `task` call, but do not treat them as part of the natural-language task text. `--free` tells the companion to pick a random first-party `opencode/*` free-tier model from `opencode models`; it is restricted to `opencode/*` because OpenRouter free models have inconsistent tool-use support. `--free` is mutually exclusive with `--model`. +- `--worktree` is an isolation flag. Preserve it for the forwarded `task` call, but do not treat it as part of the natural-language task text. When present, OpenCode runs in an isolated git worktree instead of editing the working directory in-place. - If the request includes `--resume`, do not ask whether to continue. The user already chose. - If the request includes `--fresh`, do not ask whether to continue. The user already chose. - Otherwise, before starting OpenCode, check for a resumable rescue session from this Claude session by running: @@ -43,7 +44,26 @@ Operating rules: - Do not paraphrase, summarize, rewrite, or add commentary before or after it. - Do not ask the subagent to inspect files, monitor progress, poll `/opencode:status`, fetch `/opencode:result`, call `/opencode:cancel`, summarize output, or do follow-up work of its own. - Leave `--agent` unset unless the user explicitly asks for a specific agent (build or plan). -- Leave the model unset unless the user explicitly asks for one. +- Leave the model unset unless the user explicitly asks for a specific model or `--free`. - Leave `--resume` and `--fresh` in the forwarded request. The subagent handles that routing when it builds the `task` command. - If the helper reports that OpenCode is missing or unauthenticated, stop and tell the user to run `/opencode:setup`. -- If the user did not supply a request, ask what OpenCode should investigate or fix. +- If the user did not supply a request, check for a saved review from `/opencode:review` or `/opencode:adversarial-review`: + +```bash +node "${CLAUDE_PLUGIN_ROOT}/scripts/opencode-companion.mjs" last-review +``` + + - If stdout is `LAST_REVIEW_AVAILABLE`, use `AskUserQuestion` exactly once with two options: + - `Fix issues from last review (Recommended)` — prepend the saved review content as context for the rescue task + - `Describe a new task` — ask what OpenCode should investigate or fix + - If the user chooses to fix from last review, read the saved review via: + +```bash +node "${CLAUDE_PLUGIN_ROOT}/scripts/opencode-companion.mjs" last-review --content +``` + + and include its stdout verbatim in the forwarded task text, prefixed with: + + `The following issues were found in a prior OpenCode review. Please fix them:\n\n` + + - If stdout is `NO_LAST_REVIEW`, ask what OpenCode should investigate or fix. diff --git a/plugins/opencode/commands/result.md b/plugins/opencode/commands/result.md index 42d1991..f2db8b6 100644 --- a/plugins/opencode/commands/result.md +++ b/plugins/opencode/commands/result.md @@ -8,7 +8,9 @@ allowed-tools: Bash(node:*) Run the result command and return output verbatim. ```bash -node "${CLAUDE_PLUGIN_ROOT}/scripts/opencode-companion.mjs" result $ARGUMENTS +node "${CLAUDE_PLUGIN_ROOT}/scripts/safe-command.mjs" result <<'OPENCODE_ARGS' +$ARGUMENTS +OPENCODE_ARGS ``` - Return the command stdout verbatim, exactly as-is. diff --git a/plugins/opencode/commands/setup.md b/plugins/opencode/commands/setup.md index 31317db..c1d8655 100644 --- a/plugins/opencode/commands/setup.md +++ b/plugins/opencode/commands/setup.md @@ -1,13 +1,15 @@ --- description: Check whether the local OpenCode CLI is ready and optionally toggle the stop-time review gate -argument-hint: '[--enable-review-gate|--disable-review-gate]' +argument-hint: '[--enable-review-gate|--disable-review-gate] [--review-gate-max ] [--review-gate-cooldown ]' allowed-tools: Bash(node:*), Bash(npm:*), Bash(brew:*), Bash(curl:*), AskUserQuestion --- Run: ```bash -node "${CLAUDE_PLUGIN_ROOT}/scripts/opencode-companion.mjs" setup --json $ARGUMENTS +node "${CLAUDE_PLUGIN_ROOT}/scripts/safe-command.mjs" setup <<'OPENCODE_ARGS' +$ARGUMENTS +OPENCODE_ARGS ``` If the result says OpenCode is unavailable: @@ -25,7 +27,9 @@ npm install -g opencode-ai - Then rerun: ```bash -node "${CLAUDE_PLUGIN_ROOT}/scripts/opencode-companion.mjs" setup --json $ARGUMENTS +node "${CLAUDE_PLUGIN_ROOT}/scripts/safe-command.mjs" setup <<'OPENCODE_ARGS' +$ARGUMENTS +OPENCODE_ARGS ``` If OpenCode is already installed: diff --git a/plugins/opencode/commands/status.md b/plugins/opencode/commands/status.md index 326dedc..b85300c 100644 --- a/plugins/opencode/commands/status.md +++ b/plugins/opencode/commands/status.md @@ -8,7 +8,9 @@ allowed-tools: Bash(node:*) Run the status command and return output verbatim. ```bash -node "${CLAUDE_PLUGIN_ROOT}/scripts/opencode-companion.mjs" status $ARGUMENTS +node "${CLAUDE_PLUGIN_ROOT}/scripts/safe-command.mjs" status <<'OPENCODE_ARGS' +$ARGUMENTS +OPENCODE_ARGS ``` - Return the command stdout verbatim, exactly as-is. diff --git a/plugins/opencode/prompts/adversarial-review.md b/plugins/opencode/prompts/adversarial-review.md index 1a4c955..da4ef3f 100644 --- a/plugins/opencode/prompts/adversarial-review.md +++ b/plugins/opencode/prompts/adversarial-review.md @@ -78,6 +78,10 @@ Before finalizing, check that each finding is: - actionable for an engineer fixing the issue + +{{REVIEW_COLLECTION_GUIDANCE}} + + {{REVIEW_INPUT}} diff --git a/plugins/opencode/scripts/lib/fs.mjs b/plugins/opencode/scripts/lib/fs.mjs index 6c84187..a8dda5a 100644 --- a/plugins/opencode/scripts/lib/fs.mjs +++ b/plugins/opencode/scripts/lib/fs.mjs @@ -55,8 +55,9 @@ export function appendLine(filePath, line) { export function tailLines(filePath, n = 10) { try { const content = fs.readFileSync(filePath, "utf8"); - const lines = content.split("\n").filter(Boolean); - return lines.slice(-n); + const lines = content.split("\n"); + const nonEmpty = lines.filter((line) => line.length > 0); + return nonEmpty.slice(-n); } catch { return []; } diff --git a/plugins/opencode/scripts/lib/git.mjs b/plugins/opencode/scripts/lib/git.mjs index 05d40ff..1b5d27b 100644 --- a/plugins/opencode/scripts/lib/git.mjs +++ b/plugins/opencode/scripts/lib/git.mjs @@ -5,6 +5,9 @@ // `gh` instead of only local working-tree state. (Apache License 2.0 §4(b) // modification notice.) +import fs from "node:fs"; +import path from "node:path"; +import crypto from "node:crypto"; import { runCommand } from "./process.mjs"; /** @@ -37,9 +40,14 @@ export async function getCurrentBranch(cwd) { /** * Get the diff for review, supporting base-branch and working-tree modes. + * + * When `opts.maxBytes` is set, the read is bounded at that cap. The returned + * shape gains an `overflowed` flag in that case so callers can tell the + * difference between "small diff, all of it" and "big diff, first N bytes". + * * @param {string} cwd - * @param {{ base?: string, cached?: boolean }} opts - * @returns {Promise} + * @param {{ base?: string, cached?: boolean, maxBytes?: number }} opts + * @returns {Promise<{ stdout: string, overflowed: boolean }>} */ export async function getDiff(cwd, opts = {}) { const args = ["diff"]; @@ -48,8 +56,11 @@ export async function getDiff(cwd, opts = {}) { } else if (opts.cached) { args.push("--cached"); } - const { stdout } = await runCommand("git", args, { cwd }); - return stdout; + const result = await runCommand("git", args, { + cwd, + maxOutputBytes: opts.maxBytes, + }); + return { stdout: result.stdout, overflowed: Boolean(result.overflowed) }; } /** @@ -158,18 +169,273 @@ export async function getPrInfo(cwd, prNumber) { /** * Fetch the unified diff for a pull request via `gh pr diff`. + * + * When `opts.maxBytes` is set, the read is bounded at that cap. `overflowed` + * reports whether the full diff exceeded the cap — callers can then decide + * to short-circuit to lightweight-mode without materializing the rest. + * * @param {string} cwd * @param {number} prNumber - * @returns {Promise} + * @param {{ maxBytes?: number }} [opts] + * @returns {Promise<{ stdout: string, overflowed: boolean }>} */ -export async function getPrDiff(cwd, prNumber) { - const { stdout, stderr, exitCode } = await runCommand( +export async function getPrDiff(cwd, prNumber, opts = {}) { + const { stdout, stderr, exitCode, overflowed } = await runCommand( "gh", ["pr", "diff", String(prNumber)], - { cwd } + { cwd, maxOutputBytes: opts.maxBytes } ); - if (exitCode !== 0) { + // An overflow kill is not a real failure — we got the bytes we wanted. + if (exitCode !== 0 && !overflowed) { throw new Error(`gh pr diff ${prNumber} failed: ${stderr.trim() || "unknown error"}`); } - return stdout; + return { stdout, overflowed: Boolean(overflowed) }; +} + +// ------------------------------------------------------------------ +// Worktree helpers (for --worktree isolated rescue runs) +// ------------------------------------------------------------------ + +function isPathPresent(targetPath) { + try { + fs.lstatSync(targetPath); + return true; + } catch (err) { + if (err?.code === "ENOENT") return false; + throw err; + } +} + +async function getGitPath(repoRoot, gitPath) { + const result = await runCommand("git", ["rev-parse", "--git-path", gitPath], { cwd: repoRoot }); + if (result.exitCode !== 0) { + throw new Error(`git rev-parse --git-path ${gitPath} failed: ${result.stderr.trim()}`); + } + return path.resolve(repoRoot, result.stdout.trim()); +} + +async function assertNoInProgressGitOperation(repoRoot) { + const checks = [ + ["merge", "MERGE_HEAD"], + ["rebase", "REBASE_HEAD"], + ["rebase", "rebase-merge"], + ["rebase", "rebase-apply"], + ["bisect", "BISECT_LOG"], + ]; + + for (const [operation, gitPath] of checks) { + const marker = await getGitPath(repoRoot, gitPath); + if (isPathPresent(marker)) { + throw new Error( + `Cannot create an OpenCode worktree while the repository has an in-progress ${operation}. ` + + `Finish or abort it first (${gitPath} exists).` + ); + } + } +} + +async function branchExists(repoRoot, branch) { + const result = await runCommand( + "git", + ["show-ref", "--verify", "--quiet", `refs/heads/${branch}`], + { cwd: repoRoot } + ); + return result.exitCode === 0; +} + +async function pruneStaleOpencodeWorktrees(repoRoot) { + await runCommand("git", ["worktree", "prune"], { cwd: repoRoot }); + + const listResult = await runCommand("git", ["worktree", "list", "--porcelain"], { cwd: repoRoot }); + const activeBranches = new Set(); + for (const line of listResult.stdout.split(/\r?\n/)) { + const match = line.match(/^branch refs\/heads\/(.+)$/); + if (match) activeBranches.add(match[1]); + } + + const branchResult = await runCommand( + "git", + ["for-each-ref", "--format=%(refname:short)", "refs/heads/opencode"], + { cwd: repoRoot } + ); + if (branchResult.exitCode !== 0) return; + + for (const branch of branchResult.stdout.trim().split(/\r?\n/).filter(Boolean)) { + if (activeBranches.has(branch)) continue; + const merged = await runCommand( + "git", + ["merge-base", "--is-ancestor", branch, "HEAD"], + { cwd: repoRoot } + ); + if (merged.exitCode === 0) { + await deleteWorktreeBranch(repoRoot, branch); + } + } +} + +function makeWorktreeId() { + return `${Date.now()}-${crypto.randomBytes(4).toString("hex")}`; +} + +/** + * Create a disposable git worktree under `/.worktrees/opencode-` + * on a new `opencode/` branch. Also adds `.worktrees/` to the repo's + * `.git/info/exclude` so the directory never shows up in `git status`. + * + * @param {string} repoRoot + * @returns {Promise<{ worktreePath: string, branch: string, repoRoot: string, baseCommit: string, timestamp: number }>} + */ +export async function createWorktree(repoRoot) { + await assertNoInProgressGitOperation(repoRoot); + await pruneStaleOpencodeWorktrees(repoRoot); + + const worktreesDir = path.join(repoRoot, ".worktrees"); + fs.mkdirSync(worktreesDir, { recursive: true, mode: 0o700 }); + + // Resolve the real git dir (handles linked worktrees where .git is a file). + const gitDir = await getGitPath(repoRoot, "."); + const excludePath = path.join(gitDir, "info", "exclude"); + const existing = fs.existsSync(excludePath) ? fs.readFileSync(excludePath, "utf8") : ""; + if (!existing.includes(".worktrees")) { + fs.mkdirSync(path.dirname(excludePath), { recursive: true }); + const sep = existing.length === 0 || existing.endsWith("\n") ? "" : "\n"; + fs.appendFileSync(excludePath, `${sep}.worktrees/\n`); + } + + const baseCommitResult = await runCommand("git", ["rev-parse", "HEAD"], { cwd: repoRoot }); + if (baseCommitResult.exitCode !== 0) { + throw new Error(`git rev-parse HEAD failed: ${baseCommitResult.stderr.trim()}`); + } + const baseCommit = baseCommitResult.stdout.trim(); + + for (let attempt = 0; attempt < 5; attempt += 1) { + const id = makeWorktreeId(); + const worktreePath = path.join(worktreesDir, `opencode-${id}`); + const branch = `opencode/${id}`; + if (fs.existsSync(worktreePath) || await branchExists(repoRoot, branch)) { + continue; + } + + const addResult = await runCommand( + "git", + ["worktree", "add", worktreePath, "-b", branch], + { cwd: repoRoot } + ); + if (addResult.exitCode === 0) { + return { worktreePath, branch, repoRoot, baseCommit, timestamp: Date.now() }; + } + + if (!/already exists|is already checked out|invalid reference/i.test(addResult.stderr)) { + throw new Error(`git worktree add failed: ${addResult.stderr.trim()}`); + } + } + + throw new Error("Unable to allocate a unique OpenCode worktree after 5 attempts."); +} + +/** + * Remove a worktree (force). Swallows "not a working tree" so callers can + * safely retry cleanup. + * @param {string} repoRoot + * @param {string} worktreePath + */ +export async function removeWorktree(repoRoot, worktreePath) { + const { exitCode, stderr } = await runCommand( + "git", + ["worktree", "remove", "--force", worktreePath], + { cwd: repoRoot } + ); + if (exitCode !== 0 && !stderr.includes("is not a working tree")) { + throw new Error(`git worktree remove failed: ${stderr.trim()}`); + } +} + +/** + * Delete a branch (force). Failures are swallowed — this is best-effort + * cleanup after the worktree has already been removed. + */ +export async function deleteWorktreeBranch(repoRoot, branch) { + await runCommand("git", ["branch", "-D", branch], { cwd: repoRoot }); +} + +/** + * Compute the diff the worktree made on top of the base commit. Stages + * everything first so uncommitted edits (which is what OpenCode actually + * produces) show up in the diff. + * @returns {Promise<{ stat: string, patch: string }>} + */ +export async function getWorktreeDiff(worktreePath, baseCommit) { + await runCommand("git", ["add", "-A"], { cwd: worktreePath }); + const statR = await runCommand( + "git", + ["diff", "--cached", baseCommit, "--stat"], + { cwd: worktreePath } + ); + if (statR.exitCode !== 0 || !statR.stdout.trim()) { + return { stat: "", patch: "" }; + } + const patchR = await runCommand( + "git", + ["diff", "--cached", baseCommit], + { cwd: worktreePath } + ); + return { stat: statR.stdout.trim(), patch: patchR.stdout }; +} + +/** + * Apply the worktree diff back to `repoRoot` as a staged patch. Returns + * `{ applied, detail }` — detail includes any git error when apply fails. + */ +export async function applyWorktreePatch(repoRoot, worktreePath, baseCommit) { + await runCommand("git", ["add", "-A"], { cwd: worktreePath }); + const patchR = await runCommand( + "git", + ["diff", "--cached", baseCommit], + { cwd: worktreePath } + ); + if (patchR.exitCode !== 0 || !patchR.stdout.trim()) { + return { applied: false, detail: "No changes to apply." }; + } + const patchPath = path.join( + repoRoot, + `.opencode-worktree-${Date.now()}-${Math.random().toString(16).slice(2)}.patch` + ); + let applied = false; + try { + fs.writeFileSync(patchPath, patchR.stdout, "utf8"); + const applyR = await runCommand( + "git", + ["apply", "--index", patchPath], + { cwd: repoRoot } + ); + if (applyR.exitCode !== 0) { + const stderr = applyR.stderr.trim(); + return { + applied: false, + detail: formatApplyFailureDetail(stderr, patchPath), + }; + } + applied = true; + return { applied: true, detail: "Changes applied and staged." }; + } finally { + if (applied) { + fs.rmSync(patchPath, { force: true }); + } + } +} + +function formatApplyFailureDetail(stderr, patchPath) { + const detail = stderr || "Patch apply failed."; + const lower = detail.toLowerCase(); + let hint = "Resolve the conflict manually, then retry with `git apply --index`."; + if (lower.includes("binary patch") || lower.includes("without full index line")) { + hint = "The patch appears to include binary changes; inspect the preserved worktree and copy binary files manually."; + } else if (lower.includes("standard format") || lower.includes("corrupt patch")) { + hint = "The generated patch is not in a format git can apply; inspect or edit the preserved patch before retrying."; + } else if (lower.includes("permission denied")) { + hint = "Git could not read or write a target path; check file permissions before retrying."; + } else if (lower.includes("does not apply") || lower.includes("patch failed")) { + hint = "The target files diverged; edit the preserved patch or apply the changes manually."; + } + return `${detail}\nPreserved patch: ${patchPath}\nHint: ${hint}`; } diff --git a/plugins/opencode/scripts/lib/job-control.mjs b/plugins/opencode/scripts/lib/job-control.mjs index 9672ccf..38009bf 100644 --- a/plugins/opencode/scripts/lib/job-control.mjs +++ b/plugins/opencode/scripts/lib/job-control.mjs @@ -1,7 +1,99 @@ // Job control: query, sort, enrich, and build status snapshots. import { tailLines } from "./fs.mjs"; -import { jobLogPath } from "./state.mjs"; +import { jobLogPath, loadState, upsertJob } from "./state.mjs"; +import { isProcessAlive } from "./process.mjs"; + +function isActiveJobStatus(status) { + return status !== "completed" && status !== "failed"; +} + +function shouldReconcileDeadPids() { + return !/^(1|true|yes)$/i.test(process.env.OPENCODE_COMPANION_NO_RECONCILE ?? ""); +} + +/** + * Mark a job as failed because its tracked PID is no longer alive. Re-reads + * the latest persisted state before writing to guard against a legitimate + * completion racing the probe. + * + * @param {string} workspacePath + * @param {string} jobId + * @param {number} pid - the pid we observed as dead + * @param {string|null} [pidStartToken] - the process-start token observed as dead + * @returns {boolean} true if a write happened + */ +export function markDeadPidJobFailed(workspacePath, jobId, pid, pidStartToken = null) { + const latest = loadState(workspacePath).jobs?.find((j) => j.id === jobId); + if (!latest) return false; + + // Only overwrite active states; never downgrade terminal states. + if (!isActiveJobStatus(latest.status)) return false; + + // Only overwrite if the PID still matches what we observed as dead. Guards + // against a job that legitimately restarted with a new PID between the + // probe and the write. + if (latest.pid !== pid) return false; + if (pidStartToken && latest.pidStartToken && latest.pidStartToken !== pidStartToken) { + return false; + } + + upsertJob(workspacePath, { + id: jobId, + status: "failed", + phase: "failed", + pid: null, + pidStartToken: null, + errorMessage: `Tracked process PID ${pid} exited unexpectedly without writing a terminal status.`, + completedAt: new Date().toISOString(), + }); + return true; +} + +/** + * If a job is still marked active but its tracked PID is dead, reconcile it + * to failed and return the updated record. Otherwise return the original. + * + * Called from every status read path so a single status query is enough to + * surface dead workers — no need to wait for SessionEnd. + * + * @param {string} workspacePath + * @param {object} job + * @returns {object} + */ +export function reconcileIfDead(workspacePath, job) { + if (!shouldReconcileDeadPids()) return job; + if (!job || !isActiveJobStatus(job.status)) return job; + const pid = Number.isFinite(job.pid) ? job.pid : null; + if (pid === null) return job; + if (isProcessAlive(pid, job.pidStartToken)) return job; + + try { + markDeadPidJobFailed(workspacePath, job.id, pid, job.pidStartToken); + } catch { + // Never let reconciliation errors crash a status read. + return job; + } + + const latest = loadState(workspacePath).jobs?.find((j) => j.id === job.id); + return latest ?? job; +} + +/** + * Reconcile all active jobs in the given list against live PIDs. + * Returns a new list where dead-PID jobs have been rewritten to failed. + * + * Cheap shortcut used by handlers that otherwise operate on pure job arrays + * (handleCancel, handleResult) so a single call surfaces dead workers. + * + * @param {object[]} jobs + * @param {string} workspacePath + * @returns {object[]} + */ +export function reconcileAllJobs(jobs, workspacePath) { + if (!Array.isArray(jobs) || jobs.length === 0) return jobs; + return jobs.map((j) => reconcileIfDead(workspacePath, j)); +} /** * Sort jobs newest first by updatedAt. @@ -83,10 +175,13 @@ export function buildStatusSnapshot(jobs, workspacePath, opts = {}) { } const sorted = sortJobsNewestFirst(filtered); - const enriched = sorted.map((j) => enrichJob(j, workspacePath)); + // Reconcile any active jobs whose tracked PID is dead before enriching, so + // a single status read surfaces stuck workers immediately. + const reconciled = sorted.map((j) => reconcileIfDead(workspacePath, j)); + const enriched = reconciled.map((j) => enrichJob(j, workspacePath)); - const running = enriched.filter((j) => j.status === "running"); - const finished = enriched.filter((j) => j.status !== "running"); + const running = enriched.filter((j) => isActiveJobStatus(j.status)); + const finished = enriched.filter((j) => !isActiveJobStatus(j.status)); const latestFinished = finished[0] ?? null; const recent = finished.slice(0, 5); @@ -130,17 +225,31 @@ export function resolveResultJob(jobs, ref) { } /** - * Resolve a job that can be canceled (running). + * Resolve a job that can be canceled (any active/non-terminal state). + * + * When opts.sessionId is set, the default target (no ref) is restricted to + * jobs from that session so `/opencode:cancel` doesn't reach across Claude + * sessions and kill unrelated work. An explicit ref still searches all active + * jobs — if the user names a job, they asked for it by name. + * * @param {object[]} jobs * @param {string} [ref] - * @returns {{ job: object|null, ambiguous: boolean }} + * @param {{ sessionId?: string }} [opts] + * @returns {{ job: object|null, ambiguous: boolean, sessionScoped?: boolean }} */ -export function resolveCancelableJob(jobs, ref) { - const running = jobs.filter((j) => j.status === "running"); - if (!ref) { - return { job: running[0] ?? null, ambiguous: running.length > 1 }; +export function resolveCancelableJob(jobs, ref, opts = {}) { + const running = jobs.filter((j) => isActiveJobStatus(j.status)); + if (ref) { + return matchJobReference(running, ref); } - return matchJobReference(running, ref); + const scoped = opts.sessionId + ? running.filter((j) => j.sessionId === opts.sessionId) + : running; + return { + job: scoped[0] ?? null, + ambiguous: scoped.length > 1, + sessionScoped: Boolean(opts.sessionId), + }; } /** diff --git a/plugins/opencode/scripts/lib/opencode-server.mjs b/plugins/opencode/scripts/lib/opencode-server.mjs index 4c17208..2748607 100644 --- a/plugins/opencode/scripts/lib/opencode-server.mjs +++ b/plugins/opencode/scripts/lib/opencode-server.mjs @@ -20,7 +20,9 @@ // (Apache License 2.0 §4(b) modification notice — see NOTICE.) import { spawn } from "node:child_process"; +import fs from "node:fs"; import path from "node:path"; +import { platformShellOption } from "./process.mjs"; const DEFAULT_PORT = 4096; const DEFAULT_HOST = "127.0.0.1"; @@ -35,7 +37,11 @@ const SERVER_START_TIMEOUT = 30_000; export function getBundledConfigDir() { const pluginRoot = process.env.CLAUDE_PLUGIN_ROOT; if (!pluginRoot) return null; - return path.join(pluginRoot, "opencode-config"); + const configDir = path.join(pluginRoot, "opencode-config"); + try { + if (fs.existsSync(configDir)) return configDir; + } catch {} + return null; } /** @@ -95,6 +101,8 @@ export async function ensureServer(opts = {}) { detached: true, cwd: opts.cwd, env, + shell: platformShellOption(), + windowsHide: true, }); proc.unref(); diff --git a/plugins/opencode/scripts/lib/process.mjs b/plugins/opencode/scripts/lib/process.mjs index 12f6d38..7645604 100644 --- a/plugins/opencode/scripts/lib/process.mjs +++ b/plugins/opencode/scripts/lib/process.mjs @@ -8,21 +8,51 @@ // `/opencode:setup` always report `providers: []`. (Apache License 2.0 // §4(b) modification notice.) -import { spawn } from "node:child_process"; +import { spawn, spawnSync } from "node:child_process"; import fs from "node:fs"; import os from "node:os"; import path from "node:path"; +/** + * Shell option for child_process.spawn that is correct on every platform: + * + * - POSIX: `false` — pass argv directly to `execvp`. + * - Windows: if `$SHELL` points at a POSIX shell (Git Bash, MSYS), use it so + * users who wrote their PATH for Git Bash don't get cmd.exe behavior. + * Otherwise `true` falls back to Node's default (cmd.exe), which is still + * needed so .cmd / .bat shims resolve. + * + * Without this, bare names like `opencode`, `git`, `gh`, `where` spawned on + * Windows hit ENOENT because Node won't resolve .cmd shims on its own. + * @returns {string|true|false} + */ +export function platformShellOption() { + if (process.platform !== "win32") return false; + return process.env.SHELL || true; +} + /** * Resolve the full path to the `opencode` binary. * @returns {Promise} */ export async function resolveOpencodeBinary() { return new Promise((resolve) => { - const proc = spawn("which", ["opencode"], { stdio: ["ignore", "pipe", "ignore"] }); + const isWin = process.platform === "win32"; + const locator = isWin ? "where" : "which"; + const proc = spawn(locator, ["opencode"], { + stdio: ["ignore", "pipe", "ignore"], + shell: platformShellOption(), + windowsHide: true, + }); let out = ""; proc.stdout.on("data", (d) => (out += d)); - proc.on("close", (code) => resolve(code === 0 ? out.trim() : null)); + proc.on("error", () => resolve(null)); + proc.on("close", (code) => { + if (code !== 0) return resolve(null); + // `where` returns all matches separated by CRLF; pick the first. + const first = out.trim().split(/\r?\n/)[0] ?? ""; + resolve(first || null); + }); }); } @@ -43,6 +73,8 @@ export async function getOpencodeVersion() { return new Promise((resolve) => { const proc = spawn("opencode", ["--version"], { stdio: ["ignore", "pipe", "ignore"], + shell: platformShellOption(), + windowsHide: true, }); let out = ""; proc.stdout.on("data", (d) => (out += d)); @@ -51,11 +83,18 @@ export async function getOpencodeVersion() { } /** - * Run a command and return { stdout, stderr, exitCode }. + * Run a command and return { stdout, stderr, exitCode, overflowed }. + * + * Supports bounded output reads via `opts.maxOutputBytes`. When that cap is + * set and the child's stdout would exceed it, the child is killed and the + * returned `overflowed` flag is set to `true`. This lets callers probe + * potentially-huge outputs (e.g. `git diff` for a big changeset) without + * materializing the whole buffer in memory. + * * @param {string} cmd * @param {string[]} args - * @param {object} [opts] - * @returns {Promise<{ stdout: string, stderr: string, exitCode: number }>} + * @param {{ cwd?: string, env?: object, maxOutputBytes?: number }} [opts] + * @returns {Promise<{ stdout: string, stderr: string, exitCode: number, overflowed: boolean }>} */ export function runCommand(cmd, args, opts = {}) { return new Promise((resolve) => { @@ -63,12 +102,48 @@ export function runCommand(cmd, args, opts = {}) { stdio: ["ignore", "pipe", "pipe"], cwd: opts.cwd, env: { ...process.env, ...opts.env }, + shell: platformShellOption(), + windowsHide: true, }); + + const maxOutputBytes = Number.isFinite(opts.maxOutputBytes) && opts.maxOutputBytes > 0 + ? opts.maxOutputBytes + : null; let stdout = ""; + let stdoutBytes = 0; let stderr = ""; - proc.stdout.on("data", (d) => (stdout += d)); + let overflowed = false; + + proc.stdout.on("data", (chunk) => { + if (overflowed) return; + const buf = Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk); + stdoutBytes += buf.length; + if (maxOutputBytes !== null && stdoutBytes > maxOutputBytes) { + overflowed = true; + // Keep whatever we've seen up to the cap, then kill the child. + const keep = maxOutputBytes - (stdoutBytes - buf.length); + if (keep > 0) stdout += buf.slice(0, keep).toString("utf8"); + try { + proc.kill("SIGTERM"); + } catch { + // best-effort; ignore + } + return; + } + stdout += buf.toString("utf8"); + }); proc.stderr.on("data", (d) => (stderr += d)); - proc.on("close", (exitCode) => resolve({ stdout, stderr, exitCode: exitCode ?? 1 })); + proc.on("close", (exitCode) => + resolve({ + stdout, + stderr, + // When we killed the child for overflow the exit code is not + // meaningful — surface a synthetic 0 so callers can still consume + // the partial stdout via the overflowed flag. + exitCode: overflowed ? 0 : exitCode ?? 1, + overflowed, + }) + ); }); } @@ -85,11 +160,92 @@ export function spawnDetached(cmd, args, opts = {}) { detached: true, cwd: opts.cwd, env: { ...process.env, ...opts.env }, + shell: platformShellOption(), + windowsHide: true, }); child.unref(); return child; } +/** + * Return a stable best-effort process start token for PID-recycling checks. + * The token format is intentionally opaque and platform-prefixed. + * @param {number | null | undefined} pid + * @returns {string|null} + */ +export function getProcessStartToken(pid) { + if (pid == null || !Number.isFinite(pid) || pid <= 0) return null; + + if (process.platform === "linux") { + try { + const stat = fs.readFileSync(`/proc/${pid}/stat`, "utf8"); + const endOfComm = stat.lastIndexOf(")"); + if (endOfComm !== -1) { + const fieldsFromState = stat.slice(endOfComm + 2).trim().split(/\s+/); + const startTime = fieldsFromState[19]; + if (startTime) return `linux:${startTime}`; + } + } catch { + return null; + } + } + + if (process.platform === "darwin" || process.platform === "freebsd") { + const result = spawnSync("ps", ["-o", "lstart=", "-p", String(pid)], { + encoding: "utf8", + shell: platformShellOption(), + windowsHide: true, + }); + const started = result.status === 0 ? result.stdout.trim() : ""; + return started ? `${process.platform}:${started}` : null; + } + + if (process.platform === "win32") { + const result = spawnSync( + "powershell.exe", + [ + "-NoProfile", + "-Command", + `(Get-CimInstance Win32_Process -Filter "ProcessId = ${pid}").CreationDate`, + ], + { + encoding: "utf8", + windowsHide: true, + } + ); + const started = result.status === 0 ? result.stdout.trim() : ""; + return started ? `win32:${started}` : null; + } + + return null; +} + +/** + * Check whether a process is still alive. Uses signal 0 which does not + * affect the process — only probes existence. When an expected start token + * is supplied and the platform can read the current process start token, a + * token mismatch is treated as dead to avoid PID-recycling false positives. + * @param {number | null | undefined} pid + * @param {string | null | undefined} expectedStartToken + * @returns {boolean} + */ +export function isProcessAlive(pid, expectedStartToken = null) { + if (pid == null || !Number.isFinite(pid) || pid <= 0) return false; + try { + process.kill(pid, 0); + } catch (err) { + // ESRCH = dead. EPERM/EACCES = exists but no permission. + if (err?.code !== "EPERM" && err?.code !== "EACCES") return false; + return true; + } + + if (expectedStartToken) { + const actualStartToken = getProcessStartToken(pid); + if (actualStartToken && actualStartToken !== expectedStartToken) return false; + } + return true; +} + // ------------------------------------------------------------------ // OpenCode auth.json discovery // ------------------------------------------------------------------ diff --git a/plugins/opencode/scripts/lib/prompts.mjs b/plugins/opencode/scripts/lib/prompts.mjs index e1fad54..5573490 100644 --- a/plugins/opencode/scripts/lib/prompts.mjs +++ b/plugins/opencode/scripts/lib/prompts.mjs @@ -7,7 +7,34 @@ import fs from "node:fs"; import path from "node:path"; -import { getDiff, getStatus, getChangedFiles, getPrInfo, getPrDiff } from "./git.mjs"; +import { + getDiff, + getStatus, + getChangedFiles, + getDiffStat, + getPrInfo, + getPrDiff, +} from "./git.mjs"; + +// Inline-diff thresholds. When a review exceeds either, we keep the prompt +// bounded by including a diff excerpt instead of the full diff. The review +// agent is intentionally shell-disabled, so the prompt must contain the +// evidence the model is allowed to use. +const DEFAULT_INLINE_DIFF_MAX_FILES = 5; +const DEFAULT_INLINE_DIFF_MAX_BYTES = 256 * 1024; + +function buildCollectionGuidance(diffIsComplete) { + return diffIsComplete + ? "Use the repository context below as primary evidence." + : "The repository context below contains a bounded diff excerpt, not the complete diff. Only report findings supported by the provided excerpt, metadata, status, and changed-file list; explicitly say when omitted diff content prevents a conclusion."; +} + +function truncateUtf8(text, maxBytes) { + if (!text) return text; + const buf = Buffer.from(text, "utf8"); + if (buf.length <= maxBytes) return text; + return buf.subarray(0, maxBytes).toString("utf8").replace(/\uFFFD$/, ""); +} /** * Build the review prompt for OpenCode. @@ -21,35 +48,89 @@ import { getDiff, getStatus, getChangedFiles, getPrInfo, getPrDiff } from "./git * @returns {Promise} */ export async function buildReviewPrompt(cwd, opts, pluginRoot) { - let diff, status, changedFiles; + const maxFiles = Number.isFinite(opts.maxInlineDiffFiles) + ? opts.maxInlineDiffFiles + : DEFAULT_INLINE_DIFF_MAX_FILES; + const maxBytes = Number.isFinite(opts.maxInlineDiffBytes) + ? opts.maxInlineDiffBytes + : DEFAULT_INLINE_DIFF_MAX_BYTES; + + let diff = ""; + let status = ""; + let changedFiles = []; let prInfo = null; + let diffStat = ""; + let overByteLimit = false; + // Step 1: cheap metadata. The status / changed-file list / shortstat + // reads do not materialize the full diff and are safe on any size. if (opts.pr) { prInfo = await getPrInfo(cwd, opts.pr); - diff = await getPrDiff(cwd, opts.pr); status = ""; // PR review intentionally ignores the local working tree changedFiles = prInfo.files; } else { - diff = await getDiff(cwd, { base: opts.base }); status = await getStatus(cwd); changedFiles = await getChangedFiles(cwd, { base: opts.base }); + diffStat = await getDiffStat(cwd, { base: opts.base }); + } + + // Step 2: fetch the diff body, but bound the read at maxBytes + 1. If + // the git/gh subprocess would produce more bytes than that, the helper + // reports `overflowed: true` and we treat the diff as over the byte + // limit without ever materializing the rest. Past this point we know + // the diff string in memory is at most maxBytes + 1 bytes. + const readCap = maxBytes + 1; + if (opts.pr) { + const pr = await getPrDiff(cwd, opts.pr, { maxBytes: readCap }); + diff = pr.stdout; + overByteLimit = pr.overflowed; + } else { + const wt = await getDiff(cwd, { base: opts.base, maxBytes: readCap }); + diff = wt.stdout; + overByteLimit = wt.overflowed; } + const overFileLimit = changedFiles.length > maxFiles; + + // The "original" diff byte count is used for the user-facing context + // note. When we overflowed the read, we don't know the true size — use + // the cap as a lower bound. + const diffBytes = overByteLimit ? readCap : Buffer.byteLength(diff, "utf8"); + const diffIsComplete = !overByteLimit; + const diffForPrompt = overByteLimit ? truncateUtf8(diff, maxBytes) : diff; + const collectionGuidance = buildCollectionGuidance(diffIsComplete); + const targetLabel = prInfo ? `Pull request #${prInfo.number} "${prInfo.title}" (${prInfo.headRefName} -> ${prInfo.baseRefName})` : opts.base ? `Branch diff against ${opts.base}` : "Working tree changes"; + const reviewContext = buildReviewContext(diffForPrompt, status, changedFiles, prInfo, { + diffIsComplete, + originalDiffBytes: diffBytes, + maxInlineDiffBytes: maxBytes, + overFileLimit, + overByteLimit, + diffStat, + }); + let systemPrompt; if (opts.adversarial) { const templatePath = path.join(pluginRoot, "prompts", "adversarial-review.md"); systemPrompt = fs.readFileSync(templatePath, "utf8") .replace("{{TARGET_LABEL}}", targetLabel) .replace("{{USER_FOCUS}}", opts.focus || "General review") - .replace("{{REVIEW_INPUT}}", buildReviewContext(diff, status, changedFiles, prInfo)); + .replace("{{REVIEW_COLLECTION_GUIDANCE}}", collectionGuidance) + .replace("{{REVIEW_INPUT}}", reviewContext); } else { - systemPrompt = buildStandardReviewPrompt(diff, status, changedFiles, { ...opts, targetLabel, prInfo }); + systemPrompt = buildStandardReviewPrompt(diff, status, changedFiles, { + ...opts, + targetLabel, + prInfo, + reviewContext, + collectionGuidance, + }); } return systemPrompt; @@ -62,6 +143,11 @@ function buildStandardReviewPrompt(diff, status, changedFiles, opts) { const targetLabel = opts.targetLabel ?? (opts.base ? `branch diff against ${opts.base}` : "working tree changes"); + const reviewContext = + opts.reviewContext + ?? buildReviewContext(diff, status, changedFiles, opts.prInfo, { diffIsComplete: true }); + const collectionGuidance = opts.collectionGuidance ?? buildCollectionGuidance(true); + return `You are performing a code review of ${targetLabel}. Review the following changes and provide structured feedback in JSON format matching the review-output schema. @@ -75,13 +161,19 @@ Focus on: Be concise and actionable. Only report real issues, not style preferences. -${buildReviewContext(diff, status, changedFiles, opts.prInfo)}`; +${collectionGuidance} + +${reviewContext}`; } /** * Build the repository context block for review prompts. + * + * When `opts.diffIsComplete` is false, the `` block is a bounded + * excerpt. The surrounding note tells the model not to invent findings from + * omitted content. */ -function buildReviewContext(diff, status, changedFiles, prInfo) { +function buildReviewContext(diff, status, changedFiles, prInfo, opts = {}) { const sections = []; if (prInfo) { @@ -105,6 +197,29 @@ function buildReviewContext(diff, status, changedFiles, prInfo) { sections.push(`\n${changedFiles.join("\n")}\n`); } + if (opts.overFileLimit || opts.overByteLimit) { + const reasons = []; + if (opts.overFileLimit) reasons.push(`file count ${changedFiles.length}`); + if (opts.overByteLimit) { + reasons.push(`diff size ${opts.originalDiffBytes} bytes`); + } + const budget = opts.overByteLimit && opts.maxInlineDiffBytes + ? `; excerpt budget ${opts.maxInlineDiffBytes} bytes` + : ""; + const note = opts.diffIsComplete === false + ? "Diff context is bounded" + : "Review spans a broad changed-file set, but the diff below is complete"; + sections.push( + `\n` + + `${note} (${reasons.join(", ")}${budget}). ` + + `Findings must be supported by the diff evidence below.\n` + + `` + ); + if (opts.diffStat) { + sections.push(`\n${opts.diffStat}\n`); + } + } + if (diff) { sections.push(`\n${diff}\n`); } diff --git a/plugins/opencode/scripts/lib/render.mjs b/plugins/opencode/scripts/lib/render.mjs index 843f6a4..b1fc1d5 100644 --- a/plugins/opencode/scripts/lib/render.mjs +++ b/plugins/opencode/scripts/lib/render.mjs @@ -208,7 +208,14 @@ export function renderSetup(status) { lines.push(`- **Providers**: None configured. Run \`!opencode providers\` to set up.`); } if (status.reviewGate !== undefined) { - lines.push(`- **Review Gate**: ${status.reviewGate ? "Enabled" : "Disabled"}`); + const parts = [status.reviewGate ? "Enabled" : "Disabled"]; + if (status.reviewGateMaxPerSession != null) { + parts.push(`limit ${status.reviewGateMaxPerSession}/session`); + } + if (status.reviewGateCooldownMinutes != null) { + parts.push(`cooldown ${status.reviewGateCooldownMinutes} min`); + } + lines.push(`- **Review Gate**: ${parts.join(", ")}`); } return lines.join("\n"); diff --git a/plugins/opencode/scripts/lib/review-agent.mjs b/plugins/opencode/scripts/lib/review-agent.mjs index 187a37a..647df78 100644 --- a/plugins/opencode/scripts/lib/review-agent.mjs +++ b/plugins/opencode/scripts/lib/review-agent.mjs @@ -73,7 +73,7 @@ export async function resolveReviewAgent(client, log = () => {}) { const names = extractAgentNames(agents); if (names.includes("review")) { - return { agent: "review" }; + return { agent: "review", tools: undefined }; } log( diff --git a/plugins/opencode/scripts/lib/state.mjs b/plugins/opencode/scripts/lib/state.mjs index d5f02f0..1c537bb 100644 --- a/plugins/opencode/scripts/lib/state.mjs +++ b/plugins/opencode/scripts/lib/state.mjs @@ -3,23 +3,260 @@ // JSON state file, per-job files and logs. import crypto from "node:crypto"; +import fs from "node:fs"; +import os from "node:os"; import path from "node:path"; import { ensureDir, readJson, writeJson } from "./fs.mjs"; const MAX_JOBS = 50; +const FALLBACK_STATE_ROOT_DIR = path.join(os.tmpdir(), "opencode-companion"); +const FALLBACK_LOCK_STALE_MS = 30_000; +const MIGRATION_LOCK_STALE_MS = 5 * 60 * 1000; +const MIGRATION_WAIT_MS = 2000; +const PATH_KEYS = new Set(["logFile", "dataFile"]); +const STATE_LOCK_STALE_MS = 30_000; +const STATE_LOCK_WAIT_MS = 5_000; +const STATE_LOCK_RETRY_MS = 50; + +function workspaceHash(workspacePath) { + return crypto.createHash("sha256").update(workspacePath).digest("hex").slice(0, 16); +} + +function sleepSync(ms) { + Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, ms); +} + +function waitForMigration(primaryState) { + const deadline = Date.now() + MIGRATION_WAIT_MS; + while (!fs.existsSync(primaryState) && Date.now() < deadline) { + sleepSync(25); + } +} + +function acquireFallbackLock(fallbackDir) { + const lockPath = `${fallbackDir}.migration.lock`; + ensureDir(path.dirname(lockPath)); + const deadline = Date.now() + FALLBACK_LOCK_STALE_MS; + while (true) { + try { + return { fd: fs.openSync(lockPath, "wx"), lockPath }; + } catch (err) { + if (err?.code !== "EEXIST") throw err; + try { + const stat = fs.statSync(lockPath); + if (Date.now() - stat.mtimeMs > FALLBACK_LOCK_STALE_MS) { + fs.rmSync(lockPath, { force: true }); + continue; + } + } catch (statErr) { + if (statErr?.code !== "ENOENT") throw statErr; + } + if (Date.now() >= deadline) return null; + sleepSync(STATE_LOCK_RETRY_MS); + } + } +} + +function releaseFallbackLock(lock) { + if (!lock) return; + if (lock.fd != null) { + try { fs.closeSync(lock.fd); } catch {} + } + try { fs.rmSync(lock.lockPath, { force: true }); } catch {} +} + +function acquireMigrationLock(lockPath, primaryState) { + fs.mkdirSync(path.dirname(lockPath), { recursive: true, mode: 0o700 }); + try { + return fs.openSync(lockPath, "wx"); + } catch (err) { + if (err?.code !== "EEXIST") throw err; + + try { + const stat = fs.statSync(lockPath); + if (Date.now() - stat.mtimeMs > MIGRATION_LOCK_STALE_MS) { + fs.rmSync(lockPath, { force: true }); + return fs.openSync(lockPath, "wx"); + } + } catch (statErr) { + if (statErr?.code !== "ENOENT") throw statErr; + } + + waitForMigration(primaryState); + return null; + } +} + +function assertSafeMigrationTree(dir) { + for (const entry of fs.readdirSync(dir)) { + const entryPath = path.join(dir, entry); + const stat = fs.lstatSync(entryPath); + if (stat.isSymbolicLink()) { + throw new Error(`Refusing to migrate state containing symlink: ${entryPath}`); + } + if (stat.isDirectory()) { + assertSafeMigrationTree(entryPath); + } else if (!stat.isFile()) { + throw new Error(`Refusing to migrate non-regular state file: ${entryPath}`); + } + } +} + +function chmodPrivateRecursive(dir) { + fs.chmodSync(dir, 0o700); + for (const entry of fs.readdirSync(dir)) { + const entryPath = path.join(dir, entry); + const stat = fs.lstatSync(entryPath); + if (stat.isDirectory()) { + chmodPrivateRecursive(entryPath); + } else if (stat.isFile()) { + fs.chmodSync(entryPath, 0o600); + } + } +} + +function rewritePathPrefix(value, fallbackDir, primaryDir) { + if (typeof value !== "string") return value; + if (value === fallbackDir) return primaryDir; + const boundary = fallbackDir.endsWith(path.sep) ? fallbackDir : `${fallbackDir}${path.sep}`; + if (value.startsWith(boundary)) { + return path.join(primaryDir, value.slice(boundary.length)); + } + return value; +} + +function rewriteKnownPathValues(value, fallbackDir, primaryDir, key = null) { + if (Array.isArray(value)) { + return value.map((item) => rewriteKnownPathValues(item, fallbackDir, primaryDir)); + } + if (value && typeof value === "object") { + const rewritten = {}; + for (const [childKey, childValue] of Object.entries(value)) { + rewritten[childKey] = rewriteKnownPathValues(childValue, fallbackDir, primaryDir, childKey); + } + return rewritten; + } + return PATH_KEYS.has(key) ? rewritePathPrefix(value, fallbackDir, primaryDir) : value; +} + +function rewriteJsonPathFile(filePath, fallbackDir, primaryDir) { + try { + const data = JSON.parse(fs.readFileSync(filePath, "utf8")); + const rewritten = rewriteKnownPathValues(data, fallbackDir, primaryDir); + fs.writeFileSync(filePath, `${JSON.stringify(rewritten, null, 2)}\n`, "utf8"); + } catch { + // non-fatal — malformed job data should not prevent state migration + } +} + +function rewriteMigratedJsonPaths(rootDir, fallbackDir, primaryDir) { + const stack = [rootDir]; + while (stack.length > 0) { + const dir = stack.pop(); + for (const entry of fs.readdirSync(dir, { withFileTypes: true })) { + const entryPath = path.join(dir, entry.name); + if (entry.isDirectory()) { + stack.push(entryPath); + } else if (entry.isFile() && entry.name.endsWith(".json")) { + rewriteJsonPathFile(entryPath, fallbackDir, primaryDir); + } + } + } +} + +/** + * One-time migration: if state for this workspace exists only in the tmpdir + * fallback (written by a command that ran without CLAUDE_PLUGIN_DATA), copy + * it into the persistent plugin-data dir so future reads/writes go there and + * state survives /tmp cleanup. + * + * Absolute paths embedded in the migrated JSON (logFile references, job data + * paths) are rewritten to point at the new location. + */ +function migrateTmpdirStateIfNeeded(fallbackDir, primaryDir) { + const primaryState = path.join(primaryDir, "state.json"); + const fallbackState = path.join(fallbackDir, "state.json"); + if (fs.existsSync(primaryState) || !fs.existsSync(fallbackState)) return; + + const lockPath = `${primaryDir}.migrate.lock`; + const stageDir = `${primaryDir}.migrate-${process.pid}-${Date.now()}-${crypto.randomBytes(4).toString("hex")}`; + let lockFd = null; + let fallbackLock = null; + try { + lockFd = acquireMigrationLock(lockPath, primaryState); + if (lockFd === null) return; + if (fs.existsSync(primaryState) || !fs.existsSync(fallbackState)) return; + + fallbackLock = acquireFallbackLock(fallbackDir); + if (fallbackLock === null) return; + + assertSafeMigrationTree(fallbackDir); + fs.rmSync(stageDir, { recursive: true, force: true }); + fs.cpSync(fallbackDir, stageDir, { + recursive: true, + verbatimSymlinks: true, + }); + rewriteMigratedJsonPaths(stageDir, fallbackDir, primaryDir); + chmodPrivateRecursive(stageDir); + + if (fs.existsSync(primaryDir) && !fs.existsSync(primaryState)) { + fs.rmSync(primaryDir, { recursive: true, force: true }); + } + fs.renameSync(stageDir, primaryDir); + } catch { + // If migration fails for any reason, fall through and let the caller + // operate on whatever state is visible. A failed migration must never + // crash a status/cancel call. + } finally { + fs.rmSync(stageDir, { recursive: true, force: true }); + if (lockFd !== null) { + try { + fs.closeSync(lockFd); + } catch { + // best-effort + } + fs.rmSync(lockPath, { force: true }); + } + releaseFallbackLock(fallbackLock); + } +} + /** * Compute the state directory root for a workspace. * @param {string} workspacePath * @returns {string} */ export function stateRoot(workspacePath) { - const base = - process.env.CLAUDE_PLUGIN_DATA - ? path.join(process.env.CLAUDE_PLUGIN_DATA, "state") - : path.join("/tmp", "opencode-companion"); - const hash = crypto.createHash("sha256").update(workspacePath).digest("hex").slice(0, 16); - return path.join(base, hash); + const hash = workspaceHash(workspacePath); + const pluginDataDir = process.env.CLAUDE_PLUGIN_DATA; + + if (pluginDataDir) { + const primaryDir = path.join(pluginDataDir, "state", hash); + const fallbackDir = path.join(FALLBACK_STATE_ROOT_DIR, hash); + migrateTmpdirStateIfNeeded(fallbackDir, primaryDir); + + // If migration did not complete — e.g. another process holds the + // `.migrate.lock`, the wait timed out, or the migrator crashed and + // left a fresh lock — and fallback state still exists while primary + // state does not, keep operating against the fallback directory. + // Otherwise loadState would return an empty state from the missing + // primary file and the next write would create `primary/state.json` + // with only the new entry, orphaning every existing fallback job. + // Future stateRoot calls will retry migration until it succeeds. + try { + const primaryStateExists = fs.existsSync(path.join(primaryDir, "state.json")); + const fallbackStateExists = fs.existsSync(path.join(fallbackDir, "state.json")); + if (!primaryStateExists && fallbackStateExists) { + return fallbackDir; + } + } catch { + // existsSync should not throw in normal use; be defensive. + } + return primaryDir; + } + + return path.join(FALLBACK_STATE_ROOT_DIR, hash); } /** @@ -31,15 +268,106 @@ function stateFile(root) { return path.join(root, "state.json"); } +/** + * Acquire an exclusive lock on the state file for a given root directory. + * Uses a sibling `.lock` file created with O_EXCL. Stale locks older than + * STATE_LOCK_STALE_MS are forcibly removed. Blocks up to STATE_LOCK_WAIT_MS + * with STATE_LOCK_RETRY_MS intervals. + * + * @param {string} root - the stateRoot directory + * @returns {{ fd: number, lockPath: string }} + */ +function acquireStateLock(root) { + const lockPath = stateFile(root) + ".lock"; + ensureDir(path.dirname(lockPath)); + + const deadline = Date.now() + STATE_LOCK_WAIT_MS; + + while (true) { + try { + const fd = fs.openSync(lockPath, "wx"); + try { + fs.writeSync(fd, `${process.pid}\n${new Date().toISOString()}\n`); + } catch {} + return { fd, lockPath }; + } catch (err) { + if (err?.code !== "EEXIST") throw err; + + try { + const stat = fs.statSync(lockPath); + if (Date.now() - stat.mtimeMs > STATE_LOCK_STALE_MS) { + fs.rmSync(lockPath, { force: true }); + continue; + } + } catch (statErr) { + if (statErr?.code !== "ENOENT") throw statErr; + continue; + } + + if (Date.now() >= deadline) { + throw new Error( + `Timed out waiting for state lock after ${Math.round(STATE_LOCK_WAIT_MS / 1000)}s: ${lockPath}. ` + + "If no other companion process is running, delete the lock file manually." + ); + } + + sleepSync(STATE_LOCK_RETRY_MS); + } + } +} + +/** + * Release a state lock previously acquired by acquireStateLock. + * @param {{ fd: number | null, lockPath: string } | null} lock + */ +function releaseStateLock(lock) { + if (!lock) return; + if (lock.fd != null) { + try { fs.closeSync(lock.fd); } catch {} + } + try { + fs.rmSync(lock.lockPath, { force: true }); + const dirPath = path.dirname(lock.lockPath); + let dirFd = null; + try { + dirFd = fs.openSync(dirPath, "r"); + fs.fsyncSync(dirFd); + } catch { + // fsync best-effort + } finally { + if (dirFd !== null) { + try { fs.closeSync(dirFd); } catch {} + } + } + } catch {} +} + +/** + * Load state from an already-resolved root directory (no migration check). + * @param {string} root + * @returns {{ config: object, jobs: object[] }} + */ +function loadStateFromRoot(root) { + const data = readJson(stateFile(root)); + return data ?? { config: {}, jobs: [] }; +} + +/** + * Save state to an already-resolved root directory (no migration check). + * @param {string} root + * @param {object} state + */ +function saveStateToRoot(root, state) { + writeJson(stateFile(root), state); +} + /** * Load the state for a workspace. * @param {string} workspacePath * @returns {{ config: object, jobs: object[] }} */ export function loadState(workspacePath) { - const root = stateRoot(workspacePath); - const data = readJson(stateFile(root)); - return data ?? { config: {}, jobs: [] }; + return loadStateFromRoot(stateRoot(workspacePath)); } /** @@ -48,21 +376,28 @@ export function loadState(workspacePath) { * @param {object} state */ export function saveState(workspacePath, state) { - const root = stateRoot(workspacePath); - writeJson(stateFile(root), state); + saveStateToRoot(stateRoot(workspacePath), state); } /** - * Update the state atomically using a mutator function. + * Update the state atomically using a mutator function. Acquires an + * exclusive file lock for the read-modify-write cycle so concurrent + * companion processes cannot lose each other's writes. * @param {string} workspacePath * @param {(state: object) => void} mutator * @returns {object} the updated state */ export function updateState(workspacePath, mutator) { - const state = loadState(workspacePath); - mutator(state); - saveState(workspacePath, state); - return state; + const root = stateRoot(workspacePath); + const lock = acquireStateLock(root); + try { + const state = loadStateFromRoot(root); + mutator(state); + saveStateToRoot(root, state); + return state; + } finally { + releaseStateLock(lock); + } } /** diff --git a/plugins/opencode/scripts/lib/tracked-jobs.mjs b/plugins/opencode/scripts/lib/tracked-jobs.mjs index 48b0b66..6205a47 100644 --- a/plugins/opencode/scripts/lib/tracked-jobs.mjs +++ b/plugins/opencode/scripts/lib/tracked-jobs.mjs @@ -4,9 +4,26 @@ import fs from "node:fs"; import path from "node:path"; import { ensureDir, appendLine } from "./fs.mjs"; import { generateJobId, upsertJob, jobLogPath, jobDataPath } from "./state.mjs"; +import { getProcessStartToken } from "./process.mjs"; const SESSION_ID_ENV = "OPENCODE_COMPANION_SESSION_ID"; +// Hard ceiling for any single tracked job. 30 minutes is generous enough for +// long OpenCode turns but bounded so a hung runner cannot keep the companion +// process alive forever. Override via OPENCODE_COMPANION_JOB_TIMEOUT_MS. +const DEFAULT_JOB_TIMEOUT_MS = 30 * 60 * 1000; + +function resolveJobTimeoutMs(options = {}) { + if (Number.isFinite(options.timeoutMs) && options.timeoutMs > 0) { + return options.timeoutMs; + } + const fromEnv = Number(process.env.OPENCODE_COMPANION_JOB_TIMEOUT_MS); + if (Number.isFinite(fromEnv) && fromEnv > 0) { + return fromEnv; + } + return DEFAULT_JOB_TIMEOUT_MS; +} + /** * Get the current Claude session ID from environment. * @returns {string|undefined} @@ -41,11 +58,17 @@ export function createJobRecord(workspacePath, type, meta = {}) { * @param {string} workspacePath * @param {object} job * @param {(ctx: { report: Function, log: Function }) => Promise} runner + * @param {{ timeoutMs?: number }} [options] * @returns {Promise} the job result */ -export async function runTrackedJob(workspacePath, job, runner) { +export async function runTrackedJob(workspacePath, job, runner, options = {}) { // Mark as running - upsertJob(workspacePath, { id: job.id, status: "running", pid: process.pid }); + upsertJob(workspacePath, { + id: job.id, + status: "running", + pid: process.pid, + pidStartToken: getProcessStartToken(process.pid), + }); const logFile = jobLogPath(workspacePath, job.id); ensureDir(path.dirname(logFile)); @@ -61,14 +84,41 @@ export async function runTrackedJob(workspacePath, job, runner) { appendLine(logFile, `[${new Date().toISOString()}] ${message}`); }; + // Race the runner against a hard wall-clock timeout so a hung runner + // (dropped SSE stream, wedged post-response handler, unresolved downstream + // fetch) cannot leave the job in `running` forever. See issue #41. + const timeoutMs = resolveJobTimeoutMs(options); + let timeoutHandle = null; + const timeoutPromise = new Promise((_resolve, reject) => { + timeoutHandle = setTimeout(() => { + reject( + new Error( + `Tracked job ${job.id} exceeded the ${Math.round(timeoutMs / 1000)}s hard timeout. ` + + "The runner did not produce a terminal status. " + + "Set OPENCODE_COMPANION_JOB_TIMEOUT_MS to adjust." + ) + ); + }, timeoutMs); + }); + + const clearTimer = () => { + if (timeoutHandle) { + clearTimeout(timeoutHandle); + timeoutHandle = null; + } + }; + try { report("starting", `Job ${job.id} started`); - const result = await runner({ report, log }); + const result = await Promise.race([runner({ report, log }), timeoutPromise]); + clearTimer(); // Mark as completed upsertJob(workspacePath, { id: job.id, status: "completed", + pid: null, + pidStartToken: null, completedAt: new Date().toISOString(), result: result?.rendered ?? result?.summary ?? null, }); @@ -81,9 +131,13 @@ export async function runTrackedJob(workspacePath, job, runner) { report("completed", `Job ${job.id} completed`); return result; } catch (err) { + clearTimer(); upsertJob(workspacePath, { id: job.id, status: "failed", + phase: "failed", + pid: null, + pidStartToken: null, completedAt: new Date().toISOString(), errorMessage: err.message, }); diff --git a/plugins/opencode/scripts/lib/worktree.mjs b/plugins/opencode/scripts/lib/worktree.mjs new file mode 100644 index 0000000..b39599b --- /dev/null +++ b/plugins/opencode/scripts/lib/worktree.mjs @@ -0,0 +1,70 @@ +// Disposable git-worktree sessions for isolated write-capable rescue runs. +// Wraps the lower-level helpers in lib/git.mjs so handleTask can swap the +// working directory transparently and offer the user a keep/discard choice +// at the end. + +import { + getGitRoot, + createWorktree, + removeWorktree, + deleteWorktreeBranch, + getWorktreeDiff, + applyWorktreePatch, +} from "./git.mjs"; + +/** + * Create a new worktree session rooted at `cwd`'s repo. Throws if cwd is + * not inside a git repository. + * @param {string} cwd + * @returns {Promise<{ worktreePath: string, branch: string, repoRoot: string, baseCommit: string, timestamp: number }>} + */ +export async function createWorktreeSession(cwd) { + const repoRoot = await getGitRoot(cwd); + if (!repoRoot) { + throw new Error("Not a git repository — --worktree requires one."); + } + return createWorktree(repoRoot); +} + +/** + * Compute the diff produced inside a worktree session. + * @param {{ worktreePath: string, baseCommit: string }} session + * @returns {Promise<{ stat: string, patch: string }>} + */ +export async function diffWorktreeSession(session) { + return getWorktreeDiff(session.worktreePath, session.baseCommit); +} + +/** + * Tear down a worktree session. When `keep` is true, the diff is first + * applied back to the repo as a staged patch; on success the worktree and + * branch are removed. On apply failure the worktree is preserved so the + * user can recover manually. + * + * @param {{ worktreePath: string, branch: string, repoRoot: string, baseCommit: string }} session + * @param {{ keep?: boolean }} [opts] + * @returns {Promise<{ applied: boolean, detail: string }>} + */ +export async function cleanupWorktreeSession(session, opts = {}) { + const keep = Boolean(opts.keep); + + if (keep) { + const result = await applyWorktreePatch( + session.repoRoot, + session.worktreePath, + session.baseCommit + ); + // Only tear down when the apply succeeded or there was nothing to apply. + // On a real apply failure we leave the worktree in place for recovery. + if (!result.applied && result.detail !== "No changes to apply.") { + return result; + } + await removeWorktree(session.repoRoot, session.worktreePath); + await deleteWorktreeBranch(session.repoRoot, session.branch); + return result; + } + + await removeWorktree(session.repoRoot, session.worktreePath); + await deleteWorktreeBranch(session.repoRoot, session.branch); + return { applied: false, detail: "Worktree discarded." }; +} diff --git a/plugins/opencode/scripts/opencode-companion.mjs b/plugins/opencode/scripts/opencode-companion.mjs index 4ff2215..223e953 100644 --- a/plugins/opencode/scripts/opencode-companion.mjs +++ b/plugins/opencode/scripts/opencode-companion.mjs @@ -46,6 +46,8 @@ // now honor `--model` / `--free` end-to-end. // (Apache License 2.0 §4(b) modification notice.) +import crypto from "node:crypto"; +import os from "node:os"; import path from "node:path"; import process from "node:process"; import fs from "node:fs"; @@ -55,7 +57,14 @@ import { isOpencodeInstalled, getOpencodeVersion, spawnDetached, getConfiguredPr import { isServerRunning, ensureServer, createClient, connect } from "./lib/opencode-server.mjs"; import { resolveWorkspace } from "./lib/workspace.mjs"; import { loadState, updateState, upsertJob, generateJobId, jobDataPath } from "./lib/state.mjs"; -import { buildStatusSnapshot, resolveResultJob, resolveCancelableJob, enrichJob } from "./lib/job-control.mjs"; +import { + buildStatusSnapshot, + resolveResultJob, + resolveCancelableJob, + enrichJob, + reconcileAllJobs, + matchJobReference, +} from "./lib/job-control.mjs"; import { createJobRecord, runTrackedJob, getClaudeSessionId } from "./lib/tracked-jobs.mjs"; import { renderStatus, @@ -67,6 +76,11 @@ import { } from "./lib/render.mjs"; import { buildReviewPrompt, buildTaskPrompt } from "./lib/prompts.mjs"; import { getDiff, getStatus as getGitStatus, detectPrReference } from "./lib/git.mjs"; +import { + createWorktreeSession, + diffWorktreeSession, + cleanupWorktreeSession, +} from "./lib/worktree.mjs"; import { readJson } from "./lib/fs.mjs"; import { resolveReviewAgent } from "./lib/review-agent.mjs"; import { parseModelString, selectFreeModel } from "./lib/model.mjs"; @@ -86,6 +100,8 @@ const handlers = { task: handleTask, "task-worker": handleTaskWorker, "task-resume-candidate": handleTaskResumeCandidate, + "last-review": handleLastReview, + "worktree-cleanup": handleWorktreeCleanup, status: handleStatus, result: handleResult, cancel: handleCancel, @@ -109,9 +125,36 @@ handler(argv).catch((err) => { async function handleSetup(argv) { const { options } = parseArgs(argv, { + valueOptions: ["review-gate-max", "review-gate-cooldown"], booleanOptions: ["json", "enable-review-gate", "disable-review-gate"], }); + let reviewGateOverride; + if (options["enable-review-gate"]) reviewGateOverride = true; + if (options["disable-review-gate"]) reviewGateOverride = false; + + let reviewGateMaxPerSessionOverride; + if (options["review-gate-max"] != null) { + const raw = options["review-gate-max"]; + const max = raw === "off" ? null : Number(raw); + if (max !== null && (!Number.isInteger(max) || max < 1)) { + console.error(`--review-gate-max must be a positive integer or "off".`); + process.exit(1); + } + reviewGateMaxPerSessionOverride = max; + } + + let reviewGateCooldownMinutesOverride; + if (options["review-gate-cooldown"] != null) { + const raw = options["review-gate-cooldown"]; + const cooldown = raw === "off" ? null : Number(raw); + if (cooldown !== null && (!Number.isInteger(cooldown) || cooldown < 1)) { + console.error(`--review-gate-cooldown must be a positive integer (minutes) or "off".`); + process.exit(1); + } + reviewGateCooldownMinutesOverride = cooldown; + } + const installed = await isOpencodeInstalled(); const version = installed ? await getOpencodeVersion() : null; @@ -127,31 +170,48 @@ async function handleSetup(argv) { // auth methods each provider supports. auth.json is the same source // of truth that `opencode providers list` uses, and it works whether // or not the OpenCode server is running. - providers = getConfiguredProviders(); + try { + providers = getConfiguredProviders(); + } catch (err) { + console.error(`Warning: could not read configured providers: ${err.message}`); + } } - // Handle review gate toggle + // Apply setup config changes only after all inputs have been validated. const workspace = await resolveWorkspace(); - let reviewGate = false; - - if (options["enable-review-gate"]) { - updateState(workspace, (state) => { - state.config = state.config || {}; - state.config.reviewGate = true; - }); - reviewGate = true; - } else if (options["disable-review-gate"]) { + if ( + reviewGateOverride !== undefined || + reviewGateMaxPerSessionOverride !== undefined || + reviewGateCooldownMinutesOverride !== undefined + ) { updateState(workspace, (state) => { state.config = state.config || {}; - state.config.reviewGate = false; + if (reviewGateOverride !== undefined) { + state.config.reviewGate = reviewGateOverride; + } + if (reviewGateMaxPerSessionOverride !== undefined) { + state.config.reviewGateMaxPerSession = reviewGateMaxPerSessionOverride; + } + if (reviewGateCooldownMinutesOverride !== undefined) { + state.config.reviewGateCooldownMinutes = reviewGateCooldownMinutesOverride; + } }); - reviewGate = false; - } else { - const state = loadState(workspace); - reviewGate = state.config?.reviewGate ?? false; } - const status = { installed, version, serverRunning, providers, reviewGate }; + const finalState = loadState(workspace); + const reviewGate = finalState.config?.reviewGate ?? false; + const reviewGateMaxPerSession = finalState.config?.reviewGateMaxPerSession ?? null; + const reviewGateCooldownMinutes = finalState.config?.reviewGateCooldownMinutes ?? null; + + const status = { + installed, + version, + serverRunning, + providers, + reviewGate, + reviewGateMaxPerSession, + reviewGateCooldownMinutes, + }; if (options.json) { console.log(JSON.stringify(status, null, 2)); @@ -203,8 +263,8 @@ async function handleReview(argv) { }); const prNumber = options.pr ? Number(options.pr) : null; - if (options.pr && !Number.isFinite(prNumber)) { - console.error(`Invalid --pr value: ${options.pr} (must be a number)`); + if (options.pr && (!Number.isFinite(prNumber) || prNumber <= 0)) { + console.error(`Invalid --pr value: ${options.pr} (must be a positive number)`); process.exit(1); } @@ -265,6 +325,7 @@ async function handleReview(argv) { }; }); + saveLastReview(workspace, result.rendered); console.log(result.rendered); } catch (err) { console.error(`Review failed: ${err.message}`); @@ -294,8 +355,8 @@ async function handleAdversarialReview(argv) { let prNumber = null; if (options.pr) { prNumber = Number(options.pr); - if (!Number.isFinite(prNumber)) { - console.error(`Invalid --pr value: ${options.pr} (must be a number)`); + if (!Number.isFinite(prNumber) || prNumber <= 0) { + console.error(`Invalid --pr value: ${options.pr} (must be a positive number)`); process.exit(1); } } else { @@ -356,6 +417,7 @@ async function handleAdversarialReview(argv) { }; }); + saveLastReview(workspace, result.rendered); console.log(result.rendered); } catch (err) { console.error(`Adversarial review failed: ${err.message}`); @@ -370,11 +432,11 @@ async function handleAdversarialReview(argv) { async function handleTask(argv) { const { options, positional } = parseArgs(argv, { valueOptions: ["model", "agent"], - booleanOptions: ["write", "background", "wait", "resume-last", "fresh", "free"], + booleanOptions: ["write", "background", "wait", "resume-last", "fresh", "worktree", "free"], }); const taskText = extractTaskText(argv, ["model", "agent"], [ - "write", "background", "wait", "resume-last", "fresh", "free", + "write", "background", "wait", "resume-last", "fresh", "worktree", "free", ]); if (!taskText) { @@ -396,6 +458,12 @@ async function handleTask(argv) { const workspace = await resolveWorkspace(); const isWrite = options.write !== undefined ? options.write : true; const agentName = options.agent ?? (isWrite ? "build" : "plan"); + const useWorktree = Boolean(options.worktree); + + if (useWorktree && !isWrite) { + console.error("--worktree requires --write (nothing to isolate in read-only mode)."); + process.exit(1); + } // Check for resume let resumeSessionId = null; @@ -415,6 +483,7 @@ async function handleTask(argv) { const job = createJobRecord(workspace, "task", { agent: agentName, resumeSessionId, + worktree: useWorktree, }); // Background mode: spawn a detached worker @@ -428,6 +497,7 @@ async function handleTask(argv) { "--agent", agentName, ]; if (isWrite) workerArgs.push("--write"); + if (useWorktree) workerArgs.push("--worktree"); if (resumeSessionId) workerArgs.push("--resume-session", resumeSessionId); // Pass the resolved model (from --model or --free) as a concrete // "provider/model-id" string so the worker doesn't need to re-run @@ -447,11 +517,34 @@ async function handleTask(argv) { return; } + // Set up a worktree session if requested. Foreground mode only. + let worktreeSession = null; + let effectiveCwd = workspace; + if (useWorktree) { + try { + worktreeSession = await createWorktreeSession(workspace); + effectiveCwd = worktreeSession.worktreePath; + upsertJob(workspace, { + id: job.id, + worktreeSession: { + worktreePath: worktreeSession.worktreePath, + branch: worktreeSession.branch, + repoRoot: worktreeSession.repoRoot, + baseCommit: worktreeSession.baseCommit, + timestamp: worktreeSession.timestamp, + }, + }); + } catch (err) { + console.error(`Failed to create worktree: ${err.message}`); + process.exit(1); + } + } + // Foreground mode try { const result = await runTrackedJob(workspace, job, async ({ report, log }) => { report("starting", "Connecting to OpenCode server..."); - const client = await connect({ cwd: workspace }); + const client = await connect({ cwd: effectiveCwd }); let sessionId; if (resumeSessionId) { @@ -467,7 +560,14 @@ async function handleTask(argv) { const prompt = buildTaskPrompt(taskText, { write: isWrite }); report("investigating", "Sending task to OpenCode..."); - log(`Agent: ${agentName}, Write: ${isWrite}, Prompt: ${prompt.length} chars${requestedModel?.raw ? `, model: ${requestedModel.raw}${options.free ? " (--free picked)" : ""}` : ""}`); + log( + `Agent: ${agentName}, Write: ${isWrite}, ` + + `Worktree: ${useWorktree ? worktreeSession.branch : "no"}, ` + + `Prompt: ${prompt.length} chars` + + (requestedModel?.raw + ? `, model: ${requestedModel.raw}${options.free ? " (--free picked)" : ""}` + : "") + ); const response = await client.sendPrompt(sessionId, prompt, { agent: agentName, @@ -486,30 +586,152 @@ async function handleTask(argv) { if (diff?.files) { changedFiles = diff.files.map((f) => f.path || f.name).filter(Boolean); } - } catch { - // diff endpoint may not be available + } catch (err) { + log(`Warning: could not retrieve diff - ${err.message}`); + } + } + + // If using a worktree, compute the actual git diff stat produced on + // disk. This is what the user will have to keep or discard. + let worktreeDiff = null; + if (worktreeSession) { + try { + worktreeDiff = await diffWorktreeSession(worktreeSession); + } catch (err) { + log(`Failed to compute worktree diff: ${err.message}`); } } return { - rendered: text, + rendered: worktreeSession + ? renderWorktreeTaskOutput(text, worktreeSession, worktreeDiff, job.id) + : text, messages: response, changedFiles, summary: text.slice(0, 500), + worktreeSession: worktreeSession + ? { + worktreePath: worktreeSession.worktreePath, + branch: worktreeSession.branch, + repoRoot: worktreeSession.repoRoot, + baseCommit: worktreeSession.baseCommit, + } + : null, }; }); console.log(result.rendered); } catch (err) { + // On any failure with a worktree, clean it up so we don't leak dirs. + if (worktreeSession) { + try { + await cleanupWorktreeSession(worktreeSession, { keep: false }); + } catch { + // best-effort + } + } console.error(`Task failed: ${err.message}`); process.exit(1); } } +function renderWorktreeTaskOutput(text, session, diff, jobId) { + const lines = []; + if (text) { + lines.push(text.trimEnd()); + lines.push(""); + } + lines.push("---"); + lines.push(""); + lines.push("## Worktree"); + lines.push(""); + lines.push(`Branch: \`${session.branch}\``); + lines.push(`Path: \`${session.worktreePath}\``); + lines.push(""); + if (diff?.stat) { + lines.push("### Changes"); + lines.push(""); + lines.push("```"); + lines.push(diff.stat); + lines.push("```"); + lines.push(""); + } else { + lines.push("OpenCode made no file changes in the worktree."); + lines.push(""); + } + lines.push("### Next steps"); + lines.push(""); + lines.push(`- **Keep**: \`node "\${CLAUDE_PLUGIN_ROOT}/scripts/opencode-companion.mjs" worktree-cleanup ${jobId} --action keep\``); + lines.push(`- **Discard**: \`node "\${CLAUDE_PLUGIN_ROOT}/scripts/opencode-companion.mjs" worktree-cleanup ${jobId} --action discard\``); + return lines.join("\n"); +} + +async function handleWorktreeCleanup(argv) { + const { options, positional } = parseArgs(argv, { + valueOptions: ["action"], + booleanOptions: ["json"], + }); + + const jobId = positional[0]; + if (!jobId) { + console.error("Usage: worktree-cleanup --action "); + process.exit(1); + } + const action = options.action; + if (action !== "keep" && action !== "discard") { + console.error("--action must be 'keep' or 'discard'."); + process.exit(1); + } + + const workspace = await resolveWorkspace(); + const state = loadState(workspace); + const { job, ambiguous } = matchJobReference(state.jobs ?? [], jobId); + if (ambiguous) { + console.error("Ambiguous job reference. Please provide a more specific ID prefix."); + process.exit(1); + } + if (!job) { + console.error(`No job found for ${jobId}.`); + process.exit(1); + } + + const session = job.worktreeSession; + if (!session?.worktreePath || !session?.branch || !session?.repoRoot) { + console.error(`Job ${jobId} has no worktree session. Was it run with --worktree?`); + process.exit(1); + } + + const result = await cleanupWorktreeSession(session, { keep: action === "keep" }); + + if (options.json) { + console.log(JSON.stringify({ jobId: job.id, action, result }, null, 2)); + return; + } + + const lines = ["# Worktree Cleanup", ""]; + if (action === "keep") { + if (result.applied) { + lines.push(`Applied changes from \`${session.branch}\` and cleaned up.`); + } else if (result.detail === "No changes to apply.") { + lines.push(`No changes to apply. Worktree and branch \`${session.branch}\` cleaned up.`); + } else { + lines.push(`Failed to apply changes: ${result.detail}`); + lines.push(""); + lines.push( + `The worktree and branch \`${session.branch}\` have been preserved at ` + + `\`${session.worktreePath}\` for manual recovery.` + ); + } + } else { + lines.push(`Discarded worktree \`${session.worktreePath}\` and branch \`${session.branch}\`.`); + } + console.log(lines.join("\n")); +} + async function handleTaskWorker(argv) { const { options } = parseArgs(argv, { valueOptions: ["job-id", "workspace", "task-text", "agent", "model", "resume-session"], - booleanOptions: ["write"], + booleanOptions: ["write", "worktree"], }); const workspace = options.workspace; @@ -517,6 +739,7 @@ async function handleTaskWorker(argv) { const taskText = options["task-text"]; const agentName = options.agent ?? "build"; const isWrite = !!options.write; + const useWorktree = Boolean(options.worktree); const resumeSessionId = options["resume-session"]; // Parent `handleTask` resolves --free/--model into a concrete // provider/model-id string before spawning us, so the worker just @@ -527,10 +750,60 @@ async function handleTaskWorker(argv) { process.exit(1); } + let worktreeSession = null; + let effectiveCwd = workspace; + if (useWorktree) { + try { + worktreeSession = await createWorktreeSession(workspace); + effectiveCwd = worktreeSession.worktreePath; + upsertJob(workspace, { + id: jobId, + worktreeSession: { + worktreePath: worktreeSession.worktreePath, + branch: worktreeSession.branch, + repoRoot: worktreeSession.repoRoot, + baseCommit: worktreeSession.baseCommit, + timestamp: worktreeSession.timestamp, + }, + }); + } catch (err) { + upsertJob(workspace, { + id: jobId, + status: "failed", + phase: "failed", + completedAt: new Date().toISOString(), + errorMessage: `Failed to create worktree: ${err.message}`, + }); + process.exit(1); + } + } + + let signalsHandled = false; + const handleSignal = async (signal) => { + if (signalsHandled) return; + signalsHandled = true; + upsertJob(workspace, { + id: jobId, + status: "failed", + completedAt: new Date().toISOString(), + errorMessage: `Worker terminated by ${signal}`, + }); + if (worktreeSession) { + try { + await cleanupWorktreeSession(worktreeSession, { keep: false }); + } catch { + // best-effort + } + } + process.exit(128 + (signal === "SIGINT" ? 2 : 15)); + }; + process.on("SIGINT", () => handleSignal("SIGINT")); + process.on("SIGTERM", () => handleSignal("SIGTERM")); + try { await runTrackedJob(workspace, { id: jobId }, async ({ report, log }) => { report("starting", "Background worker connecting to OpenCode..."); - const client = await connect({ cwd: workspace }); + const client = await connect({ cwd: effectiveCwd }); let sessionId; if (resumeSessionId) { @@ -545,7 +818,11 @@ async function handleTaskWorker(argv) { const prompt = buildTaskPrompt(taskText, { write: isWrite }); report("investigating", "Running task..."); - if (workerModel) log(`Worker model: ${options.model}`); + log( + `Agent: ${agentName}, Write: ${isWrite}, ` + + `Worktree: ${worktreeSession?.branch ?? "no"}` + + (workerModel ? `, model: ${options.model}` : "") + ); const response = await client.sendPrompt(sessionId, prompt, { agent: agentName, @@ -553,11 +830,41 @@ async function handleTaskWorker(argv) { }); const text = extractResponseText(response); + + let worktreeDiff = null; + if (worktreeSession) { + try { + worktreeDiff = await diffWorktreeSession(worktreeSession); + } catch (err) { + log(`Failed to compute worktree diff: ${err.message}`); + } + } + report("finalizing", "Done"); - return { rendered: text, summary: text.slice(0, 500) }; + return { + rendered: worktreeSession + ? renderWorktreeTaskOutput(text, worktreeSession, worktreeDiff, jobId) + : text, + summary: text.slice(0, 500), + worktreeSession: worktreeSession + ? { + worktreePath: worktreeSession.worktreePath, + branch: worktreeSession.branch, + repoRoot: worktreeSession.repoRoot, + baseCommit: worktreeSession.baseCommit, + } + : null, + }; }); } catch (err) { + if (worktreeSession) { + try { + await cleanupWorktreeSession(worktreeSession, { keep: false }); + } catch { + // best-effort + } + } // Error is already logged by runTrackedJob process.exit(1); } @@ -608,8 +915,9 @@ async function handleResult(argv) { const workspace = await resolveWorkspace(); const state = loadState(workspace); + const reconciled = reconcileAllJobs(state.jobs ?? [], workspace); - const { job, ambiguous } = resolveResultJob(state.jobs ?? [], ref); + const { job, ambiguous } = resolveResultJob(reconciled, ref); if (ambiguous) { console.error("Ambiguous job reference. Please provide a more specific ID prefix."); @@ -636,16 +944,26 @@ async function handleCancel(argv) { const workspace = await resolveWorkspace(); const state = loadState(workspace); + const sessionId = getClaudeSessionId(); + const reconciled = reconcileAllJobs(state.jobs ?? [], workspace); - const { job, ambiguous } = resolveCancelableJob(state.jobs ?? [], ref); + const { job, ambiguous, sessionScoped } = resolveCancelableJob( + reconciled, + ref, + { sessionId } + ); if (ambiguous) { - console.error("Multiple running jobs. Please specify a job ID prefix."); + console.error("Multiple active jobs. Please specify a job ID prefix."); process.exit(1); } if (!job) { - console.log("No active job to cancel."); + console.log( + sessionScoped + ? "No active OpenCode jobs to cancel for this session." + : "No active job to cancel." + ); return; } @@ -678,6 +996,75 @@ async function handleCancel(argv) { console.log(`Canceled job: ${job.id}`); } +// ------------------------------------------------------------------ +// Last-review persistence +// ------------------------------------------------------------------ + +/** + * Per-repo path where the most recent successful review is saved so the + * rescue command can pick it up without the user copy-pasting findings. + * @param {string} workspace + * @returns {{ dir: string, file: string }} + */ +function lastReviewPath(workspace) { + const hash = crypto.createHash("sha256").update(workspace).digest("hex").slice(0, 16); + const dir = process.env.CLAUDE_PLUGIN_DATA || path.join(os.homedir(), ".opencode-companion"); + return { dir, file: path.join(dir, `last-review-${hash}.md`) }; +} + +/** + * Best-effort persistence of a rendered review so the rescue flow can read + * it later. Never throws — a failed write must not fail the review itself. + * @param {string} workspace + * @param {string} rendered + */ +function saveLastReview(workspace, rendered) { + if (!rendered) return; + let tmp = null; + try { + const { dir, file } = lastReviewPath(workspace); + fs.mkdirSync(dir, { recursive: true, mode: 0o700 }); + tmp = `${file}.${process.pid}.${Date.now()}.tmp`; + fs.writeFileSync(tmp, rendered, "utf8"); + fs.renameSync(tmp, file); + tmp = null; + } catch { + // best-effort + } finally { + if (tmp) fs.rmSync(tmp, { force: true }); + } +} + +async function handleLastReview(argv) { + const { options } = parseArgs(argv, { booleanOptions: ["json", "content"] }); + const workspace = await resolveWorkspace(); + const { file } = lastReviewPath(workspace); + + if (!fs.existsSync(file)) { + if (options.json) console.log(JSON.stringify({ available: false })); + else console.log("NO_LAST_REVIEW"); + return; + } + + if (options.content) { + process.stdout.write(fs.readFileSync(file, "utf8")); + return; + } + + if (options.json) { + const stat = fs.statSync(file); + console.log( + JSON.stringify({ + available: true, + updatedAt: stat.mtime.toISOString(), + path: file, + }) + ); + } else { + console.log("LAST_REVIEW_AVAILABLE"); + } +} + // ------------------------------------------------------------------ // Helpers // ------------------------------------------------------------------ diff --git a/plugins/opencode/scripts/safe-command.mjs b/plugins/opencode/scripts/safe-command.mjs new file mode 100644 index 0000000..0aeae1a --- /dev/null +++ b/plugins/opencode/scripts/safe-command.mjs @@ -0,0 +1,147 @@ +#!/usr/bin/env node + +// Safe slash-command bridge. +// +// Claude command files feed raw `$ARGUMENTS` to this script via a quoted +// heredoc. That keeps shell metacharacters as data instead of executable +// syntax. This script then validates and forwards only the supported argv +// shapes to opencode-companion.mjs. + +import { spawnSync } from "node:child_process"; +import fs from "node:fs"; +import path from "node:path"; +import process from "node:process"; + +const companionScript = path.join(import.meta.dirname, "opencode-companion.mjs"); +const subcommand = process.argv[2]; +const raw = fs.readFileSync(0, "utf8").trim(); + +try { + const args = buildForwardArgs(subcommand, raw); + const result = spawnSync(process.execPath, [companionScript, ...args], { + cwd: process.cwd(), + env: process.env, + stdio: "inherit", + }); + if (result.error) throw result.error; + process.exit(result.status ?? 1); +} catch (err) { + console.error(err.message); + process.exit(1); +} + +function buildForwardArgs(command, text) { + if (command === "status") { + if (text) throw new Error("status does not accept arguments."); + return ["status"]; + } + + if (command === "cancel" || command === "result") { + if (!text) return [command]; + if (!/^[A-Za-z0-9._:-]+$/.test(text)) { + throw new Error("Invalid job reference. Use a job ID or safe ID prefix."); + } + return [command, text]; + } + + if (command === "setup") { + return ["setup", "--json", ...parseSetupArgs(text)]; + } + + throw new Error(`Unsupported safe command: ${command}`); +} + +function parseSetupArgs(text) { + const tokens = splitShellLike(text); + const out = []; + + for (let i = 0; i < tokens.length; i += 1) { + const token = tokens[i]; + if (token === "--json") { + continue; + } + if (token === "--enable-review-gate" || token === "--disable-review-gate") { + out.push(token); + continue; + } + if (token === "--review-gate-max" || token === "--review-gate-cooldown") { + const value = tokens[++i]; + if (value == null) { + throw new Error(`${token} requires a value.`); + } + if (value !== "off" && !/^[1-9][0-9]*$/.test(value)) { + throw new Error(`${token} must be a positive integer or "off".`); + } + out.push(token, value); + continue; + } + throw new Error(`Unsupported setup argument: ${token}`); + } + + return out; +} + +function splitShellLike(text) { + const tokens = []; + let current = ""; + let inToken = false; + let quote = null; + let escaping = false; + + for (const ch of text) { + if (escaping) { + current += ch; + inToken = true; + escaping = false; + continue; + } + + if (quote === "'") { + if (ch === "'") quote = null; + else current += ch; + inToken = true; + continue; + } + + if (quote === "\"") { + if (ch === "\"") { + quote = null; + } else if (ch === "\\") { + escaping = true; + } else { + current += ch; + } + inToken = true; + continue; + } + + if (/\s/.test(ch)) { + if (inToken) { + tokens.push(current); + current = ""; + inToken = false; + } + continue; + } + + if (ch === "'" || ch === "\"") { + quote = ch; + inToken = true; + continue; + } + + if (ch === "\\") { + escaping = true; + inToken = true; + continue; + } + + current += ch; + inToken = true; + } + + if (escaping) current += "\\"; + if (quote) throw new Error("Unterminated quoted argument."); + if (inToken) tokens.push(current); + return tokens; +} diff --git a/plugins/opencode/scripts/stop-review-gate-hook.mjs b/plugins/opencode/scripts/stop-review-gate-hook.mjs index 699d21a..3814a26 100644 --- a/plugins/opencode/scripts/stop-review-gate-hook.mjs +++ b/plugins/opencode/scripts/stop-review-gate-hook.mjs @@ -8,12 +8,38 @@ import process from "node:process"; import fs from "node:fs"; import path from "node:path"; import { resolveWorkspace } from "./lib/workspace.mjs"; -import { loadState } from "./lib/state.mjs"; +import { loadState, updateState } from "./lib/state.mjs"; import { isServerRunning, connect } from "./lib/opencode-server.mjs"; import { resolveReviewAgent } from "./lib/review-agent.mjs"; const PLUGIN_ROOT = process.env.CLAUDE_PLUGIN_ROOT || path.resolve(import.meta.dirname, ".."); +function currentSessionId() { + return ( + process.env.OPENCODE_COMPANION_SESSION_ID || + process.env.CLAUDE_SESSION_ID || + null + ); +} + +/** + * Prune review-gate usage entries older than 7 days so long-lived workspace + * state doesn't grow unbounded across many Claude sessions. + */ +function pruneOldUsage(usage) { + if (!usage || typeof usage !== "object") return {}; + const cutoff = Date.now() - 7 * 24 * 60 * 60 * 1000; + const kept = {}; + for (const [sid, entry] of Object.entries(usage)) { + if (!entry?.lastRunAt) continue; + const ts = new Date(entry.lastRunAt).getTime(); + if (Number.isFinite(ts) && ts >= cutoff) { + kept[sid] = entry; + } + } + return kept; +} + async function main() { const workspace = await resolveWorkspace(); @@ -25,6 +51,27 @@ async function main() { return; } + // Throttle check — honor per-session cap and cooldown if configured. + // Skipped when no session id is available (can't scope the limit safely). + const sessionId = currentSessionId(); + if (sessionId) { + const usage = state.reviewGateUsage?.[sessionId] ?? { count: 0, lastRunAt: null }; + const max = state.config.reviewGateMaxPerSession; + if (Number.isFinite(max) && usage.count >= max) { + console.log(`ALLOW: Review gate session cap (${max}) reached.`); + return; + } + const cooldownMin = state.config.reviewGateCooldownMinutes; + if (Number.isFinite(cooldownMin) && usage.lastRunAt) { + const elapsedMs = Date.now() - new Date(usage.lastRunAt).getTime(); + if (Number.isFinite(elapsedMs) && elapsedMs < cooldownMin * 60 * 1000) { + const remaining = Math.ceil((cooldownMin * 60 * 1000 - elapsedMs) / 1000); + console.log(`ALLOW: Review gate cooldown (${remaining}s remaining).`); + return; + } + } + } + // Check if server is available if (!(await isServerRunning())) { console.log("ALLOW: OpenCode server not running."); @@ -75,6 +122,20 @@ async function main() { const text = extractText(response); const firstLine = text.trim().split("\n")[0]; + // Bump usage before returning a verdict so the count reflects work + // actually done, even if the verdict BLOCKs. + if (sessionId) { + const nowIso = new Date().toISOString(); + updateState(workspace, (s) => { + s.reviewGateUsage = pruneOldUsage(s.reviewGateUsage); + const prior = s.reviewGateUsage[sessionId] ?? { count: 0, lastRunAt: null }; + s.reviewGateUsage[sessionId] = { + count: prior.count + 1, + lastRunAt: nowIso, + }; + }); + } + if (firstLine.startsWith("BLOCK")) { // Output BLOCK to stderr so Claude Code sees it process.stderr.write(`OpenCode review gate: ${firstLine}\n`); diff --git a/plugins/opencode/skills/opencode-runtime/SKILL.md b/plugins/opencode/skills/opencode-runtime/SKILL.md index ea41db0..192caa3 100644 --- a/plugins/opencode/skills/opencode-runtime/SKILL.md +++ b/plugins/opencode/skills/opencode-runtime/SKILL.md @@ -28,6 +28,7 @@ Command selection: - If the forwarded request includes `--free`, pass it through to `task`. The companion will shell out to `opencode models`, filter for first-party `opencode/*` free-tier entries (`:free` or `-free`), and pick one at random. `--free` is restricted to `opencode/*` because OpenRouter free models have inconsistent tool-use support. - If the forwarded request includes both `--free` and `--model`, do not invoke `task` — return nothing, because the companion will reject the combination. - If the forwarded request includes `--agent`, pass it through to `task`. +- If the forwarded request includes `--worktree`, pass it through to `task`. This runs OpenCode in a disposable git worktree so the user can keep or discard the change after the run finishes. - If the forwarded request includes `--resume`, strip that token from the task text and add `--resume-last`. - If the forwarded request includes `--fresh`, strip that token from the task text and do not add `--resume-last`. - `--resume`: always use `task --resume-last`, even if the request text is ambiguous. diff --git a/tests/companion-cli.test.mjs b/tests/companion-cli.test.mjs new file mode 100644 index 0000000..39f4df0 --- /dev/null +++ b/tests/companion-cli.test.mjs @@ -0,0 +1,128 @@ +import { describe, it, beforeEach, afterEach } from "node:test"; +import assert from "node:assert/strict"; +import { spawn } from "node:child_process"; +import fs from "node:fs"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; +import { createTmpDir, cleanupTmpDir, setupTestEnv } from "./helpers.mjs"; +import { runCommand } from "../plugins/opencode/scripts/lib/process.mjs"; +import { loadState, saveState } from "../plugins/opencode/scripts/lib/state.mjs"; + +const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), ".."); +const companionScript = path.join(repoRoot, "plugins", "opencode", "scripts", "opencode-companion.mjs"); +const safeCommandScript = path.join(repoRoot, "plugins", "opencode", "scripts", "safe-command.mjs"); + +let tmpDir; +let workspace; + +beforeEach(() => { + tmpDir = createTmpDir("companion-cli"); + workspace = path.join(tmpDir, "workspace"); + fs.mkdirSync(workspace); + setupTestEnv(tmpDir); +}); + +afterEach(() => { + cleanupTmpDir(tmpDir); +}); + +function runNodeScript(args, opts = {}) { + return new Promise((resolve, reject) => { + const child = spawn(process.execPath, args, { + cwd: opts.cwd ?? workspace, + env: { ...process.env, CLAUDE_PLUGIN_DATA: tmpDir, ...(opts.env ?? {}) }, + stdio: ["pipe", "pipe", "pipe"], + }); + let stdout = ""; + let stderr = ""; + child.stdout.on("data", (chunk) => (stdout += chunk)); + child.stderr.on("data", (chunk) => (stderr += chunk)); + child.on("error", reject); + child.on("close", (exitCode) => resolve({ exitCode, stdout, stderr })); + child.stdin.end(opts.input ?? ""); + }); +} + +describe("opencode-companion CLI", () => { + it("worktree-cleanup rejects ambiguous job ID prefixes", async () => { + saveState(workspace, { + config: {}, + jobs: [ + { + id: "task-abc", + type: "task", + worktreeSession: { + worktreePath: path.join(tmpDir, "wt-a"), + branch: "opencode/a", + repoRoot: workspace, + }, + }, + { + id: "task-abd", + type: "task", + worktreeSession: { + worktreePath: path.join(tmpDir, "wt-b"), + branch: "opencode/b", + repoRoot: workspace, + }, + }, + ], + }); + + const result = await runCommand( + "node", + [companionScript, "worktree-cleanup", "task-a", "--action", "discard"], + { cwd: workspace, env: { CLAUDE_PLUGIN_DATA: tmpDir } } + ); + + assert.equal(result.exitCode, 1); + assert.match(result.stderr, /Ambiguous job reference/); + assert.equal(result.stdout, ""); + }); + + it("setup validates all config inputs before mutating state", async () => { + saveState(workspace, { + config: { reviewGate: false }, + jobs: [], + }); + + const result = await runNodeScript([ + companionScript, + "setup", + "--json", + "--enable-review-gate", + "--review-gate-max", + "0", + ]); + + assert.equal(result.exitCode, 1); + assert.match(result.stderr, /--review-gate-max must be a positive integer/); + assert.equal(loadState(workspace).config.reviewGate, false); + assert.equal(loadState(workspace).config.reviewGateMaxPerSession, undefined); + }); + + it("safe-command forwards setup multi-token args from stdin", async () => { + const result = await runNodeScript( + [safeCommandScript, "setup"], + { input: "--enable-review-gate --review-gate-max 2 --review-gate-cooldown off\n" } + ); + + assert.equal(result.exitCode, 0); + const status = JSON.parse(result.stdout); + assert.equal(status.reviewGate, true); + assert.equal(status.reviewGateMaxPerSession, 2); + assert.equal(status.reviewGateCooldownMinutes, null); + }); + + it("safe-command rejects shell-shaped job refs as data", async () => { + const marker = path.join(tmpDir, "should-not-exist"); + const result = await runNodeScript( + [safeCommandScript, "cancel"], + { input: `$(touch ${marker})\n` } + ); + + assert.equal(result.exitCode, 1); + assert.match(result.stderr, /Invalid job reference/); + assert.equal(fs.existsSync(marker), false); + }); +}); diff --git a/tests/dead-pid-reconcile.test.mjs b/tests/dead-pid-reconcile.test.mjs new file mode 100644 index 0000000..eff64ab --- /dev/null +++ b/tests/dead-pid-reconcile.test.mjs @@ -0,0 +1,266 @@ +import { describe, it, before, after, beforeEach } from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { + markDeadPidJobFailed, + reconcileIfDead, + reconcileAllJobs, + buildStatusSnapshot, +} from "../plugins/opencode/scripts/lib/job-control.mjs"; +import { upsertJob, loadState, saveState } from "../plugins/opencode/scripts/lib/state.mjs"; +import { + getProcessStartToken, + isProcessAlive, +} from "../plugins/opencode/scripts/lib/process.mjs"; + +// PID 999999 is virtually guaranteed to be dead on macOS/Linux (well above +// pid_max for typical configurations and any short-lived workload). +const DEAD_PID = 999_999; + +describe("isProcessAlive", () => { + it("returns false for null/undefined/invalid pids", () => { + assert.equal(isProcessAlive(null), false); + assert.equal(isProcessAlive(undefined), false); + assert.equal(isProcessAlive(0), false); + assert.equal(isProcessAlive(-1), false); + assert.equal(isProcessAlive(NaN), false); + }); + + it("returns true for the current process", () => { + assert.equal(isProcessAlive(process.pid), true); + }); + + it("returns false when the PID start token does not match", () => { + const token = getProcessStartToken(process.pid); + if (!token) return; + assert.equal(isProcessAlive(process.pid, `${token}-stale`), false); + }); + + it("returns false for a clearly dead pid", () => { + assert.equal(isProcessAlive(DEAD_PID), false); + }); +}); + +describe("markDeadPidJobFailed", () => { + let workspace; + let previousPluginData; + + before(() => { + workspace = fs.mkdtempSync(path.join(os.tmpdir(), "opencode-deadpid-")); + previousPluginData = process.env.CLAUDE_PLUGIN_DATA; + process.env.CLAUDE_PLUGIN_DATA = fs.mkdtempSync( + path.join(os.tmpdir(), "opencode-deadpid-data-") + ); + }); + + after(() => { + if (previousPluginData == null) { + delete process.env.CLAUDE_PLUGIN_DATA; + } else { + process.env.CLAUDE_PLUGIN_DATA = previousPluginData; + } + }); + + beforeEach(() => { + // Actually wipe on-disk state between tests. The prior mutator-only + // approach left entries from earlier tests in state.json, which made + // the suite order-dependent as soon as IDs collided or MAX_JOBS + // pruning kicked in. + saveState(workspace, { config: {}, jobs: [] }); + }); + + it("rewrites a running job with a dead pid to failed", () => { + upsertJob(workspace, { + id: "task-1", + status: "running", + type: "task", + pid: DEAD_PID, + }); + const written = markDeadPidJobFailed(workspace, "task-1", DEAD_PID); + assert.equal(written, true); + + const stored = loadState(workspace).jobs.find((j) => j.id === "task-1"); + assert.equal(stored.status, "failed"); + assert.equal(stored.phase, "failed"); + assert.equal(stored.pid, null); + assert.match(stored.errorMessage, new RegExp(`PID ${DEAD_PID}`)); + }); + + it("refuses to downgrade a terminal state", () => { + upsertJob(workspace, { + id: "task-2", + status: "completed", + type: "task", + pid: DEAD_PID, + }); + const written = markDeadPidJobFailed(workspace, "task-2", DEAD_PID); + assert.equal(written, false); + + const stored = loadState(workspace).jobs.find((j) => j.id === "task-2"); + assert.equal(stored.status, "completed"); + }); + + it("refuses when the pid has changed since probe", () => { + upsertJob(workspace, { + id: "task-3", + status: "running", + type: "task", + pid: 12345, + }); + // Probe observed DEAD_PID, but latest pid is 12345 → refuse. + const written = markDeadPidJobFailed(workspace, "task-3", DEAD_PID); + assert.equal(written, false); + + const stored = loadState(workspace).jobs.find((j) => j.id === "task-3"); + assert.equal(stored.status, "running"); + }); + + it("refuses when the pid start token has changed since probe", () => { + upsertJob(workspace, { + id: "task-4", + status: "running", + type: "task", + pid: 12345, + pidStartToken: "start-a", + }); + const written = markDeadPidJobFailed(workspace, "task-4", 12345, "start-b"); + assert.equal(written, false); + + const stored = loadState(workspace).jobs.find((j) => j.id === "task-4"); + assert.equal(stored.status, "running"); + }); +}); + +describe("reconcileIfDead", () => { + let workspace; + let previousPluginData; + + before(() => { + workspace = fs.mkdtempSync(path.join(os.tmpdir(), "opencode-reconcile-")); + previousPluginData = process.env.CLAUDE_PLUGIN_DATA; + process.env.CLAUDE_PLUGIN_DATA = fs.mkdtempSync( + path.join(os.tmpdir(), "opencode-reconcile-data-") + ); + }); + + after(() => { + if (previousPluginData == null) { + delete process.env.CLAUDE_PLUGIN_DATA; + } else { + process.env.CLAUDE_PLUGIN_DATA = previousPluginData; + } + }); + + it("leaves a running job with a live pid alone", () => { + upsertJob(workspace, { + id: "live-1", + status: "running", + type: "task", + pid: process.pid, + }); + const result = reconcileIfDead(workspace, { + id: "live-1", + status: "running", + pid: process.pid, + }); + assert.equal(result.status, "running"); + }); + + it("reconciles a running job with a dead pid to failed", () => { + upsertJob(workspace, { + id: "dead-1", + status: "running", + type: "task", + pid: DEAD_PID, + }); + const result = reconcileIfDead(workspace, { + id: "dead-1", + status: "running", + pid: DEAD_PID, + }); + assert.equal(result.status, "failed"); + assert.equal(result.phase, "failed"); + }); + + it("honors OPENCODE_COMPANION_NO_RECONCILE", () => { + const previous = process.env.OPENCODE_COMPANION_NO_RECONCILE; + process.env.OPENCODE_COMPANION_NO_RECONCILE = "1"; + try { + upsertJob(workspace, { + id: "dead-opt-out", + status: "running", + type: "task", + pid: DEAD_PID, + }); + const job = { id: "dead-opt-out", status: "running", pid: DEAD_PID }; + const result = reconcileIfDead(workspace, job); + assert.equal(result, job); + } finally { + if (previous == null) delete process.env.OPENCODE_COMPANION_NO_RECONCILE; + else process.env.OPENCODE_COMPANION_NO_RECONCILE = previous; + } + }); + + it("leaves running jobs with no pid alone", () => { + const job = { id: "nopid", status: "running", pid: null }; + const result = reconcileIfDead(workspace, job); + assert.equal(result, job); + }); + + it("leaves terminal-state jobs alone", () => { + const job = { id: "done", status: "completed", pid: DEAD_PID }; + const result = reconcileIfDead(workspace, job); + assert.equal(result, job); + }); +}); + +describe("buildStatusSnapshot reconciles dead pids inline", () => { + let workspace; + let previousPluginData; + + before(() => { + workspace = fs.mkdtempSync(path.join(os.tmpdir(), "opencode-snapshot-")); + previousPluginData = process.env.CLAUDE_PLUGIN_DATA; + process.env.CLAUDE_PLUGIN_DATA = fs.mkdtempSync( + path.join(os.tmpdir(), "opencode-snapshot-data-") + ); + }); + + after(() => { + if (previousPluginData == null) { + delete process.env.CLAUDE_PLUGIN_DATA; + } else { + process.env.CLAUDE_PLUGIN_DATA = previousPluginData; + } + }); + + it("surfaces a dead-pid running job as failed in one read", () => { + const now = new Date().toISOString(); + const jobs = [ + { + id: "zombie", + status: "running", + type: "task", + pid: DEAD_PID, + createdAt: now, + updatedAt: now, + }, + ]; + upsertJob(workspace, jobs[0]); + + const snapshot = buildStatusSnapshot(jobs, workspace); + assert.equal(snapshot.running.length, 0); + assert.ok(snapshot.latestFinished); + assert.equal(snapshot.latestFinished.id, "zombie"); + assert.equal(snapshot.latestFinished.status, "failed"); + }); +}); + +describe("reconcileAllJobs", () => { + it("returns input unchanged when empty", () => { + assert.deepEqual(reconcileAllJobs([], "/tmp"), []); + assert.equal(reconcileAllJobs(null, "/tmp"), null); + }); +}); diff --git a/tests/job-control.test.mjs b/tests/job-control.test.mjs index ed82802..aafacd3 100644 --- a/tests/job-control.test.mjs +++ b/tests/job-control.test.mjs @@ -61,12 +61,58 @@ describe("job-control", () => { assert.equal(job.id, "task-def"); }); - it("resolveCancelableJob returns null when no running jobs", () => { + it("resolveCancelableJob returns null when no active jobs", () => { const noRunning = jobs.filter((j) => j.status !== "running"); const { job } = resolveCancelableJob(noRunning); assert.equal(job, null); }); + it("resolveCancelableJob treats pending jobs as cancelable", () => { + const pendingJobs = [ + { id: "task-pending", status: "pending", type: "task", updatedAt: "2026-01-01T03:00:00Z", createdAt: "2026-01-01T03:00:00Z" }, + ]; + const { job } = resolveCancelableJob(pendingJobs, "task-pending"); + assert.equal(job.id, "task-pending"); + }); + + it("resolveCancelableJob default is scoped to sessionId when provided", () => { + const multiSession = [ + { id: "task-mine", status: "running", type: "task", sessionId: "S1", updatedAt: "2026-01-01T02:00:00Z", createdAt: "2026-01-01T01:30:00Z" }, + { id: "task-other", status: "running", type: "task", sessionId: "S2", updatedAt: "2026-01-01T02:05:00Z", createdAt: "2026-01-01T01:35:00Z" }, + ]; + const { job, sessionScoped } = resolveCancelableJob(multiSession, undefined, { sessionId: "S1" }); + assert.equal(job.id, "task-mine"); + assert.equal(sessionScoped, true); + }); + + it("resolveCancelableJob default returns null when session has no active jobs", () => { + const multiSession = [ + { id: "task-other", status: "running", type: "task", sessionId: "S2", updatedAt: "2026-01-01T02:05:00Z", createdAt: "2026-01-01T01:35:00Z" }, + ]; + const { job, sessionScoped } = resolveCancelableJob(multiSession, undefined, { sessionId: "S1" }); + assert.equal(job, null); + assert.equal(sessionScoped, true); + }); + + it("resolveCancelableJob explicit ref searches across sessions", () => { + const multiSession = [ + { id: "task-mine", status: "running", type: "task", sessionId: "S1", updatedAt: "2026-01-01T02:00:00Z", createdAt: "2026-01-01T01:30:00Z" }, + { id: "task-other", status: "running", type: "task", sessionId: "S2", updatedAt: "2026-01-01T02:05:00Z", createdAt: "2026-01-01T01:35:00Z" }, + ]; + const { job } = resolveCancelableJob(multiSession, "task-other", { sessionId: "S1" }); + assert.equal(job.id, "task-other"); + }); + + it("resolveCancelableJob default is ambiguous when session has multiple active jobs", () => { + const multiSession = [ + { id: "task-a", status: "running", type: "task", sessionId: "S1", updatedAt: "2026-01-01T02:00:00Z", createdAt: "2026-01-01T01:30:00Z" }, + { id: "task-b", status: "pending", type: "task", sessionId: "S1", updatedAt: "2026-01-01T02:05:00Z", createdAt: "2026-01-01T01:35:00Z" }, + ]; + const { job, ambiguous } = resolveCancelableJob(multiSession, undefined, { sessionId: "S1" }); + assert.equal(job.id, "task-a"); + assert.equal(ambiguous, true); + }); + it("buildStatusSnapshot separates running and finished", () => { const snapshot = buildStatusSnapshot(jobs, "/tmp/test"); assert.equal(snapshot.running.length, 1); @@ -74,4 +120,14 @@ describe("job-control", () => { assert.ok(snapshot.latestFinished); assert.equal(snapshot.recent.length, 2); }); + + it("buildStatusSnapshot treats pending jobs as active, not finished", () => { + const pendingJobs = [ + { id: "task-pending", status: "pending", type: "task", updatedAt: "2026-01-01T03:00:00Z", createdAt: "2026-01-01T03:00:00Z" }, + ...jobs, + ]; + const snapshot = buildStatusSnapshot(pendingJobs, "/tmp/test"); + assert.equal(snapshot.running.some((j) => j.id === "task-pending"), true); + assert.equal(snapshot.recent.some((j) => j.id === "task-pending"), false); + }); }); diff --git a/tests/process.test.mjs b/tests/process.test.mjs index 7951df1..4ff3f1e 100644 --- a/tests/process.test.mjs +++ b/tests/process.test.mjs @@ -25,6 +25,29 @@ describe("process", () => { const { stderr, exitCode } = await runCommand("sh", ["-c", "echo err >&2"]); assert.ok(stderr.includes("err")); }); + + it("runCommand caps stdout at maxOutputBytes and reports overflowed", async () => { + // 20k bytes of 'a' via awk — POSIX-portable, works on BSD and GNU. + const { stdout, overflowed } = await runCommand( + "awk", + ["BEGIN{for(i=0;i<20000;i++)printf \"a\"}"], + { maxOutputBytes: 1000 } + ); + assert.equal(overflowed, true); + assert.ok(stdout.length <= 1000, `stdout length ${stdout.length} exceeded cap 1000`); + // The captured prefix should still be 'a' characters (partial read). + assert.match(stdout, /^a+$/); + }); + + it("runCommand does not overflow when output is under the cap", async () => { + const { stdout, overflowed } = await runCommand( + "sh", + ["-c", "printf 'hello'"], + { maxOutputBytes: 1000 } + ); + assert.equal(overflowed, false); + assert.equal(stdout, "hello"); + }); }); // Tests for OpenCode auth.json discovery + provider detection. diff --git a/tests/review-prompt-size.test.mjs b/tests/review-prompt-size.test.mjs new file mode 100644 index 0000000..a692614 --- /dev/null +++ b/tests/review-prompt-size.test.mjs @@ -0,0 +1,147 @@ +import { describe, it, beforeEach, afterEach } from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; +import { createTmpDir, cleanupTmpDir } from "./helpers.mjs"; +import { runCommand } from "../plugins/opencode/scripts/lib/process.mjs"; +import { buildReviewPrompt } from "../plugins/opencode/scripts/lib/prompts.mjs"; + +const PLUGIN_ROOT = path.resolve( + path.dirname(fileURLToPath(import.meta.url)), + "..", + "plugins", + "opencode" +); + +let tmpDir; + +beforeEach(async () => { + tmpDir = createTmpDir("review-prompt-size-"); + await runCommand("git", ["init", "-q"], { cwd: tmpDir }); + await runCommand("git", ["config", "user.email", "t@t.t"], { cwd: tmpDir }); + await runCommand("git", ["config", "user.name", "t"], { cwd: tmpDir }); + // Initial commit with 4 tracked files so subsequent tests can modify + // without needing to stage new files. + for (const name of ["a.js", "b.js", "c.js", "d.js"]) { + fs.writeFileSync(path.join(tmpDir, name), `export const ${name[0]} = 1;\n`); + } + await runCommand("git", ["add", "."], { cwd: tmpDir }); + await runCommand("git", ["commit", "-q", "-m", "init"], { cwd: tmpDir }); +}); + +afterEach(() => { + cleanupTmpDir(tmpDir); +}); + +describe("buildReviewPrompt large-diff fallback", () => { + it("inlines the diff for a small working-tree change", async () => { + fs.writeFileSync(path.join(tmpDir, "a.js"), "export const a = 'INLINE_MARKER';\n"); + + const prompt = await buildReviewPrompt( + tmpDir, + { adversarial: true, focus: "test" }, + PLUGIN_ROOT + ); + + assert.match(prompt, //); + assert.match(prompt, /INLINE_MARKER/); + assert.match(prompt, /primary evidence/); + assert.doesNotMatch(prompt, /diff too large to inline/); + }); + + it("includes a bounded diff excerpt when maxInlineDiffBytes is exceeded", async () => { + fs.writeFileSync( + path.join(tmpDir, "a.js"), + `export const a = '${"x".repeat(2048)}';\n` + ); + + const prompt = await buildReviewPrompt( + tmpDir, + { + adversarial: true, + focus: "test", + maxInlineDiffBytes: 128, + maxInlineDiffFiles: 100, + }, + PLUGIN_ROOT + ); + + assert.match(prompt, //); + assert.match(prompt, /\n/); + assert.match(prompt, //); + assert.match(prompt, /bounded diff excerpt/); + assert.doesNotMatch(prompt, /read-only git commands/); + // The x-heavy content should not be inlined. + assert.doesNotMatch(prompt, /xxxxxxxxxxxxxx/); + }); + + it("marks broad file-count reviews but keeps diff evidence in the prompt", async () => { + // Modify 3 already-tracked files; cap at 2. + for (const name of ["b.js", "c.js", "d.js"]) { + fs.writeFileSync(path.join(tmpDir, name), `export const ${name[0]} = 'modified';\n`); + } + + const prompt = await buildReviewPrompt( + tmpDir, + { + adversarial: true, + focus: "test", + maxInlineDiffFiles: 2, + maxInlineDiffBytes: 10_000_000, + }, + PLUGIN_ROOT + ); + + assert.match(prompt, //); + assert.match(prompt, /\n/); + assert.match(prompt, /modified/); + assert.match(prompt, /broad changed-file set/); + assert.doesNotMatch(prompt, /bounded diff excerpt/); + }); + + it("bounds the diff read so huge diffs never fully materialize", async () => { + // Create a file whose modification generates a diff much larger than the + // inline cap. With the bounded-read fix the diff fetch should stop early + // and report overflowed, producing a lightweight excerpt — not a full + // in-memory materialization of the multi-megabyte diff. + const big = "x".repeat(50_000); + fs.writeFileSync(path.join(tmpDir, "a.js"), `export const a = '${big}';\n`); + + const prompt = await buildReviewPrompt( + tmpDir, + { + adversarial: true, + focus: "test", + maxInlineDiffFiles: 100, + maxInlineDiffBytes: 2048, // 2 KB cap + }, + PLUGIN_ROOT + ); + + // The excerpt-marker must be present and the x-heavy content must not + // have fully landed in the prompt (we would see >= 50000 x's otherwise). + assert.match(prompt, /bounded diff excerpt/); + const xCount = (prompt.match(/x/g) ?? []).length; + assert.ok( + xCount < 10_000, + `expected bounded diff, but saw ${xCount} 'x' chars in prompt` + ); + }); + + it("injects collection guidance into adversarial template", async () => { + fs.writeFileSync(path.join(tmpDir, "a.js"), "export const a = 'XXXX';\n"); + + const prompt = await buildReviewPrompt( + tmpDir, + { adversarial: true, focus: "test" }, + PLUGIN_ROOT + ); + + // The template still has both opening/closing tags. + assert.match(prompt, //); + assert.match(prompt, /<\/review_collection_guidance>/); + // The placeholder must be replaced, not left as literal. + assert.doesNotMatch(prompt, /\{\{REVIEW_COLLECTION_GUIDANCE\}\}/); + }); +}); diff --git a/tests/state-lock.test.mjs b/tests/state-lock.test.mjs new file mode 100644 index 0000000..14df759 --- /dev/null +++ b/tests/state-lock.test.mjs @@ -0,0 +1,153 @@ +import { describe, it, beforeEach, afterEach } from "node:test"; +import assert from "node:assert/strict"; +import { fork } from "node:child_process"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { fileURLToPath, pathToFileURL } from "node:url"; +import { createTmpDir, cleanupTmpDir, setupTestEnv } from "./helpers.mjs"; +import { + loadState, + updateState, + upsertJob, + stateRoot, +} from "../plugins/opencode/scripts/lib/state.mjs"; + +const __dirname = path.dirname(fileURLToPath(import.meta.url)); +const stateModulePath = path.resolve(__dirname, "..", "plugins", "opencode", "scripts", "lib", "state.mjs"); +// Module specifiers in generated ESM source must be valid URLs. On Windows +// the resolved path contains backslashes and a drive letter, which would be +// an invalid specifier / emit escape sequences inside the generated import. +const stateModuleSpecifier = pathToFileURL(stateModulePath).href; + +let tmpDir; +const workspace = "/lock-test/workspace"; + +beforeEach(() => { + tmpDir = createTmpDir("state-lock-"); + setupTestEnv(tmpDir); +}); + +afterEach(() => { + cleanupTmpDir(tmpDir); +}); + +describe("updateState lock", () => { + it("creates and removes a lock file during updateState", () => { + updateState(workspace, (state) => { + const root = stateRoot(workspace); + const lockPath = path.join(root, "state.json.lock"); + assert.ok( + fs.existsSync(lockPath), + "lock file should exist while mutator is running" + ); + state.config.test = true; + }); + + const root = stateRoot(workspace); + const lockPath = path.join(root, "state.json.lock"); + assert.ok( + !fs.existsSync(lockPath), + "lock file should be removed after updateState completes" + ); + }); + + it("removes lock file even when mutator throws", () => { + try { + updateState(workspace, () => { + throw new Error("mutator explosion"); + }); + } catch {} + + const root = stateRoot(workspace); + const lockPath = path.join(root, "state.json.lock"); + assert.ok( + !fs.existsSync(lockPath), + "lock file should be cleaned up after mutator error" + ); + }); + + it("cleans up stale lock files older than 30 seconds", () => { + const root = stateRoot(workspace); + const lockPath = path.join(root, "state.json.lock"); + fs.mkdirSync(root, { recursive: true }); + + fs.writeFileSync(lockPath, "stale\n"); + const staleTime = new Date(Date.now() - 60_000); + fs.utimesSync(lockPath, staleTime, staleTime); + + updateState(workspace, (state) => { + state.config.staleRecovery = true; + }); + + const loaded = loadState(workspace); + assert.equal(loaded.config.staleRecovery, true); + assert.ok(!fs.existsSync(lockPath), "stale lock should have been removed"); + }); + + it("concurrent processes do not lose each other's writes", async () => { + const scriptDir = fs.mkdtempSync(path.join(os.tmpdir(), "concurrent-test-")); + const script = path.join(scriptDir, "writer.mjs"); + + const dataDir = tmpDir; + + fs.writeFileSync( + script, + `import { upsertJob } from "${stateModuleSpecifier}";\n` + + `import fs from "node:fs";\n` + + `process.env.CLAUDE_PLUGIN_DATA = "${dataDir}";\n` + + `process.env.OPENCODE_COMPANION_SESSION_ID = "child-session";\n` + + `const workspace = "${workspace}";\n` + + `const marker = "${script}.ready";\n` + + `const goSignal = "${script}.go";\n` + + `const doneSignal = "${script}.done";\n` + + `fs.writeFileSync(marker, "ready");\n` + + `while (!fs.existsSync(goSignal)) await new Promise(r => setTimeout(r, 20));\n` + + `upsertJob(workspace, { id: "child-job", status: "running", type: "task" });\n` + + `fs.writeFileSync(doneSignal, "done");\n` + ); + + try { + const child = fork(script, [], { stdio: "pipe" }); + + const readyMarker = `${script}.ready`; + const goMarker = `${script}.go`; + const doneMarker = `${script}.done`; + + const readyDeadline = Date.now() + 10_000; + while (!fs.existsSync(readyMarker) && Date.now() < readyDeadline) { + await new Promise((r) => setTimeout(r, 50)); + } + assert.ok(fs.existsSync(readyMarker), "child did not signal ready in time"); + + fs.writeFileSync(goMarker, "go"); + + const doneDeadline = Date.now() + 10_000; + while (!fs.existsSync(doneMarker) && Date.now() < doneDeadline) { + await new Promise((r) => setTimeout(r, 50)); + } + assert.ok(fs.existsSync(doneMarker), "child did not finish in time"); + + await new Promise((resolve) => { + child.on("exit", resolve); + setTimeout(() => { child.kill(); resolve(); }, 5_000); + }); + + upsertJob(workspace, { id: "parent-job", status: "completed", type: "review" }); + + const state = loadState(workspace); + const childJob = state.jobs.find((j) => j.id === "child-job"); + const parentJob = state.jobs.find((j) => j.id === "parent-job"); + + assert.ok(childJob, "child job was lost — concurrent write race"); + assert.ok(parentJob, "parent job was lost — concurrent write race"); + assert.equal(childJob.status, "running"); + assert.equal(parentJob.status, "completed"); + } finally { + try { fs.rmSync(scriptDir, { recursive: true, force: true }); } catch {} + for (const ext of [".ready", ".go", ".done"]) { + try { fs.rmSync(`${script}${ext}`, { force: true }); } catch {} + } + } + }); +}); diff --git a/tests/state-migration.test.mjs b/tests/state-migration.test.mjs new file mode 100644 index 0000000..8de7d5f --- /dev/null +++ b/tests/state-migration.test.mjs @@ -0,0 +1,216 @@ +import { describe, it, beforeEach, afterEach } from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { createTmpDir, cleanupTmpDir } from "./helpers.mjs"; +import { + stateRoot, + loadState, + upsertJob, +} from "../plugins/opencode/scripts/lib/state.mjs"; + +let previousPluginData; + +beforeEach(() => { + previousPluginData = process.env.CLAUDE_PLUGIN_DATA; +}); + +afterEach(() => { + if (previousPluginData == null) { + delete process.env.CLAUDE_PLUGIN_DATA; + } else { + process.env.CLAUDE_PLUGIN_DATA = previousPluginData; + } +}); + +describe("stateRoot fallback", () => { + it("uses os.tmpdir when CLAUDE_PLUGIN_DATA is unset", () => { + delete process.env.CLAUDE_PLUGIN_DATA; + const root = stateRoot("/some/workspace"); + assert.ok(root.startsWith(os.tmpdir()), `${root} does not start with ${os.tmpdir()}`); + assert.match(root, /opencode-companion/); + }); + + it("uses CLAUDE_PLUGIN_DATA when set", () => { + const dataDir = createTmpDir("opencode-data-"); + try { + process.env.CLAUDE_PLUGIN_DATA = dataDir; + const root = stateRoot("/some/workspace"); + assert.ok(root.startsWith(dataDir), `${root} does not start with ${dataDir}`); + } finally { + cleanupTmpDir(dataDir); + } + }); +}); + +describe("stateRoot tmpdir → plugin-data migration", () => { + let workspace; + let dataDir; + + beforeEach(() => { + workspace = "/migrate/test/workspace-" + Math.random().toString(16).slice(2); + dataDir = createTmpDir("opencode-migrate-data-"); + }); + + afterEach(() => { + cleanupTmpDir(dataDir); + }); + + it("migrates existing tmpdir state to plugin-data dir", () => { + // First, seed state in the tmpdir fallback (no CLAUDE_PLUGIN_DATA). + delete process.env.CLAUDE_PLUGIN_DATA; + upsertJob(workspace, { id: "pre-migration", status: "completed" }); + const fallbackDir = stateRoot(workspace); + assert.ok(fallbackDir.startsWith(os.tmpdir())); + const fallbackStateFile = path.join(fallbackDir, "state.json"); + assert.ok(fs.existsSync(fallbackStateFile)); + + // Now set CLAUDE_PLUGIN_DATA and call stateRoot again — should migrate. + process.env.CLAUDE_PLUGIN_DATA = dataDir; + const primaryDir = stateRoot(workspace); + assert.ok(primaryDir.startsWith(dataDir)); + const primaryStateFile = path.join(primaryDir, "state.json"); + assert.ok(fs.existsSync(primaryStateFile), "state.json did not migrate to primary dir"); + + const migrated = loadState(workspace); + const job = migrated.jobs.find((j) => j.id === "pre-migration"); + assert.ok(job, "migrated state does not contain seeded job"); + assert.equal(job.status, "completed"); + }); + + it("rewrites absolute fallback paths in migrated JSON", () => { + delete process.env.CLAUDE_PLUGIN_DATA; + upsertJob(workspace, { id: "j1", status: "running" }); + const fallbackDir = stateRoot(workspace); + + // Write a job/*.json file containing an absolute reference to fallbackDir. + const jobsDir = path.join(fallbackDir, "jobs"); + fs.mkdirSync(jobsDir, { recursive: true }); + const jobFile = path.join(jobsDir, "j1.json"); + fs.writeFileSync( + jobFile, + JSON.stringify({ + id: "j1", + logFile: path.join(fallbackDir, "jobs", "j1.log"), + errorMessage: `do not rewrite ordinary text mentioning ${fallbackDir}`, + }), + "utf8" + ); + + process.env.CLAUDE_PLUGIN_DATA = dataDir; + const primaryDir = stateRoot(workspace); + const migratedJob = JSON.parse( + fs.readFileSync(path.join(primaryDir, "jobs", "j1.json"), "utf8") + ); + assert.ok( + migratedJob.logFile.startsWith(primaryDir), + `logFile not rewritten: ${migratedJob.logFile}` + ); + assert.ok(!migratedJob.logFile.startsWith(fallbackDir)); + assert.equal( + migratedJob.errorMessage, + `do not rewrite ordinary text mentioning ${fallbackDir}` + ); + }); + + it("migrates with private directory and file modes on POSIX", () => { + if (process.platform === "win32") return; + + delete process.env.CLAUDE_PLUGIN_DATA; + upsertJob(workspace, { id: "mode-check", status: "completed" }); + + process.env.CLAUDE_PLUGIN_DATA = dataDir; + const primaryDir = stateRoot(workspace); + const primaryState = path.join(primaryDir, "state.json"); + + assert.equal(fs.statSync(primaryDir).mode & 0o777, 0o700); + assert.equal(fs.statSync(primaryState).mode & 0o777, 0o600); + }); + + it("refuses to migrate fallback state containing symlinks", () => { + if (process.platform === "win32") return; + + delete process.env.CLAUDE_PLUGIN_DATA; + upsertJob(workspace, { id: "symlink-check", status: "completed" }); + const fallbackDir = stateRoot(workspace); + fs.symlinkSync("/etc/passwd", path.join(fallbackDir, "leak")); + + process.env.CLAUDE_PLUGIN_DATA = dataDir; + // Migration must reject the symlink and leave the primary dir empty. + // We can't use stateRoot() here because the fallback-guard from issue + // P1 now routes reads back to fallbackDir on incomplete migration, so + // compute the expected primary path directly. + const hash = path.basename(fallbackDir); // workspaceHash + const expectedPrimaryDir = path.join(dataDir, "state", hash); + stateRoot(workspace); // triggers migration attempt + assert.equal( + fs.existsSync(path.join(expectedPrimaryDir, "state.json")), + false, + "symlink-containing migration should leave primary state empty" + ); + }); + + it("keeps reading fallback state while migration lock is held", () => { + // Seed fallback state without CLAUDE_PLUGIN_DATA. + delete process.env.CLAUDE_PLUGIN_DATA; + upsertJob(workspace, { id: "pre-migration", status: "running", type: "task" }); + const fallbackDir = stateRoot(workspace); + const fallbackState = path.join(fallbackDir, "state.json"); + assert.ok(fs.existsSync(fallbackState)); + + // Switch to CLAUDE_PLUGIN_DATA and pre-create the migrate lock so + // migrateTmpdirStateIfNeeded bails out without copying anything. + // The lock path derives from primaryDir; compute it the same way + // stateRoot does. + process.env.CLAUDE_PLUGIN_DATA = dataDir; + const hash = path.basename(fallbackDir); // workspaceHash(workspace) + const primaryDir = path.join(dataDir, "state", hash); + const lockPath = `${primaryDir}.migrate.lock`; + fs.mkdirSync(path.dirname(lockPath), { recursive: true }); + fs.writeFileSync(lockPath, "held by another process"); + + try { + // stateRoot must fall back to the tmpdir dir while the lock is + // held — otherwise loadState reads an empty primary and writes + // clobber the seeded job. + const root = stateRoot(workspace); + assert.equal( + root, + fallbackDir, + "expected fallback dir while migrate lock is held, got primary" + ); + + const loaded = loadState(workspace); + const job = loaded.jobs.find((j) => j.id === "pre-migration"); + assert.ok(job, "seeded fallback job disappeared under held migrate lock"); + + // A write in this window should land in fallback without clobbering + // the existing job. + upsertJob(workspace, { id: "in-flight", status: "running", type: "task" }); + const after = loadState(workspace); + assert.ok(after.jobs.find((j) => j.id === "pre-migration"), "pre-migration clobbered"); + assert.ok(after.jobs.find((j) => j.id === "in-flight"), "in-flight write lost"); + } finally { + fs.rmSync(lockPath, { force: true }); + } + }); + + it("does not re-migrate when primary state already exists", () => { + delete process.env.CLAUDE_PLUGIN_DATA; + upsertJob(workspace, { id: "fallback-only", status: "completed" }); + + process.env.CLAUDE_PLUGIN_DATA = dataDir; + // First call migrates. + stateRoot(workspace); + const primaryDir = stateRoot(workspace); + upsertJob(workspace, { id: "primary-update", status: "running" }); + + // Now wipe the fallback so a re-migration would be observable. + // The existing primary state.json still has both seeded jobs. + const state = loadState(workspace); + assert.ok(state.jobs.find((j) => j.id === "fallback-only")); + assert.ok(state.jobs.find((j) => j.id === "primary-update")); + assert.ok(fs.existsSync(path.join(primaryDir, "state.json"))); + }); +}); diff --git a/tests/tracked-jobs-timeout.test.mjs b/tests/tracked-jobs-timeout.test.mjs new file mode 100644 index 0000000..6ec6ef4 --- /dev/null +++ b/tests/tracked-jobs-timeout.test.mjs @@ -0,0 +1,80 @@ +import { describe, it, before, after } from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { runTrackedJob } from "../plugins/opencode/scripts/lib/tracked-jobs.mjs"; +import { loadState } from "../plugins/opencode/scripts/lib/state.mjs"; + +describe("runTrackedJob timeout", () => { + let workspace; + let previousPluginData; + + before(() => { + workspace = fs.mkdtempSync(path.join(os.tmpdir(), "opencode-timeout-")); + previousPluginData = process.env.CLAUDE_PLUGIN_DATA; + process.env.CLAUDE_PLUGIN_DATA = fs.mkdtempSync( + path.join(os.tmpdir(), "opencode-timeout-data-") + ); + }); + + after(() => { + if (previousPluginData == null) { + delete process.env.CLAUDE_PLUGIN_DATA; + } else { + process.env.CLAUDE_PLUGIN_DATA = previousPluginData; + } + }); + + it("aborts a runner that never resolves and marks the job failed", async () => { + const job = { id: "timeout-never-1" }; + + await assert.rejects( + runTrackedJob( + workspace, + job, + () => new Promise(() => {}), + { timeoutMs: 50 } + ), + /hard timeout/i + ); + + const state = loadState(workspace); + const stored = state.jobs.find((j) => j.id === job.id); + assert.ok(stored); + assert.equal(stored.status, "failed"); + assert.equal(stored.phase, "failed"); + assert.match(stored.errorMessage, /hard timeout/i); + }); + + it("does not fire for runners that resolve quickly", async () => { + const job = { id: "timeout-quick-1" }; + + const result = await runTrackedJob( + workspace, + job, + async () => ({ rendered: "ok" }), + { timeoutMs: 60_000 } + ); + + assert.equal(result.rendered, "ok"); + const state = loadState(workspace); + const stored = state.jobs.find((j) => j.id === job.id); + assert.equal(stored.status, "completed"); + }); + + it("honors OPENCODE_COMPANION_JOB_TIMEOUT_MS env override", async () => { + const previous = process.env.OPENCODE_COMPANION_JOB_TIMEOUT_MS; + process.env.OPENCODE_COMPANION_JOB_TIMEOUT_MS = "40"; + try { + const job = { id: "timeout-env-1" }; + await assert.rejects( + runTrackedJob(workspace, job, () => new Promise(() => {})), + /hard timeout/i + ); + } finally { + if (previous == null) delete process.env.OPENCODE_COMPANION_JOB_TIMEOUT_MS; + else process.env.OPENCODE_COMPANION_JOB_TIMEOUT_MS = previous; + } + }); +}); diff --git a/tests/worktree.test.mjs b/tests/worktree.test.mjs new file mode 100644 index 0000000..1e9ebf1 --- /dev/null +++ b/tests/worktree.test.mjs @@ -0,0 +1,135 @@ +import { describe, it, beforeEach, afterEach } from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import path from "node:path"; +import { createTmpDir, cleanupTmpDir } from "./helpers.mjs"; +import { runCommand } from "../plugins/opencode/scripts/lib/process.mjs"; +import { + createWorktreeSession, + diffWorktreeSession, + cleanupWorktreeSession, +} from "../plugins/opencode/scripts/lib/worktree.mjs"; + +let repo; + +beforeEach(async () => { + repo = createTmpDir("worktree-"); + await runCommand("git", ["init", "-q"], { cwd: repo }); + await runCommand("git", ["config", "user.email", "t@t.t"], { cwd: repo }); + await runCommand("git", ["config", "user.name", "t"], { cwd: repo }); + fs.writeFileSync(path.join(repo, "a.js"), "export const a = 1;\n"); + await runCommand("git", ["add", "."], { cwd: repo }); + await runCommand("git", ["commit", "-q", "-m", "init"], { cwd: repo }); +}); + +afterEach(() => { + cleanupTmpDir(repo); +}); + +describe("worktree session", () => { + it("creates a worktree, branch, and gitignore exclude entry", async () => { + const session = await createWorktreeSession(repo); + try { + assert.ok(session.worktreePath); + assert.match(session.branch, /^opencode\//); + assert.ok(fs.existsSync(session.worktreePath)); + assert.ok(fs.existsSync(path.join(session.worktreePath, "a.js"))); + + const exclude = fs.readFileSync(path.join(repo, ".git", "info", "exclude"), "utf8"); + assert.match(exclude, /\.worktrees\//); + + // Branch exists + const branches = await runCommand("git", ["branch"], { cwd: repo }); + assert.match(branches.stdout, new RegExp(session.branch.replace(/\//g, "\\/"))); + } finally { + await cleanupWorktreeSession(session, { keep: false }).catch(() => {}); + } + }); + + it("diffWorktreeSession reports edits made inside the worktree", async () => { + const session = await createWorktreeSession(repo); + try { + fs.writeFileSync( + path.join(session.worktreePath, "a.js"), + "export const a = 'CHANGED';\n" + ); + const diff = await diffWorktreeSession(session); + assert.match(diff.stat, /1 file changed/); + assert.match(diff.patch, /CHANGED/); + } finally { + await cleanupWorktreeSession(session, { keep: false }).catch(() => {}); + } + }); + + it("cleanupWorktreeSession with keep:true applies the patch to the main tree", async () => { + const session = await createWorktreeSession(repo); + fs.writeFileSync( + path.join(session.worktreePath, "a.js"), + "export const a = 'KEPT';\n" + ); + const result = await cleanupWorktreeSession(session, { keep: true }); + assert.equal(result.applied, true); + // Worktree and branch should be gone. + assert.equal(fs.existsSync(session.worktreePath), false); + const branches = await runCommand("git", ["branch"], { cwd: repo }); + assert.doesNotMatch(branches.stdout, new RegExp(session.branch)); + // The change is staged in the main tree. + const staged = await runCommand("git", ["diff", "--cached"], { cwd: repo }); + assert.match(staged.stdout, /KEPT/); + }); + + it("cleanupWorktreeSession with keep:false discards worktree and branch", async () => { + const session = await createWorktreeSession(repo); + fs.writeFileSync( + path.join(session.worktreePath, "a.js"), + "export const a = 'DISCARDED';\n" + ); + await cleanupWorktreeSession(session, { keep: false }); + assert.equal(fs.existsSync(session.worktreePath), false); + const branches = await runCommand("git", ["branch"], { cwd: repo }); + assert.doesNotMatch(branches.stdout, new RegExp(session.branch)); + // Main tree untouched. + const staged = await runCommand("git", ["diff"], { cwd: repo }); + assert.doesNotMatch(staged.stdout, /DISCARDED/); + }); + + it("cleanupWorktreeSession with no changes is a clean no-op", async () => { + const session = await createWorktreeSession(repo); + const result = await cleanupWorktreeSession(session, { keep: true }); + assert.equal(result.applied, false); + assert.equal(result.detail, "No changes to apply."); + assert.equal(fs.existsSync(session.worktreePath), false); + }); + + it("refuses to create a worktree while a merge is in progress", async () => { + fs.writeFileSync(path.join(repo, ".git", "MERGE_HEAD"), "deadbeef\n"); + await assert.rejects( + createWorktreeSession(repo), + /in-progress merge/i + ); + }); + + it("preserves the patch file and worktree when keep apply fails", async () => { + const session = await createWorktreeSession(repo); + try { + fs.writeFileSync( + path.join(session.worktreePath, "a.js"), + "export const a = 'WORKTREE';\n" + ); + fs.writeFileSync(path.join(repo, "a.js"), "export const a = 'MAIN';\n"); + + const result = await cleanupWorktreeSession(session, { keep: true }); + assert.equal(result.applied, false); + assert.match(result.detail, /Preserved patch:/); + assert.match(result.detail, /Hint:/); + assert.equal(fs.existsSync(session.worktreePath), true); + + const patchPath = result.detail.match(/Preserved patch: (.+)/)?.[1]?.trim(); + assert.ok(patchPath, "detail did not include a preserved patch path"); + assert.equal(fs.existsSync(patchPath), true); + fs.rmSync(patchPath, { force: true }); + } finally { + await cleanupWorktreeSession(session, { keep: false }).catch(() => {}); + } + }); +});