fix(server): report invalid provider events without a second schema failure#23650

open1
main
GitHub fork: t3/provider-event-error-boundary
opened19d agobyWWout Stiens
View on GitHubTwo-way sync enabledPR last imported 15h ago

A provider event with invalid data could fail again while T3 constructed its normalization error, hiding the validation failure behind a schema defect.

Record the event type and preserve the original cause without validating the rejected payload again. Accepted-event validation is unchanged.

Verified with a failing-first service test against isolated SQLite, the valid-event control, 24 ingestor tests, eight recovery cases, server typecheck, and check:changed. A fresh independent audit returned ready to file at 3a0330c1d4.

Related to the Pi question failure in #22405. This fixes the shared error boundary; the Pi dialog mapper is a separate change.

Prepared for Wout by gpt-6.1-sol in the Codex harness through T3 Code.

Checks

23 successful · 10 skipped
All checks passed
3a0330c

Added 1 commit

  • fix(server): report rejected provider events without revalidating them3a0330c
Synced from GitHub

Approvability

Verdict: Approved at 3a0330c

Macroscope's review found this PR approvable — This is a small, self-contained server bug fix that preserves accepted-event behavior while making invalid provider events report their original schema failure reliably. The added test covers the failing case and a valid-event control.

You can add or adjust custom eligibility rules. Learn more.

Synced from GitHub

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
📥 Commits

Reviewing files that changed from the base of the PR and between f054d1b8018721bd997d99cbb918a7b17bf098a4 and 3a0330c1d4cabef79c0330fcfc7f231df59d682d.

📒 Files selected for processing: 2
  • apps/server/src/orchestration-v2/ProviderEventIngestor.test.ts
  • apps/server/src/orchestration-v2/ProviderEventIngestor.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

📝 Walkthrough

The normalization error now stores the rejected event type instead of the full event. Tests cover invalid and valid turn_item.updated question events and verify their effects on ingestion state.

Changes: Provider event normalization

Layer / File(s)Summary
Record rejected event type
apps/server/src/orchestration-v2/ProviderEventIngestor.ts
ProviderEventNormalizeError stores and reports the rejected event type. Normalization passes the event type when it creates the error.
Test question event validation
apps/server/src/orchestration-v2/ProviderEventIngestor.test.ts
Tests verify that an empty question fails validation without adding an event or turn-item projection. A nonempty question passes validation and is stored and projected.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Possibly related issues

  • pingdotgg/t3code#22405 — The issue describes normalization error construction re-validating an invalid payload. This change stores the event type instead of the rejected payload.

Suggested reviewers: t3dotgg

Merge Risk: ⚪ Minimal · up to 3a033

No merge-blocking issue remains identified; merge after normal checks.

Pre-merge checks: Passed 4
Check nameStatusExplanation
Title checkPassedThe title clearly identifies the server fix and the prevention of a second schema failure when reporting invalid provider events.
Description checkPassedThe description explains the problem, the fix, scope, related issue, and verification steps. It does not use the template headings or explicitly document maintainer approval, but it provides the requi…
Linked Issues checkPassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes checkPassedCheck skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

The description explains the problem, the fix, scope, related issue, and verification steps. It does not use the template headings or explicitly document maintainer approval, but it provides the required information for this focused fix.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.