refactor(idempotency): cache only successes; drop the vestigial status column (#96)

docs/adr/0007-idempotency-caching-policy.md

@@ -1,8 +1,13 @@
 # 0007 — Idempotency caching policy
 
-- Status: Accepted
+- Status: Superseded by [ADR 0038](0038-cache-only-successful-idempotent-responses.md) (2026-07-20)
 - Date: 2026-06-22
 
+> **Superseded.** This record's "cache successes *and* hard validation rejections" decision was
+> never implemented — the code caches only successes, which is the safer policy. ADR 0038 records
+> the as-built decision and drops the vestigial `status` column (#96). The rest of this record
+> (exactly-once via the claimed key, no stale-caching of budget denials) still holds.
+
 ## Context
 
 Clients retry, and a retried command must apply its effect exactly once. But

docs/adr/0038-cache-only-successful-idempotent-responses.md

@@ -0,0 +1,48 @@
+# 0038 — Cache only successful idempotent responses
+
+- Status: Accepted
+- Date: 2026-07-20
+- Supersedes: [ADR 0007](0007-idempotency-caching-policy.md)
+
+## Context
+
+ADR 0007 set the idempotency caching policy: claim a per-principal key with a unique-index
+`INSERT` in the same transaction as the effect, replay the stored response on a duplicate, and
+roll back an insufficient-budget denial so it is never stale-cached. But it also said to cache
+*hard validation rejections* (for example, an unknown `(provider, model)`), and reserved a `status`
+column on the idempotency row to distinguish a cached success from a cached rejection.
+
+That half was never built. Every command handler caches a response only on success — each denial,
+validation error included, raises and rolls the whole transaction back — so `claim` decides
+FRESH/REPLAY/MISMATCH from `command_fingerprint` and `response` alone and never reads `status`. The
+column has one value forever (`'succeeded'`) and is dead weight, and the ADR, the schema, and the
+code disagree.
+
+The as-built behaviour is also the *safer* policy. Caching a hard rejection would pin a stale
+answer past the point it stops being true: an unknown-model rejection cached under a key would keep
+replaying even after a later price-book version adds that model — exactly the stale-cache hazard
+ADR 0007 forbids for budget denials. Re-evaluating a rejected command on its next distinct-key
+attempt is correct; caching it is not.
+
+## Decision
+
+- **Cache only successes.** A command persists its idempotency record only when it succeeds; every
+  denial (insufficient budget, unknown model, unauthorized/unknown scope, key reuse) raises and
+  rolls its transaction back, so no rejection is ever cached and each is re-evaluated on retry.
+- **Drop the `status` column.** With only one cached outcome there is nothing to distinguish, so
+  the write-only `status` column is removed from both idempotency tables (`idempotency_key` and the
+  durable `metered_receipt`, ADR 0037), and `store_response` no longer takes a status argument
+  (migration `0007`).
+
+Everything else in ADR 0007 stands: exactly-once via the claimed key, no stale-caching of budget
+denials, key-reuse rejection on a differing fingerprint, and composition with the reservation
+identity-claim (ADR 0017).
+
+## Consequences
+
+- The idempotency row records exactly what it means — a completed command's response — with no
+  vestigial column implying an unbuilt rejection-caching path.
+- A retried command that previously failed validation is always re-evaluated, so it can succeed
+  once the underlying cause (an unpriced model, say) is fixed, rather than replaying a stale error.
+- If durable hard-rejection caching is ever wanted, it is a deliberate future decision that must
+  also solve the stale-cache invalidation this ADR avoids by not caching rejections at all.

docs/adr/README.md

@@ -46,6 +46,7 @@ The format follows Michael Nygard's
 - [0035 — SDK guard enforcement contract: reserve-before-dispatch, fail-closed, pluggable tokenizer](0035-sdk-guard-enforcement-contract.md)
 - [0036 — Cache-creation token class in the cost model](0036-cache-creation-token-class.md)
 - [0037 — Metering command and LiteLLM callback](0037-metering-command-and-litellm-callback.md)
+- [0038 — Cache only successful idempotent responses](0038-cache-only-successful-idempotent-responses.md)
 
 ## Adding a record
 

migrations/versions/0007_drop_idempotency_status.py

@@ -0,0 +1,35 @@
+"""drop the vestigial idempotency status column
+
+The idempotency ``status`` column was only ever written (``'succeeded'``) and never read: ``claim``
+decides FRESH/REPLAY/MISMATCH from ``command_fingerprint`` and ``response`` alone. ADR 0007 had
+reserved it for caching hard validation rejections, but the implementation only ever caches
+successes — the safer policy, since a cached unknown-model rejection would pin a stale answer even
+after a later price-book version adds the model. ADR 0038 supersedes ADR 0007 to "cache only
+successes"; this drops the now-vestigial column from both idempotency tables (#96).
+
+Revision ID: 0007_drop_idempotency_status
+Revises: 0006_metered_receipt
+Create Date: 2026-07-20
+"""
+
+from __future__ import annotations
+
+import sqlalchemy as sa
+from alembic import op
+
+revision = "0007_drop_idempotency_status"
+down_revision = "0006_metered_receipt"
+branch_labels = None
+depends_on = None
+
+_TABLES = ("idempotency_key", "metered_receipt")
+
+
+def upgrade() -> None:
+    for table in _TABLES:
+        op.drop_column(table, "status")
+
+
+def downgrade() -> None:
+    for table in _TABLES:
+        op.add_column(table, sa.Column("status", sa.Text(), nullable=True))

src/tollgate/adapters/postgres/idempotency_repo.py

@@ -82,7 +82,7 @@ async def claim(self, principal_id: str, key: str, fingerprint: str) -> Idempote
         raise EnforcementUnavailable("idempotency claim did not converge under reaper contention")
 
     async def store_response(
-        self, principal_id: str, key: str, status: str, response: Mapping[str, Any]
+        self, principal_id: str, key: str, response: Mapping[str, Any]
     ) -> None:
         """Cache a command's response on its key row so a later duplicate replays it.
 
@@ -97,7 +97,7 @@ async def store_response(
                 self._table.c.principal_id == principal_id,
                 self._table.c.key == key,
             )
-            .values(status=status, response=dict(response))
+            .values(response=dict(response))
         )
         result = await self._conn.execute(stmt)
         if result.rowcount != 1:

src/tollgate/adapters/postgres/schema.py

@@ -258,7 +258,6 @@ def _enum_check(column: str, values: tuple[str, ...], name: str) -> CheckConstra
     Column("principal_id", Text, nullable=False),
     Column("key", Text, nullable=False),
     Column("command_fingerprint", Text, nullable=False),
-    Column("status", Text, nullable=True),
     Column("response", JSONB, nullable=True),
     Column("created_at", DateTime(timezone=True), nullable=False, server_default=func.now()),
     PrimaryKeyConstraint("principal_id", "key"),
@@ -281,7 +280,6 @@ def _enum_check(column: str, values: tuple[str, ...], name: str) -> CheckConstra
     Column("principal_id", Text, nullable=False),
     Column("key", Text, nullable=False),
     Column("command_fingerprint", Text, nullable=False),
-    Column("status", Text, nullable=True),
     Column("response", JSONB, nullable=True),
     Column("created_at", DateTime(timezone=True), nullable=False, server_default=func.now()),
     PrimaryKeyConstraint("principal_id", "key"),

src/tollgate/application/handlers/cancel.py

@@ -16,7 +16,6 @@
 
 from tollgate.application.auth import AuthContext
 from tollgate.application.handlers.common import (
-    RESPONSE_SUCCEEDED,
     command_fingerprint,
     load_owned_reservation,
     ordered_lines,
@@ -120,7 +119,6 @@ async def cancel(self, auth: AuthContext, command: CancelCommand) -> CancelResul
             await tx.idempotency.store_response(
                 principal_id,
                 command.idempotency_key,
-                RESPONSE_SUCCEEDED,
                 _result_to_response(result),
             )
             return result

src/tollgate/application/handlers/commit.py

@@ -18,7 +18,6 @@
 
 from tollgate.application.auth import AuthContext
 from tollgate.application.handlers.common import (
-    RESPONSE_SUCCEEDED,
     command_fingerprint,
     load_owned_reservation,
     ordered_lines,
@@ -140,7 +139,6 @@ async def commit(self, auth: AuthContext, command: CommitCommand) -> CommitResul
             await tx.idempotency.store_response(
                 principal_id,
                 command.idempotency_key,
-                RESPONSE_SUCCEEDED,
                 _result_to_response(result),
             )
             return result

src/tollgate/application/handlers/common.py

@@ -13,7 +13,7 @@
 import hashlib
 import json
 from collections.abc import Sequence
-from typing import Any, Final
+from typing import Any
 
 from tollgate.application.auth import AuthContext, require_scope
 from tollgate.application.ports import BudgetRepository, ReservationRepository
@@ -27,11 +27,6 @@
     resolve_applicable_set,
 )
 
-#: The idempotency-row status stored with a cached success (§5.1). Only successes are cached —
-#: every denial rolls its transaction back — so this is the only status the handlers write; one
-#: constant keeps the five call sites from drifting.
-RESPONSE_SUCCEEDED: Final[str] = "succeeded"
-
 
 def command_fingerprint(payload: dict[str, Any]) -> str:
     """Hash a command's salient fields into a stable idempotency fingerprint (§5.1).

src/tollgate/application/handlers/grace.py

@@ -22,7 +22,6 @@
 
 from tollgate.application.auth import AuthContext
 from tollgate.application.handlers.common import (
-    RESPONSE_SUCCEEDED,
     command_fingerprint,
     resolve_applicable_nodes,
 )
@@ -143,7 +142,6 @@ async def backfill(
             await tx.metered_receipt.store_response(
                 principal_id,
                 command.idempotency_key,
-                RESPONSE_SUCCEEDED,
                 _result_to_response(result),
             )
             return result

src/tollgate/application/handlers/meter.py

@@ -20,7 +20,6 @@
 
 from tollgate.application.auth import AuthContext
 from tollgate.application.handlers.common import (
-    RESPONSE_SUCCEEDED,
     command_fingerprint,
     resolve_applicable_nodes,
 )
@@ -138,7 +137,6 @@ async def meter(self, auth: AuthContext, command: MeterCommand) -> MeterResult:
             await tx.metered_receipt.store_response(
                 principal_id,
                 command.idempotency_key,
-                RESPONSE_SUCCEEDED,
                 _result_to_response(result),
             )
             return result

src/tollgate/application/handlers/reserve.py

@@ -20,7 +20,6 @@
 
 from tollgate.application.auth import AuthContext
 from tollgate.application.handlers.common import (
-    RESPONSE_SUCCEEDED,
     command_fingerprint,
     resolve_applicable_nodes,
 )
@@ -205,7 +204,6 @@ async def reserve(self, auth: AuthContext, command: ReserveCommand) -> ReserveRe
             await tx.idempotency.store_response(
                 principal_id,
                 command.idempotency_key,
-                RESPONSE_SUCCEEDED,
                 _result_to_response(result),
             )
             return result

src/tollgate/application/ports.py

@@ -166,7 +166,7 @@ async def claim(self, principal_id: str, key: str, fingerprint: str) -> Idempote
         ...
 
     async def store_response(
-        self, principal_id: str, key: str, status: str, response: Mapping[str, Any]
+        self, principal_id: str, key: str, response: Mapping[str, Any]
     ) -> None:
         """Cache a command's response on its key row so a later duplicate replays it."""
         ...

tests/integration/test_idempotency_concurrency.py

@@ -26,7 +26,7 @@ async def _claim_once(
         repo = PostgresIdempotencyRepository(conn)
         claim = await repo.claim(principal_id, key, fingerprint)
         if claim.outcome is ClaimOutcome.FRESH:
-            await repo.store_response(principal_id, key, "succeeded", {"reservation_id": "r1"})
+            await repo.store_response(principal_id, key, {"reservation_id": "r1"})
         await txn.commit()
         return claim.outcome
 

tests/integration/test_idempotency_repo.py

@@ -27,7 +27,7 @@ async def test_claim_same_key_same_fingerprint_replays_stored_response(
 ) -> None:
     repo = PostgresIdempotencyRepository(db_conn)
     await repo.claim("p1", "k1", "fp-1")
-    await repo.store_response("p1", "k1", "succeeded", {"reservation_id": "r1"})
+    await repo.store_response("p1", "k1", {"reservation_id": "r1"})
     claim = await repo.claim("p1", "k1", "fp-1")
     assert claim.outcome is ClaimOutcome.REPLAY
     assert claim.response == {"reservation_id": "r1"}
@@ -39,7 +39,7 @@ async def test_store_response_on_an_unclaimed_key_fails_loud(db_conn: AsyncConne
     # and raising beats silently dropping the response and leaving a keyless success (#107).
     repo = PostgresIdempotencyRepository(db_conn)
     with pytest.raises(TollgateError):
-        await repo.store_response("p1", "never-claimed", "succeeded", {"x": 1})
+        await repo.store_response("p1", "never-claimed", {"x": 1})
 
 
 async def test_claim_same_key_different_fingerprint_is_mismatch(db_conn: AsyncConnection) -> None:

tests/integration/test_reserve_handler.py

@@ -158,7 +158,7 @@ async def test_reserve_persists_the_whole_envelope(committing_engine: AsyncEngin
     assert await _scalar(committing_engine, "SELECT count(*) FROM ledger WHERE kind='reserve'") == 2
     assert (
         await _scalar(
-            committing_engine, "SELECT count(*) FROM idempotency_key WHERE status='succeeded'"
+            committing_engine, "SELECT count(*) FROM idempotency_key WHERE response IS NOT NULL"
         )
         == 1
     )
@@ -224,7 +224,7 @@ async def test_insufficient_budget_denial_is_not_stale_cached(
     assert result.estimated_micro == 300
     assert (
         await _scalar(
-            committing_engine, "SELECT count(*) FROM idempotency_key WHERE status='succeeded'"
+            committing_engine, "SELECT count(*) FROM idempotency_key WHERE response IS NOT NULL"
         )
         == 1
     )

tests/integration/test_terminal_handlers.py

@@ -211,7 +211,7 @@ async def test_commit_reconciles_and_persists_the_envelope(
     assert await _scalar(committing_engine, "SELECT count(*) FROM ledger WHERE kind='overage'") == 0
     assert (
         await _scalar(
-            committing_engine, "SELECT count(*) FROM idempotency_key WHERE status='succeeded'"
+            committing_engine, "SELECT count(*) FROM idempotency_key WHERE response IS NOT NULL"
         )
         == 2
     )  # the reserve and the commit

tests/unit/test_cancel.py

@@ -96,15 +96,15 @@ def __init__(
         self, outcome: ClaimOutcome = ClaimOutcome.FRESH, response: Mapping[str, Any] | None = None
     ) -> None:
         self._claim = IdempotencyClaim(outcome, response=response)
-        self.stored: list[tuple[str, str, str, dict[str, Any]]] = []
+        self.stored: list[tuple[str, str, dict[str, Any]]] = []
 
     async def claim(self, principal_id: str, key: str, fingerprint: str) -> IdempotencyClaim:
         return self._claim
 
     async def store_response(
-        self, principal_id: str, key: str, status: str, response: Mapping[str, Any]
+        self, principal_id: str, key: str, response: Mapping[str, Any]
     ) -> None:
-        self.stored.append((principal_id, key, status, dict(response)))
+        self.stored.append((principal_id, key, dict(response)))
 
     async def delete_expired(self, cutoff: datetime, limit: int) -> int:
         raise AssertionError("this handler never reaps keys")
@@ -300,7 +300,7 @@ async def test_cancel_releases_every_line_and_persists_the_envelope() -> None:
     assert all(e.reservation_id == "res-1" for e in ctx.ledger.appended)
     assert all(e.price_book_version == "2026-06-22" for e in ctx.ledger.appended)
     assert ctx.idempotency.stored == [
-        ("u1", "idem-cancel", "succeeded", {"reservation_id": "res-1", "released_micro": 300})
+        ("u1", "idem-cancel", {"reservation_id": "res-1", "released_micro": 300})
     ]
     assert uow.committed is True
 

tests/unit/test_commit.py

@@ -112,15 +112,15 @@ def __init__(
         self, outcome: ClaimOutcome = ClaimOutcome.FRESH, response: Mapping[str, Any] | None = None
     ) -> None:
         self._claim = IdempotencyClaim(outcome, response=response)
-        self.stored: list[tuple[str, str, str, dict[str, Any]]] = []
+        self.stored: list[tuple[str, str, dict[str, Any]]] = []
 
     async def claim(self, principal_id: str, key: str, fingerprint: str) -> IdempotencyClaim:
         return self._claim
 
     async def store_response(
-        self, principal_id: str, key: str, status: str, response: Mapping[str, Any]
+        self, principal_id: str, key: str, response: Mapping[str, Any]
     ) -> None:
-        self.stored.append((principal_id, key, status, dict(response)))
+        self.stored.append((principal_id, key, dict(response)))
 
     async def delete_expired(self, cutoff: datetime, limit: int) -> int:
         raise AssertionError("this handler never reaps keys")
@@ -351,7 +351,6 @@ async def test_commit_reconciles_and_persists_the_envelope() -> None:
         (
             "u1",
             "idem-commit",
-            "succeeded",
             {"reservation_id": "res-1", "committed_micro": _ACTUAL, "overage_micro": 0},
         )
     ]

tests/unit/test_extend.py

@@ -107,7 +107,7 @@ async def claim(self, principal_id: str, key: str, fingerprint: str) -> Idempote
         raise AssertionError("extend is naturally idempotent and needs no key (§4)")
 
     async def store_response(
-        self, principal_id: str, key: str, status: str, response: Mapping[str, Any]
+        self, principal_id: str, key: str, response: Mapping[str, Any]
     ) -> None:
         raise AssertionError("extend caches no response")