fix: attach per-seller credentials on booking calls

[atc964]

What breaks today

Seller-agent PR #77 (merged to seller main today as cf44c27, "fix: require verified buyer on deal booking; operator-gate the MCP deal tool") added an enforcement gate to POST /api/v1/deals:

# ad_seller/interfaces/api/routers/deals.py
if api_key_record is None:
    raise HTTPException(
        status_code=401,
        detail={"error": "authentication_required", "message": "API key required to book a deal."},
    )

The buyer never attaches any credential to that request, so every real booking against a seller running #77 now fails with 401 authentication_required. This was proven on a live rig fleet, not caught by either repo's own CI (see below).

Root cause (file/line citations)

The outbound POST is made by DealsClient.book_deal (src/ad_buyer/clients/deals_client.py:265), sent through an httpx.AsyncClient built once in DealsClient.__init__ (deals_client.py:122-153). That constructor already knows how to attach a credential:

if api_key:
    headers["X-Api-Key"] = api_key
elif bearer_token:
    headers["Authorization"] = f"Bearer {bearer_token}"

But nothing ever supplies api_key/bearer_token. MultiSellerOrchestrator's three call sites that construct a DealsClient for quoting, negotiation, and booking called the factory with zero kwargs:

• src/ad_buyer/orchestration/multi_seller.py:1104 (pre-fix) — quote stage
• src/ad_buyer/orchestration/multi_seller.py:1524 (pre-fix) — negotiation re-quote
• src/ad_buyer/orchestration/multi_seller.py:1733 (pre-fix) — booking

Both production factories forward **kwargs faithfully (flows/deal_booking_flow.py:122, interfaces/chat/main.py:413-418), so the plumbing to pass a credential through already existed end-to-end — it just was never fed one. Meanwhile ApiKeyStore (src/ad_buyer/auth/key_store.py) already had per-seller keys on disk (~/.ad_buyer/seller_keys.json), and AuthMiddleware (src/ad_buyer/auth/middleware.py) already knew how to pull a key from that store and attach it — but grep -rn "AuthMiddleware(" src/ returns zero matches. It was fully implemented, dead code.

Why neither repo's CI catches this

Each repo tests only its own side of the contract: the buyer's unit/integration suite mocks or stubs the seller's HTTP surface and never asserts on outbound auth headers reaching a real seller enforcing #77; the seller's suite tests its own auth gate in isolation and has no visibility into what the buyer actually sends. The gap is only visible when both real services talk to each other, which is what the rig's real-mode integration gate does and CI does not.

What changed

• MultiSellerOrchestrator.__init__ gained an optional key_store: ApiKeyStore | None parameter, defaulting to ApiKeyStore() (~/.ad_buyer/seller_keys.json) when not injected — the same default construction interfaces/mcp_server.py::_get_api_key_store() already uses for its key-management tools.
• New _client_for_booking(seller_url) helper looks up self._key_store.get_key(seller_url) and forwards it as api_key= to the existing deals_client_factory, which DealsClient already turns into an X-Api-Key header.
• All three call sites in orchestration/multi_seller.py now go through _client_for_booking instead of calling the factory directly.
• A seller with no stored key still gets a client built exactly as before (no credential) — behavior is unchanged for deployments with no per-seller keys configured — but that path now logs at INFO so a resulting 401 has a local, immediate explanation instead of being a bare surprise.
• flows/deal_booking_flow.py::build_default_orchestrator and interfaces/chat/main.py's orchestrator construction needed no direct edit — both construct MultiSellerOrchestrator without a key_store kwarg, so both pick up the constructor's ApiKeyStore() default automatically. Kept the diff to a single file for the logic change.

Why the header, not a guess

Confirmed by reading the seller's actual dependency (ad_seller/interfaces/api/deps.py::_get_optional_api_key_record, read-only, not modified in this PR): it accepts either Authorization: Bearer <key> (aliased) or X-Api-Key: <key>. DealsClient already sends X-Api-Key when given api_key=, which is exactly what ApiKeyStore/AuthMiddleware were built around (header_type: Literal["api_key", "bearer"] = "api_key"), so this fix uses api_key=, not bearer_token=.

Judgment calls

