Skip to content

fix(docs): remove invented signing error-code table, point at the taxonomy - #6078

Open
KonstantinMirin wants to merge 1 commit into
adcontextprotocol:mainfrom
KonstantinMirin:fix/request-signing-error-codes
Open

fix(docs): remove invented signing error-code table, point at the taxonomy#6078
KonstantinMirin wants to merge 1 commit into
adcontextprotocol:mainfrom
KonstantinMirin:fix/request-signing-error-codes

Conversation

@KonstantinMirin

@KonstantinMirin KonstantinMirin commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

Fixes #6076. Instance 3 of #6075.

The defect

### Error codes on the primary L1 request-signing page listed seven code names — missing_signature, invalid_signature, expired_signature, replayed_nonce, revoked_key, unknown_key, unsupported_algorithm. None of them exists anywhere in the 3.1.1 compliance tree.

universal/signed-requests.yaml grades the code byte-for-byte against each vector's expected_outcome.error_code. So an implementer following this page fails all 28 negative vectors while having correctly implemented what the docs told them — right status, right header, right rejection reason, wrong string. The docs and the conformance suite describe two different protocols.

Why delete rather than correct

The line immediately below the table already read:

For the full error code taxonomy, see Transport error taxonomy.

So the table was a hand-made summary that both duplicated and contradicted a canonical table the page linked on the very next line. security.mdx#transport-error-taxonomy carries 18 request_signature_* codes plus request_target_uri_malformed and request_body_malformed, each with retry semantics, and the WWW-Authenticate header rules including the no-realm constraint — none of which a seven-row summary can carry.

Retyping that enum with the right names would fix today's symptom and rebuild the mechanism. A corrected table is still a second hand-maintained copy of an 18-row enum; it goes stale the moment a code is added, which is exactly how this one went wrong. That is the anti-pattern filed as #6075 — docs restating machine-readable spec facts by hand with nothing checking they agree — and the fix for an instance of it should not be a fresh instance of it.

I initially wrote a corrected 15-row table scoped to "the codes the negative vectors grade", on the theory that a subset with a definition is re-derivable rather than arbitrary. That was wrong, and this PR was revised. A definition makes a table verifiable, not self-updating — a 29th negative vector introducing a new code staleness it just the same. The only copy that cannot drift is the one that does not exist.

Reviewers: @aao-secretariat's approval above predates this revision and was given against the corrected-table version. Re-review appreciated.

What is kept

The framing sentence, which is normative and correct.

Two additions, both non-enumerative, so neither can drift the way a code list does:

  • The byte-for-byte grading rule. Owned by universal/signed-requests.yaml, stated nowhere on this page, and precisely the fact a ## Testing section should carry — it tells an implementer why the exact string matters, which is what the old table failed to convey while getting the strings wrong.
  • A second pointer, to the brand_json_url discovery section. The discovery-phase request_signature_brand_* and request_signature_key_origin_* codes live in a different security.mdx section that the single previous link did not reach.

Net −12/+4.

Establishing the canonical set

Even though nothing is transcribed, the sources were reconciled to confirm the pointer aims at a complete list:

  • security.mdx is the only complete list — 27 concrete request_signature_* codes across #transport-error-taxonomy and the brand_json_url discovery table. Both are now linked.
  • Vectors — the 28 files under dist/compliance/3.1.1/test-vectors/request-signing/negative/ carry 15 distinct codes, a strict subset of the 27. Verified programmatically; no disagreement in either direction.
  • The schema enumdist/schemas/3.1.1/enums/error-code.json has 92 values and zero request_signature_*.

That last one looks like a conflict but is not: these are HTTP transport-layer codes carried in WWW-Authenticate, not AdCP protocol error codes, so their absence from the protocol enum is correct rather than drift. The only schema naming any of them is get-adcp-capabilities-response.json, in prose, for 3.

On the mapping specifically: expired_signature was confirmed against the vectors, not assumed — 003-expired-signature, 004-window-too-long, and 013-expires-le-created all expect request_signature_window_invalid. One window predicate, one code; there is no separate "expired" code.

