POST /api/v1/change-requests accepts no idempotency_key and creates duplicate change requests on replay

[jaanijuk]

Summary

POST /api/v1/change-requests is the fourth of four money-mutating surfaces the shared contract enumerates under FD-12. The other three implement it; this one has no idempotency_key field at all, so a replayed request silently creates a second change request and routes a second approval for the same logical change.

Observed: the contract names this endpoint

iab-agentic-primitives v0.5.1 (ee9ea3c) defines the covered set explicitly in PROTOCOL_RECONCILIATION.md, decision row P2:

every money-mutating request inherits IdempotentRequest — QuoteRequest, DealBookingRequest, NegotiationMessage, ChangeRequestCreate all carry a REQUIRED idempotency_key

The section headed "Idempotency replay semantics (FD-12) - normative summary" lists /api/v1/change-requests alongside /api/v1/quotes, /api/v1/deals and /api/v1/negotiations/messages. And protocol/deals.py:98 declares:

class ChangeRequestCreate(IdempotentRequest):
    """Request body for ``POST /api/v1/change-requests`` (money-mutating: FD-12)."""

Observed: the seller does not use it

Three of the four routers import the contract type directly and implement FD-12. The fourth does not:

Router	Request type	FD-12
quotes.py:18	QuoteRequest (contract)	implemented
deals.py:20	DealBookingRequest (contract)	implemented
negotiation.py:20	NegotiationMessage (contract)	implemented
change_requests.py:12	CreateChangeRequestModel (local)	absent

schemas.py:355 carries six fields — order_id, change_type, diffs, proposed_values, reason, requested_by — and no key. idempotency_key does not appear anywhere in schemas.py, and no service module implements idempotency, so it is not handled at a lower layer either.

The model does not set extra="forbid", so a caller who supplies idempotency_key has it silently discarded rather than rejected.

Observed: runtime

Against v2.4.2 (e5b367d), two identical requests carrying the same idempotency_key:

call 1 -> CR-236B96AC3FCC  status: pending_approval  severity: material
call 2 -> CR-299478430C51  status: pending_approval  severity: material

Two distinct change requests, both classified material, both routed to approval. So a retried request does not only duplicate a record — it places two identical material changes into the seller's approval queue for one logical change, leaving a human to determine which to action.

For contrast, on the same build:

POST /api/v1/quotes with no idempotency_key -> 422

Reproduction

# 1. Create an order. deal_id is required here; see the note below.
ORD=$(curl -s -X POST localhost:8001/api/v1/orders \
  -H "Content-Type: application/json" -d '{"deal_id":"DEAL-PROBE-1"}' \
  | python3 -c "import json,sys; print(json.load(sys.stdin)['order_id'])")

# 2. Send the same body with the same key twice.
BODY=$(printf '{"order_id":"%s","change_type":"impressions","reason":"probe","idempotency_key":"probe-key-0001"}' "$ORD")
for i in 1 2; do
  curl -s -X POST localhost:8001/api/v1/change-requests \
    -H "Content-Type: application/json" -d "$BODY" \
    -o "/tmp/cr$i.json" -w "call $i: HTTP %{http_code}  "
  python3 -c "import json; d=json.load(open('/tmp/cr$i.json')); print(d['change_request_id'], d['status'], d['severity'])"
done

Observed on v2.4.2:

call 1: HTTP 200  CR-236B96AC3FCC pending_approval material
call 2: HTTP 200  CR-299478430C51 pending_approval material

Two different change_request_id values from an identical body under the same key is the finding.

Note: the order must carry a deal_id. Creating one without it produces an unhandled 500 on this endpoint; see #61.

Environment

seller-agent v2.4.2, commit e5b367d2b780aa4d0e03b6363d744ba3adf2b221, installed with uv sync --locked, resolving iab-agentic-primitives==0.5.1 (ee9ea3c). Python 3.12.3, WSL2 Ubuntu 24.04.

Also confirmed on main, which is currently the same commit (e5b367d) at time of reporting: change_requests.py imports the local CreateChangeRequestModel, and idempotency_key does not appear in schemas.py.

Inferred

PROTOCOL_RECONCILIATION.md describes the retired makegood and cancellation sub-routes as re-pointed at /api/v1/change-requests, with a note that the buyer's client call should stay disabled "until the seller implements change requests". A change-requests surface does exist and persists state, so it is not clear whether it predates the contract and a conformant implementation is still planned, or whether it was implemented and FD-12 was missed. The gap is the same either way, but the intended status would be useful to know.

Suggested remedy

The pattern exists three times in this codebase, and #50 (quotes) is the closest analogue. It also left reusable helpers in contract_mappers.py — idempotency_conflict_detail(), request_payload_hash() and idempotency_buyer_scope() — and established the ordering that review on that PR found necessary: resolve and verify the caller before the idempotency lookup, then scope the storage key by the resulting identity (quotes.py:71 and :89).

Two parts:

• Accept idempotency_key on the request, either by adopting the contract's ChangeRequestCreate or by adding the field to CreateChangeRequestModel.
• Replay an identical request rather than minting a new change_request_id, and return idempotency_conflict (409) where the same key arrives with a different payload.

One scoping question I do not have an answer to. /quotes scopes by verified buyer context, but this endpoint uses _get_optional_api_key_record rather than _verified_buyer_context, so the same verified principal is not available here — the same constraint that came up for book_deal on #50. Since order_id is required and the order already carries the buyer relationship, scoping by order may be more natural than scoping by caller identity. Flagging it rather than proposing it, as the right answer depends on how change requests are intended to be authorised.

Acceptance criteria

• An identical POST /api/v1/change-requests replayed under the same idempotency_key returns the original change request and does not create a second approval.
• The same key with a materially different payload returns idempotency_conflict.
• A request omitting idempotency_key is handled consistently with the other three FD-12 surfaces.

[atc964]

Confirmed, and to answer the implicit question directly: this was missed, not deferred. When the contract types were adopted across the money surfaces, change-requests kept its local request model and the gap survived because nothing exercised replay on that route. Your framing is right on all counts. Direction: mirror the deals/quotes pattern (replay on matching payload hash, 409 idempotency_conflict on a reused key with a changed payload), scoped per order rather than per buyer, given the route's optional auth, with the same 24 hour TTL. A PR is welcome; it pairs naturally with #61 since both touch the same router, so feel free to stack them if that is cleaner for you. It will get the same reproduction-based review the earlier idempotency PRs got.