feat(compliance): add capabilities_response_schema_invalid notice + capability_pointer field - #6256
Conversation
|
Pushed a revision after a closer pass over the contract's own rules. Four changes worth calling out, three of which are corrections to what I first filed: A changeset is required after all. Contract The dedupe rule needed extending, or the design defeats itself. Loosened the both-present constraint. I had written that Also moved the entry to the end of On |
|
All four corrections look right against the contract:
On A fourth severity would be warranted if the severity value changes runner behavior (abort on Generated by Claude Code |
|
Two things here need you, and neither is fixable from my side.
Second, the sign-off above was a comment rather than a review, so Everything else is green: 22 of 23, with the changeset and the 2.6.0 → 2.7.0 contract bump in. |
bokelley
left a comment
There was a problem hiding this comment.
Four corrections verified correct in the prior thread:
- Changeset + 2.7.0 bump — gate is accurate; additive surface change correctly maps to minor contract version.
- Composite dedupe key — (
code,capability_pointer) is the right key; code-only dedup would collapse all per-pointer notices into one, defeating the field's purpose. - Loosened co-present constraint — "pointer MUST resolve to the node named by the path or to a position within it" correctly handles
signed_requests_specialism_deprecated(path:specialisms, pointer:/specialisms/0). The strict same-location reading would have outlawed the motivating case. - Registration order — entry moved to end of
canonical_codes; correct per convention.
capability_pointer design and capabilities_response_schema_invalid at severity: info are both sound. Formal approval closes the review gate already cleared in comment form.
Generated by Claude Code
|
Formal review submitted — approval closes the review gate. On Fixing it requires a Generated by Claude Code |
…apability_pointer Part A of adcontextprotocol#6254. A schema-invalid get_adcp_capabilities response currently surfaces as N independent track failures with no indication they share a root cause; two documented repros (adcontextprotocol#6242, adcontextprotocol#6206) cost hours of triage each. Adds the canonical notice code capabilities_response_schema_invalid so a runner can name the root cause once, ahead of the track results, without suppressing any of them. Adds capability_pointer (RFC 6901) as a new optional notice field rather than overloading capability_path, which is documented as a dotted path naming the capability flag. Schema validators emit RFC 6901 natively, the dotted translation is lossy for array positions, and RFC 6901 is already first-class here via validation_result.json_pointer. Because one notice is emitted per failing validation error, all sharing a code, the run_summary.notices dedupe rule is extended to dedupe by (code, capability_pointer) when a pointer is present — otherwise a conforming consumer would collapse them and discard every location but one. Contract version 2.6.0 -> 2.7.0 (additive surface: one optional field, one canonical code), matching how cascade_rules and the notices block bumped it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Rebased onto main — #6276 took the same Re-verified after the rebase: The force-push dismissed the approval, so this needs a re-approve when you get a moment. Nothing changed in the diff apart from the version number. |
bokelley
left a comment
There was a problem hiding this comment.
Re-approving post-rebase. The only change from the 08-06 approved diff is the version line (2.7.0 → 2.9.0 to follow #6276's 2.8.0 bump); all four corrections are intact — changeset, composite dedupe key by (code, capability_pointer), loosened both-present constraint (pointer MUST resolve to the node named by the path or a position within it), and end-of-list entry order.
capability_pointer (Option 2, RFC 6901) is the confirmed field shape. severity: info stays per the prior reasoning — the runner always continues, input_schema_field_stripped is the right precedent.
code_review CI failure is actor-scoped (fork PR without write access), not a diff issue.
Generated by Claude Code
Part A of #6254. Adds the canonical notice code so a runner can name the root cause once, ahead of the track results, when a schema-invalid
get_adcp_capabilitiesresponse would otherwise fan out into N unrelated-looking track failures.Two documented repros, both costing hours of triage: #6242 (
accountblock missingsupported_billing,sandboxtyped as an object — 10 steps failed across three tracks) and #6206 (one uncachedcontext.correlation_idpresented as five unrelated problems).What's here
capabilities_response_schema_invalidincanonical_codes.severity: info, followinginput_schema_field_stripped— the closest peer, alsoinfo, also carrying nocapability_path. Thenotesblock keeps the Part B constraint explicit: the runner MUST still execute and report every storyboard it would otherwise have run. The notice explains the results, it does not replace them, and suppressing downstream failures would hide genuine defects behind a preflight line.One notice per failing validation error, each with its own pointer, rather than one aggregate. The "affects N storyboards" count is omitted per the triage, since it needs a dry-run pass.
Per your instruction, the notes also scope the code away from #6206: a capabilities response that is schema-valid but stale or cached is a different condition and must not reuse this code.
capability_pointer(RFC 6901) as a new entry inoptional_fields, per Option 2.On the open question
The doc comment says a notice MAY carry both, because they answer different questions rather than being two spellings of one.
capability_pathnames the capability flag that motivated the notice;capability_pointerlocates a position in the response document. This new code often has no meaningful flag at all — a wrong type inside a block, or an array element — while the existing codes carry a flag with no validation pointer. "Exactly one" would force a false choice on leaf fields where both are genuinely available.It also states that a consumer MUST NOT reject a notice for carrying only one, and that when both are present they MUST refer to the same location.
Scope
Documentation-only change to the conformance harness contract; no schema, no storyboard, no runner code. No changeset, per the triage. Part B (pre-execution validation in the
@adcp/sdkrunner that emits this notice) is a separate change inadcp-client.Checked before filing:
runner-output-contract.yamlis excluded fromlint-universal-storyboard-doc-parityas a fixture,server/tests/unit/compliance-notices.test.tsasserts specific existing codes rather than enumerating them, and the file parses.