From c9e7cd8a641c18a0015ec95430d1582a2ec87f45 Mon Sep 17 00:00:00 2001 From: Abdul Wahab Date: Mon, 24 Aug 2026 05:23:51 +0500 Subject: [PATCH] Fix: emit Codex Stop hook as decision/block (#603) Codex Stop rejects Claude's hookSpecificOutput shape. Detect Codex from turn_id at runtime and emit { decision: "block", reason } so existing installs keep working without rewriting hook commands. AI-assisted change, prepared with Cursor Grok under maintainer direction. Fixes #603 Fixes #643 Co-authored-by: Cursor --- skill/scripts/hook-lib.mjs | 49 +++++++++++++++++------- skill/scripts/hook.mjs | 2 +- tests/hook.test.mjs | 77 +++++++++++++++++++++++++++++++++++++- 3 files changed, 113 insertions(+), 15 deletions(-) diff --git a/skill/scripts/hook-lib.mjs b/skill/scripts/hook-lib.mjs index 794ac59a1..0a2d827e7 100644 --- a/skill/scripts/hook-lib.mjs +++ b/skill/scripts/hook-lib.mjs @@ -1251,7 +1251,8 @@ export function resolveHarness(env = {}, event = null) { const explicit = env?.IMPECCABLE_HOOK_HARNESS; if (explicit === 'cursor') return 'cursor'; if (explicit === 'github') return 'github'; - if (explicit === 'claude' || explicit === 'codex') return 'claude'; + if (explicit === 'claude') return 'claude'; + if (explicit === 'codex') return 'codex'; // GitHub Copilot's postToolUse event uses camelCase `toolName`/`toolArgs` and // has no `tool_name`/`tool_input`. That shape is the discriminator. if (event && typeof event === 'object' @@ -1260,6 +1261,11 @@ export function resolveHarness(env = {}, event = null) { return 'github'; } if (typeof event?.conversation_id === 'string' && event.conversation_id) return 'cursor'; + // Codex turn-scoped events carry `turn_id`. Claude Code does not. Detecting + // it here means an already-installed Codex hook emits the Codex Stop + // contract without rewriting the hook command to set IMPECCABLE_HOOK_HARNESS. + // https://developers.openai.com/codex/hooks#stop + if (typeof event?.turn_id === 'string' && event.turn_id) return 'codex'; return 'claude'; } @@ -2163,8 +2169,11 @@ export const STOP_MAX_FILES = 20; * { exitCode, stdout, audit, emission? } * * Never throws; exits silent (and fast) when the session touched no UI - * files. Output uses the Stop hookSpecificOutput channel: additionalContext - * is delivered to the model and the conversation continues so it can act. + * files. Output goes out on the harness's Stop continuation channel: Claude + * Code reads hookSpecificOutput.additionalContext, Codex takes a + * decision: "block" whose reason becomes the continuation prompt. Either + * way the findings reach the model and the conversation continues so it + * can act. */ export async function runStopHook({ stdinJson, env = {}, cwd = process.cwd(), now = Date.now, detector } = {}) { const audit = { ts: new Date(now()).toISOString(), event: 'Stop' }; @@ -2191,16 +2200,21 @@ export async function runStopHook({ stdinJson, env = {}, cwd = process.cwd(), no return result({ skipped: 'stdin-empty', durationMs: Date.now() - started }); } - // Claude Code's Stop-hook contract: `stop_hook_active` is true when this - // hook is being re-invoked only because a prior invocation kept the turn - // alive (here, via hookSpecificOutput.additionalContext). Re-scanning and - // re-blocking now would loop until Claude Code's consecutive-block cap - // force-ends the turn (issue #400). The prior fire already surfaced the - // findings; whether to act on them is the agent's call. Exit fast with no - // output before any scan. Only Claude Code sends this field; other - // harnesses omit it, so the strict `=== true` is a no-op for them. This - // guard makes the loop impossible regardless of the finding cache key's - // line-number sensitivity (out of scope here; see findingCacheKey). + // Stop-hook re-entry guard: `stop_hook_active` is true when this hook is + // being re-invoked only because a prior invocation kept the turn alive + // (Claude Code via hookSpecificOutput.additionalContext, Codex via a + // decision: "block" continuation). Re-scanning and re-blocking now could + // loop (issue #400). The prior fire already surfaced the findings; + // whether to act on them is the agent's call. Exit fast with no output + // before any scan. Claude Code and Codex both send this field: Codex + // mirrors the Claude contract (StopCommandInput in + // codex-rs/hooks/src/schema.rs) and latches it true for the rest of the + // turn once a block is honored (codex-rs/core/src/session/turn.rs), so + // this guard is also what caps the Codex deep pass at one continuation + // per turn. Cursor and GitHub Copilot omit the field, so the strict + // `=== true` is a no-op for them. The guard makes the loop impossible + // regardless of the finding cache key's line-number sensitivity (out of + // scope here; see findingCacheKey). if (event.stop_hook_active === true) { return result({ skipped: 'stop-hook-active', durationMs: Date.now() - started }); } @@ -2337,6 +2351,15 @@ export function payload(text, eventName = 'PostToolUse', harness = 'claude') { if (harness === 'github') { return JSON.stringify({ additionalContext: text }); } + // Codex shares Claude Code's PostToolUse additional-context shape, but its + // Stop schema rejects unknown fields. Findings that should continue the + // turn must be a top-level blocking decision. + // https://developers.openai.com/codex/hooks#stop (schema of record: + // codex-rs/hooks/src/schema.rs, StopCommandOutputWire) + if (harness === 'codex' && eventName === 'Stop') { + if (!String(text ?? '').trim()) return ''; + return JSON.stringify({ decision: 'block', reason: text }); + } return JSON.stringify({ hookSpecificOutput: { hookEventName: eventName, additionalContext: text }, }); diff --git a/skill/scripts/hook.mjs b/skill/scripts/hook.mjs index 5813ea4f2..9dd67b1c7 100644 --- a/skill/scripts/hook.mjs +++ b/skill/scripts/hook.mjs @@ -10,7 +10,7 @@ * `hookSpecificOutput.additionalContext` when findings exist. * - Stop: runs the FULL detector rule set over every UI file touched this * session (the deep pass), deduped against what the per-edit pass already - * surfaced, and emits once via the Stop additionalContext channel. + * surfaced, and emits once via the harness-specific continuation channel. * * Contract: never break a turn. Always exit 0. Clean files emit a small ack * unless quiet mode is enabled; a clean Stop pass is silent. diff --git a/tests/hook.test.mjs b/tests/hook.test.mjs index cfcbf26dc..1152ccb5f 100644 --- a/tests/hook.test.mjs +++ b/tests/hook.test.mjs @@ -1365,12 +1365,33 @@ describe('writeAuditLog()', () => { }); describe('payload()', () => { - it('produces hookSpecificOutput for Claude/Codex', () => { + it('produces hookSpecificOutput for Claude', () => { const obj = JSON.parse(payload('hello')); assert.equal(obj.hookSpecificOutput.hookEventName, 'PostToolUse'); assert.equal(obj.hookSpecificOutput.additionalContext, 'hello'); }); + it('keeps Codex PostToolUse on the Claude-compatible context channel', () => { + const obj = JSON.parse(payload('hello', 'PostToolUse', 'codex')); + assert.equal(obj.hookSpecificOutput.hookEventName, 'PostToolUse'); + assert.equal(obj.hookSpecificOutput.additionalContext, 'hello'); + }); + + it('produces a blocking decision for Codex Stop', () => { + const obj = JSON.parse(payload('hello', 'Stop', 'codex')); + assert.deepEqual(obj, { decision: 'block', reason: 'hello' }); + }); + + it('emits nothing for a Codex Stop with no findings text', () => { + assert.equal(payload('', 'Stop', 'codex'), ''); + }); + + it('keeps Claude Stop on the additional-context channel', () => { + const obj = JSON.parse(payload('hello', 'Stop', 'claude')); + assert.equal(obj.hookSpecificOutput.hookEventName, 'Stop'); + assert.equal(obj.hookSpecificOutput.additionalContext, 'hello'); + }); + it('produces additional_context for Cursor', () => { const obj = JSON.parse(payload('hello', 'PostToolUse', 'cursor')); assert.equal(obj.additional_context, 'hello'); @@ -2712,10 +2733,19 @@ describe('resolveTargetFiles()', () => { describe('resolveHarness() / normalizeHookEvent()', () => { it('routes explicit env and Cursor conversation_id to cursor harness', () => { assert.equal(resolveHarness({ IMPECCABLE_HOOK_HARNESS: 'cursor' }), 'cursor'); + assert.equal(resolveHarness({ IMPECCABLE_HOOK_HARNESS: 'codex' }), 'codex'); assert.equal(resolveHarness({}, { conversation_id: 'c1' }), 'cursor'); + assert.equal(resolveHarness({}, { turn_id: 'turn-1' }), 'codex'); assert.equal(resolveHarness({}), 'claude'); }); + it('prefers explicit harness and Cursor detection over the Codex turn_id', () => { + assert.equal(resolveHarness({ IMPECCABLE_HOOK_HARNESS: 'claude' }, { turn_id: 'turn-1' }), 'claude'); + assert.equal(resolveHarness({}, { conversation_id: 'c1', turn_id: 'turn-1' }), 'cursor'); + assert.equal(resolveHarness({}, { turn_id: '' }), 'claude'); + assert.equal(resolveHarness({}, { turn_id: 42 }), 'claude'); + }); + it('maps Cursor postToolUse Write path into file_path + cwd', () => { const normalized = normalizeHookEvent({ conversation_id: 'c1', @@ -3818,6 +3848,51 @@ describe('runStopHook()', () => { assert.equal(stop.emission.kind, 'stop-deep-pass'); }); + it('emits Codex Stop findings as a blocking decision', async () => { + const sid = 'stop-codex'; + write('package.json', '{}'); + const file = write('src/Card.tsx', 'noop'); + const det = fakeDetector([finding('marketing-buzzword', 3)]); + const editEventCodex = { ...editEvent(file, sid), turn_id: 'turn-1' }; + const stopEventCodex = { ...stopEvent(sid), turn_id: 'turn-1' }; + + const edit = await runHook({ stdinJson: JSON.stringify(editEventCodex), env: {}, cwd, detector: det }); + assert.equal(edit.audit.harness, 'codex'); + assert.equal(edit.audit.deferred, 1); + const editOut = JSON.parse(edit.stdout); + assert.ok(editOut.hookSpecificOutput, 'Codex per-edit output stays on the PostToolUse context channel'); + assert.equal(editOut.decision, undefined); + + const stop = await runStopHook({ stdinJson: JSON.stringify(stopEventCodex), env: {}, cwd, detector: det }); + assert.equal(stop.exitCode, 0); + assert.equal(stop.audit.harness, 'codex'); + assert.equal(stop.audit.emitted, true, JSON.stringify(stop.audit)); + const out = JSON.parse(stop.stdout); + assert.equal(out.decision, 'block'); + assert.match(out.reason, /marketing-buzzword/); + assert.ok(out.reason.trim().length > 0, 'Codex ignores a block whose reason trims empty'); + assert.equal(out.hookSpecificOutput, undefined); + }); + + it('skips the Codex Stop re-fire after a block instead of blocking again', async () => { + const sid = 'stop-codex-refire'; + write('package.json', '{}'); + const file = write('src/Card.tsx', 'noop'); + const det = fakeDetector([finding('marketing-buzzword', 3)]); + + await runHook({ + stdinJson: JSON.stringify({ ...editEvent(file, sid), turn_id: 'turn-1' }), + env: {}, + cwd, + detector: det, + }); + const refire = { ...stopEvent(sid), turn_id: 'turn-1', stop_hook_active: true }; + const stop = await runStopHook({ stdinJson: JSON.stringify(refire), env: {}, cwd, detector: det }); + assert.equal(stop.exitCode, 0); + assert.equal(stop.stdout, ''); + assert.equal(stop.audit.skipped, 'stop-hook-active'); + }); + it('keeps a policy footer when the grouped Stop render is clamped to the minimum budget', async () => { const sid = 'stop-clamp'; fs.mkdirSync(path.join(cwd, '.impeccable'), { recursive: true });