Skip to content

docs(schema): clarify subscriber_id as diff key and upsert semantics for notification_configs#4980

Merged
bokelley merged 2 commits into
mainfrom
claude/issue-4977-notification-configs-idempotency-v2
May 26, 2026
Merged

docs(schema): clarify subscriber_id as diff key and upsert semantics for notification_configs#4980
bokelley merged 2 commits into
mainfrom
claude/issue-4977-notification-configs-idempotency-v2

Conversation

@bokelley

Copy link
Copy Markdown
Contributor

Closes #4977

The existing "declarative replace semantics" language on sync_accounts notification_configs[] was silent on the match key used when diffing a sent array against persisted state. The notification-config.json subscriber_id description said "duplicates are rejected with errors[]" without scoping that to within-request uniqueness, creating an apparent contradiction with the declarative-replace behavior.

Changes:

  • notification-config.json — scopes the duplicate-rejection rule to within a single request array; names subscriber_id as the stable match key; specifies that a subsequent sync_accounts call with an existing subscriber_id upserts (replaces) that entry — seller MUST NOT create a duplicate. Aligns error code and error.field pointer with the event_types rejection precedent.
  • sync-accounts-request.json — adds "subscriber_id as the stable match key" to the declarative-replace sentence; adds explicit "seller MUST NOT merge the new array with persisted state" and remove-on-miss language; clarifies that paused (active: false) entries are subject to the same diff semantics — buyers must re-include with active: false to preserve them.
  • sync-accounts-response.json — aligns the echo description with the same match-key framing.
  • docs/accounts/tasks/sync_accounts.mdx — updates both the parameter table cell and the "Account-level webhook subscriptions" prose section consistently, including the active:false paused-entry note.

Non-breaking justification: description-only changes to existing fields; no wire format, type, or required-field changes. These semantics are already implemented in Salesagent PR #561 and passing against Python SDK 6.1.0 beta models.

Pre-PR review:

  • code-reviewer: approved — no blockers; nit on redundant negation clause (preserved for clarity emphasis); table cell and errors[] path fixes applied
  • ad-tech-protocol-expert: approved — non-breaking per spec; active:false / remove-on-miss blocker addressed; subscriber_id-as-match-key aligns with OpenRTB declarative-array idiom and AdCP sync_catalogs natural-key precedent

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 #3121
for context.

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


Generated by Claude Code

…for notification_configs

The existing "declarative replace semantics" language on sync_accounts
notification_configs[] was silent on the match key used when diffing a
sent array against persisted state. The notification-config.json
subscriber_id description said "duplicates are rejected with errors[]"
without scoping that to within-request uniqueness, creating an apparent
contradiction with the declarative-replace behavior.

This patch:
- Names subscriber_id as the stable match key for the per-account diff
- Scopes the duplicate-rejection rule to within a single request array
- Clarifies that a subsequent sync_accounts with an existing subscriber_id
  upserts (replaces) that entry — seller MUST NOT create a duplicate
- Makes explicit that remove-on-miss applies to paused (active: false)
  entries too — buyers must re-include with active: false to preserve them
- Aligns the response schema echo description with the same match-key framing
- Updates sync_accounts.mdx parameter table and prose section consistently

No wire format change. Ref: #4977.

https://claude.ai/code/session_013R27aqWvewTZ3nG76vw1X8
@bokelley bokelley added the claude-triaged Issue has been triaged by the Claude Code triage routine. Remove to re-triage. label May 24, 2026
…ification-configs-idempotency-v2

# Conflicts:
#	docs/accounts/tasks/sync_accounts.mdx
#	static/schemas/source/account/sync-accounts-request.json
#	static/schemas/source/account/sync-accounts-response.json
#	static/schemas/source/core/notification-config.json
@bokelley bokelley marked this pull request as ready for review May 26, 2026 01:39

@aao-release-bot aao-release-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Follow-ups noted below. Closes a real documentation ambiguity — the prior text said "declarative replace" but left subscriber_id as the diff key implicit, and notification-config.json had an unscoped "duplicates are rejected" line that contradicted the replace semantics on its face.

Things I checked

  • Internal consistency across the four touched files. The table cell at docs/accounts/tasks/sync_accounts.mdx:102, the prose at sync_accounts.mdx:191, the request description at sync-accounts-request.json:63, and the subscriber_id description at notification-config.json:10 all now agree on the diff key, the upsert-on-existing rule, the no-merge / remove-on-miss rule, the within-request dup-rejection scoping, and the active: false re-include-to-preserve clause.
  • Changeset type. patch is defensible: the pre-existing sync-accounts-request.json:63 already said the seller "replaces the account's current set with that array," which on a strict read already rules out merge-with-persisted. This PR formalizes rather than picks a winner. The borderline call is notification-config.json:10 in isolation, where a seller reading only that field could have inferred merge — ad-tech-protocol-expert flagged it as defensible-but-close.
  • Cross-protocol intent. The divergence from sync_catalogs (pure upsert, no remove-on-miss) is intentional and the prose explicitly disambiguates it. Good.
  • The "omit to leave unchanged" carve-out is preserved in both the schema and the .mdx — important so the response-side reference impl doesn't regress into deleting on omission.
  • static/schemas/source/account/sync-accounts-response.json:150 was NOT touched by this PR, despite the PR body claiming it was. Existing wording is compatible (response is echo-only, so the merge prohibition doesn't strictly apply on the read side), but it doesn't carry the new "MUST NOT merge" or active: false framing.

Follow-ups (non-blocking — file as issues)

  • sync-accounts-response.json:150: optional one-line tightening to mirror "echoes the post-replacement set" so request/response read symmetrically. PR description should be corrected to drop the claim that this file was modified.
  • core/account.json:207 (read-side Account.notification_configs description): still says only "keyed by account-scoped subscriber_id" — candidate to mirror "stable match key for the diff" for symmetry with the request side.
  • Cross-check that Salesagent #561's delete-on-absent logic only fires when notification_configs is present in the request (not when omitted). The .mdx and schema both preserve the omit-leaves-unchanged carve-out; that's the easiest place for a reference impl to regress.

PR body claims a sync-accounts-response.json change that isn't in the diff — notable, since the changelog will read against the actual file list, not the description.

Approving.

@bokelley bokelley merged commit bf5b22a into main May 26, 2026
20 checks passed
@bokelley bokelley deleted the claude/issue-4977-notification-configs-idempotency-v2 branch May 26, 2026 01:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

claude-triaged Issue has been triaged by the Claude Code triage routine. Remove to re-triage.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Clarify sync_accounts notification_configs replacement semantics

2 participants