From 7f928c40f386ae64a601376583c093df8ab17dc8 Mon Sep 17 00:00:00 2001 From: jinku Date: Wed, 29 Jul 2026 22:19:18 -0700 Subject: [PATCH] release: v1.4.2 Stop the Claude Code <-> Codex delegation loop for users running both bridge plugins. A Codex app-server spawned from inside Claude Code inherits ~/.codex, so its headless review threads read this plugin's skills and delegated the review back to Claude Code, spinning on the wait tool for minutes and returning narration instead of findings. Session hooks now stamp hostOrigin on the current-session marker when Claude Code host env markers reach them, and review/adversarial-review/ task refuse delegation from such threads with instructions to do the work directly. The review skills stop naming AskUserQuestion (a Claude Code tool Codex lacks) and proceed with the recommended mode headless, and the turn-end review gate skips sessions with no recorded user turn. Co-Authored-By: Claude Fable 5 --- .codex-plugin/plugin.json | 2 +- CHANGELOG.md | 6 ++ hooks/lib/host-origin.mjs | 22 +++++ hooks/session-lifecycle-hook.mjs | 5 +- hooks/stop-review-gate-hook.mjs | 14 ++- hooks/unread-result-hook.mjs | 8 +- package-lock.json | 4 +- package.json | 2 +- scripts/claude-companion.mjs | 33 +++++++ scripts/lib/state.mjs | 23 ++++- skills/adversarial-review/SKILL.md | 3 +- skills/review/SKILL.md | 3 +- tests/hooks.test.mjs | 94 ++++++++++++++++++ tests/integration/claude-companion.test.mjs | 101 +++++++++++++++++++- tests/skills-contracts.test.mjs | 29 ++++++ 15 files changed, 332 insertions(+), 17 deletions(-) create mode 100644 hooks/lib/host-origin.mjs diff --git a/.codex-plugin/plugin.json b/.codex-plugin/plugin.json index 516d818..32d565b 100644 --- a/.codex-plugin/plugin.json +++ b/.codex-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "cc", - "version": "1.4.1", + "version": "1.4.2", "description": "Claude Code Plugin for Codex. Delegate code reviews, investigations, and tracked tasks to Claude Code from inside Codex.", "author": { "name": "Sendbird, Inc.", diff --git a/CHANGELOG.md b/CHANGELOG.md index 93e70f7..f1d120b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## v1.4.2 + +- Refuse companion delegation from Codex threads that are themselves driven by Claude Code. The reverse-direction plugin (Claude Code → Codex) spawns a bare `codex app-server` that inherits `~/.codex`, so its headless review threads see this plugin's skills and delegated the review back to Claude Code — looping the work between the two assistants and burning minutes on `wait`-tool spins with narration in place of findings. Session hooks now stamp `hostOrigin: "claude-code"` on the current-session marker when Claude Code host env markers (`CLAUDECODE` / `CLAUDE_CODE_ENTRYPOINT`) reach them, and `review`, `adversarial-review`, and `task` refuse delegation from such threads with explicit instructions to perform the work directly in that thread. Interactive sessions (env session id present), background forwarding children owned by a different session, and unstamped state all stay open, so the gate fails open everywhere the loop cannot occur. +- Stop naming `AskUserQuestion` — a Claude Code tool that Codex does not have — in the review skills' execution-mode ask. Codex's own `request_user_input` is gated behind `[tools] experimental_request_user_input` and does not exist in non-interactive threads, so the skills now use a question tool only when the thread actually has one, ask inline when a user is reading, and proceed with the recommended mode in headless threads instead of spinning on a collaboration tool looking for a picker that cannot appear. +- Skip the turn-end review gate when no turn baseline was recorded for the session. The baseline is written on `UserPromptSubmit`, so its absence means no user prompt drove the session — for example an externally hosted headless thread — and there is no turn to review. Previously the gate treated a missing baseline as a signal to run the full Claude review. + ## v1.4.1 - Stop defaulting reasoning effort per model. v1.4.0 gave every friendly alias a `high` default, which duplicated a catalog that belongs to the host CLI — exactly like the pinned model IDs removed in that same release — and was wrong for `haiku`, since Haiku 4.5 is not in the reasoning-effort model tier. `--effort` is now forwarded only when you pass it, so each model keeps whatever effort Claude Code defaults to and Claude Code stays the authority on which levels a model supports. `--model` still defaults to `opus`. Users who relied on the v1.2.0 `opus` + `xhigh` behavior should pass `--effort xhigh` explicitly. diff --git a/hooks/lib/host-origin.mjs b/hooks/lib/host-origin.mjs new file mode 100644 index 0000000..2459a8e --- /dev/null +++ b/hooks/lib/host-origin.mjs @@ -0,0 +1,22 @@ +/** + * Copyright 2026 Sendbird, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ +import process from "node:process"; + +/** + * Detect whether this Codex session is hosted by an external assistant rather + * than a user-driven Codex frontend. A Codex app-server spawned from inside + * Claude Code inherits the Claude Code process env (measured: CLAUDECODE=1 / + * CLAUDE_CODE_ENTRYPOINT reach plugin hooks). Threads in such an app-server + * are host-driven, so companion delegation must not loop back to Claude Code. + * + * Every writer of the current-session marker must stamp this, or a later + * rewrite would erase the origin and reopen the delegation loop. + */ +export function detectExternalHostOrigin() { + if (process.env.CLAUDECODE || process.env.CLAUDE_CODE_ENTRYPOINT) { + return "claude-code"; + } + return null; +} diff --git a/hooks/session-lifecycle-hook.mjs b/hooks/session-lifecycle-hook.mjs index 306efe2..62cd3cc 100644 --- a/hooks/session-lifecycle-hook.mjs +++ b/hooks/session-lifecycle-hook.mjs @@ -19,6 +19,7 @@ import process from "node:process"; import { fileURLToPath } from "node:url"; import { readHookInput } from "./lib/hook-input.mjs"; +import { detectExternalHostOrigin } from "./lib/host-origin.mjs"; import { cleanupAfterOfficialUninstall } from "./lib/plugin-install-guard.mjs"; import { setCurrentSession } from "../scripts/lib/state.mjs"; import { SESSION_ID_ENV } from "../scripts/lib/tracked-jobs.mjs"; @@ -61,7 +62,9 @@ function handleSessionStart(input) { // Forward plugin data dir if set appendEnvVar(PLUGIN_DATA_ENV, process.env[PLUGIN_DATA_ENV]); if (input.session_id && !nestedSession) { - setCurrentSession(cwd, input.session_id); + setCurrentSession(cwd, input.session_id, { + hostOrigin: detectExternalHostOrigin(), + }); } } diff --git a/hooks/stop-review-gate-hook.mjs b/hooks/stop-review-gate-hook.mjs index 6bedb54..2621fb7 100644 --- a/hooks/stop-review-gate-hook.mjs +++ b/hooks/stop-review-gate-hook.mjs @@ -53,6 +53,8 @@ const SKIP_INTERACTIVE_HOOKS_ENV = "CLAUDE_COMPANION_SKIP_INTERACTIVE_HOOKS"; const STOP_REVIEW_SUCCESS_NOTE = "Claude Code turn-end review passed."; const STOP_REVIEW_NO_EDIT_NOTE = "Claude Code turn-end review skipped: the most recent turn made no net edits."; +const STOP_REVIEW_NO_BASELINE_NOTE = + "Claude Code turn-end review skipped: no user turn was recorded for this Codex session."; const MAX_INLINE_REASON_CHARS = 1_500; function emitDecision(payload) { @@ -290,8 +292,14 @@ function evaluateTurnEditGate(cwd, workspaceRoot, sessionId) { const baseline = readTurnBaseline(workspaceRoot, sessionId); if (!baseline?.fingerprint) { + // The baseline is written by UserPromptSubmit, so its absence means no user + // prompt drove this Codex session and there is no turn to review. Reachable + // when another host drives Codex headlessly, e.g. a Claude Code review + // thread that inherits this plugin. return { - shouldSkipReview: false, + shouldSkipReview: true, + skipStatus: "skipped_no_turn_baseline", + skipNote: STOP_REVIEW_NO_BASELINE_NOTE, reason: "No turn baseline was recorded for this session.", baseline, current: null, @@ -393,13 +401,13 @@ async function main() { }; if (turnEditGate.shouldSkipReview) { persistFinal({ - status: "skipped_no_turn_edits", + status: turnEditGate.skipStatus ?? "skipped_no_turn_edits", reason: turnEditGate.reason, claudeInvoked: false, runningTaskNote, ...fingerprintFields, }); - logNote(STOP_REVIEW_NO_EDIT_NOTE); + logNote(turnEditGate.skipNote ?? STOP_REVIEW_NO_EDIT_NOTE); logNote(runningTaskNote); return; } diff --git a/hooks/unread-result-hook.mjs b/hooks/unread-result-hook.mjs index 819fe3c..61c7105 100644 --- a/hooks/unread-result-hook.mjs +++ b/hooks/unread-result-hook.mjs @@ -10,6 +10,7 @@ import path from "node:path"; import { fileURLToPath } from "node:url"; import { readHookInput } from "./lib/hook-input.mjs"; +import { detectExternalHostOrigin } from "./lib/host-origin.mjs"; import { cleanupAfterOfficialUninstall } from "./lib/plugin-install-guard.mjs"; import { getConfig, @@ -112,7 +113,8 @@ function captureTurnBaseline(workspaceRoot, sessionId, cwd) { fingerprint, }); } catch { - // Baseline capture is best-effort. If it fails, Stop falls back to running review. + // Baseline capture is best-effort. If it fails, Stop skips the review for + // this turn rather than reviewing a turn it cannot delimit. } } @@ -134,7 +136,9 @@ async function main() { } try { - setCurrentSession(workspaceRoot, sessionId); + setCurrentSession(workspaceRoot, sessionId, { + hostOrigin: detectExternalHostOrigin(), + }); } catch { // Best effort only: an invalid session id should not fail a user prompt. } diff --git a/package-lock.json b/package-lock.json index b533376..9215c96 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "cc-plugin-codex", - "version": "1.4.0", + "version": "1.4.2", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "cc-plugin-codex", - "version": "1.4.0", + "version": "1.4.2", "license": "Apache-2.0", "bin": { "cc-plugin-codex": "scripts/installer-cli.mjs" diff --git a/package.json b/package.json index 1f04275..0608d75 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "cc-plugin-codex", - "version": "1.4.1", + "version": "1.4.2", "description": "Claude Code Plugin for Codex by Sendbird", "type": "module", "author": { diff --git a/scripts/claude-companion.mjs b/scripts/claude-companion.mjs index 0124e33..179f834 100644 --- a/scripts/claude-companion.mjs +++ b/scripts/claude-companion.mjs @@ -78,6 +78,7 @@ import { generateJobId, getConfig, getCurrentSession, + getCurrentSessionMarker, listJobs, patchJob, JOB_RESERVATION_SUFFIX, @@ -246,6 +247,36 @@ function alignCurrentSessionToOwner(workspaceRoot, ownerSessionId) { setCurrentSession(workspaceRoot, ownerSessionId); } +/** + * Refuse delegation from a Codex thread that is itself driven by an external + * host (e.g. a headless review thread spawned by Claude Code). Delegating back + * to Claude Code from there loops the work between the two assistants. + * + * Interactive Codex sessions receive SESSION_ID_ENV through the session hook's + * env-file export; externally hosted app-server threads do not (measured), so + * an absent env session id plus a `hostOrigin` stamp on the current-session + * marker identifies the loop. Fail-open everywhere else. + */ +function assertDelegationAllowed(workspaceRoot, ownerSessionId, workLabel) { + if (process.env[SESSION_ID_ENV]) { + return; + } + const marker = getCurrentSessionMarker(workspaceRoot); + if (!marker || marker.hostOrigin !== "claude-code") { + return; + } + if (ownerSessionId && ownerSessionId !== marker.sessionId) { + return; + } + throw new Error( + [ + `This Codex thread is driven by Claude Code, not by a user prompt, so delegating this ${workLabel} back to Claude Code would loop it between the two assistants.`, + `Do not retry this command and do not look for another way to reach Claude Code.`, + `Perform the requested ${workLabel} yourself in this thread and present your own findings directly.`, + ].join("\n") + ); +} + async function withReleasedReservation(workspaceRoot, explicitJobId, fn) { try { return await fn(); @@ -1515,6 +1546,7 @@ async function handleReviewCommand(argv, config) { await withReleasedReservation(workspaceRoot, explicitJobId, async () => { // Validate inside the reservation guard so failures do not leak markers. config.validateRequest?.(target, focusText); + assertDelegationAllowed(workspaceRoot, ownerSessionId, "review"); const metadata = buildReviewJobMetadata(config.reviewName, target); alignCurrentSessionToOwner(workspaceRoot, ownerSessionId); @@ -1634,6 +1666,7 @@ async function handleTask(argv) { const ownerSessionId = resolveOwnerSessionId(options["owner-session-id"]); const explicitJobId = resolveExplicitJobId(options["job-id"], workspaceRoot); await withReleasedReservation(workspaceRoot, explicitJobId, async () => { + assertDelegationAllowed(workspaceRoot, ownerSessionId, "task"); const taskMetadata = buildTaskRunMetadata({ prompt, resumeLast diff --git a/scripts/lib/state.mjs b/scripts/lib/state.mjs index 065ef3a..e51457b 100644 --- a/scripts/lib/state.mjs +++ b/scripts/lib/state.mjs @@ -267,16 +267,17 @@ export function getConfig(cwd) { // Current session marker (fallback when Codex does not propagate env vars) // --------------------------------------------------------------------------- -export function setCurrentSession(cwd, sessionId) { +export function setCurrentSession(cwd, sessionId, options = {}) { sanitizeId(sessionId, "session ID"); ensureStateDir(cwd); writeAtomic(resolveCurrentSessionFile(cwd), { sessionId, + ...(options.hostOrigin ? { hostOrigin: String(options.hostOrigin) } : {}), updatedAt: nowIso(), }); } -export function getCurrentSession(cwd) { +function readCurrentSessionPayload(cwd) { const filePath = resolveCurrentSessionFile(cwd); try { const payload = JSON.parse(fs.readFileSync(filePath, "utf8")); @@ -290,12 +291,28 @@ export function getCurrentSession(cwd) { fs.unlinkSync(filePath); return null; } - return sanitizeId(payload.sessionId, "session ID"); + sanitizeId(payload.sessionId, "session ID"); + return payload; } catch { return null; } } +export function getCurrentSession(cwd) { + return readCurrentSessionPayload(cwd)?.sessionId ?? null; +} + +export function getCurrentSessionMarker(cwd) { + const payload = readCurrentSessionPayload(cwd); + if (!payload) { + return null; + } + return { + sessionId: payload.sessionId, + hostOrigin: typeof payload.hostOrigin === "string" ? payload.hostOrigin : null, + }; +} + export function clearCurrentSession(cwd, sessionId = null) { const filePath = resolveCurrentSessionFile(cwd); if (sessionId != null) { diff --git a/skills/adversarial-review/SKILL.md b/skills/adversarial-review/SKILL.md index c5dd7cc..546a747 100644 --- a/skills/adversarial-review/SKILL.md +++ b/skills/adversarial-review/SKILL.md @@ -41,9 +41,10 @@ Execution mode rules: - Recommend waiting only when the scoped review is clearly tiny, roughly 1-2 files total and no sign of a broader directory-sized change. - In every other case, including unclear size, recommend background. - When in doubt, run the review instead of declaring that there is nothing to review. -- Then use `AskUserQuestion` exactly once with two options, putting the recommended option first and suffixing its label with `(Recommended)`: +- Then ask the user once which execution mode to use, offering two options with the recommended one first and its label suffixed `(Recommended)`: - `Wait for results` - `Run in background` +- Use a question tool for that ask only when this thread actually has one. Codex exposes `request_user_input` only behind `[tools] experimental_request_user_input`, and it does not exist in non-interactive threads. If you have no question tool but a user is reading this thread, ask in your own reply and stop there. In a non-interactive thread with no user to answer, skip the ask and proceed with the recommended mode. Never spin on a wait or collaboration tool looking for a picker this thread does not have. Argument handling: - Preserve the user's arguments exactly. diff --git a/skills/review/SKILL.md b/skills/review/SKILL.md index 66c8ecd..fbc8b00 100644 --- a/skills/review/SKILL.md +++ b/skills/review/SKILL.md @@ -42,9 +42,10 @@ Execution mode rules: - Recommend waiting only when the review is clearly tiny, roughly 1-2 files total and no sign of a broader directory-sized change. - In every other case, including unclear size, recommend background. - When in doubt, run the review instead of declaring that there is nothing to review. -- Then use `AskUserQuestion` exactly once with two options, putting the recommended option first and suffixing its label with `(Recommended)`: +- Then ask the user once which execution mode to use, offering two options with the recommended one first and its label suffixed `(Recommended)`: - `Wait for results` - `Run in background` +- Use a question tool for that ask only when this thread actually has one. Codex exposes `request_user_input` only behind `[tools] experimental_request_user_input`, and it does not exist in non-interactive threads. If you have no question tool but a user is reading this thread, ask in your own reply and stop there. In a non-interactive thread with no user to answer, skip the ask and proceed with the recommended mode. Never spin on a wait or collaboration tool looking for a picker this thread does not have. Argument handling: - Preserve the user's arguments exactly. diff --git a/tests/hooks.test.mjs b/tests/hooks.test.mjs index facf1f3..3412826 100644 --- a/tests/hooks.test.mjs +++ b/tests/hooks.test.mjs @@ -307,6 +307,10 @@ function writeTurnBaselineSnapshot(testEnv, sessionId, fingerprint) { ); } +function writeStaleTurnBaseline(testEnv, sessionId) { + writeTurnBaselineSnapshot(testEnv, sessionId, { signature: "stale-baseline" }); +} + describe("hooks", () => { it("native plugin hook events stay within upstream Codex hook event names", () => { const upstreamHookEventNames = new Set([ @@ -562,6 +566,50 @@ describe("hooks", () => { } }); + it("stop-review hook skips Claude when no user turn was recorded for the session", () => { + const testEnv = createHookEnvironment(); + + try { + const stateDir = stateDirFor(testEnv.homeDir, testEnv.workspaceDir); + fs.mkdirSync(stateDir, { recursive: true }); + fs.writeFileSync( + path.join(stateDir, "config.json"), + JSON.stringify({ version: 1, stopReviewGate: true }, null, 2) + "\n", + "utf8" + ); + + // No turn-baseline snapshot: UserPromptSubmit never ran for this session, + // which is what a headless Codex thread driven by another host looks like. + const argsFile = path.join(testEnv.rootDir, "claude-args.json"); + const result = runHook( + STOP_HOOK, + [], + { + cwd: testEnv.workspaceDir, + session_id: "headless-session", + last_assistant_message: "review me", + }, + { + ...testEnv.env, + CLAUDE_ARGS_FILE: argsFile, + } + ); + + assert.equal(result.stdout.trim(), ""); + assert.match(result.stderr, /no user turn was recorded/i); + assert.ok( + !fs.existsSync(argsFile), + "a session with no recorded user turn should skip Claude invocation" + ); + + const snapshot = readStopReviewSnapshot(testEnv); + assert.equal(snapshot.status, "skipped_no_turn_baseline"); + assert.equal(snapshot.claudeInvoked, false); + } finally { + cleanupHookEnvironment(testEnv); + } + }); + it("unread-result hook reaps stale running jobs on UserPromptSubmit", () => { const testEnv = createHookEnvironment(); @@ -674,6 +722,46 @@ describe("hooks", () => { } }); + it("session start stamps hostOrigin when the app-server was spawned under Claude Code", () => { + const testEnv = createHookEnvironment(); + + try { + const env = { ...testEnv.env, CLAUDECODE: "1" }; + delete env.CLAUDE_COMPANION_SESSION_ID; + runHook( + SESSION_HOOK, + [], + { cwd: testEnv.workspaceDir, session_id: "cc-thread" }, + env + ); + assert.equal(readCurrentSessionMarker(testEnv).hostOrigin, "claude-code"); + } finally { + cleanupHookEnvironment(testEnv); + } + }); + + it("session start leaves hostOrigin unset for plain Codex sessions", () => { + const testEnv = createHookEnvironment(); + + try { + const env = { ...testEnv.env }; + delete env.CLAUDECODE; + delete env.CLAUDE_CODE_ENTRYPOINT; + delete env.CLAUDE_COMPANION_SESSION_ID; + runHook( + SESSION_HOOK, + [], + { cwd: testEnv.workspaceDir, session_id: "plain-session" }, + env + ); + const marker = readCurrentSessionMarker(testEnv); + assert.equal(marker.sessionId, "plain-session"); + assert.equal("hostOrigin" in marker, false); + } finally { + cleanupHookEnvironment(testEnv); + } + }); + it("stop-review hook blocks unknown Claude completion states even if partial output looks like ALLOW", () => { const testEnv = createHookEnvironment(); @@ -685,6 +773,7 @@ describe("hooks", () => { JSON.stringify({ version: 1, stopReviewGate: true }, null, 2) + "\n", "utf8" ); + writeStaleTurnBaseline(testEnv, "hook-session"); const result = runHook( STOP_HOOK, @@ -729,6 +818,7 @@ describe("hooks", () => { JSON.stringify({ version: 1, stopReviewGate: true }, null, 2) + "\n", "utf8" ); + writeStaleTurnBaseline(testEnv, "hook-session"); const result = runHook( STOP_HOOK, @@ -776,6 +866,7 @@ describe("hooks", () => { JSON.stringify({ version: 1, stopReviewGate: true }, null, 2) + "\n", "utf8" ); + writeStaleTurnBaseline(testEnv, "hook-session"); const result = runHook( STOP_HOOK, @@ -816,6 +907,7 @@ describe("hooks", () => { JSON.stringify({ version: 1, stopReviewGate: true }, null, 2) + "\n", "utf8" ); + writeStaleTurnBaseline(testEnv, "hook-session"); const result = runHook( STOP_HOOK, @@ -859,6 +951,7 @@ describe("hooks", () => { JSON.stringify({ version: 1, stopReviewGate: true }, null, 2) + "\n", "utf8" ); + writeStaleTurnBaseline(testEnv, "hook-session"); const result = runHook( STOP_HOOK, @@ -898,6 +991,7 @@ describe("hooks", () => { JSON.stringify({ version: 1, stopReviewGate: true }, null, 2) + "\n", "utf8" ); + writeStaleTurnBaseline(testEnv, "hook-session"); writeStateJob(testEnv, "running-review-job", { id: "running-review-job", status: "running", diff --git a/tests/integration/claude-companion.test.mjs b/tests/integration/claude-companion.test.mjs index d7b70e9..5e808ba 100644 --- a/tests/integration/claude-companion.test.mjs +++ b/tests/integration/claude-companion.test.mjs @@ -352,7 +352,7 @@ function writeSessionScopedJob(testEnv, jobId, payload) { return { stateDir, jobsDir }; } -function writeCurrentSessionMarker(testEnv, sessionId) { +function writeCurrentSessionMarker(testEnv, sessionId, options = {}) { const realWorkspace = fs.realpathSync.native(testEnv.workspaceDir); const workspaceHash = createHash("sha256").update(realWorkspace).digest("hex").slice(0, 12); const stateDir = path.join( @@ -367,7 +367,15 @@ function writeCurrentSessionMarker(testEnv, sessionId) { fs.mkdirSync(stateDir, { recursive: true }); fs.writeFileSync( path.join(stateDir, "current-session.json"), - JSON.stringify({ sessionId, updatedAt: new Date().toISOString() }, null, 2) + "\n", + JSON.stringify( + { + sessionId, + ...(options.hostOrigin ? { hostOrigin: options.hostOrigin } : {}), + updatedAt: new Date().toISOString(), + }, + null, + 2 + ) + "\n", "utf8" ); } @@ -1182,6 +1190,95 @@ describe("claude-companion integration", () => { } }); + it("refuses review delegation from a Claude-Code-driven thread and tells it to review directly", () => { + const testEnv = createTestEnvironment(); + + try { + setupGitWorkspace(testEnv.workspaceDir); + seedWorkingTreeDiff(testEnv.workspaceDir); + writeCurrentSessionMarker(testEnv, "cc-thread", { hostOrigin: "claude-code" }); + + const env = { ...testEnv.env }; + delete env[SESSION_ID_ENV]; + + const result = runCompanionExpectFailure( + ["review", "--cwd", testEnv.workspaceDir, "--scope", "working-tree"], + { env } + ); + + assert.match(result.stderr, /driven by Claude Code/); + assert.match(result.stderr, /Perform the requested review yourself/); + assert.equal(listStoredJobs(testEnv).length, 0); + + const taskResult = runCompanionExpectFailure( + ["task", "--cwd", testEnv.workspaceDir, "investigate something"], + { env } + ); + assert.match(taskResult.stderr, /Perform the requested task yourself/); + assert.equal(listStoredJobs(testEnv).length, 0); + } finally { + cleanupTestEnvironment(testEnv); + } + }); + + it("keeps delegation open for interactive sessions, other owners, and unstamped state", () => { + const testEnv = createTestEnvironment(); + + try { + setupGitWorkspace(testEnv.workspaceDir); + seedWorkingTreeDiff(testEnv.workspaceDir); + writeCurrentSessionMarker(testEnv, "cc-thread", { hostOrigin: "claude-code" }); + + // Interactive Codex session: the env-file export is present. + runCompanion( + ["review", "--cwd", testEnv.workspaceDir, "--scope", "working-tree"], + { env: { ...testEnv.env, [SESSION_ID_ENV]: "cc-thread" } } + ); + + // Background forwarding child owned by a different (interactive) parent. + writeCurrentSessionMarker(testEnv, "cc-thread", { hostOrigin: "claude-code" }); + const noEnv = { ...testEnv.env }; + delete noEnv[SESSION_ID_ENV]; + runCompanion( + [ + "review", + "--cwd", + testEnv.workspaceDir, + "--scope", + "working-tree", + "--owner-session-id", + "interactive-parent", + ], + { env: noEnv } + ); + + // Same owner as the externally driven thread stays refused. + writeCurrentSessionMarker(testEnv, "cc-thread", { hostOrigin: "claude-code" }); + const refused = runCompanionExpectFailure( + [ + "review", + "--cwd", + testEnv.workspaceDir, + "--scope", + "working-tree", + "--owner-session-id", + "cc-thread", + ], + { env: noEnv } + ); + assert.match(refused.stderr, /driven by Claude Code/); + + // Unstamped current-session state fails open. + writeCurrentSessionMarker(testEnv, "plain-session"); + runCompanion( + ["review", "--cwd", testEnv.workspaceDir, "--scope", "working-tree"], + { env: noEnv } + ); + } finally { + cleanupTestEnvironment(testEnv); + } + }); + it("sends a Windows-sized review prompt through stdin instead of argv", () => { const testEnv = createTestEnvironment(); diff --git a/tests/skills-contracts.test.mjs b/tests/skills-contracts.test.mjs index 3c681ac..55c5778 100644 --- a/tests/skills-contracts.test.mjs +++ b/tests/skills-contracts.test.mjs @@ -452,3 +452,32 @@ test("simple runtime skills resolve the active plugin root from the skill path", assert.doesNotMatch(skillText, //i); } }); + +test("review skills never hard-require a question tool the thread may not have", () => { + const contracts = ["skills/review/SKILL.md", "skills/adversarial-review/SKILL.md"]; + + for (const contractPath of contracts) { + const contract = read(contractPath); + // AskUserQuestion is a Claude Code tool; Codex has no tool by that name. + assert.doesNotMatch( + contract, + /AskUserQuestion/, + `${contractPath} must not name a Claude Code tool as Codex's question tool` + ); + assert.match( + contract, + /request_user_input/, + `${contractPath} must name Codex's own question tool` + ); + assert.match( + contract, + /only when this thread actually has one/i, + `${contractPath} must make the ask conditional on that tool existing` + ); + assert.match( + contract, + /does not exist in non-interactive threads/i, + `${contractPath} must state that the question tool is absent headless` + ); + } +});