Codex's PR-E review found three protocol-fidelity blockers + two doc gaps, all
verified against ~/repos/refs:
- (A) Top-level `decision` accepted allow/deny/ask, but both reference schemas
reserve those for hookSpecificOutput.permissionDecision — the legacy top-level
decision is approve/block ONLY. Split topLevelDecisionOf (approve/block) from
permissionDecisionOf (allow/deny/ask), so an out-of-band {"decision":"deny"} is
now invalid and ignored instead of becoming a real blocking decision.
- (A) hookSpecificOutput was parsed without its hookEventName discriminator.
HookOutput now surfaces hookEventName so a bridge can discard a block whose
claimed event doesn't match the firing one (the schemas key the block by event).
- (A) runHook discarded raw stdout. HookOutput now carries `stdout` (trimmed,
verbatim) so a bridge can reproduce CC's plain-stdout rendering / Codex's
plain-stdout-as-additionalContext behavior.
- (B) hook/* SessionEventMap variants were only named in prose; added a payload/role
table to core-data-structures/session.md (a maintained catalog surface).
- (B) Removed PR-stack-position references (PR-F / "future bridge packages") from a
test comment and the RFC, per the current-state-wording rule.
New codec tests: top-level allow/deny/ask invalid+ignored, hookEventName capture,
raw stdout preserved on plain + JSON + empty stdout. 51 tests, per-file 100%.
135 lines
6.0 KiB
TypeScript
135 lines
6.0 KiB
TypeScript
import { describe, expect, it } from 'vitest'
|
|
import { parseHookOutput } from '@deepseek-ai/dsh-hook-protocol'
|
|
|
|
describe('parseHookOutput — exit code semantics', () => {
|
|
it('exit 0 with no stdout is a neutral success', () => {
|
|
const out = parseHookOutput(0, '', '')
|
|
expect(out.exitCode).toBe(0)
|
|
expect(out.decision).toBeUndefined()
|
|
expect(out.continue).toBeUndefined()
|
|
})
|
|
|
|
it('exit 2 is a blocking error: stderr becomes the block decision + reason', () => {
|
|
const out = parseHookOutput(2, '', 'this command is not allowed')
|
|
expect(out.decision).toBe('block')
|
|
expect(out.reason).toBe('this command is not allowed')
|
|
expect(out.stderr).toBe('this command is not allowed')
|
|
})
|
|
|
|
it('exit 2 with empty stderr still blocks, with no reason', () => {
|
|
const out = parseHookOutput(2, '', ' ')
|
|
expect(out.decision).toBe('block')
|
|
expect(out.reason).toBeUndefined()
|
|
})
|
|
|
|
it('other non-zero exit is a non-blocking error (no decision, stderr recorded)', () => {
|
|
const out = parseHookOutput(1, '', 'some warning')
|
|
expect(out.decision).toBeUndefined()
|
|
expect(out.exitCode).toBe(1)
|
|
expect(out.stderr).toBe('some warning')
|
|
})
|
|
|
|
it('undefined exit (could not run) carries no decision', () => {
|
|
const out = parseHookOutput(undefined, '', 'spawn failed: ENOENT')
|
|
expect(out.exitCode).toBeUndefined()
|
|
expect(out.decision).toBeUndefined()
|
|
expect(out.stderr).toBe('spawn failed: ENOENT')
|
|
})
|
|
})
|
|
|
|
describe('parseHookOutput — structured stdout (exit 0 only)', () => {
|
|
it('parses top-level continue/stopReason/suppressOutput/systemMessage', () => {
|
|
const out = parseHookOutput(0, JSON.stringify({
|
|
continue: false, stopReason: 'budget exceeded', suppressOutput: true, systemMessage: 'heads up',
|
|
}), '')
|
|
expect(out.continue).toBe(false)
|
|
expect(out.stopReason).toBe('budget exceeded')
|
|
expect(out.suppressOutput).toBe(true)
|
|
expect(out.systemMessage).toBe('heads up')
|
|
})
|
|
|
|
it('parses legacy top-level decision + reason (approve/block ONLY)', () => {
|
|
expect(parseHookOutput(0, JSON.stringify({ decision: 'block', reason: 'nope' }), '').decision).toBe('block')
|
|
expect(parseHookOutput(0, JSON.stringify({ decision: 'approve' }), '').decision).toBe('approve')
|
|
})
|
|
|
|
it('a top-level decision of allow/deny/ask is INVALID and ignored (reserved for permissionDecision)', () => {
|
|
// Both reference schemas restrict the legacy top-level `decision` to
|
|
// approve/block; allow/deny/ask must come from hookSpecificOutput.permissionDecision.
|
|
expect(parseHookOutput(0, JSON.stringify({ decision: 'deny' }), '').decision).toBeUndefined()
|
|
expect(parseHookOutput(0, JSON.stringify({ decision: 'allow' }), '').decision).toBeUndefined()
|
|
expect(parseHookOutput(0, JSON.stringify({ decision: 'ask' }), '').decision).toBeUndefined()
|
|
})
|
|
|
|
it('captures hookEventName from hookSpecificOutput (the discriminator a bridge validates)', () => {
|
|
const out = parseHookOutput(0, JSON.stringify({ hookSpecificOutput: { hookEventName: 'PreToolUse', permissionDecision: 'deny' } }), '')
|
|
expect(out.hookEventName).toBe('PreToolUse')
|
|
expect(out.decision).toBe('deny')
|
|
})
|
|
|
|
it('hookSpecificOutput.permissionDecision OVERRIDES the legacy top-level decision', () => {
|
|
const out = parseHookOutput(0, JSON.stringify({
|
|
decision: 'approve',
|
|
hookSpecificOutput: { permissionDecision: 'deny', permissionDecisionReason: 'denied by policy' },
|
|
}), '')
|
|
expect(out.decision).toBe('deny')
|
|
expect(out.reason).toBe('denied by policy')
|
|
})
|
|
|
|
it('parses allow/ask permissionDecision (the bridge decides whether to honor)', () => {
|
|
expect(parseHookOutput(0, JSON.stringify({ hookSpecificOutput: { permissionDecision: 'allow' } }), '').decision).toBe('allow')
|
|
expect(parseHookOutput(0, JSON.stringify({ hookSpecificOutput: { permissionDecision: 'ask' } }), '').decision).toBe('ask')
|
|
})
|
|
|
|
it('parses additionalContext and updatedInput from hookSpecificOutput', () => {
|
|
const out = parseHookOutput(0, JSON.stringify({
|
|
hookSpecificOutput: { additionalContext: 'remember X', updatedInput: { command: 'safe' } },
|
|
}), '')
|
|
expect(out.additionalContext).toBe('remember X')
|
|
expect(out.updatedInput).toEqual({ command: 'safe' })
|
|
})
|
|
|
|
it('an unknown decision string is ignored (not coerced)', () => {
|
|
expect(parseHookOutput(0, JSON.stringify({ decision: 'maybe' }), '').decision).toBeUndefined()
|
|
})
|
|
|
|
it('malformed JSON on a clean exit is lenient (no structured output, no throw)', () => {
|
|
const out = parseHookOutput(0, '{ not valid json', '')
|
|
expect(out.decision).toBeUndefined()
|
|
expect(out.continue).toBeUndefined()
|
|
})
|
|
|
|
it('non-object stdout (plain text) on exit 0 is left for the bridge (no JSON attempt)', () => {
|
|
const out = parseHookOutput(0, 'just some text output', '')
|
|
expect(out.decision).toBeUndefined()
|
|
expect(out.continue).toBeUndefined()
|
|
// The raw stdout is preserved verbatim so the bridge can render/use it
|
|
// (CC output; Codex additionalContext) — trimmed.
|
|
expect(out.stdout).toBe('just some text output')
|
|
})
|
|
|
|
it('preserves raw stdout (trimmed) alongside parsed structured fields', () => {
|
|
const json = JSON.stringify({ decision: 'block' })
|
|
const out = parseHookOutput(0, ` ${json} \n`, '')
|
|
expect(out.stdout).toBe(json)
|
|
expect(out.decision).toBe('block')
|
|
})
|
|
|
|
it('stdout is empty string when the hook emits none', () => {
|
|
expect(parseHookOutput(0, '', '').stdout).toBe('')
|
|
})
|
|
|
|
it('a JSON array stdout parses but yields no fields (not an object)', () => {
|
|
// Starts with '{'? No — '[' — so it is not even attempted. Neutral.
|
|
const out = parseHookOutput(0, '[1,2,3]', '')
|
|
expect(out.decision).toBeUndefined()
|
|
})
|
|
|
|
it('structured stdout is IGNORED on a blocking (exit 2) run — stderr is authoritative', () => {
|
|
const out = parseHookOutput(2, JSON.stringify({ decision: 'approve' }), 'blocked')
|
|
// exit 2 forces block regardless of what stdout claims
|
|
expect(out.decision).toBe('block')
|
|
expect(out.reason).toBe('blocked')
|
|
})
|
|
})
|