diff --git a/README.md b/README.md index 937a3037b..735e917e5 100644 --- a/README.md +++ b/README.md @@ -135,7 +135,8 @@ Use it when you want Codex to: - take a faster or cheaper pass with a smaller model > [!NOTE] -> Depending on the task and the model you choose these tasks might take a long time and it's generally recommended to force the task to be in the background or move the agent to the background. +> Depending on the task and model, rescues can take a long time. With `--background`, the plugin launches a durable background worker and follows it with bounded status waits before retrieving the final result, so no single Bash call must stay attached for the whole Codex run. +> If a waiting background rescue is interrupted, that background rescue keeps running and remains recoverable with `/codex:status` and `/codex:result`; use `/codex:cancel` when you explicitly want to stop it. It supports `--background`, `--wait`, `--resume`, and `--fresh`. If you omit `--resume` and `--fresh`, the plugin can offer to continue the latest rescue thread for this repo. diff --git a/plugins/codex/agents/codex-rescue.md b/plugins/codex/agents/codex-rescue.md index 7009ec86a..38c45c143 100644 --- a/plugins/codex/agents/codex-rescue.md +++ b/plugins/codex/agents/codex-rescue.md @@ -19,13 +19,17 @@ Selection guidance: Forwarding rules: -- Use exactly one `Bash` call to invoke `node "${CLAUDE_PLUGIN_ROOT}/scripts/codex-companion.mjs" task ...`. -- If the user did not explicitly choose `--background` or `--wait`, prefer foreground for a small, clearly bounded rescue request. -- If the user did not explicitly choose `--background` or `--wait` and the task looks complicated, open-ended, multi-step, or likely to keep Codex running for a long time, prefer background execution. +- The outer `/codex:rescue` command owns background vs foreground execution of this subagent; do not reinterpret that choice inside the wrapper. +- Never set `run_in_background` on any Bash call. +- For a forwarded `--background` request, use the durable companion path: first run a foreground Bash call with `node "${CLAUDE_PLUGIN_ROOT}/scripts/codex-companion.mjs" task --background --json ...` and read `jobId` from its JSON. +- While that job is `queued` or `running`, use foreground Bash calls to `node "${CLAUDE_PLUGIN_ROOT}/scripts/codex-companion.mjs" status "$jobId" --wait --timeout-ms 60000 --json`. Each wait is bounded so no Bash call stays attached for the whole Codex run. +- When the background job is terminal, run `node "${CLAUDE_PLUGIN_ROOT}/scripts/codex-companion.mjs" result "$jobId" --raw` in a foreground Bash call and return that stdout exactly as-is. +- If this wrapper is interrupted after `jobId` exists, the detached job intentionally continues; a later `/codex:status` or `/codex:result` can recover it, and only an explicit `/codex:cancel` should stop it. +- For every other rescue, use a single foreground `Bash` call to invoke `node "${CLAUDE_PLUGIN_ROOT}/scripts/codex-companion.mjs" task ...` without `--background`, and return that stdout exactly as-is. - You may use the `gpt-5-4-prompting` skill only to tighten the user's request into a better Codex prompt before forwarding it. - Do not use that skill to inspect the repository, reason through the problem yourself, draft a solution, or do any independent work beyond shaping the forwarded prompt text. -- Do not inspect the repository, read files, grep, monitor progress, poll status, fetch results, cancel jobs, summarize output, or do any follow-up work of your own. -- Do not call `review`, `adversarial-review`, `status`, `result`, or `cancel`. This subagent only forwards to `task`. +- Do not inspect the repository, read files, grep, or solve the task yourself. The bounded `status` waits and final `result` lookup above are control-plane operations only. +- Do not call `review` or `adversarial-review`, and do not call `cancel`. Use `status` and `result` only for the background job created by this rescue. - Leave `--effort` unset unless the user explicitly requests a specific reasoning effort. - Leave model unset by default. Only add `--model` when the user explicitly asks for a specific model. - If the user asks for `spark`, map that to `--model gpt-5.3-codex-spark`. @@ -39,7 +43,7 @@ Forwarding rules: - Otherwise forward the task as a fresh `task` run. - Preserve the user's task text as-is apart from stripping routing flags. - Return the stdout of the `codex-companion` command exactly as-is. -- If the Bash call fails or Codex cannot be invoked, return nothing. +- If the foreground task or initial background launch fails, return nothing. Once a background `jobId` exists, do not redispatch the task because a later status lookup fails; the detached worker remains the authority for that run. Response style: diff --git a/plugins/codex/commands/rescue.md b/plugins/codex/commands/rescue.md index 56de9555d..9c827e03b 100644 --- a/plugins/codex/commands/rescue.md +++ b/plugins/codex/commands/rescue.md @@ -16,7 +16,7 @@ Execution mode: - If the request includes `--background`, run the `codex:codex-rescue` subagent in the background. - If the request includes `--wait`, run the `codex:codex-rescue` subagent in the foreground. - If neither flag is present, default to foreground. -- `--background` and `--wait` are execution flags for Claude Code. Do not forward them to `task`, and do not treat them as part of the natural-language task text. +- `--background` and `--wait` are execution controls, not natural-language task text. `--background` selects the subagent's durable detached-task protocol; `--wait` selects the attached foreground-task protocol. - `--model` and `--effort` are runtime-selection flags. Preserve them for the forwarded `task` call, but do not treat them as part of the natural-language task text. - If the request includes `--resume`, do not ask whether to continue. The user already chose. - If the request includes `--fresh`, do not ask whether to continue. The user already chose. @@ -38,10 +38,13 @@ node "${CLAUDE_PLUGIN_ROOT}/scripts/codex-companion.mjs" task-resume-candidate - Operating rules: -- The subagent is a thin forwarder only. It should use one `Bash` call to invoke `node "${CLAUDE_PLUGIN_ROOT}/scripts/codex-companion.mjs" task ...` and return that command's stdout as-is. +- The subagent is transport-only. The Agent itself is the only layer allowed to be backgrounded; inside the subagent, never set Bash `run_in_background`. +- A background rescue must use a durable `task --background --json` launch, capture its `jobId`, wait in bounded foreground calls with `status "$jobId" --wait --timeout-ms 60000 --json`, then return the stdout of `result "$jobId" --raw` exactly as-is. +- If the waiting subagent is interrupted after a background `jobId` exists, do not cancel the detached job; it remains authoritative and recoverable through `/codex:status` and `/codex:result` until it finishes or the user explicitly runs `/codex:cancel`. +- A foreground rescue uses one foreground Bash call to `task` without `--background` and returns that stdout as-is. - Return the Codex companion stdout verbatim to the user. - Do not paraphrase, summarize, rewrite, or add commentary before or after it. -- Do not ask the subagent to inspect files, monitor progress, poll `/codex:status`, fetch `/codex:result`, call `/codex:cancel`, summarize output, or do follow-up work of its own. +- Do not ask the subagent to inspect files, solve the task, call user-facing `/codex:status` / `/codex:result` / `/codex:cancel`, or summarize Codex output. The direct runtime `status` / `result` calls above are allowed only for its own background job. - Leave `--effort` unset unless the user explicitly asks for a specific reasoning effort. - Leave the model unset unless the user explicitly asks for one. If they ask for `spark`, map it to `gpt-5.3-codex-spark`. - Leave `--resume` and `--fresh` in the forwarded request. The subagent handles that routing when it builds the `task` command. diff --git a/plugins/codex/scripts/codex-companion.mjs b/plugins/codex/scripts/codex-companion.mjs index 83df468ad..1950f4628 100644 --- a/plugins/codex/scripts/codex-companion.mjs +++ b/plugins/codex/scripts/codex-companion.mjs @@ -30,9 +30,12 @@ import { generateJobId, getConfig, listJobs, + readJobPid, + removeJobPid, setConfig, upsertJob, - writeJobFile + writeJobFile, + writeJobPid } from "./lib/state.mjs"; import { buildSingleJobSnapshot, @@ -82,7 +85,7 @@ function printUsage() { " 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]", - " node scripts/codex-companion.mjs result [job-id] [--json]", + " node scripts/codex-companion.mjs result [job-id] [--json|--raw]", " node scripts/codex-companion.mjs cancel [job-id] [--json]" ].join("\n") ); @@ -685,17 +688,22 @@ function enqueueBackgroundTask(cwd, job, request) { const { logFile } = createTrackedProgress(job); appendLogLine(logFile, "Queued for background execution."); - const child = spawnDetachedTaskWorker(cwd, job.id); const queuedRecord = { ...job, status: "queued", phase: "queued", - pid: child.pid ?? null, + pid: null, logFile, request }; + // Persist the request before the detached worker can start reading it. The worker + // becomes the PID authority when runTrackedJob records its own process.pid. writeJobFile(job.workspaceRoot, job.id, queuedRecord); upsertJob(job.workspaceRoot, queuedRecord); + // After spawn, the parent does not touch mutable job state. The worker publishes + // its own PID before reading queued state, while cancellation claims terminal state + // before PID discovery, so either side of startup is safe without a parent read/write race. + spawnDetachedTaskWorker(cwd, job.id); return { payload: { @@ -846,13 +854,20 @@ async function handleTaskWorker(argv) { const cwd = resolveCommandCwd(options); const workspaceRoot = resolveCommandWorkspace(options); + writeJobPid(workspaceRoot, options["job-id"], process.pid); const storedJob = readStoredJob(workspaceRoot, options["job-id"]); if (!storedJob) { + removeJobPid(workspaceRoot, options["job-id"]); throw new Error(`No stored job found for ${options["job-id"]}.`); } + if (storedJob.status !== "queued") { + removeJobPid(workspaceRoot, options["job-id"]); + return; + } const request = storedJob.request; if (!request || typeof request !== "object") { + removeJobPid(workspaceRoot, options["job-id"]); throw new Error(`Stored job ${options["job-id"]} is missing its task request payload.`); } @@ -865,19 +880,23 @@ async function handleTaskWorker(argv) { logFile: storedJob.logFile ?? null } ); - await runTrackedJob( - { - ...storedJob, - workspaceRoot, - logFile - }, - () => - executeTaskRun({ - ...request, - onProgress: progress - }), - { logFile } - ); + try { + await runTrackedJob( + { + ...storedJob, + workspaceRoot, + logFile + }, + () => + executeTaskRun({ + ...request, + onProgress: progress + }), + { logFile } + ); + } finally { + removeJobPid(workspaceRoot, options["job-id"]); + } } async function handleStatus(argv) { @@ -907,16 +926,40 @@ async function handleStatus(argv) { outputResult(renderStatusPayload(report, options.json), options.json); } +function extractStoredCodexRawOutput(storedJob) { + if (typeof storedJob?.result?.rawOutput === "string") { + return storedJob.result.rawOutput; + } + if (typeof storedJob?.result?.codex?.stdout === "string") { + return storedJob.result.codex.stdout; + } + return null; +} + function handleResult(argv) { const { options, positionals } = parseCommandInput(argv, { valueOptions: ["cwd"], - booleanOptions: ["json"] + booleanOptions: ["json", "raw"] }); + if (options.json && options.raw) { + throw new Error("`result --json` and `result --raw` are mutually exclusive."); + } + const cwd = resolveCommandCwd(options); const reference = positionals[0] ?? ""; const { workspaceRoot, job } = resolveResultJob(cwd, reference); const storedJob = readStoredJob(workspaceRoot, job.id); + + if (options.raw) { + const rawOutput = extractStoredCodexRawOutput(storedJob); + if (rawOutput == null) { + throw new Error(`No raw Codex output was stored for job ${job.id}.`); + } + process.stdout.write(rawOutput); + return; + } + const payload = { job, storedJob @@ -983,9 +1026,6 @@ async function handleCancel(argv) { ); } - terminateProcessTree(job.pid ?? Number.NaN); - appendLogLine(job.logFile, "Cancelled by user."); - const completedAt = nowIso(); const nextJob = { ...job, @@ -995,20 +1035,32 @@ async function handleCancel(argv) { completedAt, errorMessage: "Cancelled by user." }; + const persistCancellation = () => { + writeJobFile(workspaceRoot, job.id, { + ...existing, + ...nextJob, + cancelledAt: completedAt + }); + upsertJob(workspaceRoot, { + id: job.id, + status: "cancelled", + phase: "cancelled", + pid: null, + errorMessage: "Cancelled by user.", + completedAt + }); + }; - writeJobFile(workspaceRoot, job.id, { - ...existing, - ...nextJob, - cancelledAt: completedAt - }); - upsertJob(workspaceRoot, { - id: job.id, - status: "cancelled", - phase: "cancelled", - pid: null, - errorMessage: "Cancelled by user.", - completedAt - }); + // Claim terminal cancellation before PID discovery so a worker that starts + // concurrently must either observe cancelled state or expose a PID we can stop. + persistCancellation(); + const workerPid = Number.isFinite(job.pid) ? job.pid : readJobPid(workspaceRoot, job.id); + terminateProcessTree(workerPid ?? Number.NaN); + removeJobPid(workspaceRoot, job.id); + appendLogLine(job.logFile, "Cancelled by user."); + // Reassert the terminal state in case an already-starting worker published + // `running` between the initial cancellation claim and process termination. + persistCancellation(); const payload = { jobId: job.id, diff --git a/plugins/codex/scripts/lib/state.mjs b/plugins/codex/scripts/lib/state.mjs index 2da23498f..080ce8b58 100644 --- a/plugins/codex/scripts/lib/state.mjs +++ b/plugins/codex/scripts/lib/state.mjs @@ -108,6 +108,7 @@ export function saveState(cwd, state) { continue; } removeJobFile(resolveJobFile(cwd, job.id)); + removeFileIfExists(resolveJobPidFile(cwd, job.id)); removeFileIfExists(job.logFile); } @@ -189,3 +190,31 @@ export function resolveJobFile(cwd, jobId) { ensureStateDir(cwd); return path.join(resolveJobsDir(cwd), `${jobId}.json`); } + +export function resolveJobPidFile(cwd, jobId) { + ensureStateDir(cwd); + return path.join(resolveJobsDir(cwd), `${jobId}.pid`); +} + +export function writeJobPid(cwd, jobId, pid) { + const pidFile = resolveJobPidFile(cwd, jobId); + if (!Number.isFinite(pid)) { + removeFileIfExists(pidFile); + return null; + } + fs.writeFileSync(pidFile, `${pid}\n`, "utf8"); + return pidFile; +} + +export function readJobPid(cwd, jobId) { + const pidFile = resolveJobPidFile(cwd, jobId); + if (!fs.existsSync(pidFile)) { + return null; + } + const pid = Number.parseInt(fs.readFileSync(pidFile, "utf8").trim(), 10); + return Number.isFinite(pid) ? pid : null; +} + +export function removeJobPid(cwd, jobId) { + removeFileIfExists(resolveJobPidFile(cwd, jobId)); +} diff --git a/plugins/codex/skills/codex-cli-runtime/SKILL.md b/plugins/codex/skills/codex-cli-runtime/SKILL.md index 0e91bfb50..1b8848f3c 100644 --- a/plugins/codex/skills/codex-cli-runtime/SKILL.md +++ b/plugins/codex/skills/codex-cli-runtime/SKILL.md @@ -12,9 +12,9 @@ Primary helper: - `node "${CLAUDE_PLUGIN_ROOT}/scripts/codex-companion.mjs" task ""` Execution rules: -- The rescue subagent is a forwarder, not an orchestrator. Its only job is to invoke `task` once and return that stdout unchanged. +- The rescue subagent is a transport wrapper, not a coding orchestrator. Foreground rescues use one attached `task` call; background rescues use a detached task plus bounded status waits and a final result lookup. - Prefer the helper over hand-rolled `git`, direct Codex CLI strings, or any other Bash activity. -- Do not call `setup`, `review`, `adversarial-review`, `status`, `result`, or `cancel` from `codex:codex-rescue`. +- Do not call `setup`, `review`, or `adversarial-review` from `codex:codex-rescue`. Do not call `cancel`. Use `status` and `result` only for the detached background job launched by this rescue. - Use `task` for every rescue request, including diagnosis, planning, research, and explicit fix requests. - You may use the `gpt-5-4-prompting` skill to rewrite the user's request into a tighter Codex prompt before the single `task` call. - That prompt drafting is the only Claude-side work allowed. Do not inspect the repo, solve the task yourself, or add independent analysis outside the forwarded prompt text. @@ -24,8 +24,11 @@ Execution rules: - Default to a write-capable Codex run by adding `--write` unless the user explicitly asks for read-only behavior or only wants review, diagnosis, or research without edits. Command selection: -- Use exactly one `task` invocation per rescue handoff. -- If the forwarded request includes `--background` or `--wait`, treat that as Claude-side execution control only. Strip it before calling `task`, and do not treat it as part of the natural-language task text. +- Use exactly one `task` launch per rescue handoff. Never run the Bash tool in background from this subagent. +- The outer `/codex:rescue` command owns whether the rescue subagent runs in the background or foreground. Do not treat `--background` or `--wait` as natural-language task text. +- For `--background`, launch `task --background --json` in a foreground Bash call, capture `jobId`, then use foreground `status "$jobId" --wait --timeout-ms 60000 --json` calls until terminal and finish with `result "$jobId" --raw`. +- Keep each background status wait bounded to 60 seconds. The detached task worker survives between waits and remains recoverable if the wrapper is interrupted. Wrapper interruption after a `jobId` exists does not cancel it; cancellation is an explicit user action through `/codex:cancel`. +- For `--wait` or a foreground rescue, strip the execution flag and call `task` without `--background` so the Bash call returns Codex's final stdout directly. - If the forwarded request includes `--model`, normalize `spark` to `gpt-5.3-codex-spark` and pass it through to `task`. - If the forwarded request includes `--effort`, pass it through to `task`. - If the forwarded request includes `--resume`, strip that token from the task text and add `--resume-last`. @@ -38,6 +41,6 @@ Command selection: Safety rules: - Default to write-capable Codex work in `codex:codex-rescue` unless the user explicitly asks for read-only behavior. - Preserve the user's task text as-is apart from stripping routing flags. -- Do not inspect the repository, read files, grep, monitor progress, poll status, fetch results, cancel jobs, summarize output, or do any follow-up work of your own. -- Return the stdout of the `task` command exactly as-is. -- If the Bash call fails or Codex cannot be invoked, return nothing. +- Do not inspect the repository, read files, grep, or solve the task yourself. Background `status` waits and the final `result` lookup are the only permitted follow-up operations. +- Return the final Codex companion stdout exactly as-is: the attached `task` stdout for foreground rescues, or the final `result "$jobId" --raw` stdout for background rescues. +- If the foreground task or initial background launch fails, return nothing. Once a background `jobId` exists, do not redispatch it because a later status lookup fails. diff --git a/tests/commands.test.mjs b/tests/commands.test.mjs index c34b06059..f7b26dbf7 100644 --- a/tests/commands.test.mjs +++ b/tests/commands.test.mjs @@ -111,7 +111,7 @@ test("rescue command absorbs continue semantics", () => { assert.match(rescue, /Start a new Codex thread/); assert.match(rescue, /run the `codex:codex-rescue` subagent in the background/i); assert.match(rescue, /default to foreground/i); - assert.match(rescue, /Do not forward them to `task`/i); + assert.match(rescue, /execution controls, not natural-language task text/i); assert.match(rescue, /`--model` and `--effort` are runtime-selection flags/i); assert.match(rescue, /Leave `--effort` unset unless the user explicitly asks for a specific reasoning effort/i); assert.match(rescue, /If they ask for `spark`, map it to `gpt-5\.3-codex-spark`/i); @@ -119,45 +119,62 @@ test("rescue command absorbs continue semantics", () => { assert.match(rescue, /If the request includes `--fresh`, do not ask whether to continue/i); assert.match(rescue, /If the user chooses continue, add `--resume`/i); assert.match(rescue, /If the user chooses a new thread, add `--fresh`/i); - assert.match(rescue, /thin forwarder only/i); + assert.match(rescue, /subagent is transport-only/i); + assert.match(rescue, /Agent itself is the only layer allowed to be backgrounded/i); + assert.match(rescue, /never set Bash `run_in_background`/i); assert.match(rescue, /Return the Codex companion stdout verbatim to the user/i); assert.match(rescue, /Do not paraphrase, summarize, rewrite, or add commentary before or after it/i); - assert.match(rescue, /return that command's stdout as-is/i); + assert.match(rescue, /return.*stdout.*exactly as-is/i); assert.match(rescue, /Leave `--resume` and `--fresh` in the forwarded request/i); assert.match(agent, /--resume/); assert.match(agent, /--fresh/); assert.match(agent, /thin forwarding wrapper/i); - assert.match(agent, /prefer foreground for a small, clearly bounded rescue request/i); - assert.match(agent, /If the user did not explicitly choose `--background` or `--wait` and the task looks complicated, open-ended, multi-step, or likely to keep Codex running for a long time, prefer background execution/i); - assert.match(agent, /Use exactly one `Bash` call/i); - assert.match(agent, /Do not inspect the repository, read files, grep, monitor progress, poll status, fetch results, cancel jobs, summarize output, or do any follow-up work of your own/i); - assert.match(agent, /Do not call `review`, `adversarial-review`, `status`, `result`, or `cancel`/i); + assert.match(agent, /Never set `run_in_background` on any Bash call/i); + assert.match(agent, /outer `\/codex:rescue` command owns background vs foreground execution/i); + assert.doesNotMatch(agent, /prefer background execution/i); + assert.match(agent, /For a forwarded `--background` request/i); + assert.match(agent, /task --background --json/i); + assert.match(agent, /status .*--wait.*--timeout-ms 60000.*--json/i); + assert.match(agent, /result .*job.*--raw/i); + assert.match(agent, /For every other rescue/i); + assert.match(agent, /single foreground `Bash` call/i); + assert.match(agent, /Do not inspect the repository, read files, grep, or solve the task yourself/i); + assert.match(agent, /Do not call `review` or `adversarial-review`/i); assert.match(agent, /Leave `--effort` unset unless the user explicitly requests a specific reasoning effort/i); assert.match(agent, /Leave model unset by default/i); assert.match(agent, /If the user asks for `spark`, map that to `--model gpt-5\.3-codex-spark`/i); assert.match(agent, /If the user asks for a concrete model name such as `gpt-5\.4-mini`, pass it through with `--model`/i); assert.match(agent, /Return the stdout of the `codex-companion` command exactly as-is/i); - assert.match(agent, /If the Bash call fails or Codex cannot be invoked, return nothing/i); + assert.match(agent, /foreground task or initial background launch fails, return nothing/i); assert.match(agent, /gpt-5-4-prompting/); assert.match(agent, /only to tighten the user's request into a better Codex prompt/i); assert.match(agent, /Do not use that skill to inspect the repository, reason through the problem yourself, draft a solution, or do any independent work/i); - assert.match(runtimeSkill, /only job is to invoke `task` once and return that stdout unchanged/i); - assert.match(runtimeSkill, /Do not call `setup`, `review`, `adversarial-review`, `status`, `result`, or `cancel`/i); + assert.match(runtimeSkill, /foreground rescues use one attached `task` call/i); + assert.match(runtimeSkill, /background rescues use a detached task plus bounded status waits/i); + assert.match(runtimeSkill, /Do not call `setup`, `review`, or `adversarial-review`/i); assert.match(runtimeSkill, /use the `gpt-5-4-prompting` skill to rewrite the user's request into a tighter Codex prompt/i); assert.match(runtimeSkill, /That prompt drafting is the only Claude-side work allowed/i); assert.match(runtimeSkill, /Leave `--effort` unset unless the user explicitly requests a specific effort/i); assert.match(runtimeSkill, /Leave model unset by default/i); assert.match(runtimeSkill, /Map `spark` to `--model gpt-5\.3-codex-spark`/i); - assert.match(runtimeSkill, /If the forwarded request includes `--background` or `--wait`, treat that as Claude-side execution control only/i); - assert.match(runtimeSkill, /Strip it before calling `task`/i); + assert.match(runtimeSkill, /Do not treat `--background` or `--wait` as natural-language task text/i); + assert.match(runtimeSkill, /Never run the Bash tool in background/i); + assert.match(runtimeSkill, /foreground rescue.*call `task` without `--background`/i); assert.match(runtimeSkill, /`--effort`: accepted values are `none`, `minimal`, `low`, `medium`, `high`, `xhigh`/i); - assert.match(runtimeSkill, /Do not inspect the repository, read files, grep, monitor progress, poll status, fetch results, cancel jobs, summarize output, or do any follow-up work of your own/i); - assert.match(runtimeSkill, /If the Bash call fails or Codex cannot be invoked, return nothing/i); + assert.match(runtimeSkill, /Background `status` waits and the final `result` lookup are the only permitted follow-up operations/i); + assert.match(runtimeSkill, /result .*--raw/i); + assert.match(runtimeSkill, /foreground task or initial background launch fails, return nothing/i); assert.match(readme, /`codex:codex-rescue` subagent/i); assert.match(readme, /if you do not pass `--model` or `--effort`, Codex chooses its own defaults/i); assert.match(readme, /--model gpt-5\.4-mini --effort medium/i); assert.match(readme, /`spark`, the plugin maps that to `gpt-5\.3-codex-spark`/i); assert.match(readme, /continue a previous Codex task/i); + assert.match(readme, /durable background worker/i); + assert.match(readme, /bounded status waits/i); + assert.match(rescue, /interrupted after a background `jobId` exists.*do not cancel/i); + assert.match(agent, /interrupted after `jobId` exists.*detached job.*continues/i); + assert.match(runtimeSkill, /wrapper interruption.*does not cancel/i); + assert.match(readme, /interrupted.*background rescue.*keeps running.*`\/codex:cancel`/i); assert.match(readme, /### `\/codex:setup`/); assert.match(readme, /### `\/codex:review`/); assert.match(readme, /### `\/codex:adversarial-review`/); diff --git a/tests/runtime.test.mjs b/tests/runtime.test.mjs index 8f276835b..f1d7c1a23 100644 --- a/tests/runtime.test.mjs +++ b/tests/runtime.test.mjs @@ -920,6 +920,115 @@ test("task using the shared broker still completes when Codex spawns subagents", assert.equal(result.stdout, "Handled the requested task.\nTask prompt accepted.\n"); }); +test("background task persists its queued request before spawning the detached worker", () => { + const source = fs.readFileSync(SCRIPT, "utf8"); + const start = source.indexOf("function enqueueBackgroundTask"); + const end = source.indexOf("\nasync function handleReviewCommand", start); + assert.ok(start >= 0 && end > start, "enqueueBackgroundTask source must be discoverable"); + const body = source.slice(start, end); + const writeJob = body.indexOf("writeJobFile("); + const upsertJob = body.indexOf("upsertJob("); + const spawnWorker = body.indexOf("spawnDetachedTaskWorker("); + assert.ok(writeJob >= 0 && writeJob < spawnWorker, "job file must exist before worker spawn"); + assert.ok(upsertJob >= 0 && upsertJob < spawnWorker, "queued state must exist before worker spawn"); +}); + +test("background parent does not touch mutable job state after spawning the worker", () => { + const source = fs.readFileSync(SCRIPT, "utf8"); + const start = source.indexOf("function enqueueBackgroundTask"); + const end = source.indexOf("\nasync function handleReviewCommand", start); + assert.ok(start >= 0 && end > start, "enqueueBackgroundTask source must be discoverable"); + const body = source.slice(start, end); + const spawnWorker = body.indexOf("spawnDetachedTaskWorker("); + assert.ok(spawnWorker >= 0, "detached worker must be spawned"); + const afterSpawn = body.slice(spawnWorker); + assert.equal(afterSpawn.includes("readStoredJob("), false, "parent must not parse the mutable job file after worker spawn"); + assert.equal(afterSpawn.includes("writeJobFile("), false, "parent must not rewrite the job file after worker spawn"); + assert.equal(afterSpawn.includes("upsertJob("), false, "parent must not rewrite indexed state after worker spawn"); + assert.equal(afterSpawn.includes("writeJobPid("), false, "worker must be the sole PID-sidecar writer after spawn"); +}); + +test("task-worker publishes its own pid before reading queued state", () => { + const source = fs.readFileSync(SCRIPT, "utf8"); + const start = source.indexOf("async function handleTaskWorker"); + const end = source.indexOf("\nasync function handleStatus", start); + assert.ok(start >= 0 && end > start, "handleTaskWorker source must be discoverable"); + const body = source.slice(start, end); + const publishPid = body.indexOf("writeJobPid(workspaceRoot, options[\"job-id\"], process.pid)"); + const readJob = body.indexOf("readStoredJob(workspaceRoot, options[\"job-id\"])"); + assert.ok(publishPid >= 0, "worker must publish its own pid during startup"); + assert.ok(readJob >= 0, "worker must read queued state"); + assert.ok(publishPid < readJob, "worker pid must be visible to cancellation before queued state is claimed"); +}); + +test("cancel records terminal state before reading the worker pid and reasserts it after termination", () => { + const source = fs.readFileSync(SCRIPT, "utf8"); + const start = source.indexOf("async function handleCancel"); + const end = source.indexOf("\nasync function main", start); + assert.ok(start >= 0 && end > start, "handleCancel source must be discoverable"); + const body = source.slice(start, end); + const firstClaim = body.indexOf("persistCancellation();"); + const readPid = body.indexOf("readJobPid(workspaceRoot, job.id)"); + const terminate = body.indexOf("terminateProcessTree("); + const finalClaim = body.indexOf("persistCancellation();", firstClaim + 1); + assert.ok(firstClaim >= 0, "cancel must claim terminal state"); + assert.ok(readPid > firstClaim, "cancelled state must be persisted before pid lookup"); + assert.ok(terminate > readPid, "pid lookup must precede process termination"); + assert.ok(finalClaim > terminate, "cancelled state must be reasserted after termination"); +}); + +test("task-worker does not revive a job cancelled before worker startup", () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + installFakeCodex(binDir); + 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 stateDir = resolveStateDir(repo); + const jobsDir = path.join(stateDir, "jobs"); + fs.mkdirSync(jobsDir, { recursive: true }); + const jobId = "task-cancelled-before-worker"; + const record = { + id: jobId, + kind: "task", + jobClass: "task", + title: "Cancelled rescue", + summary: "Cancelled before worker startup", + workspaceRoot: repo, + status: "cancelled", + phase: "cancelled", + pid: null, + write: true, + request: { + cwd: repo, + model: null, + effort: null, + prompt: "do not run this task", + write: true, + resumeLast: false, + jobId + } + }; + fs.writeFileSync(path.join(jobsDir, `${jobId}.json`), `${JSON.stringify(record, null, 2)}\n`, "utf8"); + fs.writeFileSync( + path.join(stateDir, "state.json"), + `${JSON.stringify({ version: 1, config: { stopReviewGate: false }, jobs: [record] }, null, 2)}\n`, + "utf8" + ); + + const worker = run("node", [SCRIPT, "task-worker", "--cwd", repo, "--job-id", jobId], { + cwd: repo, + env: buildEnv(binDir) + }); + assert.equal(worker.status, 0, worker.stderr); + const stored = JSON.parse(fs.readFileSync(path.join(jobsDir, `${jobId}.json`), "utf8")); + const state = JSON.parse(fs.readFileSync(path.join(stateDir, "state.json"), "utf8")); + assert.equal(stored.status, "cancelled"); + assert.equal(state.jobs.find((job) => job.id === jobId)?.status, "cancelled"); +}); + test("task --background enqueues a detached worker and exposes per-job status", async () => { const repo = makeTempDir(); const binDir = makeTempDir(); @@ -2257,3 +2366,45 @@ 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("result --raw returns only stored Codex output", () => { + const workspace = makeTempDir(); + const stateDir = resolveStateDir(workspace); + const jobsDir = path.join(stateDir, "jobs"); + fs.mkdirSync(jobsDir, { recursive: true }); + + fs.writeFileSync( + path.join(jobsDir, "task-raw.json"), + JSON.stringify({ + id: "task-raw", + status: "completed", + title: "Codex Task", + threadId: "thr_raw", + result: { codex: { stdout: "RAW CODEX OUTPUT\nsecond line" } } + }, null, 2), + "utf8" + ); + fs.writeFileSync( + path.join(stateDir, "state.json"), + `${JSON.stringify({ + version: 1, + config: { stopReviewGate: false }, + jobs: [{ + id: "task-raw", + status: "completed", + title: "Codex Task", + jobClass: "task", + threadId: "thr_raw", + summary: "background rescue", + createdAt: "2026-09-05T17:00:00.000Z", + updatedAt: "2026-09-05T17:01:00.000Z" + }] + }, null, 2)}\n`, + "utf8" + ); + + const result = run("node", [SCRIPT, "result", "task-raw", "--raw"], { cwd: workspace }); + assert.equal(result.status, 0, result.stderr); + assert.equal(result.stdout, "RAW CODEX OUTPUT\nsecond line"); +});