From 2db7ca82e1d56854f55ce2be760d6dcb7134d2fe Mon Sep 17 00:00:00 2001 From: ALV0612 Date: Wed, 2 Sep 2026 09:33:04 +0700 Subject: [PATCH] fix(runtime): terminate turns on terminal errors --- plugins/codex/scripts/codex-companion.mjs | 12 +- plugins/codex/scripts/lib/codex.mjs | 47 ++++++- plugins/codex/scripts/lib/render.mjs | 3 + tests/fake-codex-fixture.mjs | 62 ++++++++++ tests/helpers.mjs | 1 + tests/runtime.test.mjs | 143 ++++++++++++++++++++++ 6 files changed, 261 insertions(+), 7 deletions(-) diff --git a/plugins/codex/scripts/codex-companion.mjs b/plugins/codex/scripts/codex-companion.mjs index 83df468ad..6636af9c0 100644 --- a/plugins/codex/scripts/codex-companion.mjs +++ b/plugins/codex/scripts/codex-companion.mjs @@ -381,14 +381,16 @@ async function executeReviewRun(request) { status: result.status, stderr: result.stderr, stdout: result.reviewText, - reasoning: result.reasoningSummary + reasoning: result.reasoningSummary, + ...(result.status !== 0 && result.error ? { error: result.error } : {}) } }; const rendered = renderNativeReviewResult( { status: result.status, stdout: result.reviewText, - stderr: result.stderr + stderr: result.stderr, + error: result.error }, { reviewLabel: reviewName, targetLabel: target.label, reasoningSummary: result.reasoningSummary } ); @@ -437,7 +439,8 @@ async function executeReviewRun(request) { result: parsed.parsed, rawOutput: parsed.rawOutput, parseError: parsed.parseError, - reasoningSummary: result.reasoningSummary + reasoningSummary: result.reasoningSummary, + ...(result.status !== 0 && result.error ? { error: result.error } : {}) }; return { @@ -513,7 +516,8 @@ async function executeTaskRun(request) { threadId: result.threadId, rawOutput, touchedFiles: result.touchedFiles, - reasoningSummary: result.reasoningSummary + reasoningSummary: result.reasoningSummary, + ...(result.status !== 0 && result.error ? { error: result.error } : {}) }; return { diff --git a/plugins/codex/scripts/lib/codex.mjs b/plugins/codex/scripts/lib/codex.mjs index fead00cc4..78e8ba363 100644 --- a/plugins/codex/scripts/lib/codex.mjs +++ b/plugins/codex/scripts/lib/codex.mjs @@ -370,6 +370,31 @@ function completeTurn(state, turn = null, options = {}) { state.resolveCompletion(state); } +function failTurn(state, error) { + if (state.completed) { + return; + } + + state.error = error; + completeTurn(state, { + id: state.turnId ?? "error-turn", + items: [], + itemsView: "full", + status: "failed", + error, + startedAt: null, + completedAt: Math.floor(Date.now() / 1000), + durationMs: null + }); +} + +function formatTurnError(error) { + if (typeof error?.message === "string" && error.message.trim()) { + return error.message.trim(); + } + return String(error ?? "Unknown Codex error").trim(); +} + function scheduleInferredCompletion(state) { if (state.completed || state.finalTurn || !state.finalAnswerSeen) { return; @@ -534,10 +559,26 @@ function applyTurnNotification(state, message) { emitProgress(state.onProgress, update?.message, update?.phase ?? null); } break; - case "error": - state.error = message.params.error; - emitProgress(state.onProgress, `Codex error: ${message.params.error.message}`, "failed"); + case "error": { + const error = message.params.error; + const messageThreadId = message.params.threadId ?? null; + if (messageThreadId !== state.threadId) { + const sourceLabel = labelForThread(state, messageThreadId); + emitProgress( + state.onProgress, + `${sourceLabel ? `Subagent ${sourceLabel}` : "Subagent"} error: ${formatTurnError(error)}`, + "investigating" + ); + break; + } + + state.error = error; + emitProgress(state.onProgress, `Codex error: ${formatTurnError(error)}`, message.params.willRetry ? "running" : "failed"); + if (!message.params.willRetry) { + failTurn(state, error); + } break; + } case "turn/completed": if ((message.params.threadId ?? null) !== state.threadId) { state.activeSubagentTurns.delete(message.params.threadId); diff --git a/plugins/codex/scripts/lib/render.mjs b/plugins/codex/scripts/lib/render.mjs index 2ec185236..953c42c0e 100644 --- a/plugins/codex/scripts/lib/render.mjs +++ b/plugins/codex/scripts/lib/render.mjs @@ -288,6 +288,7 @@ export function renderReviewResult(parsedResult, meta) { export function renderNativeReviewResult(result, meta) { const stdout = result.stdout.trim(); const stderr = result.stderr.trim(); + const errorMessage = String(result.error?.message ?? "").trim(); const lines = [ `# Codex ${meta.reviewLabel}`, "", @@ -299,6 +300,8 @@ export function renderNativeReviewResult(result, meta) { lines.push(stdout); } else if (result.status === 0) { lines.push("Codex review completed without any stdout output."); + } else if (errorMessage) { + lines.push(errorMessage); } else { lines.push("Codex review failed."); } diff --git a/tests/fake-codex-fixture.mjs b/tests/fake-codex-fixture.mjs index f83c96a0d..e2551bcc5 100644 --- a/tests/fake-codex-fixture.mjs +++ b/tests/fake-codex-fixture.mjs @@ -413,6 +413,24 @@ rl.on("line", (line) => { } const turnId = nextTurnId(state); send({ id: message.id, result: { turn: buildTurn(turnId), reviewThreadId: reviewThread.id } }); + if (BEHAVIOR === "terminal-review-error") { + send({ method: "turn/started", params: { threadId: reviewThread.id, turn: buildTurn(turnId) } }); + send({ + method: "error", + params: { + threadId: reviewThread.id, + turnId, + error: { + message: "git merge-base rejected a tree object", + codexErrorInfo: null, + additionalDetails: "error: object is a tree, not a commit", + misalignment: null + }, + willRetry: false + } + }); + break; + } emitTurnCompleted(reviewThread.id, turnId, [ { started: { type: "enteredReviewMode", id: turnId, review: "current changes" } @@ -452,8 +470,52 @@ rl.on("line", (line) => { prompt }; saveState(state); + + const terminalError = { + message: "git merge-base rejected a tree object", + codexErrorInfo: null, + additionalDetails: "error: object is a tree, not a commit", + misalignment: null + }; + if (BEHAVIOR === "terminal-error-buffered") { + send({ method: "error", params: { threadId: thread.id, turnId, error: terminalError, willRetry: false } }); + send({ id: message.id, result: { turn: buildTurn(turnId) } }); + break; + } + send({ id: message.id, result: { turn: buildTurn(turnId) } }); + if (BEHAVIOR === "terminal-error" || BEHAVIOR === "terminal-error-then-completed") { + send({ method: "turn/started", params: { threadId: thread.id, turn: buildTurn(turnId) } }); + send({ method: "error", params: { threadId: thread.id, turnId, error: terminalError, willRetry: false } }); + if (BEHAVIOR === "terminal-error-then-completed") { + send({ method: "turn/completed", params: { threadId: thread.id, turn: buildTurn(turnId, "completed") } }); + } + break; + } + + if (BEHAVIOR === "retryable-error") { + send({ method: "turn/started", params: { threadId: thread.id, turn: buildTurn(turnId) } }); + send({ method: "error", params: { threadId: thread.id, turnId, error: terminalError, willRetry: true } }); + emitTurnCompleted(thread.id, turnId, [{ + completed: { type: "agentMessage", id: "msg_" + turnId, text: "Recovered after retry.", phase: "final_answer" } + }]); + break; + } + + if (BEHAVIOR === "unrelated-error") { + send({ method: "error", params: { threadId: "unrelated_thread", turnId: "unrelated_turn", error: terminalError, willRetry: false } }); + } + + if (BEHAVIOR === "tracked-subagent-error") { + const subThread = nextThread(state, thread.cwd, true); + const subTurnId = nextTurnId(state); + send({ method: "thread/started", params: { thread: { ...buildThread(subThread), agentNickname: "error-probe" } } }); + send({ method: "turn/started", params: { threadId: subThread.id, turn: buildTurn(subTurnId) } }); + send({ method: "error", params: { threadId: subThread.id, turnId: subTurnId, error: terminalError, willRetry: false } }); + send({ method: "turn/completed", params: { threadId: subThread.id, turn: buildTurn(subTurnId, "failed", terminalError) } }); + } + const payload = message.params.outputSchema && message.params.outputSchema.properties && message.params.outputSchema.properties.verdict ? structuredReviewPayload(prompt) : taskPayload(prompt, thread.name && thread.name.startsWith("Codex Companion Task") && prompt.includes("Continue from the current thread state")); diff --git a/tests/helpers.mjs b/tests/helpers.mjs index d6981197a..41ced54dd 100644 --- a/tests/helpers.mjs +++ b/tests/helpers.mjs @@ -18,6 +18,7 @@ export function run(command, args, options = {}) { env: options.env, encoding: "utf8", input: options.input, + timeout: options.timeout, shell: options.shell ?? (process.platform === "win32" && !path.isAbsolute(command)), windowsHide: true }); diff --git a/tests/runtime.test.mjs b/tests/runtime.test.mjs index 8f276835b..216a935f8 100644 --- a/tests/runtime.test.mjs +++ b/tests/runtime.test.mjs @@ -2257,3 +2257,146 @@ test("setup and status honor --cwd when reading shared session runtime", () => { assert.equal(payload.sessionRuntime.mode, "shared"); assert.equal(payload.sessionRuntime.endpoint, "unix:/tmp/fake-broker.sock"); }); + +test("task terminates when app-server emits a non-retryable turn error", () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + installFakeCodex(binDir, "terminal-error"); + 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 result = run("node", [SCRIPT, "task", "trigger terminal error"], { + cwd: repo, + env: buildEnv(binDir), + timeout: 5000 + }); + + assert.equal(result.signal, null, `task timed out: ${result.stderr}`); + assert.notEqual(result.status, 0); + assert.match(result.stderr, /git merge-base rejected a tree object/); + assert.match(result.stdout, /git merge-base rejected a tree object/); + + const stateDir = resolveStateDir(repo); + const state = JSON.parse(fs.readFileSync(path.join(stateDir, "state.json"), "utf8")); + assert.equal(state.jobs[0].status, "failed"); + assert.equal(state.jobs[0].phase, "failed"); + assert.ok(state.jobs[0].completedAt); + const stored = JSON.parse(fs.readFileSync(path.join(stateDir, "jobs", `${state.jobs[0].id}.json`), "utf8")); + assert.equal(stored.result.status, 1); + assert.equal(stored.result.error.message, "git merge-base rejected a tree object"); + assert.match(stored.rendered, /git merge-base rejected a tree object/); +}); + +for (const behavior of ["terminal-error-buffered", "terminal-error-then-completed"]) { + test(`task preserves terminal failure for ${behavior}`, () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + installFakeCodex(binDir, behavior); + 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 result = run("node", [SCRIPT, "task", "trigger terminal error"], { + cwd: repo, + env: buildEnv(binDir), + timeout: 5000 + }); + + assert.equal(result.signal, null, `task timed out: ${result.stderr}`); + assert.notEqual(result.status, 0); + assert.match(result.stdout, /git merge-base rejected a tree object/); + }); +} + +test("task continues after a retryable turn error", () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + installFakeCodex(binDir, "retryable-error"); + 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 result = run("node", [SCRIPT, "task", "retry the turn"], { + cwd: repo, + env: buildEnv(binDir), + timeout: 5000 + }); + + assert.equal(result.status, 0, result.stderr); + assert.equal(result.stdout, "Recovered after retry.\n"); + assert.match(result.stderr, /git merge-base rejected a tree object/); +}); + +test("task ignores an error notification for an unrelated thread", () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + installFakeCodex(binDir, "unrelated-error"); + 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 result = run("node", [SCRIPT, "task", "ignore unrelated error"], { + cwd: repo, + env: buildEnv(binDir), + timeout: 5000 + }); + + assert.equal(result.status, 0, result.stderr); + assert.match(result.stdout, /Handled the requested task/); + assert.doesNotMatch(result.stderr, /git merge-base rejected a tree object/); +}); + +test("review terminates and renders a non-retryable app-server error", () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + installFakeCodex(binDir, "terminal-review-error"); + 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 }); + fs.writeFileSync(path.join(repo, "README.md"), "hello again\n"); + + const result = run("node", [SCRIPT, "review", "--scope", "working-tree"], { + cwd: repo, + env: buildEnv(binDir), + timeout: 5000 + }); + + assert.equal(result.signal, null, `review timed out: ${result.stderr}`); + assert.notEqual(result.status, 0); + assert.match(result.stderr, /git merge-base rejected a tree object/); + assert.match(result.stdout, /git merge-base rejected a tree object/); + + const stateDir = resolveStateDir(repo); + const state = JSON.parse(fs.readFileSync(path.join(stateDir, "state.json"), "utf8")); + assert.equal(state.jobs[0].status, "failed"); + const stored = JSON.parse(fs.readFileSync(path.join(stateDir, "jobs", `${state.jobs[0].id}.json`), "utf8")); + assert.equal(stored.result.codex.error.message, "git merge-base rejected a tree object"); +}); + +test("task does not let a tracked subagent error overwrite the root turn result", () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + installFakeCodex(binDir, "tracked-subagent-error"); + 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 result = run("node", [SCRIPT, "task", "keep the root turn authoritative"], { + cwd: repo, + env: buildEnv(binDir), + timeout: 5000 + }); + + assert.equal(result.status, 0, result.stderr); + assert.match(result.stdout, /Handled the requested task/); + const stateDir = resolveStateDir(repo); + const state = JSON.parse(fs.readFileSync(path.join(stateDir, "state.json"), "utf8")); + assert.equal(state.jobs[0].status, "completed"); +});