Skip to content

docs(skills): document mutate-last idempotency contract (#744)#745

Merged
bokelley merged 1 commit into
mainfrom
bokelley/idempotency-terminal-audit
Apr 21, 2026
Merged

docs(skills): document mutate-last idempotency contract (#744)#745
bokelley merged 1 commit into
mainfrom
bokelley/idempotency-terminal-audit

Conversation

@bokelley

@bokelley bokelley commented Apr 21, 2026

Copy link
Copy Markdown
Contributor

Summary

Audit from #744 (should recovery: terminal errors cache instead of release the idempotency claim?) — conclusion: keep release-all-errors, document the handler-author contract that #743 widened the surface for.

Audit reasoning

  • Current dispatcher (create-adcp-server.ts:1865-1936): success → idempotency.save; any error (isError: true envelope OR throw) → idempotency.release.
  • The AdCP recovery: terminal catalog is mostly state-dependentACCOUNT_SUSPENDED, BUDGET_EXHAUSTED, ACCOUNT_PAYMENT_REQUIRED, ACCOUNT_SETUP_REQUIRED all flip after out-of-band remediation. Caching them would return stale errors for the full replay TTL — worse UX than re-executing.
  • Only UNSUPPORTED_FEATURE and ACCOUNT_NOT_FOUND are truly immutable, and re-executing them is cheap (capability check, lookup).
  • The double-side-effect risk (await db.insert(...); throw adcpError(...)) is a handler-correctness issue, not a dispatcher issue. feat(server): auto-unwrap thrown adcpError envelopes in dispatcher #743 blesses throw adcpError(...) as a supported path, which widens that surface — so the mutate-last contract needs to be explicit in the skill corpus.

Changes

  • skills/build-seller-agent/SKILL.md idempotency section — extends the existing "only caches successful responses" bullet with the terminal-catalog rationale, then adds a new "Handler contract: mutate last" paragraph with a worked budgetApproved broken-vs-correct example and guidance for rare partial-write paths (natural-key upsert, ctx.store.get → merge).
  • skills/build-creative-agent/SKILL.md:367 — swaps the now-stale "throw adcpError(...) bypasses the envelope and surfaces SERVICE_UNAVAILABLE" comment (invalidated by feat(server): auto-unwrap thrown adcpError envelopes in dispatcher #743's auto-unwrap) for the still-true claim-release rationale.
  • Changeset: patch-level (@adcp/client), docs-only.

What's NOT in this PR

  • No edits to compliance/cache/latest/universal/idempotency.yaml. That file is machine-synced from adcontextprotocol/adcp via npm run sync-schemas — any local edit is clobbered on the next sync. Locking error-claim-release semantics in as a spec-level conformance invariant belongs upstream. Filed as adcontextprotocol/adcp#2760 — proposes either a new error_claim_release phase (using comply_test_controller to force a terminal error, then retrying with the same key + different payload, asserting the second request executes) or a narrative-bullet + reviewer-checklist variant if the controller hook is too invasive.

Test plan

  • npm run typecheck clean (pre-push hook)
  • npm run build:lib clean (pre-push hook)
  • Docs-only — no behavior changes, so no test additions
  • Skill matrix run after merge to confirm the mutate-last guidance lands in fresh-Claude builds

Related

🤖 Generated with Claude Code

Addresses the audit question in #744: should `recovery: terminal`
errors cache on replay, or keep release-all-errors? Audit conclusion:
keep release-all-errors (most AdCP terminals are state-dependent;
caching would return stale errors post-remediation). Document the
handler-author implication that #743 widened the surface for.

- skills/build-seller-agent: new "mutate last" paragraph in the
  idempotency section, with broken-vs-correct budget-approval example
  and guidance for partial-write paths.
- skills/build-creative-agent: swap the now-stale
  "throw surfaces as SERVICE_UNAVAILABLE" rationale (invalidated by
  #743's auto-unwrap) for the still-true claim-release rationale.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@bokelley bokelley merged commit faef971 into main Apr 21, 2026
13 checks passed
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