diff --git a/plugins/codex/scripts/codex-companion.mjs b/plugins/codex/scripts/codex-companion.mjs index 35222fd5..91d38174 100644 --- a/plugins/codex/scripts/codex-companion.mjs +++ b/plugins/codex/scripts/codex-companion.mjs @@ -21,7 +21,7 @@ import { runAppServerTurn } from "./lib/codex.mjs"; import { readStdinIfPiped } from "./lib/fs.mjs"; -import { collectReviewContext, ensureGitRepository, resolveReviewTarget } from "./lib/git.mjs"; +import { collectReviewContext, ensureGitRepository, getWorkspaceWriteFingerprint, resolveReviewTarget } from "./lib/git.mjs"; import { binaryAvailable, terminateProcessTree } from "./lib/process.mjs"; import { loadPromptTemplate, interpolateTemplate } from "./lib/prompts.mjs"; import { @@ -479,6 +479,8 @@ async function executeTaskRun(request) { throw new Error("Provide a prompt, a prompt file, piped stdin, or use --resume-last."); } + const writeFingerprintBefore = request.write ? getWorkspaceWriteFingerprint(workspaceRoot) : null; + const result = await runAppServerTurn(workspaceRoot, { resumeThreadId, prompt: request.prompt, @@ -491,9 +493,26 @@ async function executeTaskRun(request) { threadName: resumeThreadId ? null : buildPersistentTaskThreadName(request.prompt || DEFAULT_CONTINUE_PROMPT) }); + // A write task that produced no apply_patch changes AND left the working tree + // fingerprint untouched landed zero workspace writes. Sandbox failures can + // reach this state while the turn itself reports success (the model narrates + // the loss in prose but nothing mechanical fails), so surface it as a failed, + // degraded job instead of "completed". Fingerprint unavailable (non-git + // workspace) skips detection rather than risking a false positive. + let degraded = null; + if ( + request.write && + result.status === 0 && + (result.touchedFiles ?? []).length === 0 && + writeFingerprintBefore !== null && + getWorkspaceWriteFingerprint(workspaceRoot) === writeFingerprintBefore + ) { + degraded = "zero-writes"; + } + const rawOutput = typeof result.finalMessage === "string" ? result.finalMessage : ""; const failureMessage = result.error?.message ?? result.stderr ?? ""; - const rendered = renderTaskResult( + let rendered = renderTaskResult( { rawOutput, failureMessage, @@ -505,21 +524,30 @@ async function executeTaskRun(request) { write: Boolean(request.write) } ); + if (degraded) { + rendered = + "DEGRADED: this write task landed zero workspace writes.\n" + + "No apply_patch changes were recorded and the working tree is unchanged. " + + "Treat the task as failed and check the job log before assuming any edits exist.\n\n" + + rendered; + } const payload = { status: result.status, threadId: result.threadId, rawOutput, touchedFiles: result.touchedFiles, - reasoningSummary: result.reasoningSummary + reasoningSummary: result.reasoningSummary, + ...(degraded ? { degraded } : {}) }; + const summary = firstMeaningfulLine(rawOutput, firstMeaningfulLine(failureMessage, `${taskMetadata.title} finished.`)); return { - exitStatus: result.status, + exitStatus: degraded ? 1 : result.status, threadId: result.threadId, turnId: result.turnId, payload, rendered, - summary: firstMeaningfulLine(rawOutput, firstMeaningfulLine(failureMessage, `${taskMetadata.title} finished.`)), + summary: degraded ? `DEGRADED (zero workspace writes): ${summary}` : summary, jobTitle: taskMetadata.title, jobClass: "task", write: Boolean(request.write) diff --git a/plugins/codex/scripts/lib/git.mjs b/plugins/codex/scripts/lib/git.mjs index 1749cfc8..fe7f55b6 100644 --- a/plugins/codex/scripts/lib/git.mjs +++ b/plugins/codex/scripts/lib/git.mjs @@ -1,3 +1,4 @@ +import { createHash } from "node:crypto"; import fs from "node:fs"; import path from "node:path"; @@ -118,6 +119,32 @@ export function getCurrentBranch(cwd) { return gitChecked(cwd, ["branch", "--show-current"]).stdout.trim() || "HEAD"; } +export function getWorkspaceWriteFingerprint(cwd) { + try { + const status = gitChecked(cwd, ["status", "--porcelain=v1", "-uall"]).stdout; + const unstagedDiff = gitChecked(cwd, ["diff"]).stdout; + const stagedDiff = gitChecked(cwd, ["diff", "--cached"]).stdout; + const untracked = gitChecked(cwd, ["ls-files", "--others", "--exclude-standard"]) + .stdout.trim() + .split("\n") + .filter(Boolean) + .sort(); + const untrackedStats = untracked.map((relativePath) => { + try { + const stats = fs.statSync(path.join(cwd, relativePath)); + return `${relativePath}\0${stats.size}\0${stats.mtimeMs}`; + } catch { + return `${relativePath}\0unreadable`; + } + }); + return createHash("sha256") + .update([status, unstagedDiff, stagedDiff, untrackedStats.join("\n")].join("\0")) + .digest("hex"); + } catch { + return null; + } +} + export function getWorkingTreeState(cwd) { const staged = gitChecked(cwd, ["diff", "--cached", "--name-only"]).stdout.trim().split("\n").filter(Boolean); const unstaged = gitChecked(cwd, ["diff", "--name-only"]).stdout.trim().split("\n").filter(Boolean); diff --git a/tests/fake-codex-fixture.mjs b/tests/fake-codex-fixture.mjs index debcadce..410d5949 100644 --- a/tests/fake-codex-fixture.mjs +++ b/tests/fake-codex-fixture.mjs @@ -498,6 +498,13 @@ rl.on("line", (line) => { break; } + if (BEHAVIOR === "task-shell-write") { + fs.writeFileSync(path.join(thread.cwd, "shell-write.txt"), "shell write\\n"); + } + if (BEHAVIOR === "task-shell-write-existing") { + fs.appendFileSync(path.join(thread.cwd, "README.md"), "appended by fake codex\\n"); + } + const items = [ ...(BEHAVIOR === "with-reasoning" ? [ @@ -511,6 +518,18 @@ rl.on("line", (line) => { } ] : []), + ...(BEHAVIOR === "task-write-file-change" + ? [ + { + completed: { + type: "fileChange", + id: "filechange_" + turnId, + status: "completed", + changes: [{ path: path.join(thread.cwd, "src", "app.js"), kind: "add" }] + } + } + ] + : []), { completed: { type: "agentMessage", id: "msg_" + turnId, text: payload, phase: "final_answer" } } diff --git a/tests/runtime.test.mjs b/tests/runtime.test.mjs index 90408372..e9ddf197 100644 --- a/tests/runtime.test.mjs +++ b/tests/runtime.test.mjs @@ -565,7 +565,7 @@ test("session start hook exports the Claude session id and plugin data dir for l test("write task output focuses on the Codex result without generic follow-up hints", () => { const repo = makeTempDir(); const binDir = makeTempDir(); - installFakeCodex(binDir); + installFakeCodex(binDir, "task-shell-write"); initGitRepo(repo); fs.writeFileSync(path.join(repo, "README.md"), "hello\n"); run("git", ["add", "README.md"], { cwd: repo }); @@ -1380,7 +1380,7 @@ test("result without a job id prefers the latest finished job from the current C test("result for a finished write-capable task returns the raw Codex final response", () => { const repo = makeTempDir(); const binDir = makeTempDir(); - installFakeCodex(binDir); + installFakeCodex(binDir, "task-shell-write"); initGitRepo(repo); fs.writeFileSync(path.join(repo, "README.md"), "hello\n"); run("git", ["add", "README.md"], { cwd: repo }); @@ -1791,7 +1791,7 @@ test("stop hook runs a stop-time review task and blocks on findings when the rev const repo = makeTempDir(); const binDir = makeTempDir(); const fakeStatePath = path.join(binDir, "fake-codex-state.json"); - installFakeCodex(binDir); + installFakeCodex(binDir, "task-shell-write"); initGitRepo(repo); fs.writeFileSync(path.join(repo, "README.md"), "hello\n"); run("git", ["add", "README.md"], { cwd: repo }); diff --git a/tests/write-task-degraded.test.mjs b/tests/write-task-degraded.test.mjs new file mode 100644 index 00000000..3daa92ad --- /dev/null +++ b/tests/write-task-degraded.test.mjs @@ -0,0 +1,127 @@ +import fs from "node:fs"; +import path from "node:path"; +import test from "node:test"; +import assert from "node:assert/strict"; +import { fileURLToPath } from "node:url"; + +import { buildEnv, installFakeCodex } from "./fake-codex-fixture.mjs"; +import { initGitRepo, makeTempDir, run } from "./helpers.mjs"; +import { resolveStateDir } from "../plugins/codex/scripts/lib/state.mjs"; + +const ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), ".."); +const SCRIPT = path.join(ROOT, "plugins", "codex", "scripts", "codex-companion.mjs"); + +function setupRepo() { + const repo = makeTempDir(); + 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 }); + return repo; +} + +function readLatestJob(workspace) { + const stateDir = resolveStateDir(workspace); + const state = JSON.parse(fs.readFileSync(path.join(stateDir, "state.json"), "utf8")); + const jobFile = path.join(stateDir, "jobs", `${state.jobs[0].id}.json`); + return JSON.parse(fs.readFileSync(jobFile, "utf8")); +} + +test("write task that lands zero workspace writes finishes failed with a zero-write notice", () => { + const repo = setupRepo(); + const binDir = makeTempDir(); + installFakeCodex(binDir); + + const result = run("node", [SCRIPT, "task", "--write", "please implement the fix"], { + cwd: repo, + env: buildEnv(binDir) + }); + + assert.notEqual(result.status, 0, "zero-write write task should exit non-zero"); + assert.match(result.stdout, /zero workspace writes/i); + const job = readLatestJob(repo); + assert.equal(job.status, "failed"); + assert.equal(job.result.degraded, "zero-writes"); +}); + +test("write task with apply_patch file changes stays completed", () => { + const repo = setupRepo(); + const binDir = makeTempDir(); + installFakeCodex(binDir, "task-write-file-change"); + + const result = run("node", [SCRIPT, "task", "--write", "please implement the fix"], { + cwd: repo, + env: buildEnv(binDir) + }); + + assert.equal(result.status, 0, result.stderr); + const job = readLatestJob(repo); + assert.equal(job.status, "completed"); + assert.equal(job.result.degraded ?? null, null); +}); + +test("write task with shell-only writes stays completed", () => { + const repo = setupRepo(); + const binDir = makeTempDir(); + installFakeCodex(binDir, "task-shell-write"); + + const result = run("node", [SCRIPT, "task", "--write", "please implement the fix"], { + cwd: repo, + env: buildEnv(binDir) + }); + + assert.equal(result.status, 0, result.stderr); + assert.ok(fs.existsSync(path.join(repo, "shell-write.txt")), "fixture should have written into the workspace"); + const job = readLatestJob(repo); + assert.equal(job.status, "completed"); + assert.equal(job.result.degraded ?? null, null); +}); + +test("write task that only modifies an already-dirty tracked file stays completed", () => { + const repo = setupRepo(); + fs.appendFileSync(path.join(repo, "README.md"), "dirty before task\n"); + const binDir = makeTempDir(); + installFakeCodex(binDir, "task-shell-write-existing"); + + const result = run("node", [SCRIPT, "task", "--write", "please implement the fix"], { + cwd: repo, + env: buildEnv(binDir) + }); + + assert.equal(result.status, 0, result.stderr); + const job = readLatestJob(repo); + assert.equal(job.status, "completed"); + assert.equal(job.result.degraded ?? null, null); +}); + +test("read-only task with zero writes stays completed", () => { + const repo = setupRepo(); + const binDir = makeTempDir(); + installFakeCodex(binDir); + + const result = run("node", [SCRIPT, "task", "summarize the repo"], { + cwd: repo, + env: buildEnv(binDir) + }); + + assert.equal(result.status, 0, result.stderr); + const job = readLatestJob(repo); + assert.equal(job.status, "completed"); + assert.equal(job.result.degraded ?? null, null); +}); + +test("write task in a non-git workspace skips zero-write detection", () => { + const workspace = makeTempDir(); + const binDir = makeTempDir(); + installFakeCodex(binDir); + + const result = run("node", [SCRIPT, "task", "--write", "please implement the fix"], { + cwd: workspace, + env: buildEnv(binDir) + }); + + assert.equal(result.status, 0, result.stderr); + const job = readLatestJob(workspace); + assert.equal(job.status, "completed"); + assert.equal(job.result.degraded ?? null, null); +});