Merge pull request #647 from pbakaus/fix/603-codex-stop-payload

Fix: emit Codex Stop hook as decision/block (#603)

skill/scripts/hook-lib.mjs

@@ -1252,7 +1252,8 @@ export function resolveHarness(env = {}, event = null) {
   if (explicit === 'cursor') return 'cursor';
   if (explicit === 'github') return 'github';
   if (explicit === 'grok') return 'grok';
-  if (explicit === 'claude' || explicit === 'codex') return 'claude';
+  if (explicit === 'claude') return 'claude';
+  if (explicit === 'codex') return 'codex';
   // Grok Build sends camelCase `toolName`/`toolInput`/`hookEventName` and no
   // snake_case pair. GitHub Copilot sends camelCase `toolName`/`toolArgs`.
   // Check Grok first: the old GitHub heuristic (`toolName` and no
@@ -1265,6 +1266,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';
 }
 
@@ -2224,8 +2230,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 and Grok Build read 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' };
@@ -2256,17 +2265,21 @@ export async function runStopHook({ stdinJson, env = {}, cwd = process.cwd(), no
     audit.harness = harness;
     event = normalizeHookEvent(event, cwd, harness);
 
-    // 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. Claude sends `stop_hook_active`; Grok sends
-    // `stopHookActive`, copied onto the snake_case field above. The strict
-    // `=== true` is a no-op when the field is absent. 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). Grok
+    // sends `stopHookActive`, copied onto the snake_case field above. 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 });
     }
@@ -2419,6 +2432,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 },
   });

skill/scripts/hook.mjs

@@ -12,7 +12,7 @@
  *     discards that stdout; the scan still warms the session cache for Stop.
  *   - 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.

tests/hook.test.mjs

@@ -1366,12 +1366,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');
@@ -2739,10 +2760,20 @@ 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({ IMPECCABLE_HOOK_HARNESS: 'grok' }, { turn_id: 'turn-1' }), 'grok');
+    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',
@@ -3886,6 +3917,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 });