Skip to content

feat(decisioning): forward ctx to AccountStore.upsert/list (#1310)#1315

Merged
bokelley merged 3 commits into
mainfrom
bokelley/sdk-gap-1310
May 2, 2026
Merged

feat(decisioning): forward ctx to AccountStore.upsert/list (#1310)#1315
bokelley merged 3 commits into
mainfrom
bokelley/sdk-gap-1310

Conversation

@bokelley

@bokelley bokelley commented May 2, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Adds optional ResolveContext second arg to AccountStore.upsert and AccountStore.list.
  • Framework forwards authInfo, toolName, and agent (when agentRegistry is configured) — same shape already threaded to accounts.resolve, reportUsage, getAccountFinancials.
  • Unblocks principal-keyed gates on sync_accounts / list_accounts (e.g. the BILLING_NOT_PERMITTED_FOR_AGENT per-buyer-agent gate from adcp#3851) without re-deriving identity from the request.

Closes #1272.
Closes #1310. (Same gap, refiled with the billing-gate motivation; #1272 has the multi-tenant org_id framing.)

Why

Two motivations converged on the same gap:

  • AccountStore.upsert() and .list() should receive ResolveContext #1272 (multi-tenant tenancy): adopters whose auth principal determines tenant need authInfo inside upsert() to write a tenant key onto new accounts, and inside list() to scope the roster. Without forwarded ctx, every workaround fails (instance fields race; ALS doesn't propagate across the SDK's await chains; per-request stores violate the SDK's long-lived-store contract).
  • AccountStore.upsert / list: forward request ctx (authInfo, principal) for principal-based gates #1310 (billing gates): per adcp#3851, the spec defines BILLING_NOT_PERMITTED_FOR_AGENT as a per-buyer-agent gate keyed on the calling principal. The training-agent reference impl wires it on the legacy /mcp route (where principal flows naturally); v6 per-tenant routes can't expose sync_accounts with the gate because there's no way to read the principal inside accounts.upsert. Wiring accounts.upsert would silently NO-OP the gate.

reportUsage and getAccountFinancials already get ctx; this closes the symmetric gap on the only two AccountStore methods that didn't.

Compatibility

ctx is optional on the platform side. Existing upsert(refs) / list(filter) implementations keep working — adopters needing principal-based gates / tenant scoping opt in by accepting the second arg.

Test plan

  • Six new tests in test/server-account-store-ctx-forwarding.test.js cover:
    • upsert receives authInfo from request ctx
    • upsert receives resolved BuyerAgent when agentRegistry is configured
    • upsert ctx survives without authentication wired (authInfo undefined; toolName still set)
    • same three for list (filter still passed as first arg)
  • All 146 tests in adjacent files (server-buyer-agent-resolve-seam, server-buyer-agent-credential-synthesis, server-decisioning-from-platform, server-ctx-metadata-leak-paranoia) pass.
  • npm run build clean; npm run format:check clean.

🤖 Generated with Claude Code

Adds an optional `ResolveContext` second argument to
`AccountStore.upsert` and `AccountStore.list`; framework forwards
`authInfo`, `toolName`, and `agent` (when an `agentRegistry` is
configured) — same shape already threaded to `accounts.resolve`,
`reportUsage`, `getAccountFinancials`.

Unblocks adopters implementing principal-keyed gates on
`sync_accounts` / `list_accounts` (e.g. `BILLING_NOT_PERMITTED_FOR_AGENT`
per-buyer-agent gate from adcontextprotocol/adcp#3851) without
re-deriving identity from the request.

Backwards-compatible — `ctx` is optional on the platform side, so
existing implementations that don't accept the second arg keep
working.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bokelley and others added 2 commits May 2, 2026 14:37
The build-seller-agent SKILL.md example types `upsert` / `list` as
`(params, ctx) => …` but returns the wrong shapes (a pre-existing,
already-baselined bug in the example). With ctx now declared on the
SDK signature, TS infers the example's params as concrete types
instead of `any`, so the diagnostic prefix changes and four implicit-
any TS7006 entries drop out. Same example bug, sharper error text.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- JSDoc on `upsert?` recommends `ctx.agent` (registry-resolved durable
  identity) over `ctx.authInfo.credential` (raw transport credential)
  for commercial-relationship gates. Per ad-tech-protocol-expert
  review on PR #1315.
- `SyncAccountsResultRow` carries an explicit MUST-NOT-LEAK comment
  for `authInfo`, mirroring the rule already documented on
  `Account.authInfo`. Per security-reviewer defense-in-depth note on
  PR #1315.
- Changeset adds a security-relevant migration note: pre-release,
  adopters with no way to scope `list` per-principal returned every
  account to every authenticated caller. Scoping is now possible but
  opt-in — multi-tenant adopters should add it in the upgrade.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@bokelley
bokelley merged commit c1537e1 into main May 2, 2026
9 checks passed
@bokelley
bokelley deleted the bokelley/sdk-gap-1310 branch May 2, 2026 18:43
bokelley added a commit that referenced this pull request May 2, 2026
Routes all four AccountStore handler call sites (`syncAccounts`,
`listAccounts`, `reportUsage`, `getAccountFinancials`) plus the two
dispatcher resolver paths (`resolveAccount`, `resolveAccountFromAuth`)
through a single internal `toResolveCtx` helper.

Closes the symmetric gap flagged on PR #1315 review:
`reportUsage` and `getAccountFinancials` previously received `authInfo`
and `toolName` but not `agent`, even though `accounts.resolve` and the
new `upsert` / `list` wiring forward all three. Adopters implementing
principal-keyed financial-read or usage gates (e.g. on top of the
`BILLING_NOT_PERMITTED_FOR_AGENT` machinery from
adcontextprotocol/adcp#3851) now get the resolved buyer-agent identity
on every account-store surface.

Structural improvement, not just additive: any future addition to
`ResolveContext` lands on every account-store method automatically.
The previous inline literals at six call sites were what produced the
asymmetric `agent` gap in the first place.

JSDoc on `reportUsage?` and `getAccountFinancials?` updated to mention
`ctx.agent` and steer adopters toward it for commercial-relationship
gates (consistent with the guidance landed on `upsert?` in PR #1315).

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bokelley added a commit that referenced this pull request May 2, 2026
…1323)

* feat(decisioning): forward ctx.agent to tasks_get accounts.resolve

`tasks_get` now resolves the calling BuyerAgent (when an `agentRegistry`
is configured) and threads it to `platform.accounts.resolve` as
`ctx.agent`. Closes the asymmetry left by PR #1315 + #1321: the custom-
tool path historically bypassed registry resolution because it sits
outside the dispatcher's main handler-dispatch flow, leaving adopters'
resolve impls to see `ctx.agent: undefined` on tasks_get calls but
populated on every other tool.

**Status enforcement is deliberately NOT replicated.** A buyer agent
suspended after kicking off an HITL task must still be able to poll
for terminal state — refusing the poll would strand work with no
visibility. The dispatcher's main path enforces status at request
entry; the polling path does not. Sellers who want hard cutoff
implement that policy inside their own `accounts.resolve` (read
`ctx.agent.status`, throw `AdcpError`). Tests pin both contracts
(agent IS forwarded; suspended/blocked agents CAN still poll) so a
future refactor that tightens status enforcement onto the polling
path fails loudly rather than silently breaking running workflows.

Registry failures during a poll fall through to `agent: undefined`
rather than breaking the poll — same defensive shape as the
dispatcher.

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

* chore: log buyer-agent registry failure on tasks_get poll

Per security-reviewer defense-in-depth note on PR #1323. The
swallow stays (poll must survive transient IDP outages), but a
log entry makes upstream IDP failures visible to operators —
without it, buyers seeing REFERENCE_NOT_FOUND for valid tasks
because adopters' resolvers return null without `ctx.agent`
would be invisible in adopter logs.

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

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant