Skip to content

feat(reporting): serve reporting subscriptions from the principal runtime (#3504, 2/3) - #3508

Merged
bokelley merged 37 commits into
mainfrom
bokelley/reporting-subscription-runtime
Oct 31, 2026
Merged

bokelley merged 37 commits into
mainfrom
bokelley/reporting-subscription-runtime

Conversation

@bokelley

@bokelley bokelley commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

Second of the series for #3504. It contains #3505's commits, so merge #3505 first and review only the commits from feat(reporting): serve reporting subscriptions from the principal runtime onward. It targets main because the CI workflow runs only for PRs into main. This slice adds the seller principal runtime, the coverage engine that turns subscriptions into owed periods and exceptions, status, lost authority, and capabilities. Everything is opt-in. Without the new options, behaviour is byte-for-byte unchanged, and a golden transcript captured from the pre-change runtime proves it (principal-reporting-subscriptions.test.js, "option off").

What ships

1. sync_principal / get_principal (issue item 1)

createPrincipalLifecycle({ reportingSubscriptions: { listDeliveryConfigIds?, replaceAccountConfigs?, maxSubscriptions? } }) sits beside prepareReportingDestination and declarations. Without it the section is refused with UNSUPPORTED_FEATURE, as before.

Validation (src/lib/server/principal/reporting-subscriptions.ts) covers:

  • scope is exactly one of all_authorized_accounts or account_ids; exclude_account_ids only with all-accounts scope
  • period_timezone only with exactly one account
  • billing requires official finality and consumer_receipt; consumer_receipt requires a destination
  • destination_id must exist after the same request applies, including a request that removes a destination still named by an active subscription
  • path_template has no ./.. segments and only the four placeholders; {account_id} is required, and the template too for file_transfer, when the scope can cover several accounts
  • active subscriptions sharing a destination need distinct {account_id} templates
  • duplicate subscription_id values are rejected
  • the namespace clash with the caller's delivery_config_id values is rejected. The reverse direction is exposed as reportingSubscriptionIds(scope) for the host's sync_accounts.
  • two active subscriptions at the same scope level may not both apply to one account and feed_purpose

Versioning:

  • A semantic change requires a new version; a reused version with different content, or a regressed version, is rejected.
  • Each version records accepted_at; earlier versions stay in prior_versions with superseded_at.
  • active and replace_account_configs change in place.
  • Omission or active: false deactivates, and the subscription stays readable as inactive.
  • Reactivation is recorded as an inactive interval.
  • replace_account_configs is applied once per version, or when the flag turns true, through a post-commit host hook. Without the hook it is refused with UNSUPPORTED_FEATURE.

dry_run and expected_configuration_version work as they do for the other sections, because the same CAS transaction applies them.

Readback is core/reporting-subscription-state.json. The state is active, inactive, or action_required with issues (when the destination is suspended, removed, action_required or rejected). A destination transition that moves a subscription to action_required fires principal.changed (reporting_subscription_changed; the reason was added to the union). The engine maintains accounts_with_exceptions and accounts_unauthorized through recordReportingSubscriptionCoverage, which notifies on every change.

The runtime also exposes capabilities, the adcp.principal block with supported_sections derived from its options, and listReportingSubscriptions (readback plus retained version history).

2 and 3. Coverage engine (issue items 2 and 3)

createReportingSubscriptionEngineV1 (from @adcp/sdk/reporting/service). The host supplies only facts it alone knows:

  • listAuthorizedAccounts(principal, { cursor, signal }), paged. It may carry authorized_at (the chain start), granted_at (required for a re-grant inside the chain), timezone with timezone_changed_at (required on a zone change), operator, source_timezone, and pending_action with pending_action_since.
  • listAccountBuys(principal, accountId): the buy facts (confirmed_at, start_time, end_time, canceled_at, completed_at, each package's reporting_delivery_offering_ids, and optional prior flights)
  • resolveRevocationInstant(principal, accountId, { lastObservedAt, observedAt }): when authority was actually lost. The result is clamped to that window.
  • isPeriodDelivered(principal, period), for withheld_period_count
  • optionally listAccountDeliveryConfigs, the caller's own account-level configurations, for precedence

The host can push an authority change at its real instant with authorizationChanged(principal, accountId, { authorized, at }), or call reconcile(principal) to trigger a reconcile.

The engine maintains:

  • the grant history, with unauthorized_at and re-grant times
  • max_accounts, applied deterministically: served accounts keep their slots, then byte order. The rest get capacity.
  • precedence: the caller's own sync_accounts configuration for the account and feed_purpose beats every subscription, then its account_ids subscription beats its all-accounts one (overridden with overridden_by)
  • offering_unsupported per buy, or per package with package_ids
  • excluded_by_buyer
  • action_required with pending_action
  • timezone_cutovers, and gaps[] after a re-grant without backfill

Every exception opens and closes at an E instant (cutover rule). Exceptions are retained for status_retention_days after until.

State is sharded: one CAS document per principal (the max_accounts slots) and one per (principal, account). Owed periods are frozen as they elapse (see the review round below).

What it serves:

  • subscription_coverage[]. createReliableReportingService({ reportingSubscriptions: { engine, resolvePrincipal } }) adds it to summary and periods views. It is empty for an account the caller cannot read, so caller isolation is unchanged.
  • accounts_unauthorized[], with unauthorized_at, last_owed_period_end and withheld_period_count, for status_retention_days or until re-grant.
  • owedPeriods / owedObligations. These are computed with the same expectedSubscriptionPeriods rule as feat(reporting): buyer-computable coverage for reporting subscriptions (#3504, 1/3) #3505, so the seller owes exactly what the rule implies minus its declared exceptions. Obligations carry frozen, byte-sorted media_buy_ids, the resolved offering_id, and report_definition_id.
  • receiptRoster(tenantId, accountId): which principals of a tenant owe receipts for an account, derived from active subscriptions.

5. Capabilities

The service advertises configuration_tasks: ['sync_principal', 'sync_accounts'], and ['sync_principal'] alone with accountConfigurations: false. In that case configuration_task is omitted and install() no longer requires accounts.upsert. It also advertises subscriptions: { supported, max_accounts?, backfill_on_regrant, dimensions? } from the engine's own settings.

createAdcpServerFromPlatform:

Callers pinned to 3.2 get neither configuration_tasks/subscriptions nor the reporting_subscriptions section or feature id. A sync_principal-only seller drops reporting_delivery for them, as the spec requires. The end-to-end test runs with strict response validation on both 3.3 and 3.2.1.

Conformance

test/lib/reporting-subscription-engine.test.js pits a seller (principal runtime, engine and host facts) against a buyer that keeps only what it can observe: its own requests, get_principal readback (validated against the 3.3 schema), list_accounts authorized_at, get_media_buys, and subscription_coverage. Buyer periods must equal seller-owed periods, period by period. The cases follow the issue:

  • a mid-period confirmed_at, cancellation mid-flight, and early completion
  • a flight extension
  • a new subscription version (match change, including all)
  • revocation and re-grant with and without backfill_on_regrant, including last_owed_period_end, withheld_period_count, and caller isolation
  • max_accounts
  • precedence over an account-level configuration, and of account_ids over all-accounts
  • exact against finest against all over package, package × creative and package × country
  • offering_unsupported (buy and package) and excluded_by_buyer
  • action_required

Deviations and decisions

  • Obligations are computed but not yet materialized into the ledger. owedObligations() is the seller's authoritative owed set. Writing those into the reporting ledger, so the existing producer executes them, conflicts with three ledger assumptions:

    • one configuration per (account, delivery_config_id, delivery_config_version) (configurationId digest)
    • contiguous ownership for coverage completeness (evaluateReportingLedgerCoverageV1)
    • constituents frozen per generation rather than per period

    That needs a ledger change in step with the PostgreSQL store, so it moves to slice 3 with storage (see Remaining).

  • The engine store is in-memory here. ReportingSubscriptionEngineStoreV1 is the sharded port (get/put per key, listKeys, listPrincipals(tenantId)), and production refuses a process-local store. The durable implementation over AdcpStateStore/PostgreSQL is in feat(reporting): buyer runtime and durable storage for reporting subscriptions (#3504, 3/3) #3509.

  • withheld_period_count counts the periods owed before unauthorized_at that the host's required isPeriodDelivered reports as undelivered.

  • action_required covers destination problems only. A subscription moves to it when its destination is suspended, removed, action_required or rejected. A validating destination is the seller's to finish, so it stays active.

Conformance to adcontextprotocol/adcp#8585 (commit b7268ff7)

  • Readback. It lists inactive_intervals ({deactivated_at, reactivated_at?}, oldest first, bounded at 64), and the engine feeds them to the coverage function in wire shape.
  • Engine. Timezone cutovers carry previous_timezone. Cutovers and re-grant gaps are recorded per offering_id when the covered offerings' boundaries differ, and as one account-wide entry when they agree.
  • Authorization chains. authorized_at now means the start of the authorization chain, so a host keeps reporting the original value across a re-grant inside retention. The engine used to read a re-grant's time from authorized_at; it now stamps the re-grant when authority is observed again (an explicit authorizationChanged still carries its own instant). The conformance suite models a buyer that sees only the unchanged authorized_at and the seller's gaps[].
  • Service. createReliableReportingService({ reportingSubscriptions }) refuses to construct unless every delivery offering publishes standard_dimensions.
  • New conformance tests: reactivation recomputed by the buyer from inactive_intervals, and an account timezone change with previous_timezone.

Wire gating (please review). The SDK pins 3.3.0-beta.1, and that release's offering, subscription-state and coverage schemas are closed (additionalProperties: false). Emitting a post-beta.1 field therefore fails the SDK's own response validation, which is strict by default outside production. The fields are standard_dimensions, unmapped_dimensions, inactive_intervals, previous_timezone, a per-offering offering_id, and prior_versions[].configuration.

So the seller stores and computes everything, and each response emits a field only when the schema bundle served to the caller defines it. The gate is reportingSubscriptionReleaseFieldsV1 in src/lib/server/reporting-subscription-release.ts, which reads the served bundle's schemas. An unknown or absent served version emits none of these fields, so the gate fails closed. It opens on its own once the SDK syncs beta.2.

Until then:

  • a beta.1 caller gets schema-valid responses without the fields
  • per-offering gaps reach it as account-wide gaps, which can only remove more periods, never fewer

The new fields are hand-modeled until beta.2; #3505's body explains why I didn't sync the unreleased bundle.

Expert review round (2026-10-29), commits 4323ee42, 36e2e4cc and f103466a

Critical and high:

  • Withdrawn offering history erasure: fixed. offering_unsupported for a withdrawal or product edit opens at the E of the withdrawal or observation, using the per-buy resolution history. A test covers a product dropping the offering mid-flight; the elapsed history is kept.

  • Retroactive history: fixed. Owed periods are frozen as they elapse (frozen.periods, frozen.through) and never recomputed. owedPeriods returns frozen periods plus computed ones ending after through. Pruning past status_retention_days removes periods from the queryable set and never re-owes them.

    • A change to a buy's resolution (product edit, package added or removed, buy gone) is recorded as a buy revision with its E and the earlier snapshot. Periods of that buy starting before E keep resolving from the snapshot, and periods the edit adds start at E.
    • Tests cover pruning, a product drop, a product gain, and capacity freed by another account's revocation.
  • Reconcile-time stamping: fixed. Change instants come from source data:

    • granted_at on a re-grant inside the chain (required, or the reconcile fails)
    • timezone_changed_at on a zone change (required)
    • resolveRevocationInstant for a revocation
    • the overriding configuration's own active_until for overridden

    Capacity and action_required open at the coverage start when first observed. Reconcile time is used only where the change itself is first observed (product edits, pending_action without pending_action_since).

Medium and low:

  • configuration_version bump: fixed. recordReportingSubscriptionCoverage no longer bumps configuration_version.
  • replace_account_configs hook: fixed. It runs after the section commits. A failed hook stays pending and is retried on the next sync, including an unchanged one.
  • receiptRoster/listPrincipals crossing tenants: fixed. Both take a tenant id.
  • New chain ignored when the revocation was unobserved: fixed. A new chain whose authorized_at follows an unseen revocation closes the old chain at the resolved revocation instant and opens a gap.
  • Frozen destination method: fixed. A destination transport change without a version change is rejected.
  • Release gate failing open: fixed. The gate reads the served bundle's schemas, and an undefined served version emits nothing.
  • ID-collision check optional: fixed. listDeliveryConfigIds is required unless accountConfigurations: false.
  • Silent truncation: fixed. More than 64 inactive_intervals rejects the sync instead of truncating. Coverage recording rejects lists over 1000. Per the follow-up bot review, engine.reconcile isolates a subscription over that limit instead of failing the whole principal: it keeps its last lists and is reported in readback_overflow and through onReadbackOverflow.
  • Path-template expansion collisions: fixed. Shared path_templates that can expand to the same path for some subscription are rejected.
  • withheld_period_count default: fixed. isPeriodDelivered is required.
  • Per-calendar exception from: partially fixed. Each exception opens at the E of its own offerings' calendars. excluded[] carries no offering_id, so with several calendars the published from is still one account-wide instant, the latest E. This is flagged on feat(reporting): buyer-computable coverage for reporting subscriptions (#3504, 1/3) #3505 as a spec gap.
  • Unbounded CAS document: fixed. State is sharded per principal and per account.
  • adcp#8588: prior_versions[].configuration is stored and emitted, release-gated.
  • adcp#8593: the engine owes the confirmation period through the shared coverage function.

Remaining (slice 3)

  • Ledger materialization of owedObligations (per-period constituents, multi-offering generations, sparse coverage completeness), PostgreSQL migrations in @adcp/reporting-storage for the engine store and subscription history, and maintenance retention (item 6).
  • Managed delivery wiring: destination resolution, path expansion (expandReportingSubscriptionPathTemplate ships here), and account_id on every row (item 4).
  • Consumer runtime subscription expectations, refresh on principal.changed, and a client helper for sync_principal subscriptions (item 8).
  • A conformance storyboard against the reference seller.

Review follow-ups

  • 3073c31c. readFacts fails closed when the roster walk doesn't finish, so a partial roster never reads as revocations. protocol.principal is stripped together with adcp.principal for callers pinned below 3.2. subscription_coverage is added only when the served release is 3.3 or later. Tests cover a 3.1 capabilities pin and a 3.2.1 get_reporting_status, both under strict validation.
  • cb2c8a35. path_template placeholders are scanned in one linear pass (CodeQL polynomial-regex alert), and nested or unbalanced braces are rejected.

Follow-ups (2026-10-30)

  • adcp#8613 (decided for beta.3), a1c4fbb3.
    • subscription_coverage[] lists a deactivated subscription with status: "inactive" and deactivated_at, and only while it still has retained obligations for the account. That means an owed period, frozen or still running, ending within status_retention_days; after that the entry is omitted.
    • The value is gated on the served bundle (inactiveCoverageStatus). A caller served a release without it sees the account-level status as before, without deactivated_at. This matches adcp-client-python#1730.
  • adcp#8607 (revocation means no access), c5bc42f8. engine.acceptsReceipt and its post-loss receipt window are removed. Once authority is lost, the account is unknown to every task. feat(reporting): owner-scoped subscription receipts and statuses; coverage follows the grant #3528 implements the reduced-grant alternative.
  • replace_account_configs once per version, c5bc42f8.
    • The DX review reported that resending true reran the host hook. I could not reproduce it. The existing logic already applies once per subscription_version, or when the flag changes to true.
    • A new test locks this down: identical resends, dry runs, writes that add another subscription, and an active toggle never reapply. An omitted flag (its default, false) followed by true applies again, as the spec says.
  • Inactive-interval cap on omission, 73120d5a. Omission now refuses to open a 65th interval, like active: false does.
  • Where authorized_at comes from (not redesigned yet; a DX plan to make the SDK the single source is pending approval). There are two independent sources today:
    • Seller coverage: the engine reads authorized_at from the host's listAuthorizedAccounts facts (ReportingSubscriptionAccountFactV1.authorized_at, the authorization chain start). granted_at marks a re-grant inside the chain.
    • Buyer coverage: the buyer reads authorization.authorized_at from list_accounts, which the host's accounts.list produces.
    • The SDK does not reconcile the two. If a host computes them differently, the buyer's and seller's owed sets differ.
  • File budget. The wire-types module is now counted (+4). It was missed earlier, and main's latest additions used up the slack.

Local checks

  • npm run typecheck, build:lib, lint:boundaries and check:package-size pass. The file budget is +4 for the release gate module.
  • Prettier passes on the changed files.
  • npm run test:lib:fast passes.
  • Passing suites: reporting-subscription-engine.test.js (26), principal-reporting-subscriptions.test.js, reliable-reporting-service.test.js and reporting-subscription-coverage.test.js (262 total).

Refs #3504

🤖 Generated with Claude Code

bokelley and others added 2 commits October 29, 2026 09:30
Add the pure expectedSubscriptionPeriods rule for AdCP 3.3 principal-level
reporting subscriptions (experimental media_buy.reporting_subscriptions),
the normative period_generation calendar it uses, selector resolution
(exact, finest, all with byte-order ties), scope membership, and a builder
for the 3.3 registration capabilities (configuration_tasks, the deprecated
configuration_task, and the subscriptions block).

createAdcpServerFromPlatform adds media_buy.reporting_subscriptions to
experimental_features only beside a subscriptions block.

installConfiguration now throws a typed ReportingConfigurationImmutableError
with the unchanged message.

The cross-SDK parity table is
test/fixtures/reporting-subscriptions/coverage-cases.json.

Refs #3504

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

Advertising media_buy.reporting_subscriptions also requires
adcp.principal.supported_sections to list reporting_subscriptions, which the
SDK does not emit yet. Keep the capability builder pure in this slice and add
the feature id together with the principal block in the runtime slice, so an
adopter can never produce a 3.3-invalid capabilities response.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Comment thread src/lib/server/create-adcp-server.ts Resolved
aao-secretariat[bot]
aao-secretariat Bot previously approved these changes Oct 29, 2026

@aao-secretariat aao-secretariat Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Ladon verdict: Approve

Approve — AdCP 3.3 principal reporting subscriptions (opt-in), 19 files, additive.

No critical/high findings. Two medium findings only, neither in data-loss/schema/infra categories; high_risk is false and gated_paths is false, so no escalation row fires. With fewer than three medium findings and no team gate, this falls through to row 9 (approve).

Checked:

  • Changeset present and correctly typed (minor, additive exports/options) — satisfies changeset-vs-wire-impact.
  • Opt-off path byte-for-byte unchanged, proven by golden transcript.
  • Witness-not-translator holds; version-downgrade (3.2/3.3) handling and tests present.
  • CAS mutate callbacks side-effect-free (safe retries); host hooks thread AbortSignal.

Medium findings (non-blocking):

  • src/lib/reporting/subscriptions/engine.ts:480 — readFacts silently truncates authorized accounts at 10k-page cap, which can fabricate accounts_unauthorized/withheld receipts.
  • src/lib/server/create-adcp-server.ts:9844 — protocol.principal not stripped from experimental_features for <3.2 callers (backward-compat completeness).

Follow-up (non-blocking): confirm withSubscriptionCoverage's subscription_coverage field is stripped from get_reporting_status responses to <3.3 callers via response projection.

Medium findings

  • src/lib/reporting/subscriptions/engine.ts:480 — readFacts silently truncates authorized accounts at 10k-page cap
  • src/lib/server/create-adcp-server.ts:9844 — protocol.principal not stripped from experimental_features for <3.2 callers

@bokelley
bokelley changed the base branch from bokelley/reporting-subscription-coverage to main October 29, 2026 10:30
@bokelley
bokelley dismissed aao-secretariat[bot]’s stale review October 29, 2026 10:30

The base branch was changed.

@bokelley bokelley closed this Oct 29, 2026
@bokelley bokelley reopened this Oct 29, 2026

@aao-secretariat aao-secretariat Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Ladon verdict: Approve

Approve — subsequent pass on PR #3508 resolves both prior Medium findings with no new blockers.

This is a follow-up pass (delta: 233 lines, 5 files, mostly tests). The two Medium findings from the prior approve run are cleanly resolved:

  • engine.ts:480 — readFacts now fails closed: returns on terminal cursor and throws refusing a partial roster on 10k-page exhaustion, with a test confirming no account is read as a revocation after rejection.
  • create-adcp-server.ts:9844 — protocol.principal is now stripped from experimental_features for <3.2 callers, matching the adjacent 3.3-gate filters, with a 3.1 test.

The new subscription_coverage 3.3 gate in service/index.ts reads servedAdcpVersion via the same immutable-property pattern as ledger/response-version.ts; undefined-defaults-to-include is consistent with the ADCP_VERSION fallback elsewhere — no witness-not-translator concern. Changesets are present for the src/lib/** changes.

No Critical/High/Medium findings in the current run. Not high-risk, no gated paths, no author team gate. Falls through the decision table to row 9.

aao-secretariat[bot]
aao-secretariat Bot previously approved these changes Oct 29, 2026

@aao-secretariat aao-secretariat Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Ladon verdict: Approve

Approve — clean refactor with no blocking findings.

This PR refines path_template placeholder validation in the PR-new reporting-subscriptions.ts, replacing a two-step regex approach with a single linear-scan templatePlaceholders helper that returns undefined on unpaired braces. Logic is correct: nested {, orphan }, and unclosed trailing { all yield undefined, and fail() throws (returns never) before the placeholders! loop, so there is no null-deref risk. New tests cover all three unbalanced cases.

Checked:

  • No critical/high/medium findings surfaced by the reviewer.
  • Changeset coverage: two .changeset/*.md entries present for the reporting-subscription changes touching src/lib/**; the refactor refines a file already introduced in this PR, so no additional changeset is required.
  • Witness-not-translator: no mock/fallback data or wire-shape re-shaping introduced.
  • No high-risk paths, no gated paths, no author team gate.

Decision table: rows 1–8 do not fire (no findings at all, high_risk false, gated_paths false, no team gate). Falls through to row 9 → approve.

aao-secretariat[bot]
aao-secretariat Bot previously approved these changes Oct 29, 2026

@aao-secretariat aao-secretariat Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Ladon verdict: Approve

Approve.

This is a subsequent review on a prior clean approve. The delta since the prior approve (head cb2c8a3) is a single line: removal of an unused DAY constant from test/lib/reporting-subscription-engine.test.js. The reviewer confirmed zero remaining \bDAY\b references (live timing uses DAY_MS), making this a safe dead-code cleanup with no breakage.

Checks applied:

  • No critical/high findings (row 1 does not fire).
  • gated_paths is false (row 2 does not fire) — note review_decision is REVIEW_REQUIRED but the gate is path-based and not triggered here.
  • high_risk is false, no deletions/modifications of sensitive files (rows 3–5 do not fire).
  • Prior decision was approve, not escalate (row 6 does not apply).
  • No no-auto-approve team match (row 7 does not fire).
  • Zero medium findings (row 8 does not fire).

Falls through to row 9 → approve. Changeset coverage is present (.changeset/reporting-subscription-*.md). No blocking findings.

Adopt the rules adcontextprotocol/adcp#8585 settled:

- Selection reads each offering's published standard_dimensions and
  unmapped_dimensions, and fails closed on an offering without them.
- period_timezone: a utc offering matches only UTC or Etc/UTC; a
  source_timezone offering only its own published zone (never an
  account-resolved one).
- The coverage function takes the readback's inactive_intervals in wire
  shape, per-offering timezone_cutovers with previous_timezone
  (account.timezone is now the current zone), and per-offering gaps.
- The parity table fixes the billing-cycle case's scope (both keys, which
  maxProperties 1 forbids), adds cases for the settled rules, and its
  subscriptions are now schema-validated.

The new wire fields are hand-modeled in subscriptions/wire.ts until
3.3.0-beta.2 ships.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
aao-secretariat[bot]
aao-secretariat Bot previously approved these changes Oct 29, 2026

@aao-secretariat aao-secretariat Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Ladon verdict: Approve

Approve — clean delta, no blocking or medium findings.

This PR adds AdCP 3.3 principal-level reporting subscriptions: an opt-in seller runtime + coverage engine. The change is additive — responses are unchanged without the new options.

Checks that passed:

  • Witness-not-translator: No fabrication/re-shaping. Version gating in create-adcp-server.ts and from-platform.ts correctly strips 3.3 fields below 3.2/3.3 and post-beta.1 settled fields below 3.3.0-beta.2, matched by wire-strip helpers and tested on the wire.
  • DR-0033 pathway: Version gating + wire-strip helpers provide the backward pathway for the new wire fields, tested on the wire.
  • Changesets: Two changesets present and correct for opt-in additive changes in beta pre-mode; package-size bumps measured.
  • Fail-closed behavior: engine.ts readFacts bounds the roster walk at 10,000 pages and refuses a partial roster rather than reading missing accounts as revocations. Durable-store production guard sound.
  • Path safety: encodePathSegment percent-encodes '/' and '.'/'..'; PATH_TEMPLATE rejects traversal/unknown placeholders.
  • Ops: abort signals threaded, bounded CAS retry. Buyer/seller parity enforced by assertAgree harness.

One non-blocking note (calendarFor source_timezone asymmetry in engine.ts vs coverage.ts) was traced to an unreachable path and is harmless.

gated_paths is false and no no-auto-approve team match, so review_decision: REVIEW_REQUIRED does not force a gate. Prior decision was approve; no new findings. Falls through to row 9.

@bokelley bokelley left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Review: principal runtime and subscription engine (#3508, commits 5776a9b..b7268ff)

Checked against adcontextprotocol/adcp main (42dc2467, includes #8585). I drove createReportingSubscriptionEngineV1 directly from src/ with a stub principal runtime, an in-memory store and a controllable clock. Every item marked "verified" below is the engine's actual output.

Overall. Validation, versioning and the readback shape are solid. The engine's owed set is not stable over time, though. Elapsed periods disappear or reappear when host facts change, when an offering is withdrawn, or when internal state is pruned. Several cutover instants are stamped at reconcile time instead of when the change happened. owedObligations() is documented as "the seller's authoritative owed set", and billing settles on it, so I would fix the critical and high items before anything consumes it.

Confirmed bugs

1. Critical: withdrawing an offering, or a product dropping it, erases a buy's entire history. At engine.ts:721 and engine.ts:477, a buy-level offering_unsupported exception opens with from = max(accepted_at, confirmed_at). That is a raw past instant, not the cutover E at the moment the resolution changed. computeOwed and every buyer then remove every period from from on.

Verified with v1 accepted 11-01, a buy confirmed 11-02 with a flight from 11-03 to 12-31, and a UTC P1D offering A:

  • reconcile at 11-15: owed 11-03 through 11-14
  • A gets withdrawn_at: 2026-11-20 (the supported path), or the product drops A; reconcile at 11-21
  • owed: nothing. excluded: [{reason: offering_unsupported, media_buy_id: mb, from: 2026-11-02T00:00:00Z}]

Per x-adcp-validation.cutover (a resolution change is a cutover) and "Exception from and until values are such E instants", 11-03 through 11-19 stay owed. These are elapsed and possibly billed periods.

Fix: open the exception at the cutover E of the change. That is ceil of the change instant on the outgoing calendars: the withdrawn_at or available_from instant, or the observation instant for product edits. Use max(accepted_at, confirmed_at) only when the buy never resolved under the current version. Do the same for a reopened exception: today a resolve → unresolve → resolve → unresolve sequence reopens from the original past instant.

2. High: pruning closed exceptions, gaps and timezone cutovers changes elapsed history. engine.ts:482-488 prunes them status_retention_days after until or effective_at. computeOwed (engine.ts:914-947) recomputes the entire history from whatever state remains.

  • Capacity. Verified with maxAccounts: 1 and statusRetentionDays: 7. Account b has capacity from 11-02 to 11-11. On 11-12 the owed periods for b are 11-01, 11-11. After a reconcile on 11-25, the owed periods for b up to 11-12 are 11-01 … 11-11, so nine periods the seller refused are now "owed".
  • Timezone change. Verified with an account that moves from New York to Los Angeles on 11-04. Before the prune, the owed starts are 11-02T05, 11-03T05, 11-04T05, 11-05T08, …. After the prune they are 11-01T07, 11-02T08, 11-03T08, 11-04T08, 11-05T08, …: history is re-bucketed into the new zone, and a period appears that was never owed.

Spec: "an elapsed period never becomes owed retroactively", and exceptions only ever remove periods. Fix: retention applies to the wire projection, not to the state that defines the owed set. Either keep internal state for as long as any affected obligation is retained, or freeze owed periods at scope_resolved_at (the ledger materialization planned for slice 3) and stop recomputing elapsed history.

3. High: cutover instants are stamped at reconcile time, not when the change happened. The spec defines E from the instant T of the change. The engine uses nowMs in several places:

  • Override closing (engine.ts:466-470). until = ceil_subscription(now). It ignores the config's active_until, and it uses the subscription's calendar instead of the outgoing configuration's. Verified: a config is active from 11-01 until 11-10T12:00 (as replace_account_configs would set it), and the reconcile runs at 11-12T03:00. The result is until: 2026-11-13, and the subscription owes from 11-13. Per the spec, the configuration owes through its period starting before T, so E = 11-11T00:00Z and the subscription owes from 11-11. 11-11 and 11-12 are owed by nobody. ReportingSubscriptionAccountConfigV1.schedule (engine.ts:105-111) and the until?: {at, calendar} slot (engine.ts:616) are declared for exactly this but never used.
  • Capacity and action_required (engine.ts:691,702) open at ceil(now), so periods between the account's coverage start and the first reconcile are owed. Verified in the capacity scenario above: account b owes 11-01 although it was over max_accounts from the start.
  • Revocation and re-grant. Revocation (engine.ts:586-589) records unauthorized_at at the reconcile instant. A re-grant under an unchanged authorized_at (engine.ts:530) is stamped at the reconcile instant. Verified: a revocation at an unknown time, observed 11-07T10, and a re-grant observed 11-09T10 give a gap [11-07, 11-10). With a slow reconcile cadence, data for periods ending after the real revocation counts as owed, and possibly deliverable, until the engine notices.
  • Account timezone change (engine.ts:786): E = ceil_old(now) rather than the first old boundary after the actual change.

Fix: let the hooks report event instants (unauthorized_at, a re-grant instant, timezone_changed_at, plus the config's active_until and schedule), and compute E from them. Treat authorizationChanged as the primary path, and reconcile-time stamping as a documented fallback that reports a lag. When a capacity or action_required condition is first observed for a newly covered account, open it at the account's coverage start.

4. High (design): the owed set is recomputed from current host facts. owedPeriods (engine.ts:949-964) and readbackFor call listAccountBuys now and resolve against the static options.offerings.

  • Removing a withdrawn offering from offerings, instead of keeping it with withdrawn_at, makes all its historical periods vanish.
  • A product whose reporting_delivery_offering_ids changes, or a package added to or removed from a buy, re-resolves every elapsed period without a cutover.

These are separate from item 1. Until obligations are materialized, owedObligations() should not be described as authoritative. Either freeze the per-period resolution, or require hosts to supply dated product and offering history.

5. Medium: engine readback advances configuration_version. At runtime.ts:980, recordReportingSubscriptionCoverage issues a new configurationVersion whenever accounts_with_exceptions or accounts_unauthorized change. sync_principal: "configuration_version advances on every successful mutation … through this task or through sync_agent_notification_configs — and never on seller-driven setup-state transitions." Failure scenario: a buyer reads version V and the engine reconciles a revocation. The buyer's guarded sync_principal with expected_configuration_version: V returns CONFLICT for any section, even though it changed nothing. Fix: do not touch configurationVersion; principal.changed already signals the change.

6. Medium: if the replace_account_configs hook fails after commit, the replacement is lost. At runtime.ts:359-362 the hook runs after the principal row commits, with replaceAppliedVersion already persisted. If the hook throws, the caller gets an error for a write that was applied. A retry with the same body finds replaceAppliedVersion === version and never calls the hook again. The configs stay active, so the subscription stays overridden indefinitely.

Separately, the hook must deactivate configs only "on in-scope accounts where the subscription resolves an offering … never accounts it excludes, or with offering_unsupported or capacity". The host has none of that state; the engine does.

Fix: use a durable outbox and retry, or mark the version applied only after the hook succeeds. Pass the target account list from the engine, or provide an engine helper that computes it.

7. Medium: receiptRoster(accountId) crosses tenants. At engine.ts:1008-1030, store.listPrincipals() returns principals of every tenant, and the filter is only state.accounts[accountId]. If account IDs are tenant-scoped, tenant B's principals land on the receipt roster for tenant A's acct_1. Fix: take tenantId (or a full account scope) and filter listPrincipals by it.

8. Medium: a new authorization chain is ignored when the revocation was never observed. At engine.ts:525, if the last grant is open, applyGrant(authorized=true) returns without looking at fact.authorized_at. Failure scenario: the account is revoked and re-granted after retention between two reconciles. The host now reports a later authorized_at (a new chain), but the engine keeps the old chain. The seller owes the revoked interval; a buyer computing from the wire authorized_at owes from the new chain start. Fix: treat a later authorized_at on an open grant as a revocation followed by a new chain. If the revocation instant is unknown, bound it by the previous observation.

9. Medium: the destination method is frozen at acceptance, but the buyer reads the current destination. At reporting-subscriptions.ts:284-285,315, destinationMethod is captured once per version. Failure scenario: the buyer replaces destination archive under the same destination_id, moving from s3 to gcs, without bumping the subscription version. The seller keeps resolving s3 offerings, while a buyer computing with the destination's current pattern and transport resolves gcs offerings, or nothing. Fix: treat a destination method change as a resolution-change cutover, and re-freeze with a dated record. Alternatively, reject a pattern or transport change for a destination named by an active subscription unless the subscription version is bumped.

10. Medium: the release gate fails open, and is keyed to a version literal. wire.ts:83 serves the beta.2 fields when servedAdcpVersion === undefined. Every protocol dispatch path I traced sets validationVersion. A host that calls platform.getReportingStatus or the principal handlers from its own router, which is a supported composition, gets beta.2 fields validated against a beta.1 bundle.

The gate is also a semver literal (3.3.0-beta.2, with a TODO). If beta.2 is cut without #8585, the gate opens on a bundle whose closed schemas reject the fields.

Fix:

  • Fail closed on undefined, with an explicit opt-in for direct calls in tests.
  • Gate on the bundle's schema instead of a release literal: for example, whether the bundled reporting-delivery-offering.json defines standard_dimensions, the way bundleSupportsAdcpVersionField already works.

The gate is otherwise correct for protocol traffic. The served value is the validating bundle key, and get_adcp_capabilities, get_principal/sync_principal and get_reporting_status all strip the fields below beta.2.

Low

  • subscription_id collision check is optional (runtime.ts:431). With listDeliveryConfigIds absent, a subscription_id equal to one of the caller's delivery_config_id values is silently accepted, although sellers MUST reject it. Require the hook whenever configuration_tasks includes sync_accounts.
  • Readback truncation. reporting-subscriptions.ts:444 keeps only the last 64 inactive_intervals, while the engine computes with all of them. After 64 toggles, a buyer recomputing from the readback owes periods the seller doesn't. accounts_with_exceptions and accounts_unauthorized are silently cut at 1000 (runtime.ts:964,967).
  • Path templates can collide after expansion. At reporting-subscriptions.ts:521, "templates must differ" is checked textually. {account_id}/{subscription_id} for subscription x and {account_id}/x for subscription y both expand to acct/x, so the second feed overwrites the first. Consider requiring {subscription_id} or distinct literal prefixes. Spec question: ./.. are encoded as %2E (reporting-subscriptions.ts:541), and RFC 3986 §2.3 tells normalizers to decode %2E. A transport that normalizes would turn .%2E (template .{account_id}, account .) back into ... That is safe on S3 but worth a spec note for path-based transports.
  • withheld_period_count heuristic (engine.ts:896,902-911). Without isPeriodDelivered, every past-due period is assumed delivered, so a late seller under-reports withheld periods. slaMs also ignores Y/M components.
  • Multi-calendar override from (engine.ts:450-452,699). One from equal to the max ceil across offerings makes the earlier-ending offering owe periods that start after T. excluded[] has no offering_id, so this is a spec limitation; please document it.
  • Capability refusal is one-way. from-platform.ts refuses subscriptions without the principal section, but not the reverse (reporting_subscriptions in supported_sections without configuration_tasks including sync_principal). Nothing checks that list_accounts populates authorization.authorized_at, or that buys populate completed_at. Sellers advertising subscriptions MUST do both, and buyers cannot compute coverage without them.
  • Stale scope in subscription_coverage. Entries are returned for every subscription with stored state for the account, including subscriptions whose current scope no longer includes it. The spec scopes entries to subscriptions whose scope includes the account, plus retained inactive ones.
  • "Option off is byte-identical" holds on the wire for the principal runtime (golden) and the service (registration keeps key order). It does not cover:
    • the new per-release capability projection in create-adcp-server.ts, which runs for every seller and is not in the golden
    • PrincipalLifecycleRuntime, which now extends PrincipalLifecycleRuntimeSubscriptions with required members, a type-level break for custom implementations and mocks
    • a principal row written with the option on and later served with it off, which still returns reporting_subscriptions

Tests

reporting-subscription-engine.test.js compares the buyer and seller, but both sides call the same expectedSubscriptionPeriods, so they agree by construction. The suite tests whether facts are plumbed through, not the rule itself. None of the divergences above is exercised. Please add:

  • offering withdrawal and a product change mid-flight
  • pruning after status_retention_days
  • override close with a config active_until earlier than the reconcile
  • capacity for an account first observed after its coverage start
  • an unobserved revocation followed by a new chain
  • a destination transport change
  • engine readback followed by a guarded sync_principal

- An empty flight (stop at or before start) overlaps nothing.
- A cutover carries the outgoing stretch's own owing start into E, so two
  cutovers inside one period never owe a period twice.
- Sub-daily periods under a local alignment restart at each local midnight
  on wall-clock multiples (adcp#8588); calendars gain next() and the walk
  iterates by instant with an iteration budget and skip-ahead.
- A billing anchor in a DST fold is interval zero itself.
- A wire authorized_at equal to the chain start continues the chain.
- The start rule lives in one function for the pending owner decision.
- Schema validation accepts unknown reporting-dimension values.
- The parity table grows to 79 cases with obligations on every case and
  non-wire inputs labelled.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Comment thread src/lib/server/principal/runtime.ts Resolved
aao-secretariat[bot]
aao-secretariat Bot previously approved these changes Oct 29, 2026

@aao-secretariat aao-secretariat Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Ladon verdict: Approve

Approve — AdCP 3.3 principal reporting subscriptions (subsequent pass).

The single current finding is a Medium (not critical/high), so row 1 does not fire. gated_paths is false (row 2 N/A). high_risk is false (rows 3, 5 N/A). The Medium finding's category is a runtime reconcile edge case, not data-loss/schema/infra (row 4 N/A). Prior decision was approve, not escalate (row 6 N/A). No no-auto-approve team match (row 7 N/A). Only one Medium finding — fewer than three (row 8 N/A). Falls through to row 9 → approve.

Medium findings

  • src/lib/server/principal/runtime.ts:1021 — recordReportingSubscriptionCoverage throws uncaught when a subscription owes >1000 capacity exceptions; engine.ts readbackFor builds those lists unbounded, so an all_authorized_accounts subscription on a roster larger than max_accounts can halt the principal's reconcile. Worth a follow-up to bound/guard, but a single Medium with no blocking trigger.

What I verified from the reviewer: the change is additive/opt-in (byte-for-byte unchanged without options); the ordinal→next() refactor in coverage.ts advances start strictly on every path and is bounded by max_iterations (no infinite loop); changesets are minor and correct; release-field stripping is the documented DR-0032/0033 version-translation exception. 1 Medium, no blocking issues → approve.

Medium findings

  • src/lib/server/principal/runtime.ts:1021 — Reconcile fails permanently when a subscription owes >1000 capacity exceptions

bokelley and others added 4 commits October 29, 2026 19:49
The start bound is the first boundary at or after the later of the
version's accepted_at and the account's authorized_at; a buy is owed every
period that ends after its confirmed_at and overlaps its flight, so the
period in which the buy is confirmed is owed in full. The rule stays in
firstOwedBoundary. Parity table: the mid-period confirmed_at and hourly
cases change, plus every case whose first period contains confirmed_at;
new cases for an ASAP buy confirmed mid-day and a monthly billing buy
confirmed on the 12th.

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

Opt-in support for the AdCP 3.3 sync_principal reporting_subscriptions
section (experimental media_buy.reporting_subscriptions):

- createPrincipalLifecycle({ reportingSubscriptions }) validates, versions
  and reads back subscriptions (core/reporting-subscription-state.json),
  honours dry_run and expected_configuration_version, applies
  replace_account_configs once per version through a host hook, derives
  action_required from the destination, and emits principal.changed with
  reason reporting_subscription_changed. It also exposes the adcp.principal
  capability block.
- createReportingSubscriptionEngineV1 turns three host hooks into grant
  history, max_accounts slots, precedence, exceptions at cutover instants,
  timezone cutovers, re-grant gaps, accounts_unauthorized, the receipt
  roster and the post-loss receipt window, and computes owed periods with
  the same expectedSubscriptionPeriods rule buyers run.
- createReliableReportingService({ reportingSubscriptions }) advertises
  configuration_tasks and the subscriptions block and adds
  subscription_coverage to summary and periods views.
- createAdcpServerFromPlatform passes capabilities.principal and
  capability_changes through, adds protocol.principal and
  media_buy.reporting_subscriptions to experimental_features, refuses
  subscriptions without the reporting_subscriptions section, and strips
  3.3-only fields for callers pinned below 3.3.

Without the options every response is byte-for-byte unchanged; a golden
transcript captured from the pre-change runtime proves it.

Refs #3504

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

- readFacts refuses a roster that does not finish within the page cap, as it
  already did for a repeating cursor: a partial roster would read as
  revocations and fabricate accounts_unauthorized entries.
- Callers pinned below 3.2 no longer see protocol.principal once the
  adcp.principal block is removed for them.
- subscription_coverage is added only for callers served 3.3 or later;
  earlier get_reporting_status response shapes are closed.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The placeholder regex backtracked polynomially on runs of '{' (CodeQL).
Tokenize braces in a single pass instead, which also rejects nested and
unbalanced braces explicitly.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
aao-secretariat[bot]
aao-secretariat Bot previously approved these changes Oct 30, 2026

@aao-secretariat aao-secretariat Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Ladon verdict: Approve

Approve.

Clean delta. Removes engine.acceptsReceipt (interface + impl) per adcp#8607 — revocation means no access, so there is no post-loss receipt window. The removal is tracked by changeset prose, an absence assertion, and a new replace_account_configs idempotency test. Reviewer confirmed no dangling callers (only the test's 'acceptsReceipt' in w.engine check remains) and that retentionMs stays load-bearing elsewhere in engine.ts.

No critical/high/medium findings. Only a single Low (stale JSDoc at engine.ts:1786 still referencing the removed "post-loss receipt window", outside the delta hunk) — non-blocking nit.

Changeset present (two .changeset/*.md files covering the reporting-subscription coverage + runtime changes), satisfying changeset-vs-wire-impact. No no-auto-approve team match; gated_paths false; high_risk false. Prior decision was also approve with no findings — net state unchanged / clean. Falls through to row 9.

Nit (non-blocking): src/lib/reporting/subscriptions/engine.ts:1786 — update stale JSDoc that still advertises the removed "post-loss receipt window".

@bokelley
bokelley dismissed stale reviews from aao-secretariat[bot] via c55592b October 30, 2026 17:02

@aao-secretariat aao-secretariat Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Ladon verdict: Approve

Approve — clean pass, no blocking or medium findings.

Checked against the decision table top-down:

  • No critical/high findings (row 1 N/A).
  • gated_paths is false (row 2 N/A); review_decision REVIEW_REQUIRED does not force escalation absent a gate.
  • high_risk is false, no deletions (rows 3, 5 N/A).
  • No medium findings in data-loss/schema/infra categories (row 4 N/A).
  • Prior decision was approve, not escalate (row 6 N/A).
  • No no-auto-approve team match (row 7 N/A).
  • Zero medium findings, so fewer than three (row 8 N/A).
  • Falls through to row 9 → approve.

Reviewer reports a clean pass: delta dominated by mechanical version bump (beta.7→beta.9), maintained schema sync (3.1.29→3.1.31), docs, and a dev-only performance harness. Load-bearing additions (shared content-keyed MCP validator cache with per-entry Ajv; compact-schema.ts remote-schema factoring with return-schema fallbacks) were verified. No witness-not-translator concern — validation/compliance fixtures do not reshape buyer wire data. Changesets present and correctly patch-scoped under beta pre-mode. Prior clean approval still holds.

aao-secretariat[bot]
aao-secretariat Bot previously approved these changes Oct 30, 2026

@aao-secretariat aao-secretariat Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Ladon verdict: Approve

Approve — clean subsequent pass, no blocking findings.

This is a subsequent review following a prior approve (no findings). The current run surfaces no critical, high, or medium findings.

What I checked:

  • Changesets present: .changeset/reporting-subscription-coverage.md and .changeset/reporting-subscription-runtime.md cover the src/lib/** changes with a typed patch appropriate for an additive internal optimization. No hand-edited version line.
  • Witness-not-translator: The core change (shared content-keyed MCP output-schema validators via compactMcpSchemaForValidation) is fail-safe — normalize() returns the original schema on any unsupported graph ($async, dynamic/recursive scopes, relative/fragment IDs, duplicate resources, refs into annotation data) and only hoists relocatable nodes after refs are made absolute, so relocation cannot change validation. Validity-equivalence invariant is asserted against the official provider under fast-check fuzzing. No wire-shape re-shaping or fabrication.
  • Schema bump: 3.1.29→3.1.31 threaded consistently across package.json/scripts/docs/tests (maintained minor bump).
  • Transport: No reimplemented HTTP/SSE; change is in reporting/validation layers.

Decision-table trace: no critical/high (row 1 n/a); gated_paths false (row 2 n/a); high_risk false (rows 3,5 n/a); no medium findings (rows 4,8 n/a); prior decision was approve, not escalate (row 6 n/a); no no-auto-approve team match (row 7 n/a). Falls through to row 9 → approve.

bokelley and others added 4 commits October 30, 2026 18:56
…ndle

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

Main adopted 3.3.0-beta.2, which no longer ships the beta.1 bundle and
defines the fields settled after beta.1 (standard_dimensions,
inactive_intervals, prior_versions[].configuration, previous_timezone,
per-offering entries), so the release gate now emits them to beta.2
callers. inactive in coverage stays gated until beta.3.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@bokelley
bokelley dismissed stale reviews from aao-secretariat[bot] via be83582 October 30, 2026 19:29

@aao-secretariat aao-secretariat Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Ladon verdict: Approve

Approve — clean pass, no blocking or medium findings.

Checked: this delta is a mechanical AdCP 3.3.0-beta.1→beta.2 regen (generated types, docs, changesets, version pins) plus three reviewed source changes — the legacy-ref constraint-satisfaction reconciler in creative-delivery.ts (size/duration from resolved refs only, fail-closed on unknown constraints), the typed RequestValidationError / dispatched:false pre-dispatch marker (no submitted-value leak, message preserved, per-site markers on the four VersionUnsupportedError throws), and the envelope-status ESM/CJS loader binding fix for #3529 (cache cleared on rebind, unresolved versions no longer cached). Changesets present and correctly typed.

Decision table walkthrough: no critical/high findings (row 1 n/a); gated_paths false (row 2 n/a); high_risk false (rows 3/5 n/a); no medium findings (rows 4/8 n/a); prior decision was approve so row 6 n/a; no no-auto-approve team match (row 7 n/a). Falls through to row 9 → approve.

Note: review_decision is REVIEW_REQUIRED, but since gated_paths is false this does not force escalation — row 2 only fires when gated_paths is true.

aao-secretariat[bot]
aao-secretariat Bot previously approved these changes Oct 30, 2026

@aao-secretariat aao-secretariat Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Ladon verdict: Approve

Approve — clean subsequent pass, no blocking or medium findings.

This is a subsequent review (prior outcome: approve, no findings). The delta is the AdCP 3.3.0-beta.1→beta.2 adoption (version pins, regenerated generated types, docs) plus three load-bearing source changes merged from main: typed RequestValidationError/isPreDispatchError with dispatched: false markers, the legacy sync_creatives per-package ref reconciliation by constraint satisfaction with playerSizedRefs deprecation, and the #3529 ESM envelope-status loader binding.

Verified clean:

  • Reconciliation logic sound: per-axis width/height/duration union is intentional and documented, acceptors fail closed on unknown size/duration keys, cache only stores resolved versions — no witness-not-translator / fabrication violation.
  • Changesets present and correctly typed (minor for beta adoption/typed error, patch for regression fix) under beta pre-mode — no changeset-vs-wire-impact violation.
  • No official-client or version-pathway violations.

Decision table: no critical/high findings (row 1 n/a); gated_paths false (row 2 n/a); high_risk false (rows 3/5 n/a); no medium findings (rows 4/8 n/a); prior decision was approve (row 6 n/a); no no-auto-approve team match (row 7 n/a). Falls through to row 9 → approve.

Note: review_decision is REVIEW_REQUIRED, but gated_paths is false, so row 2 does not fire.

bokelley and others added 2 commits October 30, 2026 19:50
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@aao-secretariat

Copy link
Copy Markdown

Ladon cannot review this PR until merge conflicts are resolved.

bokelley and others added 2 commits October 30, 2026 20:20
…bscription-coverage

Brings in #3548. The packed-byte cap keeps the higher of main's and this branch's.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…/reporting-subscription-runtime

# Conflicts:
#	scripts/check-package-size.mjs
aao-secretariat[bot]
aao-secretariat Bot previously approved these changes Oct 30, 2026

@aao-secretariat aao-secretariat Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Ladon verdict: Approve

Approve — clean pass, no blocking or medium findings.

Checked:

  • No findings at critical/high/medium severity in this run (row 1 does not fire).
  • gated_paths: false → row 2 does not apply despite review_decision: REVIEW_REQUIRED.
  • high_risk: false, no deletions/modifications triggers (rows 3–5 inert).
  • Prior decision was approve, so sticky escalation (row 6) does not apply.
  • No no-auto-approve team match (row 7 inert).
  • Zero medium findings, well under the three-medium threshold (row 8 inert).

Delta is CI/test-scoped: comment-only additions to scripts/check-package-size.mjs, a retired version-pin swap to the live ADCP_VERSION in a test, a one-line string collapse in a coverage test, and an empty-frontmatter changeset correctly declaring no bump for a CI-only change touching no published library code. No witness-not-translator, version-pathway, wire-shape, or changeset-impact concerns in scope. Falls through to row 9 → approve.

bokelley and others added 4 commits October 30, 2026 22:24
A sub-daily billing_cycle was routed to the local sub-daily calendar,
which aligns to local midnight and ignores period_anchor, so every
boundary was off by the anchor's time of day. adcontextprotocol/adcp#8622
makes a sub-daily billing_cycle invalid; refuse it.

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

A later write that merely omitted replace_account_configs cleared both the
applied marker and a committed-but-unconfirmed replacement, so a failed host
hook was silently abandoned. An omitted flag now leaves both untouched; an
explicit false clears them, so a later true applies again.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…t owed period end

- An open action_required exception now takes the host's current
  pending_action (url, message, reason) when it changes, keeps its from, and
  notifies principal.changed. It used to keep the first action forever.
- accounts_unauthorized last_owed_period_end takes the latest frozen
  period_end. Frozen periods are not stored in end order, so the last entry
  could under-state the window.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
aao-secretariat[bot]
aao-secretariat Bot previously approved these changes Oct 30, 2026

@aao-secretariat aao-secretariat Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Ladon verdict: Approve

Approve — clean delta pass with no blocking or medium findings.

Checked:

  • No critical/high/medium findings surfaced by the reviewer.
  • gated_paths: false and no no-auto-approve team match, so no hard gates apply (row 2/7 do not fire despite review_decision: REVIEW_REQUIRED, since that only gates when gated_paths is true).
  • high_risk: false — no deletion/modification escalation triggers (rows 3/5).
  • Prior decision was approve, so sticky escalation (row 6) does not apply.
  • Three focused bug fixes to the new, unreleased reporting-subscriptions feature, each tested and citing upstream basis (adcp#8622, JCS canonical comparison, ===false retry-preservation). Changesets present for the work, satisfying changeset-vs-wire-impact.
  • No witness-not-translator violations: fixes fail-close/preserve rather than fabricate wire shapes.

None of rows 1–8 fire; falls through to row 9 → approve.

@bokelley
bokelley merged commit 2bd88d2 into main Oct 31, 2026
45 checks passed
@bokelley
bokelley dismissed aao-secretariat[bot]’s stale review October 31, 2026 14:17

The merge-base changed after approval.

@bokelley
bokelley deleted the bokelley/reporting-subscription-runtime branch October 31, 2026 14:17
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.

2 participants