diff --git a/README.md b/README.md index 937a3037b..039e6e13e 100644 --- a/README.md +++ b/README.md @@ -11,6 +11,7 @@ they already have. - `/codex:review` for a normal read-only Codex review - `/codex:adversarial-review` for a steerable challenge review +- `/codex:verified-review` for a native review with an independent evidence pass - `/codex:rescue`, `/codex:transfer`, `/codex:status`, `/codex:result`, and `/codex:cancel` to delegate work, hand off sessions, and manage background jobs ## Requirements @@ -123,6 +124,20 @@ Examples: This command is read-only. It does not fix code. +### `/codex:verified-review` + +Runs one native Codex review, then a fresh read-only Codex turn that independently verifies every finding. The final report labels each finding as `confirmed`, `false-positive`, `style-only`, or `unverified`, with evidence and any check output. + +It accepts the normal review target options, including `--base ` and `--scope auto|working-tree|branch`. Add `--check ""` repeatedly to authorize only those validation commands for the verification turn: + +```bash +/codex:verified-review +/codex:verified-review --base main --check "npm test" --check "npm run build" +/codex:verified-review --background --check "npm test" +``` + +`--check` is a trust boundary: each command is passed exactly as user-supplied to Codex's local read-only verification sandbox and may invoke arbitrary local programs available there. The plugin does not infer or run default test, build, lint, or check commands. Without `--check`, verification uses only read-only repository inspection. Use [`/codex:status`](#codexstatus) and [`/codex:result`](#codexresult) for background work. + ### `/codex:rescue` Hands a task to Codex through the `codex:codex-rescue` subagent. diff --git a/plugins/codex/commands/verified-review.md b/plugins/codex/commands/verified-review.md new file mode 100644 index 000000000..f6564edee --- /dev/null +++ b/plugins/codex/commands/verified-review.md @@ -0,0 +1,68 @@ +--- +description: Run a native Codex review, then independently verify every finding +argument-hint: '[--wait|--background] [--base ] [--scope auto|working-tree|branch] [--check ""]...' +disable-model-invocation: true +allowed-tools: Read, Glob, Grep, Bash(node:*), Bash(git:*), AskUserQuestion +--- + +Run a verified Codex review through the shared plugin runtime. + +Raw slash-command arguments: +`$ARGUMENTS` + +Safe input transport: +- Additional command context supplies `CODEX_VERIFIED_REVIEW_CAPTURE_ID=` for these raw arguments. Treat that UUID as opaque. +- Before any execution, find one valid `CODEX_VERIFIED_REVIEW_CAPTURE_ID` UUID marker in that context. If it is absent or invalid, fail closed: do not invoke the companion and report that the verified review cannot safely access its captured input. +- Pass only `--captured-input ""` to the companion. Never copy, interpolate, export, pipe, or otherwise place raw `$ARGUMENTS` in Bash, a template string, an environment variable, or stdin. + +Core constraint: +- This command is review-only and read-only. +- Do not fix issues, apply patches, or suggest that you are about to make changes. +- Run one native Codex review, then one fresh ephemeral read-only Codex verification turn. +- The verifier must classify every finding as `confirmed`, `false-positive`, `style-only`, or `unverified` and include its evidence. +- Return the command stdout verbatim to the user. Do not paraphrase, summarize, or add commentary before or after it. + +Explicit check trust boundary: +- Each repeated `--check ""` value is an explicit user-authorized shell command for the verification turn. +- Preserve each value exactly. Never invent, rewrite, expand, or add a default test, build, lint, or check command. +- Those commands run in the local repository through Codex's read-only verification sandbox. Treat their text as trusted user input: they can invoke arbitrary local programs available to Codex. +- Without `--check`, the verifier must not run validation commands; it may only inspect repository files and git state read-only. + +Execution mode rules: +- If raw arguments include `--wait`, do not ask. Run in the foreground. +- If raw arguments include `--background`, do not ask. Run in a Claude background task. +- Otherwise, estimate review size before asking: + - If raw arguments explicitly select a branch with `--base` or `--scope branch`, never run Bash to size that branch; recommend background. + - For auto or working-tree review with no explicit branch selector, run only fixed, argument-free working-tree sizing commands: `git status --short --untracked-files=all`, `git diff --shortstat --cached`, and `git diff --shortstat`. + - Never copy or interpolate a raw base or ref into Bash. + - Treat untracked files or directories as reviewable work even when `git diff --shortstat` is empty. + - If the working tree is clean, the companion will fall back to branch review, or the size is unclear, recommend background. + - Recommend waiting only when the scoped review is clearly tiny, roughly 1-2 files total and no sign of a broader directory-sized change. + - In every other case, including unclear size, recommend background. +- Then use `AskUserQuestion` exactly once with two options, putting the recommended option first and suffixing it with `(Recommended)`: + - `Wait for results` + - `Run in background` + +Argument handling: +- The captured input preserves `--base`, `--scope`, every `--check`, `--wait`, and `--background` exactly. +- `/codex:verified-review` supports only auto, working-tree, and branch review scopes. It does not support staged-only review, unstaged-only review, or focus text. +- The companion reads the captured input and parses `--wait`, `--background`, and repeated `--check`; Claude Code's `Bash(..., run_in_background: true)` is what detaches this slash-command turn. + +Foreground flow: +- Run: +```bash +node "${CLAUDE_PLUGIN_ROOT}/scripts/codex-companion.mjs" verified-review --captured-input "" +``` +- Return stdout verbatim, exactly as-is. + +Background flow: +- Launch with `Bash` in the background: +```typescript +Bash({ + command: `node "${CLAUDE_PLUGIN_ROOT}/scripts/codex-companion.mjs" verified-review --captured-input ""`, + description: "Codex verified review", + run_in_background: true +}) +``` +- Do not call `BashOutput` or wait for completion in this turn. +- After launching, tell the user: "Codex verified review started in the background. Check `/codex:status` for progress." diff --git a/plugins/codex/hooks/hooks.json b/plugins/codex/hooks/hooks.json index 19e33b818..7a6a32c9b 100644 --- a/plugins/codex/hooks/hooks.json +++ b/plugins/codex/hooks/hooks.json @@ -1,6 +1,18 @@ { "description": "Optional stop-time review gate for Codex Companion.", "hooks": { + "UserPromptExpansion": [ + { + "matcher": "^(?:codex:)?verified-review$", + "hooks": [ + { + "type": "command", + "command": "node \"${CLAUDE_PLUGIN_ROOT}/scripts/verified-review-input-hook.mjs\"", + "timeout": 5 + } + ] + } + ], "SessionStart": [ { "hooks": [ diff --git a/plugins/codex/prompts/verified-review.md b/plugins/codex/prompts/verified-review.md new file mode 100644 index 000000000..a13d18664 --- /dev/null +++ b/plugins/codex/prompts/verified-review.md @@ -0,0 +1,31 @@ + +You are the independent verification pass after a native Codex review. + + + +Verify every finding from the native review against the current repository state. +Target: {{TARGET_LABEL}} + + + +{{NATIVE_REVIEW_OUTPUT}} + + + +{{NATIVE_FINDINGS}} + + + +{{EXPLICIT_CHECKS}} + + + +- Work in this fresh, read-only turn. Do not rely on the native review's conclusion without checking its evidence. +- Classify every entry in the JSON array inside `` exactly once. Set each returned finding's `native_finding_id` to that entry's ID. Do not add, omit, or repeat IDs. If the array is empty, return an empty findings array. +- Prefix every returned finding title with one of: `[confirmed]`, `[false-positive]`, `[style-only]`, or `[unverified]`. +- Put concrete verification evidence in every finding body, including source locations, observed behavior, and any explicit-check result that applies. +- Execute only these explicitly supplied commands as validation checks. Run each supplied command exactly once; do not infer, substitute, expand, or run a default test, build, lint, or check command. +- You may run additional read-only inspection commands for repository files and git state. Those are inspection evidence, not validation checks. Do not edit files or execute commands that change repository state. +- If no explicit checks were supplied, run none. If an explicit check cannot run, include its command, failure evidence, and an `[unverified]` classification where relevant. +- Return only valid JSON matching the supplied schema. Keep findings compact and evidence-based. + diff --git a/plugins/codex/schemas/verified-review-output.schema.json b/plugins/codex/schemas/verified-review-output.schema.json new file mode 100644 index 000000000..5cfaea905 --- /dev/null +++ b/plugins/codex/schemas/verified-review-output.schema.json @@ -0,0 +1,30 @@ +{ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "type": "object", + "additionalProperties": false, + "required": ["verdict", "summary", "findings", "next_steps"], + "properties": { + "verdict": { "type": "string", "enum": ["approve", "needs-attention"] }, + "summary": { "type": "string", "minLength": 1 }, + "findings": { + "type": "array", + "items": { + "type": "object", + "additionalProperties": false, + "required": ["native_finding_id", "severity", "title", "body", "file", "line_start", "line_end", "confidence", "recommendation"], + "properties": { + "native_finding_id": { "type": "string", "minLength": 1 }, + "severity": { "type": "string", "enum": ["critical", "high", "medium", "low"] }, + "title": { "type": "string", "minLength": 1 }, + "body": { "type": "string", "minLength": 1 }, + "file": { "type": "string", "minLength": 1 }, + "line_start": { "type": "integer", "minimum": 1 }, + "line_end": { "type": "integer", "minimum": 1 }, + "confidence": { "type": "number", "minimum": 0, "maximum": 1 }, + "recommendation": { "type": "string" } + } + } + }, + "next_steps": { "type": "array", "items": { "type": "string", "minLength": 1 } } + } +} diff --git a/plugins/codex/scripts/codex-companion.mjs b/plugins/codex/scripts/codex-companion.mjs index 83df468ad..efb006c4d 100644 --- a/plugins/codex/scripts/codex-companion.mjs +++ b/plugins/codex/scripts/codex-companion.mjs @@ -7,6 +7,10 @@ import process from "node:process"; import { fileURLToPath } from "node:url"; import { parseArgs, splitRawArgumentString } from "./lib/args.mjs"; +import { + consumeVerifiedReviewInput, + VERIFIED_REVIEW_CAPTURE_ID_PATTERN +} from "./lib/verified-review-input.mjs"; import { buildPersistentTaskThreadName, DEFAULT_CONTINUE_PROMPT, @@ -44,11 +48,13 @@ import { } from "./lib/job-control.mjs"; import { appendLogLine, + cancelTrackedJob, createJobLogFile, createJobProgressUpdater, createJobRecord, createProgressReporter, nowIso, + recordQueuedJobPid, runTrackedJob, SESSION_ID_ENV } from "./lib/tracked-jobs.mjs"; @@ -61,11 +67,13 @@ import { renderJobStatusReport, renderSetupReport, renderStatusReport, - renderTaskResult + renderTaskResult, + validateReviewResultShape } from "./lib/render.mjs"; const ROOT_DIR = path.resolve(fileURLToPath(new URL("..", import.meta.url))); const REVIEW_SCHEMA = path.join(ROOT_DIR, "schemas", "review-output.schema.json"); +const VERIFIED_REVIEW_SCHEMA = path.join(ROOT_DIR, "schemas", "verified-review-output.schema.json"); const DEFAULT_STATUS_WAIT_TIMEOUT_MS = 240000; const DEFAULT_STATUS_POLL_INTERVAL_MS = 2000; const VALID_REASONING_EFFORTS = new Set(["none", "minimal", "low", "medium", "high", "xhigh"]); @@ -79,6 +87,7 @@ function printUsage() { " node scripts/codex-companion.mjs setup [--enable-review-gate|--disable-review-gate] [--json]", " node scripts/codex-companion.mjs review [--wait|--background] [--base ] [--scope ]", " node scripts/codex-companion.mjs adversarial-review [--wait|--background] [--base ] [--scope ] [focus text]", + " node scripts/codex-companion.mjs verified-review [--wait|--background] [--base ] [--scope ] [--check ]...", " node scripts/codex-companion.mjs task [--background] [--write] [--resume-last|--resume|--fresh] [--model ] [--effort ] [prompt]", " node scripts/codex-companion.mjs transfer [--source ] [--json]", " node scripts/codex-companion.mjs status [job-id] [--all] [--json]", @@ -249,6 +258,18 @@ function buildAdversarialReviewPrompt(context, focusText) { }); } +function buildVerifiedReviewPrompt({ target, nativeReview, nativeFindings, checks }) { + const template = loadPromptTemplate(ROOT_DIR, "verified-review"); + const explicitChecks = checks.length > 0 ? checks.map((command) => `- ${command}`).join("\n") : "- None supplied."; + const findingList = JSON.stringify(nativeFindings, null, 2); + return interpolateTemplate(template, { + TARGET_LABEL: target.label, + NATIVE_REVIEW_OUTPUT: nativeReview || "Native review returned no text.", + NATIVE_FINDINGS: findingList, + EXPLICIT_CHECKS: explicitChecks + }); +} + function ensureCodexAvailable(cwd) { const availability = getCodexAvailability(cwd); if (!availability.available) { @@ -457,6 +478,372 @@ async function executeReviewRun(request) { }; } +const VERIFIED_FINDING_PREFIX = /^\[(?:confirmed|false-positive|style-only|unverified)\]\s+/; +const VERIFIED_FINDING_SEVERITIES = new Set(["critical", "high", "medium", "low"]); +const NATIVE_FINDING_ITEM = /^(?:[-*+]\s+|\d+[.)]\s+|\[P\d+\]\s+)(\S.*)$/i; +const NATIVE_REVIEW_FENCE = /^\s*(`{3,}|~{3,})(.*)$/; +const NATIVE_REVIEW_HEADER = /^reviewed\b(?!.*\b(?:but|however|found|issue|finding|problem)\b).*?[.!]?$/i; +const NATIVE_REVIEW_CLEAN = /^(?:[-*+]\s+)?(?:none|no\s+(?:material\s+)?(?:issues?|findings?|problems?)(?:\s+found)?|looks\s+good|nothing\s+to\s+report)\.?$/i; + +function extractNativeFindings(reviewText) { + const text = String(reviewText ?? ""); + if (!text.trim()) { + return { findings: [], error: "Native review returned no text, so there are no findings to verify." }; + } + + const findings = []; + let current = null; + let fence = null; + const topLevelLines = []; + for (const rawLine of text.split(/\r?\n/)) { + const normalized = rawLine.replace(/\s+/g, " ").trim(); + const fenceMatch = rawLine.match(NATIVE_REVIEW_FENCE); + const isFenceClose = fence && fenceMatch && fenceMatch[1][0] === fence.marker && fenceMatch[1].length >= fence.length && !fenceMatch[2].trim(); + if (fence || fenceMatch) { + if (current && normalized) { + current.text += `\n${normalized}`; + } + if (isFenceClose) { + fence = null; + } else if (!fence && fenceMatch) { + fence = { marker: fenceMatch[1][0], length: fenceMatch[1].length }; + } + continue; + } + if (!normalized) { + continue; + } + if (/^\s/.test(rawLine)) { + if (current) { + current.text += `\n${normalized}`; + } + continue; + } + + topLevelLines.push(normalized); + const match = normalized.match(NATIVE_FINDING_ITEM); + if (match) { + current = { id: `native-${findings.length + 1}`, text: match[1] }; + findings.push(current); + } else if (current) { + current.text += `\n${normalized}`; + } + } + + if (fence) { + return { + findings: [], + error: "Native review contains an unterminated fenced block, so its finding set cannot be verified." + }; + } + + if (topLevelLines.some((line) => NATIVE_REVIEW_CLEAN.test(line))) { + if (topLevelLines.every((line) => NATIVE_REVIEW_HEADER.test(line) || NATIVE_REVIEW_CLEAN.test(line))) { + return { findings: [], error: null }; + } + return { + findings: [], + error: "Native review combines a clean sentinel with substantive text, so its finding set is ambiguous." + }; + } + if (findings.length === 0) { + return { + findings: [], + error: "Native review output did not use a recognizable finding list or an explicit clean result. It cannot be verified one-to-one." + }; + } + return { findings, error: null }; +} + +function invalidVerifiedReview(parsed, parseError) { + return { ...parsed, parsed: null, parseError }; +} + +function validateVerifiedFindingShape(finding, index) { + const label = `findings[${index}]`; + if (!finding || typeof finding !== "object" || Array.isArray(finding)) { + return `${label} must be an object.`; + } + for (const field of ["native_finding_id", "title", "body", "file"]) { + if (typeof finding[field] !== "string" || !finding[field].trim()) { + return `${label}.${field} must be a non-empty string.`; + } + } + if (!VERIFIED_FINDING_SEVERITIES.has(finding.severity)) { + return `${label}.severity is invalid.`; + } + for (const field of ["line_start", "line_end"]) { + if (!Number.isInteger(finding[field]) || finding[field] < 1) { + return `${label}.${field} must be a positive integer.`; + } + } + if (finding.line_end < finding.line_start) { + return `${label}.line_end must be greater than or equal to line_start.`; + } + if (!Number.isFinite(finding.confidence) || finding.confidence < 0 || finding.confidence > 1) { + return `${label}.confidence must be a number from 0 to 1.`; + } + if (typeof finding.recommendation !== "string") { + return `${label}.recommendation must be a string.`; + } + return null; +} + +function parseVerifiedReviewOutput(result, nativeFindings) { + const parsed = parseStructuredOutput(result.finalMessage, { + status: result.status, + failureMessage: result.error?.message ?? result.stderr + }); + if (!parsed.parsed) { + return parsed; + } + + const shapeError = validateReviewResultShape(parsed.parsed); + if (shapeError) { + return invalidVerifiedReview(parsed, shapeError); + } + + for (let index = 0; index < parsed.parsed.findings.length; index += 1) { + const findingError = validateVerifiedFindingShape(parsed.parsed.findings[index], index); + if (findingError) { + return invalidVerifiedReview(parsed, findingError); + } + } + + if (!parsed.parsed.findings.every((finding) => VERIFIED_FINDING_PREFIX.test(finding?.title ?? ""))) { + return invalidVerifiedReview( + parsed, + "Every verified-review finding title must begin with [confirmed], [false-positive], [style-only], or [unverified]." + ); + } + + const expectedIds = nativeFindings.map((finding) => finding.id); + const actualIds = parsed.parsed.findings.map((finding) => finding?.native_finding_id); + const actualIdSet = new Set(actualIds); + if ( + actualIds.some((id) => typeof id !== "string") || + actualIds.length !== expectedIds.length || + actualIdSet.size !== actualIds.length || + actualIdSet.size !== expectedIds.length || + expectedIds.some((id) => !actualIdSet.has(id)) + ) { + return invalidVerifiedReview( + parsed, + "Verified findings must map one-to-one to the native finding IDs; missing, duplicate, and unexpected IDs are not verifiable." + ); + } + + return parsed; +} + +function summarizeCheckExecutions(commandExecutions) { + return commandExecutions.map((execution) => ({ + command: String(execution.command ?? ""), + commandActions: Array.isArray(execution.commandActions) ? execution.commandActions : [], + status: execution.status ?? null, + exitCode: execution.exitCode ?? null, + log: String(execution.aggregatedOutput ?? execution.output ?? execution.stdout ?? execution.stderr ?? "").trim() + })); +} + +function separateCheckExecutions(requestedChecks, executions) { + const requested = new Set(requestedChecks); + const checks = []; + const inspectionExecutions = []; + const unauthorizedValidationExecutions = []; + for (const execution of executions) { + if (requested.has(execution.command)) { + checks.push(execution); + } else if ( + execution.commandActions.length > 0 && + execution.commandActions.every((action) => ["read", "listFiles", "search"].includes(typeof action === "string" ? action : action?.type)) + ) { + inspectionExecutions.push(execution); + } else { + unauthorizedValidationExecutions.push(execution); + } + } + return { checks, inspectionExecutions, unauthorizedValidationExecutions }; +} + +function compareCheckExecutions(requestedChecks, checks) { + const expected = new Map(); + const actual = new Map(); + for (const command of requestedChecks) { + expected.set(command, (expected.get(command) ?? 0) + 1); + } + for (const check of checks) { + actual.set(check.command, (actual.get(check.command) ?? 0) + 1); + } + + const missingChecks = []; + const duplicateChecks = []; + for (const [command, count] of expected) { + const observed = actual.get(command) ?? 0; + for (let index = observed; index < count; index += 1) missingChecks.push(command); + for (let index = count; index < observed; index += 1) duplicateChecks.push(command); + } + return { missingChecks, duplicateChecks }; +} + +function renderVerifiedReviewResult(parsed, meta) { + const renderedReview = renderReviewResult(parsed, { + reviewLabel: "Verified Review", + targetLabel: meta.targetLabel, + reasoningSummary: meta.reasoningSummary + }).trimEnd(); + const lines = [renderedReview, "", "Verification evidence:", `- Native review exit: ${meta.native.status}`]; + + if (meta.native.stdout) { + lines.push("", "Native review output:", "", "```text", meta.native.stdout.trimEnd(), "```"); + } + + if (meta.requestedChecks.length === 0) { + lines.push("- Requested explicit checks: none supplied."); + } else { + lines.push("- Requested explicit checks:"); + for (const command of meta.requestedChecks) { + lines.push(` - \`${command.replace(/`/g, "\\`")}\``); + } + } + lines.push("- Explicit check executions:"); + if (meta.checks.length === 0) { + lines.push(" - None executed."); + } else { + for (const check of meta.checks) { + lines.push(` - Command: \`${check.command.replace(/`/g, "\\`")}\``); + lines.push(` Exit: ${check.exitCode ?? "unknown"}; status: ${check.status ?? "unknown"}`); + if (check.log) { + lines.push("", " Log:", "", "```text", check.log, "```"); + } + } + } + + if (meta.checkCoverage.missingChecks.length > 0) { + lines.push("- Missing explicit checks:", ...meta.checkCoverage.missingChecks.map((command) => ` - \`${command.replace(/`/g, "\\`")}\``)); + } + if (meta.checkCoverage.duplicateChecks.length > 0) { + lines.push("- Duplicate explicit checks:", ...meta.checkCoverage.duplicateChecks.map((command) => ` - \`${command.replace(/`/g, "\\`")}\``)); + } + if (meta.inspectionExecutions.length > 0) { + lines.push("- Read-only inspection executions:"); + for (const execution of meta.inspectionExecutions) { + lines.push(` - Command: \`${execution.command.replace(/`/g, "\\`")}\``); + lines.push(` Exit: ${execution.exitCode ?? "unknown"}; status: ${execution.status ?? "unknown"}`); + if (execution.log) { + lines.push("", " Log:", "", "```text", execution.log, "```"); + } + } + } + if (meta.unauthorizedValidationExecutions.length > 0) { + lines.push("- Unauthorized validation commands:"); + for (const execution of meta.unauthorizedValidationExecutions) { + lines.push(` - Command: \`${execution.command.replace(/`/g, "\\`")}\``); + lines.push(` Exit: ${execution.exitCode ?? "unknown"}; status: ${execution.status ?? "unknown"}`); + if (execution.log) { + lines.push("", " Log:", "", "```text", execution.log, "```"); + } + } + } + + return `${lines.join("\n").trimEnd()}\n`; +} + +async function executeVerifiedReviewRun(request) { + ensureCodexAvailable(request.cwd); + ensureGitRepository(request.cwd); + + const target = resolveReviewTarget(request.cwd, { + base: request.base, + scope: request.scope + }); + const nativeTarget = validateNativeReviewRequest(target, ""); + const native = await runAppServerReview(request.cwd, { + target: nativeTarget, + onProgress: request.onProgress + }); + const nativeFindingExtraction = extractNativeFindings(native.reviewText); + const verification = await runAppServerTurn(request.cwd, { + prompt: buildVerifiedReviewPrompt({ + target, + nativeReview: native.reviewText, + nativeFindings: nativeFindingExtraction.findings, + checks: request.checks + }), + sandbox: "read-only", + outputSchema: readOutputSchema(VERIFIED_REVIEW_SCHEMA), + onProgress: request.onProgress + }); + const verificationParsed = parseVerifiedReviewOutput(verification, nativeFindingExtraction.findings); + const executions = summarizeCheckExecutions(verification.commandExecutions); + const { checks, inspectionExecutions, unauthorizedValidationExecutions } = separateCheckExecutions(request.checks, executions); + const checkCoverage = compareCheckExecutions(request.checks, checks); + const checkCoverageError = [ + ...checkCoverage.missingChecks.map((command) => `missing: ${command}`), + ...checkCoverage.duplicateChecks.map((command) => `duplicate: ${command}`) + ]; + const parsed = nativeFindingExtraction.error + ? invalidVerifiedReview(verificationParsed, nativeFindingExtraction.error) + : checkCoverageError.length > 0 + ? invalidVerifiedReview(verificationParsed, `Explicit check execution mismatch (${checkCoverageError.join("; ")}).`) + : unauthorizedValidationExecutions.length > 0 + ? invalidVerifiedReview(verificationParsed, "Unauthorized validation commands ran outside the requested checks or read-only inspection actions.") + : verificationParsed; + const payload = { + review: "Verified Review", + target, + native: { + status: native.status, + threadId: native.threadId, + sourceThreadId: native.sourceThreadId, + turnId: native.turnId, + stdout: native.reviewText, + stderr: native.stderr, + reasoning: native.reasoningSummary, + findings: nativeFindingExtraction.findings + }, + verification: { + status: verification.status, + threadId: verification.threadId, + turnId: verification.turnId, + stdout: verification.finalMessage, + stderr: verification.stderr, + reasoning: verification.reasoningSummary, + requestedChecks: request.checks, + checks, + inspectionExecutions, + unauthorizedValidationExecutions, + missingChecks: checkCoverage.missingChecks, + duplicateChecks: checkCoverage.duplicateChecks + }, + result: parsed.parsed, + rawOutput: parsed.rawOutput, + parseError: parsed.parseError + }; + const exitStatus = native.status === 0 && verification.status === 0 && parsed.parsed ? 0 : 1; + + return { + exitStatus, + threadId: verification.threadId, + turnId: verification.turnId, + payload, + rendered: renderVerifiedReviewResult(parsed, { + targetLabel: target.label, + native, + requestedChecks: request.checks, + checks, + inspectionExecutions, + unauthorizedValidationExecutions, + checkCoverage, + reasoningSummary: verification.reasoningSummary + }), + summary: parsed.parsed?.summary ?? parsed.parseError ?? firstMeaningfulLine(verification.finalMessage, "Verified Review finished."), + jobTitle: "Codex Verified Review", + jobClass: "review", + targetLabel: target.label + }; +} + async function executeTaskRun(request) { const workspaceRoot = resolveWorkspaceRoot(request.cwd); @@ -531,7 +918,7 @@ async function executeTaskRun(request) { function buildReviewJobMetadata(reviewName, target) { return { - kind: reviewName === "Adversarial Review" ? "adversarial-review" : "review", + kind: reviewName === "Adversarial Review" ? "adversarial-review" : reviewName === "Verified Review" ? "verified-review" : "review", title: reviewName === "Review" ? "Codex Review" : `Codex ${reviewName}`, summary: `${reviewName} ${target.label}` }; @@ -561,6 +948,9 @@ function getJobKindLabel(kind, jobClass) { if (kind === "adversarial-review") { return "adversarial-review"; } + if (kind === "verified-review") { + return "verified-review"; + } return jobClass === "review" ? "review" : "rescue"; } @@ -661,6 +1051,9 @@ async function runForegroundCommand(job, runner, options = {}) { stderr: !options.json }); const execution = await runTrackedJob(job, () => runner(progress), { logFile }); + if (!execution) { + return null; + } outputResult(options.json ? execution.payload : execution.rendered, options.json); if (execution.exitStatus !== 0) { process.exitCode = execution.exitStatus; @@ -684,7 +1077,6 @@ function spawnDetachedTaskWorker(cwd, jobId) { function enqueueBackgroundTask(cwd, job, request) { const { logFile } = createTrackedProgress(job); appendLogLine(logFile, "Queued for background execution."); - const child = spawnDetachedTaskWorker(cwd, job.id); const queuedRecord = { ...job, @@ -709,6 +1101,49 @@ function enqueueBackgroundTask(cwd, job, request) { }; } +function spawnDetachedVerifiedReviewWorker(cwd, jobId) { + const scriptPath = path.join(ROOT_DIR, "scripts", "codex-companion.mjs"); + const child = spawn(process.execPath, [scriptPath, "verified-review-worker", "--cwd", cwd, "--job-id", jobId], { + cwd, + env: process.env, + detached: true, + stdio: "ignore", + windowsHide: true + }); + child.unref(); + return child; +} + +function enqueueBackgroundVerifiedReview(cwd, job, request) { + const { logFile } = createTrackedProgress(job); + appendLogLine(logFile, "Queued for background execution."); + const queuedRecord = { + ...job, + status: "queued", + phase: "queued", + pid: null, + logFile, + request + }; + writeJobFile(job.workspaceRoot, job.id, queuedRecord); + upsertJob(job.workspaceRoot, queuedRecord); + const child = spawnDetachedVerifiedReviewWorker(cwd, job.id); + if (!recordQueuedJobPid(job.workspaceRoot, job.id, child.pid ?? null)) { + terminateProcessTree(child.pid ?? Number.NaN); + } + + return { + payload: { + jobId: job.id, + status: "queued", + title: job.title, + summary: job.summary, + logFile + }, + logFile + }; +} + async function handleReviewCommand(argv, config) { const { options, positionals } = parseCommandInput(argv, { valueOptions: ["base", "scope", "model", "cwd"], @@ -759,6 +1194,80 @@ async function handleReview(argv) { }); } +function readExplicitChecks(checks) { + const values = (checks ?? []).map((check) => String(check)); + if (values.some((check) => !check.trim())) { + throw new Error("Each --check command must be non-empty."); + } + return values; +} + +async function resolveVerifiedReviewArgv(argv) { + const hasCapturedInput = argv.some( + (argument) => argument === "--captured-input" || argument.startsWith("--captured-input=") + ); + if (!hasCapturedInput) { + return argv; + } + + if ( + argv.length !== 2 || + argv[0] !== "--captured-input" || + !VERIFIED_REVIEW_CAPTURE_ID_PATTERN.test(argv[1]) + ) { + throw new Error("`--captured-input` must be the only verified-review option and use a valid capture ID."); + } + + const sessionId = getCurrentClaudeSessionId(); + if (!sessionId) { + throw new Error("`--captured-input` requires an active Claude session."); + } + + const captured = await consumeVerifiedReviewInput(process.cwd(), argv[1], { sessionId }); + if (!captured || typeof captured.rawArguments !== "string") { + throw new Error("Captured verified-review input is invalid."); + } + return [captured.rawArguments]; +} + +async function handleVerifiedReview(argv) { + argv = await resolveVerifiedReviewArgv(argv); + const { options, positionals } = parseCommandInput(argv, { + valueOptions: ["base", "scope", "cwd", "check"], + repeatableValueOptions: ["check"], + booleanOptions: ["json", "background", "wait"] + }); + if (positionals.length > 0) { + throw new Error("`/codex:verified-review` accepts review options and repeated --check commands only."); + } + + const cwd = resolveCommandCwd(options); + const workspaceRoot = resolveCommandWorkspace(options); + const checks = readExplicitChecks(options.check); + const target = resolveReviewTarget(cwd, { base: options.base, scope: options.scope }); + const metadata = buildReviewJobMetadata("Verified Review", target); + const job = createCompanionJob({ + prefix: "review", + kind: metadata.kind, + title: metadata.title, + workspaceRoot, + jobClass: "review", + summary: metadata.summary + }); + const request = { cwd, base: options.base, scope: options.scope, checks, jobId: job.id }; + + if (options.background) { + ensureCodexAvailable(cwd); + const { payload } = enqueueBackgroundVerifiedReview(cwd, job, request); + outputCommandResult(payload, renderQueuedTaskLaunch(payload), options.json); + return; + } + + await runForegroundCommand(job, (progress) => executeVerifiedReviewRun({ ...request, onProgress: progress }), { + json: options.json + }); +} + async function handleTask(argv) { const { options, positionals } = parseCommandInput(argv, { valueOptions: ["model", "effort", "cwd", "prompt-file"], @@ -880,6 +1389,34 @@ async function handleTaskWorker(argv) { ); } +async function handleVerifiedReviewWorker(argv) { + const { options } = parseCommandInput(argv, { + valueOptions: ["cwd", "job-id"] + }); + if (!options["job-id"]) { + throw new Error("Missing required --job-id for verified-review-worker."); + } + + const cwd = resolveCommandCwd(options); + const workspaceRoot = resolveCommandWorkspace(options); + const storedJob = readStoredJob(workspaceRoot, options["job-id"]); + if (!storedJob?.request || typeof storedJob.request !== "object") { + throw new Error(`Stored job ${options["job-id"]} is missing its verified review request payload.`); + } + if (storedJob.status === "cancelled") { + return; + } + const { logFile, progress } = createTrackedProgress( + { ...storedJob, workspaceRoot }, + { logFile: storedJob.logFile ?? null } + ); + await runTrackedJob( + { ...storedJob, workspaceRoot, logFile }, + () => executeVerifiedReviewRun({ ...storedJob.request, onProgress: progress }), + { logFile } + ); +} + async function handleStatus(argv) { const { options, positionals } = parseCommandInput(argv, { valueOptions: ["cwd", "timeout-ms", "poll-interval-ms"], @@ -969,9 +1506,19 @@ async function handleCancel(argv) { const cwd = resolveCommandCwd(options); const reference = positionals[0] ?? ""; const { workspaceRoot, job } = resolveCancelableJob(cwd, reference, { env: process.env }); - const existing = readStoredJob(workspaceRoot, job.id) ?? {}; - const threadId = existing.threadId ?? job.threadId ?? null; - const turnId = existing.turnId ?? job.turnId ?? null; + const completedAt = nowIso(); + const cancellation = cancelTrackedJob(workspaceRoot, job.id, { + ...job, + completedAt, + errorMessage: "Cancelled by user.", + cancelledAt: completedAt + }); + if (!cancellation) { + throw new Error(`No active job found for "${job.id}".`); + } + const { previous, job: nextJob } = cancellation; + const threadId = previous.threadId ?? job.threadId ?? null; + const turnId = previous.turnId ?? job.turnId ?? null; const interrupt = await interruptAppServerTurn(cwd, { threadId, turnId }); if (interrupt.attempted) { @@ -983,32 +1530,8 @@ async function handleCancel(argv) { ); } - terminateProcessTree(job.pid ?? Number.NaN); - appendLogLine(job.logFile, "Cancelled by user."); - - const completedAt = nowIso(); - const nextJob = { - ...job, - status: "cancelled", - phase: "cancelled", - pid: null, - completedAt, - errorMessage: "Cancelled by user." - }; - - writeJobFile(workspaceRoot, job.id, { - ...existing, - ...nextJob, - cancelledAt: completedAt - }); - upsertJob(workspaceRoot, { - id: job.id, - status: "cancelled", - phase: "cancelled", - pid: null, - errorMessage: "Cancelled by user.", - completedAt - }); + terminateProcessTree(previous.pid ?? job.pid ?? Number.NaN); + appendLogLine(nextJob.logFile ?? job.logFile, "Cancelled by user."); const payload = { jobId: job.id, @@ -1040,6 +1563,9 @@ async function main() { reviewName: "Adversarial Review" }); break; + case "verified-review": + await handleVerifiedReview(argv); + break; case "task": await handleTask(argv); break; @@ -1049,6 +1575,9 @@ async function main() { case "task-worker": await handleTaskWorker(argv); break; + case "verified-review-worker": + await handleVerifiedReviewWorker(argv); + break; case "status": await handleStatus(argv); break; diff --git a/plugins/codex/scripts/lib/args.mjs b/plugins/codex/scripts/lib/args.mjs index 6b1518502..01cea156e 100644 --- a/plugins/codex/scripts/lib/args.mjs +++ b/plugins/codex/scripts/lib/args.mjs @@ -1,5 +1,6 @@ export function parseArgs(argv, config = {}) { const valueOptions = new Set(config.valueOptions ?? []); + const repeatableValueOptions = new Set(config.repeatableValueOptions ?? []); const booleanOptions = new Set(config.booleanOptions ?? []); const aliasMap = config.aliasMap ?? {}; const options = {}; @@ -25,7 +26,10 @@ export function parseArgs(argv, config = {}) { } if (token.startsWith("--")) { - const [rawKey, inlineValue] = token.slice(2).split("=", 2); + const rawOption = token.slice(2); + const equalsIndex = rawOption.indexOf("="); + const rawKey = equalsIndex === -1 ? rawOption : rawOption.slice(0, equalsIndex); + const inlineValue = equalsIndex === -1 ? undefined : rawOption.slice(equalsIndex + 1); const key = aliasMap[rawKey] ?? rawKey; if (booleanOptions.has(key)) { @@ -38,7 +42,11 @@ export function parseArgs(argv, config = {}) { if (nextValue === undefined) { throw new Error(`Missing value for --${rawKey}`); } - options[key] = nextValue; + if (repeatableValueOptions.has(key)) { + options[key] = [...(options[key] ?? []), nextValue]; + } else { + options[key] = nextValue; + } if (inlineValue === undefined) { index += 1; } @@ -62,7 +70,11 @@ export function parseArgs(argv, config = {}) { if (nextValue === undefined) { throw new Error(`Missing value for -${shortKey}`); } - options[key] = nextValue; + if (repeatableValueOptions.has(key)) { + options[key] = [...(options[key] ?? []), nextValue]; + } else { + options[key] = nextValue; + } index += 1; continue; } @@ -77,29 +89,32 @@ export function splitRawArgumentString(raw) { const tokens = []; let current = ""; let quote = null; - let escaping = false; - for (const character of raw) { - if (escaping) { - current += character; - escaping = false; - continue; - } - - if (character === "\\") { - escaping = true; - continue; - } + for (let index = 0; index < raw.length; index += 1) { + const character = raw[index]; if (quote) { if (character === quote) { quote = null; + } else if (quote === "\"" && character === "\\" && (raw[index + 1] === "\"" || raw[index + 1] === "\\")) { + current += raw[index + 1]; + index += 1; } else { current += character; } continue; } + if (character === "\\") { + if (raw[index + 1] === undefined) { + current += "\\"; + } else { + current += raw[index + 1]; + index += 1; + } + continue; + } + if (character === "'" || character === "\"") { quote = character; continue; @@ -115,11 +130,6 @@ export function splitRawArgumentString(raw) { current += character; } - - if (escaping) { - current += "\\"; - } - if (current) { tokens.push(current); } diff --git a/plugins/codex/scripts/lib/render.mjs b/plugins/codex/scripts/lib/render.mjs index 2ec185236..3403571fb 100644 --- a/plugins/codex/scripts/lib/render.mjs +++ b/plugins/codex/scripts/lib/render.mjs @@ -21,7 +21,7 @@ function formatLineRange(finding) { return `:${finding.line_start}-${finding.line_end}`; } -function validateReviewResultShape(data) { +export function validateReviewResultShape(data) { if (!data || typeof data !== "object" || Array.isArray(data)) { return "Expected a top-level JSON object."; } diff --git a/plugins/codex/scripts/lib/tracked-jobs.mjs b/plugins/codex/scripts/lib/tracked-jobs.mjs index 902869012..ca2c86fea 100644 --- a/plugins/codex/scripts/lib/tracked-jobs.mjs +++ b/plugins/codex/scripts/lib/tracked-jobs.mjs @@ -4,6 +4,10 @@ import process from "node:process"; import { readJobFile, resolveJobFile, resolveJobLogFile, upsertJob, writeJobFile } from "./state.mjs"; export const SESSION_ID_ENV = "CODEX_COMPANION_SESSION_ID"; +const JOB_LOCK_WAIT_MS = 5; +const JOB_LOCK_TIMEOUT_MS = 1000; +const JOB_LOCK_STALE_MS = 10000; +const jobLockWaitArray = new Int32Array(new SharedArrayBuffer(4)); export function nowIso() { return new Date().toISOString(); @@ -99,17 +103,19 @@ export function createJobProgressUpdater(workspaceRoot, jobId) { return; } - upsertJob(workspaceRoot, patch); - - const jobFile = resolveJobFile(workspaceRoot, jobId); - if (!fs.existsSync(jobFile)) { - return; - } - - const storedJob = readJobFile(jobFile); - writeJobFile(workspaceRoot, jobId, { - ...storedJob, - ...patch + withJobLock(workspaceRoot, jobId, () => { + const storedJob = readStoredJobOrNull(workspaceRoot, jobId); + if (!storedJob || ["cancelled", "completed", "failed"].includes(storedJob.status)) { + return; + } + writeJobFile(workspaceRoot, jobId, { + ...storedJob, + ...patch + }); + upsertJob(workspaceRoot, { + ...patch, + status: storedJob.status + }); }); }; } @@ -139,66 +145,183 @@ function readStoredJobOrNull(workspaceRoot, jobId) { return readJobFile(jobFile); } +function withJobLock(workspaceRoot, jobId, operation) { + const jobFile = resolveJobFile(workspaceRoot, jobId); + const lockFile = `${jobFile}.lock`; + const deadline = Date.now() + JOB_LOCK_TIMEOUT_MS; + let descriptor; + + while (descriptor == null) { + try { + descriptor = fs.openSync(lockFile, "wx"); + } catch (error) { + if (error?.code !== "EEXIST") { + throw error; + } + try { + if (Date.now() - fs.statSync(lockFile).mtimeMs > JOB_LOCK_STALE_MS) { + fs.unlinkSync(lockFile); + continue; + } + } catch (statError) { + if (statError?.code !== "ENOENT") { + throw statError; + } + } + if (Date.now() >= deadline) { + throw new Error(`Timed out waiting for job ${jobId} state transition.`); + } + Atomics.wait(jobLockWaitArray, 0, 0, JOB_LOCK_WAIT_MS); + } + } + + try { + return operation(); + } finally { + fs.closeSync(descriptor); + fs.unlinkSync(lockFile); + } +} + +function writeRunningJob(job, options) { + return withJobLock(job.workspaceRoot, job.id, () => { + const storedJob = readStoredJobOrNull(job.workspaceRoot, job.id); + if (storedJob?.status === "cancelled") { + return null; + } + const runningRecord = { + ...job, + ...storedJob, + status: "running", + startedAt: nowIso(), + phase: "starting", + pid: process.pid, + logFile: options.logFile ?? storedJob?.logFile ?? job.logFile ?? null + }; + writeJobFile(job.workspaceRoot, job.id, runningRecord); + upsertJob(job.workspaceRoot, runningRecord); + return runningRecord; + }); +} + +export function cancelTrackedJob(workspaceRoot, jobId, patch) { + return withJobLock(workspaceRoot, jobId, () => { + const storedJob = readStoredJobOrNull(workspaceRoot, jobId) ?? patch; + if (!["queued", "running"].includes(storedJob.status)) { + return null; + } + const cancelledJob = { + ...storedJob, + ...patch, + status: "cancelled", + phase: "cancelled", + pid: null + }; + writeJobFile(workspaceRoot, jobId, cancelledJob); + upsertJob(workspaceRoot, { + id: jobId, + status: "cancelled", + phase: "cancelled", + pid: null, + errorMessage: cancelledJob.errorMessage ?? null, + completedAt: cancelledJob.completedAt ?? null + }); + return { previous: storedJob, job: cancelledJob }; + }); +} + +export function recordQueuedJobPid(workspaceRoot, jobId, pid) { + if (!Number.isInteger(pid) || pid <= 0) { + return false; + } + return withJobLock(workspaceRoot, jobId, () => { + const storedJob = readStoredJobOrNull(workspaceRoot, jobId); + if (storedJob?.status === "running") { + return storedJob.pid === pid; + } + if (storedJob?.status !== "queued") { + return false; + } + const queuedJob = { ...storedJob, pid }; + writeJobFile(workspaceRoot, jobId, queuedJob); + upsertJob(workspaceRoot, { id: jobId, status: "queued", pid }); + return true; + }); +} + export async function runTrackedJob(job, runner, options = {}) { - const runningRecord = { - ...job, - status: "running", - startedAt: nowIso(), - phase: "starting", - pid: process.pid, - logFile: options.logFile ?? job.logFile ?? null - }; - writeJobFile(job.workspaceRoot, job.id, runningRecord); - upsertJob(job.workspaceRoot, runningRecord); + const runningRecord = writeRunningJob(job, options); + if (!runningRecord) { + return null; + } try { const execution = await runner(); const completionStatus = execution.exitStatus === 0 ? "completed" : "failed"; const completedAt = nowIso(); - writeJobFile(job.workspaceRoot, job.id, { - ...runningRecord, - status: completionStatus, - threadId: execution.threadId ?? null, - turnId: execution.turnId ?? null, - pid: null, - phase: completionStatus === "completed" ? "done" : "failed", - completedAt, - result: execution.payload, - rendered: execution.rendered - }); - upsertJob(job.workspaceRoot, { - id: job.id, - status: completionStatus, - threadId: execution.threadId ?? null, - turnId: execution.turnId ?? null, - summary: execution.summary, - phase: completionStatus === "completed" ? "done" : "failed", - pid: null, - completedAt + const recorded = withJobLock(job.workspaceRoot, job.id, () => { + const storedJob = readStoredJobOrNull(job.workspaceRoot, job.id); + if (storedJob?.status === "cancelled") { + return false; + } + writeJobFile(job.workspaceRoot, job.id, { + ...runningRecord, + ...storedJob, + status: completionStatus, + threadId: execution.threadId ?? null, + turnId: execution.turnId ?? null, + pid: null, + phase: completionStatus === "completed" ? "done" : "failed", + completedAt, + result: execution.payload, + rendered: execution.rendered + }); + upsertJob(job.workspaceRoot, { + id: job.id, + status: completionStatus, + threadId: execution.threadId ?? null, + turnId: execution.turnId ?? null, + summary: execution.summary, + phase: completionStatus === "completed" ? "done" : "failed", + pid: null, + completedAt + }); + return true; }); - appendLogBlock(options.logFile ?? job.logFile ?? null, "Final output", execution.rendered); + if (recorded) { + appendLogBlock(options.logFile ?? job.logFile ?? null, "Final output", execution.rendered); + } return execution; } catch (error) { const errorMessage = error instanceof Error ? error.message : String(error); - const existing = readStoredJobOrNull(job.workspaceRoot, job.id) ?? runningRecord; const completedAt = nowIso(); - writeJobFile(job.workspaceRoot, job.id, { - ...existing, - status: "failed", - phase: "failed", - errorMessage, - pid: null, - completedAt, - logFile: options.logFile ?? job.logFile ?? existing.logFile ?? null - }); - upsertJob(job.workspaceRoot, { - id: job.id, - status: "failed", - phase: "failed", - pid: null, - errorMessage, - completedAt + const recorded = withJobLock(job.workspaceRoot, job.id, () => { + const existing = readStoredJobOrNull(job.workspaceRoot, job.id) ?? runningRecord; + if (existing.status === "cancelled") { + return false; + } + writeJobFile(job.workspaceRoot, job.id, { + ...existing, + status: "failed", + phase: "failed", + errorMessage, + pid: null, + completedAt, + logFile: options.logFile ?? job.logFile ?? existing.logFile ?? null + }); + upsertJob(job.workspaceRoot, { + id: job.id, + status: "failed", + phase: "failed", + pid: null, + errorMessage, + completedAt + }); + return true; }); + if (!recorded) { + return null; + } throw error; } } diff --git a/plugins/codex/scripts/lib/verified-review-input.mjs b/plugins/codex/scripts/lib/verified-review-input.mjs new file mode 100644 index 000000000..64914892a --- /dev/null +++ b/plugins/codex/scripts/lib/verified-review-input.mjs @@ -0,0 +1,246 @@ +import { randomUUID } from "node:crypto"; +import fs from "node:fs"; +import path from "node:path"; + +import { resolveStateDir } from "./state.mjs"; +import { resolveWorkspaceRoot } from "./workspace.mjs"; + +const CAPTURE_VERSION = 1; +const CAPTURES_DIR_NAME = "verified-review-inputs"; +const CAPTURE_TTL_MS = 10 * 60 * 1000; +export const VERIFIED_REVIEW_CAPTURE_ID_PATTERN = + /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; +export const VERIFIED_REVIEW_INPUT_MARKER = "CODEX_VERIFIED_REVIEW_CAPTURE_ID"; + +function canonicalWorkspaceRoot(cwd) { + const workspaceRoot = resolveWorkspaceRoot(cwd); + try { + return fs.realpathSync.native(workspaceRoot); + } catch { + return workspaceRoot; + } +} + +function capturesDir(cwd) { + return path.join(path.dirname(resolveStateDir(cwd)), CAPTURES_DIR_NAME); +} + +function captureFile(cwd, captureId) { + return path.join(capturesDir(cwd), `${captureId}.json`); +} + +function ensureCapturesDir(cwd) { + const directory = capturesDir(cwd); + fs.mkdirSync(directory, { recursive: true, mode: 0o700 }); + fs.chmodSync(directory, 0o700); + return directory; +} + +function isCaptureName(name) { + const id = path.basename(name, ".json"); + return name.endsWith(".json") && VERIFIED_REVIEW_CAPTURE_ID_PATTERN.test(id); +} + +function isClaimedCaptureName(name) { + const match = /^\.([0-9a-f-]{36})\..+\.claimed$/i.exec(name); + return Boolean(match && VERIFIED_REVIEW_CAPTURE_ID_PATTERN.test(match[1])); +} + +function isExpiredRecord(record, now = Date.now()) { + return Number.isFinite(record?.createdAt) && record.createdAt + CAPTURE_TTL_MS <= now; +} + +function isMalformedRecord(record) { + return !record || typeof record !== "object" || !Number.isFinite(record.createdAt); +} + +function removeFile(filePath) { + try { + fs.unlinkSync(filePath); + return true; + } catch (error) { + if (error?.code === "ENOENT") { + return false; + } + throw error; + } +} + +function removeMalformedFileIfExpired(filePath, now) { + try { + return fs.statSync(filePath).mtimeMs + CAPTURE_TTL_MS <= now && removeFile(filePath); + } catch (error) { + if (error?.code === "ENOENT") { + return false; + } + throw error; + } +} + +function pruneExpiredCaptureRecords(cwd) { + let names; + try { + names = fs.readdirSync(capturesDir(cwd)); + } catch (error) { + if (error?.code === "ENOENT") { + return 0; + } + throw error; + } + + const now = Date.now(); + let removed = 0; + for (const name of names) { + if (!isCaptureName(name) && !isClaimedCaptureName(name)) { + continue; + } + + const filePath = path.join(capturesDir(cwd), name); + try { + const record = JSON.parse(fs.readFileSync(filePath, "utf8")); + if (isExpiredRecord(record, now) && removeFile(filePath)) { + removed += 1; + } else if (isMalformedRecord(record) && removeMalformedFileIfExpired(filePath, now)) { + removed += 1; + } + } catch (error) { + if (error?.code === "ENOENT") { + continue; + } + if (removeMalformedFileIfExpired(filePath, now)) { + removed += 1; + } + } + } + return removed; +} + +function isRecordForContext(record, { cwd, sessionId, captureId }) { + return ( + record && + record.version === CAPTURE_VERSION && + record.id === captureId && + record.sessionId === sessionId && + record.workspaceRoot === canonicalWorkspaceRoot(cwd) && + Number.isFinite(record.createdAt) && + typeof record.rawArguments === "string" + ); +} + +export function isVerifiedReviewCommand(value) { + return /^(?:codex:)?verified-review$/.test(String(value ?? "")); +} + +export function captureVerifiedReviewInput({ cwd, sessionId, rawArguments }) { + if (!sessionId) { + throw new Error("A Claude session ID is required to capture verified-review arguments."); + } + if (typeof rawArguments !== "string") { + throw new Error("Verified-review command arguments must be a string."); + } + + pruneExpiredCaptureRecords(cwd); + + const id = randomUUID(); + const filePath = captureFile(cwd, id); + const record = { + version: CAPTURE_VERSION, + id, + sessionId: String(sessionId), + workspaceRoot: canonicalWorkspaceRoot(cwd), + createdAt: Date.now(), + rawArguments + }; + + ensureCapturesDir(cwd); + const file = fs.openSync(filePath, "wx", 0o600); + try { + fs.writeFileSync(file, `${JSON.stringify(record)}\n`, "utf8"); + } finally { + fs.closeSync(file); + } + + return { id }; +} + +export function consumeVerifiedReviewInput(cwd, captureId, { sessionId } = {}) { + if (!sessionId || !VERIFIED_REVIEW_CAPTURE_ID_PATTERN.test(String(captureId ?? ""))) { + return null; + } + + pruneExpiredCaptureRecords(cwd); + + const normalizedId = String(captureId); + const source = captureFile(cwd, normalizedId); + const claimed = path.join(capturesDir(cwd), `.${normalizedId}.${process.pid}.${randomUUID()}.claimed`); + + try { + fs.renameSync(source, claimed); + } catch (error) { + if (error?.code === "ENOENT") { + return null; + } + throw error; + } + + try { + const record = JSON.parse(fs.readFileSync(claimed, "utf8")); + if ( + isExpiredRecord(record) || + !isRecordForContext(record, { cwd, sessionId: String(sessionId), captureId: normalizedId }) + ) { + return null; + } + return { rawArguments: record.rawArguments }; + } catch { + return null; + } finally { + removeFile(claimed); + } +} + +export function cleanupVerifiedReviewInputs({ cwd, sessionId }) { + if (!sessionId) { + return 0; + } + + pruneExpiredCaptureRecords(cwd); + + let names; + try { + names = fs.readdirSync(capturesDir(cwd)); + } catch (error) { + if (error?.code === "ENOENT") { + return 0; + } + throw error; + } + + const normalizedSessionId = String(sessionId); + let removed = 0; + for (const name of names) { + if (!isCaptureName(name)) { + continue; + } + + const filePath = path.join(capturesDir(cwd), name); + try { + const record = JSON.parse(fs.readFileSync(filePath, "utf8")); + if (record.sessionId !== normalizedSessionId) { + continue; + } + if (removeFile(filePath)) { + removed += 1; + } + } catch (error) { + if (error?.code === "ENOENT") { + continue; + } + if (removeMalformedFileIfExpired(filePath, Date.now())) { + removed += 1; + } + } + } + + return removed; +} diff --git a/plugins/codex/scripts/session-lifecycle-hook.mjs b/plugins/codex/scripts/session-lifecycle-hook.mjs index 778571e6c..8b14957f3 100644 --- a/plugins/codex/scripts/session-lifecycle-hook.mjs +++ b/plugins/codex/scripts/session-lifecycle-hook.mjs @@ -15,6 +15,8 @@ import { } from "./lib/broker-lifecycle.mjs"; import { loadState, resolveStateFile, saveState } from "./lib/state.mjs"; import { TRANSCRIPT_PATH_ENV } from "./lib/claude-session-transfer.mjs"; +import { cleanupVerifiedReviewInputs } from "./lib/verified-review-input.mjs"; +import { cancelTrackedJob } from "./lib/tracked-jobs.mjs"; import { resolveWorkspaceRoot } from "./lib/workspace.mjs"; export const SESSION_ID_ENV = "CODEX_COMPANION_SESSION_ID"; @@ -61,8 +63,9 @@ function cleanupSessionJobs(cwd, sessionId) { if (!stillRunning) { continue; } + const cancellation = cancelTrackedJob(workspaceRoot, job.id, job); try { - terminateProcessTree(job.pid ?? Number.NaN); + terminateProcessTree(cancellation?.previous.pid ?? job.pid ?? Number.NaN); } catch { // Ignore teardown failures during session shutdown. } @@ -102,6 +105,10 @@ async function handleSessionEnd(input) { } cleanupSessionJobs(cwd, input.session_id || process.env[SESSION_ID_ENV]); + cleanupVerifiedReviewInputs({ + cwd, + sessionId: input.session_id || process.env[SESSION_ID_ENV] + }); teardownBrokerSession({ endpoint: brokerEndpoint, pidFile, diff --git a/plugins/codex/scripts/verified-review-input-hook.mjs b/plugins/codex/scripts/verified-review-input-hook.mjs new file mode 100644 index 000000000..2e96504fa --- /dev/null +++ b/plugins/codex/scripts/verified-review-input-hook.mjs @@ -0,0 +1,43 @@ +#!/usr/bin/env node + +import fs from "node:fs"; +import process from "node:process"; + +import { + VERIFIED_REVIEW_INPUT_MARKER, + captureVerifiedReviewInput, + isVerifiedReviewCommand +} from "./lib/verified-review-input.mjs"; + +function readHookInput() { + const raw = fs.readFileSync(0, "utf8").trim(); + return raw ? JSON.parse(raw) : {}; +} + +function main() { + const input = readHookInput(); + if (input.hook_event_name !== "UserPromptExpansion" || !isVerifiedReviewCommand(input.command_name)) { + return; + } + + const capture = captureVerifiedReviewInput({ + cwd: input.cwd || process.env.CLAUDE_PROJECT_DIR || process.cwd(), + sessionId: input.session_id, + rawArguments: input.command_args + }); + process.stdout.write( + `${JSON.stringify({ + hookSpecificOutput: { + hookEventName: "UserPromptExpansion", + additionalContext: `${VERIFIED_REVIEW_INPUT_MARKER}=${capture.id}` + } + })}\n` + ); +} + +try { + main(); +} catch (error) { + process.stderr.write(`${error instanceof Error ? error.message : String(error)}\n`); + process.exit(2); +} diff --git a/tests/args.test.mjs b/tests/args.test.mjs new file mode 100644 index 000000000..5e74b16d4 --- /dev/null +++ b/tests/args.test.mjs @@ -0,0 +1,45 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { parseArgs, splitRawArgumentString } from "../plugins/codex/scripts/lib/args.mjs"; + +test("parseArgs preserves all equals signs in inline values", () => { + assert.deepEqual( + parseArgs(["--check=FOO=bar npm test", "--check=KEY=value=again"], { + valueOptions: ["check"], + repeatableValueOptions: ["check"] + }), + { + options: { check: ["FOO=bar npm test", "KEY=value=again"] }, + positionals: [] + } + ); +}); + +test("parseArgs keeps empty inline values and separate values", () => { + assert.deepEqual( + parseArgs(["--value=", "--other", "plain"], { valueOptions: ["value", "other"] }), + { options: { value: "", other: "plain" }, positionals: [] } + ); +}); + +test("splitRawArgumentString preserves regex backslashes inside a quoted check command", () => { + assert.deepEqual( + splitRawArgumentString(`--check "rg '\\bfoo\\b'"`), + ["--check", "rg '\\bfoo\\b'"] + ); +}); + +test("splitRawArgumentString retains quoted values and unquoted escaped spaces", () => { + assert.deepEqual( + splitRawArgumentString(`--base "origin/main" focus\\ text --check "npm test"`), + ["--base", "origin/main", "focus text", "--check", "npm test"] + ); +}); + +test("splitRawArgumentString strips double-quote escape syntax without expanding commands", () => { + assert.deepEqual( + splitRawArgumentString(`--check "printf \\\"ok\\\""`), + ["--check", "printf \"ok\""] + ); +}); diff --git a/tests/commands.test.mjs b/tests/commands.test.mjs index c34b06059..56a1cfd9a 100644 --- a/tests/commands.test.mjs +++ b/tests/commands.test.mjs @@ -70,6 +70,41 @@ test("adversarial review command uses AskUserQuestion and background Bash while assert.match(source, /can still take extra focus text after the flags/i); }); +test("verified review command exposes the two-pass read-only contract", () => { + const source = read("commands/verified-review.md"); + const readme = fs.readFileSync(path.join(ROOT, "README.md"), "utf8"); + const executableBlocks = [...source.matchAll(/```(?:bash|typescript)\n([\s\S]*?)```/g)].map((match) => match[1]); + + assert.match(source, /AskUserQuestion/); + assert.match(source, /\bBash\(/); + assert.match(source, /CODEX_VERIFIED_REVIEW_CAPTURE_ID=/); + assert.match(source, /fail closed/i); + assert.match(source, /--captured-input ""/); + assert.match(source, /Never copy, interpolate, export, pipe, or otherwise place raw `\$ARGUMENTS` in Bash/i); + assert.doesNotMatch(source, /git diff --shortstat \.\.\.HEAD/); + assert.match(source, /If raw arguments explicitly select a branch with `--base` or `--scope branch`, never run Bash to size that branch; recommend background/i); + assert.match(source, /Never copy or interpolate a raw base or ref into Bash/i); + assert.match(source, /git status --short --untracked-files=all/); + assert.match(source, /git diff --shortstat --cached/); + assert.match(source, /If the working tree is clean, the companion will fall back to branch review, or the size is unclear, recommend background/i); + assert.equal(executableBlocks.length, 2); + for (const block of executableBlocks) { + assert.match(block, /verified-review --captured-input ""/); + assert.doesNotMatch(block, /\$ARGUMENTS/); + assert.doesNotMatch(block, /|/); + } + assert.doesNotMatch(source, /verified-review "\$ARGUMENTS"/); + assert.match(source, /\[--scope auto\|working-tree\|branch\]/); + assert.match(source, /\[--base \]/); + assert.match(source, /\[--check ""\]/); + assert.match(source, /run_in_background:\s*true/); + assert.match(source, /Do not fix issues/i); + assert.match(source, /read-only/i); + assert.match(source, /Return the command stdout verbatim to the user/i); + assert.match(readme, /### `\/codex:verified-review`/); + assert.match(readme, /--check/i); +}); + test("continue is not exposed as a user-facing command", () => { const commandFiles = fs.readdirSync(path.join(PLUGIN_ROOT, "commands")).sort(); assert.deepEqual(commandFiles, [ @@ -80,7 +115,8 @@ test("continue is not exposed as a user-facing command", () => { "review.md", "setup.md", "status.md", - "transfer.md" + "transfer.md", + "verified-review.md" ]); }); diff --git a/tests/fake-codex-fixture.mjs b/tests/fake-codex-fixture.mjs index f83c96a0d..42686cccb 100644 --- a/tests/fake-codex-fixture.mjs +++ b/tests/fake-codex-fixture.mjs @@ -181,6 +181,56 @@ function emitTurnCompletedLater(threadId, turnId, item, delayMs) { } function nativeReviewText(target) { + if (BEHAVIOR === "verified-review-native-empty") { + return ""; + } + if (BEHAVIOR === "verified-review-native-clean") { + return "No material issues found."; + } + if (BEHAVIOR === "verified-review-native-clean-after-heading") { + return "Reviewed uncommitted changes.\\nNo material issues found."; + } + if (BEHAVIOR === "verified-review-native-ambiguous") { + return "Looks good, but X is broken"; + } + if (BEHAVIOR === "verified-review-native-none") { + return "- None."; + } + if (BEHAVIOR === "verified-review-native-no-findings") { + return "No findings."; + } + if (BEHAVIOR === "verified-review-native-no-issues") { + return "No issues."; + } + if (BEHAVIOR === "verified-review-native-no-problems") { + return "No problems."; + } + if (BEHAVIOR === "verified-review-native-clean-mixed") { + return "No material issues found.\\nBut src/app.js can still throw."; + } + if (BEHAVIOR === "verified-review-native-nested-and-fenced") { + return [ + "Reviewed uncommitted changes.", + "- [high] Missing empty-state guard (src/app.js:4)", + " - Nested explanation is not another finding.", + " \`\`\`diff", + " - removed code is not a finding", + " + added code is not a finding", + " [P1] code marker is not a finding", + " \`\`\`", + "- [low] Naming could be clearer (src/app.js:1)", + " 1. Nested list item is not another finding.", + "~~~text", + "- fenced marker is not a finding", + "+ fenced marker is not a finding", + "1. fenced marker is not a finding", + "[P2] fenced marker is not a finding", + "~~~" + ].join("\\n"); + } + if (BEHAVIOR.startsWith("verified-review-")) { + return "Reviewed uncommitted changes.\\n- [high] Missing empty-state guard (src/app.js:4)\\n- [low] Naming could be clearer (src/app.js:1)"; + } if (target.type === "baseBranch") { return "Reviewed changes against " + target.branch + ".\\nNo material issues found."; } @@ -191,6 +241,72 @@ function nativeReviewText(target) { } function structuredReviewPayload(prompt) { + if (BEHAVIOR === "verified-review-invalid-shape") { + return JSON.stringify({ + verdict: "approve", + summary: "Verifier omitted a required field.", + findings: [] + }); + } + + if (BEHAVIOR.startsWith("verified-review-")) { + const emptyNativeFindings = new Set([ + "verified-review-native-empty", + "verified-review-native-clean", + "verified-review-native-clean-after-heading", + "verified-review-native-none", + "verified-review-native-no-findings", + "verified-review-native-no-issues", + "verified-review-native-no-problems", + "verified-review-native-clean-mixed" + ]); + const nativeFindingIds = + BEHAVIOR === "verified-review-missing-finding" + ? ["native-1"] + : BEHAVIOR === "verified-review-duplicate-finding" + ? ["native-1", "native-1"] + : BEHAVIOR === "verified-review-unknown-finding" + ? ["native-1", "native-99"] + : ["native-1", "native-2"]; + const findings = (emptyNativeFindings.has(BEHAVIOR) ? [] : [ + { + native_finding_id: nativeFindingIds[0], + severity: "high", + title: "[confirmed] Missing empty-state guard", + body: "The unguarded index access can throw for an empty collection.", + file: "src/app.js", + line_start: 4, + line_end: 4, + confidence: 0.95, + recommendation: "Handle empty collections before indexing." + }, + { + native_finding_id: nativeFindingIds[1], + severity: "low", + title: "[style-only] Naming could be clearer", + body: "This is readability-only and has no behavior impact.", + file: "src/app.js", + line_start: 1, + line_end: 1, + confidence: 0.8, + recommendation: "Rename when editing this code next." + } + ]).filter((finding) => finding.native_finding_id); + if (BEHAVIOR === "verified-review-invalid-finding-shape") { + delete findings[0].body; + } + if (BEHAVIOR === "verified-review-inverted-line-range") { + findings[0].line_start = 20; + findings[0].line_end = 10; + } + return JSON.stringify({ + verdict: "needs-attention", + summary: "Every native finding was independently classified.", + findings, + next_steps: ["Fix the confirmed empty-state guard."] + }); + } + if (prompt.includes("adversarial software review")) { if (BEHAVIOR === "adversarial-clean") { return JSON.stringify({ @@ -232,6 +348,91 @@ function structuredReviewPayload(prompt) { }); } +function verifiedReviewCheckItems(turnId, prompt) { + const explicitChecksStart = prompt.indexOf(""); + const explicitChecksEnd = prompt.indexOf(""); + const hasRequestedCheck = + explicitChecksStart >= 0 && + explicitChecksEnd > explicitChecksStart && + prompt.slice(explicitChecksStart, explicitChecksEnd).includes("- npm test -- --runInBand"); + if (!BEHAVIOR.startsWith("verified-review-") || !prompt.includes("Execute only these explicitly supplied commands")) { + return []; + } + + const items = [ + { + started: { + type: "commandExecution", + id: "command_" + turnId + "_inspection", + command: "git diff --stat", + status: "inProgress" + }, + completed: { + type: "commandExecution", + id: "command_" + turnId + "_inspection", + command: "git diff --stat", + status: "completed", + exitCode: 0, + aggregatedOutput: " src/app.js | 2 + -", + commandActions: ["read"] + } + } + ]; + + if (BEHAVIOR === "verified-review-check-readonly-inspection") { + items.push({ + started: { + type: "commandExecution", + id: "command_" + turnId + "_inspection_check", + command: "git diff --check", + status: "inProgress" + }, + completed: { + type: "commandExecution", + id: "command_" + turnId + "_inspection_check", + command: "git diff --check", + status: "completed", + exitCode: 0, + aggregatedOutput: "", + commandActions: ["read"] + } + }); + } + + if (!hasRequestedCheck) { + return items; + } + + if (BEHAVIOR === "verified-review-check-skipped") { + return items; + } + + const commands = + BEHAVIOR === "verified-review-check-duplicate" + ? ["npm test -- --runInBand", "npm test -- --runInBand"] + : BEHAVIOR === "verified-review-unauthorized-check" + ? ["npm test -- --runInBand", "npm run build"] + : ["npm test -- --runInBand"]; + + return [...items, ...commands.map((command, index) => ({ + started: { + type: "commandExecution", + id: "command_" + turnId + "_" + index, + command, + status: "inProgress" + }, + completed: { + type: "commandExecution", + id: "command_" + turnId + "_" + index, + command, + status: "completed", + exitCode: 0, + aggregatedOutput: "all tests passed", + commandActions: command === "npm run build" ? ["execute"] : ["read"] + } + }))]; +} + function taskPayload(prompt, resume) { if (prompt.includes("") && prompt.includes("Only review the work from the previous Claude turn.")) { if (BEHAVIOR === "adversarial-clean") { @@ -406,6 +607,15 @@ rl.on("line", (line) => { case "review/start": { const thread = ensureThread(state, message.params.threadId); + state.reviewStarts = [ + ...(state.reviewStarts || []), + { + threadId: message.params.threadId, + delivery: message.params.delivery, + target: message.params.target + } + ]; + saveState(state); let reviewThread = thread; if (message.params.delivery === "detached") { reviewThread = nextThread(state, thread.cwd, true); @@ -444,13 +654,15 @@ rl.on("line", (line) => { .join("\\n"); const turnId = nextTurnId(state); thread.updatedAt = now(); - state.lastTurnStart = { + const turnStart = { threadId: message.params.threadId, turnId, model: message.params.model ?? null, effort: message.params.effort ?? null, prompt }; + state.lastTurnStart = turnStart; + state.turnStarts = [...(state.turnStarts || []), turnStart]; saveState(state); send({ id: message.id, result: { turn: buildTurn(turnId) } }); @@ -567,8 +779,9 @@ rl.on("line", (line) => { break; } - const items = [ - ...(BEHAVIOR === "with-reasoning" + const items = [ + ...verifiedReviewCheckItems(turnId, prompt), + ...(BEHAVIOR === "with-reasoning" ? [ { completed: { diff --git a/tests/runtime.test.mjs b/tests/runtime.test.mjs index 8f276835b..2b7076aca 100644 --- a/tests/runtime.test.mjs +++ b/tests/runtime.test.mjs @@ -8,13 +8,25 @@ import { fileURLToPath } from "node:url"; import { buildEnv, installFakeCodex } from "./fake-codex-fixture.mjs"; import { initGitRepo, makeTempDir, run } from "./helpers.mjs"; import { loadBrokerSession, saveBrokerSession } from "../plugins/codex/scripts/lib/broker-lifecycle.mjs"; -import { resolveStateDir } from "../plugins/codex/scripts/lib/state.mjs"; +import { readJobFile, resolveJobFile, resolveStateDir, upsertJob, writeJobFile } from "../plugins/codex/scripts/lib/state.mjs"; +import { + cancelTrackedJob, + createJobProgressUpdater, + recordQueuedJobPid, + runTrackedJob +} from "../plugins/codex/scripts/lib/tracked-jobs.mjs"; +import { + captureVerifiedReviewInput, + cleanupVerifiedReviewInputs, + consumeVerifiedReviewInput +} from "../plugins/codex/scripts/lib/verified-review-input.mjs"; const ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), ".."); const PLUGIN_ROOT = path.join(ROOT, "plugins", "codex"); const SCRIPT = path.join(PLUGIN_ROOT, "scripts", "codex-companion.mjs"); const STOP_HOOK = path.join(PLUGIN_ROOT, "scripts", "stop-review-gate-hook.mjs"); const SESSION_HOOK = path.join(PLUGIN_ROOT, "scripts", "session-lifecycle-hook.mjs"); +const VERIFIED_REVIEW_INPUT_HOOK = path.join(PLUGIN_ROOT, "scripts", "verified-review-input-hook.mjs"); async function waitFor(predicate, { timeoutMs = 5000, intervalMs = 50 } = {}) { const start = Date.now(); @@ -28,6 +40,52 @@ async function waitFor(predicate, { timeoutMs = 5000, intervalMs = 50 } = {}) { throw new Error("Timed out waiting for condition."); } +function createVerifiedReviewRepo(behavior) { + const repo = makeTempDir(); + const binDir = makeTempDir(); + installFakeCodex(binDir, behavior); + initGitRepo(repo); + fs.mkdirSync(path.join(repo, "src")); + fs.writeFileSync(path.join(repo, "src", "app.js"), "export const value = 1;\n"); + run("git", ["add", "src/app.js"], { cwd: repo }); + run("git", ["commit", "-m", "init"], { cwd: repo }); + fs.writeFileSync(path.join(repo, "src", "app.js"), "export const value = items[0].id;\n"); + return { repo, binDir }; +} + +function verifiedReviewInputFiles(cwd) { + const directory = path.join(path.dirname(resolveStateDir(cwd)), "verified-review-inputs"); + return fs.existsSync(directory) ? fs.readdirSync(directory).filter((name) => name.endsWith(".json")) : []; +} + +function verifiedReviewInputDir(cwd) { + return path.join(path.dirname(resolveStateDir(cwd)), "verified-review-inputs"); +} + +function captureVerifiedReviewArguments({ cwd, env, sessionId, rawArguments }) { + const result = run("node", [VERIFIED_REVIEW_INPUT_HOOK], { + cwd, + env, + input: JSON.stringify({ + hook_event_name: "UserPromptExpansion", + command_name: "codex:verified-review", + command_args: rawArguments, + session_id: sessionId, + cwd + }) + }); + assert.equal(result.status, 0, result.stderr); + const context = JSON.parse(result.stdout).hookSpecificOutput.additionalContext; + const match = /^CODEX_VERIFIED_REVIEW_CAPTURE_ID=([0-9a-f-]{36})$/i.exec(context); + assert.ok(match, `Missing verified-review capture ID in ${context}`); + return match[1]; +} + +function fakeCodexState(binDir) { + const filePath = path.join(binDir, "fake-codex-state.json"); + return fs.existsSync(filePath) ? JSON.parse(fs.readFileSync(filePath, "utf8")) : null; +} + test("setup reports ready when fake codex is installed and authenticated", () => { const binDir = makeTempDir(); installFakeCodex(binDir); @@ -1062,6 +1120,608 @@ test("review accepts --background while still running as a tracked review job", assert.match(status.stdout, /completed/); }); +test("verified review runs one native pass, then one fresh read-only verification pass with explicit check evidence", () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + const statePath = path.join(binDir, "fake-codex-state.json"); + installFakeCodex(binDir, "verified-review-findings"); + initGitRepo(repo); + fs.mkdirSync(path.join(repo, "src")); + fs.writeFileSync(path.join(repo, "src", "app.js"), "export const value = 1;\n"); + run("git", ["add", "src/app.js"], { cwd: repo }); + run("git", ["commit", "-m", "init"], { cwd: repo }); + const sourceBefore = "export const value = items[0].id;\n"; + fs.writeFileSync(path.join(repo, "src", "app.js"), sourceBefore); + + const result = run( + "node", + [SCRIPT, "verified-review", "--scope", "working-tree", "--check", "npm test -- --runInBand", "--json"], + { cwd: repo, env: buildEnv(binDir) } + ); + + assert.equal(result.status, 0, result.stderr); + assert.equal(fs.readFileSync(path.join(repo, "src", "app.js"), "utf8"), sourceBefore); + const payload = JSON.parse(result.stdout); + assert.equal(payload.review, "Verified Review"); + assert.equal(payload.native.stdout.includes("Missing empty-state guard"), true); + assert.deepEqual(payload.native.findings.map((finding) => finding.id), ["native-1", "native-2"]); + assert.deepEqual(payload.verification.requestedChecks, ["npm test -- --runInBand"]); + assert.equal(payload.verification.checks.length, 1); + assert.deepEqual(payload.verification.checks.map((check) => check.command), ["npm test -- --runInBand"]); + assert.deepEqual(payload.verification.inspectionExecutions.map((execution) => execution.command), ["git diff --stat"]); + assert.deepEqual(payload.verification.missingChecks, []); + assert.deepEqual(payload.verification.duplicateChecks, []); + assert.equal(payload.verification.checks[0].exitCode, 0); + assert.deepEqual( + payload.result.findings.map((finding) => finding.title), + ["[confirmed] Missing empty-state guard", "[style-only] Naming could be clearer"] + ); + assert.deepEqual(payload.result.findings.map((finding) => finding.native_finding_id), ["native-1", "native-2"]); + + const state = JSON.parse(fs.readFileSync(statePath, "utf8")); + assert.equal(state.reviewStarts.length, 1); + assert.equal(state.turnStarts.length, 1); + assert.notEqual(state.reviewStarts[0].threadId, state.turnStarts[0].threadId); + assert.equal(state.threads.find((thread) => thread.id === state.turnStarts[0].threadId).ephemeral, true); + assert.match(state.turnStarts[0].prompt, /npm test -- --runInBand/); + assert.match(state.turnStarts[0].prompt, /read-only/i); + + const stateDir = resolveStateDir(repo); + const jobState = JSON.parse(fs.readFileSync(path.join(stateDir, "state.json"), "utf8")); + const log = fs.readFileSync(jobState.jobs[0].logFile, "utf8"); + assert.match(log, /npm test -- --runInBand/); + assert.match(log, /exit 0/); +}); + +test("verified review captures slash arguments as opaque input before the shell can expand them", () => { + const { repo, binDir } = createVerifiedReviewRepo("verified-review-findings"); + const sessionId = "sess-captured-input"; + const marker = path.join(makeTempDir(), "must-not-exist"); + const literal = `$HOME \`never-run\` $(touch ${marker}) 'single quote' "double quote" \\\\path 한국어\nembedded line`; + const rawArguments = `--base "${literal.replaceAll("\\", "\\\\").replaceAll('"', '\\"')}"`; + const env = { + ...buildEnv(binDir), + CODEX_COMPANION_SESSION_ID: sessionId + }; + const captureId = captureVerifiedReviewArguments({ cwd: repo, env, sessionId, rawArguments }); + const captureFile = path.join(verifiedReviewInputDir(repo), `${captureId}.json`); + + assert.equal(JSON.parse(fs.readFileSync(captureFile, "utf8")).rawArguments, rawArguments); + assert.equal(fs.existsSync(marker), false); + + const result = run("node", [SCRIPT, "verified-review", "--captured-input", captureId], { cwd: repo, env }); + + assert.equal(result.status, 0, result.stderr); + assert.equal(fs.existsSync(marker), false); + assert.equal(fs.existsSync(captureFile), false); + const state = fakeCodexState(binDir); + assert.equal(state.reviewStarts.length, 1); + assert.equal(state.turnStarts.length, 1); + assert.equal(state.reviewStarts[0].target.branch, literal); +}); + +test("captured verified-review input fails closed before Codex for invalid ownership and reuse", () => { + const { repo, binDir } = createVerifiedReviewRepo("verified-review-findings"); + const ownerSession = "sess-capture-owner"; + const ownerEnv = { + ...buildEnv(binDir), + CODEX_COMPANION_SESSION_ID: ownerSession + }; + const runCaptured = (captureId, env = ownerEnv, cwd = repo) => + run("node", [SCRIPT, "verified-review", "--captured-input", captureId], { cwd, env }); + const noCodexRan = () => assert.equal(fakeCodexState(binDir), null); + + const malformed = runCaptured("not-a-capture-id"); + assert.notEqual(malformed.status, 0); + noCodexRan(); + + const missing = runCaptured("00000000-0000-4000-8000-000000000000"); + assert.notEqual(missing.status, 0); + noCodexRan(); + + const malformedId = "11111111-1111-4111-8111-111111111111"; + const captureDir = verifiedReviewInputDir(repo); + fs.mkdirSync(captureDir, { recursive: true }); + fs.writeFileSync(path.join(captureDir, `${malformedId}.json`), "not json", "utf8"); + const malformedRecord = runCaptured(malformedId); + assert.notEqual(malformedRecord.status, 0); + noCodexRan(); + + const wrongSessionId = captureVerifiedReviewInput({ + cwd: repo, + sessionId: ownerSession, + rawArguments: "--scope working-tree" + }).id; + const wrongSession = runCaptured(wrongSessionId, { + ...ownerEnv, + CODEX_COMPANION_SESSION_ID: "sess-not-owner" + }); + assert.notEqual(wrongSession.status, 0); + noCodexRan(); + + const otherWorkspace = makeTempDir(); + initGitRepo(otherWorkspace); + const wrongWorkspaceId = captureVerifiedReviewInput({ + cwd: repo, + sessionId: ownerSession, + rawArguments: "--scope working-tree" + }).id; + const wrongWorkspace = runCaptured(wrongWorkspaceId, ownerEnv, otherWorkspace); + assert.notEqual(wrongWorkspace.status, 0); + noCodexRan(); + + const reusableId = captureVerifiedReviewInput({ + cwd: repo, + sessionId: ownerSession, + rawArguments: "--scope working-tree" + }).id; + const firstUse = runCaptured(reusableId); + assert.equal(firstUse.status, 0, firstUse.stderr); + const secondUse = runCaptured(reusableId); + assert.notEqual(secondUse.status, 0); + assert.equal(fakeCodexState(binDir).reviewStarts.length, 1); + assert.equal(fakeCodexState(binDir).turnStarts.length, 1); +}); + +test("verified-review capture runtime failures exit 2 without emitting a capture marker", () => { + const repo = makeTempDir(); + const pluginData = makeTempDir(); + initGitRepo(repo); + const captureDir = path.join(pluginData, "state", "verified-review-inputs"); + fs.mkdirSync(path.dirname(captureDir), { recursive: true }); + fs.writeFileSync(captureDir, "not a directory", "utf8"); + const result = run("node", [VERIFIED_REVIEW_INPUT_HOOK], { + cwd: repo, + env: { ...process.env, CLAUDE_PLUGIN_DATA: pluginData }, + input: JSON.stringify({ + hook_event_name: "UserPromptExpansion", + command_name: "verified-review", + command_args: "--scope branch", + session_id: "sess-hook-failure", + cwd: repo + }) + }); + + assert.equal(result.status, 2, result.stderr); + assert.equal(result.stdout, ""); + assert.doesNotMatch(result.stdout, /CODEX_VERIFIED_REVIEW_CAPTURE_ID/); + assert.equal(fs.readFileSync(captureDir, "utf8"), "not a directory"); +}); + +test("captured verified-review input expires before use", () => { + const repo = makeTempDir(); + initGitRepo(repo); + const capture = captureVerifiedReviewInput({ cwd: repo, sessionId: "sess-expired", rawArguments: "--scope branch" }); + const captureFile = path.join(verifiedReviewInputDir(repo), `${capture.id}.json`); + const record = JSON.parse(fs.readFileSync(captureFile, "utf8")); + fs.writeFileSync(captureFile, `${JSON.stringify({ ...record, createdAt: Date.now() - 10 * 60 * 1000 - 1 })}\n`); + + assert.deepEqual(consumeVerifiedReviewInput(repo, capture.id, { sessionId: "sess-expired" }), null); + assert.equal(fs.existsSync(captureFile), false); +}); + +test("verified-review capture cleanup spans workspaces, retains live claims, and prunes stale claims", () => { + const repo = makeTempDir(); + const otherWorkspace = makeTempDir(); + initGitRepo(repo); + initGitRepo(otherWorkspace); + const env = { ...process.env }; + const first = captureVerifiedReviewInput({ cwd: repo, sessionId: "sess-cleanup", rawArguments: "--scope working-tree" }); + const second = captureVerifiedReviewInput({ + cwd: otherWorkspace, + sessionId: "sess-cleanup", + rawArguments: "--scope branch" + }); + const otherSession = captureVerifiedReviewInput({ + cwd: otherWorkspace, + sessionId: "sess-other", + rawArguments: "--check npm test" + }); + const stale = captureVerifiedReviewInput({ + cwd: repo, + sessionId: "sess-stale", + rawArguments: "--base main" + }); + const captureDir = verifiedReviewInputDir(repo); + const stalePath = path.join(captureDir, `${stale.id}.json`); + const staleRecord = JSON.parse(fs.readFileSync(stalePath, "utf8")); + fs.renameSync(stalePath, path.join(captureDir, `.${stale.id}.crash.claimed`)); + fs.writeFileSync( + path.join(captureDir, `.${stale.id}.crash.claimed`), + `${JSON.stringify({ ...staleRecord, createdAt: Date.now() - 10 * 60 * 1000 - 1 })}\n` + ); + const liveClaim = captureVerifiedReviewInput({ + cwd: repo, + sessionId: "sess-cleanup", + rawArguments: "--scope staged" + }); + const liveClaimPath = path.join(captureDir, `.${liveClaim.id}.active.claimed`); + fs.renameSync(path.join(captureDir, `${liveClaim.id}.json`), liveClaimPath); + const malformedId = "22222222-2222-4222-8222-222222222222"; + const malformedPath = path.join(captureDir, `.${malformedId}.crash.claimed`); + fs.writeFileSync(malformedPath, "not json", "utf8"); + const staleAt = new Date(Date.now() - 10 * 60 * 1000 - 1); + fs.utimesSync(malformedPath, staleAt, staleAt); + + assert.equal(verifiedReviewInputFiles(repo).length, 3); + assert.deepEqual(consumeVerifiedReviewInput(repo, first.id, { sessionId: "sess-cleanup" }), { + rawArguments: "--scope working-tree" + }); + const cleanup = run("node", [SESSION_HOOK, "SessionEnd"], { + cwd: repo, + env, + input: JSON.stringify({ hook_event_name: "SessionEnd", cwd: repo, session_id: "sess-cleanup" }) + }); + assert.equal(cleanup.status, 0, cleanup.stderr); + assert.equal(verifiedReviewInputFiles(repo).length, 1); + assert.deepEqual(consumeVerifiedReviewInput(otherWorkspace, second.id, { sessionId: "sess-cleanup" }), null); + assert.equal(fs.existsSync(liveClaimPath), true); + assert.equal(fs.existsSync(path.join(captureDir, `.${stale.id}.crash.claimed`)), false); + assert.equal(fs.existsSync(malformedPath), false); + assert.deepEqual(consumeVerifiedReviewInput(otherWorkspace, otherSession.id, { sessionId: "sess-other" }), { + rawArguments: "--check npm test" + }); + assert.equal(cleanupVerifiedReviewInputs({ cwd: repo, sessionId: "sess-other" }), 0); +}); + +test("verified review rejects omitted, duplicate, and unknown native finding classifications", () => { + for (const behavior of [ + "verified-review-missing-finding", + "verified-review-duplicate-finding", + "verified-review-unknown-finding" + ]) { + const { repo, binDir } = createVerifiedReviewRepo(behavior); + const result = run("node", [SCRIPT, "verified-review", "--json"], { + cwd: repo, + env: buildEnv(binDir) + }); + + assert.notEqual(result.status, 0, `${behavior} unexpectedly succeeded`); + const payload = JSON.parse(result.stdout); + assert.equal(payload.result, null); + assert.match(payload.parseError, /native finding|classification|one-to-one|duplicate|unknown/i); + } +}); + +test("verified review fails closed on schema-invalid verifier output", () => { + for (const [behavior, expectedError] of [ + ["verified-review-invalid-shape", /next_steps/], + ["verified-review-invalid-finding-shape", /findings\[0\]\.body/], + ["verified-review-inverted-line-range", /line_end.*line_start/] + ]) { + const { repo, binDir } = createVerifiedReviewRepo(behavior); + const result = run("node", [SCRIPT, "verified-review", "--json"], { + cwd: repo, + env: buildEnv(binDir) + }); + + assert.notEqual(result.status, 0); + const payload = JSON.parse(result.stdout); + assert.equal(payload.result, null); + assert.match(payload.parseError, expectedError); + } +}); + +test("verified review separates normal inspection from explicitly requested check executions", () => { + const requestedCheck = "npm test -- --runInBand"; + const cases = [ + { behavior: "verified-review-check-skipped", actual: [], missing: [requestedCheck], duplicate: [] }, + { behavior: "verified-review-check-duplicate", actual: [requestedCheck, requestedCheck], missing: [], duplicate: [requestedCheck] } + ]; + + for (const { behavior, actual, missing, duplicate } of cases) { + const { repo, binDir } = createVerifiedReviewRepo(behavior); + const result = run("node", [SCRIPT, "verified-review", "--check", requestedCheck, "--json"], { + cwd: repo, + env: buildEnv(binDir) + }); + + assert.notEqual(result.status, 0, `${behavior} unexpectedly succeeded`); + const payload = JSON.parse(result.stdout); + assert.deepEqual(payload.verification.requestedChecks, [requestedCheck]); + assert.deepEqual(payload.verification.checks.map((check) => check.command), actual); + assert.deepEqual(payload.verification.inspectionExecutions.map((execution) => execution.command), ["git diff --stat"]); + assert.deepEqual(payload.verification.missingChecks, missing); + assert.deepEqual(payload.verification.duplicateChecks, duplicate); + assert.match(payload.parseError, /explicit check|requested|missing|unexpected|duplicate/i); + } + + const { repo, binDir } = createVerifiedReviewRepo("verified-review-check-skipped"); + const rendered = run("node", [SCRIPT, "verified-review", "--check", requestedCheck], { + cwd: repo, + env: buildEnv(binDir) + }); + assert.notEqual(rendered.status, 0); + assert.match(rendered.stdout, /Requested explicit checks/i); + assert.match(rendered.stdout, /Missing explicit checks/i); + assert.match(rendered.stdout, /npm test -- --runInBand/); +}); + +test("verified review accepts requested checks and read-only inspection but rejects unrequested validation commands", () => { + const requestedCheck = "npm test -- --runInBand"; + const allowed = createVerifiedReviewRepo("verified-review-check-readonly-inspection"); + const allowedResult = run("node", [SCRIPT, "verified-review", "--check", requestedCheck, "--json"], { + cwd: allowed.repo, + env: buildEnv(allowed.binDir) + }); + + assert.equal(allowedResult.status, 0, allowedResult.stderr); + const allowedPayload = JSON.parse(allowedResult.stdout); + assert.deepEqual(allowedPayload.verification.checks.map((check) => check.command), [requestedCheck]); + assert.deepEqual( + allowedPayload.verification.inspectionExecutions.map((execution) => execution.command), + ["git diff --stat", "git diff --check"] + ); + assert.deepEqual(allowedPayload.verification.unauthorizedValidationExecutions, []); + + const rejected = createVerifiedReviewRepo("verified-review-unauthorized-check"); + const rejectedResult = run("node", [SCRIPT, "verified-review", "--check", requestedCheck, "--json"], { + cwd: rejected.repo, + env: buildEnv(rejected.binDir) + }); + + assert.notEqual(rejectedResult.status, 0); + const rejectedPayload = JSON.parse(rejectedResult.stdout); + assert.deepEqual( + rejectedPayload.verification.unauthorizedValidationExecutions.map((execution) => execution.command), + ["npm run build"] + ); + assert.match(rejectedPayload.parseError, /unauthorized|explicit check|validation/i); + + const rendered = run("node", [SCRIPT, "verified-review", "--check", requestedCheck], { + cwd: rejected.repo, + env: buildEnv(rejected.binDir) + }); + assert.notEqual(rendered.status, 0); + assert.match(rendered.stdout, /unauthorized validation/i); + assert.match(rendered.stdout, /npm run build/); +}); + +test("verified review accepts only explicit native clean sentinels", () => { + const cases = [ + { behavior: "verified-review-native-empty", success: false }, + { behavior: "verified-review-native-clean", success: true }, + { behavior: "verified-review-native-clean-after-heading", success: true }, + { behavior: "verified-review-native-ambiguous", success: false }, + { behavior: "verified-review-native-none", success: true }, + { behavior: "verified-review-native-no-findings", success: true }, + { behavior: "verified-review-native-no-issues", success: true }, + { behavior: "verified-review-native-no-problems", success: true }, + { behavior: "verified-review-native-clean-mixed", success: false } + ]; + + for (const { behavior, success } of cases) { + const { repo, binDir } = createVerifiedReviewRepo(behavior); + const result = run("node", [SCRIPT, "verified-review", "--json"], { + cwd: repo, + env: buildEnv(binDir) + }); + const payload = JSON.parse(result.stdout); + + assert.equal(result.status === 0, success, `${behavior} had the wrong success status`); + if (success) { + assert.deepEqual(payload.native.findings, []); + assert.equal(payload.parseError, null); + } else { + assert.equal(payload.result, null); + assert.match(payload.parseError, /native review output|no text|cannot be verified|recognizable finding|explicit clean|clean sentinel|ambiguous/i); + } + } +}); + +test("verified review ignores nested and fenced list markers in native output", () => { + const { repo, binDir } = createVerifiedReviewRepo("verified-review-native-nested-and-fenced"); + const result = run("node", [SCRIPT, "verified-review", "--json"], { + cwd: repo, + env: buildEnv(binDir) + }); + + assert.equal(result.status, 0, result.stderr); + const payload = JSON.parse(result.stdout); + assert.deepEqual(payload.native.findings.map((finding) => finding.id), ["native-1", "native-2"]); + assert.deepEqual(payload.result.findings.map((finding) => finding.native_finding_id), ["native-1", "native-2"]); + const prompt = fakeCodexState(binDir).turnStarts[0].prompt; + const serializedFindings = prompt.match(/\n([\s\S]*?)\n<\/native_findings>/)?.[1]; + assert.deepEqual(JSON.parse(serializedFindings), payload.native.findings); +}); + +test("verified review preserves native target selection for auto, working-tree, branch, and --base", () => { + const cases = [ + { args: [], targetType: "uncommittedChanges" }, + { args: ["--scope", "working-tree"], targetType: "uncommittedChanges" }, + { args: ["--scope", "branch"], targetType: "baseBranch" }, + { args: ["--base", "main"], targetType: "baseBranch" } + ]; + + for (const { args, targetType } of cases) { + const repo = makeTempDir(); + const binDir = makeTempDir(); + installFakeCodex(binDir, "verified-review-findings"); + initGitRepo(repo); + fs.writeFileSync(path.join(repo, "README.md"), "before\n"); + run("git", ["add", "README.md"], { cwd: repo }); + run("git", ["commit", "-m", "init"], { cwd: repo }); + fs.writeFileSync(path.join(repo, "README.md"), "after\n"); + + const result = run("node", [SCRIPT, "verified-review", ...args, "--json"], { + cwd: repo, + env: buildEnv(binDir) + }); + + assert.equal(result.status, 0, `${args.join(" ")} ${result.stderr}`); + const state = JSON.parse(fs.readFileSync(path.join(binDir, "fake-codex-state.json"), "utf8")); + assert.equal(state.reviewStarts.length, 1); + assert.equal(state.reviewStarts[0].target.type, targetType); + assert.equal(state.turnStarts.length, 1); + } +}); + +test("verified review background jobs expose status and result", async () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + installFakeCodex(binDir, "verified-review-findings"); + initGitRepo(repo); + fs.writeFileSync(path.join(repo, "README.md"), "before\n"); + run("git", ["add", "README.md"], { cwd: repo }); + run("git", ["commit", "-m", "init"], { cwd: repo }); + fs.writeFileSync(path.join(repo, "README.md"), "after\n"); + + const launched = run("node", [SCRIPT, "verified-review", "--background", "--json"], { + cwd: repo, + env: buildEnv(binDir) + }); + + assert.equal(launched.status, 0, launched.stderr); + const launchPayload = JSON.parse(launched.stdout); + assert.equal(launchPayload.status, "queued"); + assert.match(launchPayload.jobId, /^review-/); + + const waited = run( + "node", + [SCRIPT, "status", launchPayload.jobId, "--wait", "--timeout-ms", "15000", "--json"], + { cwd: repo, env: buildEnv(binDir) } + ); + assert.equal(waited.status, 0, waited.stderr); + assert.equal(JSON.parse(waited.stdout).job.status, "completed"); + + const resultPayload = await waitFor(() => { + const jobResult = run("node", [SCRIPT, "result", launchPayload.jobId, "--json"], { + cwd: repo, + env: buildEnv(binDir) + }); + return jobResult.status === 0 ? JSON.parse(jobResult.stdout) : null; + }); + assert.equal(resultPayload.job.kind, "verified-review"); + assert.match(resultPayload.storedJob.rendered, /Verified Review/); +}); + +test("verified review saves its queued request before spawning and recording the detached worker", () => { + const source = fs.readFileSync(SCRIPT, "utf8"); + const start = source.indexOf("function enqueueBackgroundVerifiedReview"); + const end = source.indexOf("async function handleReviewCommand", start); + const enqueueSource = source.slice(start, end); + + assert.ok(start >= 0, "verified-review enqueue function is missing"); + const writeIndex = enqueueSource.indexOf("writeJobFile(job.workspaceRoot, job.id, queuedRecord)"); + const spawnIndex = enqueueSource.indexOf("spawnDetachedVerifiedReviewWorker(cwd, job.id)"); + const pidIndex = enqueueSource.indexOf("recordQueuedJobPid(job.workspaceRoot, job.id"); + assert.ok(writeIndex < spawnIndex); + assert.ok(spawnIndex < pidIndex); +}); + +test("queued worker PID recording preserves cancellation", () => { + const workspaceRoot = makeTempDir(); + const job = { id: "review-pid-race", status: "queued", phase: "queued", pid: null }; + writeJobFile(workspaceRoot, job.id, job); + upsertJob(workspaceRoot, job); + + assert.equal(recordQueuedJobPid(workspaceRoot, job.id, 12345), true); + assert.equal(readJobFile(resolveJobFile(workspaceRoot, job.id)).pid, 12345); + cancelTrackedJob(workspaceRoot, job.id, { completedAt: "2026-08-11T00:00:00.000Z" }); + assert.equal(recordQueuedJobPid(workspaceRoot, job.id, 54321), false); + assert.equal(readJobFile(resolveJobFile(workspaceRoot, job.id)).status, "cancelled"); +}); + +test("verified review worker leaves an already-cancelled queued job untouched", () => { + const { repo, binDir } = createVerifiedReviewRepo("verified-review-findings"); + const stateDir = resolveStateDir(repo); + const jobsDir = path.join(stateDir, "jobs"); + const jobId = "review-cancelled"; + const logFile = path.join(jobsDir, `${jobId}.log`); + const storedJob = { + id: jobId, + kind: "verified-review", + title: "Codex Verified Review", + status: "cancelled", + phase: "cancelled", + pid: null, + logFile, + request: { cwd: repo, checks: [], jobId } + }; + fs.mkdirSync(jobsDir, { recursive: true }); + fs.writeFileSync(logFile, "Queued for background execution.\n", "utf8"); + fs.writeFileSync(path.join(jobsDir, `${jobId}.json`), `${JSON.stringify(storedJob, null, 2)}\n`, "utf8"); + fs.writeFileSync( + path.join(stateDir, "state.json"), + `${JSON.stringify({ version: 1, config: { stopReviewGate: false }, jobs: [storedJob] }, null, 2)}\n`, + "utf8" + ); + + const result = run("node", [SCRIPT, "verified-review-worker", "--cwd", repo, "--job-id", jobId], { + cwd: repo, + env: buildEnv(binDir) + }); + + assert.equal(result.status, 0, result.stderr); + assert.equal(JSON.parse(fs.readFileSync(path.join(jobsDir, `${jobId}.json`), "utf8")).status, "cancelled"); + assert.equal(JSON.parse(fs.readFileSync(path.join(stateDir, "state.json"), "utf8")).jobs[0].status, "cancelled"); + assert.equal(fs.existsSync(path.join(binDir, "fake-codex-state.json")), false); +}); + +test("tracked jobs do not start from a stale queued snapshot after cancellation", async () => { + const workspaceRoot = makeTempDir(); + const job = { + id: "review-cancel-race", + workspaceRoot, + kind: "verified-review", + status: "queued", + phase: "queued", + request: { cwd: workspaceRoot, checks: [] } + }; + writeJobFile(workspaceRoot, job.id, job); + upsertJob(workspaceRoot, job); + const staleWorkerSnapshot = readJobFile(resolveJobFile(workspaceRoot, job.id)); + const cancellation = cancelTrackedJob(workspaceRoot, job.id, { + completedAt: "2026-08-06T00:00:00.000Z", + errorMessage: "Cancelled by user." + }); + let started = false; + + const result = await runTrackedJob(staleWorkerSnapshot, async () => { + started = true; + return { exitStatus: 0, payload: {}, rendered: "", summary: "" }; + }); + + assert.ok(cancellation); + assert.equal(result, null); + assert.equal(started, false); + assert.equal(readJobFile(resolveJobFile(workspaceRoot, job.id)).status, "cancelled"); +}); + +test("tracked job progress and completion preserve cancellation", async () => { + const workspaceRoot = makeTempDir(); + const job = { + id: "review-cancel-progress-race", + workspaceRoot, + kind: "verified-review", + status: "queued", + phase: "queued", + request: { cwd: workspaceRoot, checks: [] } + }; + writeJobFile(workspaceRoot, job.id, job); + upsertJob(workspaceRoot, job); + let releaseRunner; + const runnerFinished = new Promise((resolve) => { + releaseRunner = resolve; + }); + const execution = runTrackedJob(job, async () => { + await runnerFinished; + return { exitStatus: 0, payload: {}, rendered: "", summary: "" }; + }); + + await waitFor(() => readJobFile(resolveJobFile(workspaceRoot, job.id)).status === "running"); + cancelTrackedJob(workspaceRoot, job.id, { + completedAt: "2026-08-06T00:00:00.000Z", + errorMessage: "Cancelled by user." + }); + createJobProgressUpdater(workspaceRoot, job.id)({ phase: "verifying", threadId: "thread-after-cancel" }); + releaseRunner(); + await execution; + + assert.equal(readJobFile(resolveJobFile(workspaceRoot, job.id)).status, "cancelled"); + assert.equal(JSON.parse(fs.readFileSync(path.join(resolveStateDir(workspaceRoot), "state.json"), "utf8")).jobs[0].status, "cancelled"); +}); + test("status shows phases, hints, and the latest finished job", () => { const workspace = makeTempDir(); const stateDir = resolveStateDir(workspace);