diff --git a/plugins/codex/agents/codex-rescue.md b/plugins/codex/agents/codex-rescue.md index 7009ec86a..2e8a028de 100644 --- a/plugins/codex/agents/codex-rescue.md +++ b/plugins/codex/agents/codex-rescue.md @@ -22,6 +22,8 @@ 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. +- For any task expected to exceed a few minutes, add `--background` to the `task` invocation by default so a caller timeout cannot terminate it. +- Treat `--cwd ` and `-C ` as workspace routing controls. Pass `--cwd ` explicitly on every `task` invocation, using the intended workspace root forwarded by the caller. - 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. diff --git a/plugins/codex/commands/rescue.md b/plugins/codex/commands/rescue.md index 56de9555d..28e0d438d 100644 --- a/plugins/codex/commands/rescue.md +++ b/plugins/codex/commands/rescue.md @@ -1,6 +1,6 @@ --- description: Delegate investigation, an explicit fix request, or follow-up rescue work to the Codex rescue subagent -argument-hint: "[--background|--wait] [--resume|--fresh] [--model ] [--effort ] [what Codex should investigate, solve, or continue]" +argument-hint: "[--background|--wait] [--cwd |-C ] [--resume|--fresh] [--model ] [--effort ] [what Codex should investigate, solve, or continue]" allowed-tools: Bash(node:*), AskUserQuestion, Agent --- @@ -18,6 +18,7 @@ Execution mode: - 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. - `--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. +- `--cwd ` and `-C ` are workspace-routing flags. Preserve the directory for the resume preflight and the forwarded `task` call, but do not treat either form 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. - Otherwise, before starting Codex, check for a resumable rescue thread from this Claude session by running: @@ -26,6 +27,12 @@ Execution mode: node "${CLAUDE_PLUGIN_ROOT}/scripts/codex-companion.mjs" task-resume-candidate --json ``` +- If the request includes `--cwd ` or `-C `, pass the same directory to that helper as `--cwd `: + +```bash +node "${CLAUDE_PLUGIN_ROOT}/scripts/codex-companion.mjs" task-resume-candidate --json --cwd "" +``` + - If that helper reports `available: true`, use `AskUserQuestion` exactly once to ask whether to continue the current Codex thread or start a new one. - The two choices must be: - `Continue current Codex thread` @@ -39,6 +46,7 @@ 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. +- Pass `--cwd ` explicitly for the intended workspace root. For work expected to exceed a few minutes, have the subagent add the task command's own `--background` flag so a caller timeout cannot terminate it. - 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. diff --git a/plugins/codex/scripts/codex-companion.mjs b/plugins/codex/scripts/codex-companion.mjs index 83df468ad..599c73c92 100644 --- a/plugins/codex/scripts/codex-companion.mjs +++ b/plugins/codex/scripts/codex-companion.mjs @@ -40,6 +40,7 @@ import { readStoredJob, resolveCancelableJob, resolveResultJob, + settleCancellationAfterTermination, sortJobsNewestFirst } from "./lib/job-control.mjs"; import { @@ -68,6 +69,7 @@ const ROOT_DIR = path.resolve(fileURLToPath(new URL("..", import.meta.url))); const REVIEW_SCHEMA = path.join(ROOT_DIR, "schemas", "review-output.schema.json"); const DEFAULT_STATUS_WAIT_TIMEOUT_MS = 240000; const DEFAULT_STATUS_POLL_INTERVAL_MS = 2000; +const TASK_WORKER_RECORD_WAIT_TIMEOUT_MS = 1000; const VALID_REASONING_EFFORTS = new Set(["none", "minimal", "low", "medium", "high", "xhigh"]); const MODEL_ALIASES = new Map([["spark", "gpt-5.3-codex-spark"]]); const STOP_REVIEW_TASK_MARKER = "Run a stop-gate review of the previous Claude turn."; @@ -79,7 +81,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 task [--background] [--write] [--resume-last|--resume|--fresh] [--model ] [--effort ] [prompt]", + " node scripts/codex-companion.mjs task [--background] [--write] [--cwd ] [--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]", @@ -156,10 +158,41 @@ function resolveCommandWorkspace(options = {}) { return resolveWorkspaceRoot(resolveCommandCwd(options)); } +function resolveTaskCwd(options = {}) { + const cwd = resolveCommandCwd(options); + if (!options.cwd) { + return cwd; + } + + let stats; + try { + stats = fs.statSync(cwd); + } catch (error) { + if (error?.code === "ENOENT") { + throw new Error(`Task workspace directory does not exist: ${cwd}`); + } + throw error; + } + if (!stats.isDirectory()) { + throw new Error(`Task workspace path is not a directory: ${cwd}`); + } + return cwd; +} + function sleep(ms) { return new Promise((resolve) => setTimeout(resolve, ms)); } +async function waitForStoredJob(workspaceRoot, jobId) { + const deadline = Date.now() + TASK_WORKER_RECORD_WAIT_TIMEOUT_MS; + let storedJob = readStoredJob(workspaceRoot, jobId); + while (!storedJob && Date.now() < deadline) { + await sleep(25); + storedJob = readStoredJob(workspaceRoot, jobId); + } + return storedJob; +} + function shorten(text, limit = 96) { const normalized = String(text ?? "").trim().replace(/\s+/g, " "); if (!normalized) { @@ -768,7 +801,7 @@ async function handleTask(argv) { } }); - const cwd = resolveCommandCwd(options); + const cwd = resolveTaskCwd(options); const workspaceRoot = resolveCommandWorkspace(options); const model = normalizeRequestedModel(options.model); const effort = normalizeReasoningEffort(options.effort); @@ -846,9 +879,13 @@ async function handleTaskWorker(argv) { const cwd = resolveCommandCwd(options); const workspaceRoot = resolveCommandWorkspace(options); - const storedJob = readStoredJob(workspaceRoot, options["job-id"]); + const storedJob = await waitForStoredJob(workspaceRoot, options["job-id"]); if (!storedJob) { - throw new Error(`No stored job found for ${options["job-id"]}.`); + return; + } + if (storedJob.status === "cancelled") { + appendLogLine(storedJob.logFile, "Skipped cancelled background job."); + return; } const request = storedJob.request; @@ -931,8 +968,8 @@ function handleTaskResumeCandidate(argv) { booleanOptions: ["json"] }); - const cwd = resolveCommandCwd(options); - const workspaceRoot = resolveCommandWorkspace(options); + const cwd = resolveTaskCwd(options); + const workspaceRoot = resolveWorkspaceRoot(cwd); const sessionId = getCurrentClaudeSessionId(); const jobs = filterJobsForCurrentClaudeSession(sortJobsNewestFirst(listJobs(workspaceRoot))); const candidate = findLatestResumableTaskJob(jobs); @@ -972,6 +1009,34 @@ async function handleCancel(argv) { const existing = readStoredJob(workspaceRoot, job.id) ?? {}; const threadId = existing.threadId ?? job.threadId ?? null; const turnId = existing.turnId ?? job.turnId ?? null; + const completedAt = nowIso(); + const cancellingJob = { + ...job, + status: "cancelled", + phase: "cancelled", + completedAt, + errorMessage: "Cancelled by user." + }; + const persistCancellation = (pid) => { + const cancelledJob = { ...cancellingJob, pid }; + writeJobFile(workspaceRoot, job.id, { + ...existing, + ...cancelledJob, + cancelledAt: completedAt + }); + upsertJob(workspaceRoot, { + id: job.id, + status: "cancelled", + phase: "cancelled", + pid, + errorMessage: "Cancelled by user.", + completedAt + }); + return cancelledJob; + }; + + persistCancellation(job.pid ?? null); + appendLogLine(job.logFile, "Cancelled by user."); const interrupt = await interruptAppServerTurn(cwd, { threadId, turnId }); if (interrupt.attempted) { @@ -983,32 +1048,27 @@ async function handleCancel(argv) { ); } - terminateProcessTree(job.pid ?? Number.NaN); - appendLogLine(job.logFile, "Cancelled by user."); + let termination = null; + let terminationError = null; + try { + termination = terminateProcessTree(job.pid ?? Number.NaN); + } catch (error) { + terminationError = error; + } - const completedAt = nowIso(); - const nextJob = { - ...job, - status: "cancelled", - phase: "cancelled", - pid: null, - completedAt, - errorMessage: "Cancelled by user." - }; + const outcome = settleCancellationAfterTermination( + workspaceRoot, + job, + existing, + termination, + terminationError + ); + if (!outcome.processStopped) { + appendLogLine(job.logFile, outcome.job.errorMessage); + throw outcome.error; + } - writeJobFile(workspaceRoot, job.id, { - ...existing, - ...nextJob, - cancelledAt: completedAt - }); - upsertJob(workspaceRoot, { - id: job.id, - status: "cancelled", - phase: "cancelled", - pid: null, - errorMessage: "Cancelled by user.", - completedAt - }); + const nextJob = persistCancellation(null); const payload = { jobId: job.id, diff --git a/plugins/codex/scripts/lib/args.mjs b/plugins/codex/scripts/lib/args.mjs index 6b1518502..f82b11794 100644 --- a/plugins/codex/scripts/lib/args.mjs +++ b/plugins/codex/scripts/lib/args.mjs @@ -73,21 +73,29 @@ export function parseArgs(argv, config = {}) { return { options, positionals }; } -export function splitRawArgumentString(raw) { +function parseRawArgumentString(raw, literalClosingQuoteBackslash) { const tokens = []; let current = ""; let quote = null; - let escaping = false; - for (const character of raw) { - if (escaping) { - current += character; - escaping = false; - continue; - } + for (let index = 0; index < raw.length; index += 1) { + const character = raw[index]; if (character === "\\") { - escaping = true; + const nextCharacter = raw[index + 1]; + if (literalClosingQuoteBackslash && quote !== null && nextCharacter === quote) { + current += character; + continue; + } + if ( + nextCharacter !== undefined && + (nextCharacter === "'" || nextCharacter === "\"" || nextCharacter === "\\" || /\s/.test(nextCharacter)) + ) { + current += nextCharacter; + index += 1; + } else { + current += character; + } continue; } @@ -116,13 +124,18 @@ export function splitRawArgumentString(raw) { current += character; } - if (escaping) { - current += "\\"; - } - if (current) { tokens.push(current); } - return tokens; + return { tokens, openQuote: quote }; +} + +export function splitRawArgumentString(raw) { + const escapePreferred = parseRawArgumentString(raw, false); + if (escapePreferred.openQuote === null) { + return escapePreferred.tokens; + } + + return parseRawArgumentString(raw, true).tokens; } diff --git a/plugins/codex/scripts/lib/job-control.mjs b/plugins/codex/scripts/lib/job-control.mjs index ad152c157..791f45063 100644 --- a/plugins/codex/scripts/lib/job-control.mjs +++ b/plugins/codex/scripts/lib/job-control.mjs @@ -1,12 +1,15 @@ import fs from "node:fs"; import { getSessionRuntimeStatus } from "./codex.mjs"; -import { getConfig, listJobs, readJobFile, resolveJobFile } from "./state.mjs"; +import { isProcessAlive } from "./process.mjs"; +import { getConfig, listJobs, readJobFile, resolveJobFile, upsertJob, writeJobFile } from "./state.mjs"; import { SESSION_ID_ENV } from "./tracked-jobs.mjs"; import { resolveWorkspaceRoot } from "./workspace.mjs"; export const DEFAULT_MAX_STATUS_JOBS = 8; export const DEFAULT_MAX_PROGRESS_LINES = 4; +export const CANCELLATION_TERMINATION_FAILED_MESSAGE = + "Cancellation requested but process termination failed; retry /codex:cancel."; export function sortJobsNewestFirst(jobs) { return [...jobs].sort((left, right) => String(right.updatedAt ?? "").localeCompare(String(left.updatedAt ?? ""))); @@ -188,6 +191,48 @@ export function readStoredJob(workspaceRoot, jobId) { return readJobFile(jobFile); } +export function settleCancellationAfterTermination( + workspaceRoot, + job, + existing, + termination, + terminationError = null, + options = {} +) { + const terminationFailed = Boolean(terminationError) || termination?.delivered !== true; + const pid = job.pid ?? null; + const isProcessAliveImpl = options.isProcessAliveImpl ?? isProcessAlive; + + if (!terminationFailed || !Number.isInteger(pid) || pid <= 0 || !isProcessAliveImpl(pid)) { + return { processStopped: true, job: null, error: null }; + } + + const restoredJob = { + ...existing, + ...job, + status: job.status, + phase: job.phase ?? existing.phase ?? job.status, + pid, + errorMessage: CANCELLATION_TERMINATION_FAILED_MESSAGE + }; + writeJobFile(workspaceRoot, job.id, restoredJob); + upsertJob(workspaceRoot, { + ...job, + status: job.status, + phase: restoredJob.phase, + pid, + completedAt: job.completedAt ?? null, + cancelledAt: job.cancelledAt ?? null, + errorMessage: CANCELLATION_TERMINATION_FAILED_MESSAGE + }); + + return { + processStopped: false, + job: restoredJob, + error: terminationError ?? new Error(CANCELLATION_TERMINATION_FAILED_MESSAGE) + }; +} + function matchJobReference(jobs, reference, predicate = () => true) { const filtered = jobs.filter(predicate); if (!reference) { diff --git a/plugins/codex/scripts/lib/process.mjs b/plugins/codex/scripts/lib/process.mjs index dd8fc3751..01fbbfe94 100644 --- a/plugins/codex/scripts/lib/process.mjs +++ b/plugins/codex/scripts/lib/process.mjs @@ -54,6 +54,20 @@ function looksLikeMissingProcessMessage(text) { return /not found|no running instance|cannot find|does not exist|no such process/i.test(text); } +export function isProcessAlive(pid, options = {}) { + if (!Number.isInteger(pid) || pid <= 0) { + return false; + } + + const killImpl = options.killImpl ?? process.kill.bind(process); + try { + killImpl(pid, 0); + return true; + } catch (error) { + return error?.code === "EPERM"; + } +} + export function terminateProcessTree(pid, options = {}) { if (!Number.isFinite(pid)) { return { attempted: false, delivered: false, method: null }; @@ -66,7 +80,8 @@ export function terminateProcessTree(pid, options = {}) { if (platform === "win32") { const result = runCommandImpl("taskkill", ["/PID", String(pid), "/T", "/F"], { cwd: options.cwd, - env: options.env + env: options.env, + shell: false }); if (!result.error && result.status === 0) { @@ -74,7 +89,7 @@ export function terminateProcessTree(pid, options = {}) { } const combinedOutput = `${result.stderr}\n${result.stdout}`.trim(); - if (!result.error && looksLikeMissingProcessMessage(combinedOutput)) { + if (!result.error && (looksLikeMissingProcessMessage(combinedOutput) || !isProcessAlive(pid, { killImpl }))) { return { attempted: true, delivered: false, method: "taskkill", result }; } diff --git a/plugins/codex/scripts/lib/state.mjs b/plugins/codex/scripts/lib/state.mjs index 2da23498f..471ff628e 100644 --- a/plugins/codex/scripts/lib/state.mjs +++ b/plugins/codex/scripts/lib/state.mjs @@ -3,6 +3,7 @@ import fs from "node:fs"; import os from "node:os"; import path from "node:path"; +import { isProcessAlive } from "./process.mjs"; import { resolveWorkspaceRoot } from "./workspace.mjs"; const STATE_VERSION = 1; @@ -11,6 +12,7 @@ const FALLBACK_STATE_ROOT_DIR = path.join(os.tmpdir(), "codex-companion"); const STATE_FILE_NAME = "state.json"; const JOBS_DIR_NAME = "jobs"; const MAX_JOBS = 50; +const UNREPORTED_PROCESS_EXIT_MESSAGE = "Process exited without reporting."; function nowIso() { return new Date().toISOString(); @@ -146,8 +148,56 @@ export function upsertJob(cwd, jobPatch) { }); } +function reconcileRunningJobs(cwd, state) { + const completedAt = nowIso(); + const staleJobs = []; + const jobs = state.jobs.map((job) => { + if (job.status !== "running" || !Number.isInteger(job.pid) || job.pid <= 0 || isProcessAlive(job.pid)) { + return job; + } + + const failedJob = { + ...job, + status: "failed", + phase: "failed", + pid: null, + completedAt, + updatedAt: completedAt, + errorMessage: UNREPORTED_PROCESS_EXIT_MESSAGE + }; + staleJobs.push(failedJob); + return failedJob; + }); + + if (staleJobs.length === 0) { + return state.jobs; + } + + const nextState = saveState(cwd, { ...state, jobs }); + for (const job of staleJobs) { + const jobFile = resolveJobFile(cwd, job.id); + if (!fs.existsSync(jobFile)) { + continue; + } + try { + writeJobFile(cwd, job.id, { + ...readJobFile(jobFile), + status: job.status, + phase: job.phase, + pid: job.pid, + completedAt: job.completedAt, + errorMessage: job.errorMessage + }); + } catch { + // The state record is still authoritative when a per-job file is unreadable. + } + } + return nextState.jobs; +} + export function listJobs(cwd) { - return loadState(cwd).jobs; + const state = loadState(cwd); + return reconcileRunningJobs(cwd, state); } export function setConfig(cwd, key, value) { diff --git a/plugins/codex/scripts/lib/tracked-jobs.mjs b/plugins/codex/scripts/lib/tracked-jobs.mjs index 902869012..6f69002c0 100644 --- a/plugins/codex/scripts/lib/tracked-jobs.mjs +++ b/plugins/codex/scripts/lib/tracked-jobs.mjs @@ -139,20 +139,55 @@ function readStoredJobOrNull(workspaceRoot, jobId) { return readJobFile(jobFile); } +function stopTrackedJobIfCancelled(job, logFile) { + const storedJob = readStoredJobOrNull(job.workspaceRoot, job.id); + if (storedJob?.status !== "cancelled") { + return null; + } + + upsertJob(job.workspaceRoot, { + id: job.id, + status: "cancelled", + phase: "cancelled", + pid: storedJob.pid ?? null, + errorMessage: storedJob.errorMessage ?? "Cancelled by user.", + completedAt: storedJob.completedAt ?? nowIso() + }); + appendLogLine(logFile, "Stopped after cancellation."); + return { + exitStatus: 0, + threadId: storedJob.threadId ?? null, + turnId: storedJob.turnId ?? null, + payload: { status: "cancelled" }, + rendered: "", + summary: storedJob.summary ?? "Cancelled by user." + }; +} + export async function runTrackedJob(job, runner, options = {}) { + const logFile = options.logFile ?? job.logFile ?? null; + const cancelledBeforeStart = stopTrackedJobIfCancelled(job, logFile); + if (cancelledBeforeStart) { + return cancelledBeforeStart; + } + const runningRecord = { ...job, status: "running", startedAt: nowIso(), phase: "starting", pid: process.pid, - logFile: options.logFile ?? job.logFile ?? null + logFile }; writeJobFile(job.workspaceRoot, job.id, runningRecord); upsertJob(job.workspaceRoot, runningRecord); try { const execution = await runner(); + const cancelledDuringRun = stopTrackedJobIfCancelled(job, logFile); + if (cancelledDuringRun) { + return cancelledDuringRun; + } const completionStatus = execution.exitStatus === 0 ? "completed" : "failed"; const completedAt = nowIso(); writeJobFile(job.workspaceRoot, job.id, { @@ -179,6 +214,10 @@ export async function runTrackedJob(job, runner, options = {}) { appendLogBlock(options.logFile ?? job.logFile ?? null, "Final output", execution.rendered); return execution; } catch (error) { + const cancelledDuringRun = stopTrackedJobIfCancelled(job, logFile); + if (cancelledDuringRun) { + return cancelledDuringRun; + } const errorMessage = error instanceof Error ? error.message : String(error); const existing = readStoredJobOrNull(job.workspaceRoot, job.id) ?? runningRecord; const completedAt = nowIso(); diff --git a/tests/args.test.mjs b/tests/args.test.mjs new file mode 100644 index 000000000..8598cae8c --- /dev/null +++ b/tests/args.test.mjs @@ -0,0 +1,65 @@ +import test from "node:test"; +import assert from "node:assert/strict"; + +import { splitRawArgumentString } from "../plugins/codex/scripts/lib/args.mjs"; + +test("splitRawArgumentString preserves backslashes in an unquoted Windows path", () => { + assert.deepEqual(splitRawArgumentString(String.raw`--cwd C:\Users\me\repo`), [ + "--cwd", + String.raw`C:\Users\me\repo` + ]); +}); + +test("splitRawArgumentString preserves a quoted Windows path with spaces", () => { + assert.deepEqual(splitRawArgumentString(String.raw`--cwd "C:\Program Files\App"`), [ + "--cwd", + String.raw`C:\Program Files\App` + ]); +}); + +test("splitRawArgumentString closes a quoted Windows path after a trailing backslash", () => { + assert.deepEqual(splitRawArgumentString(String.raw`"C:\Program Files\Repo\" --write`), [ + "C:\\Program Files\\Repo\\", + "--write" + ]); +}); + +test("splitRawArgumentString closes a final quoted Windows path after a trailing backslash", () => { + assert.deepEqual(splitRawArgumentString(String.raw`--cwd "C:\Program Files\Repo\"`), [ + "--cwd", + "C:\\Program Files\\Repo\\" + ]); +}); + +test("splitRawArgumentString closes a trailing-backslash path before another quoted token", () => { + assert.deepEqual(splitRawArgumentString(String.raw`"C:\path\" "second"`), ["C:\\path\\", "second"]); +}); + +test("splitRawArgumentString accepts an escaped quote inside double quotes", () => { + assert.deepEqual(splitRawArgumentString(String.raw`"say \"hello\""`), ['say "hello"']); +}); + +test("splitRawArgumentString preserves escaped quotes at word boundaries", () => { + assert.deepEqual(splitRawArgumentString(String.raw`"say \"hello\" now"`), ['say "hello" now']); +}); + +test("splitRawArgumentString preserves an escaped quote before a non-boundary character", () => { + assert.deepEqual(splitRawArgumentString(String.raw`"say \"hi\"..."`), ['say "hi"...']); +}); + +test("splitRawArgumentString parses plain prompts with quoted phrases", () => { + assert.deepEqual(splitRawArgumentString(String.raw`run "quoted phrase" and "another phrase"`), [ + "run", + "quoted phrase", + "and", + "another phrase" + ]); +}); + +test("splitRawArgumentString collapses an escaped backslash", () => { + assert.deepEqual(splitRawArgumentString(String.raw`C:\\repo`), [String.raw`C:\repo`]); +}); + +test("splitRawArgumentString preserves a trailing backslash", () => { + assert.deepEqual(splitRawArgumentString("--cwd C:\\repo\\"), ["--cwd", "C:\\repo\\"]); +}); diff --git a/tests/commands.test.mjs b/tests/commands.test.mjs index c34b06059..746baf187 100644 --- a/tests/commands.test.mjs +++ b/tests/commands.test.mjs @@ -102,10 +102,13 @@ test("rescue command absorbs continue semantics", () => { assert.match(rescue, /do not call `Skill\(codex:codex-rescue\)`/i); assert.doesNotMatch(rescue, /^context:\s*fork\b/m); assert.match(rescue, /--background\|--wait/); + assert.match(rescue, /--cwd /); assert.match(rescue, /--resume\|--fresh/); assert.match(rescue, /--model /); assert.match(rescue, /--effort /); assert.match(rescue, /task-resume-candidate --json/); + assert.match(rescue, /task-resume-candidate --json --cwd ""/); + assert.match(rescue, /request includes `--cwd ` or `-C `.*same directory.*helper/i); assert.match(rescue, /AskUserQuestion/); assert.match(rescue, /Continue current Codex thread/); assert.match(rescue, /Start a new Codex thread/); @@ -129,6 +132,9 @@ test("rescue command absorbs continue semantics", () => { 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, /expected to exceed a few minutes.*`--background`/i); + assert.match(agent, /pass `--cwd ` explicitly/i); + assert.match(agent, /`--cwd ` and `-C `.*workspace routing controls/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); diff --git a/tests/process.test.mjs b/tests/process.test.mjs index 80e0715b0..c9b8390fc 100644 --- a/tests/process.test.mjs +++ b/tests/process.test.mjs @@ -7,8 +7,8 @@ test("terminateProcessTree uses taskkill on Windows", () => { let captured = null; const outcome = terminateProcessTree(1234, { platform: "win32", - runCommandImpl(command, args) { - captured = { command, args }; + runCommandImpl(command, args, options) { + captured = { command, args, options }; return { command, args, @@ -26,7 +26,12 @@ test("terminateProcessTree uses taskkill on Windows", () => { assert.deepEqual(captured, { command: "taskkill", - args: ["/PID", "1234", "/T", "/F"] + args: ["/PID", "1234", "/T", "/F"], + options: { + cwd: undefined, + env: undefined, + shell: false + } }); assert.equal(outcome.delivered, true); assert.equal(outcome.method, "taskkill"); @@ -53,3 +58,29 @@ test("terminateProcessTree treats missing Windows processes as already stopped", assert.equal(outcome.result.status, 128); assert.match(outcome.result.stdout, /not found/i); }); + +test("terminateProcessTree recognizes a missing Windows process with localized output", () => { + const outcome = terminateProcessTree(1234, { + platform: "win32", + runCommandImpl(command, args) { + return { + command, + args, + status: 128, + signal: null, + stdout: "Localized taskkill error.", + stderr: "", + error: null + }; + }, + killImpl() { + const error = new Error("No such process"); + error.code = "ESRCH"; + throw error; + } + }); + + assert.equal(outcome.attempted, true); + assert.equal(outcome.delivered, false); + assert.equal(outcome.method, "taskkill"); +}); diff --git a/tests/runtime.test.mjs b/tests/runtime.test.mjs index 8f276835b..d0b38baeb 100644 --- a/tests/runtime.test.mjs +++ b/tests/runtime.test.mjs @@ -8,7 +8,12 @@ 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 { + resolveCancelableJob, + settleCancellationAfterTermination +} from "../plugins/codex/scripts/lib/job-control.mjs"; +import { resolveStateDir, upsertJob, writeJobFile } from "../plugins/codex/scripts/lib/state.mjs"; +import { runTrackedJob } from "../plugins/codex/scripts/lib/tracked-jobs.mjs"; const ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), ".."); const PLUGIN_ROOT = path.join(ROOT, "plugins", "codex"); @@ -193,6 +198,45 @@ test("task runs without auth preflight so Codex can refresh an expired session", assert.match(result.stdout, /Handled the requested task/); }); +test("task uses an explicit workspace cwd from an unrelated invocation directory", () => { + const repo = makeTempDir(); + const invocationDir = makeTempDir(); + const binDir = makeTempDir(); + const fakeStatePath = path.join(binDir, "fake-codex-state.json"); + 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 result = run("node", [SCRIPT, "task", "-C", repo, "inspect the target workspace"], { + cwd: invocationDir, + env: buildEnv(binDir) + }); + + assert.equal(result.status, 0, result.stderr); + const fakeState = JSON.parse(fs.readFileSync(fakeStatePath, "utf8")); + assert.equal(path.resolve(fakeState.threads[0].cwd), path.resolve(repo)); +}); + +test("task rejects a nonexistent explicit workspace cwd", () => { + const invocationDir = makeTempDir(); + const missingDir = path.join(invocationDir, "missing-workspace"); + const result = run("node", [SCRIPT, "task", "--cwd", missingDir, "inspect the target workspace"], { + cwd: invocationDir + }); + + assert.notEqual(result.status, 0); + assert.match(result.stderr, /Task workspace directory does not exist/); +}); + +test("command help documents the task cwd option", () => { + const result = run("node", [SCRIPT, "--help"], { cwd: ROOT }); + + assert.equal(result.status, 0, result.stderr); + assert.match(result.stdout, /task \[--background\].*\[--cwd \]/); +}); + test("transfer delegates the current Claude session directly to native import", () => { const home = makeTempDir(); const repo = path.join(home, "repo"); @@ -503,8 +547,9 @@ test("task --resume-last resumes the latest persisted task thread", () => { assert.equal(result.stdout, "Resumed the prior run.\nFollow-up prompt accepted.\n"); }); -test("task-resume-candidate returns the latest rescue thread from the current session", () => { +test("task-resume-candidate uses an explicit workspace cwd from an unrelated invocation directory", () => { const workspace = makeTempDir(); + const invocationDir = makeTempDir(); const stateDir = resolveStateDir(workspace); const jobsDir = path.join(stateDir, "jobs"); fs.mkdirSync(jobsDir, { recursive: true }); @@ -554,8 +599,8 @@ test("task-resume-candidate returns the latest rescue thread from the current se "utf8" ); - const result = run("node", [SCRIPT, "task-resume-candidate", "--json"], { - cwd: workspace, + const result = run("node", [SCRIPT, "task-resume-candidate", "-C", workspace, "--json"], { + cwd: invocationDir, env: { ...process.env, CODEX_COMPANION_SESSION_ID: "sess-current" @@ -570,6 +615,17 @@ test("task-resume-candidate returns the latest rescue thread from the current se assert.equal(payload.candidate.threadId, "thr_current"); }); +test("task-resume-candidate rejects a nonexistent explicit workspace cwd", () => { + const invocationDir = makeTempDir(); + const missingDir = path.join(invocationDir, "missing-workspace"); + const result = run("node", [SCRIPT, "task-resume-candidate", "--cwd", missingDir, "--json"], { + cwd: invocationDir + }); + + assert.notEqual(result.status, 0); + assert.match(result.stderr, /Task workspace directory does not exist/); +}); + test("task --resume-last does not resume a task from another Claude session", () => { const repo = makeTempDir(); const binDir = makeTempDir(); @@ -1352,6 +1408,67 @@ test("status --wait times out cleanly when a job is still active", () => { assert.equal(payload.waitTimedOut, true); }); +test("status and resume candidates mark a running job with a dead pid as failed", async () => { + const workspace = makeTempDir(); + const stateDir = resolveStateDir(workspace); + const jobsDir = path.join(stateDir, "jobs"); + fs.mkdirSync(jobsDir, { recursive: true }); + + const exitedWorker = spawn(process.execPath, ["-e", ""], { stdio: "ignore" }); + const deadPid = exitedWorker.pid; + await new Promise((resolve, reject) => { + exitedWorker.once("error", reject); + exitedWorker.once("exit", resolve); + }); + + const jobId = "task-stale"; + const logFile = path.join(jobsDir, `${jobId}.log`); + const jobFile = path.join(jobsDir, `${jobId}.json`); + const staleJob = { + id: jobId, + status: "running", + phase: "running", + title: "Codex Task", + jobClass: "task", + sessionId: "sess-stale", + threadId: "thr_stale", + summary: "Investigate flaky test", + pid: deadPid, + logFile, + createdAt: "2026-03-18T15:30:00.000Z", + startedAt: "2026-03-18T15:30:01.000Z", + updatedAt: "2026-03-18T15:30:02.000Z" + }; + fs.writeFileSync(logFile, "[2026-03-18T15:30:00.000Z] Starting Codex Task.\n", "utf8"); + fs.writeFileSync(jobFile, `${JSON.stringify(staleJob, null, 2)}\n`, "utf8"); + fs.writeFileSync( + path.join(stateDir, "state.json"), + `${JSON.stringify({ version: 1, config: { stopReviewGate: false }, jobs: [staleJob] }, null, 2)}\n`, + "utf8" + ); + + const env = { ...process.env, CODEX_COMPANION_SESSION_ID: "sess-stale" }; + const candidateResult = run("node", [SCRIPT, "task-resume-candidate", "--json"], { cwd: workspace, env }); + assert.equal(candidateResult.status, 0, candidateResult.stderr); + const candidate = JSON.parse(candidateResult.stdout); + assert.equal(candidate.available, true); + assert.equal(candidate.candidate.status, "failed"); + + const statusResult = run("node", [SCRIPT, "status", "--json"], { cwd: workspace, env }); + assert.equal(statusResult.status, 0, statusResult.stderr); + const status = JSON.parse(statusResult.stdout); + assert.deepEqual(status.running, []); + assert.equal(status.latestFinished.status, "failed"); + assert.equal(status.latestFinished.errorMessage, "Process exited without reporting."); + + const persistedState = JSON.parse(fs.readFileSync(path.join(stateDir, "state.json"), "utf8")); + assert.equal(persistedState.jobs[0].status, "failed"); + assert.equal(persistedState.jobs[0].pid, null); + const persistedJob = JSON.parse(fs.readFileSync(jobFile, "utf8")); + assert.equal(persistedJob.status, "failed"); + assert.equal(persistedJob.errorMessage, "Process exited without reporting."); +}); + test("result returns the stored output for the latest finished job by default", () => { const workspace = makeTempDir(); const stateDir = resolveStateDir(workspace); @@ -1634,6 +1751,229 @@ test("cancel stops an active background job and marks it cancelled", async (t) = assert.match(fs.readFileSync(logFile, "utf8"), /Cancelled by user/); }); +test("failed cancellation restores a live job so cancellation can be retried", () => { + const workspace = makeTempDir(); + const job = { + id: "task-cancel-failed-live", + status: "running", + phase: "investigating", + title: "Codex Task", + jobClass: "task", + pid: process.pid + }; + const existing = { + ...job, + request: { + cwd: workspace, + prompt: "Investigate the flaky test" + } + }; + const completedAt = "2026-03-18T15:31:00.000Z"; + + writeJobFile(workspace, job.id, { + ...existing, + status: "cancelled", + phase: "cancelled", + completedAt, + cancelledAt: completedAt, + errorMessage: "Cancelled by user." + }); + upsertJob(workspace, { + ...job, + status: "cancelled", + phase: "cancelled", + completedAt, + errorMessage: "Cancelled by user." + }); + + const terminationError = new Error("Access is denied."); + const outcome = settleCancellationAfterTermination( + workspace, + job, + existing, + null, + terminationError, + { isProcessAliveImpl: () => true } + ); + + assert.equal(outcome.processStopped, false); + assert.equal(outcome.error, terminationError); + assert.equal(outcome.job.status, "running"); + assert.equal(outcome.job.pid, process.pid); + assert.match(outcome.job.errorMessage, /retry \/codex:cancel/i); + + const retryable = resolveCancelableJob(workspace, job.id); + assert.equal(retryable.job.status, "running"); + assert.equal(retryable.job.pid, process.pid); + + const stored = JSON.parse( + fs.readFileSync(path.join(resolveStateDir(workspace), "jobs", `${job.id}.json`), "utf8") + ); + assert.equal(stored.status, "running"); + assert.equal(stored.phase, "investigating"); + assert.equal(stored.pid, process.pid); + assert.equal("completedAt" in stored, false); + assert.equal("cancelledAt" in stored, false); + assert.match(stored.errorMessage, /retry \/codex:cancel/i); +}); + +test("failed cancellation remains cancelled when the process is already dead", () => { + const workspace = makeTempDir(); + const job = { + id: "task-cancel-failed-dead", + status: "running", + phase: "investigating", + title: "Codex Task", + jobClass: "task", + pid: 999999 + }; + const existing = { ...job }; + const completedAt = "2026-03-18T15:31:00.000Z"; + const cancelledJob = { + ...job, + status: "cancelled", + phase: "cancelled", + completedAt, + errorMessage: "Cancelled by user." + }; + + writeJobFile(workspace, job.id, { + ...cancelledJob, + cancelledAt: completedAt + }); + upsertJob(workspace, cancelledJob); + + const outcome = settleCancellationAfterTermination( + workspace, + job, + existing, + { attempted: true, delivered: false, method: "taskkill" }, + null, + { isProcessAliveImpl: () => false } + ); + + assert.equal(outcome.processStopped, true); + assert.equal(outcome.error, null); + + const state = JSON.parse(fs.readFileSync(path.join(resolveStateDir(workspace), "state.json"), "utf8")); + assert.equal(state.jobs[0].status, "cancelled"); + assert.equal( + JSON.parse(fs.readFileSync(path.join(resolveStateDir(workspace), "jobs", `${job.id}.json`), "utf8")).status, + "cancelled" + ); +}); + +test("cancelled queued jobs remain stored and are skipped by a late worker", () => { + const workspace = makeTempDir(); + const stateDir = resolveStateDir(workspace); + const jobsDir = path.join(stateDir, "jobs"); + fs.mkdirSync(jobsDir, { recursive: true }); + + const jobId = "task-queued"; + const logFile = path.join(jobsDir, `${jobId}.log`); + const jobFile = path.join(jobsDir, `${jobId}.json`); + const queuedJob = { + id: jobId, + status: "queued", + phase: "queued", + title: "Codex Task", + jobClass: "task", + summary: "Investigate flaky test", + pid: null, + logFile, + request: { + cwd: workspace, + model: null, + effort: null, + prompt: "Investigate the flaky test", + write: false, + resumeLast: false, + jobId + }, + createdAt: "2026-03-18T15:30:00.000Z", + updatedAt: "2026-03-18T15:30:00.000Z" + }; + fs.writeFileSync(logFile, "[2026-03-18T15:30:00.000Z] Queued for background execution.\n", "utf8"); + fs.writeFileSync(jobFile, `${JSON.stringify(queuedJob, null, 2)}\n`, "utf8"); + fs.writeFileSync( + path.join(stateDir, "state.json"), + `${JSON.stringify({ version: 1, config: { stopReviewGate: false }, jobs: [queuedJob] }, null, 2)}\n`, + "utf8" + ); + + const cancel = run("node", [SCRIPT, "cancel", jobId, "--json"], { cwd: workspace }); + assert.equal(cancel.status, 0, cancel.stderr); + assert.equal(JSON.parse(cancel.stdout).status, "cancelled"); + assert.equal(fs.existsSync(jobFile), true); + + const worker = run("node", [SCRIPT, "task-worker", "--cwd", workspace, "--job-id", jobId], { + cwd: workspace + }); + assert.equal(worker.status, 0, worker.stderr); + + const state = JSON.parse(fs.readFileSync(path.join(stateDir, "state.json"), "utf8")); + assert.equal(state.jobs[0].status, "cancelled"); + assert.equal(JSON.parse(fs.readFileSync(jobFile, "utf8")).status, "cancelled"); +}); + +test("tracked jobs preserve cancellation observed during execution", async () => { + const workspace = makeTempDir(); + const jobId = "task-cancel-race"; + const stateDir = resolveStateDir(workspace); + const logFile = path.join(stateDir, "jobs", `${jobId}.log`); + const job = { + id: jobId, + status: "queued", + title: "Codex Task", + workspaceRoot: workspace, + jobClass: "task", + summary: "Investigate flaky test", + logFile + }; + fs.mkdirSync(path.dirname(logFile), { recursive: true }); + fs.writeFileSync(logFile, "", "utf8"); + writeJobFile(workspace, jobId, job); + upsertJob(workspace, job); + + const execution = await runTrackedJob( + job, + async () => { + const cancelledAt = new Date().toISOString(); + const runningJob = JSON.parse(fs.readFileSync(path.join(stateDir, "jobs", `${jobId}.json`), "utf8")); + writeJobFile(workspace, jobId, { + ...runningJob, + status: "cancelled", + phase: "cancelled", + pid: null, + completedAt: cancelledAt, + errorMessage: "Cancelled by user." + }); + upsertJob(workspace, { + id: jobId, + status: "cancelled", + phase: "cancelled", + pid: null, + completedAt: cancelledAt, + errorMessage: "Cancelled by user." + }); + return { + exitStatus: 0, + threadId: "thr_cancelled", + turnId: "turn_cancelled", + payload: { status: 0 }, + rendered: "should not replace cancellation", + summary: "Should not replace cancellation" + }; + }, + { logFile } + ); + + assert.equal(execution.payload.status, "cancelled"); + const state = JSON.parse(fs.readFileSync(path.join(stateDir, "state.json"), "utf8")); + assert.equal(state.jobs[0].status, "cancelled"); + assert.equal(JSON.parse(fs.readFileSync(path.join(stateDir, "jobs", `${jobId}.json`), "utf8")).status, "cancelled"); +}); + test("cancel without a job id ignores active jobs from other Claude sessions", () => { const workspace = makeTempDir(); const stateDir = resolveStateDir(workspace);