From c93681c45d3f53a95cc33af3ed341be8e3af499e Mon Sep 17 00:00:00 2001 From: CBEPX <458940+CBEPX@users.noreply.github.com> Date: Mon, 21 Sep 2026 16:10:26 +0300 Subject: [PATCH 1/3] fix(hooks): bound prompt lifecycle work --- .codex-plugin/plugin.json | 2 +- CHANGELOG.md | 11 ++++ README.md | 4 +- hooks/hooks.json | 2 + hooks/session-lifecycle-hook.mjs | 1 - hooks/unread-result-hook.mjs | 42 ++++++++++----- internal-skills/cli-runtime/runtime.md | 9 ++-- internal-skills/review-runtime/runtime.md | 10 ++-- package-lock.json | 4 +- package.json | 2 +- scripts/lib/git.mjs | 6 +-- scripts/lib/mcp-capabilities.mjs | 4 +- scripts/lib/state.mjs | 16 ++++-- scripts/lib/workspace.mjs | 3 ++ skills/adversarial-review/SKILL.md | 20 ++++---- skills/rescue/SKILL.md | 16 +++--- skills/review/SKILL.md | 20 ++++---- stryker.shard.config.mjs | 8 +-- tests/e2e/codex-skills-e2e.test.mjs | 10 ++-- tests/hooks.test.mjs | 9 ++-- tests/mutation-config.test.mjs | 8 +-- tests/skills-contracts.test.mjs | 18 +++---- tests/unread-result-hook.test.mjs | 62 +++++++++++++++++++++++ 23 files changed, 193 insertions(+), 94 deletions(-) diff --git a/.codex-plugin/plugin.json b/.codex-plugin/plugin.json index 7f55e30..6f9abaf 100644 --- a/.codex-plugin/plugin.json +++ b/.codex-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "cc", - "version": "1.7.5", + "version": "1.7.6", "description": "Claude Code Plugin for Codex. Run reviews, tracked tasks, and independent Codex-Claude design or research workflows.", "author": { "name": "CBEPX", diff --git a/CHANGELOG.md b/CHANGELOG.md index 03b9925..02ce4ba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,17 @@ ## [Unreleased] +## v1.7.6 + +### Changed + +- Bound session-start and prompt hooks at the manifest boundary and bound prompt-time Git discovery and stale-job identity probes inside the hook budget. +- Align collaboration skill instructions with `fork_turns: "none"`, parent-only `send_message` delivery, computed defaults when no question tool is available, and explicit Fable reasoning effort. + +### Fixed + +- Keep malformed or oversized lifecycle hook payloads fail-open and retain unread background results when parent delivery fails. + ## v1.7.5 ### Changed diff --git a/README.md b/README.md index 50d9c7c..29cab79 100644 --- a/README.md +++ b/README.md @@ -140,7 +140,7 @@ $cc:review --user-mcp-tool mcp__context7__resolve-library-id **Flags:** `--base `, `--scope `, `--wait`, `--background`, `--model `, `--effort `, `--user-mcp-tool `, `--allow-project-mcp-servers` -**Defaults:** model `opus` is passed to Claude Code as its native alias with `xhigh` effort. `sonnet` is passed through with `high` effort; `haiku` and `fable` are passed through with no default effort setting. Claude Code resolves aliases to the current model for the active provider and account (for example, Opus 5). Pass a full model ID to pin a version; for older pinned IDs, pass `--effort` explicitly instead of inheriting a current-family default. +**Defaults:** model `opus` is passed to Claude Code as its native alias with `xhigh` effort. `sonnet` is passed through with `high` effort; `haiku` and `fable` have no implicit effort. Explicit Fable effort, including `medium`, is forwarded unchanged. Claude Code resolves aliases to the current model for the active provider and account (for example, Opus 5). Pass a full model ID to pin a version; for older pinned IDs, pass `--effort` explicitly instead of inheriting a current-family default. Fable 5.1 (`claude-fable-5-1`) has a native 1M context window and requires Claude Code 2.1.257 or newer. The bare `fable` alias remains floating and may still resolve to Fable 5 behind Claude Apps Gateway; use the full model ID when Fable 5.1 is required. No `[1m]` suffix or hidden Fable effort default is added. See Claude Code's [model configuration](https://code.claude.com/docs/en/model-config). @@ -228,7 +228,7 @@ $cc:rescue --model sonnet --effort medium investigate the flaky test | `--fresh` | Force a new task (don't resume) | | `--write` | Allow file edits (default) | | `--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) | +| `--effort ` | Reasoning effort: `low`, `medium`, `high`, `xhigh`, `max` (default: `xhigh` for opus, `high` for sonnet, unset for haiku and fable; explicit Fable effort such as `medium` is supported) | | `--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 | diff --git a/hooks/hooks.json b/hooks/hooks.json index e87615a..015d9ca 100644 --- a/hooks/hooks.json +++ b/hooks/hooks.json @@ -7,6 +7,7 @@ { "type": "command", "command": "node \"${PLUGIN_DATA}/runtime/hook-launcher.mjs\" session-lifecycle", + "timeout": 5, "statusMessage": "Initializing Claude Code bridge" } ] @@ -43,6 +44,7 @@ { "type": "command", "command": "node \"${PLUGIN_DATA}/runtime/hook-launcher.mjs\" unread-result", + "timeout": 5, "statusMessage": "Checking for unread Claude Code results" } ] diff --git a/hooks/session-lifecycle-hook.mjs b/hooks/session-lifecycle-hook.mjs index 91f7926..d0cee0f 100644 --- a/hooks/session-lifecycle-hook.mjs +++ b/hooks/session-lifecycle-hook.mjs @@ -580,5 +580,4 @@ main().catch((error) => { process.stderr.write( `${error instanceof Error ? error.message : String(error)}\n` ); - process.exit(1); }); diff --git a/hooks/unread-result-hook.mjs b/hooks/unread-result-hook.mjs index c8a8489..3211b5d 100644 --- a/hooks/unread-result-hook.mjs +++ b/hooks/unread-result-hook.mjs @@ -36,6 +36,7 @@ const MAX_LISTED_JOBS = 3; const SKIP_INTERACTIVE_HOOKS_ENV = "CLAUDE_COMPANION_SKIP_INTERACTIVE_HOOKS"; const ROOT_DIR = path.resolve(path.dirname(fileURLToPath(import.meta.url)), ".."); const PROMPT_NOTIFICATION_BUDGET_MS = 1_500; +const PROMPT_GIT_TIMEOUT_MS = 200; function isExplicitClaudeStatusRequest(prompt) { const text = String(prompt ?? "").toLowerCase(); @@ -89,12 +90,15 @@ function buildWorkflowContext(workflows) { ].join("\n"); } -function selectUnreadTerminalJobs(workspaceRoot, sessionId) { +function selectUnreadTerminalJobs(workspaceRoot, sessionId, deadlineAt) { if (!sessionId) { return []; } - return listJobs(workspaceRoot) + return listJobs(workspaceRoot, { + deadlineAt, + identityTimeoutMs: PROMPT_GIT_TIMEOUT_MS, + }) .filter((job) => job.sessionId === sessionId) .filter((job) => !job.workflowId) .filter((job) => TERMINAL_JOB_STATUSES.has(job.status)) @@ -117,13 +121,18 @@ function selectUnreadWorkflows(workspaceRoot, sessionId) { .filter(({ workflow, event }) => !(workflow.viewedEvents ?? []).includes(event)); } -function markJobsNotified(workspaceRoot, jobs) { +function markJobsNotified(workspaceRoot, jobs, deadlineAt) { const timestamp = nowIso(); for (const job of jobs) { try { - transitionJob(workspaceRoot, job.id, [job.status], job.status, { - notifiedAt: timestamp, - }); + transitionJob( + workspaceRoot, + job.id, + [job.status], + job.status, + { notifiedAt: timestamp }, + { deadlineAt, skipLockOwnerIdentity: process.platform === "win32" } + ); } catch { // Notification state is best-effort; still surface the terminal result. } @@ -167,12 +176,14 @@ function markWorkflowsNotified(workspaceRoot, workflows, deadlineAt) { return claimed; } -function captureTurnBaseline(workspaceRoot, sessionId, cwd) { +function captureTurnBaseline(workspaceRoot, sessionId, cwd, deadlineAt) { if (!sessionId) { return; } try { - const fingerprint = getWorkingTreeFingerprint(cwd); + const fingerprint = getWorkingTreeFingerprint(cwd, { + timeout: Math.max(1, Math.min(PROMPT_GIT_TIMEOUT_MS, deadlineAt - performance.now())), + }); writeTurnBaseline(workspaceRoot, sessionId, { cwd, workspaceRoot, @@ -200,10 +211,14 @@ async function main() { return; } const cwd = input.cwd || process.cwd(); - const workspaceRoot = resolveWorkspaceRoot(cwd); + const notificationDeadlineAt = performance.now() + PROMPT_NOTIFICATION_BUDGET_MS; + const workspaceRoot = resolveWorkspaceRoot(cwd, { + filesystemFallbackOnTimeout: true, + gitTimeout: Math.max(1, Math.min(PROMPT_GIT_TIMEOUT_MS, notificationDeadlineAt - performance.now())), + processLocalHandoff: true, + }); const sessionId = input.session_id || process.env[SESSION_ID_ENV] || null; const prompt = String(input.prompt ?? ""); - const notificationDeadlineAt = performance.now() + PROMPT_NOTIFICATION_BUDGET_MS; if ( process.env[SKIP_INTERACTIVE_HOOKS_ENV] === "1" || @@ -224,20 +239,20 @@ async function main() { // Best effort: an invalid session id must not fail a user prompt. } if (config.stopReviewGate) { - captureTurnBaseline(workspaceRoot, sessionId, cwd); + captureTurnBaseline(workspaceRoot, sessionId, cwd, notificationDeadlineAt); } if (isExplicitClaudeStatusRequest(prompt)) { return; } - const jobs = selectUnreadTerminalJobs(workspaceRoot, sessionId); + const jobs = selectUnreadTerminalJobs(workspaceRoot, sessionId, notificationDeadlineAt); const workflows = selectUnreadWorkflows(workspaceRoot, sessionId); if (jobs.length === 0 && workflows.length === 0) { return; } - markJobsNotified(workspaceRoot, jobs); + markJobsNotified(workspaceRoot, jobs, notificationDeadlineAt); const claimedWorkflows = markWorkflowsNotified( workspaceRoot, workflows, @@ -255,5 +270,4 @@ main().catch((error) => { process.stderr.write( `${error instanceof Error ? error.message : String(error)}\n` ); - process.exit(1); }); diff --git a/internal-skills/cli-runtime/runtime.md b/internal-skills/cli-runtime/runtime.md index 1eafd1f..b81cfc6 100644 --- a/internal-skills/cli-runtime/runtime.md +++ b/internal-skills/cli-runtime/runtime.md @@ -25,7 +25,7 @@ Command selection: 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. +- Leave `--model` and `--effort` unset unless the user explicitly asks for them, and preserve explicit values exactly. The companion command applies its own defaults; Fable has no implicit effort but supports explicit effort such as `medium`. - `--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`. @@ -52,9 +52,10 @@ Task defaults: - 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. - For any other non-zero exit code or shell-tool error, return the raw companion output or diagnostic without a success notification. -- If the parent supplied a non-empty parent thread id for background completion, allow at most one success-only `send_input` notification before finishing. -- Mention the tool name `send_input` literally when describing that notification path. -- Use the exact tool shape `send_input({ target: , message: })`. +- If the parent supplied a non-empty parent thread id for background completion, allow at most one success-only `send_message` notification before finishing. +- Mention the tool name `send_message` literally when describing that notification path. +- Use the exact tool shape `send_message({ target: , message: })`. +- Keep deferred view state so a failed or unavailable delivery remains available to the unread-result hook. - Use steering messages that point the parent at `$cc:result` or `$cc:status` instead of embedding the raw Claude result. - For background rescue, use that same steering message as the child's own final assistant message instead of echoing the raw companion result. - Do not poll status, fetch results, cancel jobs, or add commentary after the companion output. diff --git a/internal-skills/review-runtime/runtime.md b/internal-skills/review-runtime/runtime.md index 90f313f..75b405f 100644 --- a/internal-skills/review-runtime/runtime.md +++ b/internal-skills/review-runtime/runtime.md @@ -43,15 +43,15 @@ Background contract: - 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. - For any other non-zero exit code or shell-tool error, return the raw companion output or diagnostic without a success notification. - - use at most one `send_input` completion notification on success - - mention the tool name `send_input` literally in the child instructions - - use the exact tool shape `send_input({ target: , message: })` + - use at most one `send_message` completion notification on success + - mention the tool name `send_message` literally in the child instructions + - use the exact tool shape `send_message({ target: , message: })` + - keep deferred view state so failed delivery remains available to the unread-result hook - do not silently drop the completion notification path when the parent provided a non-empty parent thread id Spawn-agent defaults: -- `fork_context: false` +- `fork_turns: "none"` - `reasoning_effort: "medium"` -- Use the implicit default role and omit `agent_type` when it is optional or absent. If the runtime schema marks `agent_type` required, pass `agent_type: "default"`. - Omit `model` so the child inherits the current Codex runtime model. - Do not add a fixed-version model fallback. diff --git a/package-lock.json b/package-lock.json index 011fdbb..dd77d96 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "cc-plugin-codex", - "version": "1.7.5", + "version": "1.7.6", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "cc-plugin-codex", - "version": "1.7.5", + "version": "1.7.6", "license": "Apache-2.0", "bin": { "cc-plugin-codex": "scripts/installer-cli.mjs" diff --git a/package.json b/package.json index 7264bb2..591c2c6 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "cc-plugin-codex", - "version": "1.7.5", + "version": "1.7.6", "description": "Claude Code Plugin for Codex (CBEPX fork)", "type": "module", "author": { diff --git a/scripts/lib/git.mjs b/scripts/lib/git.mjs index 37cadf6..0c6e9e1 100644 --- a/scripts/lib/git.mjs +++ b/scripts/lib/git.mjs @@ -93,15 +93,15 @@ function hashText(value) { return createHash("sha256").update(String(value ?? ""), "utf8").digest("hex"); } -export function getWorkingTreeFingerprint(cwd) { +export function getWorkingTreeFingerprint(cwd, options = {}) { const fingerprintGitEnv = { ...process.env, GIT_OPTIONAL_LOCKS: "0" }; const smallGitOptions = { - timeout: FINGERPRINT_GIT_TIMEOUT_MS, + timeout: options.timeout ?? FINGERPRINT_GIT_TIMEOUT_MS, maxBuffer: FINGERPRINT_SMALL_MAX_BUFFER, env: fingerprintGitEnv, }; const pathListGitOptions = { - timeout: FINGERPRINT_GIT_TIMEOUT_MS, + timeout: options.timeout ?? FINGERPRINT_GIT_TIMEOUT_MS, maxBuffer: FINGERPRINT_PATH_LIST_MAX_BUFFER, env: fingerprintGitEnv, }; diff --git a/scripts/lib/mcp-capabilities.mjs b/scripts/lib/mcp-capabilities.mjs index c244634..3afe785 100644 --- a/scripts/lib/mcp-capabilities.mjs +++ b/scripts/lib/mcp-capabilities.mjs @@ -305,7 +305,7 @@ function stdioProbe(config, timeoutMs) { params: { protocolVersion: MCP_PROTOCOL_VERSION, capabilities: {}, - clientInfo: { name: "cc-plugin-codex", version: "1.7.5" }, + clientInfo: { name: "cc-plugin-codex", version: "1.7.6" }, }, }); }); @@ -394,7 +394,7 @@ async function httpProbe(config, timeoutMs) { params: { protocolVersion: MCP_PROTOCOL_VERSION, capabilities: {}, - clientInfo: { name: "cc-plugin-codex", version: "1.7.5" }, + clientInfo: { name: "cc-plugin-codex", version: "1.7.6" }, }, }, null, deadline); if (initialized.statusCode === 401 || initialized.statusCode === 403) { diff --git a/scripts/lib/state.mjs b/scripts/lib/state.mjs index 44b2e69..b90ff42 100644 --- a/scripts/lib/state.mjs +++ b/scripts/lib/state.mjs @@ -505,8 +505,8 @@ export function listStoredJobs(cwd) { return readAllJobs(cwd); } -export function listJobs(cwd) { - const jobs = reapStaleJobs(cwd, readAllJobs(cwd)); +export function listJobs(cwd, options = {}) { + const jobs = reapStaleJobs(cwd, readAllJobs(cwd), options); return partitionJobsForRetention(jobs).retained; } @@ -578,6 +578,9 @@ export function reapStaleJobs(cwd, jobs, options = {}) { const childExitDeadline = Date.now() + childExitWaitMs; return jobs.map((job) => { + if (Number.isFinite(options.deadlineAt) && performance.now() >= options.deadlineAt) { + return job; + } if (isWithinReapGracePeriod(job)) return job; if (!REAPABLE_STATUSES.has(job.status)) return job; const workerPid = Number.isInteger(job.workerPid) && job.workerPid > 0 @@ -676,9 +679,12 @@ export function reapStaleJobs(cwd, jobs, options = {}) { identityMatches = getProcessIdentityImpl( trackedPid, - platform === "win32" - ? { timeout: WINDOWS_REAPER_IDENTITY_TIMEOUT_MS } - : undefined + { timeout: Math.max(1, Math.min( + options.identityTimeoutMs ?? WINDOWS_REAPER_IDENTITY_TIMEOUT_MS, + Number.isFinite(options.deadlineAt) + ? options.deadlineAt - performance.now() + : Infinity + )) } ) === trackedPidIdentity; } catch { identityMatches = false; diff --git a/scripts/lib/workspace.mjs b/scripts/lib/workspace.mjs index 9099051..5b43fc5 100644 --- a/scripts/lib/workspace.mjs +++ b/scripts/lib/workspace.mjs @@ -12,6 +12,9 @@ const cachedWorkspaceRoots = new Map(); function cacheWorkspaceRoot(cacheKey, workspaceRoot) { cachedWorkspaceRoots.set(cacheKey, workspaceRoot); cachedWorkspaceRoots.set(path.resolve(workspaceRoot), workspaceRoot); + try { + cachedWorkspaceRoots.set(fs.realpathSync.native(workspaceRoot), workspaceRoot); + } catch {} } function findGitRoot(cwd) { diff --git a/skills/adversarial-review/SKILL.md b/skills/adversarial-review/SKILL.md index aac856f..ba6e193 100644 --- a/skills/adversarial-review/SKILL.md +++ b/skills/adversarial-review/SKILL.md @@ -16,7 +16,7 @@ Unlike `$cc:review`, this skill accepts custom focus text after the flags. The m Resolve `` as two directories above this `SKILL.md` file. Keep the shell tool in the active Codex user workspace; never set its working directory to `` or the directory used to read this skill. Parent and foreground commands use that shell's current directory directly: `node "/scripts/claude-companion.mjs" adversarial-review ...` -Supported arguments: `--wait`, `--background`, `--base `, `--scope auto|working-tree|branch`, `--model `, `--effort `, `--user-mcp-tool `, `--allow-project-mcp-servers`, plus optional focus text after the flags (defaults: model=opus, effort=xhigh; sonnet defaults to high; haiku and fable have no effort) +Supported arguments: `--wait`, `--background`, `--base `, `--scope auto|working-tree|branch`, `--model `, `--effort `, `--user-mcp-tool `, `--allow-project-mcp-servers`, plus optional focus text after the flags (defaults: model=opus, effort=xhigh; sonnet defaults to high; haiku and fable leave effort unset, while explicit Fable effort such as `medium` is supported) Raw slash-command arguments: `$ARGUMENTS` @@ -43,7 +43,7 @@ Execution mode rules: - 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. +- Use `request_user_input` only when this thread actually has one. If it is unavailable, proceed with the computed recommended mode; never stop solely because a picker is missing. Argument handling: - Preserve the user's arguments exactly. @@ -82,12 +82,11 @@ Background flow: - If that helper returns a non-empty `parentThreadId`, pass it into the child prompt as the parent thread id for one-shot completion notification. - If it returns an empty `parentThreadId`, omit the notification path instead of emitting a blank thread-id placeholder. - Spawn exactly one transient forwarding child through `spawn_agent` with: - - `fork_context: false` + - `fork_turns: "none"` - `reasoning_effort: "medium"` -- Use the built-in default role implicitly and omit `agent_type` when it is optional or absent. If the runtime schema marks `agent_type` required, pass `agent_type: "default"`. - Omit `model` so the forwarding child inherits the current Codex runtime model. - Prefer a self-contained child message over inheriting parent history. The built-in adversarial-review child should not rely on full parent thread replay for normal operation. -- Only consider `fork_context: true` as a last resort for a short follow-up where essential context truly cannot be summarized. Avoid it for large or long-lived threads because it can exhaust the child context window. +- Keep `fork_turns: "none"`; pass a self-contained forwarding prompt. - Before spawning the built-in child, emit one short commentary update that says the child will inherit the current Codex runtime model at `medium` effort. - Do not retry with an explicit model override if spawning fails; surface the failure. - The built-in child must be a pure forwarder. It should: @@ -107,16 +106,17 @@ Background flow: - return only that command's stdout exactly, with no added commentary - ignore stderr progress chatter such as `[cc] ...` lines and preserve only the final stdout-equivalent result text - not inspect the repo or perform the review itself - - if a parent thread id is available, allow one extra `send_input` call after a successful shell result and before finishing - - the child prompt must mention the tool name `send_input` literally; do not replace it with a vague instruction like "send a message to the parent" - - that `send_input` call must target the provided parent thread id, must happen at most once, and must not run on failure paths - - that `send_input` call should use the exact tool shape `send_input({ target: , message: })` with no extra prose payload + - if a parent thread id is available, allow one extra `send_message` call after a successful shell result and before finishing + - the child prompt must mention the tool name `send_message` literally + - that `send_message` call must target the provided parent thread id, must happen at most once, and must not run on failure paths + - that call should use the exact tool shape `send_message({ target: , message: })` + - keep `--view-state defer`; a failed or unavailable `send_message` must leave the result unread for the UserPromptSubmit fallback - if the parent provided a non-empty parent thread id, do not silently drop the completion notification path from the child prompt - if a reserved review job id is available, use this exact notification message: `Background Claude Code adversarial review finished. Open it with $cc:result .` - otherwise fall back to: `Background Claude Code adversarial review finished. Inspect it with $cc:status first, then use $cc:result for the finished job you want to open.` - - that `send_input` message should use one of those exact steering messages instead of inlining the raw review result + - that `send_message` should use one of those exact steering messages instead of inlining the raw review result - use these steering messages instead of embedding the raw review result in the notification - do not embed the raw Claude result inside the notification message - do not include any other prose in that notification message diff --git a/skills/rescue/SKILL.md b/skills/rescue/SKILL.md index 2e0ece7..6b3757b 100644 --- a/skills/rescue/SKILL.md +++ b/skills/rescue/SKILL.md @@ -58,11 +58,10 @@ Main-thread routing rules: - If a legacy request still includes `--notify-parent-on-complete`, treat it as a compatibility alias. Background built-in rescue now attempts parent wake-up by default. Subagent launch: -- By default, use Codex's `spawn_agent` tool with its implicit default role and omit `agent_type` when it is optional or absent. If the runtime schema marks `agent_type` required, pass `agent_type: "default"`. +- By default, use Codex's `spawn_agent` tool. - Never satisfy background rescue by launching `claude-companion.mjs task` itself as a detached shell process. Do not use `&`, `nohup`, detached `spawn`, or any equivalent direct background process launch from the parent. - If a legacy request still includes `--builtin-agent`, treat it as a compatibility alias for the default built-in path. It should not change behavior. -- Prefer `fork_context: false` for the built-in rescue child. The parent should pass a self-contained forwarding message instead of replaying the full parent thread by default. -- Only consider `fork_context: true` as a last resort for a short follow-up where essential context truly cannot be summarized. Avoid it for large or long-lived threads because it can exhaust the child context window. +- Use `fork_turns: "none"` for the built-in rescue child and pass a self-contained forwarding message. - The built-in rescue path must omit `model` and set `reasoning_effort: "medium"` on `spawn_agent`, so the transient forwarding child inherits the current Codex runtime model. - Before spawning the built-in child, emit one short commentary update that says the child will inherit the current Codex runtime model at `medium` effort. - Do not retry with an explicit model override if spawning fails; surface the failure. @@ -129,12 +128,13 @@ Subagent launch: - For any other non-zero exit code or shell-tool error, return the raw companion output or diagnostic without a success notification. - for foreground rescue only, tell the child to return that command's stdout text exactly, with no preamble, summary, code fence, trimming, normalization, or punctuation changes - tell the child to ignore stderr progress chatter such as `[cc] ...` lines and preserve only the stdout-equivalent final result text - - if a parent thread id is provided for experimental background notification, allow one extra `send_input` call after a successful shell result and before finishing - - the child prompt must mention the tool name `send_input` literally; do not replace it with a vague instruction like "send a message to the parent" - - that `send_input` call must target the provided parent thread id, must happen at most once, and must not run on failure paths - - that `send_input` call should use the exact tool shape `send_input({ target: , message: })` with no extra prose payload + - if a parent thread id is provided for background notification, allow one extra `send_message` call after a successful shell result and before finishing + - the child prompt must mention the tool name `send_message` literally + - that `send_message` call must target the provided parent thread id, must happen at most once, and must not run on failure paths + - that call should use the exact tool shape `send_message({ target: , message: })` + - keep `--view-state defer`; a failed or unavailable `send_message` must leave the result unread for the UserPromptSubmit fallback - if the parent provided a non-empty parent thread id, do not silently drop the completion notification path from the child prompt - - that `send_input` message should use a short user-facing template that steers the parent toward explicit result retrieval instead of inlining the raw result + - that `send_message` should use a short user-facing template that steers the parent toward explicit result retrieval instead of inlining the raw result - if a reserved companion job id is available, use this exact high-level shape for the notification message: `Background Claude Code rescue finished. Open it with $cc:result .` - if no reserved job id is available, fall back to: diff --git a/skills/review/SKILL.md b/skills/review/SKILL.md index 4fb9efa..219c25a 100644 --- a/skills/review/SKILL.md +++ b/skills/review/SKILL.md @@ -16,7 +16,7 @@ If the overall request is "you review it too, also ask Claude to review in the b Resolve `` as two directories above this `SKILL.md` file. Keep the shell tool in the active Codex user workspace; never set its working directory to `` or the directory used to read this skill. Parent and foreground commands use that shell's current directory directly: `node "/scripts/claude-companion.mjs" review ...` -Supported arguments: `--wait`, `--background`, `--base `, `--scope auto|working-tree|branch`, `--model `, `--effort `, `--user-mcp-tool `, `--allow-project-mcp-servers` (defaults: model=opus, effort=xhigh; sonnet defaults to high; haiku and fable have no effort) +Supported arguments: `--wait`, `--background`, `--base `, `--scope auto|working-tree|branch`, `--model `, `--effort `, `--user-mcp-tool `, `--allow-project-mcp-servers` (defaults: model=opus, effort=xhigh; sonnet defaults to high; haiku and fable leave effort unset, while explicit Fable effort such as `medium` is supported) Raw slash-command arguments: `$ARGUMENTS` @@ -44,7 +44,7 @@ Execution mode rules: - 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. +- Use `request_user_input` only when this thread actually has one. If it is unavailable, proceed with the computed recommended mode; never stop solely because a picker is missing. Argument handling: - Preserve the user's arguments exactly. @@ -80,12 +80,11 @@ Background flow: - If that helper returns a non-empty `parentThreadId`, pass it into the child prompt as the parent thread id for one-shot completion notification. - If it returns an empty `parentThreadId`, omit the notification path instead of emitting a blank thread-id placeholder. - Spawn exactly one transient forwarding child through `spawn_agent` with: - - `fork_context: false` + - `fork_turns: "none"` - `reasoning_effort: "medium"` -- Use the built-in default role implicitly and omit `agent_type` when it is optional or absent. If the runtime schema marks `agent_type` required, pass `agent_type: "default"`. - Omit `model` so the forwarding child inherits the current Codex runtime model. - Prefer a self-contained child message over inheriting parent history. The built-in review child should not rely on full parent thread replay for normal operation. -- Only consider `fork_context: true` as a last resort for a short follow-up where essential context truly cannot be summarized. Avoid it for large or long-lived threads because it can exhaust the child context window. +- Keep `fork_turns: "none"`; pass a self-contained forwarding prompt. - Before spawning the built-in child, emit one short commentary update that says the child will inherit the current Codex runtime model at `medium` effort. - Do not retry with an explicit model override if spawning fails; surface the failure. - The built-in child must be a pure forwarder. It should: @@ -105,16 +104,17 @@ Background flow: - return only that command's stdout exactly, with no added commentary - ignore stderr progress chatter such as `[cc] ...` lines and preserve only the final stdout-equivalent result text - not inspect the repo or perform the review itself - - if a parent thread id is available, allow one extra `send_input` call after a successful shell result and before finishing - - the child prompt must mention the tool name `send_input` literally; do not replace it with a vague instruction like "send a message to the parent" - - that `send_input` call must target the provided parent thread id, must happen at most once, and must not run on failure paths - - that `send_input` call should use the exact tool shape `send_input({ target: , message: })` with no extra prose payload + - if a parent thread id is available, allow one extra `send_message` call after a successful shell result and before finishing + - the child prompt must mention the tool name `send_message` literally + - that `send_message` call must target the provided parent thread id, must happen at most once, and must not run on failure paths + - that call should use the exact tool shape `send_message({ target: , message: })` + - keep `--view-state defer`; a failed or unavailable `send_message` must leave the result unread for the UserPromptSubmit fallback - if the parent provided a non-empty parent thread id, do not silently drop the completion notification path from the child prompt - if a reserved review job id is available, use this exact notification message: `Background Claude Code review finished. Open it with $cc:result .` - otherwise fall back to: `Background Claude Code review finished. Inspect it with $cc:status first, then use $cc:result for the finished job you want to open.` - - that `send_input` message should use one of those exact steering messages instead of inlining the raw review result + - that `send_message` should use one of those exact steering messages instead of inlining the raw review result - use these steering messages instead of embedding the raw review result in the notification - do not embed the raw Claude result inside the notification message - do not include any other prose in that notification message diff --git a/stryker.shard.config.mjs b/stryker.shard.config.mjs index eafbedd..77badcd 100644 --- a/stryker.shard.config.mjs +++ b/stryker.shard.config.mjs @@ -24,10 +24,10 @@ const shards = { "scripts/lib/state.mjs:188-228", "scripts/lib/state.mjs:297-388", "scripts/lib/state.mjs:420-470", - "scripts/lib/state.mjs:547-888", - "scripts/lib/state.mjs:937-1109", - "scripts/lib/state.mjs:1175-1240", - "scripts/lib/state.mjs:1246-1292", + "scripts/lib/state.mjs:547-894", + "scripts/lib/state.mjs:943-1115", + "scripts/lib/state.mjs:1181-1246", + "scripts/lib/state.mjs:1252-1298", ], }, "tracked-jobs": { diff --git a/tests/e2e/codex-skills-e2e.test.mjs b/tests/e2e/codex-skills-e2e.test.mjs index b9430fc..3210933 100644 --- a/tests/e2e/codex-skills-e2e.test.mjs +++ b/tests/e2e/codex-skills-e2e.test.mjs @@ -906,7 +906,7 @@ function startMockProvider({ assert.ok(!bodyText.includes("gpt-5.4"), "parent turn should not pin a stale Codex model"); const spawnArgs = { - fork_context: false, + fork_turns: "none", reasoning_effort: "medium", message: spawnMessage ?? @@ -1870,7 +1870,7 @@ describe("Codex direct-skill E2E", () => { "Exit code 0 is the only successful completion.", "Exit code 124 means the job is still running", "For any other non-zero exit code or shell-tool error", - "allow one extra `send_input` call after a successful shell result", + "allow one extra `send_message` call after a successful shell result", "must target the provided parent thread id", "do not silently drop the completion notification path from the child prompt", "Background Claude Code review finished. Open it with $cc:result .", @@ -1885,7 +1885,7 @@ describe("Codex direct-skill E2E", () => { reservedJobId, "--owner-session-id", ownerSessionId, - "send_input", + "send_message", notificationMessage, ], notificationMessage, @@ -2088,7 +2088,7 @@ describe("Codex direct-skill E2E", () => { "Exit code 0 is the only successful completion.", "Exit code 124 means the job is still running", "For any other non-zero exit code or shell-tool error", - "allow one extra `send_input` call after a successful shell result", + "allow one extra `send_message` call after a successful shell result", "must target the provided parent thread id", "do not silently drop the completion notification path from the child prompt", "Background Claude Code adversarial review finished. Open it with $cc:result .", @@ -2103,7 +2103,7 @@ describe("Codex direct-skill E2E", () => { reservedJobId, "--owner-session-id", ownerSessionId, - "send_input", + "send_message", notificationMessage, "focus on race conditions", ], diff --git a/tests/hooks.test.mjs b/tests/hooks.test.mjs index 6b9fe39..9ebf3cd 100644 --- a/tests/hooks.test.mjs +++ b/tests/hooks.test.mjs @@ -1274,7 +1274,8 @@ fs.readdirSync = (directory, ...args) => { } } - assert.equal("timeout" in sessionStartHandler, false); + assert.equal(sessionStartHandler.timeout, 5); + assert.equal(manifest.hooks.UserPromptSubmit[0].hooks[0].timeout, 5); assert.equal(handler.timeout, 3); assert.match(handler.command, /session-lifecycle SessionEnd/u); }); @@ -2596,14 +2597,14 @@ if (args.at(-1) === process.env.CC_TEST_LOCK_OWNER_PID) { encoding: "utf8", }); - assert.notEqual(result.status, 0); + assert.equal(result.status, 0); assert.match(result.stderr, /Hook input exceeds/i); } finally { cleanupHookEnvironment(testEnv); } }); - it("hook input parser accepts empty input and rejects malformed JSON", () => { + it("hook input parser accepts empty input and fails open on malformed JSON", () => { const testEnv = createHookEnvironment(); try { const empty = spawnSync(process.execPath, [UNREAD_HOOK], { @@ -2621,7 +2622,7 @@ if (args.at(-1) === process.env.CC_TEST_LOCK_OWNER_PID) { input: "{invalid\n", encoding: "utf8", }); - assert.notEqual(malformed.status, 0); + assert.equal(malformed.status, 0); assert.match(malformed.stderr, /Invalid hook input JSON/i); } finally { cleanupHookEnvironment(testEnv); diff --git a/tests/mutation-config.test.mjs b/tests/mutation-config.test.mjs index a9df10d..3bde857 100644 --- a/tests/mutation-config.test.mjs +++ b/tests/mutation-config.test.mjs @@ -52,10 +52,10 @@ const expectations = [ ["scripts/lib/state.mjs:188-228", ["ensurePluginDataLayout", "resolveWorkspaceHash", "ensureStateDir"]], ["scripts/lib/state.mjs:297-388", ["setCurrentSession", "getCurrentSession", "clearCurrentSession", "markSessionCleanupPending", "listPendingSessionCleanups", "clearSessionCleanupPending"]], ["scripts/lib/state.mjs:420-470", ["writeJobFile", "normalizeStoredJob"]], - ["scripts/lib/state.mjs:547-888", ["mostRecentJobTimestamp", "isWithinReapGracePeriod", "reapStaleJobs"]], - ["scripts/lib/state.mjs:937-1109", ["unlinkLockIfUnchanged", "remainingLockDeadlineMs", "lockProcessTimeout", "recoverStaleLock", "acquireJobLock", "releaseJobLock"]], - ["scripts/lib/state.mjs:1175-1240", ["casJobStatus", "transitionJob", "writeAtomic", "withStateFileLock"]], - ["scripts/lib/state.mjs:1246-1292", ["cleanupOldJobs"]], + ["scripts/lib/state.mjs:547-894", ["mostRecentJobTimestamp", "isWithinReapGracePeriod", "reapStaleJobs"]], + ["scripts/lib/state.mjs:943-1115", ["unlinkLockIfUnchanged", "remainingLockDeadlineMs", "lockProcessTimeout", "recoverStaleLock", "acquireJobLock", "releaseJobLock"]], + ["scripts/lib/state.mjs:1181-1246", ["casJobStatus", "transitionJob", "writeAtomic", "withStateFileLock"]], + ["scripts/lib/state.mjs:1252-1298", ["cleanupOldJobs"]], ["scripts/lib/tracked-jobs.mjs:30-78", ["transitionTrackedJob", "isStatusReaperFailure", "transitionTrackedJobTerminal"]], ["scripts/lib/tracked-jobs.mjs:308-392", ["createJobRecord", "createJobProgressUpdater"]], ["scripts/lib/tracked-jobs.mjs:411-544", ["runTrackedJob"]], diff --git a/tests/skills-contracts.test.mjs b/tests/skills-contracts.test.mjs index 7135a77..24633c1 100644 --- a/tests/skills-contracts.test.mjs +++ b/tests/skills-contracts.test.mjs @@ -131,7 +131,7 @@ test("review skills preserve foreground/background routing contracts", () => { "--owner-session-id ", "--job-id ", "spawn_agent", - "`fork_context: false`", + '`fork_turns: "none"`', '`reasoning_effort: "medium"`', "Omit `model` so the forwarding child inherits the current Codex runtime model.", "If the shell tool returns a session id, keep polling that same session until the companion command exits.", @@ -139,7 +139,8 @@ test("review skills preserve foreground/background routing contracts", () => { "Exit code 124 means the job is still running; return the companion output without claiming it finished.", "For any other non-zero exit code or shell-tool error, return the raw companion output or diagnostic without a success notification.", "never leave an empty routing placeholder such as `--owner-session-id --job-id`", - "send_input({ target: , message: })", + "send_message({ target: , message: })", + "must leave the result unread", notification, ], name @@ -163,7 +164,7 @@ test("review and rescue skills retain negative routing guards", () => { "Do not fall back to raw `claude`", "generic `claude_review_runner`-style helper role", "shell backgrounding such as `&`, `nohup`, detached `spawn`", - "Only consider `fork_context: true` as a last resort", + 'Keep `fork_turns: "none"`', "Do not retry with an explicit model override if spawning fails", ], name @@ -194,8 +195,7 @@ test("review and rescue skills retain negative routing guards", () => { "This size-and-scope heuristic belongs to the main Codex thread", "If the user task text itself begins with a slash command", "Never satisfy background rescue by launching", - "Prefer `fork_context: false`", - "Only consider `fork_context: true` as a last resort", + 'Use `fork_turns: "none"`', "Do not retry with an explicit model override if spawning fails", ], "rescue routing" @@ -219,7 +219,7 @@ test("rescue keeps host execution controls out of the companion task", () => { "--owner-session-id ", "--job-id ", "--prompt-file", - "send_input({ target: , message: })", + "send_message({ target: , message: })", "Background Claude Code rescue finished. Open it with $cc:result .", "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.", @@ -267,7 +267,7 @@ test("internal runtime references preserve executable routing invariants", () => "For any other non-zero exit code or shell-tool error, return the raw companion output or diagnostic without a success notification.", "Omit `model` so the child inherits the current Codex runtime model.", "Do not add a fixed-version model fallback.", - "send_input({ target: , message: })", + "send_message({ target: , message: })", "Background Claude Code review finished. Open it with $cc:result .", "Background Claude Code adversarial review finished. Open it with $cc:result .", ], @@ -290,7 +290,7 @@ test("internal runtime references preserve executable routing invariants", () => "--job-id", "--prompt-file", "Never call `task-resume-candidate` from the rescue forwarder.", - "send_input({ target: , message: })", + "send_message({ target: , message: })", ], "rescue runtime" ); @@ -322,7 +322,7 @@ test("review skills use only an available Codex question tool", () => { 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, /If it is unavailable, proceed/i); assert.match(contract, /recommended mode/i); } }); diff --git a/tests/unread-result-hook.test.mjs b/tests/unread-result-hook.test.mjs index 75f192d..27703b0 100644 --- a/tests/unread-result-hook.test.mjs +++ b/tests/unread-result-hook.test.mjs @@ -678,6 +678,68 @@ test("records a turn baseline for the current session on UserPromptSubmit", () = } }); +test("bounds UserPromptSubmit Git discovery", () => { + const testEnv = createEnv(); + try { + const stateDir = stateDirFor(testEnv); + fs.mkdirSync(stateDir, { recursive: true }); + fs.writeFileSync( + path.join(stateDir, "config.json"), + JSON.stringify({ version: 1, stopReviewGate: true }) + "\n", + "utf8" + ); + const bin = path.join(testEnv.rootDir, "slow-tools"); + fs.mkdirSync(bin); + const executable = path.join(bin, "git"); + fs.writeFileSync(executable, "#!/bin/sh\nexec /dev/null 2>&1\nexec /bin/sleep 30\n", "utf8"); + fs.chmodSync(executable, 0o755); + + const startedAt = performance.now(); + const output = runHook(testEnv, { + hook_event_name: "UserPromptSubmit", + cwd: testEnv.workspaceDir, + session_id: "session-a", + prompt: "continue", + }, { + PATH: `${bin}${path.delimiter}${process.env.PATH ?? ""}`, + }, { timeout: 2_500 }); + + assert.ok(performance.now() - startedAt < 2_200); + assert.equal(output, ""); + } finally { + cleanupEnv(testEnv); + } +}); + +test("bounds UserPromptSubmit stale-job identity checks", (t) => { + if (process.platform !== "darwin") return t.skip("Darwin ps fixture"); + const testEnv = createEnv(); + try { + initGitRepo(testEnv.workspaceDir); + writeJob(testEnv, { + id: "stale-running", sessionId: "session-a", status: "running", + workerPid: process.pid, workerPidIdentity: "stale-identity", + createdAt: "2026-01-01T00:00:00Z", updatedAt: "2026-01-01T00:00:00Z", + }); + const bin = path.join(testEnv.rootDir, "slow-ps"); + fs.mkdirSync(bin); + const executable = path.join(bin, "ps"); + fs.writeFileSync(executable, "#!/usr/bin/env node\nsetInterval(() => {}, 1000);\n", "utf8"); + fs.chmodSync(executable, 0o755); + + const startedAt = performance.now(); + const output = runHook(testEnv, { + hook_event_name: "UserPromptSubmit", cwd: testEnv.workspaceDir, + session_id: "session-a", prompt: "continue", + }, { PATH: `${bin}${path.delimiter}${process.env.PATH ?? ""}` }, { timeout: 2_500 }); + assert.ok(performance.now() - startedAt < 2_200); + assert.equal(output, ""); + assert.equal(readJob(testEnv, "stale-running").notifiedAt, undefined); + } finally { + cleanupEnv(testEnv); + } +}); + test("does not replace an existing parent marker from an unmarked child prompt", () => { const testEnv = createEnv(); try { From f11f586e445b38c24e8c6fe35ff0de2dfbd73f09 Mon Sep 17 00:00:00 2001 From: CBEPX <458940+CBEPX@users.noreply.github.com> Date: Mon, 21 Sep 2026 16:24:46 +0300 Subject: [PATCH 2/3] fix(hooks): keep stale reaping within deadline --- scripts/lib/state.mjs | 66 ++++++++++++++++++++++++------- stryker.shard.config.mjs | 8 ++-- tests/mutation-config.test.mjs | 8 ++-- tests/state.test.mjs | 33 ++++++++++++++++ tests/unread-result-hook.test.mjs | 30 ++++++++++++++ 5 files changed, 122 insertions(+), 23 deletions(-) diff --git a/scripts/lib/state.mjs b/scripts/lib/state.mjs index b90ff42..587c4d4 100644 --- a/scripts/lib/state.mjs +++ b/scripts/lib/state.mjs @@ -575,7 +575,19 @@ export function reapStaleJobs(cwd, jobs, options = {}) { const terminateProcessTreeIfIdentityMatchesImpl = options.terminateProcessTreeIfIdentityMatchesImpl ?? terminateProcessTreeIfIdentityMatches; - const childExitDeadline = Date.now() + childExitWaitMs; + const reaperDeadlineMs = Number.isFinite(options.deadlineAt) + ? Math.max(0, Math.floor(options.deadlineAt - performance.now())) + : null; + const childExitDeadline = Date.now() + Math.min( + childExitWaitMs, + reaperDeadlineMs ?? childExitWaitMs + ); + const transitionOptions = { + ...(Number.isFinite(options.deadlineAt) + ? { deadlineAt: options.deadlineAt } + : {}), + ...(options.skipLockOwnerIdentity ? { skipLockOwnerIdentity: true } : {}), + }; return jobs.map((job) => { if (Number.isFinite(options.deadlineAt) && performance.now() >= options.deadlineAt) { @@ -610,17 +622,24 @@ export function reapStaleJobs(cwd, jobs, options = {}) { return current; } try { - transitionJob(cwd, job.id, ["queued"], "failed", { - errorMessage: "Worker did not start before the startup grace period elapsed. Auto-reaped.", - completedAt: nowIso(), - reapedBy: "status-reaper", - reapReason: "startup-timeout", - pid: null, - pidIdentity: null, - workerPid: null, - workerPidIdentity: null, - phase: "failed", - }); + transitionJob( + cwd, + job.id, + ["queued"], + "failed", + { + errorMessage: "Worker did not start before the startup grace period elapsed. Auto-reaped.", + completedAt: nowIso(), + reapedBy: "status-reaper", + reapReason: "startup-timeout", + pid: null, + pidIdentity: null, + workerPid: null, + workerPidIdentity: null, + phase: "failed", + }, + transitionOptions + ); return readJobFile(cwd, job.id) ?? job; } catch { return job; @@ -786,6 +805,22 @@ export function reapStaleJobs(cwd, jobs, options = {}) { let childCleanup = null; if (hasDistinctClaudeChild) { try { + const remainingDeadlineMs = Number.isFinite(options.deadlineAt) + ? Math.floor(options.deadlineAt - performance.now()) + : null; + if (remainingDeadlineMs !== null && remainingDeadlineMs <= 0) { + throw Object.assign(new Error("Stale-job reaper deadline expired"), { + code: "ELOCKTIMEOUT", + }); + } + const childIdentityTimeout = remainingDeadlineMs === null + ? (platform === "win32" ? WINDOWS_REAPER_IDENTITY_TIMEOUT_MS : null) + : Math.min( + remainingDeadlineMs, + platform === "win32" + ? WINDOWS_REAPER_IDENTITY_TIMEOUT_MS + : remainingDeadlineMs + ); childCleanup = terminateProcessTreeIfIdentityMatchesImpl( job.pid, job.pidIdentity, @@ -793,8 +828,8 @@ export function reapStaleJobs(cwd, jobs, options = {}) { platform, getProcessIdentityImpl, isProcessAliveImpl, - ...(platform === "win32" - ? { timeout: WINDOWS_REAPER_IDENTITY_TIMEOUT_MS } + ...(childIdentityTimeout !== null + ? { timeout: Math.max(1, childIdentityTimeout) } : {}), } ); @@ -880,7 +915,8 @@ export function reapStaleJobs(cwd, jobs, options = {}) { job.id, [job.status], nextStatus, - terminalData + terminalData, + transitionOptions ); if (transitioned.transitioned) { return readJobFile(cwd, job.id) ?? job; diff --git a/stryker.shard.config.mjs b/stryker.shard.config.mjs index 77badcd..9285460 100644 --- a/stryker.shard.config.mjs +++ b/stryker.shard.config.mjs @@ -24,10 +24,10 @@ const shards = { "scripts/lib/state.mjs:188-228", "scripts/lib/state.mjs:297-388", "scripts/lib/state.mjs:420-470", - "scripts/lib/state.mjs:547-894", - "scripts/lib/state.mjs:943-1115", - "scripts/lib/state.mjs:1181-1246", - "scripts/lib/state.mjs:1252-1298", + "scripts/lib/state.mjs:547-930", + "scripts/lib/state.mjs:979-1151", + "scripts/lib/state.mjs:1217-1282", + "scripts/lib/state.mjs:1288-1334", ], }, "tracked-jobs": { diff --git a/tests/mutation-config.test.mjs b/tests/mutation-config.test.mjs index 3bde857..943d1e2 100644 --- a/tests/mutation-config.test.mjs +++ b/tests/mutation-config.test.mjs @@ -52,10 +52,10 @@ const expectations = [ ["scripts/lib/state.mjs:188-228", ["ensurePluginDataLayout", "resolveWorkspaceHash", "ensureStateDir"]], ["scripts/lib/state.mjs:297-388", ["setCurrentSession", "getCurrentSession", "clearCurrentSession", "markSessionCleanupPending", "listPendingSessionCleanups", "clearSessionCleanupPending"]], ["scripts/lib/state.mjs:420-470", ["writeJobFile", "normalizeStoredJob"]], - ["scripts/lib/state.mjs:547-894", ["mostRecentJobTimestamp", "isWithinReapGracePeriod", "reapStaleJobs"]], - ["scripts/lib/state.mjs:943-1115", ["unlinkLockIfUnchanged", "remainingLockDeadlineMs", "lockProcessTimeout", "recoverStaleLock", "acquireJobLock", "releaseJobLock"]], - ["scripts/lib/state.mjs:1181-1246", ["casJobStatus", "transitionJob", "writeAtomic", "withStateFileLock"]], - ["scripts/lib/state.mjs:1252-1298", ["cleanupOldJobs"]], + ["scripts/lib/state.mjs:547-930", ["mostRecentJobTimestamp", "isWithinReapGracePeriod", "reapStaleJobs"]], + ["scripts/lib/state.mjs:979-1151", ["unlinkLockIfUnchanged", "remainingLockDeadlineMs", "lockProcessTimeout", "recoverStaleLock", "acquireJobLock", "releaseJobLock"]], + ["scripts/lib/state.mjs:1217-1282", ["casJobStatus", "transitionJob", "writeAtomic", "withStateFileLock"]], + ["scripts/lib/state.mjs:1288-1334", ["cleanupOldJobs"]], ["scripts/lib/tracked-jobs.mjs:30-78", ["transitionTrackedJob", "isStatusReaperFailure", "transitionTrackedJobTerminal"]], ["scripts/lib/tracked-jobs.mjs:308-392", ["createJobRecord", "createJobProgressUpdater"]], ["scripts/lib/tracked-jobs.mjs:411-544", ["runTrackedJob"]], diff --git a/tests/state.test.mjs b/tests/state.test.mjs index 9ea6761..e4a8222 100644 --- a/tests/state.test.mjs +++ b/tests/state.test.mjs @@ -1859,6 +1859,39 @@ describe("reapStaleJobs", () => { assert.equal(cleanupOptions?.timeout, 2_000); }); + it("bounds Claude child cleanup to the remaining reaper deadline", () => { + const id = "test-reap-dead-worker-child-deadline"; + const claudePid = 11126; + const workerPid = 22237; + writeJobFile(PROJECT_CWD, id, { + id, + status: "running", + pid: claudePid, + pidIdentity: "claude-identity", + workerPid, + workerPidIdentity: "worker-identity", + createdAt: nowIso(), + }); + backdateJob(id, staleTimestamp()); + /** @type {{ timeout?: number } | null} */ + let cleanupOptions = null; + + reapStaleJobs(PROJECT_CWD, [readJobFile(PROJECT_CWD, id)], { + platform: "darwin", + deadlineAt: performance.now() + 500, + isProcessAliveImpl: (pid) => pid === claudePid, + getProcessIdentityImpl: () => "worker-identity", + terminateProcessTreeIfIdentityMatchesImpl: (_pid, _identity, options) => { + cleanupOptions = options; + return { attempted: true, delivered: true }; + }, + childExitWaitMs: 0, + }); + + assert.ok(Number.isFinite(cleanupOptions?.timeout)); + assert.ok(cleanupOptions.timeout > 0 && cleanupOptions.timeout <= 500); + }); + it("clears a recycled Claude child PID when its owning worker dies", () => { const id = "test-reap-dead-worker-recycled-child"; writeJobFile(PROJECT_CWD, id, { diff --git a/tests/unread-result-hook.test.mjs b/tests/unread-result-hook.test.mjs index 27703b0..7f86096 100644 --- a/tests/unread-result-hook.test.mjs +++ b/tests/unread-result-hook.test.mjs @@ -740,6 +740,36 @@ test("bounds UserPromptSubmit stale-job identity checks", (t) => { } }); +test("bounds expired queued-job transitions by the prompt deadline", (t) => { + if (process.platform !== "darwin") return t.skip("Darwin ps fixture"); + const testEnv = createEnv(); + try { + initGitRepo(testEnv.workspaceDir); + writeJob(testEnv, { + id: "stale-queued-no-pid", sessionId: "session-a", status: "queued", + pid: null, workerPid: null, + createdAt: "2026-01-01T00:00:00Z", updatedAt: "2026-01-01T00:00:00Z", + }); + const bin = path.join(testEnv.rootDir, "slow-transition-ps"); + fs.mkdirSync(bin); + const executable = path.join(bin, "ps"); + fs.writeFileSync(executable, "#!/usr/bin/env node\nsetInterval(() => {}, 1000);\n", "utf8"); + fs.chmodSync(executable, 0o755); + + const startedAt = performance.now(); + const output = runHook(testEnv, { + hook_event_name: "UserPromptSubmit", cwd: testEnv.workspaceDir, + session_id: "session-a", prompt: "continue", + }, { PATH: `${bin}${path.delimiter}${process.env.PATH ?? ""}` }, { timeout: 2_500 }); + + assert.ok(performance.now() - startedAt < 2_200); + assert.equal(output, ""); + assert.equal(readJob(testEnv, "stale-queued-no-pid").status, "queued"); + } finally { + cleanupEnv(testEnv); + } +}); + test("does not replace an existing parent marker from an unmarked child prompt", () => { const testEnv = createEnv(); try { From 517196484442f11c4cf66a87ca097f4c170eab02 Mon Sep 17 00:00:00 2001 From: CBEPX <458940+CBEPX@users.noreply.github.com> Date: Thu, 24 Sep 2026 14:14:48 +0300 Subject: [PATCH 3/3] test(mcp): stabilize stubborn server startup for 1.7.6 --- tests/mcp-capabilities.test.mjs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/mcp-capabilities.test.mjs b/tests/mcp-capabilities.test.mjs index 84e205c..ddc7de9 100644 --- a/tests/mcp-capabilities.test.mjs +++ b/tests/mcp-capabilities.test.mjs @@ -869,8 +869,8 @@ describe("MCP capability discovery", () => { const serverPath = path.join(homeDir, "stubborn-server.mjs"); fs.writeFileSync(serverPath, [ 'import fs from "node:fs";', - "fs.writeFileSync(process.env.FAKE_MCP_PID_FILE, String(process.pid));", "process.on('SIGTERM', () => {});", + "fs.writeFileSync(process.env.FAKE_MCP_PID_FILE, String(process.pid));", "setInterval(() => {}, 1000);", ].join("\n"), "utf8"); fs.writeFileSync(path.join(homeDir, ".claude.json"), JSON.stringify({ @@ -885,7 +885,7 @@ describe("MCP capability discovery", () => { const result = await mcp.probeMcpCapabilities( mcp.collectConfiguredMcpServers(cwd, { homeDir }), - { timeoutMs: 80 } + { timeoutMs: 1000 } ); assert.equal(result.diagnostics[0].code, "probe_timeout"); const pid = Number(fs.readFileSync(pidFile, "utf8"));