Why not instantiate AuthMiddleware. AuthMiddleware is the more "designed-for-this" mechanism — it wraps an httpx.Request/Response pair and would also give 401-triggered reauth detection for free via handle_response. I did not wire it in because doing so means changing how DealsClient's httpx.AsyncClient is built (an event_hooks["request"] callback or a custom transport), which DealsClient.__init__ does not currently accept as a parameter — that's a second, larger surface change for the same immediate outcome, and DealsClient already has a working, documented mechanism for exactly this (api_key/bearer_token constructor kwargs, see its docstring). Using the constructor kwarg the class was already built to accept is the smaller, more faithful fix. AuthMiddleware's response-inspection half (401-triggered reauth) is not invoked by anything either and is a reasonable follow-up if that behavior is wanted, but it's out of scope for closing the immediate wiring gap.

URL normalization. ApiKeyStore.get_key() (key_store.py:50-53) internally calls _normalize_url (rstrips trailing /) on every lookup, and add_key does the same on every write — so both directions are normalized identically regardless of what the caller passes in. I resolve the credential exclusively through ApiKeyStore.get_key(seller_url) (never by indexing _keys directly or a copy of it), so the call site does not need to duplicate that normalization or guess at URL shape. DealsClient.seller_url is separately rstrip("/")'d (deals_client.py:132), so the URL shape used as the dict key at construction time and the shape looked up against the store are consistent either way. Verified this round-trips with a unit test that stores a key under a trailing-slash URL and looks it up via the no-trailing-slash form the orchestrator actually uses.

Assurance carried by this PR

This has not had external code review. It was validated by: (1) the buyer repo's full existing unit suite (unchanged, still green), (2) three new unit tests that fail without the fix and pass with it (proof below), and (3) the diagnosis this fix is based on, itself checked against the real rig's live 401 failure. It is being opened directly rather than held in draft because it and seller-agent #77 need to ship together and #77 is already merged — every real-mode booking is broken until this lands. Whoever reads this history later should weigh it accordingly: suite-and-integration-gate validated, not reviewer validated.

Tests

Added to tests/unit/test_auth.py (TestBookingCredentialWiring), exercising the same factory shape production uses (lambda seller_url, **kwargs: DealsClient(seller_url, **kwargs), i.e. a real DealsClient, not a mock):

1. test_seller_with_stored_key_sends_x_api_key_header — a seller WITH a stored key gets a client whose httpx.AsyncClient carries X-Api-Key.
2. test_seller_with_no_stored_key_still_constructs_uncredentialed — a seller with NO stored key still constructs a working client with no X-Api-Key/Authorization header (today's behavior, unchanged).
3. test_url_normalization_round_trips_through_booking_lookup — a key stored under a trailing-slash seller URL is found by the no-trailing-slash lookup the orchestrator actually performs.

Failing without the fix (git stash of multi_seller.py only, tests left in place):

tests/unit/test_auth.py::TestBookingCredentialWiring::test_seller_with_stored_key_sends_x_api_key_header FAILED
tests/unit/test_auth.py::TestBookingCredentialWiring::test_seller_with_no_stored_key_still_constructs_uncredentialed FAILED
tests/unit/test_auth.py::TestBookingCredentialWiring::test_url_normalization_round_trips_through_booking_lookup FAILED
E   TypeError: MultiSellerOrchestrator.__init__() got an unexpected keyword argument 'key_store'
3 failed, 20 passed in 0.46s

Passing with the fix restored:

tests/unit/test_auth.py .......................                          [100%]
23 passed in 0.46s

Full unit suite, with the fix:

3503 passed, 1 skipped in 80.59s

Ships with

This is required for compatibility with seller-agent #77, which is already merged on seller main. Every real booking against a seller running #77 fails with 401 until this lands — the two changes need to ship together.

[Sirajmx]

Verified live: real seller (current main, #77's auth gate live) +
real buyer DealsClient over a real network — pre-fix genuinely 401s, post-fix genuinely passes
auth and reaches real business logic. Also tried to break the
new structural guard test by reverting a call site back to the vulnerable pattern — it caught it
immediately, naming the offender. Full suite: 3615 passed, 83 skipped, 0 failed. No conflicts.