Skip to content

fix(training-agent): enforce idempotency replay/conflict/expired semantics (#2346)#2367

Merged
bokelley merged 2 commits intomainfrom
bokelley/issue-2346
Apr 19, 2026
Merged

fix(training-agent): enforce idempotency replay/conflict/expired semantics (#2346)#2367
bokelley merged 2 commits intomainfrom
bokelley/issue-2346

Conversation

@bokelley
Copy link
Copy Markdown
Contributor

Closes #2346.

Summary

The hosted training agent at test-agent.adcontextprotocol.org advertised
adcp.idempotency.replay_ttl_seconds: 86400 in get_adcp_capabilities but
did not enforce the contract. Two create_media_buy calls with the same key
and same payload created two distinct media buys; key reuse with a different
payload succeeded instead of returning IDEMPOTENCY_CONFLICT. Buyer SDKs
integrating against the reference agent shipped without ever exercising
their retry/conflict code paths.

A new middleware at server/src/training-agent/idempotency.ts implements the
normative behavior from docs/building/implementation/security.mdx
§"Request Safety" and the static/compliance/source/universal/idempotency.yaml
storyboard:

  • Key validation — presence + ^[A-Za-z0-9_.:-]{16,255}$ regex checked
    before the cache lookup on all 26 mutating tools; the MUTATING_TOOLS set
    is drift-guarded by a test that introspects tools/list and asserts every
    tool whose schema requires idempotency_key is listed.
  • Canonicalization — RFC 8785 JCS via the canonicalize npm package +
    SHA-256. Excludes idempotency_key, context, governance_context, and
    push_notification_config.authentication.credentials. Hash comparison
    uses timingSafeEqual on decoded digests.
  • Replay — same key + equivalent canonical payload returns the cached
    inner response with replayed: true on the envelope (omitted on fresh
    executions so strict additionalProperties: false schemas keep passing).
  • Conflict — same key + different canonical payload → IDEMPOTENCY_CONFLICT
    with code + message only; no cached payload, hash, or field pointer
    (any shape hint would turn key-reuse into a read oracle).
  • Expiry — past TTL with ±60s skew → IDEMPOTENCY_EXPIRED and evict.
  • Cache-only-success — errors re-execute on retry; error-in-body
    responses ({ errors: [...] }) are not cached.
  • Scope(auth principal, account scope, idempotency_key). The
    public sandbox token is shared across all callers, so scoping only by
    auth principal would make the three-state response observable
    cross-caller (security.mdx §"three-state response"); per-account
    partitioning closes that oracle.
  • Cap — 10k entries per principal, with opportunistic eviction of
    expired entries on overflow. When the cap is still hit, responds with
    RATE_LIMITED (rule 8) rather than silently dropping the insert and
    letting a retry re-execute.

Review cycle

Both code-reviewer and security-reviewer pointed out real issues that
landed in the final diff:

  • added structuredContent on replay path (code-review M2)
  • fixed error-in-body caching bypass (code-review M3)
  • scoped principal by account, closing the shared-token oracle (code M4 /
    security M1 + M2)
  • silent-drop-on-cap → RATE_LIMITED (code M5 / security S2)
  • dropped field: idempotency_key from conflict body (code M7)
  • timingSafeEqual for hash compare (security S1)
  • throw on canonicalize failure instead of collapsing to '' (code N10)
  • parameterized missing-key test for every mutating tool (code M8)
  • drift-guard test for MUTATING_TOOLS vs tools/list (code M1)

Test plan

  • npm run typecheck — clean
  • npx vitest run server/tests/unit/ — 1569 pass (56 new in
    idempotency.test.ts + training-agent-idempotency.test.ts)
  • npm run test:unit — 587 pass
  • npm run test:schemas, test:json-schema — pass

🤖 Generated with Claude Code

…ntics (#2346)

The hosted training agent declared adcp.idempotency.replay_ttl_seconds in
its capabilities response but did not enforce the contract, silently
double-booking retries with the same key. Middleware now validates key
presence + format, returns cached responses with replayed: true on payload
match, IDEMPOTENCY_CONFLICT on payload drift, IDEMPOTENCY_EXPIRED past TTL.
Cache is scoped by (auth principal, account, key) so shared sandbox tokens
cannot be used as a cross-caller oracle.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Comment thread server/tests/unit/training-agent.test.ts Fixed
…tion or class'

Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
@bokelley bokelley merged commit 251beea into main Apr 19, 2026
13 checks passed
@bokelley bokelley deleted the bokelley/issue-2346 branch April 19, 2026 10:08
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.

Training agent declares idempotency capability but does not enforce replay/conflict semantics

1 participant