[codex] Clarify account reference models#5062
Conversation
There was a problem hiding this comment.
LGTM. Follow-ups noted below. Clean reframe: keep the wire shape (account-ref.json discriminator, oneOf, additionalProperties: false) untouched while moving the conceptual taxonomy from authentication-flavor ("explicit/implicit") to namespace-ownership ("account-id namespace / buyer-declared"). The new three-pattern split — upstream-managed, seller-defined, buyer-declared — is the right shape for #4341 and resolves the SDK divergence root cause cleanly.
Things I checked
- Schema wire shape unchanged across all 11 source files. Discriminator on
account-ref.jsonis description-only; no field renames, no type changes, norequiredflips, no enum value changes (onlyaccount-scope.agentenum description widened, value preserved). UNSUPPORTED_PROVISIONINGinstatic/schemas/source/enums/error-code.json:107preserves both rejection triggers (natural-key→non-provisioner, AccountRef→non-settings-update) and a still-actionable recovery hint.- Changeset present at
.changeset/4341-upstream-managed-account-rosters.md, typepatch. ad-tech-protocol-expert: sound-with-caveats — three-pattern reframe is conformant with how 3.0 actually ships; the new "MUST exposelist_accountswhen credential can access >1 account" was inferable in 3.0.x and is now made normative.code-reviewer: sound-with-caveats — found terminology drift across ~8 sibling files that the rename missed.docs.jsonconfirms Mintlify is pointed atdocs/; there is nomintlify-docs/directory to drift against.
Follow-ups (non-blocking — file as issues or fold into this PR)
-
Terminology drift the rename missed. "Clarify account reference models" leaving eight files on the old terminology is a notable oversight in a PR whose entire job is the rename. Sweep these in this PR if possible:
static/schemas/source/account/sync-governance-request.json:101,125— exampledescriptionfields still say "explicit account" / "implicit account". These ship in the published schema.static/schemas/source/account/sync-accounts-request.json:299— example description still says "existing explicit account by account_id".docs/building/by-layer/L2/accounts-and-agents.mdx:3— frontmatterdescriptionstill says "explicit vs implicit account models". This is the SEO/preview surface for the canonical L2 doc.docs/media-buy/advanced-topics/sandbox.mdx:237,239— leftover "explicit account sellers" / "implicit account sellers" bullets.docs/accounts/tasks/sync_governance.mdx:21— "Sync the governance agent for an explicit account:" lede above anaccount_idexample.docs/accounts/tasks/sync_accounts.mdx:239— "register … on an explicit account" leftover.docs/reference/migration/prerelease-upgrades.mdx:219,docs/learning/test-personas.md:49,docs/sponsored-intelligence/overview.mdx:215— sibling mentions missed while peers in the same files were updated.
-
Changeset level is borderline.
patchis defensible because no field/enum/oneOf wire shape changed, but the changeset body itself contains new MUST clauses ("sellers MUST exposelist_accountswhen a credential can access more than one account"; "Required-account tasks must receive an explicitAccountRef"). A 3.0.x seller shippingrequire_operator_auth: truewith multi-account credentials but nolist_accountsbecomes non-conformant on merge.npm run test:schemas/test:json-schemawon't catch that behavioral delta. Either bump tominor, or add a sentence to the changeset body explaining why this is purely codifying what was already required. -
create-collection-list-request.json:16/create-property-list-request.json:16— new omission semantics ("MUST NOT mean an undocumented credential-local default") preserve the single-accessible-account success path but the new ink is normative-flavored. A short positive example showing "omitted + exactly one account → seller assigns it" would prevent implementers reading this as a behavior change. -
account-scope.json:12— newagentdescription ("shared across multiple declared brand/operator pairs") is clearer than "the agent's default account", but "multiple declared" implies async_accountspath; for agent-billed scopes there may be no declaration. Consider "Agent-scoped account with no brand- or operator-specific split". -
docs/accounts/overview.mdx:18— four normative clauses packed into one table cell. Tables aren't a great compliance-checking surface; lift the MUST/SHOULD into the prose immediately below.
Approving.
There was a problem hiding this comment.
LGTM. Follow-ups noted below. Wire shape is unchanged; the rewording disambiguates two patterns (account_id namespaces vs buyer-declared accounts) that were both living under "explicit/implicit" and codifies the SHOULD/MUST around list_accounts exposure that 3.0 left implicit.
Things I checked
- Wire neutrality. Every diff under
static/schemas/source/**is description-only — noproperties,required,enum,oneOf,allOf,$ref, oradditionalPropertieskeywords touched.core/account-ref.jsononeOfdiscriminator unchanged;enums/account-scope.jsonenumarray unchanged (onlyenumDescriptions.agenttext moves);enums/error-code.jsonenumlist unchanged. - Terminology consistency across
docs/accounts/**,docs/building/**,docs/protocol/**,docs/reference/**,docs/sponsored-intelligence/**, andstatic/schemas/source/**. Net-new "account-id namespace" / "buyer-declared accounts" terminology is applied uniformly. - Schema-vs-docs coherence on the new MUST/SHOULD around
list_accountsexposure:static/schemas/source/core/account-ref.json:5,static/schemas/source/protocol/get-adcp-capabilities-response.json:156,docs/accounts/overview.mdx,docs/protocol/get_adcp_capabilities.mdx,docs/building/by-layer/L2/accounts-and-agents.mdx, and the changeset all mirror the same text. No MUST loosened to SHOULD or vice versa. UNSUPPORTED_PROVISIONING: description atstatic/schemas/source/enums/error-code.json:107andenumMetadata.suggestionat:241shift in parallel.- L1 security tightening at
docs/building/by-layer/L1/security.mdx:185. The new "MUST NOT silently replace a missing requiredaccountwith a credential-implied default" closes a real footgun and is bounded honestly to schema-requiredaccount— optional-accounttasks are pointed at their per-task semantics. - Removed the wrong-as-shipped doc claim that
sync_accountsdoes not returnaccount_id. The schema has always emitted it; the new "MAY echo, MUST keep accepting natural key" framing matches the schema. - Changeset present (
.changeset/4341-upstream-managed-account-rosters.md), correctly scoped toadcontextprotocol.
Follow-ups (non-blocking — file as issues)
- Straggler.
docs/contributing/storyboard-authoring.md:26still readsExplicit-account form (when the seller issued anaccount_idvialist_accounts). Only file indocs/not covered by the terminology pass. Rename toAccount-id formfor parity. - Semver judgment. Changeset is
patch. The new MUSTs (sellers MUST exposelist_accountsfor multi-account credentials; MUST NOT silently default a requiredaccount; MUST keep accepting natural-keyAccountRefafter echoingaccount_id) are codification, but they will fail a previously-passing conformance suite if any existing seller skated by on the looser wording. The "MAY omit only when … out-of-band" escape hatch softens this materially. Worth a one-line WG ack in the PR thread that codification-as-patchis the policy choice, since two reviewers flagged it independently. - Dual-keying and cache-scope oracle.
docs/accounts/tasks/sync_accounts.mdx:172lets buyer-declared sellers echo anaccount_idwhile still accepting natural keys. Two valid keys for one account doubles the idempotency-cache-scope oracle surface called out insecurity.mdx:492. Sellers electing to dual-key SHOULD canonicalize to one form before computing the scope tuple. Worth one sentence insecurity.mdxnext time that section is touched. - Forward reference.
static/schemas/source/account/sync-accounts-request.json:5and several docs say "unless a future explicit capability declares it". Reserve a tracking issue so this doesn't become dead language.
Minor nits (non-blocking)
enumDescriptions.agentis doing two jobs.static/schemas/source/enums/account-scope.json:12bolts a request-side policy assertion ("not a hidden credential-implied default for requests that omit account") onto a response-side scoping-result enum.account_scopeonly appears on already-resolved accounts in responses, so the defaulting policy is fenced where it belongs — in the request-side docs andaccount-ref.json. Recommend trimming toAgent-scoped account with no brand- or operator-specific split.and leaving the policy line in the L2 doc and inprotocol/calling-an-agent.mdxwhere it already lives.- Inline-comment width.
docs/protocol/calling-an-agent.mdx:43— the code-block comment is now a two-sentence subordinate clause. Renders fine; just wider than the surrounding file.
LGTM.
Summary
Fixes #4341.
Clarifies the account-reference model so
account_idno longer conflates upstream-discovered platform accounts with buyer-declared accounts provisioned throughsync_accounts.What changed
AccountRef; credential-implied hidden defaults are not a protocol model.list_accountsguidance for multi-account credentials and singleton credentials.sync_accountssemantics: natural-key provisioning is buyer-declared; account-id namespace provisioning is out of scope unless a future capability declares it.Validation
npm run build:schemasnpm run test:schemasnpm run test:json-schemanpm run lint:schema-links -- --checknpm run test:docs-navgit diff --check