Skip to content

fix(server): forward authInfo/agent/toolName ctx to AccountStore.upsert and list (#1310)#1318

Closed
bokelley wants to merge 2 commits into
mainfrom
claude/issue-1310-accountstore-ctx-forwarding
Closed

fix(server): forward authInfo/agent/toolName ctx to AccountStore.upsert and list (#1310)#1318
bokelley wants to merge 2 commits into
mainfrom
claude/issue-1310-accountstore-ctx-forwarding

Conversation

@bokelley

@bokelley bokelley commented May 2, 2026

Copy link
Copy Markdown
Contributor

Closes #1310

AccountStore.upsert and AccountStore.list captured _ctx in the framework dispatcher but discarded it, silently blocking any adopter who needs the calling principal (e.g. for the per-agent billing gate BILLING_NOT_PERMITTED_FOR_AGENT from adcp#3831). This PR adds optional ctx?: ResolveContext to both interface methods (non-breaking — existing implementations compile unchanged) and updates buildAccountHandlers to forward authInfo, agent, and toolName using the same pattern already established for accounts.resolve at lines 1009-1012. Also fixes the identical pre-existing ctx.agent omission in the reportUsage and getAccountFinancials dispatcher blocks in the same function.

What was tested

  • npm run format:check — passed
  • npx tsc --project tsconfig.lib.json — only pre-existing TS2688/TS5107 tsconfig warnings (not code errors); no new errors introduced
  • test/server-decisioning-from-platform.test.js — 109/109 pass (2 new ctx-forwarding tests added for upsert and list)
  • Full suite (test/*.test.js test/lib/*.test.js) — 232 failures before and after; no new failures

Pre-PR review

  • code-reviewer: approved — no blockers; noted #3851→#3831 reference (fixed in follow-up commit) and missing ctx.agent test coverage as issues (no blocker); as const removal is intentional cleanup
  • ad-tech-protocol-expert: approved — non-breaking per spec; ResolveContext is correct shape for upsert/list; sync_accounts/list_accounts toolName strings are correct wire names; Phase 2 caveat added to JSDoc per reviewer recommendation

Nits (not fixed, surfaced for human reviewer)

  • ctx.agent forwarding is untested — wiring a full BuyerAgentRegistry in a test is medium complexity; the authInfo+toolName tests cover the new dispatcher path and the ctx.agent branch follows identical conditional-spread logic. A follow-up can add registry-backed agent forwarding tests once Phase 2 (feat(server): BuyerAgentRegistry Phase 2 — wire billing-capability enforcement (AdCP 3.1) #1292) lands.
  • The #3851 reference in the original issue (vs adcp#3831 used throughout the codebase for BILLING_NOT_PERMITTED_FOR_AGENT) may indicate a distinct follow-up spec PR — if so, the JSDoc reference should be revisited when that PR merges.

Triage-managed PR. This bot does not currently iterate on
review comments or PR conversation threads (only on the source
issue). To unblock:

  • Push fixup commits directly: gh pr checkout <num>
    fix → push.
  • Or re-trigger: comment /triage execute on the source
    issue.

See adcp#3121
for context.

Session: https://claude.ai/code/session_01V19x4iEy2Gt3yqt4d9vpj4


Generated by Claude Code

claude added 2 commits May 2, 2026 18:29
…rt and list (#1310)

- Adds optional `ctx?: ResolveContext` to `AccountStore.upsert` and `AccountStore.list` (non-breaking — existing implementations compile unchanged)
- Fixes `buildAccountHandlers` dispatcher to forward `authInfo`, `agent`, and `toolName` to both methods, matching the `accounts.resolve` pattern
- Also fixes pre-existing `ctx.agent` omission in `reportUsage` and `getAccountFinancials` dispatchers
- Updates JSDoc on `upsert` and `list` to document the billing gate use-case
- Adds regression tests asserting `ctx.authInfo` and `ctx.toolName` reach `accounts.upsert` and `accounts.list`

Closes #1310

https://claude.ai/code/session_01V19x4iEy2Gt3yqt4d9vpj4
Code-reviewer and protocol-expert noted the JSDoc and changeset
referenced #3851 where the rest of the codebase uses adcp#3831 for
BILLING_NOT_PERMITTED_FOR_AGENT. Also adds the Phase 2 (#1292)
caveat — framework-level enforcement is not yet active; adopters can
gate on ctx.agent?.billing_capabilities today.

https://claude.ai/code/session_01V19x4iEy2Gt3yqt4d9vpj4
@bokelley

bokelley commented May 2, 2026

Copy link
Copy Markdown
Contributor Author

Superseded — the same fix landed in #1315 (forwards ctx to AccountStore.upsert/list, closing #1310) and was further generalized in #1321 (the symmetric toResolveCtx helper covering all account-store call sites). Issue #1310 is closed. Thanks for the work — closing as duplicate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

AccountStore.upsert / list: forward request ctx (authInfo, principal) for principal-based gates

2 participants