diff --git a/plugins/codex/scripts/codex-companion.mjs b/plugins/codex/scripts/codex-companion.mjs index 83df468ad..10a5f5b7f 100644 --- a/plugins/codex/scripts/codex-companion.mjs +++ b/plugins/codex/scripts/codex-companion.mjs @@ -524,7 +524,7 @@ async function executeTaskRun(request) { rendered, summary: firstMeaningfulLine(rawOutput, firstMeaningfulLine(failureMessage, `${taskMetadata.title} finished.`)), jobTitle: taskMetadata.title, - jobClass: "task", + jobClass: taskMetadata.jobClass, write: Boolean(request.write) }; } @@ -541,7 +541,8 @@ function buildTaskRunMetadata({ prompt, resumeLast = false }) { if (!resumeLast && String(prompt ?? "").includes(STOP_REVIEW_TASK_MARKER)) { return { title: "Codex Stop Gate Review", - summary: "Stop-gate review of previous Claude turn" + summary: "Stop-gate review of previous Claude turn", + jobClass: "stop-review" }; } @@ -549,7 +550,8 @@ function buildTaskRunMetadata({ prompt, resumeLast = false }) { const fallbackSummary = resumeLast ? DEFAULT_CONTINUE_PROMPT : "Task"; return { title, - summary: shorten(prompt || fallbackSummary) + summary: shorten(prompt || fallbackSummary), + jobClass: "task" }; } @@ -595,7 +597,7 @@ function buildTaskJob(workspaceRoot, taskMetadata, write) { kind: "task", title: taskMetadata.title, workspaceRoot, - jobClass: "task", + jobClass: taskMetadata.jobClass, summary: taskMetadata.summary, write }); diff --git a/plugins/codex/scripts/lib/app-server-protocol.d.ts b/plugins/codex/scripts/lib/app-server-protocol.d.ts index f61a4588e..bf38d565b 100644 --- a/plugins/codex/scripts/lib/app-server-protocol.d.ts +++ b/plugins/codex/scripts/lib/app-server-protocol.d.ts @@ -12,6 +12,8 @@ import type { ReviewStartResponse, ReviewTarget, Thread, + ThreadArchiveParams, + ThreadArchiveResponse, ThreadItem, ThreadListParams, ThreadListResponse, @@ -61,6 +63,7 @@ export interface AppServerMethodMap { "externalAgentConfig/import": { params: ExternalAgentConfigImportParams; result: ExternalAgentConfigImportResponse }; "thread/start": { params: ThreadStartParams; result: ThreadStartResponse }; "thread/resume": { params: ThreadResumeParams; result: ThreadResumeResponse }; + "thread/archive": { params: ThreadArchiveParams; result: ThreadArchiveResponse }; "thread/name/set": { params: ThreadSetNameParams; result: ThreadSetNameResponse }; "thread/list": { params: ThreadListParams; result: ThreadListResponse }; "review/start": { params: ReviewStartParams; result: ReviewStartResponse }; diff --git a/plugins/codex/scripts/lib/codex.mjs b/plugins/codex/scripts/lib/codex.mjs index fead00cc4..43862f180 100644 --- a/plugins/codex/scripts/lib/codex.mjs +++ b/plugins/codex/scripts/lib/codex.mjs @@ -999,6 +999,75 @@ export async function interruptAppServerTurn(cwd, { threadId, turnId }) { } } +export async function archiveAppServerThread(cwd, { threadId }) { + if (!threadId) { + return { + attempted: false, + archived: false, + transport: null, + detail: "missing threadId" + }; + } + + const availability = getCodexAvailability(cwd); + if (!availability.available) { + return { + attempted: false, + archived: false, + transport: null, + detail: availability.detail + }; + } + + try { + const result = await withAppServer(cwd, async (client) => { + try { + await client.request("thread/archive", { threadId }); + return { + archived: true, + transport: client.transport, + detail: `Archived ${threadId}.` + }; + } catch (archiveError) { + let cursor = null; + try { + do { + const response = await client.request("thread/list", { + archived: true, + cursor, + limit: 100, + sortKey: "updated_at", + sourceKinds: ["appServer"] + }); + if (response.data.some((thread) => thread.id === threadId)) { + return { + archived: true, + transport: client.transport, + detail: `${threadId} was already archived.` + }; + } + cursor = response.nextCursor ?? null; + } while (cursor); + } catch { + // Preserve the original archive error so broker failures can retry directly. + } + throw archiveError; + } + }); + return { + attempted: true, + ...result + }; + } catch (error) { + return { + attempted: true, + archived: false, + transport: null, + detail: error instanceof Error ? error.message : String(error) + }; + } +} + export async function runAppServerReview(cwd, options = {}) { const availability = getCodexAvailability(cwd); if (!availability.available) { diff --git a/plugins/codex/scripts/stop-review-gate-hook.mjs b/plugins/codex/scripts/stop-review-gate-hook.mjs index 2346bdcf4..7ef72bf69 100644 --- a/plugins/codex/scripts/stop-review-gate-hook.mjs +++ b/plugins/codex/scripts/stop-review-gate-hook.mjs @@ -6,7 +6,7 @@ import path from "node:path"; import { spawnSync } from "node:child_process"; import { fileURLToPath } from "node:url"; -import { getCodexAvailability } from "./lib/codex.mjs"; +import { archiveAppServerThread, getCodexAvailability } from "./lib/codex.mjs"; import { loadPromptTemplate, interpolateTemplate } from "./lib/prompts.mjs"; import { getConfig, listJobs } from "./lib/state.mjs"; import { sortJobsNewestFirst } from "./lib/job-control.mjs"; @@ -71,6 +71,7 @@ function parseStopReviewOutput(rawOutput) { if (!text) { return { ok: false, + recognized: false, reason: "The stop-time Codex review task returned no final output. Run /codex:review --wait manually or bypass the gate." }; @@ -78,24 +79,26 @@ function parseStopReviewOutput(rawOutput) { const firstLine = text.split(/\r?\n/, 1)[0].trim(); if (firstLine.startsWith("ALLOW:")) { - return { ok: true, reason: null }; + return { ok: true, recognized: true, reason: null }; } if (firstLine.startsWith("BLOCK:")) { const reason = firstLine.slice("BLOCK:".length).trim() || text; return { ok: false, + recognized: true, reason: `Codex stop-time review found issues that still need fixes before ending the session: ${reason}` }; } return { ok: false, + recognized: false, reason: "The stop-time Codex review task returned an unexpected answer. Run /codex:review --wait manually or bypass the gate." }; } -function runStopReview(cwd, input = {}) { +async function runStopReview(cwd, input = {}) { const scriptPath = path.join(SCRIPT_DIR, "codex-companion.mjs"); const prompt = buildStopReviewPrompt(input); const childEnv = { @@ -129,7 +132,18 @@ function runStopReview(cwd, input = {}) { try { const payload = JSON.parse(result.stdout); - return parseStopReviewOutput(payload?.rawOutput); + const review = parseStopReviewOutput(payload?.rawOutput); + if (review.recognized && payload?.threadId) { + const archive = await archiveAppServerThread(cwd, { threadId: payload.threadId }); + if (!archive.archived) { + logNote( + `Codex stop-time review completed, but its thread could not be archived${ + archive.detail ? `: ${archive.detail}` : "." + }` + ); + } + } + return review; } catch { return { ok: false, @@ -139,7 +153,7 @@ function runStopReview(cwd, input = {}) { } } -function main() { +async function main() { const input = readHookInput(); const cwd = input.cwd || process.env.CLAUDE_PROJECT_DIR || process.cwd(); const workspaceRoot = resolveWorkspaceRoot(cwd); @@ -163,7 +177,7 @@ function main() { return; } - const review = runStopReview(cwd, input); + const review = await runStopReview(cwd, input); if (!review.ok) { emitDecision({ decision: "block", @@ -175,10 +189,8 @@ function main() { logNote(runningTaskNote); } -try { - main(); -} catch (error) { +main().catch((error) => { const message = error instanceof Error ? error.message : String(error); process.stderr.write(`${message}\n`); process.exitCode = 1; -} +}); diff --git a/tests/fake-codex-fixture.mjs b/tests/fake-codex-fixture.mjs index f83c96a0d..2e7b87d7d 100644 --- a/tests/fake-codex-fixture.mjs +++ b/tests/fake-codex-fixture.mjs @@ -123,6 +123,7 @@ function nextThread(state, cwd, ephemeral) { name: null, preview: "", ephemeral: Boolean(ephemeral), + archived: false, createdAt: now(), updatedAt: now() }; @@ -234,6 +235,9 @@ function structuredReviewPayload(prompt) { function taskPayload(prompt, resume) { if (prompt.includes("") && prompt.includes("Only review the work from the previous Claude turn.")) { + if (BEHAVIOR === "stop-unexpected-output") { + return "MAYBE: Review completed without a valid gate decision."; + } if (BEHAVIOR === "adversarial-clean") { return "ALLOW: No blocking issues found in the previous turn."; } @@ -329,6 +333,9 @@ rl.on("line", (line) => { case "thread/list": { let threads = state.threads.slice(); + threads = threads.filter((thread) => + message.params.archived === true ? thread.archived === true : thread.archived !== true + ); if (message.params.cwd) { threads = threads.filter((thread) => thread.cwd === message.params.cwd); } @@ -340,6 +347,30 @@ rl.on("line", (line) => { break; } + case "thread/archive": { + const thread = ensureThread(state, message.params.threadId); + if (BEHAVIOR === "archive-fails") { + send({ + id: message.id, + error: { code: -32600, message: "archive unavailable for " + thread.id } + }); + break; + } + thread.archived = true; + thread.updatedAt = now(); + saveState(state); + if (BEHAVIOR === "archive-already-reported") { + send({ + id: message.id, + error: { code: -32600, message: "no rollout found for thread id " + thread.id } + }); + break; + } + send({ id: message.id, result: {} }); + send({ method: "thread/archived", params: { threadId: thread.id } }); + break; + } + case "thread/resume": { if (requiresExperimental("persistExtendedHistory", message, state) || requiresExperimental("persistFullHistory", message, state)) { throw new Error("thread/resume.persistFullHistory requires experimentalApi capability"); @@ -458,6 +489,12 @@ rl.on("line", (line) => { ? structuredReviewPayload(prompt) : taskPayload(prompt, thread.name && thread.name.startsWith("Codex Companion Task") && prompt.includes("Continue from the current thread state")); + if (BEHAVIOR === "stop-turn-fails" && prompt.includes("Only review the work from the previous Claude turn.")) { + send({ method: "turn/started", params: { threadId: thread.id, turn: buildTurn(turnId) } }); + send({ method: "turn/completed", params: { threadId: thread.id, turn: buildTurn(turnId, "failed") } }); + break; + } + if ( BEHAVIOR === "with-subagent" || BEHAVIOR === "with-late-subagent-message" || diff --git a/tests/runtime.test.mjs b/tests/runtime.test.mjs index 8f276835b..8e048e0df 100644 --- a/tests/runtime.test.mjs +++ b/tests/runtime.test.mjs @@ -1941,9 +1941,13 @@ test("stop hook runs a stop-time review task and blocks on findings when the rev const setupPayload = JSON.parse(setup.stdout); assert.equal(setupPayload.reviewGateEnabled, true); + const sessionEnv = { + ...buildEnv(binDir), + CODEX_COMPANION_SESSION_ID: "sess-stop-review" + }; const taskResult = run("node", [SCRIPT, "task", "--write", "fix the issue"], { cwd: repo, - env: buildEnv(binDir) + env: sessionEnv }); assert.equal(taskResult.status, 0, taskResult.stderr); @@ -1967,6 +1971,26 @@ test("stop hook runs a stop-time review task and blocks on findings when the rev assert.match(fakeState.lastTurnStart.prompt, //i); assert.match(fakeState.lastTurnStart.prompt, /Only review the work from the previous Claude turn/i); assert.match(fakeState.lastTurnStart.prompt, /I completed the refactor and updated the retry logic\./); + const stopReviewThread = fakeState.threads.find( + (thread) => thread.id === fakeState.lastTurnStart.threadId + ); + assert.equal(stopReviewThread.archived, true); + const rescueThread = fakeState.threads.find((thread) => thread.id !== stopReviewThread.id); + assert.equal(rescueThread?.archived, false); + + const state = JSON.parse( + fs.readFileSync(path.join(resolveStateDir(repo), "state.json"), "utf8") + ); + const stopReviewJob = state.jobs.find((job) => job.title === "Codex Stop Gate Review"); + assert.equal(stopReviewJob?.jobClass, "stop-review"); + + const resumeCandidate = run( + "node", + [SCRIPT, "task-resume-candidate", "--json"], + { cwd: repo, env: sessionEnv } + ); + assert.equal(resumeCandidate.status, 0, resumeCandidate.stderr); + assert.equal(JSON.parse(resumeCandidate.stdout).candidate.threadId, rescueThread.id); const status = run("node", [SCRIPT, "status"], { cwd: repo, @@ -2039,6 +2063,7 @@ test("stop hook logs running tasks to stderr without blocking when the review ga test("stop hook allows the stop when the review gate is enabled and the stop-time review task is clean", () => { const repo = makeTempDir(); const binDir = makeTempDir(); + const fakeStatePath = path.join(binDir, "fake-codex-state.json"); installFakeCodex(binDir, "adversarial-clean"); initGitRepo(repo); fs.writeFileSync(path.join(repo, "README.md"), "hello\n"); @@ -2059,6 +2084,180 @@ test("stop hook allows the stop when the review gate is enabled and the stop-tim assert.equal(allowed.status, 0, allowed.stderr); assert.equal(allowed.stdout.trim(), ""); + const fakeState = JSON.parse(fs.readFileSync(fakeStatePath, "utf8")); + const stopReviewThread = fakeState.threads.find( + (thread) => thread.id === fakeState.lastTurnStart.threadId + ); + assert.equal(stopReviewThread.archived, true); +}); + +test("stop hook archives through direct fallback when the configured broker is stale", () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + const staleBrokerDir = makeTempDir(); + const fakeStatePath = path.join(binDir, "fake-codex-state.json"); + installFakeCodex(binDir, "adversarial-clean"); + initGitRepo(repo); + fs.writeFileSync(path.join(repo, "README.md"), "hello\n"); + run("git", ["add", "README.md"], { cwd: repo }); + run("git", ["commit", "-m", "init"], { cwd: repo }); + + const setup = run("node", [SCRIPT, "setup", "--enable-review-gate", "--json"], { + cwd: repo, + env: buildEnv(binDir) + }); + assert.equal(setup.status, 0, setup.stderr); + + const allowed = run("node", [STOP_HOOK], { + cwd: repo, + env: { + ...buildEnv(binDir), + CODEX_COMPANION_APP_SERVER_ENDPOINT: `unix:${path.join(staleBrokerDir, "missing.sock")}` + }, + input: JSON.stringify({ cwd: repo, session_id: "sess-stale-broker" }) + }); + + assert.equal(allowed.status, 0, allowed.stderr); + assert.equal(allowed.stdout.trim(), ""); + assert.doesNotMatch(allowed.stderr, /could not be archived/i); + const fakeState = JSON.parse(fs.readFileSync(fakeStatePath, "utf8")); + const stopReviewThread = fakeState.threads.find( + (thread) => thread.id === fakeState.lastTurnStart.threadId + ); + assert.equal(stopReviewThread.archived, true); +}); + +test("stop hook keeps an unrecognized review thread available for inspection", () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + const fakeStatePath = path.join(binDir, "fake-codex-state.json"); + installFakeCodex(binDir, "stop-unexpected-output"); + initGitRepo(repo); + fs.writeFileSync(path.join(repo, "README.md"), "hello\n"); + run("git", ["add", "README.md"], { cwd: repo }); + run("git", ["commit", "-m", "init"], { cwd: repo }); + + const setup = run("node", [SCRIPT, "setup", "--enable-review-gate", "--json"], { + cwd: repo, + env: buildEnv(binDir) + }); + assert.equal(setup.status, 0, setup.stderr); + + const blocked = run("node", [STOP_HOOK], { + cwd: repo, + env: buildEnv(binDir), + input: JSON.stringify({ cwd: repo, session_id: "sess-stop-unrecognized" }) + }); + + assert.equal(blocked.status, 0, blocked.stderr); + const payload = JSON.parse(blocked.stdout); + assert.equal(payload.decision, "block"); + assert.match(payload.reason, /unexpected answer/i); + const fakeState = JSON.parse(fs.readFileSync(fakeStatePath, "utf8")); + const stopReviewThread = fakeState.threads.find( + (thread) => thread.id === fakeState.lastTurnStart.threadId + ); + assert.equal(stopReviewThread.archived, false); +}); + +test("stop hook treats an already archived review thread as idempotent cleanup", () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + const fakeStatePath = path.join(binDir, "fake-codex-state.json"); + installFakeCodex(binDir, "archive-already-reported"); + initGitRepo(repo); + fs.writeFileSync(path.join(repo, "README.md"), "hello\n"); + run("git", ["add", "README.md"], { cwd: repo }); + run("git", ["commit", "-m", "init"], { cwd: repo }); + + const setup = run("node", [SCRIPT, "setup", "--enable-review-gate", "--json"], { + cwd: repo, + env: buildEnv(binDir) + }); + assert.equal(setup.status, 0, setup.stderr); + + const blocked = run("node", [STOP_HOOK], { + cwd: repo, + env: buildEnv(binDir), + input: JSON.stringify({ cwd: repo, session_id: "sess-stop-already-archived" }) + }); + + assert.equal(blocked.status, 0, blocked.stderr); + const payload = JSON.parse(blocked.stdout); + assert.equal(payload.decision, "block"); + assert.doesNotMatch(blocked.stderr, /could not be archived/i); + const fakeState = JSON.parse(fs.readFileSync(fakeStatePath, "utf8")); + const stopReviewThread = fakeState.threads.find( + (thread) => thread.id === fakeState.lastTurnStart.threadId + ); + assert.equal(stopReviewThread.archived, true); +}); + +test("stop hook keeps a failed review thread available for retry", () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + const fakeStatePath = path.join(binDir, "fake-codex-state.json"); + installFakeCodex(binDir, "stop-turn-fails"); + initGitRepo(repo); + fs.writeFileSync(path.join(repo, "README.md"), "hello\n"); + run("git", ["add", "README.md"], { cwd: repo }); + run("git", ["commit", "-m", "init"], { cwd: repo }); + + const setup = run("node", [SCRIPT, "setup", "--enable-review-gate", "--json"], { + cwd: repo, + env: buildEnv(binDir) + }); + assert.equal(setup.status, 0, setup.stderr); + + const blocked = run("node", [STOP_HOOK], { + cwd: repo, + env: buildEnv(binDir), + input: JSON.stringify({ cwd: repo, session_id: "sess-stop-failed" }) + }); + + assert.equal(blocked.status, 0, blocked.stderr); + const payload = JSON.parse(blocked.stdout); + assert.equal(payload.decision, "block"); + assert.match(payload.reason, /stop-time Codex review task failed/i); + const fakeState = JSON.parse(fs.readFileSync(fakeStatePath, "utf8")); + const stopReviewThread = fakeState.threads.find( + (thread) => thread.id === fakeState.lastTurnStart.threadId + ); + assert.equal(stopReviewThread.archived, false); +}); + +test("stop hook preserves the review decision when archive cleanup fails", () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + const fakeStatePath = path.join(binDir, "fake-codex-state.json"); + installFakeCodex(binDir, "archive-fails"); + initGitRepo(repo); + fs.writeFileSync(path.join(repo, "README.md"), "hello\n"); + run("git", ["add", "README.md"], { cwd: repo }); + run("git", ["commit", "-m", "init"], { cwd: repo }); + + const setup = run("node", [SCRIPT, "setup", "--enable-review-gate", "--json"], { + cwd: repo, + env: buildEnv(binDir) + }); + assert.equal(setup.status, 0, setup.stderr); + + const blocked = run("node", [STOP_HOOK], { + cwd: repo, + env: buildEnv(binDir), + input: JSON.stringify({ cwd: repo, session_id: "sess-stop-archive-failed" }) + }); + + assert.equal(blocked.status, 0, blocked.stderr); + const payload = JSON.parse(blocked.stdout); + assert.equal(payload.decision, "block"); + assert.match(payload.reason, /Missing empty-state guard/i); + assert.match(blocked.stderr, /thread could not be archived/i); + const fakeState = JSON.parse(fs.readFileSync(fakeStatePath, "utf8")); + const stopReviewThread = fakeState.threads.find( + (thread) => thread.id === fakeState.lastTurnStart.threadId + ); + assert.equal(stopReviewThread.archived, false); }); test("stop hook does not block when Codex is unavailable even if the review gate is enabled", () => {