fix(events): accept event key in hook.delivery schema (#1492) - #1494
Conversation
Closes #1492. After #1485 (hookify-perf-foundation) shipped, runHandler() in src/hooks/index.ts:177 started stamping `event: payload.hook_event_name` on every hook.delivery span so the new hook_perf_baseline view can group by event for tool-less events (UserPromptSubmit, Stop). The strict-mode Zod schema for hook.delivery did not list `event` as a valid key, so every emission was rejected at the runtime-events layer with `unrecognized key 'event'`. Net effect: hook_perf_baseline view stayed empty regardless of dispatch activity, and `genie doctor --perf` reported no spans observed. Fix: add HookEventSchema (string min:1 max:64, public tier) and include `event` as an OPTIONAL key in the strict object. Optional preserves backward compat for any caller emitting a span without the key. Validation: 6/6 src/lib/events/schemas/hook.delivery.test.ts pass: - accepts payload WITH event (post-#1485 emitter shape) - still accepts WITHOUT event (back-compat) - accepts tool-less events (UserPromptSubmit, Stop) with event but no tool - still rejects unknown keys (strict mode preserved) - rejects oversized event names (>64 chars) - rejects empty event names Evidence: dog-fooder-11eb verdict at state/evidence/batch-2034c4f3 reproduced the bug end-to-end. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Code Review
This pull request updates the hook.delivery schema to include an optional event field, resolving an issue where strict validation rejected emissions containing this key. It also introduces a comprehensive test suite to verify the new field's behavior, including backward compatibility and strict mode enforcement. The feedback identifies a minor inconsistency in the schema description where an extra 'CC' prefix should be removed for better alignment with existing telemetry metadata.
| * Closes #1492 — strict schema previously rejected this key, leaving the | ||
| * baseline view empty regardless of dispatch activity. | ||
| */ | ||
| const HookEventSchema = tagTier(z.string().min(1).max(64), 'C', 'CC hook event name — public'); |
There was a problem hiding this comment.
The description for HookEventSchema contains a 'CC' prefix which appears to be a typo or inconsistent with other schemas in this file (e.g., HookNameSchema uses 'hook name — public'). For consistency and clarity in telemetry metadata, it should likely be 'hook event name — public'.
| const HookEventSchema = tagTier(z.string().min(1).max(64), 'C', 'CC hook event name — public'); | |
| const HookEventSchema = tagTier(z.string().min(1).max(64), 'C', 'hook event name — public'); |
#1494 residual) Tests were inserting rows with kind='hook.delivery' (the OLD filter the view no longer matches) and asserting they appeared in hook_perf_baseline. After migration 057 changes the view to filter on subject='hook.delivery' AND data._kind='span', tests must insert rows in that shape. Updates: - DELETE sweep now matches both subject AND legacy kind for cleanup - INSERT statements add subject='hook.delivery', kind='system' (matches real emitter shape per 11eb verdict) - Test fixture data JSON now includes _kind: 'span' so the new view's WHERE data->>'_kind' = 'span' clause matches Validation: 5/5 src/db/migrations/hook-perf-baseline.test.ts pass.
…ew-subject fix(db): hook_perf_baseline filters wrong column (#1494 residual)
Closes #1492. Adds the missing
eventkey (string min:1 max:64, public tier, optional) to thehook.deliverystrict schema so spans emitted post-#1485 are accepted instead of dropped withunrecognized key 'event'.After #1485 shipped,
runHandler()(src/hooks/index.ts:177) stampsevent: payload.hook_event_nameon every span so thehook_perf_baselineview groups by event. Without this fix, every emission is rejected, the view stays empty, andgenie doctor --perfreports no spans.Validation
6/6
src/lib/events/schemas/hook.delivery.test.tspass — strict mode preserved (still rejects unknown keys), backward-compatible (works withoutevent), supports tool-less events (UserPromptSubmit/Stop with event-only).Evidence
dog-fooder-11eb verdict 2026-04-29 — repro in
/home/genie/workspace/agents/genie/.genie/agents/dog-fooder/state/evidence/batch-2034c4f3.