diff --git a/.codex-plugin/plugin.json b/.codex-plugin/plugin.json index c6fb084..a6b27ef 100644 --- a/.codex-plugin/plugin.json +++ b/.codex-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "cc", - "version": "1.6.0", + "version": "1.6.1", "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 c052fdf..a510a44 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,23 @@ ## [Unreleased] +## v1.6.1 + +### Added + +- Surface bounded Claude stream parse diagnostics while accepting a valid terminal result for read-only tasks and reviews; workspace-write tasks remain fail-closed on malformed output. +- Resolve explicit full job IDs across workspace and linked-worktree state roots without broadening prefix or no-argument lookup beyond the current workspace. + +### Changed + +- Route native hooks through a minimal stable launcher in Codex's marketplace-specific `$PLUGIN_DATA` namespace, recover only from validated sibling `cc` cache versions, and require the current Codex 0.151 `hooks` feature without rewriting the removed `plugin_hooks` flag. +- Prefer the live `CODEX_THREAD_ID` over stale workspace markers for job ownership and resume routing while blocking direct Claude-driven recursion even when no marker exists. + +### Fixed + +- Re-capture worker identity inside the worker, tag every status-reaper terminal transition, and allow a late real runner result to replace only reaper-owned failures. +- Cancel foreground Claude reviews on `SIGINT` or `SIGTERM` while preserving sandbox, MCP-config, and review-worktree cleanup; retain accurate text output when a cancel race finds an already-terminal job. + ## v1.6.0 ### Added diff --git a/README.md b/README.md index 7e35c45..15da347 100644 --- a/README.md +++ b/README.md @@ -46,21 +46,21 @@ 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.6.0 +codex plugin marketplace add CBEPX/cc-plugin-codex --ref v1.6.1 codex plugin add cc@cbepx ``` Then run `$cc:setup` once inside Codex. -`cc-plugin-codex` uses Codex native plugin hooks. The active plugin copy lives under Codex's plugin cache, and hook commands resolve through `$PLUGIN_ROOT`; there is no separate local checkout install. +`cc-plugin-codex` uses Codex native plugin hooks. Hook commands enter through a small launcher under `$PLUGIN_DATA`; it resolves the current valid version from Codex's plugin cache, so an already-running session does not keep executing a deleted cache path. The optional `npx` helper can install this fork release and enable the required Codex feature gates: ```bash CC_PLUGIN_CODEX_MARKETPLACE_NAME=cbepx \ CC_PLUGIN_CODEX_MARKETPLACE_SOURCE=CBEPX/cc-plugin-codex \ -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 +CC_PLUGIN_CODEX_MARKETPLACE_REF=v1.6.1 \ +npx -y https://github.com/CBEPX/cc-plugin-codex/releases/download/v1.6.1/cc-plugin-codex-1.6.1.tgz install ``` On Windows, prefer the marketplace path or the `npx` helper. The shell-script helper below is POSIX-only. @@ -266,7 +266,7 @@ $cc:setup --disable-review-gate # turn it off ``` Setup checks Claude Code availability, native plugin hook feature gates, and review-gate state. `--check` reports required repairs without changing config or hook trust. If Claude Code isn't installed, setup offers to install it. -This is also the repair path for marketplace-installed copies of the plugin: `$cc:setup` confirms `[features].hooks = true` and `[features].plugin_hooks = true`, then trusts this plugin's current native hook hashes so Codex loads the bundled hooks from the active plugin cache. +This is also the repair path for marketplace-installed copies of the plugin: `$cc:setup` installs or refreshes the stable hook launcher, confirms `[features].hooks = true`, then trusts this plugin's current native hook hashes. ## Background Jobs @@ -341,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.6.0 +codex plugin marketplace add CBEPX/cc-plugin-codex --ref v1.6.1 codex plugin add cc@cbepx ``` @@ -351,7 +351,7 @@ Then run: $cc:setup ``` -Marketplace/plugin install places the plugin under Codex's plugin cache. `$cc:setup` verifies Claude Code, confirms `[features].hooks = true` plus `[features].plugin_hooks = true`, and trusts the current `hooks/hooks.json` hook hashes from the active plugin cache. +Marketplace/plugin install places the plugin under Codex's plugin cache. `$cc:setup` verifies Claude Code, installs or refreshes the stable hook launcher, confirms `[features].hooks = true`, and trusts the current `hooks/hooks.json` hook hashes. ### Upstream Sendbird build @@ -362,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.6.0 \ -npx -y https://github.com/CBEPX/cc-plugin-codex/releases/download/v1.6.0/cc-plugin-codex-1.6.0.tgz install +CC_PLUGIN_CODEX_MARKETPLACE_REF=v1.6.1 \ +npx -y https://github.com/CBEPX/cc-plugin-codex/releases/download/v1.6.1/cc-plugin-codex-1.6.1.tgz install ``` After install, run: @@ -393,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.6.0 +codex plugin marketplace add CBEPX/cc-plugin-codex --ref v1.6.1 codex plugin add cc@cbepx ``` @@ -415,7 +415,7 @@ claude auth login Re-run install and restart Codex. This plugin expects Codex plugin support and no longer installs local skill-wrapper fallbacks. **Hooks not firing** -Check that `hooks = true` and `plugin_hooks = true` are set in `~/.codex/config.toml` under `[features]`. Run `$cc:setup` to verify and auto-repair the feature gates plus this plugin's hook trust hashes, then restart Codex if those flags were just changed. +Check that `hooks = true` is set in `~/.codex/config.toml` under `[features]`. Run `$cc:setup` to verify and auto-repair the stable launcher, feature gate, and this plugin's hook trust hashes, then restart Codex if the flag was just changed. **A background job finished but I did not get the result nudge** Use: diff --git a/hooks/hooks.json b/hooks/hooks.json index e1ada7b..e87615a 100644 --- a/hooks/hooks.json +++ b/hooks/hooks.json @@ -6,7 +6,7 @@ "hooks": [ { "type": "command", - "command": "node \"$PLUGIN_ROOT/hooks/session-lifecycle-hook.mjs\"", + "command": "node \"${PLUGIN_DATA}/runtime/hook-launcher.mjs\" session-lifecycle", "statusMessage": "Initializing Claude Code bridge" } ] @@ -18,7 +18,7 @@ "hooks": [ { "type": "command", - "command": "node \"$PLUGIN_ROOT/hooks/session-lifecycle-hook.mjs\" SessionEnd", + "command": "node \"${PLUGIN_DATA}/runtime/hook-launcher.mjs\" session-lifecycle SessionEnd", "timeout": 3, "statusMessage": "Cleaning up Claude Code bridge jobs" } @@ -30,7 +30,7 @@ "hooks": [ { "type": "command", - "command": "node \"$PLUGIN_ROOT/hooks/stop-review-gate-hook.mjs\"", + "command": "node \"${PLUGIN_DATA}/runtime/hook-launcher.mjs\" stop-review-gate", "timeout": 900 } ] @@ -42,7 +42,7 @@ "hooks": [ { "type": "command", - "command": "node \"$PLUGIN_ROOT/hooks/unread-result-hook.mjs\"", + "command": "node \"${PLUGIN_DATA}/runtime/hook-launcher.mjs\" unread-result", "statusMessage": "Checking for unread Claude Code results" } ] diff --git a/package-lock.json b/package-lock.json index 458d344..8bbac17 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "cc-plugin-codex", - "version": "1.6.0", + "version": "1.6.1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "cc-plugin-codex", - "version": "1.6.0", + "version": "1.6.1", "license": "Apache-2.0", "bin": { "cc-plugin-codex": "scripts/installer-cli.mjs" diff --git a/package.json b/package.json index 40b9838..0bf1939 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "cc-plugin-codex", - "version": "1.6.0", + "version": "1.6.1", "description": "Claude Code Plugin for Codex (CBEPX fork)", "type": "module", "author": { diff --git a/scripts/claude-companion.mjs b/scripts/claude-companion.mjs index 73a2d17..c98d798 100644 --- a/scripts/claude-companion.mjs +++ b/scripts/claude-companion.mjs @@ -75,6 +75,11 @@ import { ensureNativePluginHooksEnabled, nativePluginHooksStatus, } from "./lib/codex-config.mjs"; +import { + hookLauncherStatus, + installHookLauncher, +} from "./lib/hook-launcher-install.mjs"; +import { pluginDataNamespaceForMarketplace } from "./lib/plugin-identity.mjs"; import { loadPromptTemplate, interpolateTemplate } from "./lib/prompts.mjs"; import { parseStructuredOutput } from "./lib/structured-output.mjs"; import { @@ -252,16 +257,27 @@ function resolveParentThreadId() { function buildSessionRoutingContext(cwd) { const workspaceRoot = resolveWorkspaceRoot(cwd); + const parentThreadId = resolveParentThreadId(); return { workspaceRoot, - ownerSessionId: - resolveOwnerSessionId( - process.env[SESSION_ID_ENV] ?? getCurrentSession(workspaceRoot) ?? null - ), - parentThreadId: resolveParentThreadId(), + ownerSessionId: resolveOwnerSessionId( + process.env[SESSION_ID_ENV] ?? + parentThreadId ?? + getCurrentSession(workspaceRoot) + ), + parentThreadId, }; } +function resolveCommandOwnerSessionId(value, workspaceRoot) { + return resolveOwnerSessionId( + value ?? + process.env[SESSION_ID_ENV] ?? + resolveParentThreadId() ?? + getCurrentSession(workspaceRoot) + ); +} + function alignCurrentSessionToOwner(workspaceRoot, ownerSessionId) { if (!ownerSessionId) { return; @@ -273,13 +289,20 @@ function alignCurrentSessionToOwner(workspaceRoot, ownerSessionId) { } function assertDelegationAllowed(workspaceRoot, ownerSessionId, workLabel) { + const claudeDrivenEnvironment = [ + process.env.CLAUDECODE, + process.env.CLAUDE_CODE_ENTRYPOINT, + ].some((value) => String(value ?? "").trim()); const marker = getCurrentSessionMarker(workspaceRoot); - if (!marker || marker.hostOrigin !== "claude-code") { + if (!claudeDrivenEnvironment && (!marker || marker.hostOrigin !== "claude-code")) { return; } const effectiveOwnerSessionId = - ownerSessionId ?? process.env[SESSION_ID_ENV] ?? marker.sessionId; - if (effectiveOwnerSessionId !== marker.sessionId) { + ownerSessionId ?? process.env[SESSION_ID_ENV] ?? marker?.sessionId; + if ( + !claudeDrivenEnvironment && + effectiveOwnerSessionId !== marker?.sessionId + ) { return; } throw new Error( @@ -689,13 +712,27 @@ function checkHooksStatus() { }; } - const status = nativePluginHooksStatus(readCodexConfig()); - if (status.installed) { + const featureStatus = nativePluginHooksStatus(readCodexConfig()); + const pluginInfo = currentPluginCacheInstallInfo(); + const launcherStatus = hookLauncherStatus( + ROOT_DIR, + pluginInfo + ? pluginDataNamespaceForMarketplace(pluginInfo.marketplaceName) + : undefined + ); + if (featureStatus.installed && launcherStatus.installed) { return { installed: true, detail: "native Codex plugin hooks enabled" }; } + const problems = []; + if (!featureStatus.installed) { + problems.push(`missing ${featureStatus.missing.join(", ")}`); + } + if (!launcherStatus.installed) { + problems.push(launcherStatus.detail); + } return { installed: false, - detail: `native Codex plugin hooks disabled: missing ${status.missing.join(", ")}`, + detail: `native Codex plugin hooks disabled: ${problems.join("; ")}`, }; } @@ -809,9 +846,22 @@ async function handleSetup(argv) { const workspaceRoot = resolveCommandWorkspace(options); const actionsTaken = []; + if (!options.check) { + const pluginInfo = currentPluginCacheInstallInfo(); + const launcher = installHookLauncher( + ROOT_DIR, + pluginInfo + ? pluginDataNamespaceForMarketplace(pluginInfo.marketplaceName) + : undefined + ); + if (launcher.changed) { + actionsTaken.push(`Installed the stable native hook launcher at ${launcher.destination}.`); + } + } + if (!options.check && configureNativePluginHooks()) { actionsTaken.push( - "Enabled native Codex plugin hooks via [features].hooks and [features].plugin_hooks." + "Enabled native Codex plugin hooks via [features].hooks." ); actionsTaken.push("Restart Codex if this session started before the feature change."); } @@ -1211,7 +1261,9 @@ async function executeReviewRun(request) { requestedModel: result.requestedModel ?? null, finalModel: result.finalModel ?? null, contextWindow: result.contextWindow ?? null, - modelFallbacks + modelFallbacks, + parseErrors: result.parseErrors ?? [], + unresolvedParseErrors: result.unresolvedParseErrors ?? 0 } }; const rendered = appendModelFallbackSummary( @@ -1319,7 +1371,9 @@ async function executeReviewRun(request) { requestedModel: result.requestedModel ?? null, finalModel: result.finalModel ?? null, contextWindow: result.contextWindow ?? null, - modelFallbacks + modelFallbacks, + parseErrors: result.parseErrors ?? [], + unresolvedParseErrors: result.unresolvedParseErrors ?? 0 }, result: parsed.parsed, rawOutput: parsed.rawOutput, @@ -1397,6 +1451,7 @@ async function executeTaskRun(request) { effort: request.effort ?? undefined, permissionMode: request.write ? "bypassPermissions" : "dontAsk", settingsFile: sandboxSettingsFile, + allowTerminalWithParseErrors: !request.write, }; // workspace-write: all tools (no allowedTools = everything including MCP/Skill/Agent) @@ -1448,6 +1503,8 @@ async function executeTaskRun(request) { contextWindow: result.contextWindow ?? null, modelFallbacks, failure: result.failure ?? null, + parseErrors: result.parseErrors ?? [], + unresolvedParseErrors: result.unresolvedParseErrors ?? 0, rawOutput, touchedFiles: Array.isArray(result.touchedFiles) ? result.touchedFiles @@ -1863,11 +1920,52 @@ function markTerminalJobViewed(workspaceRoot, jobId, viewedAt = nowIso()) { // Foreground execution wrapper // --------------------------------------------------------------------------- +function installForegroundReviewSignalHandlers(job, onSignal) { + const handlers = new Map(); + let handlingSignal = false; + for (const { signal, exitCode } of [ + { signal: "SIGINT", exitCode: 130 }, + { signal: "SIGTERM", exitCode: 143 }, + ]) { + const handler = () => { + if (handlingSignal) return; + handlingSignal = true; + onSignal(exitCode); + for (const [registeredSignal, registeredHandler] of handlers) { + process.removeListener(registeredSignal, registeredHandler); + } + void (async () => { + try { + await cancelStoredJob(job.workspaceRoot, job); + } catch (error) { + process.stderr.write( + `[cc] Failed to cancel foreground review after ${signal}: ${error instanceof Error ? error.message : String(error)}\n` + ); + } + })(); + }; + handlers.set(signal, handler); + process.once(signal, handler); + } + return () => { + for (const [signal, handler] of handlers) { + process.removeListener(signal, handler); + } + }; +} + async function runForegroundCommand(job, runner, options = {}) { const { logFile, progress } = createTrackedProgress(job, { logFile: options.logFile, stderr: !options.json && !options.quietProgress }); + let signalExitCode = null; + const removeSignalHandlers = installForegroundReviewSignalHandlers( + job, + (exitCode) => { + signalExitCode = exitCode; + } + ); try { const execution = await runTrackedJob( job, @@ -1883,9 +1981,13 @@ async function runForegroundCommand(job, runner, options = {}) { } return execution; } finally { + removeSignalHandlers(); if (options.markViewedOnTerminal) { markTerminalJobViewed(job.workspaceRoot, job.id); } + if (signalExitCode != null) { + process.exitCode = signalExitCode; + } } } @@ -2256,7 +2358,10 @@ async function handleReviewCommand(argv, config) { scope: options.scope }); const explicitJobId = resolveExplicitJobId(options["job-id"], workspaceRoot); - const ownerSessionId = resolveOwnerSessionId(options["owner-session-id"]); + const ownerSessionId = resolveCommandOwnerSessionId( + options["owner-session-id"], + workspaceRoot + ); const markViewedOnTerminal = resolveMarkViewedOnTerminal( options["view-state"], Boolean(options.background) @@ -2420,8 +2525,9 @@ async function handleTask(argv) { if (resumeLast && fresh) { throw new Error("Choose either --resume/--resume-last or --fresh."); } - const ownerSessionId = resolveOwnerSessionId( - options["owner-session-id"] ?? process.env[SESSION_ID_ENV] + const ownerSessionId = resolveCommandOwnerSessionId( + options["owner-session-id"], + workspaceRoot ); if (resumeLast && !ownerSessionId) { throw new Error( @@ -2733,8 +2839,9 @@ function handleTaskResumeCandidate(argv) { const cwd = resolveCommandCwd(options); const workspaceRoot = resolveWorkspaceRoot(cwd); - const sessionId = resolveOwnerSessionId( - options["owner-session-id"] ?? process.env[SESSION_ID_ENV] ?? null + const sessionId = resolveCommandOwnerSessionId( + options["owner-session-id"], + workspaceRoot ); const state = resolveTaskResumeState(workspaceRoot, sessionId); const candidate = state.candidate?.job ?? null; @@ -2833,6 +2940,18 @@ async function handleCancel(argv) { const reference = positionals[0] ?? ""; const { workspaceRoot, job } = resolveCancelableJob(cwd, reference); + const result = await cancelStoredJob(workspaceRoot, job); + outputCommandResult( + result.payload, + result.transitioned + ? renderCancelReport(result.job) + : `Job ${job.id} is already ${result.payload.status}.\n`, + options.json + ); +} + +async function cancelStoredJob(workspaceRoot, job) { + // CAS: running/queued → cancelling const transition = transitionJob( workspaceRoot, @@ -2842,12 +2961,11 @@ async function handleCancel(argv) { ); if (!transition.transitioned) { const currentStatus = transition.job?.status ?? job.status; - outputCommandResult( - { jobId: job.id, status: currentStatus }, - `Job ${job.id} is already ${currentStatus}.\n`, - options.json - ); - return; + return { + payload: { jobId: job.id, status: currentStatus }, + job: transition.job ?? job, + transitioned: false, + }; } // Cancel via process group kill with PID identity verification @@ -2912,8 +3030,7 @@ async function handleCancel(argv) { title: job.title, note: cancelResult.note, }; - - outputCommandResult(payload, renderCancelReport(nextJob), options.json); + return { payload, job: nextJob, transitioned: true }; } // --------------------------------------------------------------------------- diff --git a/scripts/hook-launcher.mjs b/scripts/hook-launcher.mjs new file mode 100644 index 0000000..a271e73 --- /dev/null +++ b/scripts/hook-launcher.mjs @@ -0,0 +1,79 @@ +#!/usr/bin/env node + +/** + * Copyright 2026 Sendbird, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ +import fs from "node:fs"; +import path from "node:path"; +import { pathToFileURL } from "node:url"; + +const HOOK_FILES = Object.freeze({ + "session-lifecycle": "session-lifecycle-hook.mjs", + "stop-review-gate": "stop-review-gate-hook.mjs", + "unread-result": "unread-result-hook.mjs", +}); + +function resolveHook(root, hookFile) { + if (!root || !path.isAbsolute(root)) { + return null; + } + try { + const packageJson = JSON.parse(fs.readFileSync(path.join(root, "package.json"), "utf8")); + const hooksRoot = path.join(root, "hooks"); + const hookPath = path.join(root, "hooks", hookFile); + if ( + packageJson.name === "cc-plugin-codex" && + fs.lstatSync(root).isDirectory() && + fs.lstatSync(hooksRoot).isDirectory() && + fs.lstatSync(hookPath).isFile() + ) { + return { root, hookPath, mtimeMs: fs.statSync(root).mtimeMs }; + } + } catch {} + return null; +} + +function resolveCurrentHook(pluginRoot, hookFile) { + const direct = resolveHook(pluginRoot, hookFile); + if (direct) { + return direct; + } + + if (!pluginRoot || !path.isAbsolute(pluginRoot)) { + return null; + } + const versionsRoot = path.dirname(pluginRoot); + if ( + path.basename(versionsRoot) !== "cc" || + path.basename(path.dirname(path.dirname(versionsRoot))) !== "cache" + ) { + return null; + } + let entries; + try { + entries = fs.readdirSync(versionsRoot); + } catch { + entries = []; + } + const candidates = entries + .map((entry) => resolveHook(path.join(versionsRoot, entry), hookFile)) + .filter(Boolean) + .sort((left, right) => right.mtimeMs - left.mtimeMs || right.root.localeCompare(left.root)); + return candidates[0] ?? null; +} + +const [hookName, ...hookArgs] = process.argv.slice(2); +const hookFile = HOOK_FILES[hookName]; +if (!hookFile) { + throw new Error(`Unknown cc hook: ${hookName || ""}`); +} + +const resolved = resolveCurrentHook(process.env.PLUGIN_ROOT, hookFile); +if (!resolved) { + throw new Error(`Unable to locate the active cc plugin hook: ${hookFile}`); +} + +process.env.PLUGIN_ROOT = resolved.root; +process.argv = [process.execPath, resolved.hookPath, ...hookArgs]; +await import(pathToFileURL(resolved.hookPath).href); diff --git a/scripts/install-hooks.mjs b/scripts/install-hooks.mjs index d3bcbbd..57910f1 100644 --- a/scripts/install-hooks.mjs +++ b/scripts/install-hooks.mjs @@ -18,6 +18,8 @@ import path from "node:path"; import { fileURLToPath } from "node:url"; import { ensureNativePluginHooksEnabled } from "./lib/codex-config.mjs"; import { resolveCodexHome } from "./lib/codex-paths.mjs"; +import { installHookLauncher } from "./lib/hook-launcher-install.mjs"; +import { pluginDataNamespaceForMarketplace } from "./lib/plugin-identity.mjs"; import { removeManagedHooks, writeTextAtomic, @@ -27,6 +29,8 @@ const SCRIPT_DIR = path.dirname(fileURLToPath(import.meta.url)); const PLUGIN_ROOT = path.resolve(SCRIPT_DIR, ".."); const CODEX_DIR = resolveCodexHome(); const CODEX_CONFIG_TOML = path.join(CODEX_DIR, "config.toml"); +const MARKETPLACE_NAME = + process.env.CC_PLUGIN_CODEX_MARKETPLACE_NAME?.trim() || "cbepx"; // --------------------------------------------------------------------------- // Helpers @@ -58,6 +62,10 @@ function main() { `Cannot safely remove legacy hooks while ${path.join(CODEX_DIR, "hooks.json")} is invalid.` ); } + const launcher = installHookLauncher( + PLUGIN_ROOT, + pluginDataNamespaceForMarketplace(MARKETPLACE_NAME) + ); const nativeHooksChanged = configureNativePluginHooks(); if (nativeHooksChanged) { @@ -65,7 +73,10 @@ function main() { } else { console.log("Native Codex plugin hooks are already enabled."); } - console.log("Codex now loads this plugin's hooks from hooks/hooks.json in the active plugin cache."); + if (launcher.changed) { + console.log(`Installed stable native hook launcher at ${launcher.destination}.`); + } + console.log("Codex now loads this plugin's hooks through the stable plugin-data launcher."); } main(); diff --git a/scripts/installer-cli.mjs b/scripts/installer-cli.mjs index a1fa925..99a245c 100755 --- a/scripts/installer-cli.mjs +++ b/scripts/installer-cli.mjs @@ -13,10 +13,12 @@ import { fileURLToPath } from "node:url"; import { callCodexAppServer } from "./lib/codex-app-server.mjs"; import { ensureNativePluginHooksEnabled } from "./lib/codex-config.mjs"; import { resolveCodexHome } from "./lib/codex-paths.mjs"; +import { installHookLauncher } from "./lib/hook-launcher-install.mjs"; import { getManagedPluginSignals, LEGACY_MARKETPLACE_NAME, listManagedPluginCacheEntries, + pluginDataNamespaceForMarketplace, pluginIdForMarketplace, PLUGIN_NAME, } from "./lib/plugin-identity.mjs"; @@ -279,6 +281,10 @@ async function installOrUpdate() { allowSkipLegacyCleanup: true, }); const hooksChanged = configureNativePluginHooks(); + installHookLauncher( + PACKAGE_ROOT, + pluginDataNamespaceForMarketplace(marketplaceConfig.marketplaceName) + ); const marketplace = await addMarketplaceThroughCodex(marketplaceConfig); const marketplacePath = path.join( @@ -292,7 +298,7 @@ async function installOrUpdate() { console.log(`Installed ${PLUGIN_NAME} from ${marketplaceConfig.source} into the Codex plugin cache.`); if (hooksChanged) { - console.log("Enabled [features].hooks and [features].plugin_hooks in ~/.codex/config.toml."); + console.log("Enabled [features].hooks in ~/.codex/config.toml."); console.log("Restart Codex to make newly enabled native plugin hooks active in existing sessions."); } } diff --git a/scripts/lib/claude-cli.mjs b/scripts/lib/claude-cli.mjs index 07a9401..a522a9a 100644 --- a/scripts/lib/claude-cli.mjs +++ b/scripts/lib/claude-cli.mjs @@ -887,20 +887,23 @@ function mergeTerminalResultText(existingText, terminalText) { // Turn Completion Validation // --------------------------------------------------------------------------- -export function validateTurnCompletion(state, exitCode) { +export function validateTurnCompletion(state, exitCode, options = {}) { if (exitCode !== 0) { return { status: "failed", exitCode }; } - if (state.unresolvedParseErrors > 0) { + if (!state.receivedTerminalEvent) { return { status: "unknown", - warning: `${state.unresolvedParseErrors} unrecovered parse errors`, + warning: "No terminal result event received despite exit code 0", }; } - if (!state.receivedTerminalEvent) { + if (state.unresolvedParseErrors > 0) { + const warning = + `${state.unresolvedParseErrors} unrecovered parse error` + + (state.unresolvedParseErrors === 1 ? "" : "s"); return { - status: "unknown", - warning: "No terminal result event received despite exit code 0", + status: options.allowTerminalWithParseErrors ? "completed" : "unknown", + warning, }; } if (state.unknownEvents.length > 0) { @@ -1327,6 +1330,8 @@ export async function runClaudeTurn(cwd, prompt, options = {}) { finalModel: null, contextWindow: null, modelEvents: [], + parseErrors: [], + unresolvedParseErrors: 0, failure: classifyClaudeFailure({ stderr: command.error, exitCode: -1, @@ -1409,7 +1414,11 @@ export async function runClaudeTurn(cwd, prompt, options = {}) { MAX_STDERR_BYTES ); } - let validation = validateTurnCompletion(parser.state, code ?? 1); + let validation = validateTurnCompletion(parser.state, code ?? 1, { + allowTerminalWithParseErrors: Boolean( + options.allowTerminalWithParseErrors + ), + }); if (stdinError && validation.status !== "failed") { validation = { status: "failed", @@ -1461,6 +1470,8 @@ export async function runClaudeTurn(cwd, prompt, options = {}) { finalModel, contextWindow, modelEvents, + parseErrors: [...parser.state.parseErrors], + unresolvedParseErrors: parser.state.unresolvedParseErrors, failure, stderr, pid: proc.pid, @@ -1481,6 +1492,8 @@ export async function runClaudeTurn(cwd, prompt, options = {}) { finalModel: null, contextWindow: null, modelEvents: [], + parseErrors: [], + unresolvedParseErrors: 0, failure: classifyClaudeFailure({ stderr: err.message, exitCode: -1, @@ -1512,6 +1525,7 @@ export async function runClaudeReview(cwd, prompt, options = {}) { const result = await runClaudeTurn(cwd, prompt, { noSessionPersistence: true, allowedTools: SANDBOX_REVIEW_TOOLS, + allowTerminalWithParseErrors: true, ...options, }); @@ -1526,6 +1540,8 @@ export async function runClaudeReview(cwd, prompt, options = {}) { finalModel: result.finalModel, contextWindow: result.contextWindow, modelEvents: result.modelEvents, + parseErrors: result.parseErrors, + unresolvedParseErrors: result.unresolvedParseErrors, failure: result.failure, stderr: result.stderr, pid: result.pid, diff --git a/scripts/lib/codex-config.mjs b/scripts/lib/codex-config.mjs index 908ff93..9f53562 100644 --- a/scripts/lib/codex-config.mjs +++ b/scripts/lib/codex-config.mjs @@ -7,7 +7,7 @@ function normalizeTrailingNewline(text) { return `${String(text).replace(/\s*$/, "")}\n`; } -const REQUIRED_NATIVE_HOOK_FEATURES = ["hooks", "plugin_hooks"]; +const REQUIRED_NATIVE_HOOK_FEATURES = ["hooks"]; export function ensureNativePluginHooksEnabled(content) { const lines = String(content ?? "").split("\n"); diff --git a/scripts/lib/hook-launcher-install.mjs b/scripts/lib/hook-launcher-install.mjs new file mode 100644 index 0000000..5a03243 --- /dev/null +++ b/scripts/lib/hook-launcher-install.mjs @@ -0,0 +1,35 @@ +/** + * Copyright 2026 Sendbird, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ +import fs from "node:fs"; +import path from "node:path"; +import { resolvePluginRuntimeRoot } from "./codex-paths.mjs"; +import { writeTextAtomic } from "./managed-global-integration.mjs"; + +export function hookLauncherStatus(pluginRoot, namespace) { + const source = path.join(pluginRoot, "scripts", "hook-launcher.mjs"); + const destination = path.join(resolvePluginRuntimeRoot(namespace), "hook-launcher.mjs"); + let sourceContent; + try { + sourceContent = fs.readFileSync(source, "utf8"); + } catch { + return { installed: false, detail: `hook launcher source missing at ${source}`, destination }; + } + try { + if (fs.lstatSync(destination).isFile() && fs.readFileSync(destination, "utf8") === sourceContent) { + return { installed: true, detail: "stable hook launcher installed", destination }; + } + } catch {} + return { installed: false, detail: `stable hook launcher missing or stale at ${destination}`, destination }; +} + +export function installHookLauncher(pluginRoot, namespace) { + const status = hookLauncherStatus(pluginRoot, namespace); + if (status.installed) { + return { ...status, changed: false }; + } + const source = path.join(pluginRoot, "scripts", "hook-launcher.mjs"); + writeTextAtomic(status.destination, fs.readFileSync(source, "utf8")); + return { ...hookLauncherStatus(pluginRoot, namespace), changed: true }; +} diff --git a/scripts/lib/job-control.mjs b/scripts/lib/job-control.mjs index 5d5a64f..bfe5587 100644 --- a/scripts/lib/job-control.mjs +++ b/scripts/lib/job-control.mjs @@ -10,15 +10,20 @@ */ import fs from "node:fs"; +import path from "node:path"; + +import { resolvePluginStateRoot } from "./codex-paths.mjs"; import { getConfig, getCurrentSession, listJobs, readJobFile, + resolveJobsDir, resolveJobFile, resolveJobLogFile, TERMINAL_JOB_STATUSES, + sanitizeId, } from "./state.mjs"; import { SESSION_ID_ENV } from "./tracked-jobs.mjs"; import { resolveWorkspaceRoot } from "./workspace.mjs"; @@ -178,6 +183,56 @@ function matchJobReference(jobs, reference, predicate = () => true) { throw new Error(`No job found for "${reference}". Run status to list known jobs.`); } +function findExactJobAcrossWorkspaces(reference) { + const jobId = sanitizeId(reference, "job ID"); + const stateRoot = resolvePluginStateRoot(); + let entries = []; + try { + entries = fs.readdirSync(stateRoot, { withFileTypes: true }); + } catch { + return null; + } + + const matches = []; + for (const entry of entries) { + if (!entry.isDirectory()) continue; + const candidate = path.join(stateRoot, entry.name, "jobs", `${jobId}.json`); + try { + if (!fs.lstatSync(candidate).isFile()) continue; + const stored = JSON.parse(fs.readFileSync(candidate, "utf8")); + if ( + stored?.id !== jobId || + typeof stored.workspaceRoot !== "string" || + path.resolve(resolveJobsDir(stored.workspaceRoot), `${jobId}.json`) !== + path.resolve(candidate) + ) { + continue; + } + const job = listJobs(stored.workspaceRoot).find( + (knownJob) => knownJob.id === jobId + ); + if (job) { + matches.push({ workspaceRoot: stored.workspaceRoot, job }); + } + } catch {} + } + + if (matches.length > 1) { + throw new Error( + `Job ${jobId} exists in multiple workspaces. Run status from the intended workspace.` + ); + } + return matches[0] ?? null; +} + +function resolveReferencedJob(workspaceRoot, jobs, reference) { + const exact = jobs.find((job) => job.id === reference); + if (exact) return { workspaceRoot, job: exact }; + const global = findExactJobAcrossWorkspaces(reference); + if (global) return global; + return { workspaceRoot, job: matchJobReference(jobs, reference) }; +} + export function buildStatusSnapshot(cwd, options = {}) { const workspaceRoot = resolveWorkspaceRoot(cwd); const config = getConfig(workspaceRoot); @@ -218,11 +273,13 @@ export function buildStatusSnapshot(cwd, options = {}) { export function buildSingleJobSnapshot(cwd, reference, options = {}) { const workspaceRoot = resolveWorkspaceRoot(cwd); const jobs = sortJobsNewestFirst(listJobs(workspaceRoot)); - const selected = matchJobReference(jobs, reference); - if (!selected) throw new Error(`No job found for "${reference}".`); + const resolved = reference + ? resolveReferencedJob(workspaceRoot, jobs, reference) + : { workspaceRoot, job: matchJobReference(jobs, reference) }; + if (!resolved.job) throw new Error(`No job found for "${reference}".`); return { - workspaceRoot, - job: enrichJob(selected, { maxProgressLines: options.maxProgressLines }), + workspaceRoot: resolved.workspaceRoot, + job: enrichJob(resolved.job, { maxProgressLines: options.maxProgressLines }), }; } @@ -236,13 +293,13 @@ export function resolveResultJob(cwd, reference) { }) ); if (reference) { - const selected = matchJobReference(jobs, reference); - const enriched = enrichJob(selected); + const resolved = resolveReferencedJob(workspaceRoot, jobs, reference); + const enriched = enrichJob(resolved.job); if (TERMINAL_JOB_STATUSES.has(enriched.status)) { - return { workspaceRoot, job: enriched, state: "terminal" }; + return { workspaceRoot: resolved.workspaceRoot, job: enriched, state: "terminal" }; } if (enriched.status === "queued" || ACTIVE_STATUSES.has(enriched.status)) { - return { workspaceRoot, job: enriched, state: "active" }; + return { workspaceRoot: resolved.workspaceRoot, job: enriched, state: "active" }; } throw new Error( `Job ${enriched.id} is ${enriched.status}. Check status for more details.` @@ -263,9 +320,17 @@ export function resolveCancelableJob(cwd, reference) { const jobs = sortJobsNewestFirst(listJobs(workspaceRoot)); const activeJobs = jobs.filter((job) => job.status === "running" || job.status === "queued"); if (reference) { - const selected = matchJobReference(activeJobs, reference); - if (!selected) throw new Error(`No active job found for "${reference}".`); - return { workspaceRoot, job: selected }; + const localExact = jobs.find((job) => job.id === reference); + const resolved = localExact + ? { workspaceRoot, job: localExact } + : findExactJobAcrossWorkspaces(reference) ?? { + workspaceRoot, + job: matchJobReference(activeJobs, reference), + }; + if (resolved.job.status !== "running" && resolved.job.status !== "queued") { + throw new Error(`No active job found for "${reference}".`); + } + return resolved; } if (activeJobs.length === 1) return { workspaceRoot, job: activeJobs[0] }; if (activeJobs.length > 1) throw new Error("Multiple Claude Code jobs are active. Pass a job id to $cc:cancel."); diff --git a/scripts/lib/plugin-identity.mjs b/scripts/lib/plugin-identity.mjs index eeca51e..82ab8a2 100644 --- a/scripts/lib/plugin-identity.mjs +++ b/scripts/lib/plugin-identity.mjs @@ -27,6 +27,14 @@ export function pluginIdForMarketplace(marketplaceName) { return `${PLUGIN_NAME}@${marketplaceName}`; } +export function pluginDataNamespaceForMarketplace(marketplaceName) { + const normalized = String(marketplaceName ?? "").trim(); + if (!/^[A-Za-z0-9][A-Za-z0-9._-]*$/.test(normalized)) { + throw new Error(`Invalid marketplace name: ${marketplaceName}`); + } + return `${PLUGIN_NAME}-${normalized}`; +} + export function pluginConfigHeader(marketplaceName) { return `[plugins."${pluginIdForMarketplace(marketplaceName)}"]`; } diff --git a/scripts/lib/state.mjs b/scripts/lib/state.mjs index fd7bc49..e76f9fc 100644 --- a/scripts/lib/state.mjs +++ b/scripts/lib/state.mjs @@ -608,6 +608,8 @@ export function reapStaleJobs(cwd, jobs, options = {}) { 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, @@ -817,6 +819,11 @@ export function reapStaleJobs(cwd, jobs, options = {}) { ); } const unresolvedClaudeChild = hasDistinctClaudeChild && !childResolved; + const reapReason = identityUnavailableTooLong + ? "identity-unverifiable" + : processExists + ? "identity-mismatch" + : "process-missing"; const nextStatus = job.status === "cancelling" ? (identityUnavailableTooLong || unresolvedClaudeChild ? "cancel_failed" @@ -828,6 +835,8 @@ export function reapStaleJobs(cwd, jobs, options = {}) { `Process ${trackedPid} identity remained unverifiable beyond the bounded Windows recheck window. Manual cleanup may be required.`, completedAt: nowIso(), phase: nextStatus, + reapedBy: "status-reaper", + reapReason, reapedUnverifiable: true, workerPid: null, workerPidIdentity: null, @@ -846,6 +855,8 @@ export function reapStaleJobs(cwd, jobs, options = {}) { : "" }`, completedAt: nowIso(), + reapedBy: "status-reaper", + reapReason, ...(unresolvedClaudeChild ? {} : { pid: null, pidIdentity: null }), diff --git a/scripts/lib/tracked-jobs.mjs b/scripts/lib/tracked-jobs.mjs index 9910eef..c6fa1cb 100644 --- a/scripts/lib/tracked-jobs.mjs +++ b/scripts/lib/tracked-jobs.mjs @@ -367,18 +367,10 @@ export async function runTrackedJob(job, runner, options = {}) { options.getProcessIdentityImpl ?? getSpawnedProcessIdentity; const storedJob = readJobFile(job.workspaceRoot, job.id); - let workerPidIdentity = - [job, storedJob].find( - (candidate) => - candidate?.workerPid === workerPid && - typeof candidate.workerPidIdentity === "string" && - candidate.workerPidIdentity - )?.workerPidIdentity ?? null; - if (!workerPidIdentity) { - try { - workerPidIdentity = getWorkerProcessIdentityImpl(workerPid); - } catch {} - } + let workerPidIdentity = null; + try { + workerPidIdentity = getWorkerProcessIdentityImpl(workerPid); + } catch {} // ponytail: without a stable worker identity, fall back to the child's // identity; add an alternate worker identity source only if this becomes common. const runningRecord = { @@ -479,7 +471,8 @@ export async function runTrackedJob(job, runner, options = {}) { if ( !transitioned.transitioned && transitioned.previousStatus === "failed" && - transitioned.job?.reapedUnverifiable === true + (transitioned.job?.reapedBy === "status-reaper" || + transitioned.job?.reapedUnverifiable === true) ) { transitioned = transitionTrackedJob( job.workspaceRoot, @@ -489,6 +482,8 @@ export async function runTrackedJob(job, runner, options = {}) { { ...terminalData, errorMessage: null, + reapedBy: null, + reapReason: null, reapedUnverifiable: false, } ); diff --git a/skills/setup/SKILL.md b/skills/setup/SKILL.md index ace2f12..c71a6a9 100644 --- a/skills/setup/SKILL.md +++ b/skills/setup/SKILL.md @@ -21,7 +21,7 @@ Workflow: - If it reports that Claude Code is unavailable and `npm` is available, ask whether to install Claude Code now. - If the user agrees, run `npm install -g @anthropic-ai/claude-code` and rerun setup. - If Claude Code is already installed or `npm` is unavailable, do not ask about installation. -- Unless the user explicitly requested `--check`, if the check reports missing native plugin hook features or hook trust, run setup once without `--check`. The companion repairs `[features].hooks`, `[features].plugin_hooks`, and this plugin's native hook trust hashes itself. +- Unless the user explicitly requested `--check`, if the check reports a missing stable hook launcher, native plugin hook feature, or hook trust, run setup once without `--check`. The companion repairs the launcher, `[features].hooks`, and this plugin's native hook trust hashes itself. - After the decision flow is complete, run the final user-facing command without `--json`: `node "/scripts/claude-companion.mjs" setup $ARGUMENTS` diff --git a/stryker.shard.config.mjs b/stryker.shard.config.mjs index 53bd1e2..1018637 100644 --- a/stryker.shard.config.mjs +++ b/stryker.shard.config.mjs @@ -24,19 +24,19 @@ const shards = { "scripts/lib/state.mjs:188-228", "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/state.mjs:545-886", + "scripts/lib/state.mjs:935-1107", + "scripts/lib/state.mjs:1173-1230", + "scripts/lib/state.mjs:1236-1282", "scripts/lib/tracked-jobs.mjs:30-43", "scripts/lib/tracked-jobs.mjs:286-344", - "scripts/lib/tracked-jobs.mjs:363-522", + "scripts/lib/tracked-jobs.mjs:363-517", ], }, "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:170-273"], + mutate: ["scripts/lib/job-control.mjs:175-338"], }, managed: { command: "npm run test:mutation:managed:unit", @@ -49,8 +49,8 @@ const shards = { command: "npm run test:mutation:installer:unit", mutate: [ // Marketplace validation/config cleanup and the complete uninstall orchestration. - "scripts/installer-cli.mjs:96-234", - "scripts/installer-cli.mjs:275-371", + "scripts/installer-cli.mjs:98-236", + "scripts/installer-cli.mjs:277-377", ], }, }; diff --git a/tests/cancel-command.test.mjs b/tests/cancel-command.test.mjs index 1ff02f5..b865b0c 100644 --- a/tests/cancel-command.test.mjs +++ b/tests/cancel-command.test.mjs @@ -86,3 +86,36 @@ test("cancel uses the in-lock PID when the list snapshot is stale", () => { test("cancel does not silently succeed when the list snapshot has no PID", () => { runCancelSnapshotRace("cancel-missing-pid-race", null); }); + +test("cancel reports when a selected job became terminal before the transition", () => { + const cwd = fs.mkdtempSync(path.join(os.tmpdir(), "cc-cancel-terminal-race-")); + cleanup.push(() => fs.rmSync(cwd, { recursive: true, force: true })); + assert.equal(spawnSync("git", ["init", "-q"], { cwd }).status, 0); + const id = "cancel-terminal-race"; + const createdAt = new Date().toISOString(); + const snapshot = { id, status: "running", createdAt, updatedAt: createdAt }; + const completed = { + ...snapshot, + status: "completed", + completedAt: createdAt, + }; + writeJobFile(cwd, id, snapshot); + + const result = spawnSync( + process.execPath, + ["--import", SWAP_PRELOAD, COMPANION, "cancel", id, "--cwd", cwd], + { + encoding: "utf8", + env: { + ...process.env, + CC_TEST_SWAP_JOB_FILE: resolveJobFile(cwd, id), + CC_TEST_SWAP_JOB_JSON: `${JSON.stringify(completed, null, 2)}\n`, + CC_TEST_SWAP_JOB_AFTER_READ: "1", + }, + } + ); + + assert.equal(result.status, 0, result.stderr); + assert.match(result.stdout, /already completed/u); + assert.doesNotMatch(result.stdout, /Cancelled cancel-terminal-race/u); +}); diff --git a/tests/claude-cli.test.mjs b/tests/claude-cli.test.mjs index fdcc1f2..77d9e5c 100644 --- a/tests/claude-cli.test.mjs +++ b/tests/claude-cli.test.mjs @@ -1360,6 +1360,31 @@ describe("classifyClaudeFailure", () => { }); describe("runClaudeTurn", () => { + it("returns bounded parser diagnostics when read-only output has a valid terminal event", async () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "cc-plugin-claude-parse-")); + const oldPath = process.env.PATH ?? ""; + try { + createFakeClaudeCommand( + tmpDir, + `process.stdout.write("not-json\\n");\nconst out = JSON.stringify({ type: "result", result: "done", session_id: "sess-parse" });\nprocess.stdout.write(out + "\\n", () => process.exit(0));\n` + ); + process.env.PATH = `${tmpDir}${path.delimiter}${oldPath}`; + + const result = await runClaudeTurn(process.cwd(), "prompt", { + allowTerminalWithParseErrors: true, + }); + + assert.equal(result.status, "completed"); + assert.match(result.warning, /1 unrecovered parse error/); + assert.equal(result.unresolvedParseErrors, 1); + assert.equal(result.parseErrors.length, 1); + assert.equal(result.parseErrors[0].line, "not-json"); + } finally { + process.env.PATH = oldPath; + fs.rmSync(tmpDir, { recursive: true, force: true }); + } + }); + 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 ?? ""; @@ -2049,6 +2074,24 @@ describe("validateTurnCompletion", () => { assert.ok(result.warning.includes("3 unrecovered parse errors")); }); + it("accepts terminal output with parse errors only when explicitly allowed", () => { + const state = { receivedTerminalEvent: true, unresolvedParseErrors: 1, unknownEvents: [] }; + const result = validateTurnCompletion(state, 0, { + allowTerminalWithParseErrors: true, + }); + assert.equal(result.status, "completed"); + assert.match(result.warning, /1 unrecovered parse error/); + }); + + it("does not accept parse errors without a terminal event", () => { + const state = { receivedTerminalEvent: false, unresolvedParseErrors: 1, unknownEvents: [] }; + const result = validateTurnCompletion(state, 0, { + allowTerminalWithParseErrors: true, + }); + assert.equal(result.status, "unknown"); + assert.match(result.warning, /No terminal result event/); + }); + it("returns completed even when unknown events exist (protocol drift)", () => { const state = { receivedTerminalEvent: true, unresolvedParseErrors: 0, unknownEvents: [{ type: "new_type", ts: 1 }] }; const result = validateTurnCompletion(state, 0); diff --git a/tests/e2e/codex-skills-e2e.test.mjs b/tests/e2e/codex-skills-e2e.test.mjs index 1178d81..07f0878 100644 --- a/tests/e2e/codex-skills-e2e.test.mjs +++ b/tests/e2e/codex-skills-e2e.test.mjs @@ -202,7 +202,7 @@ function installHooks(testEnv) { const config = fs.readFileSync(configFile, "utf8"); assert.ok(!fs.existsSync(hooksFile), "native plugin hooks should not install global hooks"); assert.match(config, /hooks = true/); - assert.match(config, /plugin_hooks = true/); + assert.doesNotMatch(config, /plugin_hooks/); } function createLocalMarketplaceFixture(testEnv) { @@ -2227,7 +2227,7 @@ describe("Codex direct-skill E2E", () => { const config = fs.readFileSync(path.join(testEnv.codexHome, "config.toml"), "utf8"); assert.ok(!fs.existsSync(hooksFile), "setup should not install global hooks"); assert.match(config, /hooks = true/); - assert.match(config, /plugin_hooks = true/); + assert.doesNotMatch(config, /plugin_hooks/); } finally { await provider.close(); cleanupEnvironment(testEnv); @@ -2270,7 +2270,7 @@ describe("Codex direct-skill E2E", () => { const config = fs.readFileSync(path.join(testEnv.codexHome, "config.toml"), "utf8"); assert.ok(!fs.existsSync(hooksFile)); assert.match(config, /hooks = true/); - assert.match(config, /plugin_hooks = true/); + assert.doesNotMatch(config, /plugin_hooks/); } finally { await provider.close(); cleanupEnvironment(testEnv); diff --git a/tests/hook-launcher.test.mjs b/tests/hook-launcher.test.mjs new file mode 100644 index 0000000..aaa4631 --- /dev/null +++ b/tests/hook-launcher.test.mjs @@ -0,0 +1,97 @@ +/** + * Copyright 2026 Sendbird, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ +import { afterEach, describe, it } from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { spawnSync } from "node:child_process"; +import { fileURLToPath } from "node:url"; + +const PROJECT_ROOT = path.resolve(fileURLToPath(new URL("../", import.meta.url))); +const LAUNCHER = path.join(PROJECT_ROOT, "scripts", "hook-launcher.mjs"); +const tempDirs = []; + +afterEach(() => { + while (tempDirs.length > 0) { + fs.rmSync(tempDirs.pop(), { recursive: true, force: true }); + } +}); + +describe("hook-launcher.mjs", () => { + it("recovers from a deleted plugin root and preserves hook arguments", () => { + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "cc-hook-launcher-")); + tempDirs.push(tempDir); + const cacheRoot = path.join(tempDir, "plugins", "cache", "cbepx", "cc"); + const staleRoot = path.join(cacheRoot, "1.5.4"); + const currentRoot = path.join(cacheRoot, "1.6.1"); + const hooksDir = path.join(currentRoot, "hooks"); + const resultFile = path.join(tempDir, "hook-result.json"); + fs.mkdirSync(hooksDir, { recursive: true }); + fs.writeFileSync( + path.join(currentRoot, "package.json"), + JSON.stringify({ name: "cc-plugin-codex", version: "1.6.1" }), + "utf8" + ); + fs.writeFileSync( + path.join(hooksDir, "session-lifecycle-hook.mjs"), + `import fs from "node:fs";\nfs.writeFileSync(process.env.RESULT_FILE, JSON.stringify({ pluginRoot: process.env.PLUGIN_ROOT, args: process.argv.slice(2) }));\n`, + "utf8" + ); + + const result = spawnSync( + process.execPath, + [LAUNCHER, "session-lifecycle", "SessionEnd"], + { + env: { ...process.env, PLUGIN_ROOT: staleRoot, RESULT_FILE: resultFile }, + encoding: "utf8", + } + ); + + assert.equal(result.status, 0, result.stderr); + assert.deepEqual(JSON.parse(fs.readFileSync(resultFile, "utf8")), { + pluginRoot: currentRoot, + args: ["SessionEnd"], + }); + }); + + it("rejects unknown hook names", () => { + const result = spawnSync(process.execPath, [LAUNCHER, "../../arbitrary"], { + env: { ...process.env, PLUGIN_ROOT: PROJECT_ROOT }, + encoding: "utf8", + }); + + assert.notEqual(result.status, 0); + assert.match(result.stderr, /Unknown cc hook/u); + }); + + it("does not search the hook working directory when PLUGIN_ROOT is missing", () => { + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "cc-hook-launcher-cwd-")); + tempDirs.push(tempDir); + const attackerRoot = path.join(tempDir, "attacker"); + const hooksDir = path.join(attackerRoot, "hooks"); + const resultFile = path.join(tempDir, "executed"); + fs.mkdirSync(hooksDir, { recursive: true }); + fs.writeFileSync( + path.join(attackerRoot, "package.json"), + JSON.stringify({ name: "cc-plugin-codex" }), + "utf8" + ); + fs.writeFileSync( + path.join(hooksDir, "session-lifecycle-hook.mjs"), + `import fs from "node:fs"; fs.writeFileSync(${JSON.stringify(resultFile)}, "bad");\n`, + "utf8" + ); + + const result = spawnSync(process.execPath, [LAUNCHER, "session-lifecycle"], { + cwd: tempDir, + env: { ...process.env, PLUGIN_ROOT: "" }, + encoding: "utf8", + }); + + assert.notEqual(result.status, 0); + assert.equal(fs.existsSync(resultFile), false); + }); +}); diff --git a/tests/hooks.test.mjs b/tests/hooks.test.mjs index 889be85..4151e87 100644 --- a/tests/hooks.test.mjs +++ b/tests/hooks.test.mjs @@ -645,9 +645,19 @@ fs.readdirSync = (directory, ...args) => { const sessionStartHandler = manifest.hooks.SessionStart[0].hooks[0]; const handler = manifest.hooks.SessionEnd[0].hooks[0]; + for (const eventHandlers of Object.values(manifest.hooks)) { + for (const entry of eventHandlers) { + for (const hook of entry.hooks) { + assert.match(hook.command, /\$\{PLUGIN_DATA\}\/runtime\/hook-launcher\.mjs/u); + assert.doesNotMatch(hook.command, /"\$PLUGIN_DATA/u); + assert.doesNotMatch(hook.command, /\$PLUGIN_ROOT/u); + } + } + } + assert.equal("timeout" in sessionStartHandler, false); assert.equal(handler.timeout, 3); - assert.match(handler.command, /session-lifecycle-hook\.mjs.*SessionEnd/u); + assert.match(handler.command, /session-lifecycle SessionEnd/u); }); it("session lifecycle hook refuses to kill a stored PID without a matching identity", () => { diff --git a/tests/install-hooks.test.mjs b/tests/install-hooks.test.mjs index 502fe04..a051e90 100644 --- a/tests/install-hooks.test.mjs +++ b/tests/install-hooks.test.mjs @@ -14,6 +14,19 @@ const PROJECT_ROOT = path.resolve( fileURLToPath(new URL("../", import.meta.url)) ); const SCRIPT_PATH = path.join(PROJECT_ROOT, "scripts", "install-hooks.mjs"); +const HOOK_LAUNCHER_SOURCE = path.join(PROJECT_ROOT, "scripts", "hook-launcher.mjs"); + +function hookLauncherPath(homeDir) { + return path.join( + homeDir, + ".codex", + "plugins", + "data", + "cc-cbepx", + "runtime", + "hook-launcher.mjs" + ); +} function makeTempHome() { return fs.mkdtempSync(path.join(os.tmpdir(), "claude-install-hooks-")); @@ -60,7 +73,11 @@ describe("install-hooks.mjs", () => { const config = fs.readFileSync(configFile, "utf8"); assert.match(config, /\[features\]/); assert.match(config, /hooks = true/); - assert.match(config, /plugin_hooks = true/); + assert.doesNotMatch(config, /plugin_hooks/); + assert.equal( + fs.readFileSync(hookLauncherPath(homeDir), "utf8"), + fs.readFileSync(HOOK_LAUNCHER_SOURCE, "utf8") + ); assert.match(result.stdout, /native Codex plugin hooks/i); }); @@ -81,11 +98,29 @@ describe("install-hooks.mjs", () => { assert.match(config, /\[features\]/); assert.match(config, /hooks = true/); - assert.match(config, /plugin_hooks = true/); + assert.doesNotMatch(config, /plugin_hooks/); assert.doesNotMatch(config, /codex_hooks/); assert.match(result.stdout, /Enabled native Codex plugin hooks/i); }); + it("preserves an obsolete plugin_hooks setting while enabling hooks", () => { + const homeDir = makeTempHome(); + tempHomes.push(homeDir); + const codexDir = path.join(homeDir, ".codex"); + fs.mkdirSync(codexDir, { recursive: true }); + fs.writeFileSync( + path.join(codexDir, "config.toml"), + "[features]\nhooks = false\nplugin_hooks = false\n", + "utf8" + ); + + runInstallHooks(homeDir); + + const config = fs.readFileSync(path.join(codexDir, "config.toml"), "utf8"); + assert.match(config, /hooks = true/); + assert.match(config, /plugin_hooks = false/); + }); + it("removes stale managed global hook commands", () => { const homeDir = makeTempHome(); tempHomes.push(homeDir); diff --git a/tests/installer-cli.test.mjs b/tests/installer-cli.test.mjs index 467450a..339ae47 100644 --- a/tests/installer-cli.test.mjs +++ b/tests/installer-cli.test.mjs @@ -903,7 +903,22 @@ describe("installer-cli", () => { assert.match(config, /\[plugins\."cc@sendbird"\]/); assert.match(config, /hooks = true/); - assert.match(config, /plugin_hooks = true/); + assert.doesNotMatch(config, /plugin_hooks/); + assert.equal( + fs.readFileSync( + path.join( + homeDir, + ".codex", + "plugins", + "data", + "cc-sendbird", + "runtime", + "hook-launcher.mjs" + ), + "utf8" + ), + fs.readFileSync(path.join(sourceRoot, "scripts", "hook-launcher.mjs"), "utf8") + ); assert.ok(!fs.existsSync(legacyInstallDir), "installer should not create a stable local plugin root"); assert.ok(!fs.existsSync(hooksFile), "installer should not write global hooks.json"); assert.ok(fs.existsSync(cachedReviewSkill)); @@ -1019,7 +1034,7 @@ describe("installer-cli", () => { assert.equal(fs.statSync(managedConfig).mode & 0o777, 0o644); const config = fs.readFileSync(managedConfig, "utf8"); assert.match(config, /hooks = true/); - assert.match(config, /plugin_hooks = true/); + assert.doesNotMatch(config, /plugin_hooks/); assert.match(config, /\[plugins\."cc@sendbird"\]/); }); diff --git a/tests/integration/claude-companion.test.mjs b/tests/integration/claude-companion.test.mjs index c372382..627eecf 100644 --- a/tests/integration/claude-companion.test.mjs +++ b/tests/integration/claude-companion.test.mjs @@ -17,6 +17,26 @@ const PROJECT_ROOT = path.resolve( fileURLToPath(new URL("../../", import.meta.url)) ); const COMPANION_SCRIPT = path.join(PROJECT_ROOT, "scripts", "claude-companion.mjs"); +const HOOK_LAUNCHER_SOURCE = path.join(PROJECT_ROOT, "scripts", "hook-launcher.mjs"); + +function hookLauncherPath(testEnv) { + return path.join( + testEnv.homeDir, + ".codex", + "plugins", + "data", + "cc", + "runtime", + "hook-launcher.mjs" + ); +} + +function installHookLauncherFixture(testEnv) { + const destination = hookLauncherPath(testEnv); + fs.mkdirSync(path.dirname(destination), { recursive: true }); + fs.copyFileSync(HOOK_LAUNCHER_SOURCE, destination); + return destination; +} function createFakeClaudeBinary(binDir) { const claudePath = path.join(binDir, "claude"); @@ -264,6 +284,7 @@ function createTestEnvironment() { HOME: homeDir, USERPROFILE: homeDir, CODEX_HOME: path.join(homeDir, ".codex"), + CODEX_THREAD_ID: "", PATH: `${binDir}${path.delimiter}${process.env.PATH || ""}`, }, }; @@ -855,6 +876,10 @@ describe("claude-companion integration", () => { { env: testEnv.env } ); assert.equal(initial.reviewGateEnabled, false); + assert.equal( + fs.readFileSync(hookLauncherPath(testEnv), "utf8"), + fs.readFileSync(HOOK_LAUNCHER_SOURCE, "utf8") + ); const enabled = runCompanion( ["setup", "--cwd", testEnv.workspaceDir, "--enable-review-gate"], @@ -918,6 +943,7 @@ describe("claude-companion integration", () => { assert.equal(report.checkOnly, true); assert.equal(report.ready, false); assert.equal(report.hooks.installed, false); + assert.match(report.hooks.detail, /launcher/u); assert.equal(report.hookTrust.ready, false); assert.equal(report.hookTrust.pendingTrust, 1); assert.deepEqual(report.actionsTaken, []); @@ -934,6 +960,29 @@ describe("claude-companion integration", () => { } }); + it("setup --check reports a missing stable hook launcher without creating it", () => { + const testEnv = createTestEnvironment(); + const codexDir = path.join(testEnv.homeDir, ".codex"); + const configFile = path.join(codexDir, "config.toml"); + fs.mkdirSync(codexDir, { recursive: true }); + fs.writeFileSync(configFile, "[features]\nhooks = true\n", "utf8"); + + try { + const report = runCompanionJson( + ["setup", "--cwd", testEnv.workspaceDir, "--check", "--json"], + { env: testEnv.env } + ); + + assert.equal(report.ready, false); + assert.equal(report.hooks.installed, false); + assert.match(report.hooks.detail, /launcher/u); + assert.deepEqual(report.actionsTaken, []); + assert.equal(fs.existsSync(hookLauncherPath(testEnv)), false); + } finally { + cleanupTestEnvironment(testEnv); + } + }); + it("setup --check preserves ready config and state file metadata", () => { const testEnv = createTestEnvironment(); const codexDir = path.join(testEnv.homeDir, ".codex"); @@ -944,6 +993,7 @@ describe("claude-companion integration", () => { fs.mkdirSync(codexDir, { recursive: true }); fs.mkdirSync(stateDir, { recursive: true }); fs.writeFileSync(configFile, configContent, { mode: 0o640 }); + const launcherFile = installHookLauncherFixture(testEnv); fs.writeFileSync( stateFile, JSON.stringify({ version: 1, stopReviewGate: false }, null, 2) + "\n", @@ -952,7 +1002,8 @@ describe("claude-companion integration", () => { 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) => ({ + fs.utimesSync(launcherFile, fixedMtime, fixedMtime); + const before = [configFile, stateFile, launcherFile].map((filePath) => ({ content: fs.readFileSync(filePath, "utf8"), mode: fs.statSync(filePath).mode & 0o777, mtimeMs: fs.statSync(filePath).mtimeMs, @@ -983,7 +1034,7 @@ describe("claude-companion integration", () => { assert.equal(report.ready, true); assert.deepEqual(report.actionsTaken, []); - for (const [index, filePath] of [configFile, stateFile].entries()) { + for (const [index, filePath] of [configFile, stateFile, launcherFile].entries()) { const stat = fs.statSync(filePath); assert.equal(fs.readFileSync(filePath, "utf8"), before[index].content); assert.equal(stat.mode & 0o777, before[index].mode); @@ -1679,11 +1730,11 @@ describe("claude-companion integration", () => { } }); - it("does not classify unknown exit-zero output that mentions rate limiting as a Claude limit failure", () => { + it("accepts terminal output with parser diagnostics for a read-only task", () => { const testEnv = createTestEnvironment(); try { - const jsonResult = runCompanionExpectFailure( + const jsonPayload = runCompanionJson( [ "task", "--cwd", @@ -1694,13 +1745,13 @@ describe("claude-companion integration", () => { ], { env: testEnv.env } ); - const jsonPayload = JSON.parse(jsonResult.stdout); - assert.equal(jsonPayload.status, "unknown"); + assert.equal(jsonPayload.status, "completed"); assert.equal(jsonPayload.failure, null); + assert.equal(jsonPayload.parseErrors.length, 1); assert.match(jsonPayload.rawOutput, /completed:malformed-line document rate limiting and 429 handling/); - const textResult = runCompanionExpectFailure( + const textResult = runCompanion( [ "task", "--cwd", @@ -1710,7 +1761,7 @@ describe("claude-companion integration", () => { ], { env: testEnv.env } ); - assert.equal(textResult.status, 1); + assert.equal(textResult.status, 0); assert.match(textResult.stdout, /completed:malformed-line document rate limiting and 429 handling/); assert.doesNotMatch(textResult.stdout, /Claude usage limit reached/i); } finally { @@ -1718,6 +1769,31 @@ describe("claude-companion integration", () => { } }); + it("keeps terminal output with parser errors unknown for a workspace-write task", () => { + const testEnv = createTestEnvironment(); + + try { + const jsonResult = runCompanionExpectFailure( + [ + "task", + "--cwd", + testEnv.workspaceDir, + "--write", + "--json", + "--quiet-progress", + "malformed-line workspace-write delay=20", + ], + { env: testEnv.env } + ); + const jsonPayload = JSON.parse(jsonResult.stdout); + + assert.equal(jsonPayload.status, "unknown"); + assert.equal(jsonPayload.parseErrors.length, 1); + } finally { + cleanupTestEnvironment(testEnv); + } + }); + it("does not classify failed output that only mentions rate limiting in the final message", () => { const testEnv = createTestEnvironment(); @@ -2041,6 +2117,30 @@ describe("claude-companion integration", () => { } }); + it("refuses delegation from a Claude Code environment without a workspace marker", () => { + const testEnv = createTestEnvironment(); + + try { + setupGitWorkspace(testEnv.workspaceDir); + seedWorkingTreeDiff(testEnv.workspaceDir); + const env = { + ...testEnv.env, + CLAUDECODE: "1", + CLAUDE_CODE_ENTRYPOINT: "cli", + }; + 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.equal(listStoredJobs(testEnv).length, 0); + } finally { + cleanupTestEnvironment(testEnv); + } + }); + it("allows foreign-owner and unstamped delegation", () => { const testEnv = createTestEnvironment(); @@ -3101,6 +3201,29 @@ describe("claude-companion integration", () => { } }); + it("prefers the live CODEX_THREAD_ID over a stale workspace marker", () => { + const testEnv = createTestEnvironment(); + + try { + writeCurrentSessionMarker(testEnv, "stale-marker-session"); + const env = { + ...testEnv.env, + CODEX_THREAD_ID: "live-thread-session", + }; + delete env[SESSION_ID_ENV]; + + const payload = runCompanionJson( + ["session-routing-context", "--cwd", testEnv.workspaceDir, "--json"], + { env } + ); + + assert.equal(payload.ownerSessionId, "live-thread-session"); + assert.equal(payload.parentThreadId, "live-thread-session"); + } finally { + cleanupTestEnvironment(testEnv); + } + }); + it("drops invalid parent thread ids from session routing context", () => { const testEnv = createTestEnvironment(); @@ -3123,6 +3246,73 @@ describe("claude-companion integration", () => { } }); + it("uses CODEX_THREAD_ID as the owner when a linked worktree has no marker", () => { + const testEnv = createTestEnvironment(); + const linkedWorktree = path.join(testEnv.rootDir, "linked-worktree"); + const threadEnv = { + ...testEnv.env, + CODEX_THREAD_ID: "thread-linked-owner", + }; + delete threadEnv[SESSION_ID_ENV]; + + try { + setupGitWorkspace(testEnv.workspaceDir); + writeCurrentSessionMarker(testEnv, "root-workspace-session"); + runGit(testEnv.workspaceDir, [ + "worktree", + "add", + "-b", + "linked-routing-test", + linkedWorktree, + ]); + + const routing = runCompanionJson( + [ + "background-routing-context", + "--kind", + "task", + "--cwd", + linkedWorktree, + "--json", + ], + { env: threadEnv } + ); + assert.equal(routing.workspaceRoot, fs.realpathSync.native(linkedWorktree)); + assert.equal(routing.ownerSessionId, "thread-linked-owner"); + assert.equal(routing.parentThreadId, "thread-linked-owner"); + + runCompanion( + [ + "task", + "--cwd", + linkedWorktree, + "--job-id", + routing.jobId, + "--owner-session-id", + routing.ownerSessionId, + "--quiet-progress", + "linked-owner-seed delay=20", + ], + { env: threadEnv } + ); + + const resume = runCompanion( + [ + "task", + "--cwd", + linkedWorktree, + "--resume", + "--quiet-progress", + "linked-owner-resume delay=20", + ], + { env: threadEnv } + ); + assert.match(resume.stdout, /completed:linked-owner-resume/); + } finally { + cleanupTestEnvironment(testEnv); + } + }); + it("reports background routing context with a reserved review job id", () => { const testEnv = createTestEnvironment(); @@ -3360,7 +3550,8 @@ describe("claude-companion integration", () => { { env: testEnv.env } ); assert.equal(noSessionContextCandidate.available, false); - assert.equal(noSessionContextCandidate.sessionId, null); + assert.equal(noSessionContextCandidate.sessionId, "session-a"); + assert.equal(noSessionContextCandidate.reason, "active_task"); assert.equal(noSessionContextCandidate.candidate, null); writeCurrentSessionMarker(testEnv, "session-a"); @@ -3369,7 +3560,8 @@ describe("claude-companion integration", () => { { env: testEnv.env } ); assert.equal(markerCandidateA.available, false); - assert.equal(markerCandidateA.sessionId, null); + assert.equal(markerCandidateA.sessionId, "session-a"); + assert.equal(markerCandidateA.reason, "active_task"); assert.equal(markerCandidateA.candidate, null); writeCurrentSessionMarker(testEnv, "session-c"); @@ -3378,7 +3570,7 @@ describe("claude-companion integration", () => { { env: testEnv.env } ); assert.equal(markerCandidateC.available, false); - assert.equal(markerCandidateC.sessionId, null); + assert.equal(markerCandidateC.sessionId, "session-c"); assert.equal(markerCandidateC.candidate, null); await waitForTerminalResult(testEnv, activeA.jobId, sessionAEnv); @@ -3451,7 +3643,7 @@ describe("claude-companion integration", () => { } }); - it("keeps an ownerless task visible without using the marker for resume", async () => { + it("uses the workspace marker as the owner for resume", async () => { const testEnv = createTestEnvironment(); try { writeCurrentSessionMarker(testEnv, "stale-marker-session"); @@ -3472,11 +3664,18 @@ describe("claude-companion integration", () => { "stale-marker-session" ); - const resume = runCompanionExpectFailure( - ["task", "--cwd", testEnv.workspaceDir, "--resume", "follow-up"], + const resume = runCompanion( + [ + "task", + "--cwd", + testEnv.workspaceDir, + "--resume", + "--quiet-progress", + "marker-follow-up delay=20", + ], { env: testEnv.env } ); - assert.match(resume.stderr, /Cannot resume without an owning Codex session/); + assert.match(resume.stdout, /completed:marker-follow-up/); } finally { cleanupTestEnvironment(testEnv); } @@ -4014,6 +4213,37 @@ describe("claude-companion integration", () => { } }); + it("accepts a read-only review terminal event with parser diagnostics", () => { + const testEnv = createTestEnvironment(); + + try { + setupGitWorkspace(testEnv.workspaceDir); + fs.writeFileSync( + path.join(testEnv.workspaceDir, "notes.md"), + "malformed-line review output\n", + "utf8" + ); + + const payload = runCompanionJson( + [ + "review", + "--cwd", + testEnv.workspaceDir, + "--scope", + "working-tree", + "--json", + ], + { env: testEnv.env } + ); + + assert.equal(payload.codex.status, "completed"); + assert.equal(payload.codex.parseErrors.length, 1); + assert.match(payload.codex.warning, /1 unrecovered parse error/); + } finally { + cleanupTestEnvironment(testEnv); + } + }); + it("accepts terminal structured_output for adversarial reviews when result text is empty", () => { const testEnv = createTestEnvironment(); @@ -4293,6 +4523,107 @@ describe("claude-companion integration", () => { } }); + it("cancels a foreground review when its companion receives SIGTERM", async () => { + const testEnv = createTestEnvironment(); + const sessionEnv = { + ...testEnv.env, + [SESSION_ID_ENV]: "session-foreground-review-signal", + }; + let observer = null; + let stderr = ""; + + try { + setupGitWorkspace(testEnv.workspaceDir); + fs.writeFileSync( + path.join(testEnv.workspaceDir, "app.js"), + "export function value() { return 'foreground review signal delay=900'; }\n", + "utf8" + ); + runGit(testEnv.workspaceDir, ["add", "app.js"]); + runGit(testEnv.workspaceDir, ["commit", "-m", "foreground review signal delay=900"]); + const reserved = runCompanionJson( + ["review-reserve-job", "--cwd", testEnv.workspaceDir, "--json"], + { env: sessionEnv } + ); + + observer = spawn( + process.execPath, + [ + COMPANION_SCRIPT, + "review", + "--cwd", + testEnv.workspaceDir, + "--job-id", + reserved.jobId, + "--base", + "HEAD~1", + "--json", + ], + { + cwd: PROJECT_ROOT, + env: sessionEnv, + stdio: ["ignore", "ignore", "pipe"], + } + ); + observer.stderr.on("data", (chunk) => { + stderr += String(chunk); + }); + const closed = new Promise((resolve) => { + observer.on("close", (code, signal) => resolve({ code, signal })); + }); + + let runningJob = null; + const deadline = Date.now() + 5_000; + while (Date.now() < deadline) { + runningJob = listStoredJobs(testEnv).find( + (job) => + job.id === reserved.jobId && + job.status === "running" && + typeof job.pid === "number" + ); + if (runningJob) { + break; + } + await sleep(25); + } + assert.ok(runningJob, "expected the foreground review to enter running state"); + + process.kill(observer.pid, "SIGTERM"); + const exit = await closed; + observer = null; + + assert.equal(exit.code, 143, stderr); + assert.equal(exit.signal, null); + const storedJob = readStoredJobById(testEnv, reserved.jobId); + assert.equal(storedJob.status, "cancelled"); + assert.equal(storedJob.pid, null); + assert.equal(storedJob.pidIdentity, null); + for (const subdir of ["sandbox", "mcp", "review-worktrees"]) { + const runtimeDir = path.join( + testEnv.homeDir, + ".codex", + "plugins", + "data", + "cc", + "runtime", + subdir + ); + assert.deepEqual( + fs.existsSync(runtimeDir) ? fs.readdirSync(runtimeDir) : [], + [], + `${subdir} should be empty after signal cleanup` + ); + } + } finally { + if (observer?.pid) { + try { + process.kill(observer.pid, "SIGKILL"); + } catch {} + } + cleanupTestEnvironment(testEnv); + } + }); + it("can reserve a task job id and reuse it for a foreground task", async () => { const testEnv = createTestEnvironment(); const sessionEnv = { diff --git a/tests/job-control.test.mjs b/tests/job-control.test.mjs index b42780d..381b57c 100644 --- a/tests/job-control.test.mjs +++ b/tests/job-control.test.mjs @@ -17,6 +17,7 @@ import { buildStatusSnapshot, buildSingleJobSnapshot, resolveResultJob, + resolveCancelableJob, DEFAULT_MAX_STATUS_JOBS, DEFAULT_MAX_PROGRESS_LINES, } from "../scripts/lib/job-control.mjs"; @@ -419,6 +420,73 @@ describe("enrichJob", () => { }); describe("buildSingleJobSnapshot", () => { + it("resolves exact job ids across workspace state roots", () => { + const sourceRepo = createTempGitRepo(); + const otherRepo = createTempGitRepo(); + const completedId = "task-global-completed-a1b2c3"; + const runningId = "task-global-running-d4e5f6"; + try { + writeJobFile(otherRepo, completedId, { + id: completedId, + status: "completed", + jobClass: "task", + workspaceRoot: otherRepo, + createdAt: "2026-04-03T10:00:00Z", + completedAt: "2026-04-03T10:01:00Z", + }); + writeJobFile(otherRepo, runningId, { + id: runningId, + status: "running", + jobClass: "task", + workspaceRoot: otherRepo, + createdAt: new Date().toISOString(), + }); + + const snapshot = buildSingleJobSnapshot(sourceRepo, completedId); + assert.equal(snapshot.workspaceRoot, otherRepo); + assert.equal(snapshot.job.id, completedId); + assert.equal(resolveResultJob(sourceRepo, completedId).workspaceRoot, otherRepo); + assert.equal(resolveCancelableJob(sourceRepo, runningId).workspaceRoot, otherRepo); + assert.throws( + () => resolveResultJob(sourceRepo, "task-global-completed"), + /No job found/ + ); + } finally { + fs.rmSync(resolveJobsDir(sourceRepo), { recursive: true, force: true }); + fs.rmSync(resolveJobsDir(otherRepo), { recursive: true, force: true }); + fs.rmSync(sourceRepo, { recursive: true, force: true }); + fs.rmSync(otherRepo, { recursive: true, force: true }); + } + }); + + it("rejects duplicate exact job ids across workspace state roots", () => { + const sourceRepo = createTempGitRepo(); + const firstRepo = createTempGitRepo(); + const secondRepo = createTempGitRepo(); + const jobId = "task-global-duplicate-a1b2c3"; + try { + for (const repoDir of [firstRepo, secondRepo]) { + writeJobFile(repoDir, jobId, { + id: jobId, + status: "completed", + jobClass: "task", + workspaceRoot: repoDir, + createdAt: "2026-04-03T10:00:00Z", + }); + } + + assert.throws( + () => buildSingleJobSnapshot(sourceRepo, jobId), + /exists in multiple workspaces/ + ); + } finally { + for (const repoDir of [sourceRepo, firstRepo, secondRepo]) { + fs.rmSync(resolveJobsDir(repoDir), { recursive: true, force: true }); + fs.rmSync(repoDir, { recursive: true, force: true }); + } + } + }); + it("resolves newest, exact, and unique-prefix references", () => { withTempJobRepo((repoDir) => { for (const [id, updatedAt] of [ diff --git a/tests/mutation-config.test.mjs b/tests/mutation-config.test.mjs index e41971f..635b4f1 100644 --- a/tests/mutation-config.test.mjs +++ b/tests/mutation-config.test.mjs @@ -21,16 +21,16 @@ 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-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/state.mjs:545-886", ["mostRecentJobTimestamp", "isWithinReapGracePeriod", "reapStaleJobs"]], + ["scripts/lib/state.mjs:935-1107", ["unlinkLockIfUnchanged", "remainingLockDeadlineMs", "lockProcessTimeout", "recoverStaleLock", "acquireJobLock", "releaseJobLock"]], + ["scripts/lib/state.mjs:1173-1230", ["casJobStatus", "transitionJob", "writeAtomic"]], + ["scripts/lib/state.mjs:1236-1282", ["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"]], + ["scripts/lib/tracked-jobs.mjs:363-517", ["runTrackedJob"]], + ["scripts/lib/job-control.mjs:175-338", ["matchJobReference", "buildStatusSnapshot", "resolveCancelableJob"]], + ["scripts/installer-cli.mjs:98-236", ["readPersonalMarketplace", "prepareLegacyLocalCleanup", "isPluginAlreadyAbsent", "isPluginUninstallRefused"]], + ["scripts/installer-cli.mjs:277-377", ["installOrUpdate", "uninstall"]], ]; test("mutation line ranges still contain their intended complete functions", () => { diff --git a/tests/plugin-identity.test.mjs b/tests/plugin-identity.test.mjs index 7405cd5..f983cf0 100644 --- a/tests/plugin-identity.test.mjs +++ b/tests/plugin-identity.test.mjs @@ -13,10 +13,19 @@ import { getPreferredMarketplaceName, listManagedPluginCacheEntries, parseManagedPluginSections, + pluginDataNamespaceForMarketplace, pluginConfigHeader, pluginIdForMarketplace, } from "../scripts/lib/plugin-identity.mjs"; +it("pluginDataNamespaceForMarketplace matches the Codex host namespace", () => { + assert.equal(pluginDataNamespaceForMarketplace("cbepx"), "cc-cbepx"); + assert.throws( + () => pluginDataNamespaceForMarketplace("../outside"), + /Invalid marketplace name/u + ); +}); + const tempDirs = []; afterEach(() => { diff --git a/tests/skills-contracts.test.mjs b/tests/skills-contracts.test.mjs index 7680271..7135a77 100644 --- a/tests/skills-contracts.test.mjs +++ b/tests/skills-contracts.test.mjs @@ -304,8 +304,8 @@ test("setup keeps native hook repair in the companion flow", () => { [ 'claude-companion.mjs" setup --check --json', "`--check` is read-only", + "stable hook launcher", "[features].hooks", - "[features].plugin_hooks", "native hook trust hashes", ], "setup" diff --git a/tests/state.test.mjs b/tests/state.test.mjs index d5eb8f5..06bd3ee 100644 --- a/tests/state.test.mjs +++ b/tests/state.test.mjs @@ -1635,6 +1635,8 @@ describe("reapStaleJobs", () => { assert.equal(result.length, 1); assert.equal(result[0].status, "failed"); assert.ok(result[0].errorMessage.includes("Auto-reaped")); + assert.equal(result[0].reapedBy, "status-reaper"); + assert.equal(result[0].reapReason, "process-missing"); assert.equal(result[0].pid, null); assert.equal(result[0].pidIdentity, null); assert.ok(result[0].completedAt); diff --git a/tests/tracked-jobs.test.mjs b/tests/tracked-jobs.test.mjs index c9c5193..2cc1f72 100644 --- a/tests/tracked-jobs.test.mjs +++ b/tests/tracked-jobs.test.mjs @@ -458,6 +458,37 @@ describe("runTrackedJob", () => { } }); + it("replaces a parent-captured worker identity with the worker's own identity", async () => { + const repoDir = createTempGitRepo(); + const job = { + id: "tracked-worker-self-identity", + workspaceRoot: repoDir, + status: "queued", + title: "worker self identity", + workerPid: process.pid, + workerPidIdentity: "parent-captured-identity", + createdAt: nowIso(), + updatedAt: nowIso(), + }; + writeJobFile(repoDir, job.id, job); + + try { + await runTrackedJob( + job, + async () => { + assert.equal( + readJobFile(repoDir, job.id).workerPidIdentity, + "worker-self-identity" + ); + return { exitStatus: 0, rendered: "finished" }; + }, + { getSpawnedProcessIdentityImpl: () => "worker-self-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 = { @@ -478,7 +509,7 @@ describe("runTrackedJob", () => { assert.equal(starting.workerPid, process.pid); assert.equal( starting.workerPidIdentity, - "parent-captured-worker-identity" + "worker-self-identity" ); onSpawn({ pid: 43210, pidIdentity: "claude-identity" }); @@ -495,7 +526,7 @@ describe("runTrackedJob", () => { rendered: "finished", summary: "finished", }; - }); + }, { getSpawnedProcessIdentityImpl: () => "worker-self-identity" }); const terminal = readJobFile(repoDir, job.id); assert.equal(terminal.status, "completed"); @@ -664,6 +695,49 @@ describe("runTrackedJob", () => { } }); + it("persists a late result after an ordinary status reaper failure", async () => { + const repoDir = createTempGitRepo(); + const job = { + id: "tracked-ordinary-reaper-result-job", + workspaceRoot: repoDir, + status: "queued", + title: "late ordinary reaper result", + createdAt: nowIso(), + updatedAt: nowIso(), + }; + writeJobFile(repoDir, job.id, job); + + try { + await runTrackedJob(job, async () => { + const running = readJobFile(repoDir, job.id); + writeJobFile(repoDir, job.id, { + ...running, + status: "failed", + errorMessage: "Worker died without completing. Auto-reaped.", + reapedBy: "status-reaper", + reapReason: "process-missing", + updatedAt: nowIso(), + }); + return { + exitStatus: 0, + threadId: "thread-late-ordinary", + payload: { answer: 43 }, + rendered: "finished after reap", + summary: "finished after reap", + }; + }); + + const finalJob = readJobFile(repoDir, job.id); + assert.equal(finalJob.status, "completed"); + assert.deepEqual(finalJob.result, { answer: 43 }); + assert.equal(finalJob.errorMessage, null); + assert.equal(finalJob.reapedBy, null); + assert.equal(finalJob.reapReason, null); + } finally { + fs.rmSync(repoDir, { recursive: true, force: true }); + } + }); + it("persists an ordinary runner failure without treating it as lock contention", async () => { const repoDir = createTempGitRepo(); const job = {