Fix event ingress dropping real broker events: accept null optional fields#220
Conversation
The live broker serializes unset optional fields as JSON null, not omitted keys. The ingress schemas (#214) used .optional(), which accepts undefined but rejects null, so real agent_spawned, worker_ready, and relay_inbound events were dropped at ingress — including chat messages. Observed in production logs: [broker] Dropped malformed broker event: { kind: 'relay_inbound', reason: 'thread_id: Expected string, received null' } All optional scalar fields are now .nullish(). Regression tests pin the exact dropped payloads; wrong non-null types are still rejected. The #214 test gap: fixtures were built from the SDK's TypeScript declaration (provider?: string), which says nothing about wire-level null. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 8 minutes and 55 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
✨ 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 |
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
Bug
The broker serializes absent optional fields as
null; the #214 ingress schemas used.optional()(accepts missing, rejectsnull), so real events were dropped at ingress —agent_spawned,worker_ready, and criticallyrelay_inbound(chat messages). Observed live:(The throttled malformed-event telemetry from #195/#214 is what surfaced this.)
Fix
All 30 optional scalar fields in
src/shared/schemas/broker-events.ts→.nullish()(acceptsnullandundefined; still rejects wrong non-null types). The threez.unknown().optional()fields already accepted null. This restores pre-#214 behavior, where null-bearing events flowed through untouched.Why tests missed it
Fixtures were derived from the SDK's TypeScript declaration (
provider?: string) — TS optionality says nothing about JSONnullon the wire. New regression tests pin the exact three dropped payloads from production logs, plus a wrong-type case proving validation still rejects garbage.Verification
npx vitest run src/main/broker.test.ts→ 76/76 (4 new)npx vitest run→ 349/349 ·npm run typecheck✓ both configs ·npm run lint✓ 0 errors🤖 Generated with Claude Code