Skip to content

fix(client): filter oneOf branch sub-errors from Ajv allErrors output#1112

Closed
bokelley wants to merge 2 commits intomainfrom
claude/issue-1111-oneof-suberror-filter
Closed

fix(client): filter oneOf branch sub-errors from Ajv allErrors output#1112
bokelley wants to merge 2 commits intomainfrom
claude/issue-1111-oneof-suberror-filter

Conversation

@bokelley
Copy link
Copy Markdown
Contributor

Closes #1111

Summary

  • Adds filterOneOfSubErrors() in src/lib/validation/schema-validator.ts, applied in both validateRequest and validateResponse before error formatting.
  • Filter rule: drop any AJV error whose schemaPath is a strict descendant of a failing oneOf node's path. oneOf node errors are always kept (including nested ones via an explicit keyword === 'oneOf' guard). anyOf sub-errors are intentionally NOT filtered — anyOf allows partial matches and its per-branch errors carry useful disambiguation signal.
  • Adds a regression test in test/lib/schema-validation.test.js that asserts oneOf sub-branch errors are absent from validateRequest output and the node error is preserved.
  • Adds .changeset/oneof-suberror-filter.md (patch bump for @adcp/sdk and @adcp/client).

Root cause

schema-loader.ts initialises Ajv with allErrors: true. When Ajv evaluates a oneOf and no branch matches it:

  1. Emits the top-level oneOf node error (actionable).
  2. Emits one required/type error per property mismatch in every branch it evaluated (noise).

These sub-branch errors surfaced as spurious "must have required property" failures in storyboard CI, causing false-negative reports for downstream SDKs.

What was tested

  • Pre-existing test/lib/schema-validation.test.js suite (build/unit — schema-dependent assertions are vacuous without synced schemas but not misleading).
  • test/validation-enum-enrichment.test.js — unaffected by this change.
  • npx prettier --write and npx tsc --noEmit pass clean on the branch.

Pre-PR reviews

Two expert reviews were completed before this PR was opened:

  • code-reviewer — approved after blocker fix (nested oneOf guard added; test assertion hardened from silent-vacuous to assert.ok).
  • ad-tech-protocol-expert — approved; filter is mechanically correct, anyOf exclusion justified, no impact on context_value_rejected hint diagnostics.

Out of scope / follow-ons

  • NODE_ENV-sensitive defaultResponseMode() in client-hooks.ts (separate CI vs local discrepancy vector — distinct issue).
  • anyOf flooding in get_adcp_capabilities responses (separate scope, different keyword).

Triage-managed PR — opened by the automated triage agent per .agents/routines/triage-prompt.md. Assign a human reviewer before merging.

https://claude.ai/code/session_01Sv61kPXqT5b8bLYZ62muHV


Generated by Claude Code

claude added 2 commits April 30, 2026 17:12
…tput

Fixes #1111: Ajv allErrors:true emits per-branch required/type errors from
every non-matching oneOf variant, causing spurious storyboard CI failures
for downstream SDKs (e.g. "/products/2/name: must have required property
'name'" on products that have name populated). The oneOf node error is the
actionable signal; branch sub-errors are filtered via schemaPath prefix match.

https://claude.ai/code/session_01Sv61kPXqT5b8bLYZ62muHV
…tion

Code-reviewer pre-PR blocker: the original filter dropped nested oneOf node
errors (a oneOf inside a branch of an outer oneOf) because their schemaPath
is a descendant of the outer oneOf's path. Fix: guard err.keyword === 'oneOf'
so all oneOf node errors are always preserved — only non-oneOf sub-branch
errors are subject to the prefix-drop.

Also strengthen the regression test: assert.ok on oneOfIssue unconditionally
(not inside a silent if-guard) so a future regression that drops the node
error itself is caught rather than silently skipped.

https://claude.ai/code/session_01Sv61kPXqT5b8bLYZ62muHV
@bokelley
Copy link
Copy Markdown
Contributor Author

Triage update — source issue resolved as not-a-bug.

@bokelley closed #1111 after tracing the actual root cause to adcp-client-python: examples/seller_agent.py::seed_product was producing skeleton products missing schema-required fields (publisher_properties: minItems 1, format_ids[].agent_url, available_reporting_frequencies: minItems 1). The @adcp/client@5.21.1 validator was reporting those fields correctly, not over-reporting from non-matching oneOf branches. Fixed downstream in adcontextprotocol/adcp-client-python#321.

The filterOneOfSubErrors() change in this PR is based on the original (incorrect) hypothesis and is no longer needed. Recommend closing this PR without merging. If the Ajv oneOf error-aggregation improvement is still wanted on its own merits — not as a bug fix but as a UX polish — that would be a fresh, scoped PR with a different rationale.


Triaged by Claude Code. Session: https://claude.ai/code/session_01WVfLw36MJBxN4oMZGuPUWh


Generated by Claude Code

@bokelley
Copy link
Copy Markdown
Contributor Author

bokelley commented May 1, 2026

Superseded by #1113 (merged: "compact oneOf cascade so real residuals reach the wire"). Issue #1111 is closed. Closing this draft.

@bokelley bokelley closed this May 1, 2026
@bokelley
Copy link
Copy Markdown
Contributor Author

bokelley commented May 1, 2026

Acknowledged — noted that #1113 ("compact oneOf cascade so real residuals reach the wire") supersedes this draft and issue #1111 is closed. No further action from triage.


Generated by Claude Code


Generated by Claude Code

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

oneOf schema-validator over-reports "must have required property" from non-matching variants — storyboard CI sees false negatives that local doesn't

2 participants