Skip to content

feat(training-agent): wire accounts.upsert on all v6 tenants — billing gates flow through per-tenant routes#3984

Merged
bokelley merged 3 commits into
mainfrom
bokelley/v6-accounts-upsert
May 3, 2026
Merged

feat(training-agent): wire accounts.upsert on all v6 tenants — billing gates flow through per-tenant routes#3984
bokelley merged 3 commits into
mainfrom
bokelley/v6-accounts-upsert

Conversation

@bokelley

@bokelley bokelley commented May 3, 2026

Copy link
Copy Markdown
Contributor

Summary

Tier-3 follow-up to #3851 (BILLING_NOT_SUPPORTED + BILLING_NOT_PERMITTED_FOR_AGENT gates). Closes the gap that left sync_accounts exposed only on the deprecated legacy /mcp route — gates now fire on every per-tenant route at /api/training-agent/<tenant>/mcp (sales / signals / governance / creative / creative-builder / brand).

Unblocked by SDK 6.7.0's accounts.upsert(refs, ctx) ctx-forwarding (originally filed at adcontextprotocol/adcp-client#1310, shipped as Phase 1 of #1269).

What landed

  1. Shared helper (v6-account-helpers.ts) — single syncAccountsUpsert const that delegates to the existing v5 handleSyncAccounts. Capability + per-buyer-agent gate semantics are identical on v6 and legacy paths.

  2. All six v6 tenant platforms wire the helperaccounts.upsert: syncAccountsUpsert on each platform's AccountStore. Framework auto-registers sync_accounts as a tool on every per-tenant route now that upsert is defined.

  3. Tenant router auth bridge (tenants/router.ts) — bridges res.locals.trainingPrincipal (set by the conductor's bearer-auth middleware, upstream of the framework) onto req.auth.{token, clientId, scopes} so the SDK framework surfaces it as ctx.authInfo.clientId to platform handlers. Without this bridge the framework runs without auth context — the conductor's auth middleware doesn't otherwise propagate into the framework's MCP transport. Shape mirrors serve.js attachAuthInfo verbatim (token: '', scopes: []).

  4. Integration test (tenants/sync-accounts-gates.test.ts) — boots the real tenant router, sends real HTTP, asserts:

    • Per-agent gate fires on /sales/mcp with the clamped error.details shape (rejected + suggested billing only).
    • Autonomous recovery via suggested_billing succeeds.
    • Agent-billable bearer accepts all three values.
    • Uniform-response rule for unrecognized principals holds.
    • it.each smoke over all six v6 tenant routes confirms the gate fires symmetrically.

Reviewed by

code-reviewer. Three SHOULD FIX findings addressed before opening:

  • Multi-tenant test coverage (originally only /sales/mcp; now covers all six via it.each).
  • SSE/JSON parser disambiguation (Accept header pinned to JSON-only; SSE branch removed).
  • Stale "spike" comment in v6-platform.ts updated to reflect the canonical ctx.authInfo.clientId path.

Plus two cheap NICE TO HAVE: token: '' on the auth bridge to match serve.js verbatim; a doc note on the refs as unknown[] cast assuming v5/v6 AccountReference wire compatibility.

Verified end-to-end

Phase 2 migration path

When BuyerAgentRegistry framework-level enforcement lands (Phase 2 of adcp-client #1269), commercial-relationships.ts becomes a BuyerAgentRegistry.bearerOnly adapter, the framework reads ctx.agent.billing_capabilities directly, and syncAccountsUpsert's gate-replay-via-handleSyncAccounts goes away. Tracked alongside adcp-client#1310 follow-ups.

🤖 Generated with Claude Code

bokelley and others added 3 commits May 3, 2026 11:03
…g gates flow through /api/training-agent/<tenant>/mcp

Tier-3 follow-up to #3851 (BILLING_NOT_SUPPORTED + BILLING_NOT_PERMITTED_FOR_AGENT
gates). Closes the gap that previously left sync_accounts exposed only on
the deprecated legacy /mcp route. Unblocked by SDK 6.7.0's
accounts.upsert(refs, ctx) ctx-forwarding (originally filed at
adcp-client#1310, shipped as Phase 1 of #1269).

Shared helper (v6-account-helpers.ts) — single syncAccountsUpsert const
delegating to the v5 handleSyncAccounts so capability + per-buyer-agent
gate semantics are identical on every v6 tenant route as on legacy /mcp.

All six v6 tenant platforms wire the helper — sales, signals (in
v6-platform.ts), governance, creative, creative-builder, brand.
accounts.upsert: syncAccountsUpsert on each platform's AccountStore.
Framework auto-registers sync_accounts on every per-tenant route now
that upsert is defined.

Tenant router auth bridge (tenants/router.ts) — bridges
res.locals.trainingPrincipal (set by the conductor's bearer-auth
middleware) onto req.auth.{token, clientId, scopes} so the SDK
framework surfaces it as ctx.authInfo.clientId to platform handlers.
Without this bridge the framework runs without auth context — the
conductor's auth middleware runs upstream of the framework's MCP
transport which doesn't otherwise see res.locals. Shape mirrors
serve.js attachAuthInfo verbatim (token: '', scopes: []) so any future
framework shape-check holds against the bridged value.

Integration test (sync-accounts-gates.test.ts) — boots the real tenant
router, sends real HTTP, asserts the per-agent gate fires on
/api/training-agent/sales/mcp with the clamped error.details shape,
autonomous recovery via suggested_billing succeeds, agent-billable
bearer accepts all three values, and the uniform-response rule for
unrecognized principals holds. Plus an it.each smoke over all six v6
tenant routes confirming the gate fires symmetrically. Stays in CI.

Reviewed by code-reviewer; SHOULD FIX (multi-tenant test coverage,
SSE/JSON parser disambiguation, stale spike comment) and NICE TO HAVE
(token: '' on bridge, refs cast doc note) addressed before push.

Verified end-to-end: 10/10 integration tests pass, 22/22 unit tests
pass (#3851 regression), full server suite clean (3133 passed, 42
skipped, 0 failed). Type check clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ants

Auto-registered by the framework on every tenant whose accounts.upsert
is wired (now all six per the parent commit). The drift test in
training-agent-tool-catalog-drift.test.ts asserts the catalog matches
live tools/list per tenant; without this entry the live response
includes sync_accounts but the catalog doesn't, drift fails.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three reviewer-driven hardenings on PR #3984's integration test plus
one doc note:

1. randomUUID() per call for idempotency_key (was hard-coded).
   Hard-coded keys let a watch-mode rerun or flaky-retry harness hit
   the SDK's idempotency cache and replay the cached envelope — a
   stale cached success would mask a real regression. The cache
   lookup runs before the gate, so fresh keys are the only way these
   tests assert what they claim. nodejs-testing-expert called this
   MUST FIX.

2. Single beforeAll boot instead of per-test boots. Each bootServer()
   call rebuilds JWKS + 6 tenants + framework dispatch (~1-2s). 11
   per-test boots → 1 shared boot. Test isolation is preserved by
   construction: assertions inspect the response envelope, brand
   domains differ across tests, idempotency keys are fresh, gate
   rejections happen before any persisted state is touched.

3. vi.stubEnv for PUBLIC_TEST_AGENT_TOKEN. Module-level env mutation
   leaks across vitest workers and makes test ordering load-bearing
   for downstream files. vi.stubEnv scopes the change to the suite
   and afterAll restores it.

4. Add unauthenticated-request 401 test. Regression-pin: confirms
   requireAuth runs before tenantMcpHandler, the bridge's
   `principal && !req.auth` condition never fires on unauthed
   traffic. security-reviewer flagged the gap.

5. v6-account-helpers.ts comment documents that account state is
   tenant-independent by design (one buyer-seller account, all
   surfaces) so a future contributor doesn't try to "fix" cross-tenant
   visibility.

11/11 pass; suite runs in ~1.7s (was ~3s with per-test boots).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@bokelley bokelley merged commit 695d61e into main May 3, 2026
19 checks passed
@bokelley bokelley deleted the bokelley/v6-accounts-upsert branch May 3, 2026 15:42
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