Skip to content

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

Open
StiensWout wants to merge 1 commit into
pingdotgg:mainfrom
StiensWout:t3/provider-event-error-boundary
Open

StiensWout wants to merge 1 commit into
pingdotgg:mainfrom
StiensWout:t3/provider-event-error-boundary

Conversation

@StiensWout

Copy link
Copy Markdown
Contributor

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.

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:XS 0-9 changed lines (additions + deletions). labels Oct 22, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 22, 2026

Copy link
Copy Markdown
Contributor

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.

@coderabbitai

coderabbitai Bot commented Oct 22, 2026

Copy link
Copy Markdown
Contributor

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 f054d1b and 3a0330c.

📒 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 name Status Explanation
Title check Passed The title clearly identifies the server fix and the prevention of a second schema failure when reporting invalid provider events.
Description check Passed 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 requi…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check 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.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix Fix PR opened by an issue lead; enters the PR loop pr:gate-failed A land gate failed size:XS 0-9 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants