Skip to content

docs(errors): follow-up to #2595 — transport-errors cross-ref, alias template, narrative drift#2596

Merged
bokelley merged 2 commits into
mainfrom
bokelley/error-model-followup
Apr 20, 2026
Merged

docs(errors): follow-up to #2595 — transport-errors cross-ref, alias template, narrative drift#2596
bokelley merged 2 commits into
mainfrom
bokelley/error-model-followup

Conversation

@bokelley

Copy link
Copy Markdown
Contributor

Summary

Follow-up to the merged #2595 (error model) addressing protocol-review feedback that landed after merge.

What changed

  1. transport-errors.mdx — new "Envelope vs. payload errors" section with a table + cross-link to the normative two-layer model in error-handling.mdx. Previously the normative text only lived in error-handling.mdx; readers who landed on transport-errors had no pointer to the payload layer.

  2. transport-errors.mdx — new "Storyboard `check: error_code` contract" section. Promotes the shape-agnostic extraction rule from a YAML comment in storyboard-schema.yaml to spec-grade text so @adcp/client implementers can find it without grepping compliance source.

  3. transport-errors.mdx — adds a sixth client-detection step for payload.errors[0] as a payload-layer fallback, making the two-layer detection order explicit in prose.

  4. state-machine.yaml — prose drift fix. Replaces INVALID_STATE_TRANSITIONINVALID_STATE in narrative/expected strings (validator assertions were already INVALID_STATE; the prose was out of sync).

  5. comply-test-controller.mdx — controller-enum clarification note. The controller's error field uses a controller-specific vocabulary defined in comply-test-controller-response.json (INVALID_TRANSITION, INVALID_STATE, NOT_FOUND, UNKNOWN_SCENARIO, INVALID_PARAMS, FORBIDDEN, INTERNAL_ERROR). This is distinct from the canonical seller error-code.json enum — the transition-vs-state distinction is meaningful at the state-machine primitive layer. Storyboard assertions on controller responses use `path: "error"`, not `check: error_code`.

  6. New static/schemas/source/enums/error-code-aliases.json template. Self-describing JSON Schema with an empty aliases map. Documents the alias shape so future renames have a home without ad-hoc invention. Lint continues to warn-only on aliased codes. tests/schema-validation.test.cjs updated to skip files whose $id ends in -aliases.json from the enum-values check.

  7. storyboard-authoring.md — updated alias-file docs with the actual JSON shape so authors don't have to grep for it.

Why this is a follow-up, not an amend

PR #2595 was merged before expert review completed. Rather than revert or amend, this PR lands the review feedback on top of main. Nothing in this PR is load-bearing — it's clarifying the normative rules that already exist, fixing drift between narrative prose and validator assertions, and putting the alias-file convention on disk so the next rename has a clear home.

Test plan

  • npm run test:schemas — all 7 tests pass (the enum-values test now skips *-aliases.json files)
  • npm run test:examples — 31 validations pass
  • npm run test:error-codes — clean (0 errors, 1 pre-existing YAML warning)
  • npm run test:unit — 627 pass
  • npm run typecheck clean
  • npm run build:compliance clean

Expert reviews that drove this: ad-tech-protocol-expert + code-reviewer post-hoc review of #2595.

🤖 Generated with Claude Code

…template, narrative drift

Addresses protocol-review feedback on the merged error-model PR #2595:

1. transport-errors.mdx — add "Envelope vs. payload errors" section
   cross-linking to the normative two-layer model. Previously the
   normative text only lived in error-handling.mdx; readers who landed
   on transport-errors had no pointer.

2. transport-errors.mdx — add "Storyboard check: error_code contract"
   section. Promotes the shape-agnostic extraction contract from a
   storyboard-schema.yaml comment to spec-grade text so @adcp/client
   implementers can find it without grepping compliance source.

3. transport-errors.mdx — new client-detection step 6 for payload.errors[0]
   as a fallback path, making the two-layer detection explicit in code.

4. state-machine.yaml — replace INVALID_STATE_TRANSITION → INVALID_STATE
   in narrative/expected prose (validator assertions were already
   INVALID_STATE; the prose was drifting).

5. comply-test-controller.mdx — add a Note clarifying the controller
   response enum (INVALID_TRANSITION/INVALID_STATE/NOT_FOUND/etc. per
   comply-test-controller-response.json) is controller-specific and
   distinct from the canonical seller error-code.json. The transition-
   vs-state distinction is meaningful at the state-machine primitive
   layer; storyboard assertions on controller responses use path:
   "error", not check: error_code.

6. static/schemas/source/enums/error-code-aliases.json — new template
   file with an empty aliases map and a self-describing JSON Schema.
   Documents the alias shape so future renames have a home without ad-
   hoc invention. tests/schema-validation.test.cjs updated to skip
   files whose $id ends in -aliases.json from the enum-values check.

All tests clean: schemas, examples, error-codes lint, unit, typecheck.
@github-actions

github-actions Bot commented Apr 20, 2026

Copy link
Copy Markdown
Contributor

Schema Link Check Results

Commit: 611adc3 - refactor(lint): move alias registry out of schemas/, simplify to pure data

⚠️ Warnings (schema not yet released)

These schemas exist in source but haven't been released yet. The links will be broken until the next version is published:

  • https://adcontextprotocol.org/schemas/v3/enums/specialism.json
    • Schema exists in latest (source) but not yet released in v3
    • Action: This link will work after next 3.x release is published

To fix: Either:

  1. Wait for the next release and merge this PR after the release is published
  2. Use latest instead of a version alias if you need the link to work immediately (note: latest is the development version and may change)
  3. Coordinate with maintainers to cut a new release before merging

@bokelley

Copy link
Copy Markdown
Contributor Author

Reviewed as the follow-up to merged #2595 closing issue #2588 (error-code taxonomy + lint). One real question and one cross-cutting concern with the sister PR #2593.

Question — is aliases.json meant to ship empty at creation?

static/schemas/source/enums/error-code-aliases.json line 19 ships "aliases": {} — no entries. The $comment on line 20 says this is intentional: "Add entries here only when actively renaming a code — do not pre-register aliases 'just in case'."

That's defensible if no renames are currently in flight. But the original issue (#2588) specifically cited INVALID_TRANSITIONINVALID_STATE_TRANSITION as the motivating drift — and PR #2593 currently contains 7 uses of INVALID_TRANSITION in docs/building/implementation/comply-test-controller.mdx (lines 425, 485, 535, 552, 575, 579, 621). Three interpretations:

Worth resolving in this PR — shipping an empty alias file while a sister PR generates drift instances is the opposite of closing #2588.

Structural — hybrid schema+instance file

The file is both a JSON Schema (lines 1–18 declaring aliases as a property with a specific shape) AND an instance of itself (line 19 "aliases": {} at the instance level, permitted by additionalProperties: true at line 18). Self-validating via dogfooding, but unusual and likely to confuse tooling that expects schemas/ paths to contain pure schemas.

If you want to keep this pattern:

  • Document it explicitly ("this file is both the schema and the canonical data — the aliases key at the instance level is read by the lint; the properties.aliases definition constrains what the lint accepts").

Cleaner alternatives:

  • Split: schemas/enums/error-code-aliases.schema.json (meta) + data/error-code-aliases.json (instance, references the meta via $schema).
  • Drop the schema and ship pure data: {"aliases": {}} in an untyped JSON file, validated by the lint script directly.

Not blocking on its own — just easy to clean up before other lint registries adopt the same hybrid pattern.

Compliment

Transport-errors cross-ref in transport-errors.mdx: the clear separation of transport-layer (HTTP 4xx/5xx, JSON-RPC parse errors) from application-layer (our error codes) is the right framing. Helps agents know which codes to emit where.

Migration note