Why pointing beats summarizing here, concretely

security.mdx defines request_target_uri_malformed as covering empty authority, bare IPv6, IPv6 zone identifier, and raw non-ASCII host. There are open conformance bugs against the Python SDK for not implementing exactly that (adcontextprotocol/adcp-client-python#977, #978). The spec is clear and an implementation still diverged — so implementers are already getting the taxonomy wrong from an authoritative source. A summary on a second page competing with that source makes it worse, not better. Note also that request_target_uri_malformed is one of the codes no seven-row summary would ever have included.

Not fixed here — the compliance/cache/3.0.0/ path

### Conformance vectors hardcodes compliance/cache/3.0.0/ at lines 711 and 719, the second as a copy-pasteable --vector argument. That path is the SDK's bundled cache whose version segment is read from node_modules/@adcp/sdk/ADCP_VERSION (see scripts/overlay-compliance-cache.sh) — a moving path frozen as a literal.

I could not establish the correct form from this repo, so I left it rather than guess. This repo's own CLI reference contradicts the snippet: docs/building/verification/grading.mdx documents signing verify-vector as reading a vector from stdin, with no --vector flag, referring to the set by its published URL /compliance/latest/test-vectors/request-signing/ rather than a node_modules path. The CLI ships in @adcp/sdk, so which form is right is not decidable here. Correcting only the version string would leave a command whose flag may not exist — a cosmetic fix that makes a broken snippet look maintained. Filed as #6077. It also sits on the one line #6074 edits, so deferring keeps both PRs conflict-free.

Sweep

Swept all non-dist/ prose and source for the seven names. The wrong table existed in exactly one place — all seven names appeared once each, together, in it.

The only other hit anywhere is tests/billing/webhook-security.test.ts:47, const invalidSignature = 'invalid_signature' — a Stripe billing-webhook test mock, unrelated domain, deliberately unchanged.

No other invented signing codes exist. Cross-checked both canonical taxonomies against all 55 negative test-vector files under static/compliance/source/test-vectors/{request-signing,webhook-signing}/ — every error_code matches one of them byte-for-byte. The webhook taxonomy's webhook_* prefix is a documented deliberate parallel, not drift. Every other Signature error= site in the repo uses a <code> placeholder or ${err.code} from the SDK, so none could carry a wrong name.

Relationship to #6074

Separate PR by design. #6074 corrects stale counts in the adjacent subsection and is approved and green; this corrects the normative contract and deserves its own review rather than riding inside a counts PR. git merge-tree reports a clean merge in either order, so neither blocks the other. #6073 untouched, #6071 left open.

Verification

  • git diff origin/main --name-onlydocs/building/by-layer/L1/request-signing.mdx only. No dist/ path touched; no schema, vector, runner, or generated artifact changed.
  • Both anchors have in-repo precedent — #transport-error-taxonomy was already used by the line replaced, and security.mdx self-links #discovering-an-agents-signing-keys-via-brand_json_url.

Changeset: not required

$ node scripts/check-changeset-protocol-scope.cjs origin/main --has-protocol-scoped-changes
No protocol-scoped changes detected.     (exit 1)

$ node scripts/check-changeset-protocol-scope.cjs origin/main
Changeset protocol scope check passed.    (exit 0)

docs/building/ is not in PROTOCOL_SCOPED_PATHS, so changeset-check.yml skips the changesets status step. #6074 needed one because it also touched static/compliance/source/, which is protocol-scoped; this is docs prose only.

Why nothing caught this

scripts/lint-error-codes.cjs and scripts/lint-error-code-drift.cjs scan static/compliance/source/ and static/schemas/source/enums/error-code.json. Neither scans docs/. No check validates an error code named in prose against anything — the structural gap #6075 proposes closing. Deleting the copy is the version of that fix available today: prose that points cannot drift, so there is nothing left for a linter to check here.

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.

Corrects a broken normative contract: the old seven-code table graded against nothing and failed all 28 negative vectors for a correct implementer. Fix is right because the replacement subset has a definition — the codes the negative vectors grade — so it is re-derivable rather than arbitrary.

Things I checked

  • Table == vector codes. Pulled expected_outcome.error_code from all 28 files under dist/compliance/3.1.1/test-vectors/request-signing/negative/. 15 distinct values. The 15 table rows match them exactly — no code in the table absent from the vectors, none exercised by a vector missing from the table.
  • Wording faithfully lifted from security.mdx. Each row maps to a verifier-checklist step (security.mdx:1224–1246): request_signature_params_incomplete param list matches step 2, window_invalid "validity > 5 min" matches step 5's expires − created > 300 s, components_unexpected matches step 6's forbidden-content-digest branch. Pages agree in name and meaning, so they can't drift.
  • The five "not exercised" codes named inline all exist: request_target_uri_malformed (step 10), request_body_malformed (step 14), request_signature_revocation_stale (step 9 / security.mdx:1387), request_signature_jwks_unavailable (:1393), request_signature_jwks_untrusted (:1394).
  • Both cross-reference anchors resolve#transport-error-taxonomy and #discovering-an-agents-signing-keys-via-brand_json_url; the request_signature_brand_* / request_signature_key_origin_* codes live in the discovery table (security.mdx:1119–1127), matching the pointer.
  • expired_signature claim confirmed against vectors, not assumed003-expired-signature, 004-window-too-long, and 013-expires-le-created all expect request_signature_window_invalid. One window predicate, one code.
  • No wire surface touched. docs/building/by-layer/L1/request-signing.mdx only — no dist/, no static/schemas/source/, no vector or runner. docs/building/ is not in PROTOCOL_SCOPED_PATHS, so no changeset is required (MUST FIX #6 scopes to docs/reference/, not docs/building/).

Follow-ups (non-blocking — already filed)

  • The hardcoded compliance/cache/3.0.0/ path and the --vector flag that may not exist in the SDK CLI (#6077). Correct call to defer rather than guess the replacement — a cosmetic version bump would have hidden the flag question.
  • The structural gap: no linter validates an error code named in prose against the taxonomy or vectors, which is why the wrong table survived (#6075). Until that closes, a wrong code in prose reads identically to a right one.

The 025-jwk-alg-crv-mismatch row is broader than its current security.mdx sentence, by the vector's own $comment — widening the row to match graded behavior is correct, and the note that security.mdx's row lags is the right place to leave it.

LGTM. Follow-ups noted below.

…onomy

The `### Error codes` table listed seven code names — `missing_signature`,
`invalid_signature`, `expired_signature`, `replayed_nonce`, `revoked_key`,
`unknown_key`, `unsupported_algorithm` — that appear nowhere in the 3.1.1
compliance tree. The real codes are all `request_signature_*`.

`universal/signed-requests.yaml` grades the code byte-for-byte against each
vector's `expected_outcome.error_code`, so an implementer following this page
fails all 28 negative vectors while correctly implementing what the docs told
them: right status, right header, right rejection reason, wrong string.

Delete the table rather than correct it. It was a hand-made summary of an
enum whose canonical table — `security.mdx#transport-error-taxonomy`, 18
`request_signature_*` codes plus `request_target_uri_malformed` and
`request_body_malformed`, each with retry semantics — the page already linked
on the very next line. Re-typing that enum with the right names would
recreate the second hand-maintained copy that drifts the moment a code is
added, which is how this went wrong the first time.

Keep the framing sentence, which is normative and correct. Add the
byte-for-byte grading rule, which belongs in a Testing section and is stated
nowhere else on the page. Extend the pointer to cover the discovery-phase
`request_signature_brand_*` / `request_signature_key_origin_*` codes, which
the single previous link did not reach.

Docs prose only. No schema, vector, runner, or generated artifact changes.
@KonstantinMirin
KonstantinMirin force-pushed the fix/request-signing-error-codes branch from 084f95d to ffa9194 Compare July 28, 2026 21:12
@KonstantinMirin KonstantinMirin changed the title fix(docs): correct request-signing error codes to the graded taxonomy fix(docs): remove invented signing error-code table, point at the taxonomy Jul 28, 2026
@KonstantinMirin

Copy link
Copy Markdown
Collaborator Author

Revised the approach — force-pushed. @aao-secretariat, your approval above was against the previous version and no longer covers what is on the branch.

Previously this PR replaced the seven wrong codes with a corrected 15-row table scoped to "the codes the negative vectors grade". It now deletes the table and lets the existing link carry the codes.

The argument that changed it: the line immediately below the table already pointed at security.mdx#transport-error-taxonomy, so the table duplicated and contradicted a canonical table on the very next line. A corrected table is still a second hand-maintained copy of an 18-row enum — it goes stale the moment a code is added, which is how this one went wrong. Scoping it to a definition makes it verifiable, not self-updating; a 29th negative vector introducing a new code stales it just the same. Fixing an instance of #6075 by writing a fresh instance of #6075 is not a fix.

Net is now −12/+4. Kept: the framing sentence, the byte-for-byte grading rule (owned by signed-requests.yaml, stated nowhere else on the page, and the fact that explains why the exact string matters), and a second pointer to the brand_json_url discovery codes that the single previous link did not reach. Both additions are rules, not enumerations, so neither can drift the way a code list does.

Re-review appreciated.

@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.

Deleting a hand-maintained enum that contradicted the canonical taxonomy on the very next line is the right fix, not retyping it. The only copy that can't drift is the one that doesn't exist.

Things I checked

  • Both pointer anchors resolve. #transport-error-taxonomysecurity.mdx:1369 (#### Transport error taxonomy). #discovering-an-agents-signing-keys-via-brand_json_urlsecurity.mdx:1096. The request_signature_brand_* codes live in the table at security.mdx:1119-1125; the request_signature_key_origin_* / request_signature_brand_* short-circuit is cited at security.mdx:1236. Both links land on real content.
  • The seven names were phantom. Swept docs/, static/schemas/source/, static/compliance/source/missing_signature, expired_signature, replayed_nonce, revoked_key, unknown_key, unsupported_algorithm appear only in the deleted table (request-signing.mdx:734-740). The revocation vector 017-key-revoked.json grades request_signature_*, not revoked_key — confirming the table described a protocol the conformance suite doesn't implement.
  • expired_signature collapse is correct. No separate expiry code exists; window predicates grade request_signature_window_invalid, consistent with the removal.
  • No wire surface touched. Single file, net −8, prose only. No dist/, no schema, no vector, no runner. Changeset MUST FIX doesn't apply — docs/building/ is outside the protocol-scoped path list, and the PR's check-changeset-protocol-scope.cjs output confirms it.
  • The kept additions can't drift. The byte-for-byte grading rule and the second brand_json_url pointer are both non-enumerative — neither restates a code list, so neither reintroduces the anti-pattern being removed.

Follow-ups (non-blocking — file as issues)

  • The hardcoded compliance/cache/3.0.0/ at request-signing.mdx:711,719 is correctly deferred to #6077 — this repo's grading.mdx documents verify-vector reading from stdin with no --vector flag, so the snippet's shape is unresolvable from the diff. Leaving it rather than guessing at a half-fix is the right call.
  • Adjacent count drift (39/27 vs the 28 negative files) sits in #6074's territory, not this PR's. Noted, not owned here.

Clean deletion of a load-bearing-but-wrong summary, replaced with a pointer at the source of truth. The revision from a corrected-table to a delete is the stronger version of the fix — a definition makes a table verifiable, not self-updating.

Approving on the strength of the verified anchors plus the confirmed phantom sweep.

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.

request-signing.mdx documents seven signing error codes that do not exist — implementers who follow it fail all 28 negative vectors

1 participant