Skip to content

fix(compliance): drop stale request-signing vector counts - #6074

Open
KonstantinMirin wants to merge 3 commits into
adcontextprotocol:mainfrom
KonstantinMirin:fix/request-signing-docs-vector-count
Open

fix(compliance): drop stale request-signing vector counts#6074
KonstantinMirin wants to merge 3 commits into
adcontextprotocol:mainfrom
KonstantinMirin:fix/request-signing-docs-vector-count

Conversation

@KonstantinMirin

@KonstantinMirin KonstantinMirin commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

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/:

directory count
positive/ 12
negative/ 28

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. Said 28, which is the size of negative/ 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-required landed in 3.1.0, which is what made 28 coincide with len(negative/).

2. docs/building/by-layer/L1/request-signing.mdx:711 — found while confirming the fix was complete. Said 39 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, whose pass_criteria grades "all vectors in positive/ … and all vectors in negative/" 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, and all/both/only NN vectors. Everything still standing is accurate and deliberately left alone:

Location Claim Why unchanged
signed-requests-runner.yaml:10 "Three negative vectors" Correct — enumerates 016, 017, 020 inline; all three present in negative/. Self-verifying.
universal/signed-requests.yaml:63 "Three negative vectors" Correct — same three, enumerated inline.
webhook-signing/README.md:160 "Three negative vectors" Correct — enumerates 016, 017, 018; all present in webhook negative/. Different profile.
substitution-observer-runner.yaml:437 "Minimum 3 vectors" Correct, and a normative floor rather than a set size, followed by exactly 3 enumerated items.
request-signing/README.md:166 "…Ed25519 positive vectors" Regex artifact (25519 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

  • Comment and prose only. No schema, runner logic, storyboard, or generated artifact is affected; the graded contract in universal/signed-requests.yaml already stated no total.
  • Changeset included (patch). Touching static/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 .mdx change alone would not have required one; docs/building/ is not in PROTOCOL_SCOPED_PATHS.)
  • No dist/ changes. git diff --name-only origin/main...HEAD | grep '^dist/' is empty. The published dist/compliance/<version>/ and dist/docs/<version>/ snapshots are frozen per release by release-docs.yml and keep the old wording by design; the fix ships into the next patch via source.
  • Targets main per 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 .mdx section also references compliance/cache/3.0.0/test-vectors/request-signing/, which is stale — per scripts/overlay-compliance-cache.sh that directory is the SDK's bundled cache and its subdirectory tracks the SDK's pinned ADCP_VERSION, no longer 3.0.0. It appears twice, including as the --vector argument in a copy-pasteable command. Left alone because the correct form depends on adcp-client resolution 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).

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
aao-secretariat[bot]
aao-secretariat Bot previously approved these changes Jul 28, 2026

@aao-secretariat aao-secretariat Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.json is present in negative/, consistent with the claim that the count went stale at 3.1.0. The runner header's independently-wrong 28 (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:208 grades "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 in PROTOCOL_SCOPED_PATHS (scripts/check-changeset-protocol-scope.cjs:21-31 — only docs/reference/ and mintlify-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 --vector example) is correctly left alone here — its correct form depends on adcp-client resolution behavior not verifiable from this repo. File it separately so it doesn't get lost; the SDK's pinned ADCP_VERSION no longer being 3.0.0 is 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.

Constantine.mirin added 2 commits July 28, 2026 22:27
…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
@KonstantinMirin KonstantinMirin changed the title docs(request-signing): drop stale conformance vector counts fix(compliance): drop stale request-signing vector counts Jul 28, 2026

@aao-secretariat aao-secretariat Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.json is present in negative/, which is what made the old 28 coincide with len(negative/).
  • The graded contract already grades by directory: universal/signed-requests.yaml:208 pass_criteria reads "All vectors in positive/ ... and all vectors in negative/". The runner comment and the .mdx now match that shape and cannot drift as vectors land.
  • No wire change. No static/schemas/source/**, no runner logic, no generated artifact. Comment text in signed-requests-runner.yaml and prose in request-signing.mdx:709-712. MUST FIX #4 not in play.
  • Changeset present and correctly typed: .changeset/fix-request-signing-vector-counts.md is patch. Touching static/compliance/source/ is protocol-scoped, so a changeset is required; comment/prose has no wire impact, so patch is 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.mdx still references compliance/cache/3.0.0/test-vectors/request-signing/ twice, including as a copy-pasteable --vector argument. The author flagged this as stale (the cache subdirectory tracks the SDK's pinned ADCP_VERSION, not 3.0.0) and deliberately left it because the correct form depends on adcp-client resolution 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.

@bokelley

Copy link
Copy Markdown
Contributor

Issue #6076 proposes replacing the seven phantom signing error codes in docs/building/by-layer/L1/request-signing.mdx with the 15 request_signature_* codes the negative vectors actually grade — same surface as this PR; consider folding before merge or confirm follow-up.


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.

2 participants