diff --git a/.codex-plugin/plugin.json b/.codex-plugin/plugin.json index b561485..c6fb084 100644 --- a/.codex-plugin/plugin.json +++ b/.codex-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "cc", - "version": "1.5.4", + "version": "1.6.0", "description": "Claude Code Plugin for Codex. Delegate code reviews, investigations, tracked tasks, and transcript transfers from inside Codex.", "author": { "name": "CBEPX", diff --git a/CHANGELOG.md b/CHANGELOG.md index 846bf4d..c052fdf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,26 @@ ## [Unreleased] +## v1.6.0 + +### Added + +- Expose exact `lastProgressAt` and numeric `progressAgeMs` freshness telemetry for active jobs in JSON and Markdown status output without adding automatic stall detection or termination. +- Record the external Claude Code host on session markers so headless same-owner delegation can be rejected before creating recursive jobs. + +### Changed + +- Send Claude prompts through stdin instead of process arguments, including large Unicode review context, while keeping model and effort flags absent when their resolved values are empty. +- Make `--view-state on-terminal` the canonical foreground result-view contract; retain `on-success` as a warning alias and keep `defer` unchanged. Warn when the deprecated `--timeout-ms` alias is accepted, and require status timeout controls to accompany `--wait`. +- Document that status/result inspection may identity-check and reconcile owned orphan jobs, while healthy reads and ready `setup --check` runs leave managed files unchanged. + +### Fixed + +- Trust stderr authentication text only for non-zero Claude exits without a terminal event, preserve structured authentication and rate-limit classification precedence, and surface stdin delivery failures as tracked command failures. +- Skip stop-review execution when the current turn has no captured baseline, use only the available Codex question tool, and keep network escalation narrow and subordinate to the active sandbox policy. +- Serialize unread notification updates through same-status transitions, require post-signal liveness proof before clearing Claude child handles, clear stale worker identity metadata at the Windows unverifiable ceiling, and use the spawn-safe identity probe for tracked workers. +- Keep owning Codex session IDs out of synthetic Claude session fields while retaining legacy stored-job compatibility without rewriting state on disk. + ## v1.5.4 ### Added diff --git a/README.md b/README.md index 69684a5..7e35c45 100644 --- a/README.md +++ b/README.md @@ -46,7 +46,7 @@ It follows the shape of [openai/codex-plugin-cc](https://github.com/openai/codex Install the fork release from the CBEPX marketplace snapshot: ```bash -codex plugin marketplace add CBEPX/cc-plugin-codex --ref v1.5.4 +codex plugin marketplace add CBEPX/cc-plugin-codex --ref v1.6.0 codex plugin add cc@cbepx ``` @@ -59,8 +59,8 @@ The optional `npx` helper can install this fork release and enable the required ```bash CC_PLUGIN_CODEX_MARKETPLACE_NAME=cbepx \ CC_PLUGIN_CODEX_MARKETPLACE_SOURCE=CBEPX/cc-plugin-codex \ -CC_PLUGIN_CODEX_MARKETPLACE_REF=v1.5.4 \ -npx -y https://github.com/CBEPX/cc-plugin-codex/releases/download/v1.5.4/cc-plugin-codex-1.5.4.tgz install +CC_PLUGIN_CODEX_MARKETPLACE_REF=v1.6.0 \ +npx -y https://github.com/CBEPX/cc-plugin-codex/releases/download/v1.6.0/cc-plugin-codex-1.6.0.tgz install ``` On Windows, prefer the marketplace path or the `npx` helper. The shell-script helper below is POSIX-only. @@ -200,6 +200,8 @@ $cc:rescue --model sonnet --effort medium investigate the flaky test | `--model ` | Claude model (`opus`, `sonnet`, `haiku`, `fable`, or full ID; defaults to `opus`). Aliases are resolved by Claude Code; a full ID pins a version. | | `--effort ` | Reasoning effort: `low`, `medium`, `high`, `xhigh`, `max` (default: `xhigh` for opus, `high` for sonnet, unset for haiku and fable) | | `--prompt-file ` | Read task description from a file | +| `--view-state on-terminal` | Mark the foreground terminal outcome as viewed | +| `--view-state defer` | Leave the terminal outcome unread for later inspection | | `--wait-timeout-ms ` | Foreground observer timeout before returning a retrievable job | | `--timeout-ms ` | Deprecated alias for `--wait-timeout-ms` | @@ -278,6 +280,8 @@ All review and rescue commands support `--background`. Background jobs are track 5. **Session ownership** — jobs stay attached to the user-facing parent Codex session even when a built-in rescue/review child does the actual work, so plain `$cc:status`, `$cc:result`, and resume-candidate detection still follow the parent thread. 6. **Cleanup on exit** — when your Codex session ends, any still-running detached jobs are terminated via PID identity validation, and stale reserved job markers are cleaned up over time. +Job inspection can also reconcile stale `queued`, `running`, or `cancelling` records. It only terminates an orphaned owned process after its recorded PID identity matches; healthy active jobs are left unchanged. `$cc:result` and JSON status access may additionally record that a terminal result was viewed. + **Typical background flow:** ```text @@ -306,6 +310,7 @@ The review gate is an **optional** stop-time hook. When enabled, pressing Ctrl+C - **Token cost.** Every Ctrl+C triggers a Claude invocation. This can drain usage limits quickly if you stop often. - **15-minute timeout.** The gate has a hard timeout. If Claude doesn't respond, the stop is allowed. - **Skip-on-no-edits.** The gate computes a working-tree fingerprint baseline and skips review when the last Codex turn made no net edits. +- **Requires a recorded user turn.** If the UserPromptSubmit hook did not record a baseline for this session, the gate skips review instead of reviewing unrelated or headless work. - **Not in nested sessions.** Child sessions (e.g., rescue subagents) suppress the gate to avoid feedback loops. **Only enable when you're actively monitoring the session.** @@ -336,7 +341,7 @@ The review gate is an **optional** stop-time hook. When enabled, pressing Ctrl+C Install from the fork's marketplace snapshot: ```bash -codex plugin marketplace add CBEPX/cc-plugin-codex --ref v1.5.4 +codex plugin marketplace add CBEPX/cc-plugin-codex --ref v1.6.0 codex plugin add cc@cbepx ``` @@ -357,8 +362,8 @@ This fork does not install from the upstream Sendbird marketplace. Use the CBEPX ```bash CC_PLUGIN_CODEX_MARKETPLACE_NAME=cbepx \ CC_PLUGIN_CODEX_MARKETPLACE_SOURCE=CBEPX/cc-plugin-codex \ -CC_PLUGIN_CODEX_MARKETPLACE_REF=v1.5.4 \ -npx -y https://github.com/CBEPX/cc-plugin-codex/releases/download/v1.5.4/cc-plugin-codex-1.5.4.tgz install +CC_PLUGIN_CODEX_MARKETPLACE_REF=v1.6.0 \ +npx -y https://github.com/CBEPX/cc-plugin-codex/releases/download/v1.6.0/cc-plugin-codex-1.6.0.tgz install ``` After install, run: @@ -388,7 +393,7 @@ $cc:setup Re-run the fork marketplace install flow, pinned to the release you want: ```bash -codex plugin marketplace add CBEPX/cc-plugin-codex --ref v1.5.4 +codex plugin marketplace add CBEPX/cc-plugin-codex --ref v1.6.0 codex plugin add cc@cbepx ``` diff --git a/hooks/lib/host-origin.mjs b/hooks/lib/host-origin.mjs new file mode 100644 index 0000000..f055e20 --- /dev/null +++ b/hooks/lib/host-origin.mjs @@ -0,0 +1,11 @@ +/** + * Copyright 2026 Sendbird, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ +import process from "node:process"; + +export function detectExternalHostOrigin() { + return process.env.CLAUDECODE || process.env.CLAUDE_CODE_ENTRYPOINT + ? "claude-code" + : null; +} diff --git a/hooks/session-lifecycle-hook.mjs b/hooks/session-lifecycle-hook.mjs index d3e533f..3fa896d 100644 --- a/hooks/session-lifecycle-hook.mjs +++ b/hooks/session-lifecycle-hook.mjs @@ -22,6 +22,7 @@ import process from "node:process"; import { fileURLToPath } from "node:url"; import { readHookInput } from "./lib/hook-input.mjs"; +import { detectExternalHostOrigin } from "./lib/host-origin.mjs"; import { cleanupAfterOfficialUninstall } from "./lib/plugin-install-guard.mjs"; import { terminateProcessTreeIfIdentityMatches } from "../scripts/lib/process.mjs"; import { @@ -341,7 +342,9 @@ function handleSessionStart(input) { reportLifecycleFailure("SessionStart", error); } if (workspaceRoot) { - setCurrentSession(workspaceRoot, input.session_id); + setCurrentSession(workspaceRoot, input.session_id, { + hostOrigin: detectExternalHostOrigin(), + }); } } } diff --git a/hooks/stop-review-gate-hook.mjs b/hooks/stop-review-gate-hook.mjs index e267c37..102ac5e 100644 --- a/hooks/stop-review-gate-hook.mjs +++ b/hooks/stop-review-gate-hook.mjs @@ -51,6 +51,8 @@ const SKIP_INTERACTIVE_HOOKS_ENV = "CLAUDE_COMPANION_SKIP_INTERACTIVE_HOOKS"; const STOP_REVIEW_SUCCESS_NOTE = "Claude Code stop-time review passed."; const STOP_REVIEW_NO_EDIT_NOTE = "Claude Code stop-time review skipped: the most recent turn made no net edits."; +const STOP_REVIEW_NO_BASELINE_NOTE = + "Claude Code stop-time review skipped: no user turn was recorded for this Codex session."; function emitDecision(payload) { process.stdout.write(`${JSON.stringify(payload)}\n`); @@ -268,14 +270,26 @@ function evaluateTurnEditGate(cwd, workspaceRoot, sessionId) { } const baseline = readTurnBaseline(workspaceRoot, sessionId); - if (!baseline?.fingerprint) { + if (!baseline) { return { - shouldSkipReview: false, + shouldSkipReview: true, + skipStatus: "skipped_no_turn_baseline", + skipNote: STOP_REVIEW_NO_BASELINE_NOTE, reason: "No turn baseline was recorded for this session.", baseline, current: null, }; } + if (!baseline.fingerprint) { + return { + shouldSkipReview: false, + reason: baseline.captureError + ? `Turn-baseline capture failed: ${baseline.captureError}` + : "Turn baseline has no usable fingerprint.", + baseline, + current: null, + }; + } try { const current = getWorkingTreeFingerprint(cwd); @@ -372,13 +386,13 @@ async function main() { }; if (turnEditGate.shouldSkipReview) { persistFinal({ - status: "skipped_no_turn_edits", + status: turnEditGate.skipStatus ?? "skipped_no_turn_edits", reason: turnEditGate.reason, claudeInvoked: false, runningTaskNote, ...fingerprintFields, }); - logNote(STOP_REVIEW_NO_EDIT_NOTE); + logNote(turnEditGate.skipNote ?? STOP_REVIEW_NO_EDIT_NOTE); logNote(runningTaskNote); return; } diff --git a/hooks/unread-result-hook.mjs b/hooks/unread-result-hook.mjs index 953bb67..60d6206 100644 --- a/hooks/unread-result-hook.mjs +++ b/hooks/unread-result-hook.mjs @@ -10,12 +10,15 @@ import path from "node:path"; import { fileURLToPath } from "node:url"; import { readHookInput } from "./lib/hook-input.mjs"; +import { detectExternalHostOrigin } from "./lib/host-origin.mjs"; import { cleanupAfterOfficialUninstall } from "./lib/plugin-install-guard.mjs"; import { getConfig, + getCurrentSessionMarker, listJobs, - patchJob, + setCurrentSession, TERMINAL_JOB_STATUSES, + transitionJob, writeTurnBaseline, } from "../scripts/lib/state.mjs"; import { getWorkingTreeFingerprint } from "../scripts/lib/git.mjs"; @@ -83,9 +86,13 @@ function selectUnreadTerminalJobs(workspaceRoot, sessionId) { function markJobsNotified(workspaceRoot, jobs) { const timestamp = nowIso(); for (const job of jobs) { - patchJob(workspaceRoot, job.id, { - notifiedAt: timestamp, - }); + try { + transitionJob(workspaceRoot, job.id, [job.status], job.status, { + notifiedAt: timestamp, + }); + } catch { + // Notification state is best-effort; still surface the terminal result. + } } } @@ -101,8 +108,18 @@ function captureTurnBaseline(workspaceRoot, sessionId, cwd) { capturedAt: nowIso(), fingerprint, }); - } catch { - // Baseline capture is best-effort. If it fails, Stop falls back to running review. + } catch (error) { + try { + writeTurnBaseline(workspaceRoot, sessionId, { + cwd, + workspaceRoot, + capturedAt: nowIso(), + fingerprint: null, + captureError: error instanceof Error ? error.message : String(error), + }); + } catch { + // Baseline capture is best-effort. A missing record also keeps Stop fail-open. + } } } @@ -124,6 +141,16 @@ async function main() { } const config = getConfig(workspaceRoot); + try { + const currentSession = getCurrentSessionMarker(workspaceRoot); + if (!currentSession || currentSession.sessionId === sessionId) { + setCurrentSession(workspaceRoot, sessionId, { + hostOrigin: detectExternalHostOrigin(), + }); + } + } catch { + // Best effort: an invalid session id must not fail a user prompt. + } if (config.stopReviewGate) { captureTurnBaseline(workspaceRoot, sessionId, cwd); } diff --git a/internal-skills/cli-runtime/runtime.md b/internal-skills/cli-runtime/runtime.md index 5e25bd5..1eafd1f 100644 --- a/internal-skills/cli-runtime/runtime.md +++ b/internal-skills/cli-runtime/runtime.md @@ -26,7 +26,7 @@ Routing controls: - Treat `--cwd`, `--model`, `--effort`, `--resume`, `--resume-last`, `--fresh`, `--prompt-file`, `--view-state`, `--owner-session-id`, and `--job-id` as routing controls, not task text. - Preserve the parent helper's exact non-empty `workspaceRoot` as `--cwd ""`; never substitute the plugin root or the child's default working directory. - Leave `--model` and `--effort` unset unless the user explicitly asks for a specific model or effort. The companion command applies these defaults itself: model defaults to `opus`, effort defaults to `xhigh` for opus, `high` for sonnet, and is left unset for haiku and fable. -- `--view-state on-success` means the user will see this companion result in the current turn, so the companion may mark it viewed on success. +- `--view-state on-terminal` means the user will see this companion result in the current turn, so the companion may mark any terminal outcome viewed. - `--view-state defer` means the parent is not waiting, so the companion must leave the result unread until the user explicitly checks it. - `--owner-session-id ` is an internal parent-session routing control. Preserve it when present so tracked jobs remain visible to the parent session's `$cc:status` / `$cc:result`. - Never emit an empty routing placeholder such as `--owner-session-id --job-id`. @@ -47,6 +47,7 @@ Task defaults: - If the tool output includes stderr progress chatter and a final stdout-style result, ignore the progress chatter and preserve only the final stdout-equivalent result text. - Return the stdout of the `task` command exactly as-is. - Run the companion command in the foreground; do not add shell backgrounding such as `&`, `nohup`, or detached `spawn`. +- Only when the shell tool schema and active policy permit escalation, pass `sandbox_permissions: "require_escalated"` with the justification `Allow the Claude Code companion to contact the Claude API for this requested task.` If a higher-priority policy forbids escalation, obey it and surface any network denial. Do not enable or change global network access, and do not first try the companion command in the default network-disabled sandbox. - If the shell tool returns a session id, keep polling that same session until the companion command exits. - Exit code 0 is the only successful completion. - Exit code 124 means the job is still running; return the companion output without claiming it finished. diff --git a/internal-skills/review-runtime/runtime.md b/internal-skills/review-runtime/runtime.md index 6dfc3f1..90f313f 100644 --- a/internal-skills/review-runtime/runtime.md +++ b/internal-skills/review-runtime/runtime.md @@ -18,8 +18,9 @@ Foreground contract: - Strip `--wait` and `--background` before building the companion command. - Keep the shell tool in the active user workspace; do not set its working directory to the plugin path. - Foreground command: - - `review --view-state on-success ...` - - `adversarial-review --view-state on-success ...` + - `review --view-state on-terminal ...` + - `adversarial-review --view-state on-terminal ...` +- Only when the shell tool schema and active policy permit escalation, pass `sandbox_permissions: "require_escalated"` with the justification `Allow the Claude Code companion to contact the Claude API for this requested review.` If a higher-priority policy forbids escalation, obey it and surface any network denial. Do not enable or change global network access, and do not first try the companion command in the default network-disabled sandbox. - Return companion stdout faithfully and do not add review execution commentary around it. Background contract: @@ -37,6 +38,7 @@ Background contract: - ignore stderr progress chatter such as `[cc] ...` - do not inspect the repo or perform the review itself - run the companion command in the foreground; do not add shell backgrounding such as `&`, `nohup`, or detached `spawn` + - only when the shell tool schema and active policy permit escalation, pass `sandbox_permissions: "require_escalated"` with the justification `Allow the Claude Code companion to contact the Claude API for this requested review.` If a higher-priority policy forbids escalation, obey it and surface any network denial. Do not enable or change global network access, and do not first try the companion command in the default network-disabled sandbox - If the shell tool returns a session id, keep polling that same session until the companion command exits. - Exit code 0 is the only successful completion. - Exit code 124 means the job is still running; return the companion output without claiming it finished. diff --git a/package-lock.json b/package-lock.json index 3e86de8..458d344 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "cc-plugin-codex", - "version": "1.5.4", + "version": "1.6.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "cc-plugin-codex", - "version": "1.5.4", + "version": "1.6.0", "license": "Apache-2.0", "bin": { "cc-plugin-codex": "scripts/installer-cli.mjs" diff --git a/package.json b/package.json index b0f5d9b..40b9838 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "cc-plugin-codex", - "version": "1.5.4", + "version": "1.6.0", "description": "Claude Code Plugin for Codex (CBEPX fork)", "type": "module", "author": { @@ -61,7 +61,7 @@ "test": "node --import ./tests/test-env.mjs --test tests/*.test.mjs", "test:coverage": "c8 --all --include='scripts/**/*.mjs' --include='hooks/**/*.mjs' --reporter=text --reporter=json-summary --reporter=lcov --reports-dir=reports/coverage --check-coverage --lines=89 --statements=89 --branches=79 --functions=96 node --import ./tests/test-env.mjs --test tests/*.test.mjs tests/integration/*.test.mjs tests/e2e/*.test.mjs", "test:cross-platform": "node --import ./tests/test-env.mjs --test tests/args.test.mjs tests/cancel-command.test.mjs tests/changelog.test.mjs tests/claude-cli.test.mjs tests/fs.test.mjs tests/install-hooks.test.mjs tests/mutation-config.test.mjs tests/plugin-install-guard.test.mjs tests/process.test.mjs tests/prompts.test.mjs tests/render.test.mjs tests/sandbox-modes.test.mjs tests/skills-contracts.test.mjs tests/structured-output.test.mjs tests/version-sync.test.mjs", - "test:lifecycle-contract": "node --import ./tests/test-env.mjs --test --test-name-pattern=\"keeps a running job while its owning worker is alive|falls back to the identity-checked Claude PID|terminates a live Claude child|bounds Windows Claude child cleanup|clears a recycled Claude child PID|clears an identity-unavailable Claude child|reports cancel_failed when a live Claude child|tracks the worker separately|logs when worker identity is unavailable|does not bypass a terminal writer|ignores progress after its job file disappears\" tests/state.test.mjs tests/tracked-jobs.test.mjs", + "test:lifecycle-contract": "node --import ./tests/test-env.mjs --test --test-name-pattern=\"keeps a running job while its owning worker is alive|falls back to the identity-checked Claude PID|terminates a live Claude child|waits briefly for a signalled Claude child|bounds Windows Claude child cleanup|clears a recycled Claude child PID|clears an identity-unavailable Claude child|reports cancel_failed when a live Claude child|tracks the worker separately|logs when worker identity is unavailable|does not bypass a terminal writer|ignores progress after its job file disappears\" tests/state.test.mjs tests/tracked-jobs.test.mjs", "test:integration": "node --import ./tests/test-env.mjs --test tests/integration/*.test.mjs", "test:mutation": "npm run test:mutation:pr", "test:mutation:pr": "npm run test:mutation:critical && npm run test:mutation:shard:managed && npm run test:mutation:shard:installer", diff --git a/scripts/claude-companion.mjs b/scripts/claude-companion.mjs index 4b87acb..73a2d17 100644 --- a/scripts/claude-companion.mjs +++ b/scripts/claude-companion.mjs @@ -83,6 +83,7 @@ import { generateJobId, getConfig, getCurrentSession, + getCurrentSessionMarker, listJobs, patchJob, readJobFile, @@ -145,11 +146,11 @@ function printUsage() { [ "Usage:", " node scripts/claude-companion.mjs setup [--check] [--enable-review-gate|--disable-review-gate] [--json]", - " node scripts/claude-companion.mjs review [--wait|--background] [--base ] [--scope ] [--model ] [--effort ] [--user-mcp-tool ...] [--allow-project-mcp-servers]", - " node scripts/claude-companion.mjs adversarial-review [--wait|--background] [--base ] [--scope ] [--model ] [--effort ] [--user-mcp-tool ...] [--allow-project-mcp-servers] [focus text]", - " node scripts/claude-companion.mjs task [--background] [--write] [--resume-last|--resume|--fresh] [--model ] [--effort ] [--wait-timeout-ms ] [prompt]", + " node scripts/claude-companion.mjs review [--wait|--background] [--base ] [--scope ] [--model ] [--effort ] [--view-state ] [--owner-session-id ] [--user-mcp-tool ...] [--allow-project-mcp-servers]", + " node scripts/claude-companion.mjs adversarial-review [--wait|--background] [--base ] [--scope ] [--model ] [--effort ] [--view-state ] [--owner-session-id ] [--user-mcp-tool ...] [--allow-project-mcp-servers] [focus text]", + " node scripts/claude-companion.mjs task [--background] [--write] [--resume-last|--resume|--fresh] [--model ] [--effort ] [--view-state ] [--owner-session-id ] [--wait-timeout-ms ] [prompt]", " node scripts/claude-companion.mjs transfer [--source ] [--json]", - " node scripts/claude-companion.mjs status [job-id] [--all] [--json]", + " node scripts/claude-companion.mjs status [job-id] [--all] [--wait] [--wait-timeout-ms ] [--poll-interval-ms ] [--json]", " node scripts/claude-companion.mjs result [job-id] [--json]", " node scripts/claude-companion.mjs cancel [job-id] [--json]", " node scripts/claude-companion.mjs mcp-diagnose [--cwd ] [--user-mcp-tool ...] [--allow-project-mcp-servers] [--json]", @@ -265,7 +266,30 @@ function alignCurrentSessionToOwner(workspaceRoot, ownerSessionId) { if (!ownerSessionId) { return; } - setCurrentSession(workspaceRoot, ownerSessionId); + const marker = getCurrentSessionMarker(workspaceRoot); + setCurrentSession(workspaceRoot, ownerSessionId, { + hostOrigin: marker?.sessionId === ownerSessionId ? marker.hostOrigin : undefined, + }); +} + +function assertDelegationAllowed(workspaceRoot, ownerSessionId, workLabel) { + const marker = getCurrentSessionMarker(workspaceRoot); + if (!marker || marker.hostOrigin !== "claude-code") { + return; + } + const effectiveOwnerSessionId = + ownerSessionId ?? process.env[SESSION_ID_ENV] ?? marker.sessionId; + if (effectiveOwnerSessionId !== marker.sessionId) { + return; + } + throw new Error( + [ + `This Codex thread is driven by Claude Code, not by a user prompt, so delegating this ${workLabel} back to Claude Code would loop it between the two assistants.`, + "Do not retry this command and do not look for another way to reach Claude Code.", + "For an interactive Codex session launched from a Claude Code shell, restart it with CLAUDECODE and CLAUDE_CODE_ENTRYPOINT unset.", + `Perform the requested ${workLabel} yourself in this thread and present your own findings directly.`, + ].join("\n") + ); } async function withReleasedReservation(workspaceRoot, explicitJobId, fn) { @@ -882,7 +906,13 @@ function parseWaitTimeoutMilliseconds(options) { } const optionName = options["wait-timeout-ms"] != null ? "wait-timeout-ms" : "timeout-ms"; - return parsePositiveMilliseconds(options[optionName], `--${optionName}`); + const timeoutMs = parsePositiveMilliseconds(options[optionName], `--${optionName}`); + if (timeoutMs != null && optionName === "timeout-ms") { + process.stderr.write( + "Warning: --timeout-ms is deprecated; use --wait-timeout-ms.\n" + ); + } + return timeoutMs; } function readJsonConfig(filePath) { @@ -1556,7 +1586,7 @@ function buildReviewRequest({ reviewName, userMcpTools, allowProjectMcpServers, - markViewedOnSuccess + markViewedOnTerminal }) { return { cwd, @@ -1568,7 +1598,7 @@ function buildReviewRequest({ reviewName, userMcpTools: normalizeUserMcpTools(userMcpTools), allowProjectMcpServers: Boolean(allowProjectMcpServers), - markViewedOnSuccess + markViewedOnTerminal }; } @@ -1685,7 +1715,7 @@ function buildTaskRequest({ resumeLast, resumeSessionId, jobId, - markViewedOnSuccess + markViewedOnTerminal }) { return { cwd, @@ -1696,7 +1726,7 @@ function buildTaskRequest({ resumeLast, resumeSessionId, jobId, - markViewedOnSuccess + markViewedOnTerminal }; } @@ -1753,19 +1783,25 @@ function renderQueuedTaskLaunch(payload) { ].join("\n"); } -function resolveMarkViewedOnSuccess(viewState, launchedInBackground = false) { +function resolveMarkViewedOnTerminal(viewState, launchedInBackground = false) { const normalized = String(viewState ?? "").trim().toLowerCase(); if (!normalized) { return !launchedInBackground; } + if (normalized === "on-terminal") { + return true; + } if (normalized === "on-success") { + process.stderr.write( + "Warning: --view-state on-success is deprecated; use on-terminal.\n" + ); return true; } if (normalized === "defer") { return false; } throw new Error( - `Unsupported --view-state value: ${viewState}. Use on-success or defer.` + `Unsupported --view-state value: ${viewState}. Use on-terminal or defer.` ); } @@ -1847,7 +1883,7 @@ async function runForegroundCommand(job, runner, options = {}) { } return execution; } finally { - if (options.markViewedOnSuccess) { + if (options.markViewedOnTerminal) { markTerminalJobViewed(job.workspaceRoot, job.id); } } @@ -1936,7 +1972,7 @@ function buildStoredTaskPayload(job) { status: job?.status === "completed" ? "completed" : "failed", jobStatus: job?.status ?? null, warning: null, - sessionId: job?.threadId ?? job?.sessionId ?? null, + sessionId: job?.threadId ?? null, resultMissing: true, requestedModel: null, finalModel: null, @@ -2066,7 +2102,7 @@ async function runForegroundDetachedTask(cwd, job, request, options = {}) { }; } - if (options.markViewedOnSuccess) { + if (options.markViewedOnTerminal) { storedJob = markTerminalJobViewed(job.workspaceRoot, job.id) ?? storedJob; } @@ -2221,7 +2257,7 @@ async function handleReviewCommand(argv, config) { }); const explicitJobId = resolveExplicitJobId(options["job-id"], workspaceRoot); const ownerSessionId = resolveOwnerSessionId(options["owner-session-id"]); - const markViewedOnSuccess = resolveMarkViewedOnSuccess( + const markViewedOnTerminal = resolveMarkViewedOnTerminal( options["view-state"], Boolean(options.background) ); @@ -2233,6 +2269,7 @@ async function handleReviewCommand(argv, config) { await withReleasedReservation(workspaceRoot, explicitJobId, async () => { // Validate inside the reservation guard so failures do not leak markers. config.validateRequest?.(target, focusText); + assertDelegationAllowed(workspaceRoot, ownerSessionId, "review"); const userMcpTools = normalizeUserMcpTools(options["user-mcp-tool"]); if (userMcpTools.length > 0) { process.stderr.write( @@ -2269,7 +2306,7 @@ async function handleReviewCommand(argv, config) { reviewName: config.reviewName, userMcpTools, allowProjectMcpServers: Boolean(options["allow-project-mcp-servers"]), - markViewedOnSuccess + markViewedOnTerminal }); const { payload } = enqueueBackgroundReview(cwd, job, request); outputCommandResult( @@ -2296,7 +2333,7 @@ async function handleReviewCommand(argv, config) { onProgress: progress, onSpawn, }), - { json: options.json, markViewedOnSuccess } + { json: options.json, markViewedOnTerminal } ); }); } @@ -2373,7 +2410,7 @@ async function handleTask(argv) { const effort = resolvedEffort ? resolveEffort(resolvedEffort) : null; const prompt = readTaskPrompt(cwd, options, positionals); const foregroundTimeoutMs = parseWaitTimeoutMilliseconds(options); - const markViewedOnSuccess = resolveMarkViewedOnSuccess( + const markViewedOnTerminal = resolveMarkViewedOnTerminal( options["view-state"], Boolean(options.background) ); @@ -2401,6 +2438,7 @@ async function handleTask(argv) { const write = Boolean(options.write); const explicitJobId = resolveExplicitJobId(options["job-id"], workspaceRoot); await withReleasedReservation(workspaceRoot, explicitJobId, async () => { + assertDelegationAllowed(workspaceRoot, ownerSessionId, "task"); const taskMetadata = buildTaskRunMetadata({ prompt, resumeLast @@ -2442,7 +2480,7 @@ async function handleTask(argv) { resumeLast, resumeSessionId, jobId: job.id, - markViewedOnSuccess + markViewedOnTerminal }); const { payload } = enqueueBackgroundTask(cwd, job, request); outputCommandResult( @@ -2462,7 +2500,7 @@ async function handleTask(argv) { resumeLast, resumeSessionId, jobId: job.id, - markViewedOnSuccess + markViewedOnTerminal }); await runForegroundDetachedTask( cwd, @@ -2471,7 +2509,7 @@ async function handleTask(argv) { { json: options.json, quietProgress: Boolean(options["quiet-progress"]), - markViewedOnSuccess, + markViewedOnTerminal, timeoutMs: foregroundTimeoutMs, pollIntervalMs: options["poll-interval-ms"], } @@ -2536,7 +2574,12 @@ async function handleTaskWorker(argv) { onProgress: progress, onSpawn, }), - { logFile, markViewedOnSuccess: Boolean(request.markViewedOnSuccess) } + { + logFile, + markViewedOnTerminal: Boolean( + request.markViewedOnTerminal ?? request.markViewedOnSuccess + ), + } ); } @@ -2584,7 +2627,12 @@ async function handleReviewWorker(argv) { onProgress: progress, onSpawn, }), - { logFile, markViewedOnSuccess: Boolean(request.markViewedOnSuccess) } + { + logFile, + markViewedOnTerminal: Boolean( + request.markViewedOnTerminal ?? request.markViewedOnSuccess + ), + } ); } @@ -2595,6 +2643,14 @@ async function handleStatus(argv) { }); const cwd = resolveCommandCwd(options); + const timeoutOption = options["wait-timeout-ms"] != null + ? "wait-timeout-ms" + : options["timeout-ms"] != null + ? "timeout-ms" + : null; + if (timeoutOption && !options.wait) { + throw new Error(`--${timeoutOption} requires --wait.`); + } const waitTimeoutMs = parseWaitTimeoutMilliseconds(options); const reference = positionals[0] ?? ""; if (reference) { diff --git a/scripts/lib/claude-cli.mjs b/scripts/lib/claude-cli.mjs index adfb079..07a9401 100644 --- a/scripts/lib/claude-cli.mjs +++ b/scripts/lib/claude-cli.mjs @@ -383,7 +383,13 @@ export function classifyClaudeFailure(value = {}) { finalMessage && CLAUDE_AUTH_ERROR_RE.test(finalMessage) ); - const stderrAuth = Boolean(stderr && CLAUDE_AUTH_ERROR_RE.test(stderr)); + const stderrAuth = Boolean( + stderr && + value.exitCode !== 0 && + Number.isInteger(value.exitCode) && + value.receivedTerminalEvent === false && + CLAUDE_AUTH_ERROR_RE.test(stderr) + ); if (finalMessageAuth || stderrAuth) { return { kind: "claude_auth", @@ -1253,11 +1259,13 @@ export function buildArgs(prompt, options = {}) { if (options.noSessionPersistence) { args.push("--no-session-persistence"); } - if (options.model) { - args.push("--model", resolveModel(options.model)); + const model = resolveModel(options.model); + if (model) { + args.push("--model", model); } - if (options.effort) { - args.push("--effort", resolveEffort(options.effort)); + const effort = resolveEffort(options.effort); + if (effort) { + args.push("--effort", effort); } if (options.sessionId) { args.push("--session-id", options.sessionId); @@ -1292,7 +1300,6 @@ export function buildArgs(prompt, options = {}) { args.push("--strict-mcp-config"); } - args.push("--", prompt); return args; } @@ -1305,7 +1312,7 @@ export async function runClaudeTurn(cwd, prompt, options = {}) { outputFormat: "stream-json", ...options, }); - const requestedModel = options.model ? resolveModel(options.model) : null; + const requestedModel = resolveModel(options.model) ?? null; const command = resolveClaudeCommand(); if (command.error) { return { @@ -1320,7 +1327,11 @@ export async function runClaudeTurn(cwd, prompt, options = {}) { finalModel: null, contextWindow: null, modelEvents: [], - failure: classifyClaudeFailure({ stderr: command.error }), + failure: classifyClaudeFailure({ + stderr: command.error, + exitCode: -1, + receivedTerminalEvent: false, + }), stderr: command.error, pid: null, pidIdentity: null, @@ -1333,7 +1344,7 @@ export async function runClaudeTurn(cwd, prompt, options = {}) { cwd, detached: true, // new process group for safe cancellation windowsHide: true, - stdio: ["ignore", "pipe", "pipe"], // stdin ignored — prompt is passed as CLI arg + stdio: ["pipe", "pipe", "pipe"], env: { ...process.env, // Stream subagent (Task) text/thinking so long turns show liveness. @@ -1343,6 +1354,17 @@ export async function runClaudeTurn(cwd, prompt, options = {}) { }, }); + let stdinError = null; + proc.stdin.on("error", (error) => { + stdinError = error; + }); + try { + proc.stdin.end(String(prompt ?? ""), "utf8"); + } catch (error) { + stdinError = error; + proc.stdin.destroy(); + } + let pidIdentity = null; try { pidIdentity = getSpawnedProcessIdentity(proc.pid); @@ -1380,7 +1402,20 @@ export async function runClaudeTurn(cwd, prompt, options = {}) { if (options.onProgress) options.onProgress(evt); } - const validation = validateTurnCompletion(parser.state, code ?? 1); + if (stdinError) { + stderr = appendTextTail( + stderr, + `\nFailed to write Claude prompt to stdin: ${stdinError.message}`, + MAX_STDERR_BYTES + ); + } + let validation = validateTurnCompletion(parser.state, code ?? 1); + if (stdinError && validation.status !== "failed") { + validation = { + status: "failed", + warning: "Claude prompt delivery through stdin failed.", + }; + } const modelEvents = [...parser.state.modelEvents]; const finalModel = parser.state.finalModel; const contextWindow = parser.state.contextWindow; @@ -1391,6 +1426,8 @@ export async function runClaudeTurn(cwd, prompt, options = {}) { finalMessageHasLimitSignal: parser.state.hasTerminalLimitSignal, finalMessageHasAuthSignal: parser.state.hasTerminalAuthSignal, stderr, + exitCode: code ?? 1, + receivedTerminalEvent: parser.state.receivedTerminalEvent, }) : null; if ( @@ -1444,7 +1481,11 @@ export async function runClaudeTurn(cwd, prompt, options = {}) { finalModel: null, contextWindow: null, modelEvents: [], - failure: classifyClaudeFailure({ stderr: err.message }), + failure: classifyClaudeFailure({ + stderr: err.message, + exitCode: -1, + receivedTerminalEvent: false, + }), stderr: err.message, pid: proc.pid, pidIdentity, diff --git a/scripts/lib/job-control.mjs b/scripts/lib/job-control.mjs index 1c3f1e9..5d5a64f 100644 --- a/scripts/lib/job-control.mjs +++ b/scripts/lib/job-control.mjs @@ -87,6 +87,28 @@ function formatElapsedDuration(startValue, endValue = null) { return `${seconds}s`; } +function resolveProgressFreshness(job, logFile, now = Date.now()) { + const candidates = [ + job.lastProgressAt, + job.updatedAt, + job.startedAt, + job.createdAt, + ] + .map((value) => Date.parse(value ?? "")) + .filter(Number.isFinite); + try { + candidates.push(fs.statSync(logFile).mtimeMs); + } catch {} + if (candidates.length === 0) { + return { lastProgressAt: null, progressAgeMs: null }; + } + const lastProgressMs = Math.max(...candidates); + return { + lastProgressAt: new Date(lastProgressMs).toISOString(), + progressAgeMs: Math.max(0, now - lastProgressMs), + }; +} + const ACTIVE_STATUSES = new Set(["running", "cancelling"]); function inferJobPhase(job, progressPreview = []) { @@ -115,6 +137,9 @@ export function enrichJob(job, options = {}) { const maxProgressLines = options.maxProgressLines ?? DEFAULT_MAX_PROGRESS_LINES; const managedLogFile = job?.workspaceRoot && job?.id ? resolveJobLogFile(job.workspaceRoot, job.id) : null; + const progressFreshness = ACTIVE_STATUSES.has(job.status) + ? resolveProgressFreshness(job, managedLogFile, options.now ?? Date.now()) + : { lastProgressAt: null, progressAgeMs: null }; const enriched = { ...job, kindLabel: getJobTypeLabel(job), @@ -123,6 +148,7 @@ export function enrichJob(job, options = {}) { ? readJobProgressPreview(managedLogFile, maxProgressLines) : [], logFile: managedLogFile, + ...progressFreshness, elapsed: formatElapsedDuration( job.startedAt ?? job.createdAt, TERMINAL_JOB_STATUSES.has(job.status) ? (job.completedAt ?? null) : null diff --git a/scripts/lib/render.mjs b/scripts/lib/render.mjs index 91d25bc..d32e25d 100644 --- a/scripts/lib/render.mjs +++ b/scripts/lib/render.mjs @@ -305,13 +305,13 @@ function formatStatusDuration(job) { function renderStatusTable(rows) { const lines = [ - "| Job | Kind | Status | Phase | Started | Ended | Elapsed/Duration | Summary | Actions |", - "| --- | --- | --- | --- | --- | --- | --- | --- | --- |", + "| Job | Kind | Status | Phase | Started | Ended | Elapsed/Duration | Last Progress | Progress Age (ms) | Summary | Actions |", + "| --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- |", ]; for (const job of rows) { lines.push( - `| ${escapeMarkdownCell(job.id)} | ${escapeMarkdownCell(job.kindLabel)} | ${escapeMarkdownCell(job.status)} | ${escapeMarkdownCell(job.phase ?? "")} | ${escapeMarkdownCell(formatJobTimestamp(job.startedAt))} | ${escapeMarkdownCell(formatJobTimestamp(job.completedAt))} | ${escapeMarkdownCell(formatStatusDuration(job))} | ${escapeMarkdownCell(job.summary ?? "")} | ${formatStatusActions(job)} |` + `| ${escapeMarkdownCell(job.id)} | ${escapeMarkdownCell(job.kindLabel)} | ${escapeMarkdownCell(job.status)} | ${escapeMarkdownCell(job.phase ?? "")} | ${escapeMarkdownCell(formatJobTimestamp(job.startedAt))} | ${escapeMarkdownCell(formatJobTimestamp(job.completedAt))} | ${escapeMarkdownCell(formatStatusDuration(job))} | ${escapeMarkdownCell(job.lastProgressAt ?? "")} | ${escapeMarkdownCell(job.progressAgeMs ?? "")} | ${escapeMarkdownCell(job.summary ?? "")} | ${formatStatusActions(job)} |` ); } @@ -476,6 +476,8 @@ export function renderJobStatusReport(job, platform = process.platform) { pushKeyValueTableRow(lines, "Ended", job.completedAt ?? ""); if (isPendingJob(job)) pushKeyValueTableRow(lines, "Elapsed", job.elapsed ?? ""); else pushKeyValueTableRow(lines, "Duration", job.duration ?? job.elapsed ?? ""); + pushKeyValueTableRow(lines, "Last progress", job.lastProgressAt ?? ""); + pushKeyValueTableRow(lines, "Progress age (ms)", job.progressAgeMs ?? ""); const ownerSessionId = resolveOwningSessionId(job); const claudeSessionId = resolveClaudeSessionId(job); if (claudeSessionId) { diff --git a/scripts/lib/state.mjs b/scripts/lib/state.mjs index bea55e3..fd7bc49 100644 --- a/scripts/lib/state.mjs +++ b/scripts/lib/state.mjs @@ -294,25 +294,42 @@ export function getConfig(cwd) { // Current session marker (fallback when Codex does not propagate env vars) // --------------------------------------------------------------------------- -export function setCurrentSession(cwd, sessionId) { +export function setCurrentSession(cwd, sessionId, options = {}) { sanitizeId(sessionId, "session ID"); ensureStateDir(cwd); writeAtomic(resolveCurrentSessionFile(cwd), { sessionId, + ...(options.hostOrigin ? { hostOrigin: String(options.hostOrigin) } : {}), updatedAt: nowIso(), }); } -export function getCurrentSession(cwd) { +function readCurrentSessionPayload(cwd) { const filePath = resolveCurrentSessionFile(cwd); try { const payload = JSON.parse(fs.readFileSync(filePath, "utf8")); - return sanitizeId(payload.sessionId, "session ID"); + sanitizeId(payload.sessionId, "session ID"); + return payload; } catch { return null; } } +export function getCurrentSession(cwd) { + return readCurrentSessionPayload(cwd)?.sessionId ?? null; +} + +export function getCurrentSessionMarker(cwd) { + const payload = readCurrentSessionPayload(cwd); + if (!payload) { + return null; + } + return { + sessionId: payload.sessionId, + hostOrigin: typeof payload.hostOrigin === "string" ? payload.hostOrigin : null, + }; +} + export function clearCurrentSession(cwd, sessionId = null) { const filePath = resolveCurrentSessionFile(cwd); if (sessionId != null) { @@ -547,11 +564,16 @@ function isWithinReapGracePeriod(job, now = Date.now(), graceMs = REAP_GRACE_MS) export function reapStaleJobs(cwd, jobs, options = {}) { const platform = options.platform ?? process.platform; const isProcessAliveImpl = options.isProcessAliveImpl ?? isProcessAlive; + const childExitWaitMs = Number.isFinite(options.childExitWaitMs) + ? Math.max(0, Math.min(options.childExitWaitMs, 1_000)) + : 1_000; + const sleepSyncImpl = options.sleepSyncImpl ?? sleepSync; const getProcessIdentityImpl = options.getProcessIdentityImpl ?? getProcessIdentity; const terminateProcessTreeIfIdentityMatchesImpl = options.terminateProcessTreeIfIdentityMatchesImpl ?? terminateProcessTreeIfIdentityMatches; + const childExitDeadline = Date.now() + childExitWaitMs; return jobs.map((job) => { if (isWithinReapGracePeriod(job)) return job; @@ -774,13 +796,26 @@ export function reapStaleJobs(cwd, jobs, options = {}) { }; } } - const childResolved = Boolean( - childCleanup?.delivered || + let childResolved = Boolean( childCleanup?.reason === "process-missing" || - childCleanup?.reason === "identity-mismatch" || - (childCleanup?.reason === "identity-unavailable" && - !isProcessAliveImpl(job.pid)) + childCleanup?.reason === "identity-mismatch" ); + if (childCleanup && !childResolved) { + do { + childResolved = !isProcessAliveImpl(job.pid); + if ( + !childResolved && + childCleanup.delivered && + Date.now() < childExitDeadline + ) { + sleepSyncImpl(Math.min(50, childExitDeadline - Date.now())); + } + } while ( + !childResolved && + childCleanup.delivered && + Date.now() < childExitDeadline + ); + } const unresolvedClaudeChild = hasDistinctClaudeChild && !childResolved; const nextStatus = job.status === "cancelling" ? (identityUnavailableTooLong || unresolvedClaudeChild @@ -794,6 +829,8 @@ export function reapStaleJobs(cwd, jobs, options = {}) { completedAt: nowIso(), phase: nextStatus, reapedUnverifiable: true, + workerPid: null, + workerPidIdentity: null, ...(nextStatus === "cancel_failed" ? { pgid: job.pgid ?? job.pid ?? trackedPid } : {}), diff --git a/scripts/lib/tracked-jobs.mjs b/scripts/lib/tracked-jobs.mjs index 2a070c8..9910eef 100644 --- a/scripts/lib/tracked-jobs.mjs +++ b/scripts/lib/tracked-jobs.mjs @@ -12,7 +12,11 @@ import fs from "node:fs"; import process from "node:process"; -import { getProcessIdentity, terminateProcessTree } from "./process.mjs"; +import { + getProcessIdentity, + getSpawnedProcessIdentity, + terminateProcessTree, +} from "./process.mjs"; import { nowIso, ensureStateDir, getCurrentSession, readJobFile, resolveJobLogFile, writeJobFile, cleanupOldJobs, transitionJob } from "./state.mjs"; export { nowIso }; @@ -358,8 +362,10 @@ export function createProgressReporter({ stderr = false, logFile = null, onEvent export async function runTrackedJob(job, runner, options = {}) { const workerPid = process.pid; - const getProcessIdentityImpl = - options.getProcessIdentityImpl ?? getProcessIdentity; + const getWorkerProcessIdentityImpl = + options.getSpawnedProcessIdentityImpl ?? + options.getProcessIdentityImpl ?? + getSpawnedProcessIdentity; const storedJob = readJobFile(job.workspaceRoot, job.id); let workerPidIdentity = [job, storedJob].find( @@ -370,7 +376,7 @@ export async function runTrackedJob(job, runner, options = {}) { )?.workerPidIdentity ?? null; if (!workerPidIdentity) { try { - workerPidIdentity = getProcessIdentityImpl(workerPid); + workerPidIdentity = getWorkerProcessIdentityImpl(workerPid); } catch {} } // ponytail: without a stable worker identity, fall back to the child's diff --git a/skills/adversarial-review/SKILL.md b/skills/adversarial-review/SKILL.md index 2f7056b..aac856f 100644 --- a/skills/adversarial-review/SKILL.md +++ b/skills/adversarial-review/SKILL.md @@ -40,9 +40,10 @@ Execution mode rules: - Recommend waiting only when the scoped review is clearly tiny, roughly 1-2 files total and no sign of a broader directory-sized change. - In every other case, including unclear size, recommend background. - When in doubt, run the review instead of declaring that there is nothing to review. -- Then use `AskUserQuestion` exactly once with two options, putting the recommended option first and suffixing its label with `(Recommended)`: +- Then ask the user once which execution mode to use, offering two options with the recommended one first and its label suffixed `(Recommended)`: - `Wait for results` - `Run in background` +- Use `request_user_input` only when this thread actually has one. If an interactive thread has no question tool, ask in the reply and stop. In a non-interactive thread, proceed with the recommended mode; never spin waiting for a picker. Argument handling: - Preserve the user's arguments exactly. @@ -59,7 +60,8 @@ Argument handling: Foreground flow: - Run: - `node "/scripts/claude-companion.mjs" adversarial-review --view-state on-success ` + `node "/scripts/claude-companion.mjs" adversarial-review --view-state on-terminal ` +- Only when the shell tool schema and active policy permit escalation, pass `sandbox_permissions: "require_escalated"` with the justification `Allow the Claude Code companion to contact the Claude API for this requested review.` If a higher-priority policy forbids escalation, obey it and surface any network denial. Do not enable or change global network access, and do not first try the companion command in the default network-disabled sandbox. - Foreground adversarial review belongs to the main Codex thread. Do not spawn a review subagent, do not invoke a generic review-runner role, and do not proxy this foreground path through any background worker abstraction. - Do not fall back to raw `claude`, `claude-code`, `claude review`, `bash -lc ...claude...`, or any other direct Claude CLI syntax when the companion path is available. The foreground syntax contract here is the resolved companion command above, not a hand-rolled Claude invocation. - If the resolved companion command fails, surface that failure. Do not silently retry foreground adversarial review through a different CLI shape, a generic review runner, or a custom shell wrapper. @@ -93,6 +95,7 @@ Background flow: - execute: `node "/scripts/claude-companion.mjs" adversarial-review --cwd "" --view-state defer ` - run that command in the foreground; do not add shell backgrounding such as `&`, `nohup`, or detached `spawn` + - only when the shell tool schema and active policy permit escalation, pass `sandbox_permissions: "require_escalated"` with the justification `Allow the Claude Code companion to contact the Claude API for this requested review.` If a higher-priority policy forbids escalation, obey it and surface any network denial. Do not enable or change global network access, and do not first try the companion command in the default network-disabled sandbox - If the shell tool returns a session id, keep polling that same session until the companion command exits. - Exit code 0 is the only successful completion. - Exit code 124 means the job is still running; return the companion output without claiming it finished. diff --git a/skills/rescue/SKILL.md b/skills/rescue/SKILL.md index 5cfe21f..2e0ece7 100644 --- a/skills/rescue/SKILL.md +++ b/skills/rescue/SKILL.md @@ -76,7 +76,7 @@ Subagent launch: - If it returns an empty `ownerSessionId`, omit `--owner-session-id` entirely. Never leave an empty routing placeholder such as `--owner-session-id --job-id`. - If that helper returns a non-empty `jobId`, pass it into the companion command as an internal `--job-id ` routing flag. - Add an internal companion routing flag that reflects whether the user will see this result in the current turn: - - Foreground rescue must add `--view-state on-success` + - Foreground rescue must add `--view-state on-terminal` - Background rescue must add `--view-state defer` - Any user-supplied `--model` flag is for the Claude companion only and must be forwarded unchanged to `task`. - If that helper returns a non-empty `parentThreadId`, pass it into the child prompt as the parent thread id for one-shot completion notification. @@ -122,6 +122,7 @@ Subagent launch: - identify the child as a transient forwarding worker for Claude Code rescue - include exactly one shell command to run - run that command in the foreground; do not add shell backgrounding such as `&`, `nohup`, or detached `spawn` + - only when the shell tool schema and active policy permit escalation, pass `sandbox_permissions: "require_escalated"` with the justification `Allow the Claude Code companion to contact the Claude API for this requested task.` If a higher-priority policy forbids escalation, obey it and surface any network denial. Do not enable or change global network access, and do not first try the companion command in the default network-disabled sandbox - If the shell tool returns a session id, keep polling that same session until the companion command exits. - Exit code 0 is the only successful completion. - Exit code 124 means the job is still running; return the companion output without claiming it finished. diff --git a/skills/result/SKILL.md b/skills/result/SKILL.md index 1bffc99..6cf6764 100644 --- a/skills/result/SKILL.md +++ b/skills/result/SKILL.md @@ -15,3 +15,4 @@ Supported arguments: `[job-id]` Output: - Present the full companion stdout exactly as returned. - Do not summarize or condense it. +- Result inspection records terminal output as viewed and may reconcile stale owned jobs. Process cleanup remains PID-identity checked. diff --git a/skills/review/SKILL.md b/skills/review/SKILL.md index bdd46d5..4fb9efa 100644 --- a/skills/review/SKILL.md +++ b/skills/review/SKILL.md @@ -41,9 +41,10 @@ Execution mode rules: - Recommend waiting only when the review is clearly tiny, roughly 1-2 files total and no sign of a broader directory-sized change. - In every other case, including unclear size, recommend background. - When in doubt, run the review instead of declaring that there is nothing to review. -- Then use `AskUserQuestion` exactly once with two options, putting the recommended option first and suffixing its label with `(Recommended)`: +- Then ask the user once which execution mode to use, offering two options with the recommended one first and its label suffixed `(Recommended)`: - `Wait for results` - `Run in background` +- Use `request_user_input` only when this thread actually has one. If an interactive thread has no question tool, ask in the reply and stop. In a non-interactive thread, proceed with the recommended mode; never spin waiting for a picker. Argument handling: - Preserve the user's arguments exactly. @@ -57,7 +58,8 @@ Argument handling: Foreground flow: - Run: - `node "/scripts/claude-companion.mjs" review --view-state on-success ` + `node "/scripts/claude-companion.mjs" review --view-state on-terminal ` +- Only when the shell tool schema and active policy permit escalation, pass `sandbox_permissions: "require_escalated"` with the justification `Allow the Claude Code companion to contact the Claude API for this requested review.` If a higher-priority policy forbids escalation, obey it and surface any network denial. Do not enable or change global network access, and do not first try the companion command in the default network-disabled sandbox. - Foreground review belongs to the main Codex thread. Do not spawn a review subagent, do not invoke a generic review-runner role, and do not proxy this foreground path through any background worker abstraction. - Do not fall back to raw `claude`, `claude-code`, `claude review`, `bash -lc ...claude...`, or any other direct Claude CLI syntax when the companion path is available. The foreground syntax contract here is the resolved companion command above, not a hand-rolled Claude invocation. - If the resolved companion command fails, surface that failure. Do not silently retry foreground review through a different CLI shape, a generic review runner, or a custom shell wrapper. @@ -91,6 +93,7 @@ Background flow: - execute: `node "/scripts/claude-companion.mjs" review --cwd "" --view-state defer ` - run that command in the foreground; do not add shell backgrounding such as `&`, `nohup`, or detached `spawn` + - only when the shell tool schema and active policy permit escalation, pass `sandbox_permissions: "require_escalated"` with the justification `Allow the Claude Code companion to contact the Claude API for this requested review.` If a higher-priority policy forbids escalation, obey it and surface any network denial. Do not enable or change global network access, and do not first try the companion command in the default network-disabled sandbox - If the shell tool returns a session id, keep polling that same session until the companion command exits. - Exit code 0 is the only successful completion. - Exit code 124 means the job is still running; return the companion output without claiming it finished. diff --git a/skills/status/SKILL.md b/skills/status/SKILL.md index a867581..eac8905 100644 --- a/skills/status/SKILL.md +++ b/skills/status/SKILL.md @@ -16,3 +16,4 @@ Output: - Present the companion stdout exactly as returned. - Do not add extra prose or reformat it. - By default, status overview is scoped to the current Codex session in this repository. `--all` widens that overview to all tracked jobs in the current repository workspace. +- Status inspection may reconcile stale owned jobs. Process cleanup remains PID-identity checked; healthy active jobs are not rewritten. diff --git a/stryker.shard.config.mjs b/stryker.shard.config.mjs index b57cc96..53bd1e2 100644 --- a/stryker.shard.config.mjs +++ b/stryker.shard.config.mjs @@ -22,21 +22,21 @@ const shards = { mutate: [ // Persistence lifecycle, session lookup, and terminal job transitions. "scripts/lib/state.mjs:188-228", - "scripts/lib/state.mjs:297-371", - "scripts/lib/state.mjs:403-451", - "scripts/lib/state.mjs:528-838", - "scripts/lib/state.mjs:887-1059", - "scripts/lib/state.mjs:1125-1182", - "scripts/lib/state.mjs:1188-1234", - "scripts/lib/tracked-jobs.mjs:26-39", - "scripts/lib/tracked-jobs.mjs:282-340", - "scripts/lib/tracked-jobs.mjs:359-516", + "scripts/lib/state.mjs:297-388", + "scripts/lib/state.mjs:420-468", + "scripts/lib/state.mjs:545-875", + "scripts/lib/state.mjs:924-1096", + "scripts/lib/state.mjs:1162-1219", + "scripts/lib/state.mjs:1225-1271", + "scripts/lib/tracked-jobs.mjs:30-43", + "scripts/lib/tracked-jobs.mjs:286-344", + "scripts/lib/tracked-jobs.mjs:363-522", ], }, "job-control": { command: "npm run test:mutation:job-control:unit", // Public selection and cancellation paths; process mechanics are covered separately. - mutate: ["scripts/lib/job-control.mjs:144-247"], + mutate: ["scripts/lib/job-control.mjs:170-273"], }, managed: { command: "npm run test:mutation:managed:unit", diff --git a/tests/claude-cli.test.mjs b/tests/claude-cli.test.mjs index f390ef7..fdcc1f2 100644 --- a/tests/claude-cli.test.mjs +++ b/tests/claude-cli.test.mjs @@ -1103,15 +1103,31 @@ describe("classifyClaudeFailure", () => { assert.equal(failure.resetText, null); }); - it("classifies actionable Claude authentication failures", () => { + it("classifies authentication stderr from a failed command without a terminal event", () => { const failure = classifyClaudeFailure({ stderr: "Not logged in. Run claude auth login to continue.", + exitCode: 1, + receivedTerminalEvent: false, }); assert.equal(failure.kind, "claude_auth"); assert.match(failure.message, /auth login/); }); + it("ignores authentication stderr without trusted failure provenance", () => { + const stderr = "Not logged in. Run claude auth login to continue."; + + assert.equal(classifyClaudeFailure({ stderr }), null); + assert.equal( + classifyClaudeFailure({ + stderr, + exitCode: 1, + receivedTerminalEvent: true, + }), + null + ); + }); + it("classifies authentication failures from terminal output", () => { const failure = classifyClaudeFailure({ finalMessage: "Invalid API key. Please run /login.", @@ -1344,6 +1360,73 @@ describe("classifyClaudeFailure", () => { }); describe("runClaudeTurn", () => { + it("sends a large Unicode prompt through stdin and keeps it out of argv", async () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "cc-plugin-claude-stdin-")); + const oldPath = process.env.PATH ?? ""; + try { + createFakeClaudeCommand( + tmpDir, + `let prompt = "";\nprocess.stdin.setEncoding("utf8");\nprocess.stdin.on("data", (chunk) => { prompt += chunk; });\nprocess.stdin.on("end", () => {\n const result = JSON.stringify({ argv: process.argv.slice(2), prompt });\n const out = JSON.stringify({ type: "result", result, session_id: "sess-stdin" });\n process.stdout.write(out + "\\n", () => process.exit(0));\n});\n` + ); + process.env.PATH = `${tmpDir}${path.delimiter}${oldPath}`; + const prompt = "Привет, Claude! 🧪\n".repeat(4_000); + + const result = await runClaudeTurn(process.cwd(), prompt); + const payload = JSON.parse(result.finalMessage); + + assert.equal(result.status, "completed"); + assert.equal(payload.prompt, prompt); + assert.equal(payload.argv.includes(prompt), false); + assert.equal(payload.argv.includes("--"), false); + } finally { + process.env.PATH = oldPath; + fs.rmSync(tmpDir, { recursive: true, force: true }); + } + }); + + it("fails when the Claude process closes stdin before receiving the prompt", async () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "cc-plugin-claude-stdin-error-")); + const oldPath = process.env.PATH ?? ""; + try { + createFakeClaudeCommand( + tmpDir, + `process.stdin.destroy();\nsetTimeout(() => {\n const out = JSON.stringify({ type: "result", result: "done", session_id: "sess-stdin-error" });\n process.stdout.write(out + "\\n", () => process.exit(0));\n}, 100);\n` + ); + process.env.PATH = `${tmpDir}${path.delimiter}${oldPath}`; + + const result = await runClaudeTurn(process.cwd(), "x".repeat(8 * 1024 * 1024)); + + assert.equal(result.status, "failed"); + assert.match(result.stderr, /Failed to write Claude prompt to stdin/); + } finally { + process.env.PATH = oldPath; + fs.rmSync(tmpDir, { recursive: true, force: true }); + } + }); + + it("does not let a secondary stdin error hide an authentication failure", async () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "cc-plugin-claude-stdin-auth-")); + const oldPath = process.env.PATH ?? ""; + try { + createFakeClaudeCommand( + tmpDir, + `process.stdin.destroy();\nprocess.stderr.write("Not logged in. Run claude auth login to continue.\\n");\nsetTimeout(() => process.exit(1), 100);\n` + ); + process.env.PATH = `${tmpDir}${path.delimiter}${oldPath}`; + + const result = await runClaudeTurn(process.cwd(), "x".repeat(8 * 1024 * 1024)); + + assert.equal(result.status, "failed"); + assert.equal(result.warning, undefined); + assert.equal(result.failure?.kind, "claude_auth"); + assert.match(result.stderr, /Not logged in/); + assert.match(result.stderr, /Failed to write Claude prompt to stdin/); + } finally { + process.env.PATH = oldPath; + fs.rmSync(tmpDir, { recursive: true, force: true }); + } + }); + it("keeps only the newest stderr bytes on failed Claude runs", async () => { const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "cc-plugin-claude-")); const oldPath = process.env.PATH ?? ""; @@ -2173,12 +2256,12 @@ describe("buildArgs", () => { assert.equal(args[0], "-p"); }); - it("ends with -- separator followed by prompt", () => { - const args = buildArgs("my prompt"); - const dashDashIdx = args.indexOf("--"); - assert.ok(dashDashIdx >= 0); - assert.equal(args[dashDashIdx + 1], "my prompt"); - assert.equal(args[args.length - 1], "my prompt"); + it("keeps the prompt out of argv so runClaudeTurn can send it through stdin", () => { + const prompt = "x".repeat(70_000); + const args = buildArgs(prompt); + + assert.equal(args.includes("--"), false); + assert.equal(args.includes(prompt), false); }); it("defaults output format to json", () => { @@ -2220,6 +2303,14 @@ describe("buildArgs", () => { assert.equal(args[idx + 1], "xhigh"); }); + it("omits whitespace-only model and effort values", () => { + const args = buildArgs("p", { model: " ", effort: " " }); + + assert.equal(args.includes("--model"), false); + assert.equal(args.includes("--effort"), false); + assert.equal(args.every((value) => typeof value === "string"), true); + }); + it("passes 'max' through as --effort max when explicitly requested", () => { const args = buildArgs("p", { effort: "max" }); const idx = args.indexOf("--effort"); diff --git a/tests/e2e/codex-skills-e2e.test.mjs b/tests/e2e/codex-skills-e2e.test.mjs index 940fd23..1178d81 100644 --- a/tests/e2e/codex-skills-e2e.test.mjs +++ b/tests/e2e/codex-skills-e2e.test.mjs @@ -77,6 +77,15 @@ function sleep(ms) { return new Promise((resolve) => setTimeout(resolve, ms)); } +async function readStdin() { + let body = ""; + process.stdin.setEncoding("utf8"); + for await (const chunk of process.stdin) { + body += chunk; + } + return body; +} + async function main() { if (args[0] === "--version") { process.stdout.write("2.1.90 (Claude Code)\\n"); @@ -95,7 +104,7 @@ async function main() { } const promptIndex = args.lastIndexOf("--"); - const prompt = promptIndex >= 0 ? args.slice(promptIndex + 1).join(" ") : ""; + const prompt = promptIndex >= 0 ? args.slice(promptIndex + 1).join(" ") : await readStdin(); const delayMatch = prompt.match(/\\bdelay=(\\d+)\\b/); const delay = delayMatch ? Number(delayMatch[1]) : 25; const sessionId = @@ -1722,7 +1731,7 @@ describe("Codex direct-skill E2E", () => { userRequest, expectedNeedles: ["Claude Code Review"], shellCommands: [ - `node ${JSON.stringify(companionScript)} review --view-state on-success --scope working-tree --model haiku`, + `node ${JSON.stringify(companionScript)} review --view-state on-terminal --scope working-tree --model haiku`, ], cwd: workspaceDir, }); @@ -1793,7 +1802,7 @@ describe("Codex direct-skill E2E", () => { userRequest, expectedNeedles: ["Claude Code Review"], shellCommands: [ - `node ${JSON.stringify(COMPANION_SCRIPT)} review --view-state on-success --scope working-tree --model haiku`, + `node ${JSON.stringify(COMPANION_SCRIPT)} review --view-state on-terminal --scope working-tree --model haiku`, ], cwd: workspaceDir, }); @@ -1944,7 +1953,7 @@ describe("Codex direct-skill E2E", () => { userRequest, expectedNeedles: ["Claude Code Adversarial Review"], shellCommands: [ - `node ${JSON.stringify(COMPANION_SCRIPT)} adversarial-review --view-state on-success --scope working-tree --model haiku focus on race conditions`, + `node ${JSON.stringify(COMPANION_SCRIPT)} adversarial-review --view-state on-terminal --scope working-tree --model haiku focus on race conditions`, ], cwd: workspaceDir, }); @@ -2003,7 +2012,7 @@ describe("Codex direct-skill E2E", () => { "keep the delegated Claude part on `$cc:review`", ], shellCommands: [ - `node ${JSON.stringify(COMPANION_SCRIPT)} adversarial-review --view-state on-success --scope working-tree --model haiku focus on race conditions`, + `node ${JSON.stringify(COMPANION_SCRIPT)} adversarial-review --view-state on-terminal --scope working-tree --model haiku focus on race conditions`, ], cwd: workspaceDir, }); diff --git a/tests/hooks.test.mjs b/tests/hooks.test.mjs index 15ea374..889be85 100644 --- a/tests/hooks.test.mjs +++ b/tests/hooks.test.mjs @@ -251,6 +251,7 @@ function enableReviewGate(testEnv) { `${JSON.stringify({ version: 1, stopReviewGate: true }, null, 2)}\n`, "utf8" ); + writeStaleTurnBaseline(testEnv, "hook-session"); } function readCurrentSessionMarker(testEnv) { @@ -320,6 +321,12 @@ function writeTurnBaselineSnapshot(testEnv, sessionId, fingerprint) { ); } +function writeStaleTurnBaseline(testEnv, sessionId) { + writeTurnBaselineSnapshot(testEnv, sessionId, { + signature: "stale-baseline", + }); +} + describe("hooks", () => { it("stop-review hook uses read-only sandbox settings when review gate is enabled", () => { const testEnv = createHookEnvironment(); @@ -452,6 +459,92 @@ describe("hooks", () => { } }); + it("stop-review hook skips Claude when no user turn was recorded", () => { + const testEnv = createHookEnvironment(); + + try { + const stateDir = stateDirFor(testEnv.homeDir, testEnv.workspaceDir); + fs.mkdirSync(stateDir, { recursive: true }); + fs.writeFileSync( + path.join(stateDir, "config.json"), + JSON.stringify({ version: 1, stopReviewGate: true }, null, 2) + "\n", + "utf8" + ); + const argsFile = path.join(testEnv.rootDir, "claude-args.json"); + + const result = runHook( + STOP_HOOK, + [], + { + cwd: testEnv.workspaceDir, + session_id: "headless-session", + last_assistant_message: "review me", + }, + { ...testEnv.env, CLAUDE_ARGS_FILE: argsFile } + ); + + assert.equal(result.stdout.trim(), ""); + assert.match(result.stderr, /no user turn was recorded/i); + assert.equal(fs.existsSync(argsFile), false); + const snapshot = readStopReviewSnapshot(testEnv); + assert.equal(snapshot.status, "skipped_no_turn_baseline"); + assert.equal(snapshot.claudeInvoked, false); + } finally { + cleanupHookEnvironment(testEnv); + } + }); + + it("records baseline capture failures and lets Stop run the review", () => { + const testEnv = createHookEnvironment(); + + try { + enableReviewGate(testEnv); + runHook( + UNREAD_HOOK, + [], + { + hook_event_name: "UserPromptSubmit", + cwd: testEnv.workspaceDir, + session_id: "capture-failed-session", + prompt: "edit something", + }, + { + ...testEnv.env, + PATH: testEnv.binDir, + } + ); + + const baseline = JSON.parse( + fs.readFileSync( + path.join( + stateDirFor(testEnv.homeDir, testEnv.workspaceDir), + "turn-baseline.capture-failed-session.json" + ), + "utf8" + ) + ); + assert.equal(baseline.fingerprint, null); + assert.match(baseline.captureError, /git/i); + + const result = runHook( + STOP_HOOK, + [], + { + cwd: testEnv.workspaceDir, + session_id: "capture-failed-session", + last_assistant_message: "review me", + }, + testEnv.env + ); + assert.equal(result.stdout.trim(), ""); + const snapshot = readStopReviewSnapshot(testEnv); + assert.equal(snapshot.status, "allow"); + assert.equal(snapshot.claudeInvoked, true); + } finally { + cleanupHookEnvironment(testEnv); + } + }); + it("session lifecycle hook resolves queued session jobs on SessionEnd", () => { const testEnv = createHookEnvironment(); @@ -1364,6 +1457,50 @@ if (args.at(-1) === process.env.CC_TEST_LOCK_OWNER_PID) { } }); + it("session start stamps Claude Code as the external host origin", () => { + const testEnv = createHookEnvironment(); + + try { + /** @type {NodeJS.ProcessEnv} */ + const env = { ...testEnv.env, CLAUDECODE: "1" }; + delete env[SESSION_ID_ENV]; + runHook( + SESSION_HOOK, + [], + { cwd: testEnv.workspaceDir, session_id: "cc-thread" }, + env + ); + + assert.equal(readCurrentSessionMarker(testEnv).hostOrigin, "claude-code"); + } finally { + cleanupHookEnvironment(testEnv); + } + }); + + it("session start leaves external host origin unset for plain Codex", () => { + const testEnv = createHookEnvironment(); + + try { + /** @type {NodeJS.ProcessEnv} */ + const env = { ...testEnv.env }; + delete env.CLAUDECODE; + delete env.CLAUDE_CODE_ENTRYPOINT; + delete env[SESSION_ID_ENV]; + runHook( + SESSION_HOOK, + [], + { cwd: testEnv.workspaceDir, session_id: "plain-session" }, + env + ); + + const marker = readCurrentSessionMarker(testEnv); + assert.equal(marker.sessionId, "plain-session"); + assert.equal("hostOrigin" in marker, false); + } finally { + cleanupHookEnvironment(testEnv); + } + }); + it("session start exports the Claude transcript path for transfer", () => { const testEnv = createHookEnvironment(); @@ -1466,6 +1603,7 @@ if (args.at(-1) === process.env.CC_TEST_LOCK_OWNER_PID) { JSON.stringify({ version: 1, stopReviewGate: true }, null, 2) + "\n", "utf8" ); + writeStaleTurnBaseline(testEnv, "hook-session"); const result = runHook( STOP_HOOK, @@ -1510,6 +1648,7 @@ if (args.at(-1) === process.env.CC_TEST_LOCK_OWNER_PID) { JSON.stringify({ version: 1, stopReviewGate: true }, null, 2) + "\n", "utf8" ); + writeStaleTurnBaseline(testEnv, "hook-session"); const result = runHook( STOP_HOOK, @@ -1557,6 +1696,7 @@ if (args.at(-1) === process.env.CC_TEST_LOCK_OWNER_PID) { JSON.stringify({ version: 1, stopReviewGate: true }, null, 2) + "\n", "utf8" ); + writeStaleTurnBaseline(testEnv, "hook-session"); const result = runHook( STOP_HOOK, @@ -1703,6 +1843,7 @@ if (args.at(-1) === process.env.CC_TEST_LOCK_OWNER_PID) { JSON.stringify({ version: 1, stopReviewGate: true }, null, 2) + "\n", "utf8" ); + writeStaleTurnBaseline(testEnv, "hook-session"); const result = runHook( STOP_HOOK, @@ -1742,6 +1883,7 @@ if (args.at(-1) === process.env.CC_TEST_LOCK_OWNER_PID) { JSON.stringify({ version: 1, stopReviewGate: true }, null, 2) + "\n", "utf8" ); + writeStaleTurnBaseline(testEnv, "hook-session"); writeStateJob(testEnv, "running-review-job", { id: "running-review-job", status: "running", diff --git a/tests/integration/claude-companion.test.mjs b/tests/integration/claude-companion.test.mjs index 205e770..c372382 100644 --- a/tests/integration/claude-companion.test.mjs +++ b/tests/integration/claude-companion.test.mjs @@ -35,6 +35,15 @@ function sleep(ms) { return new Promise((resolve) => setTimeout(resolve, ms)); } +async function readStdin() { + let body = ""; + process.stdin.setEncoding("utf8"); + for await (const chunk of process.stdin) { + body += chunk; + } + return body; +} + function sanitize(value) { return String(value || "session") .toLowerCase() @@ -69,7 +78,7 @@ async function main() { const prompt = promptIndex >= 0 ? args.slice(promptIndex + 1).join(" ") - : ""; + : await readStdin(); const delay = Number((prompt.match(/\\bdelay=(\\d+)\\b/) || [])[1] || 80); if (process.env.CLAUDE_ARGS_FILE) { require("node:fs").writeFileSync( @@ -482,7 +491,7 @@ function writeSessionScopedJob(testEnv, jobId, payload) { return { stateDir, jobsDir }; } -function writeCurrentSessionMarker(testEnv, sessionId) { +function writeCurrentSessionMarker(testEnv, sessionId, options = {}) { const realWorkspace = fs.realpathSync.native(testEnv.workspaceDir); const workspaceHash = createHash("sha256").update(realWorkspace).digest("hex").slice(0, 12); const stateDir = path.join( @@ -497,7 +506,15 @@ function writeCurrentSessionMarker(testEnv, sessionId) { fs.mkdirSync(stateDir, { recursive: true }); fs.writeFileSync( path.join(stateDir, "current-session.json"), - JSON.stringify({ sessionId, updatedAt: "2026-04-03T12:00:00Z" }, null, 2) + "\n", + JSON.stringify( + { + sessionId, + ...(options.hostOrigin ? { hostOrigin: options.hostOrigin } : {}), + updatedAt: new Date().toISOString(), + }, + null, + 2 + ) + "\n", "utf8" ); } @@ -821,6 +838,14 @@ function assertCompletedReviewPayload(payload) { } describe("claude-companion integration", () => { + it("documents owner routing, canonical view state, and status wait controls in help", () => { + const result = runCompanion(["--help"]); + + assert.match(result.stdout, /--owner-session-id /); + assert.match(result.stdout, /--view-state /); + assert.match(result.stdout, /status \[job-id\].*--wait.*--wait-timeout-ms /); + }); + it("setup toggles the review gate on and off for the current workspace", () => { const testEnv = createTestEnvironment(); @@ -909,6 +934,66 @@ describe("claude-companion integration", () => { } }); + it("setup --check preserves ready config and state file metadata", () => { + const testEnv = createTestEnvironment(); + const codexDir = path.join(testEnv.homeDir, ".codex"); + const configFile = path.join(codexDir, "config.toml"); + const configContent = "[features]\nhooks = true\nplugin_hooks = true\n"; + const stateDir = stateDirFor(testEnv); + const stateFile = path.join(stateDir, "config.json"); + fs.mkdirSync(codexDir, { recursive: true }); + fs.mkdirSync(stateDir, { recursive: true }); + fs.writeFileSync(configFile, configContent, { mode: 0o640 }); + fs.writeFileSync( + stateFile, + JSON.stringify({ version: 1, stopReviewGate: false }, null, 2) + "\n", + { mode: 0o600 } + ); + const fixedMtime = new Date("2026-01-01T00:00:00.000Z"); + fs.utimesSync(configFile, fixedMtime, fixedMtime); + fs.utimesSync(stateFile, fixedMtime, fixedMtime); + const before = [configFile, stateFile].map((filePath) => ({ + content: fs.readFileSync(filePath, "utf8"), + mode: fs.statSync(filePath).mode & 0o777, + mtimeMs: fs.statSync(filePath).mtimeMs, + })); + const fakeCodex = createFakeCodexAppServer(testEnv, [ + { + key: "cc@sendbird:hooks/hooks.json:session_start:0:0", + sourcePath: path.join(PROJECT_ROOT, "hooks", "hooks.json"), + source: "plugin", + pluginId: "cc@sendbird", + currentHash: "sha256:session", + trustStatus: "trusted", + }, + ]); + + try { + const report = runCompanionJson( + ["setup", "--cwd", testEnv.workspaceDir, "--check", "--json"], + { + env: { + ...testEnv.env, + CC_PLUGIN_CODEX_EXECUTABLE: process.execPath, + CC_PLUGIN_CODEX_APP_SERVER_ARGS_JSON: JSON.stringify([fakeCodex.serverPath]), + CC_PLUGIN_CODEX_FORCE_HOOK_TRUST: "1", + }, + } + ); + + assert.equal(report.ready, true); + assert.deepEqual(report.actionsTaken, []); + for (const [index, filePath] of [configFile, stateFile].entries()) { + const stat = fs.statSync(filePath); + assert.equal(fs.readFileSync(filePath, "utf8"), before[index].content); + assert.equal(stat.mode & 0o777, before[index].mode); + assert.equal(stat.mtimeMs, before[index].mtimeMs); + } + } finally { + cleanupTestEnvironment(testEnv); + } + }); + it("rejects mutating review-gate flags in setup --check mode", () => { const testEnv = createTestEnvironment(); try { @@ -1920,6 +2005,131 @@ describe("claude-companion integration", () => { } }); + it("refuses same-owner delegation from a Claude-Code-driven thread", () => { + const testEnv = createTestEnvironment(); + + try { + setupGitWorkspace(testEnv.workspaceDir); + seedWorkingTreeDiff(testEnv.workspaceDir); + writeCurrentSessionMarker(testEnv, "cc-thread", { hostOrigin: "claude-code" }); + const env = { ...testEnv.env }; + delete env[SESSION_ID_ENV]; + + const review = runCompanionExpectFailure( + ["review", "--cwd", testEnv.workspaceDir, "--scope", "working-tree"], + { env } + ); + assert.match(review.stderr, /driven by Claude Code/); + assert.match(review.stderr, /Perform the requested review yourself/); + assert.equal(listStoredJobs(testEnv).length, 0); + + const exportedSession = runCompanionExpectFailure( + ["review", "--cwd", testEnv.workspaceDir, "--scope", "working-tree"], + { env: { ...testEnv.env, [SESSION_ID_ENV]: "cc-thread" } } + ); + assert.match(exportedSession.stderr, /driven by Claude Code/); + assert.equal(listStoredJobs(testEnv).length, 0); + + const task = runCompanionExpectFailure( + ["task", "--cwd", testEnv.workspaceDir, "investigate something"], + { env } + ); + assert.match(task.stderr, /Perform the requested task yourself/); + assert.equal(listStoredJobs(testEnv).length, 0); + } finally { + cleanupTestEnvironment(testEnv); + } + }); + + it("allows foreign-owner and unstamped delegation", () => { + const testEnv = createTestEnvironment(); + + try { + setupGitWorkspace(testEnv.workspaceDir); + seedWorkingTreeDiff(testEnv.workspaceDir); + writeCurrentSessionMarker(testEnv, "cc-thread", { hostOrigin: "claude-code" }); + + const noSessionEnv = { ...testEnv.env }; + delete noSessionEnv[SESSION_ID_ENV]; + runCompanion( + [ + "review", + "--cwd", + testEnv.workspaceDir, + "--scope", + "working-tree", + "--owner-session-id", + "interactive-parent", + ], + { env: noSessionEnv } + ); + const alignedMarker = JSON.parse( + fs.readFileSync( + path.join(stateDirFor(testEnv), "current-session.json"), + "utf8" + ) + ); + assert.equal(alignedMarker.sessionId, "interactive-parent"); + assert.equal(alignedMarker.hostOrigin, undefined); + runCompanion( + [ + "review", + "--cwd", + testEnv.workspaceDir, + "--scope", + "working-tree", + "--owner-session-id", + "interactive-parent", + ], + { env: noSessionEnv } + ); + + writeCurrentSessionMarker(testEnv, "plain-session"); + runCompanion( + ["review", "--cwd", testEnv.workspaceDir, "--scope", "working-tree"], + { env: noSessionEnv } + ); + } finally { + cleanupTestEnvironment(testEnv); + } + }); + + it("sends a Windows-sized review prompt through stdin instead of argv", () => { + const testEnv = createTestEnvironment(); + + try { + setupGitWorkspace(testEnv.workspaceDir); + const largeInput = path.join(testEnv.workspaceDir, "large-review-input.txt"); + fs.writeFileSync( + largeInput, + "before-line\n".repeat(2_000), + "utf8" + ); + runGit(testEnv.workspaceDir, ["add", "large-review-input.txt"]); + runGit(testEnv.workspaceDir, ["commit", "-m", "add review fixture"]); + fs.writeFileSync(largeInput, "review-line\n".repeat(2_000), "utf8"); + const invocationFile = path.join(testEnv.rootDir, "large-review-invocation.json"); + + const result = runCompanion( + ["review", "--cwd", testEnv.workspaceDir, "--scope", "working-tree", "--model", "haiku"], + { + env: { + ...testEnv.env, + CLAUDE_INVOCATION_FILE: invocationFile, + }, + } + ); + + const invocation = JSON.parse(fs.readFileSync(invocationFile, "utf8")); + assert.ok(Buffer.byteLength(invocation.prompt, "utf8") > 32_767); + assert.match(invocation.prompt, /review-line/); + assert.equal(invocation.args.includes(invocation.prompt), false); + assert.match(result.stdout, /Claude Code Review/); + } finally { + cleanupTestEnvironment(testEnv); + } + }); + it("reports review model fallbacks in JSON payloads", () => { const testEnv = createTestEnvironment(); @@ -2514,6 +2724,94 @@ describe("claude-companion integration", () => { } }); + it("rejects status timeout controls without --wait before numeric validation", () => { + const testEnv = createTestEnvironment(); + try { + const result = runCompanionExpectFailure( + [ + "status", + "--cwd", + testEnv.workspaceDir, + "--wait-timeout-ms", + "not-a-number", + "missing-job", + ], + { env: testEnv.env } + ); + + assert.match(result.stderr, /--wait-timeout-ms requires --wait/); + assert.doesNotMatch(result.stderr, /positive number/); + } finally { + cleanupTestEnvironment(testEnv); + } + }); + + it("warns when status accepts the deprecated timeout alias", () => { + const testEnv = createTestEnvironment(); + const jobId = "deprecated-status-timeout"; + writeSessionScopedJob(testEnv, jobId, { + id: jobId, + status: "completed", + jobClass: "task", + createdAt: "2026-04-03T10:00:00Z", + completedAt: "2026-04-03T10:00:01Z", + }); + + try { + const result = runCompanion( + [ + "status", + "--cwd", + testEnv.workspaceDir, + "--json", + "--wait", + "--timeout-ms", + "100", + jobId, + ], + { env: testEnv.env } + ); + + assert.match(result.stderr, /--timeout-ms is deprecated; use --wait-timeout-ms/); + } finally { + cleanupTestEnvironment(testEnv); + } + }); + + it("does not rewrite a healthy active job during status inspection", () => { + const testEnv = createTestEnvironment(); + const jobId = "healthy-status-read"; + const timestamp = new Date().toISOString(); + const { jobsDir } = writeSessionScopedJob(testEnv, jobId, { + id: jobId, + status: "running", + jobClass: "task", + sessionId: "session-a", + pid: process.pid, + workerPid: process.pid, + createdAt: timestamp, + updatedAt: timestamp, + }); + const jobFile = path.join(jobsDir, `${jobId}.json`); + const beforeContent = fs.readFileSync(jobFile, "utf8"); + const beforeStat = fs.statSync(jobFile); + + try { + const report = runCompanionJson( + ["status", "--cwd", testEnv.workspaceDir, "--json", "--all"], + { env: testEnv.env } + ); + + assert.ok(report.running.some((job) => job.id === jobId)); + assert.equal(fs.readFileSync(jobFile, "utf8"), beforeContent); + const afterStat = fs.statSync(jobFile); + assert.equal(afterStat.mode & 0o777, beforeStat.mode & 0o777); + assert.equal(afterStat.mtimeMs, beforeStat.mtimeMs); + } finally { + cleanupTestEnvironment(testEnv); + } + }); + it("filters status overview to the current session marker when env is unavailable", () => { const testEnv = createTestEnvironment(); @@ -3621,6 +3919,11 @@ describe("claude-companion integration", () => { const storedJob = readStoredJobById(testEnv, reserved.jobId); assert.equal(storedJob.status, "cancelled"); assert.match(storedJob.errorMessage, /Cancelled by user/); + assert.equal( + payload.sessionId, + storedJob.result?.sessionId ?? storedJob.threadId ?? null + ); + assert.notEqual(payload.sessionId, storedJob.sessionId); if (payload.resultMissing === true) { assert.equal(payload.jobStatus, "cancelled"); assert.match(payload.errorMessage, /Cancelled by user/); @@ -3772,6 +4075,76 @@ describe("claude-companion integration", () => { } }); + it("accepts canonical on-terminal view state and warns for the on-success alias", () => { + const testEnv = createTestEnvironment(); + const canonicalEnv = { + ...testEnv.env, + [SESSION_ID_ENV]: "session-view-state-canonical", + }; + const aliasEnv = { + ...testEnv.env, + [SESSION_ID_ENV]: "session-view-state-alias", + }; + + try { + runCompanion( + [ + "task", + "--cwd", + testEnv.workspaceDir, + "--view-state", + "on-terminal", + "canonical-view-state delay=20", + ], + { env: canonicalEnv } + ); + const alias = runCompanion( + [ + "task", + "--cwd", + testEnv.workspaceDir, + "--view-state", + "on-success", + "alias-view-state delay=20", + ], + { env: aliasEnv } + ); + + const canonicalJob = listStoredJobs(testEnv).find( + (job) => job.sessionId === "session-view-state-canonical" + ); + const aliasJob = listStoredJobs(testEnv).find( + (job) => job.sessionId === "session-view-state-alias" + ); + assert.match(canonicalJob.resultViewedAt, /\d{4}-\d{2}-\d{2}T/); + assert.match(aliasJob.resultViewedAt, /\d{4}-\d{2}-\d{2}T/); + assert.match(alias.stderr, /--view-state on-success is deprecated; use on-terminal/); + } finally { + cleanupTestEnvironment(testEnv); + } + }); + + it("warns when task accepts the deprecated timeout alias", () => { + const testEnv = createTestEnvironment(); + try { + const result = runCompanion( + [ + "task", + "--cwd", + testEnv.workspaceDir, + "--timeout-ms", + "5000", + "deprecated-timeout delay=20", + ], + { env: testEnv.env } + ); + + assert.match(result.stderr, /--timeout-ms is deprecated; use --wait-timeout-ms/); + } finally { + cleanupTestEnvironment(testEnv); + } + }); + it("keeps a foreground task running when the observing companion process dies", async () => { const testEnv = createTestEnvironment(); const sessionEnv = { diff --git a/tests/job-control.test.mjs b/tests/job-control.test.mjs index b2c6f53..b42780d 100644 --- a/tests/job-control.test.mjs +++ b/tests/job-control.test.mjs @@ -355,6 +355,32 @@ describe("enrichJob", () => { assert.match(enriched.elapsed, /\d+m/); }); + it("reports exact active-job progress freshness from the newest known activity", () => { + const repoDir = createTempGitRepo(); + const logFile = resolveJobLogFile(repoDir, "j-progress-age"); + fs.mkdirSync(path.dirname(logFile), { recursive: true }); + fs.writeFileSync(logFile, "[2026-04-03T10:00:04.000Z] Working.\n", "utf8"); + const logTime = new Date("2026-04-03T10:00:05.000Z"); + fs.utimesSync(logFile, logTime, logTime); + + try { + const enriched = enrichJob( + { + id: "j-progress-age", + status: "running", + workspaceRoot: repoDir, + updatedAt: "2026-04-03T10:00:03.000Z", + }, + { now: Date.parse("2026-04-03T10:00:08.250Z") } + ); + + assert.equal(enriched.lastProgressAt, "2026-04-03T10:00:05.000Z"); + assert.equal(enriched.progressAgeMs, 3250); + } finally { + fs.rmSync(repoDir, { recursive: true, force: true }); + } + }); + it("calculates duration for completed job", () => { const start = "2024-01-01T10:00:00Z"; const end = "2024-01-01T10:05:30Z"; diff --git a/tests/mutation-config.test.mjs b/tests/mutation-config.test.mjs index 9c8fbaa..e41971f 100644 --- a/tests/mutation-config.test.mjs +++ b/tests/mutation-config.test.mjs @@ -19,16 +19,16 @@ const expectations = [ ["scripts/lib/process.mjs:185-367", ["terminateProcessTreeIfIdentityMatches"]], ["scripts/lib/process.mjs:390-504", ["getProcessIdentity", "getSpawnedProcessIdentity", "validateProcessIdentity", "isProcessAlive", "isProcessGroupAlive"]], ["scripts/lib/state.mjs:188-228", ["ensurePluginDataLayout", "resolveWorkspaceHash", "ensureStateDir"]], - ["scripts/lib/state.mjs:297-371", ["setCurrentSession", "getCurrentSession", "clearCurrentSession", "markSessionCleanupPending", "listPendingSessionCleanups", "clearSessionCleanupPending"]], - ["scripts/lib/state.mjs:403-451", ["writeJobFile", "normalizeStoredJob"]], - ["scripts/lib/state.mjs:528-838", ["mostRecentJobTimestamp", "isWithinReapGracePeriod", "reapStaleJobs"]], - ["scripts/lib/state.mjs:887-1059", ["unlinkLockIfUnchanged", "remainingLockDeadlineMs", "lockProcessTimeout", "recoverStaleLock", "acquireJobLock", "releaseJobLock"]], - ["scripts/lib/state.mjs:1125-1182", ["casJobStatus", "transitionJob", "writeAtomic"]], - ["scripts/lib/state.mjs:1188-1234", ["cleanupOldJobs"]], - ["scripts/lib/tracked-jobs.mjs:26-39", ["transitionTrackedJob"]], - ["scripts/lib/tracked-jobs.mjs:282-340", ["createJobProgressUpdater"]], - ["scripts/lib/tracked-jobs.mjs:359-516", ["runTrackedJob"]], - ["scripts/lib/job-control.mjs:144-247", ["matchJobReference", "buildStatusSnapshot", "resolveCancelableJob"]], + ["scripts/lib/state.mjs:297-388", ["setCurrentSession", "getCurrentSession", "clearCurrentSession", "markSessionCleanupPending", "listPendingSessionCleanups", "clearSessionCleanupPending"]], + ["scripts/lib/state.mjs:420-468", ["writeJobFile", "normalizeStoredJob"]], + ["scripts/lib/state.mjs:545-875", ["mostRecentJobTimestamp", "isWithinReapGracePeriod", "reapStaleJobs"]], + ["scripts/lib/state.mjs:924-1096", ["unlinkLockIfUnchanged", "remainingLockDeadlineMs", "lockProcessTimeout", "recoverStaleLock", "acquireJobLock", "releaseJobLock"]], + ["scripts/lib/state.mjs:1162-1219", ["casJobStatus", "transitionJob", "writeAtomic"]], + ["scripts/lib/state.mjs:1225-1271", ["cleanupOldJobs"]], + ["scripts/lib/tracked-jobs.mjs:30-43", ["transitionTrackedJob"]], + ["scripts/lib/tracked-jobs.mjs:286-344", ["createJobProgressUpdater"]], + ["scripts/lib/tracked-jobs.mjs:363-522", ["runTrackedJob"]], + ["scripts/lib/job-control.mjs:170-273", ["matchJobReference", "buildStatusSnapshot", "resolveCancelableJob"]], ["scripts/installer-cli.mjs:96-234", ["readPersonalMarketplace", "prepareLegacyLocalCleanup", "isPluginAlreadyAbsent", "isPluginUninstallRefused"]], ["scripts/installer-cli.mjs:275-371", ["installOrUpdate", "uninstall"]], ]; @@ -73,6 +73,7 @@ test("Windows lifecycle gate patterns stay aligned with their tests", () => { ["tests/state.test.mjs", "keeps a running job while its owning worker is alive"], ["tests/state.test.mjs", "falls back to the identity-checked Claude PID"], ["tests/state.test.mjs", "terminates a live Claude child"], + ["tests/state.test.mjs", "waits briefly for a signalled Claude child"], ["tests/state.test.mjs", "bounds Windows Claude child cleanup"], ["tests/state.test.mjs", "clears a recycled Claude child PID"], ["tests/state.test.mjs", "clears an identity-unavailable Claude child"], diff --git a/tests/render.test.mjs b/tests/render.test.mjs index 3e8ea61..c9153af 100644 --- a/tests/render.test.mjs +++ b/tests/render.test.mjs @@ -473,6 +473,8 @@ describe("renderStatusReport", () => { kindLabel: "review", startedAt: "2026-04-02T19:00:00.000Z", elapsed: "2m 30s", + lastProgressAt: "2026-04-02T19:02:25.000Z", + progressAgeMs: 5000, updatedAt: "2026-04-02T19:02:30.000Z", }, ], @@ -501,11 +503,13 @@ describe("renderStatusReport", () => { needsReview: false, }; const output = renderStatusReport(report); - assert.ok(output.startsWith("| Job | Kind | Status | Phase | Started | Ended | Elapsed/Duration | Summary | Actions |")); + assert.ok(output.startsWith("| Job | Kind | Status | Phase | Started | Ended | Elapsed/Duration | Last Progress | Progress Age (ms) | Summary | Actions |")); assert.ok(output.includes("`$cc:status j1`")); assert.ok(output.includes("`$cc:cancel j1`")); assert.ok(output.includes("`$cc:result j2`")); assert.ok(output.includes("2026-04-02T19:00:00.000Z")); + assert.ok(output.includes("2026-04-02T19:02:25.000Z")); + assert.ok(output.includes("5000")); assert.ok(output.indexOf("j1") < output.indexOf("j2")); assert.ok(output.indexOf("j2") < output.indexOf("j3")); }); @@ -602,9 +606,13 @@ describe("renderJobStatusReport", () => { kindLabel: "review", startedAt: "2026-04-02T19:00:00.000Z", elapsed: "5s", + lastProgressAt: "2026-04-02T19:00:04.000Z", + progressAgeMs: 1000, }; const output = renderJobStatusReport(job); assert.ok(output.includes("| Elapsed | 5s |")); + assert.ok(output.includes("| Last progress | 2026-04-02T19:00:04.000Z |")); + assert.ok(output.includes("| Progress age (ms) | 1000 |")); assert.ok(output.includes("| Cancel | `$cc:cancel j2` |")); }); diff --git a/tests/skills-contracts.test.mjs b/tests/skills-contracts.test.mjs index 738f7d9..7680271 100644 --- a/tests/skills-contracts.test.mjs +++ b/tests/skills-contracts.test.mjs @@ -125,7 +125,7 @@ test("review skills preserve foreground/background routing contracts", () => { skill, [ `claude-companion.mjs" ${name} ...`, - `${name} --view-state on-success`, + `${name} --view-state on-terminal`, "background-routing-context --kind review --json", `${name} --cwd "" --view-state defer`, "--owner-session-id ", @@ -214,7 +214,7 @@ test("rescue keeps host execution controls out of the companion task", () => { "task-resume-candidate --owner-session-id --json", "background-routing-context --kind task --json", '--cwd ""', - "--view-state on-success", + "--view-state on-terminal", "--view-state defer", "--owner-session-id ", "--job-id ", @@ -254,8 +254,8 @@ test("internal runtime references preserve executable routing invariants", () => [ 'node "/scripts/claude-companion.mjs" review ...', 'node "/scripts/claude-companion.mjs" adversarial-review ...', - "review --view-state on-success", - "adversarial-review --view-state on-success", + "review --view-state on-terminal", + "adversarial-review --view-state on-terminal", "background-routing-context --kind review --json", "Never derive the workspace from the plugin root", "Never emit an empty routing placeholder such as `--owner-session-id --job-id`", @@ -312,3 +312,45 @@ test("setup keeps native hook repair in the companion flow", () => { ); assert.doesNotMatch(setup, /install-hooks\.mjs/i); }); + +test("review skills use only an available Codex question tool", () => { + for (const relativePath of [ + "skills/review/SKILL.md", + "skills/adversarial-review/SKILL.md", + ]) { + const contract = read(relativePath); + assert.doesNotMatch(contract, /AskUserQuestion/); + assert.match(contract, /request_user_input/); + assert.match(contract, /only when this thread actually has one/i); + assert.match(contract, /non-interactive thread/i); + assert.match(contract, /recommended mode/i); + } +}); + +test("companion skills keep network escalation narrow and policy-aware", () => { + const contracts = [ + read("skills/review/SKILL.md"), + read("skills/adversarial-review/SKILL.md"), + read("skills/rescue/SKILL.md"), + read("internal-skills/review-runtime/runtime.md"), + read("internal-skills/cli-runtime/runtime.md"), + ]; + + for (const contract of contracts) { + assert.match(contract, /sandbox_permissions: "require_escalated"/i); + assert.match(contract, /only (?:when|if).*(?:schema|tool).*(?:policy|permit)/i); + assert.match(contract, /higher-priority policy/i); + assert.match(contract, /do not (?:enable|change).*global network/i); + assert.match(contract, /do not first try.*network-disabled sandbox/i); + } +}); + +test("synthetic task results never use the owning Codex session as a Claude session", () => { + const source = read("scripts/claude-companion.mjs"); + const start = source.indexOf("function buildStoredTaskPayload"); + const end = source.indexOf("\nfunction renderForegroundTaskStillRunning", start); + const implementation = source.slice(start, end); + + assert.match(implementation, /sessionId: job\?\.threadId \?\? null/); + assert.doesNotMatch(implementation, /sessionId:.*job\?\.sessionId/); +}); diff --git a/tests/state.test.mjs b/tests/state.test.mjs index c03f017..d5eb8f5 100644 --- a/tests/state.test.mjs +++ b/tests/state.test.mjs @@ -36,6 +36,7 @@ import { casJobStatus, setCurrentSession, getCurrentSession, + getCurrentSessionMarker, clearCurrentSession, markSessionCleanupPending, listPendingSessionCleanups, @@ -1204,6 +1205,16 @@ describe("current session marker", () => { assert.equal(getCurrentSession(repoDir), sessionId); }); + it("stores the external host origin without changing the session fallback", () => { + setCurrentSession(repoDir, sessionId, { hostOrigin: "claude-code" }); + + assert.equal(getCurrentSession(repoDir), sessionId); + assert.deepEqual(getCurrentSessionMarker(repoDir), { + sessionId, + hostOrigin: "claude-code", + }); + }); + it("clears the current session id", () => { setCurrentSession(repoDir, sessionId); clearCurrentSession(repoDir, sessionId); @@ -1713,16 +1724,18 @@ describe("reapStaleJobs", () => { }); backdateJob(id, staleTimestamp()); const terminated = []; + let childAlive = true; const result = reapStaleJobs( PROJECT_CWD, [readJobFile(PROJECT_CWD, id)], { platform: "linux", - isProcessAliveImpl: (pid) => pid === claudePid, + isProcessAliveImpl: (pid) => pid === claudePid && childAlive, getProcessIdentityImpl: () => "worker-identity", terminateProcessTreeIfIdentityMatchesImpl: (pid, identity) => { terminated.push([pid, identity]); + childAlive = false; return { attempted: true, delivered: true }; }, } @@ -1734,6 +1747,84 @@ describe("reapStaleJobs", () => { assert.equal(result[0].pidIdentity, null); }); + it("retains Claude child cleanup handles when signal delivery does not stop it", () => { + const id = "test-reap-dead-worker-live-child-after-signal"; + const claudePid = 11119; + const workerPid = 22230; + writeJobFile(PROJECT_CWD, id, { + id, + status: "running", + pid: claudePid, + pidIdentity: "claude-identity", + workerPid, + workerPidIdentity: "worker-identity", + createdAt: nowIso(), + }); + backdateJob(id, staleTimestamp()); + + const result = reapStaleJobs( + PROJECT_CWD, + [readJobFile(PROJECT_CWD, id)], + { + platform: "linux", + isProcessAliveImpl: (pid) => pid === claudePid, + getProcessIdentityImpl: () => "worker-identity", + terminateProcessTreeIfIdentityMatchesImpl: () => ({ + attempted: true, + delivered: true, + }), + childExitWaitMs: 0, + } + ); + + assert.equal(result[0].status, "failed"); + assert.equal(result[0].pid, claudePid); + assert.equal(result[0].pidIdentity, "claude-identity"); + assert.match(result[0].errorMessage, /manual cleanup/i); + }); + + it("waits briefly for a signalled Claude child to exit before retaining handles", () => { + const id = "test-reap-dead-worker-child-exits-after-signal"; + const claudePid = 11120; + const workerPid = 22231; + writeJobFile(PROJECT_CWD, id, { + id, + status: "running", + pid: claudePid, + pidIdentity: "claude-identity", + workerPid, + workerPidIdentity: "worker-identity", + createdAt: nowIso(), + }); + backdateJob(id, staleTimestamp()); + let childChecks = 0; + + const result = reapStaleJobs( + PROJECT_CWD, + [readJobFile(PROJECT_CWD, id)], + { + platform: "linux", + isProcessAliveImpl: (pid) => { + if (pid !== claudePid) return false; + childChecks += 1; + return childChecks < 3; + }, + getProcessIdentityImpl: () => "worker-identity", + terminateProcessTreeIfIdentityMatchesImpl: () => ({ + attempted: true, + delivered: true, + }), + childExitWaitMs: 1_000, + sleepSyncImpl: () => {}, + } + ); + + assert.equal(childChecks, 3); + assert.equal(result[0].pid, null); + assert.equal(result[0].pidIdentity, null); + assert.doesNotMatch(result[0].errorMessage, /manual cleanup/i); + }); + it("bounds Windows Claude child cleanup to the reaper identity timeout", () => { const id = "test-reap-dead-worker-windows-child-timeout"; const claudePid = 11116; @@ -2314,6 +2405,8 @@ describe("reapStaleJobs", () => { status: "running", pid: process.pid, pidIdentity: "stored-identity", + workerPid: process.pid, + workerPidIdentity: "stored-identity", createdAt: nowIso(), }); backdateJob(id, new Date(Date.now() - 301_000).toISOString()); @@ -2340,6 +2433,8 @@ describe("reapStaleJobs", () => { assert.equal(result[0].status, "failed"); assert.equal(result[0].pid, process.pid); assert.equal(result[0].pidIdentity, "stored-identity"); + assert.equal(result[0].workerPid, null); + assert.equal(result[0].workerPidIdentity, null); assert.equal(result[0].reapedUnverifiable, true); assert.match(result[0].errorMessage, /identity remained unverifiable/i); }); diff --git a/tests/tracked-jobs.test.mjs b/tests/tracked-jobs.test.mjs index 337a4fc..c9c5193 100644 --- a/tests/tracked-jobs.test.mjs +++ b/tests/tracked-jobs.test.mjs @@ -426,6 +426,38 @@ describe("createJobProgressUpdater", () => { // --------------------------------------------------------------------------- describe("runTrackedJob", () => { + it("uses the spawn-safe identity probe for its current worker", async () => { + const repoDir = createTempGitRepo(); + const job = { + id: "tracked-spawn-safe-worker-identity", + workspaceRoot: repoDir, + status: "queued", + title: "spawn-safe worker identity", + createdAt: nowIso(), + updatedAt: nowIso(), + }; + writeJobFile(repoDir, job.id, job); + + try { + await runTrackedJob( + job, + async () => { + assert.equal( + readJobFile(repoDir, job.id).workerPidIdentity, + "spawn-safe-identity" + ); + return { exitStatus: 0, rendered: "finished" }; + }, + { + getProcessIdentityImpl: () => "generic-identity", + getSpawnedProcessIdentityImpl: () => "spawn-safe-identity", + } + ); + } finally { + fs.rmSync(repoDir, { recursive: true, force: true }); + } + }); + it("tracks the worker separately from the cancellable Claude process", async () => { const repoDir = createTempGitRepo(); const job = { diff --git a/tests/unread-result-hook.test.mjs b/tests/unread-result-hook.test.mjs index 46f219a..75f7e40 100644 --- a/tests/unread-result-hook.test.mjs +++ b/tests/unread-result-hook.test.mjs @@ -9,7 +9,7 @@ import os from "node:os"; import path from "node:path"; import { spawnSync } from "node:child_process"; import { createHash } from "node:crypto"; -import { fileURLToPath } from "node:url"; +import { fileURLToPath, pathToFileURL } from "node:url"; const PROJECT_ROOT = path.resolve( fileURLToPath(new URL("../", import.meta.url)) @@ -82,7 +82,7 @@ function readJob(testEnv, jobId) { ); } -function runHook(testEnv, payload) { +function runHook(testEnv, payload, extraEnv = {}) { const result = spawnSync(process.execPath, [HOOK_SCRIPT], { cwd: PROJECT_ROOT, env: { @@ -90,6 +90,7 @@ function runHook(testEnv, payload) { HOME: testEnv.homeDir, USERPROFILE: testEnv.homeDir, CODEX_HOME: path.join(testEnv.homeDir, ".codex"), + ...extraEnv, }, input: JSON.stringify(payload), encoding: "utf8", @@ -172,6 +173,90 @@ test("injects one-shot context for same-session completed unread jobs and marks } }); +test("does not mark a job notified after its terminal status changes", () => { + const testEnv = createEnv(); + try { + const job = { + id: "task-status-race", + sessionId: "session-a", + status: "completed", + summary: "finished before retry", + createdAt: "2026-04-03T10:00:00Z", + updatedAt: "2026-04-03T10:01:00Z", + completedAt: "2026-04-03T10:01:00Z", + }; + writeJob(testEnv, job); + const jobFile = path.join( + stateDirFor(testEnv), + "jobs", + `${job.id}.json` + ); + + runHook( + testEnv, + { + hook_event_name: "UserPromptSubmit", + cwd: testEnv.workspaceDir, + session_id: "session-a", + prompt: "continue working", + }, + { + NODE_OPTIONS: `--import=${pathToFileURL( + path.join(PROJECT_ROOT, "tests", "fixtures", "swap-job-after-read.mjs") + ).href}`, + CC_TEST_SWAP_JOB_FILE: jobFile, + CC_TEST_SWAP_JOB_JSON: JSON.stringify({ + ...job, + status: "running", + completedAt: null, + }), + } + ); + + assert.equal(readJob(testEnv, job.id).status, "running"); + assert.equal(readJob(testEnv, job.id).notifiedAt, undefined); + } finally { + cleanupEnv(testEnv); + } +}); + +test("still announces terminal jobs when notification state is temporarily locked", () => { + const testEnv = createEnv(); + try { + const job = { + id: "task-lock-race", + sessionId: "session-a", + status: "completed", + summary: "finished during lock contention", + createdAt: "2026-04-03T10:00:00Z", + updatedAt: "2026-04-03T10:01:00Z", + completedAt: "2026-04-03T10:01:00Z", + }; + writeJob(testEnv, job); + fs.writeFileSync( + path.join(stateDirFor(testEnv), "jobs", `${job.id}.json.lock`), + JSON.stringify({ + pid: process.pid, + timestamp: Date.now(), + token: "held-by-test", + }) + "\n", + "utf8" + ); + + const output = runHook(testEnv, { + hook_event_name: "UserPromptSubmit", + cwd: testEnv.workspaceDir, + session_id: "session-a", + prompt: "continue working", + }); + + assert.match(output, /task-lock-race/); + assert.equal(readJob(testEnv, job.id).notifiedAt, undefined); + } finally { + cleanupEnv(testEnv); + } +}); + test("announces failed terminal jobs with their outcome", () => { const testEnv = createEnv(); try { @@ -257,6 +342,42 @@ test("records a turn baseline for the current session on UserPromptSubmit", () = } }); +test("does not replace an existing parent marker from an unmarked child prompt", () => { + const testEnv = createEnv(); + try { + const stateDir = stateDirFor(testEnv); + fs.mkdirSync(stateDir, { recursive: true }); + fs.writeFileSync( + path.join(stateDir, "current-session.json"), + JSON.stringify( + { + sessionId: "parent-session", + hostOrigin: "claude-code", + updatedAt: new Date().toISOString(), + }, + null, + 2 + ) + "\n", + "utf8" + ); + + runHook(testEnv, { + hook_event_name: "UserPromptSubmit", + cwd: testEnv.workspaceDir, + session_id: "child-session", + prompt: "continue working", + }); + + const marker = JSON.parse( + fs.readFileSync(path.join(stateDir, "current-session.json"), "utf8") + ); + assert.equal(marker.sessionId, "parent-session"); + assert.equal(marker.hostOrigin, "claude-code"); + } finally { + cleanupEnv(testEnv); + } +}); + test("does not record a turn baseline when the review gate is disabled", () => { const testEnv = createEnv(); try {