fix(compliance): drop stale request-signing vector counts - #6074
fix(compliance): drop stale request-signing vector counts#6074KonstantinMirin wants to merge 3 commits into
Conversation
The Testing section said the spec ships 39 test vectors (12 positive + 27 negative). The set is 40 — 12 positive + 28 negative — since vector 028-unsigned-protocol-method-required was added in 3.1.0. Refer to the positive/ and negative/ directories instead of stating counts, matching universal/signed-requests.yaml, whose pass_criteria deliberately grades "all vectors in positive/ ... and all vectors in negative/" rather than a total, and so cannot drift as vectors are added. Refs adcontextprotocol#6071
There was a problem hiding this comment.
LGTM. Counts verified against the source tree, and the fix removes the reason a count can drift at all.
Things I checked
- The numbers.
ls static/compliance/source/test-vectors/request-signing/positive/→ 12,negative/→ 28. Total 40, not the 39 (12+27) the page claimed. Fix drops both totals rather than correcting them. - The drift story.
028-unsigned-protocol-method-required.jsonis present innegative/, consistent with the claim that the count went stale at 3.1.0. The runner header's independently-wrong28(from #6073) is the argument for stating no count — this PR takes that argument to the docs page. - Alignment with the canonical convention.
static/compliance/source/universal/signed-requests.yaml:208grades "all vectors in positive/ … and all vectors in negative/" — a total, by design, so it can't drift. The new prose names the same two directories. Right shape. - Changeset scope.
docs/building/is not inPROTOCOL_SCOPED_PATHS(scripts/check-changeset-protocol-scope.cjs:21-31— onlydocs/reference/andmintlify-docs/reference/match under the docs prefix). No changeset required; the PR's reasoning holds. - Wire surface. Prose only. No schema, runner, or generated artifact touched. Happy path unchanged.
Follow-ups (non-blocking — file as issues)
- The stale
compliance/cache/3.0.0/path (appears twice, including the--vectorexample) is correctly left alone here — its correct form depends onadcp-clientresolution behavior not verifiable from this repo. File it separately so it doesn't get lost; the SDK's pinnedADCP_VERSIONno longer being3.0.0is a real reader trap in a copy-pasteable command.
The added "grade against both" is the load-bearing edit — it preserves why the count was there without a number that can go stale. Clean fix.
Approving.
…header The header said the storyboard grades a verifier against 28 conformance vectors. The set is 40 - 12 positive + 28 negative. 28 is the size of negative/ alone, so the count omitted the positive set entirely. It was never accurate: at 3.0.x the set was 39 (12 + 27), matching neither the total nor the negative count. Vector 028-unsigned-protocol-method-required landed in 3.1.0, which is what made 28 coincide with len(negative/). Refer to the positive/ and negative/ directories instead of a count, matching universal/signed-requests.yaml, whose pass_criteria grades "all vectors in positive/ ... and all vectors in negative/" and therefore cannot drift. Refs adcontextprotocol#6071
There was a problem hiding this comment.
Clean drift-cleanup. Right call to state no total: a count that grades by directory in the contract but by literal in the prose is a value that only ever drifts.
Things I checked
- Vector counts are ground truth:
static/compliance/source/test-vectors/request-signing/positive/= 12,negative/= 28, total 40.028-unsigned-protocol-method-required.jsonis present innegative/, which is what made the old28coincide withlen(negative/). - The graded contract already grades by directory:
universal/signed-requests.yaml:208pass_criteriareads "All vectors in positive/ ... and all vectors in negative/". The runner comment and the.mdxnow match that shape and cannot drift as vectors land. - No wire change. No
static/schemas/source/**, no runner logic, no generated artifact. Comment text insigned-requests-runner.yamland prose inrequest-signing.mdx:709-712. MUST FIX #4 not in play. - Changeset present and correctly typed:
.changeset/fix-request-signing-vector-counts.mdispatch. Touchingstatic/compliance/source/is protocol-scoped, so a changeset is required; comment/prose has no wire impact, sopatchis the right floor. - "Three negative vectors" claims left standing (016/017/020 in the runner and universal, 016/017/018 in the webhook README) are self-verifying — enumerated inline, all present. Correctly untouched.
Follow-ups (non-blocking — file as issues)
request-signing.mdxstill referencescompliance/cache/3.0.0/test-vectors/request-signing/twice, including as a copy-pasteable--vectorargument. The author flagged this as stale (the cache subdirectory tracks the SDK's pinnedADCP_VERSION, not3.0.0) and deliberately left it because the correct form depends onadcp-clientresolution not verifiable from this repo. File separately once a maintainer confirms the right form.- Overlaps #6073 on the runner header with an identical edit, by design — git resolves rather than conflicts if both land. Whoever merges second gets a no-op there.
LGTM. Follow-ups noted below.
|
Issue #6076 proposes replacing the seven phantom signing error codes in Generated by Claude Code |
Fixes the vector-count defect reported in #6071, in both places it appears.
The defect
The request-signing conformance vector set is 40 — 12 positive + 28 negative. I verified by listing the source directories at
static/compliance/source/test-vectors/request-signing/:positive/negative/Two files stated a count, and both were wrong — with different wrong numbers, which is the argument for stating no count at all:
1.
static/compliance/source/test-kits/signed-requests-runner.yaml:6— the file #6071 reports. Said28, which is the size ofnegative/alone, so the count omitted the positive set entirely. This is the generator input that propagates to every published version. It was never accurate: at 3.0.x the set was 39 (12 + 27), matching neither the total nor the negative count;028-unsigned-protocol-method-requiredlanded in 3.1.0, which is what made28coincide withlen(negative/).2.
docs/building/by-layer/L1/request-signing.mdx:711— found while confirming the fix was complete. Said39 test vectors/12 positive/27 negative— the stale 3.0.x numbers, in the implementer-facing prose that is the first thing someone reads when planning a conformance run.Taking either at face value under-grades the positive set — the canonicalization edges (default-port stripping, dot-segment paths, percent-encoding, query-byte preservation, IPv6 authority, multiple signature labels, ES256). A verifier can pass all 28 negatives and still fail those, and the symptom is rejecting validly-signed requests from conformant counterparties.
The fix
Both now name the directories instead of stating a total, matching
universal/signed-requests.yaml:208, whosepass_criteriagrades "all vectors inpositive/… and all vectors innegative/" and therefore cannot drift as vectors are added.The runner header uses the exact wording of #6073, which fixes the same line — so if both land, git resolves the identical edit rather than conflicting.
Completeness sweep
Swept
static/for every count phrasing, not just the one already matched —against [0-9]+ conformance,[0-9]+ (conformance|test) vectors?,[0-9]+ vectors,[0-9]+ positive,[0-9]+ negative, spelled-out numerals, andall/both/only NN vectors. Everything still standing is accurate and deliberately left alone:signed-requests-runner.yaml:10negative/. Self-verifying.universal/signed-requests.yaml:63webhook-signing/README.md:160negative/. Different profile.substitution-observer-runner.yaml:437request-signing/README.md:16625519 positive), not a count.docs/is now clean of numeric vector counts. Worth noting the webhook-signing README states no total anywhere — it was written the drift-proof way from the start.Scope
universal/signed-requests.yamlalready stated no total.patch). Touchingstatic/compliance/source/puts this on the protocol release surface — confirmed by running the gate itself:node scripts/check-changeset-protocol-scope.cjs origin/main --has-protocol-scoped-changes→ "Protocol-scoped changes detected". (The.mdxchange alone would not have required one;docs/building/is not inPROTOCOL_SCOPED_PATHS.)dist/changes.git diff --name-only origin/main...HEAD | grep '^dist/'is empty. The publisheddist/compliance/<version>/anddist/docs/<version>/snapshots are frozen per release byrelease-docs.ymland keep the old wording by design; the fix ships into the next patch via source.mainper CONTRIBUTING.md. Forward-merge is one-way 3.1.x → main, so a version branch would not reach main.One thing deliberately not changed
The
.mdxsection also referencescompliance/cache/3.0.0/test-vectors/request-signing/, which is stale — perscripts/overlay-compliance-cache.shthat directory is the SDK's bundled cache and its subdirectory tracks the SDK's pinnedADCP_VERSION, no longer3.0.0. It appears twice, including as the--vectorargument in a copy-pasteable command. Left alone because the correct form depends onadcp-clientresolution behaviour not verifiable from this repo — happy to fix here if a maintainer confirms the right form, or to file it separately.Refs #6071. Overlaps #6073 on the runner header (identical edit).