diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 585b3f5..a929370 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -6,13 +6,22 @@ on: branches: - main +permissions: + contents: read + jobs: test: runs-on: ubuntu-latest + name: Node ${{ matrix.node }} + strategy: + fail-fast: false + matrix: + node: [20, 22, 24] steps: - - uses: actions/checkout@v4 - - uses: actions/setup-node@v4 + - uses: actions/checkout@v7 + - uses: actions/setup-node@v7 with: - node-version: "20" + node-version: ${{ matrix.node }} + package-manager-cache: false - run: npm test - run: node tests/validate-plugin.mjs diff --git a/.node-version b/.node-version new file mode 100644 index 0000000..a45fd52 --- /dev/null +++ b/.node-version @@ -0,0 +1 @@ +24 diff --git a/README.md b/README.md index 9d8402d..b60ee31 100644 --- a/README.md +++ b/README.md @@ -244,6 +244,26 @@ so `$claude status`, `$claude result`, and `$claude cancel` can work across turns. It does not intentionally collect analytics, phone home, or send data to the repository owner. +## State Storage + +By default the companion stores state under: + +```text +~/.codex/claude-plugin-codex +``` + +Set `CLAUDE_COMPANION_STATE_ROOT` to use another local directory: + +```bash +CLAUDE_COMPANION_STATE_ROOT=/path/to/writable/state \ + node plugins/claude-code-advisor/scripts/claude-companion.mjs setup --json +``` + +This is useful in sandboxed Codex environments where the default Codex home +path is readable but not writable. The state root should be local, private, and +excluded from version control because it can contain job prompts, Claude output, +workspace paths, and review results. + ## Terms This project is provided under the MIT License. You are responsible for how you @@ -269,6 +289,9 @@ The companion owns: ## Development +Use Node.js 24 for development. The repository includes `.node-version` for +compatible version managers, and CI also checks Node.js 20 and 22 compatibility. + ```bash npm test npm run validate @@ -290,7 +313,10 @@ npm run test:e2e:codex This requires `codex plugin marketplace add ./`, `Claude` installed from Codex's plugin directory, and a logged-in Claude Code CLI. It starts a fresh `codex exec` session and verifies that `$claude advise --model sonnet` routes -through the installed skill. Sonnet is used only for this small routing test. +through the installed skill. The test uses Codex's `workspace-write` sandbox, +supplies a private temporary companion state root inside the checkout, and +removes that state before checking the worktree. Sonnet is used only for this +small routing test. ## Current Limits @@ -309,12 +335,17 @@ through the installed skill. Sonnet is used only for this small routing test. - Foreground prepared task routes use a larger default turn budget than structured review. If Claude reports that it hit the max-turn limit, rerun with `--max-turns ` or narrow the task. +- Working-tree structured reviews stop when untracked files exist because their + contents are absent from a Git diff and review mode cannot read the workspace. + Stage the intended files before rerunning the review. - `$claude monitor` checks a background job every 30 seconds by default. It - reads `claude logs` and `claude agents`, filters routine terminal noise, and - marks repeated output as stale after two minutes. -- Structured review depends on Claude returning valid JSON inside the - `--output-format json` result envelope. The companion validates and retries - once before failing. + reads `claude logs` and `claude agents --json --all`, filters routine terminal + noise, and marks repeated output as stale after two minutes. +- Structured review extracts a single complete JSON object from Claude's + `--output-format json` result envelope, tolerating leading status prose or + tool-call markup while rejecting ambiguous multiple objects. The extracted + review payload is still validated strictly, and the companion retries once + before failing. ## Troubleshooting @@ -323,6 +354,13 @@ or start a new thread. Codex may still point at an older cached skill path after a plugin version bump. If the error remains, remove and reinstall the `claude-plugin-codex` marketplace. +If `$claude setup` or a companion command fails with a write permission error +under `~/.codex/claude-plugin-codex`, rerun it with `CLAUDE_COMPANION_STATE_ROOT` +pointing at a writable directory. Within that root, the companion restricts its +workspace and thread directories to mode `0700` and state/pointer files to mode +`0600`. Do not point it at the project repository unless you also ignore that +path in Git. + ## License MIT. diff --git a/package.json b/package.json index ee34508..7bd4b4a 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "claude-plugin-codex", - "version": "0.1.12", + "version": "0.1.13", "description": "Bring local Claude Code into Codex for reviews, prepared tasks, advice, and rescue work.", "type": "module", "private": true, diff --git a/plugins/claude-code-advisor/.codex-plugin/plugin.json b/plugins/claude-code-advisor/.codex-plugin/plugin.json index 71b93e9..d494af3 100644 --- a/plugins/claude-code-advisor/.codex-plugin/plugin.json +++ b/plugins/claude-code-advisor/.codex-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "claude-code-advisor", - "version": "0.1.12", + "version": "0.1.13", "description": "Bring local Claude Code into Codex for reviews, prepared tasks, advice, and rescue work.", "author": { "name": "Yanchuk" @@ -33,9 +33,8 @@ "termsOfServiceURL": "https://github.com/yanchuk/claude-plugin-codex#terms", "defaultPrompt": [ "Check Claude Code setup.", - "Review this diff with Claude Code.", - "Do a prepared task with Claude Code.", - "Rescue this task with Claude Code." + "Review or adversarially review this diff with Claude Code.", + "Do a prepared task or rescue this work with Claude Code." ], "brandColor": "#D97706", "composerIcon": "./assets/icon.svg", diff --git a/plugins/claude-code-advisor/scripts/claude-companion.mjs b/plugins/claude-code-advisor/scripts/claude-companion.mjs index 490c951..120acf7 100755 --- a/plugins/claude-code-advisor/scripts/claude-companion.mjs +++ b/plugins/claude-code-advisor/scripts/claude-companion.mjs @@ -317,19 +317,36 @@ function assertBackgroundMcpSafe(ctx, options = {}) { function persistContext(ctx, state) { const saved = saveState(ctx.stateDir, state); - fs.mkdirSync(ctx.indexDir, { recursive: true }); - fs.writeFileSync(path.join(ctx.indexDir, "latest-state-dir"), `${ctx.stateDir}\n`, "utf8"); + fs.mkdirSync(ctx.indexDir, { recursive: true, mode: 0o700 }); + fs.chmodSync(ctx.indexDir, 0o700); + const latestStateFile = path.join(ctx.indexDir, "latest-state-dir"); + fs.writeFileSync(latestStateFile, `${ctx.stateDir}\n`, { encoding: "utf8", mode: 0o600 }); + fs.chmodSync(latestStateFile, 0o600); return saved; } function gitContext(cwd, options = {}) { const target = options.base ? `${options.base}...HEAD` : null; + if (!target) { + const untracked = spawnSync("git", ["ls-files", "--others", "--exclude-standard"], { cwd, encoding: "utf8" }); + const paths = untracked.status === 0 ? untracked.stdout.trim().split(/\r?\n/).filter(Boolean) : []; + if (paths.length) { + const shown = paths.slice(0, 20).map((file) => `- ${file}`).join("\n"); + const remaining = paths.length > 20 ? `\n- ...and ${paths.length - 20} more` : ""; + throw new Error( + `Working-tree review cannot safely include untracked file contents. Stage the intended files first:\n${shown}${remaining}` + ); + } + } const args = target ? ["diff", "--stat", target] : ["status", "--short", "--untracked-files=all"]; const result = spawnSync("git", args, { cwd, encoding: "utf8" }); const first = result.status === 0 ? result.stdout : result.stderr; - const diffArgs = target ? ["diff", "--", target] : ["diff", "--"]; - const diff = spawnSync("git", diffArgs, { cwd, encoding: "utf8", maxBuffer: 1024 * 1024 }); - return [first, diff.status === 0 ? diff.stdout : ""].join("\n").trim(); + const diffArgs = target ? ["diff", target, "--"] : ["diff", "HEAD", "--"]; + let diff = spawnSync("git", diffArgs, { cwd, encoding: "utf8", maxBuffer: 1024 * 1024 }); + if (!target && diff.status !== 0) { + diff = spawnSync("git", ["diff", "--"], { cwd, encoding: "utf8", maxBuffer: 1024 * 1024 }); + } + return [first, diff.status === 0 ? diff.stdout : diff.stderr].join("\n").trim(); } function detectCapabilities() { @@ -569,6 +586,34 @@ function findJob(ctx, reference) { return ctx.state.jobs.find((job) => job.id === reference || job.claudeSessionId === reference) || null; } +function parseAgentsJson(output, claudeSessionId) { + const text = String(output || "").trim(); + if (!text) { + return { output: "" }; + } + try { + const parsed = JSON.parse(text); + const sessions = Array.isArray(parsed) ? parsed : parsed.sessions || parsed.agents || []; + const match = sessions.find((session) => { + return ( + session?.id === claudeSessionId || + session?.sessionId === claudeSessionId || + session?.session_id === claudeSessionId + ); + }); + const lifecycle = `${match?.status || ""} ${match?.state || ""}`.toLowerCase(); + return { + output: text, + sessions, + match: match || null, + active: /\b(active|running|busy|working)\b/.test(lifecycle), + completed: /\b(done|completed|complete|stopped|exited|finished)\b/.test(lifecycle) + }; + } catch { + return { output: text }; + } +} + function readLiveStatus(job, options = {}) { if (!job?.claudeSessionId) { return { @@ -581,16 +626,17 @@ function readLiveStatus(job, options = {}) { } const timeoutMs = Number(options["timeout-ms"] || 10000); const logs = runClaude(["logs", job.claudeSessionId], { timeoutMs }); - const agents = runClaude(["agents"], { timeoutMs }); + const agents = runClaude(["agents", "--json", "--all"], { timeoutMs }); const agentsOutput = stripTerminalControl(`${agents.stdout || ""}${agents.stderr || ""}`); const logsOutput = stripTerminalControl(`${logs.stdout || ""}${logs.stderr || ""}`); + const agentStatus = agents.status === 0 ? parseAgentsJson(agentsOutput, job.claudeSessionId) : { output: agentsOutput }; const meaningfulLogLines = extractMeaningfulLogLines(logsOutput); - const completed = isCompletedLogOutput(logsOutput); + const completed = isCompletedLogOutput(logsOutput) || Boolean(agentStatus.completed); return { checkedAt: new Date().toISOString(), jobId: job.id, claudeSessionId: job.claudeSessionId, - active: logs.status === 0 && !completed, + active: !completed && (logs.status === 0 || Boolean(agentStatus.active)), completed, available: logs.status === 0 || agents.status === 0, logs: { @@ -600,7 +646,8 @@ function readLiveStatus(job, options = {}) { }, agents: { available: agents.status === 0, - output: agentsOutput.trim() + output: agentStatus.output || agentsOutput.trim(), + match: agentStatus.match || null } }; } @@ -771,10 +818,17 @@ function handleCancel(argv) { try { const live = summarizeLiveStatus(readLiveStatus(job, options), {}, options); latest = persistMonitorSnapshot(ctx, job, live); + if (live.completed) { + output({ jobId: latest.id, status: "completed" }, options.json); + return; + } } catch { latest = job; } - runClaude(["stop", job.claudeSessionId], { timeoutMs: Number(options["timeout-ms"] || 10000) }); + const stopped = runClaude(["stop", job.claudeSessionId], { timeoutMs: Number(options["timeout-ms"] || 10000) }); + if (stopped.status !== 0) { + throw new Error(stopped.stderr || stopped.stdout || `Claude failed to stop session ${job.claudeSessionId}.`); + } } const cancelled = completeJob(ctx, latest, { status: "cancelled" }); output({ jobId: cancelled.id, status: "cancelled" }, options.json); diff --git a/plugins/claude-code-advisor/scripts/lib/runtime.mjs b/plugins/claude-code-advisor/scripts/lib/runtime.mjs index daf9be4..3d58b92 100644 --- a/plugins/claude-code-advisor/scripts/lib/runtime.mjs +++ b/plugins/claude-code-advisor/scripts/lib/runtime.mjs @@ -83,13 +83,16 @@ export function loadState(stateDir) { } export function saveState(stateDir, state) { - fs.mkdirSync(stateDir, { recursive: true }); + fs.mkdirSync(stateDir, { recursive: true, mode: 0o700 }); + fs.chmodSync(stateDir, 0o700); const next = { ...emptyState(), ...state, jobs: [...(state.jobs || [])].sort((a, b) => String(b.updatedAt || "").localeCompare(String(a.updatedAt || ""))) }; - fs.writeFileSync(path.join(stateDir, "state.json"), `${JSON.stringify(next, null, 2)}\n`, "utf8"); + const stateFile = path.join(stateDir, "state.json"); + fs.writeFileSync(stateFile, `${JSON.stringify(next, null, 2)}\n`, { encoding: "utf8", mode: 0o600 }); + fs.chmodSync(stateFile, 0o600); return next; } @@ -267,8 +270,8 @@ export function parseClaudeJsonResult(raw) { } const envelope = JSON.parse(text); const sessionId = envelope.session_id || envelope.sessionId || null; - const contentRaw = typeof envelope.result === "string" ? envelope.result : text; - const content = typeof envelope.result === "string" ? JSON.parse(envelope.result) : envelope; + const contentRaw = typeof envelope.result === "string" ? normalizeClaudeResult(envelope.result) : text; + const content = typeof envelope.result === "string" ? JSON.parse(contentRaw) : envelope; return { envelope, content, @@ -277,6 +280,62 @@ export function parseClaudeJsonResult(raw) { }; } +function normalizeClaudeResult(raw) { + const trimmed = String(raw || "").trim(); + const toolCalls = trimmed.match(/^[\s\S]*?<\/function_calls>\s*/); + const withoutToolCalls = toolCalls ? trimmed.slice(toolCalls[0].length).trim() : trimmed; + try { + JSON.parse(withoutToolCalls); + return withoutToolCalls; + } catch { + const candidates = extractJsonObjects(withoutToolCalls); + if (candidates.length > 1) { + throw new Error("Ambiguous JSON Claude result: multiple complete objects were returned."); + } + return candidates[0] || withoutToolCalls; + } +} + +function extractJsonObjects(value) { + const text = String(value || ""); + const objects = []; + let start = -1; + let depth = 0; + let inString = false; + let escaped = false; + for (let index = 0; index < text.length; index += 1) { + const char = text[index]; + if (escaped) { + escaped = false; + continue; + } + if (inString && char === "\\") { + escaped = true; + continue; + } + if (char === '"') { + inString = !inString; + continue; + } + if (inString) { + continue; + } + if (char === "{") { + if (depth === 0) { + start = index; + } + depth += 1; + } else if (char === "}" && depth > 0) { + depth -= 1; + if (depth === 0) { + objects.push(text.slice(start, index + 1).trim()); + start = -1; + } + } + } + return objects; +} + export function buildReviewPrompt({ kind, targetLabel, gitContext, focus = "" }) { const reviewKind = kind === "adversarial-review" ? "adversarial reviewer" : "code reviewer"; const focusLine = focus ? `Focus: ${focus}\n` : ""; diff --git a/tests/commands.test.mjs b/tests/commands.test.mjs index 8b0abed..a345e39 100644 --- a/tests/commands.test.mjs +++ b/tests/commands.test.mjs @@ -3,9 +3,12 @@ import fs from "node:fs"; import os from "node:os"; import path from "node:path"; import { execFileSync, spawnSync } from "node:child_process"; +import { fileURLToPath } from "node:url"; import test from "node:test"; -const companion = new URL("../plugins/claude-code-advisor/scripts/claude-companion.mjs", import.meta.url); +const companion = fileURLToPath( + new URL("../plugins/claude-code-advisor/scripts/claude-companion.mjs", import.meta.url) +); function makeFakeClaude(scriptBody) { const dir = fs.mkdtempSync(path.join(os.tmpdir(), "fake-claude-")); @@ -44,7 +47,8 @@ if (args.includes("-p")) { console.log("{}"); process.exit(0); } console.error("unsupported"); process.exit(2); `); const stateRoot = fs.mkdtempSync(path.join(os.tmpdir(), "claude-state-")); - const stdout = execFileSync(process.execPath, [companion.pathname, "setup", "--json"], { + fs.chmodSync(stateRoot, 0o755); + const stdout = execFileSync(process.execPath, [companion, "setup", "--json"], { env: { ...process.env, PATH: `${fake.dir}:${process.env.PATH}`, CLAUDE_COMPANION_STATE_ROOT: stateRoot }, cwd: stateRoot, encoding: "utf8" @@ -54,6 +58,16 @@ console.error("unsupported"); process.exit(2); assert.equal(payload.ready, true); assert.equal(payload.capabilities.print, true); assert.equal(payload.capabilities.background, false); + assert.equal(fs.statSync(stateRoot).mode & 0o777, 0o755); + const workspaceIndex = fs.readdirSync(stateRoot).find((entry) => entry.startsWith("claude-state-")); + assert.ok(workspaceIndex); + const indexDir = path.join(stateRoot, workspaceIndex); + const latestStateFile = path.join(indexDir, "latest-state-dir"); + const stateDir = fs.readFileSync(latestStateFile, "utf8").trim(); + assert.equal(fs.statSync(indexDir).mode & 0o777, 0o700); + assert.equal(fs.statSync(stateDir).mode & 0o777, 0o700); + assert.equal(fs.statSync(path.join(stateDir, "state.json")).mode & 0o777, 0o600); + assert.equal(fs.statSync(latestStateFile).mode & 0o777, 0o600); }); test("review returns validated JSON and stores result", () => { @@ -73,7 +87,7 @@ console.error("unsupported"); process.exit(2); CLAUDE_COMPANION_STATE_ROOT: stateRoot, CODEX_THREAD_ID: "thread-a" }; - const stdout = execFileSync(process.execPath, [companion.pathname, "review", "--json"], { + const stdout = execFileSync(process.execPath, [companion, "review", "--json"], { env, cwd: stateRoot, encoding: "utf8" @@ -83,7 +97,7 @@ console.error("unsupported"); process.exit(2); assert.equal(payload.status, "completed"); assert.equal(payload.result.findings[0].severity, "MAJOR"); - const result = execFileSync(process.execPath, [companion.pathname, "result", payload.jobId, "--json"], { + const result = execFileSync(process.execPath, [companion, "result", payload.jobId, "--json"], { env, cwd: stateRoot, encoding: "utf8" @@ -91,6 +105,98 @@ console.error("unsupported"); process.exit(2); assert.equal(JSON.parse(result).job.id, payload.jobId); }); +test("review base range includes the patch in the Claude prompt", () => { + const promptLog = path.join(os.tmpdir(), `fake-review-base-${Date.now()}.log`); + const fake = makeFakeClaude(` +const fs = require("node:fs"); +const args = process.argv.slice(2); +if (args.includes("-p")) { + fs.writeFileSync(${JSON.stringify(promptLog)}, args[args.indexOf("-p") + 1]); + console.log(JSON.stringify({findings:[]})); + process.exit(0); +} +console.error("unsupported"); process.exit(2); +`); + const repo = fs.mkdtempSync(path.join(os.tmpdir(), "claude-review-base-")); + execFileSync("git", ["init", "-q"], { cwd: repo }); + execFileSync("git", ["config", "user.name", "Test"], { cwd: repo }); + execFileSync("git", ["config", "user.email", "test@example.invalid"], { cwd: repo }); + fs.writeFileSync(path.join(repo, "sample.txt"), "before\n", "utf8"); + execFileSync("git", ["add", "sample.txt"], { cwd: repo }); + execFileSync("git", ["commit", "-qm", "base"], { cwd: repo }); + const base = execFileSync("git", ["rev-parse", "HEAD"], { cwd: repo, encoding: "utf8" }).trim(); + fs.writeFileSync(path.join(repo, "sample.txt"), "after-base-range\n", "utf8"); + execFileSync("git", ["add", "sample.txt"], { cwd: repo }); + execFileSync("git", ["commit", "-qm", "change"], { cwd: repo }); + const stateRoot = fs.mkdtempSync(path.join(os.tmpdir(), "claude-state-")); + + execFileSync(process.execPath, [companion, "review", "--base", base, "--json"], { + env: { ...process.env, PATH: `${fake.dir}:${process.env.PATH}`, CLAUDE_COMPANION_STATE_ROOT: stateRoot }, + cwd: repo, + encoding: "utf8" + }); + + assert.match(fs.readFileSync(promptLog, "utf8"), /after-base-range/); +}); + +test("working-tree review includes staged patch content", () => { + const promptLog = path.join(os.tmpdir(), `fake-review-staged-${Date.now()}.log`); + const fake = makeFakeClaude(` +const fs = require("node:fs"); +const args = process.argv.slice(2); +if (args.includes("-p")) { + fs.writeFileSync(${JSON.stringify(promptLog)}, args[args.indexOf("-p") + 1]); + console.log(JSON.stringify({findings:[]})); + process.exit(0); +} +console.error("unsupported"); process.exit(2); +`); + const repo = fs.mkdtempSync(path.join(os.tmpdir(), "claude-review-staged-")); + execFileSync("git", ["init", "-q"], { cwd: repo }); + execFileSync("git", ["config", "user.name", "Test"], { cwd: repo }); + execFileSync("git", ["config", "user.email", "test@example.invalid"], { cwd: repo }); + fs.writeFileSync(path.join(repo, "sample.txt"), "before\n", "utf8"); + execFileSync("git", ["add", "sample.txt"], { cwd: repo }); + execFileSync("git", ["commit", "-qm", "base"], { cwd: repo }); + fs.writeFileSync(path.join(repo, "sample.txt"), "after-staging\n", "utf8"); + execFileSync("git", ["add", "sample.txt"], { cwd: repo }); + const stateRoot = fs.mkdtempSync(path.join(os.tmpdir(), "claude-state-")); + + execFileSync(process.execPath, [companion, "review", "--json"], { + env: { ...process.env, PATH: `${fake.dir}:${process.env.PATH}`, CLAUDE_COMPANION_STATE_ROOT: stateRoot }, + cwd: repo, + encoding: "utf8" + }); + + assert.match(fs.readFileSync(promptLog, "utf8"), /after-staging/); +}); + +test("working-tree review refuses untracked files whose contents would be omitted", () => { + const fake = makeFakeClaude(` +console.error("Claude should not run when untracked files are present"); +process.exit(2); +`); + const repo = fs.mkdtempSync(path.join(os.tmpdir(), "claude-review-untracked-")); + execFileSync("git", ["init", "-q"], { cwd: repo }); + execFileSync("git", ["config", "user.name", "Test"], { cwd: repo }); + execFileSync("git", ["config", "user.email", "test@example.invalid"], { cwd: repo }); + fs.writeFileSync(path.join(repo, "tracked.txt"), "tracked\n", "utf8"); + execFileSync("git", ["add", "tracked.txt"], { cwd: repo }); + execFileSync("git", ["commit", "-qm", "base"], { cwd: repo }); + fs.writeFileSync(path.join(repo, "new-source.mjs"), "export const value = 1;\n", "utf8"); + const stateRoot = fs.mkdtempSync(path.join(os.tmpdir(), "claude-state-")); + + const reviewed = spawnSync(process.execPath, [companion, "review", "--json"], { + env: { ...process.env, PATH: `${fake.dir}:${process.env.PATH}`, CLAUDE_COMPANION_STATE_ROOT: stateRoot }, + cwd: repo, + encoding: "utf8" + }); + + assert.notEqual(reviewed.status, 0); + assert.match(reviewed.stderr, /Stage the intended files first/); + assert.match(reviewed.stderr, /new-source\.mjs/); +}); + test("background advise stores Claude session id and cancel calls claude stop", () => { const stopLog = path.join(os.tmpdir(), `fake-stop-${Date.now()}.log`); const fake = makeFakeClaude(` @@ -105,7 +211,7 @@ console.error("unsupported"); process.exit(2); `); const stateRoot = fs.mkdtempSync(path.join(os.tmpdir(), "claude-state-")); const env = { ...process.env, PATH: `${fake.dir}:${process.env.PATH}`, CLAUDE_COMPANION_STATE_ROOT: stateRoot }; - const stdout = execFileSync(process.execPath, [companion.pathname, "advise", "--background", "check architecture", "--json"], { + const stdout = execFileSync(process.execPath, [companion, "advise", "--background", "check architecture", "--json"], { env, cwd: stateRoot, encoding: "utf8" @@ -115,7 +221,7 @@ console.error("unsupported"); process.exit(2); assert.equal(payload.status, "running"); assert.equal(payload.claudeSessionId, "bg123"); - const cancel = execFileSync(process.execPath, [companion.pathname, "cancel", payload.jobId, "--json"], { + const cancel = execFileSync(process.execPath, [companion, "cancel", payload.jobId, "--json"], { env, cwd: stateRoot, encoding: "utf8" @@ -123,7 +229,7 @@ console.error("unsupported"); process.exit(2); assert.equal(JSON.parse(cancel).status, "cancelled"); assert.equal(fs.readFileSync(stopLog, "utf8"), "bg123"); - const result = execFileSync(process.execPath, [companion.pathname, "result", payload.jobId, "--json"], { + const result = execFileSync(process.execPath, [companion, "result", payload.jobId, "--json"], { env, cwd: stateRoot, encoding: "utf8" @@ -134,6 +240,40 @@ console.error("unsupported"); process.exit(2); assert.equal(stored.job.lastMeaningfulOutput, "latest output"); }); +test("cancel fails without persisting cancellation when Claude stop fails", () => { + const fake = makeFakeClaude(` +const args = process.argv.slice(2); +if (args.includes("--version")) { console.log("2.1.132 (Claude Code)"); process.exit(0); } +if (args[0] === "--bg") { console.log("backgrounded · bg-stop-fails (idle - send a prompt to start)"); process.exit(0); } +if (args[0] === "logs") { console.log("still working"); process.exit(0); } +if (args[0] === "agents") { console.log(JSON.stringify([{ id: "bg-stop-fails", status: "running" }])); process.exit(0); } +if (args[0] === "stop") { console.error("stop failed"); process.exit(2); } +console.error("unsupported"); process.exit(2); +`); + const stateRoot = fs.mkdtempSync(path.join(os.tmpdir(), "claude-state-")); + const env = { ...process.env, PATH: `${fake.dir}:${process.env.PATH}`, CLAUDE_COMPANION_STATE_ROOT: stateRoot }; + const launched = JSON.parse(execFileSync( + process.execPath, + [companion, "advise", "--background", "check architecture", "--json"], + { env, cwd: stateRoot, encoding: "utf8" } + )); + + const cancelled = spawnSync( + process.execPath, + [companion, "cancel", launched.jobId, "--json"], + { env, cwd: stateRoot, encoding: "utf8" } + ); + + assert.notEqual(cancelled.status, 0); + assert.match(cancelled.stderr, /stop failed/); + const stored = JSON.parse(execFileSync( + process.execPath, + [companion, "result", launched.jobId, "--json"], + { env, cwd: stateRoot, encoding: "utf8" } + )); + assert.equal(stored.job.status, "running"); +}); + test("review defaults to a single Claude turn", () => { const fake = makeFakeClaude(` const args = process.argv.slice(2); @@ -149,7 +289,7 @@ if (args.includes("-p")) { console.error("unsupported"); process.exit(2); `); const stateRoot = fs.mkdtempSync(path.join(os.tmpdir(), "claude-state-")); - const stdout = execFileSync(process.execPath, [companion.pathname, "review", "--json"], { + const stdout = execFileSync(process.execPath, [companion, "review", "--json"], { env: { ...process.env, PATH: `${fake.dir}:${process.env.PATH}`, CLAUDE_COMPANION_STATE_ROOT: stateRoot }, cwd: stateRoot, encoding: "utf8" @@ -170,7 +310,7 @@ console.error("unsupported"); process.exit(2); `); const stateRoot = fs.mkdtempSync(path.join(os.tmpdir(), "claude-state-")); const env = { ...process.env, PATH: `${fake.dir}:${process.env.PATH}`, CLAUDE_COMPANION_STATE_ROOT: stateRoot }; - const launched = execFileSync(process.execPath, [companion.pathname, "advise", "--background", "check architecture", "--json"], { + const launched = execFileSync(process.execPath, [companion, "advise", "--background", "check architecture", "--json"], { env, cwd: stateRoot, encoding: "utf8" @@ -178,7 +318,7 @@ console.error("unsupported"); process.exit(2); const job = JSON.parse(launched); const watched = execFileSync( process.execPath, - [companion.pathname, "monitor", job.jobId, "--interval-ms", "1", "--max-checks", "2", "--json"], + [companion, "monitor", job.jobId, "--interval-ms", "1", "--max-checks", "2", "--json"], { env, cwd: stateRoot, encoding: "utf8" } ); const snapshots = watched.trim().split(/\r?\n/).map((line) => JSON.parse(line)); @@ -187,7 +327,7 @@ console.error("unsupported"); process.exit(2); assert.equal(snapshots[0].active, true); assert.equal(snapshots[0].logs.output, "progress: still working"); - const result = execFileSync(process.execPath, [companion.pathname, "result", job.jobId, "--json"], { + const result = execFileSync(process.execPath, [companion, "result", job.jobId, "--json"], { env, cwd: stateRoot, encoding: "utf8" @@ -197,6 +337,48 @@ console.error("unsupported"); process.exit(2); assert.equal(stored.job.lastMonitorSnapshot.summary.lastMeaningfulLine, "progress: still working"); }); +test("monitor uses agents json state to mark a background job completed", () => { + const fake = makeFakeClaude(` +const args = process.argv.slice(2); +if (args.includes("--version")) { console.log("2.1.132 (Claude Code)"); process.exit(0); } +if (args[0] === "--bg") { console.log("backgrounded · bg123 (idle - send a prompt to start)"); process.exit(0); } +if (args[0] === "logs") { console.log("final answer available"); process.exit(0); } +if (JSON.stringify(args) === JSON.stringify(["agents", "--json", "--all"])) { + console.log(JSON.stringify([{ id: "bg123", status: "idle", state: "done" }])); + process.exit(0); +} +if (args[0] === "agents") { console.error("agents must be read with --json"); process.exit(2); } +console.error("unsupported"); process.exit(2); +`); + const stateRoot = fs.mkdtempSync(path.join(os.tmpdir(), "claude-state-")); + const env = { ...process.env, PATH: `${fake.dir}:${process.env.PATH}`, CLAUDE_COMPANION_STATE_ROOT: stateRoot }; + const launched = execFileSync(process.execPath, [companion, "advise", "--background", "check architecture", "--json"], { + env, + cwd: stateRoot, + encoding: "utf8" + }); + const job = JSON.parse(launched); + const watched = execFileSync(process.execPath, [companion, "monitor", job.jobId, "--json"], { + env, + cwd: stateRoot, + encoding: "utf8" + }); + const snapshot = JSON.parse(watched.trim()); + + assert.equal(snapshot.active, false); + assert.equal(snapshot.completed, true); + assert.equal(snapshot.agents.match.state, "done"); + + const result = execFileSync(process.execPath, [companion, "result", job.jobId, "--json"], { + env, + cwd: stateRoot, + encoding: "utf8" + }); + const stored = JSON.parse(result); + assert.equal(stored.job.status, "completed"); + assert.equal(stored.result, "final answer available"); +}); + test("monitor summarizes meaningful progress and stale repeated logs", () => { const fake = makeFakeClaude(` const args = process.argv.slice(2); @@ -215,7 +397,7 @@ console.error("unsupported"); process.exit(2); `); const stateRoot = fs.mkdtempSync(path.join(os.tmpdir(), "claude-state-")); const env = { ...process.env, PATH: `${fake.dir}:${process.env.PATH}`, CLAUDE_COMPANION_STATE_ROOT: stateRoot }; - const launched = execFileSync(process.execPath, [companion.pathname, "advise", "--background", "check architecture", "--json"], { + const launched = execFileSync(process.execPath, [companion, "advise", "--background", "check architecture", "--json"], { env, cwd: stateRoot, encoding: "utf8" @@ -224,7 +406,7 @@ console.error("unsupported"); process.exit(2); const watched = execFileSync( process.execPath, [ - companion.pathname, + companion, "monitor", job.jobId, "--interval-ms", @@ -248,7 +430,7 @@ console.error("unsupported"); process.exit(2); const human = execFileSync( process.execPath, - [companion.pathname, "monitor", job.jobId, "--interval-ms", "1", "--max-checks", "1"], + [companion, "monitor", job.jobId, "--interval-ms", "1", "--max-checks", "1"], { env, cwd: stateRoot, encoding: "utf8" } ); assert.match(human, /Last meaningful output: progress: compiling tests/); @@ -280,13 +462,13 @@ console.error("unsupported"); process.exit(2); `); const stateRoot = fs.mkdtempSync(path.join(os.tmpdir(), "claude-state-")); const env = { ...process.env, PATH: `${fake.dir}:${process.env.PATH}`, CLAUDE_COMPANION_STATE_ROOT: stateRoot }; - const launched = execFileSync(process.execPath, [companion.pathname, "advise", "--background", "check architecture", "--json"], { + const launched = execFileSync(process.execPath, [companion, "advise", "--background", "check architecture", "--json"], { env, cwd: stateRoot, encoding: "utf8" }); const job = JSON.parse(launched); - const watched = execFileSync(process.execPath, [companion.pathname, "monitor", job.jobId, "--json"], { + const watched = execFileSync(process.execPath, [companion, "monitor", job.jobId, "--json"], { env, cwd: stateRoot, encoding: "utf8" @@ -298,7 +480,7 @@ console.error("unsupported"); process.exit(2); assert.equal(snapshot.summary.state, "inactive"); assert.equal(snapshot.summary.lastMeaningfulLine, "PASS"); - const result = execFileSync(process.execPath, [companion.pathname, "result", job.jobId, "--json"], { + const result = execFileSync(process.execPath, [companion, "result", job.jobId, "--json"], { env, cwd: stateRoot, encoding: "utf8" @@ -311,7 +493,7 @@ console.error("unsupported"); process.exit(2); test("foreground advise defaults to a larger turn budget", () => { const fake = makeFakeClaudeExpectingMaxTurns(20); const stateRoot = fs.mkdtempSync(path.join(os.tmpdir(), "claude-state-")); - const stdout = execFileSync(process.execPath, [companion.pathname, "advise", "check architecture", "--json"], { + const stdout = execFileSync(process.execPath, [companion, "advise", "check architecture", "--json"], { env: { ...process.env, PATH: `${fake.dir}:${process.env.PATH}`, CLAUDE_COMPANION_STATE_ROOT: stateRoot }, cwd: stateRoot, encoding: "utf8" @@ -325,7 +507,7 @@ test("foreground advise defaults to a larger turn budget", () => { test("foreground do defaults to a larger turn budget", () => { const fake = makeFakeClaudeExpectingMaxTurns(20); const stateRoot = fs.mkdtempSync(path.join(os.tmpdir(), "claude-state-")); - const stdout = execFileSync(process.execPath, [companion.pathname, "do", "inspect local code", "--json"], { + const stdout = execFileSync(process.execPath, [companion, "do", "inspect local code", "--json"], { env: { ...process.env, PATH: `${fake.dir}:${process.env.PATH}`, CLAUDE_COMPANION_STATE_ROOT: stateRoot }, cwd: stateRoot, encoding: "utf8" @@ -339,7 +521,7 @@ test("foreground do defaults to a larger turn budget", () => { test("foreground rescue defaults to a larger turn budget", () => { const fake = makeFakeClaudeExpectingMaxTurns(20); const stateRoot = fs.mkdtempSync(path.join(os.tmpdir(), "claude-state-")); - const stdout = execFileSync(process.execPath, [companion.pathname, "rescue", "diagnose the failure", "--json"], { + const stdout = execFileSync(process.execPath, [companion, "rescue", "diagnose the failure", "--json"], { env: { ...process.env, PATH: `${fake.dir}:${process.env.PATH}`, CLAUDE_COMPANION_STATE_ROOT: stateRoot }, cwd: stateRoot, encoding: "utf8" @@ -355,7 +537,7 @@ test("foreground task max-turn override takes precedence over the default", () = const stateRoot = fs.mkdtempSync(path.join(os.tmpdir(), "claude-state-")); const stdout = execFileSync( process.execPath, - [companion.pathname, "do", "--max-turns", "5", "inspect local code", "--json"], + [companion, "do", "--max-turns", "5", "inspect local code", "--json"], { env: { ...process.env, PATH: `${fake.dir}:${process.env.PATH}`, CLAUDE_COMPANION_STATE_ROOT: stateRoot }, cwd: stateRoot, @@ -378,7 +560,7 @@ if (args.includes("-p")) { console.error("unsupported"); process.exit(2); `); const stateRoot = fs.mkdtempSync(path.join(os.tmpdir(), "claude-state-")); - const stdout = execFileSync(process.execPath, [companion.pathname, "do", "inspect local code", "--json"], { + const stdout = execFileSync(process.execPath, [companion, "do", "inspect local code", "--json"], { env: { ...process.env, PATH: `${fake.dir}:${process.env.PATH}`, CLAUDE_COMPANION_STATE_ROOT: stateRoot }, cwd: stateRoot, encoding: "utf8" @@ -405,7 +587,7 @@ if (args.includes("-p")) { console.error("unsupported"); process.exit(2); `); const stateRoot = fs.mkdtempSync(path.join(os.tmpdir(), "claude-state-")); - const stdout = execFileSync(process.execPath, [companion.pathname, "advise", "check architecture", "--json"], { + const stdout = execFileSync(process.execPath, [companion, "advise", "check architecture", "--json"], { env: { ...process.env, PATH: `${fake.dir}:${process.env.PATH}`, CLAUDE_COMPANION_STATE_ROOT: stateRoot }, cwd: stateRoot, encoding: "utf8" @@ -431,7 +613,7 @@ if (args[0] === "--bg") { console.error("unsupported"); process.exit(2); `); const stateRoot = fs.mkdtempSync(path.join(os.tmpdir(), "claude-state-")); - const stdout = execFileSync(process.execPath, [companion.pathname, "advise", "--background", "check architecture", "--json"], { + const stdout = execFileSync(process.execPath, [companion, "advise", "--background", "check architecture", "--json"], { env: { ...process.env, PATH: `${fake.dir}:${process.env.PATH}`, CLAUDE_COMPANION_STATE_ROOT: stateRoot }, cwd: stateRoot, encoding: "utf8" @@ -453,7 +635,7 @@ console.error("unsupported"); process.exit(2); `); const stateRoot = fs.mkdtempSync(path.join(os.tmpdir(), "claude-state-")); fs.writeFileSync(path.join(stateRoot, ".mcp.json"), '{"mcpServers":{"playwright":{}}}\n', "utf8"); - const result = spawnSync(process.execPath, [companion.pathname, "advise", "--background", "check architecture", "--json"], { + const result = spawnSync(process.execPath, [companion, "advise", "--background", "check architecture", "--json"], { env: { ...process.env, PATH: `${fake.dir}:${process.env.PATH}`, CLAUDE_COMPANION_STATE_ROOT: stateRoot }, cwd: stateRoot, encoding: "utf8" @@ -478,7 +660,7 @@ console.error("unsupported"); process.exit(2); fs.mkdirSync(child, { recursive: true }); fs.writeFileSync(path.join(parent, ".mcp.json"), '{"mcpServers":{"playwright":{}}}\n', "utf8"); fs.writeFileSync(path.join(child, ".git"), "gitdir: ../.git/worktrees/task\n", "utf8"); - const result = spawnSync(process.execPath, [companion.pathname, "advise", "--background", "check architecture", "--json"], { + const result = spawnSync(process.execPath, [companion, "advise", "--background", "check architecture", "--json"], { env: { ...process.env, PATH: `${fake.dir}:${process.env.PATH}`, CLAUDE_COMPANION_STATE_ROOT: parent }, cwd: child, encoding: "utf8" @@ -506,7 +688,7 @@ console.error("unsupported"); process.exit(2); fs.writeFileSync(path.join(stateRoot, ".mcp.json"), '{"mcpServers":{"playwright":{}}}\n', "utf8"); const stdout = execFileSync( process.execPath, - [companion.pathname, "advise", "--background", "--allow-mcp", "check architecture", "--json"], + [companion, "advise", "--background", "--allow-mcp", "check architecture", "--json"], { env: { ...process.env, PATH: `${fake.dir}:${process.env.PATH}`, CLAUDE_COMPANION_STATE_ROOT: stateRoot }, cwd: stateRoot, @@ -534,7 +716,7 @@ if (args.includes("-p")) { console.error("unsupported"); process.exit(2); `); const stateRoot = fs.mkdtempSync(path.join(os.tmpdir(), "claude-state-")); - const stdout = execFileSync(process.execPath, [companion.pathname, "do", "inspect local code", "--json"], { + const stdout = execFileSync(process.execPath, [companion, "do", "inspect local code", "--json"], { env: { ...process.env, PATH: `${fake.dir}:${process.env.PATH}`, CLAUDE_COMPANION_STATE_ROOT: stateRoot }, cwd: stateRoot, encoding: "utf8" @@ -553,15 +735,16 @@ if (args[0] === "--bg") { process.exit(0); } if (args.includes("-p")) { - setTimeout(() => {}, 5000); + setTimeout(() => process.exit(0), 5000); +} else { + console.error("unsupported"); process.exit(2); } -console.error("unsupported"); process.exit(2); `); const stateRoot = fs.mkdtempSync(path.join(os.tmpdir(), "claude-state-")); const env = { ...process.env, PATH: `${fake.dir}:${process.env.PATH}`, CLAUDE_COMPANION_STATE_ROOT: stateRoot }; const stdout = execFileSync( process.execPath, - [companion.pathname, "advise", "--timeout-ms", "50", "check architecture", "--json"], + [companion, "advise", "--timeout-ms", "50", "check architecture", "--json"], { env, cwd: stateRoot, encoding: "utf8" } ); const payload = JSON.parse(stdout); @@ -570,7 +753,7 @@ console.error("unsupported"); process.exit(2); assert.equal(payload.claudeSessionId, "bg123"); assert.match(payload.output, /Foreground Claude timed out/); - const status = execFileSync(process.execPath, [companion.pathname, "status", payload.jobId, "--json"], { + const status = execFileSync(process.execPath, [companion, "status", payload.jobId, "--json"], { env, cwd: stateRoot, encoding: "utf8" @@ -582,14 +765,15 @@ test("foreground advise can disable timeout background fallback", () => { const fake = makeFakeClaude(` const args = process.argv.slice(2); if (args.includes("-p")) { - setTimeout(() => {}, 5000); + setTimeout(() => process.exit(0), 5000); +} else { + console.error("unsupported"); process.exit(2); } -console.error("unsupported"); process.exit(2); `); const stateRoot = fs.mkdtempSync(path.join(os.tmpdir(), "claude-state-")); const result = spawnSync( process.execPath, - [companion.pathname, "advise", "--timeout-ms", "50", "--no-background-fallback", "slow"], + [companion, "advise", "--timeout-ms", "50", "--no-background-fallback", "slow"], { env: { ...process.env, PATH: `${fake.dir}:${process.env.PATH}`, CLAUDE_COMPANION_STATE_ROOT: stateRoot }, cwd: stateRoot, @@ -608,7 +792,7 @@ if (args.includes("-p")) { console.log("human answer"); process.exit(0); } console.error("unsupported"); process.exit(2); `); const stateRoot = fs.mkdtempSync(path.join(os.tmpdir(), "claude-state-")); - const stdout = execFileSync(process.execPath, [companion.pathname, "advise", "check architecture"], { + const stdout = execFileSync(process.execPath, [companion, "advise", "check architecture"], { env: { ...process.env, PATH: `${fake.dir}:${process.env.PATH}`, CLAUDE_COMPANION_STATE_ROOT: stateRoot }, cwd: stateRoot, encoding: "utf8" @@ -633,7 +817,7 @@ console.error("unsupported"); process.exit(2); `); const stateRoot = fs.mkdtempSync(path.join(os.tmpdir(), "claude-state-")); const env = { ...process.env, PATH: `${fake.dir}:${process.env.PATH}`, CLAUDE_COMPANION_STATE_ROOT: stateRoot }; - const stdout = execFileSync(process.execPath, [companion.pathname, "rescue", "--write", "fix the failing test", "--json"], { + const stdout = execFileSync(process.execPath, [companion, "rescue", "--write", "fix the failing test", "--json"], { env, cwd: stateRoot, encoding: "utf8" @@ -644,7 +828,7 @@ console.error("unsupported"); process.exit(2); assert.equal(payload.status, "completed"); assert.equal(payload.output, "rescued"); - const result = execFileSync(process.execPath, [companion.pathname, "result", payload.jobId, "--json"], { + const result = execFileSync(process.execPath, [companion, "result", payload.jobId, "--json"], { env, cwd: stateRoot, encoding: "utf8" @@ -668,7 +852,7 @@ console.error("unsupported"); process.exit(2); `); const stateRoot = fs.mkdtempSync(path.join(os.tmpdir(), "claude-state-")); const env = { ...process.env, PATH: `${fake.dir}:${process.env.PATH}`, CLAUDE_COMPANION_STATE_ROOT: stateRoot }; - const stdout = execFileSync(process.execPath, [companion.pathname, "rescue", "diagnose the failure", "--json"], { + const stdout = execFileSync(process.execPath, [companion, "rescue", "diagnose the failure", "--json"], { env, cwd: stateRoot, encoding: "utf8" @@ -701,7 +885,7 @@ console.error("unsupported"); process.exit(2); const env = { ...process.env, PATH: `${fake.dir}:${process.env.PATH}`, CLAUDE_COMPANION_STATE_ROOT: stateRoot }; const stdout = execFileSync( process.execPath, - [companion.pathname, "do", "--write", "--model", "sonnet", "implement the prepared task", "--json"], + [companion, "do", "--write", "--model", "sonnet", "implement the prepared task", "--json"], { env, cwd: stateRoot, encoding: "utf8" } ); const payload = JSON.parse(stdout); @@ -716,7 +900,7 @@ test("foreground timeout fails the job without hanging", () => { setTimeout(() => {}, 5000); `); const stateRoot = fs.mkdtempSync(path.join(os.tmpdir(), "claude-state-")); - const result = spawnSync(process.execPath, [companion.pathname, "advise", "--timeout-ms", "50", "--no-background-fallback", "slow"], { + const result = spawnSync(process.execPath, [companion, "advise", "--timeout-ms", "50", "--no-background-fallback", "slow"], { env: { ...process.env, PATH: `${fake.dir}:${process.env.PATH}`, CLAUDE_COMPANION_STATE_ROOT: stateRoot }, cwd: stateRoot, encoding: "utf8" diff --git a/tests/e2e-codex-skill.mjs b/tests/e2e-codex-skill.mjs index 83b0417..24ce330 100644 --- a/tests/e2e-codex-skill.mjs +++ b/tests/e2e-codex-skill.mjs @@ -1,7 +1,11 @@ import assert from "node:assert/strict"; import { spawnSync } from "node:child_process"; +import fs from "node:fs"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; -const repoRoot = new URL("..", import.meta.url).pathname; +const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), ".."); +assert.ok(fs.existsSync(path.join(repoRoot, "package.json")), "E2E repository root is invalid."); const skillMarker = "claude-code-advisor:claude"; const advisePrompt = [ "Use $claude advise --model sonnet --max-turns 1 --timeout-ms 120000 to ask Claude Code to reply with exactly PASS.", @@ -13,6 +17,7 @@ function run(command, args, options = {}) { const result = spawnSync(command, args, { cwd: repoRoot, encoding: "utf8", + maxBuffer: 16 * 1024 * 1024, timeout: 180_000, ...options }); @@ -47,7 +52,16 @@ assert.match( ); const statusBefore = run("git", ["status", "--short"]); -const execOutput = run("codex", ["exec", "--cd", repoRoot, "--json", advisePrompt], { input: "" }); +const stateRoot = fs.mkdtempSync(path.join(repoRoot, ".claude-plugin-codex-e2e-state-")); +let execOutput; +try { + execOutput = run("codex", ["exec", "--sandbox", "workspace-write", "--cd", repoRoot, "--json", advisePrompt], { + input: "", + env: { ...process.env, CLAUDE_COMPANION_STATE_ROOT: stateRoot } + }); +} finally { + fs.rmSync(stateRoot, { recursive: true, force: true }); +} const events = parseJsonLines(execOutput); const adviseCommand = events.find((event) => { return event.type === "item.completed" diff --git a/tests/runtime.test.mjs b/tests/runtime.test.mjs index 45e6b8f..4f130ff 100644 --- a/tests/runtime.test.mjs +++ b/tests/runtime.test.mjs @@ -12,6 +12,7 @@ import { parseBackgroundLaunch, parseClaudeJsonResult, resolveStateDir, + saveState, selectResumeCandidate, validateReviewPayload } from "../plugins/claude-code-advisor/scripts/lib/runtime.mjs"; @@ -213,6 +214,117 @@ test("parseClaudeJsonResult unwraps Claude CLI json envelope", () => { assert.equal(parsed.content.findings[0].severity, "MINOR"); }); +test("parseClaudeJsonResult tolerates Claude tool-call markup before review JSON", () => { + const payload = { + findings: [ + { + severity: "MINOR", + title: "Markup", + fact: "Claude prefixed the JSON with tool-call markup", + recommendation: "Strip the tool-call block before review validation" + } + ] + }; + const raw = JSON.stringify({ + type: "result", + subtype: "success", + result: [ + "", + '', + 'git log main...HEAD --oneline', + "", + "", + "", + JSON.stringify(payload) + ].join("\n"), + session_id: "session-456" + }); + + const parsed = parseClaudeJsonResult(raw); + assert.equal(parsed.sessionId, "session-456"); + assert.deepEqual(validateReviewPayload(parsed.contentRaw), payload); + assert.equal(parsed.content.findings[0].title, "Markup"); +}); + +test("parseClaudeJsonResult tolerates Claude prose before review JSON", () => { + const payload = { + findings: [ + { + severity: "MINOR", + title: "Prose", + fact: "Claude prefixed the JSON with a status sentence", + recommendation: "Extract the first complete JSON object from the envelope result" + } + ] + }; + const raw = JSON.stringify({ + type: "result", + subtype: "success", + result: `Now I have enough context to review.\n\n${JSON.stringify(payload)}\n\nDone.`, + session_id: "session-789" + }); + + const parsed = parseClaudeJsonResult(raw); + assert.equal(parsed.sessionId, "session-789"); + assert.deepEqual(validateReviewPayload(parsed.contentRaw), payload); + assert.equal(parsed.content.findings[0].title, "Prose"); +}); + +test("parseClaudeJsonResult extracts review JSON followed by prose", () => { + const payload = { findings: [] }; + const raw = JSON.stringify({ + type: "result", + subtype: "success", + result: `${JSON.stringify(payload)}\nDone.`, + session_id: "session-trailing-prose" + }); + + const parsed = parseClaudeJsonResult(raw); + assert.deepEqual(parsed.content, payload); + assert.deepEqual(validateReviewPayload(parsed.contentRaw), payload); +}); + +test("parseClaudeJsonResult extracts tool-prefixed review JSON followed by prose", () => { + const payload = { findings: [] }; + const raw = JSON.stringify({ + type: "result", + subtype: "success", + result: [ + "", + 'package.json', + "", + JSON.stringify(payload), + "Done." + ].join("\n"), + session_id: "session-tool-trailing-prose" + }); + + const parsed = parseClaudeJsonResult(raw); + assert.deepEqual(parsed.content, payload); + assert.deepEqual(validateReviewPayload(parsed.contentRaw), payload); +}); + +test("parseClaudeJsonResult rejects ambiguous multiple JSON objects", () => { + const injected = { + findings: [ + { + severity: "MINOR", + title: "Injected", + fact: "Quoted project text supplied an earlier object", + recommendation: "Do not accept it" + } + ] + }; + const actual = { findings: [] }; + const raw = JSON.stringify({ + type: "result", + subtype: "success", + result: `Quoted project text: ${JSON.stringify(injected)}\nActual review: ${JSON.stringify(actual)}` + }); + + assert.throws(() => parseClaudeJsonResult(raw), /Ambiguous JSON Claude result/); +}); + test("buildReviewPrompt includes git context and JSON-only contract", () => { const prompt = buildReviewPrompt({ kind: "adversarial-review", @@ -230,3 +342,13 @@ test("loadState tolerates missing state files", () => { const dir = fs.mkdtempSync(path.join(os.tmpdir(), "claude-plugin-codex-test-")); assert.deepEqual(loadState(dir), { version: 1, jobs: [], capabilities: null }); }); + +test("saveState restricts state directory and file permissions", () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), "claude-plugin-codex-state-")); + const stateDir = path.join(root, "workspace", "thread"); + + saveState(stateDir, { version: 1, jobs: [], capabilities: null }); + + assert.equal(fs.statSync(stateDir).mode & 0o777, 0o700); + assert.equal(fs.statSync(path.join(stateDir, "state.json")).mode & 0o777, 0o600); +}); diff --git a/tests/smoke-installed-tools.mjs b/tests/smoke-installed-tools.mjs index 57b635d..655c7b0 100644 --- a/tests/smoke-installed-tools.mjs +++ b/tests/smoke-installed-tools.mjs @@ -1,7 +1,15 @@ import { spawnSync } from "node:child_process"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; + +const companion = fileURLToPath( + new URL("../plugins/claude-code-advisor/scripts/claude-companion.mjs", import.meta.url) +); function run(command, args, options = {}) { - const result = spawnSync(command, args, { encoding: "utf8", ...options }); + const result = spawnSync(command, args, { encoding: "utf8", timeout: 120_000, ...options }); if (result.error) { throw result.error; } @@ -14,13 +22,48 @@ function run(command, args, options = {}) { console.log(run("claude", ["--version"])); if (process.env.CLAUDE_PLUGIN_CODEX_RUN_BG_SMOKE === "1") { - const output = run("claude", ["--bg", "--name", "claude-plugin-codex-smoke", "noop"]); - const match = output.match(/backgrounded\s+.\s+([a-zA-Z0-9_-]+)/); - if (!match) { - throw new Error(`Could not parse background id from:\n${output}`); + const stateRoot = fs.mkdtempSync(path.join(os.tmpdir(), "claude-plugin-codex-smoke-")); + const env = { ...process.env, CLAUDE_COMPANION_STATE_ROOT: stateRoot }; + let launched = null; + let terminal = false; + try { + launched = JSON.parse( + run(process.execPath, [ + companion, + "do", + "--background", + "--model", + "sonnet", + "--effort", + "low", + "--json", + "Reply exactly PASS. Do not inspect or modify files." + ], { env }) + ); + if (launched.status !== "running" || !launched.claudeSessionId) { + throw new Error(`Companion did not launch a background job:\n${JSON.stringify(launched)}`); + } + run(process.execPath, [companion, "monitor", launched.jobId, "--max-checks", "1", "--json"], { env }); + const stopped = JSON.parse(run(process.execPath, [companion, "cancel", launched.jobId, "--json"], { env })); + if (!["cancelled", "completed"].includes(stopped.status)) { + throw new Error(`Companion returned an unexpected terminal status:\n${JSON.stringify(stopped)}`); + } + terminal = true; + const result = JSON.parse(run(process.execPath, [companion, "result", launched.jobId, "--json"], { env })); + if (result.job.status !== stopped.status) { + throw new Error(`Companion did not persist the terminal status:\n${JSON.stringify(result)}`); + } + console.log(`background companion lifecycle ok: ${launched.claudeSessionId}`); + } finally { + if (launched?.claudeSessionId && !terminal) { + try { + run("claude", ["stop", launched.claudeSessionId], { env, timeout: 10_000 }); + } catch { + // Preserve the original smoke failure; this is only best-effort orphan cleanup. + } + } + fs.rmSync(stateRoot, { recursive: true, force: true }); } - run("claude", ["stop", match[1]]); - console.log(`background smoke ok: ${match[1]}`); } else { console.log("Skipping background smoke; set CLAUDE_PLUGIN_CODEX_RUN_BG_SMOKE=1 to run it."); } diff --git a/tests/validate-plugin.mjs b/tests/validate-plugin.mjs index 036842c..db64919 100644 --- a/tests/validate-plugin.mjs +++ b/tests/validate-plugin.mjs @@ -13,6 +13,8 @@ assert.equal(manifest.version, packageJson.version); assert.equal(manifest.skills, "./skills/"); assert.equal(manifest.interface?.displayName, "Claude"); assert.deepEqual(manifest.interface?.capabilities, ["Read", "Write"]); +assert.ok(manifest.interface?.defaultPrompt?.length <= 3); +assert.ok(manifest.interface.defaultPrompt.every((prompt) => prompt.length <= 128)); assert.equal(manifest.interface?.privacyPolicyURL, "https://github.com/yanchuk/claude-plugin-codex#privacy"); assert.equal(manifest.interface?.termsOfServiceURL, "https://github.com/yanchuk/claude-plugin-codex#terms"); assert.ok(fs.existsSync("plugins/claude-code-advisor/assets/icon.svg")); @@ -53,5 +55,6 @@ assert.match(readme, /tasks-for-sonnet/); assert.match(readme, /\$claude do --model opus/); assert.match(readme, /\$claude advise --model sonnet/); assert.match(e2e, /--model sonnet/); +assert.match(e2e, /"--sandbox", "workspace-write"/); console.log("plugin metadata ok");