Our training agent emits the canonical INVALID_STATE_TRANSITION per PR #2582's double-cancel guard (commit e4a8b4e). No change needed on our side regardless of which interpretation above lands — the lint will either warn-via-alias or pass cleanly.

… data

Reviewer concern on PR #2596: error-code-aliases.json was a hybrid
schema+instance file (JSON Schema declaring its own shape AND an
instance of itself, permitted by additionalProperties: true). Unusual,
and likely to confuse tooling that expects /schemas/ paths to be pure
schemas.

Fix: move to scripts/error-code-aliases.json — pure data, sits next
to the lint script that reads it. No JSON Schema wrapper; just
{"aliases": {}} with a $_comment$ documenting the shape.

Touches:
- scripts/error-code-aliases.json (new, pure data)
- scripts/lint-error-codes.cjs (ALIAS_PATH points at new location)
- static/schemas/source/enums/error-code-aliases.json (removed)
- tests/schema-validation.test.cjs (revert the `-aliases.json` skip —
  the file no longer sits under /schemas/, so the enum-values test
  doesn't need to exclude it)
- docs/contributing/storyboard-authoring.md (reference path updated)
@bokelley

Copy link
Copy Markdown
Contributor Author

On the "is aliases.json meant to ship empty?" question — yes, intentionally empty at creation, and the INVALID_TRANSITION → INVALID_STATE_TRANSITION alias the review suggested doesn't need to exist. Ground truth:

  • Canonical seller enum (error-code.json): INVALID_STATE only. No INVALID_STATE_TRANSITION entry.
  • Training-agent code (task-handlers.ts:1994): emits INVALID_STATE. No INVALID_STATE_TRANSITION anywhere in server/src/training-agent/.
  • Controller-specific enum (comply-test-controller-response.json:84): INVALID_TRANSITION (distinct from INVALID_STATE, intentionally).
  • INVALID_STATE_TRANSITION appears only in 5 prose lines of state-machine.yaml — pure documentation drift, not a former canonical code. This PR fixes that drift (narrative prose → INVALID_STATE to match the validators already on main).

Interpretation (b) from the review ("INVALID_STATE_TRANSITION is canonical per #2595") is based on a misreading — #2595 changed the validator assertions to INVALID_STATE, not INVALID_STATE_TRANSITION. No alias entry is needed because there's no canonical-code rename; the prose was just out of sync with the validators, which this PR reconciles.

Structural — hybrid schema/instance file: fair critique. Addressed in 611adc3 — moved the alias registry from static/schemas/source/enums/error-code-aliases.json to scripts/error-code-aliases.json. It's now pure data (no JSON Schema wrapper) sitting next to the lint that reads it. Also reverts the -aliases.json skip in tests/schema-validation.test.cjs since the file no longer lives under /schemas/.

@bokelley bokelley merged commit a13f651 into main Apr 20, 2026
16 checks passed
@bokelley

Copy link
Copy Markdown
Contributor Author

Correction — retracting my earlier question about the empty aliases.json. I was reviewing under the wrong assumption. Actual state:

  • error-code.json (seller canonical): INVALID_STATE.
  • comply-test-controller-response.json (controller canonical, distinct): INVALID_TRANSITION.
  • INVALID_STATE_TRANSITION: appears nowhere in any enum or code — only in 5 prose lines of state-machine.yaml. That's drift, not a rename.

This PR is fixing the drift, not cataloguing a rename. Empty aliases.json is correct — there's no deprecation window to manage because no code was ever renamed. The INVALID_TRANSITION uses in PR #2593's comply-test-controller.mdx are also correct (they're the controller's distinct canonical code).

Apologies for the noise — structural nit about the hybrid schema+instance file still stands (worth a separate cleanup), but the core finding was wrong. Reading #2596 as ship-ready.

Side note — our training-agent PR emitted INVALID_STATE_TRANSITION on double-cancel per the drifted state-machine.yaml prose; fixed locally to INVALID_STATE to match the canonical seller enum.

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.

1 participant