[bokelley]

Summary

Gate slice P5 of the reporting-subscriptions DX plan (§2.1, §2.3, §3.1, §6.2), stacked on #1738. Review the commit feat(reporting): serve reporting subscriptions from one SubscriptionOptions block and its follow-up fixes. The branch also carries #1748's one-line pin of pypa/gh-action-pypi-publish (GHSA-5xjx-3w67-w9x9, published today), which every branch needs for the Workflow security job to pass.

A seller now enables media_buy.reporting_subscriptions with one block on the service it already runs:

reporting = ReliableReportingService(
    store=store,
    account_context=resolve_account_context,
    subscriptions=SubscriptionOptions(max_accounts=500),
)
reporting.sources.register("gam", gam_source, capability_offerings=[daily_offering])
platform = reporting.install(MySeller())

What the SDK derives

Duty	Derived from	Override on SubscriptionOptions
Principal runtime, get_principal/sync_principal, principal section of capabilities	a PrincipalService the service owns and links to the engine (principal_options is automatic)	principals, principal_store
Roster	the platform's list_accounts (decisioning accounts.list), read under the principal's identity	accounts(principal, *, cursor)
Buys	the platform's get_media_buys (all statuses, paged), same identity	buys(principal, account_id)
Package offerings	every registered offering	buy_offerings(media_buy)
Offerings	registered capability_offerings, each with standard_dimensions	none
Report content	account_context	configure(generation)
Principal of a request	the caller the reporting tasks resolve, plus ToolContext.tenant_id	resolve_principal(request, context)

Identity outside a request. At sync_principal the service records the principal's binding: its tenant and a copy of its AuthInfo without credential material (key ids, client ids and verified signers only; extra is dropped). Reads for the roster and buys go through the platform's own handler with that identity restored, so visibility matches what the principal sees.

Scheduling.

• sync_principal (when it changes something), sync_accounts, create_media_buy and update_media_buy served through the installed platform queue a reconcile of the principals concerned. It runs after the response on a background drain that coalesces per account and caller, so a write never waits for it or fails because of it. await reporting.drain_subscription_triggers() waits for the drain, which stops at shutdown.
• Each read of the platform's list_accounts/get_media_buys is bounded by SubscriptionOptions.read_timeout (30 seconds by default).
• Each run_worker turn first reconciles every principal a period boundary has passed for since its last reconcile, then plans (the production cycle does the same).
• await reporting.touch(account_id=...) covers changes the SDK did not serve.

replace_account_configs is applied once per subscription_version, or when the flag changes to true, through end_account_configs(principal, account_id, ids, *, at) and account_delivery_configs. Without them the flag is still refused with UNSUPPORTED_FEATURE.

PrincipalRef(tenant_id, principal_id) names a principal across the reporting APIs. grants_task, authorizes_delivery and the materializer's delivery-authority hook take it; PrincipalIdentity is still accepted by the engine.

Production. With production= options, the principal tasks and write triggers are installed on the application the production handler delegates to. SubscriptionOptions.standard_dimensions supplies an offering's standard dimensions when its declaration cannot carry them yet: production offerings validate against the served 3.3.0-beta.1 contract. It goes away with P1.

Seller configuration codes

ReliableReportingConfigurationError gains code and recovery (dispatched is always None) for the four seller codes in error-codes.json. This slice raises two of them, each with a message ending in the fix:

• SUBSCRIPTIONS_REQUIRE_STANDARD_DIMENSIONS: no offering registered, or one without standard_dimensions.
• SUBSCRIPTION_ROSTER_UNAVAILABLE: the platform serves no list_accounts and no accounts override is given (at install), a listing that fails under the service context, or an account without a timezone. A failed listing never reads as an empty roster, so nothing is revoked.

The storage codes arrive with P7. A new parity test pins the code set to the vocabulary.

Other changes

• The decisioning Account gains timezone and operator, projected onto the wire, so decisioning sellers can serve the account zone that coverage needs.
• PlatformHandler serves get_principal/sync_principal when an installed service serves subscriptions, and advertises them only then.
• The engine store gains list_principals(). The engine state document records the binding, the last reconcile instant and the applied replacements, each only once set, so existing documents stay byte-identical.
• SubscriptionOptions, ReportingSubscriptionServiceOptions and PrincipalRef are exported from adcp.reporting; PrincipalRef also from adcp.server.

Python 3.10

Full CI on Python 3.10 showed that datetime.fromisoformat refuses wire instants with 1–5 fractional digits. This tripped #1738's fractional-seconds-and-offsets vector, which only ever ran under the stack's partial CI. The managed path and that test helper now parse through the SDK's aware_timestamp.

Deviations from the plan

• buy_offerings default. The plan derives a package's offerings from the product catalog. Python platforms have no catalog the SDK can read outside a request, so the default is every registered offering, which is right for a seller whose products share their reporting offerings. Sellers with differing products supply buy_offerings.
• producer is not on SubscriptionOptions yet. Bring-your-own producer is P2 (T19/P17); adding the key now would advertise something unimplemented.
• serve(..., principal=, reporting=, storage=) is not added. The managed path uses the existing reporting.install(platform) integration point; the storage= part belongs to P7.
• The boundary sweep is per principal. It reconciles a principal when any offering's calendar, in any zone its accounts use, has a boundary after its last reconcile.
• is_period_delivered is not derived from the ledger. It stays an optional override; without it the engine keeps its SLA-based withheld count.

Tests

• tests/conformance/reporting/test_reporting_subscription_production.py adds the production plus managed composition, on memory and PostgreSQL: principal tasks delegated to the application, then sweep, planning and obligations.
• tests/test_reporting_subscription_managed.py (18 tests) covers a write that never waits for its reconcile, a hung read bounded by read_timeout, and, on an ADCPHandler seller and on a decisioning platform served by its own PlatformHandler:
  • the block alone serving subscriptions end to end, from sync_principal to obligations and subscription_coverage;
  • capabilities;
  • both config codes;
  • a failed listing that is reported and revokes nothing;
  • the sweep running only after a boundary;
  • write triggers, and no reconcile on an unchanged write;
  • each override;
  • replace_account_configs refused, then applied once per version and on a flag change;
  • a binding that keeps no credential material.
• The subscription, reliable-reporting, principal, decisioning-handler, parity and public-API suites pass, as do ruff and mypy.

Refs #1695

🤖 Generated with Claude Code

[aao-secretariat]

Ladon verdict: Comment (human reviewer recommended)

Comment — 3 medium findings, no blocking issues.

This is a mostly-additive managed reporting-subscription path (SubscriptionOptions). Floor checks are clean: forward-compat unions untouched, type-layering intact, no generated hand-edits, and the recorded principal binding strips credential material (verified by test). The authorizes_delivery/engine signature changes are not a semver break — those symbols were introduced in an unreleased commit and ship in no beta tag, landing within one unreleased 10.0.0 cycle. No critical/high findings.

Three medium findings surfaced. None falls into the data-loss/schema/infra category, so no escalation triggers (rows 4/5). High-risk files are all (modified) — but row 5 only fires when a medium lands on one of those high-risk modified files; the three mediums here are in managed.py, handler.py, and service.py (reporting), none of which are the high-risk-flagged decisioning/server files. Row 8 fires (≥3 mediums) → comment.

Medium findings (a human should look):

• src/adcp/reporting/subscriptions/managed.py:740 — Platform reads in managed subscriptions have no timeout; a hung upstream hangs the buyer write.
• src/adcp/decisioning/handler.py:2924 — Write-trigger reconcile runs inline and scales with principals sharing the account.
• src/adcp/reporting/service.py:951 — Production + managed subscriptions path (including the __class__ swap in _install_principal_tasks) is untested.

Medium findings

• src/adcp/reporting/subscriptions/managed.py:740 — Platform reads in managed subscriptions have no timeout; a hung upstream hangs the buyer write
• src/adcp/decisioning/handler.py:2924 — Write-trigger reconcile runs inline and scales with principals sharing the account
• src/adcp/reporting/service.py:951 — Production + managed subscriptions path is untested

[aao-secretariat]

Ladon verdict: Approve

Approve — delta on this pass is a two-line typing tightening in handler.py (get_principal/sync_principal params moved from Any to the public GetPrincipalRequest/SyncPrincipalRequest models). Clean: both are proper public exports, # type: ignore[override] already present, params flow unchanged through _invoke_principal_method, and the methods are new in this PR so no semver break.

Medium findings

• src/adcp/decisioning/handler.py:2926 — Write-trigger subscription reconcile runs inline on the buyer write path

The two prior mediums in managed.py and service.py are outside this delta and were not rechecked per scope rules. Only 1 current medium finding surfaced, with no critical/high findings and no escalation trigger. High-risk flag is true but all reasons are (modified) files with only a single medium concern on handler.py — row 5 requires high_risk + modified + a medium finding, which the handler.py:2926 finding does satisfy.

Re-evaluating: high_risk is true, handler.py is (modified), and there is a medium finding on it. That matches decision table row 5 → escalate.

Medium findings

• src/adcp/decisioning/handler.py:2926 — Write-trigger subscription reconcile runs inline on the buyer write path

[aao-secretariat]

Ladon verdict: Approve

Approve.

This pass is annotation-only: get_principal/sync_principal params tightened from Any to GetPrincipalRequest/SyncPrincipalRequest with specific # type: ignore[override] codes (specific codes are allowed; matches the Liskov override pattern used by every other handler method; no runtime or wire change).

Medium findings

• src/adcp/decisioning/handler.py:2926 — Write-trigger reconcile runs inline and scales with principals sharing the account (persistent, unchanged)

Only one medium finding this pass. The prior run's two other mediums (managed.py:740, service.py:951) are outside this delta and not rechecked per subsequent-pass scope rules. Decision table walk: no critical/high (row 1 no); gated_paths false (row 2 no); no deletions in high-risk reasons (row 3 no); the single medium has no data-loss/schema/infra category (row 4 no); high_risk true with modified files AND a medium present — row 5 would fire. Wait: row 5 applies because a high-risk file was modified and a medium finding exists.

Re-evaluating row 5: high_risk is true, handler.py is (modified) and matches a high-risk glob, and a medium finding exists on that file → row 5 fires → escalate.

Medium findings

• src/adcp/decisioning/handler.py:2926 — Write-trigger reconcile runs inline and scales with principals sharing the account

[aao-secretariat]

Ladon verdict: Approve

Approve.

This is a subsequent review following a prior approve. The delta reworks reporting-subscription write triggers from an inline awaited reconcile into a single-task background drain (managed.py trigger/_enqueue/_run_drain/drain/aclose, L717–752). This resolves the prior Medium finding (reconcile scaling with principals sharing the account) — a served write no longer waits on the reconcile.

Reviewer verified the key invariants: drain single-task invariant (enqueue guards on self._drain is None or done(), no await window between the while self._queued exit and task completion, so no stranded entry); read_timeout (default 30s, positive-validated) bounds every platform read via asyncio.wait_for in _call, mapping timeout → SUBSCRIPTION_ROSTER_UNAVAILABLE; and the aclose-drops-queued-work path, which is safe since obligations only materialize at period boundaries and boundary_due keys off the last reconcile timestamp. Both new behaviors are tested.

No new blocking or Medium findings. high_risk is true (decisioning/** and server/** modifications) but all entries are (modified) with no medium-or-higher concern on them, so the flag alone does not escalate. gated_paths is false and review_decision is APPROVED. No no-auto-approve team match. No decision-table trigger fires (rows 1–8 all pass), so the outcome falls through to row 9: approve.

[aao-secretariat]

Ladon verdict: Approve

Approve.

Scope: a two-file delta swapping naive datetime.fromisoformat for the existing aware_timestamp helper in managed.py:_instant and the at() test helper. The helper handles 1-6 fractional digits, subsecond offsets, and fail-closed naive/invalid rejection. The only behavioral change is a uniform ValueError on bad string input; _instant callers don't catch it, so propagation and fail-closed semantics are unchanged. No wire-shape, public-API, or spec-drift impact.

Decision path:

• No critical/high/medium findings (row 1, 8 do not fire).
• gated_paths: false; review_decision is APPROVED (row 2 N/A).
• high_risk is true, but all reasons are (modified) with no medium-or-higher finding on those files — presumed safe; no (deleted) reason (rows 3, 5 do not fire).
• No data-loss/schema/infra medium findings (row 4 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. This is a continuation of a prior clean approve; the new delta remains clean.

[aao-secretariat]

Ladon verdict: Approve

Approve — clean subsequent-pass diff with no findings.

The only delta since the prior clean pass is a single line in .github/workflows/release-publish.yml: the pypa/gh-action-pypi-publish pin moves v1.13.0→v1.14.3 (SHA-pinned with trailing version comment per repo convention), addressing GHSA-5xjx-3w67-w9x9. No code, public surface, or wire shape touched.

Checks applied:

• No critical/high findings (row 1 n/a).
• gated_paths: false; review_decision: APPROVED (row 2 n/a).
• high_risk is true but all reasons are (modified) with no medium-or-higher finding on any of them — presumed safe per the high-risk flag guidance; no (deleted) entries (rows 3, 5 n/a).
• No medium findings at all (rows 4, 8 n/a).
• Prior decision was approve, so sticky escalation (row 6) does not apply.
• No no-auto-approve team match (row 7 n/a).

Falls through to row 9 → approve. Note: reviewer could not independently confirm the pinned SHA resolves to the v1.14.3 tag due to sandbox egress restrictions — recorded as a non-confirmable observation, not a defect.

[aao-secretariat]

Ladon verdict: Approve

Approve — managed reporting-subscriptions path reviewed; 1 medium finding, no blocking issues.

Medium findings

• src/adcp/reporting/subscriptions/managed.py:104 — _MAX_PAGES bounds the buys walk but not the roster walk its comment claims to cover (engine._list_accounts is an unbounded while-loop). Worth addressing but non-blocking.

What was checked: Binding snapshot keeps only identities (key_id/client_id/keyid, extra dropped — aligns with the no-credentials-in-metadata floor); read_timeout bounds platform reads; write triggers run off the request path with reported failures; the authorizes_delivery/SubscriptionDeliveryAuthority str→PrincipalRef change is an unreleased experimental-surface symbol (feat, not a published-contract break). No generated-code hand-edits, import-layering, or forward-compat regressions.

Gates: gated_paths false; review_decision: APPROVED; no no-auto-approve team match. high_risk true but only (modified) reasons under decisioning/server paths — the single medium finding is in reporting/subscriptions/managed.py, not on any modified high-risk file, so row 5 does not fire. No critical/high findings; 1 medium (below the 3-medium comment threshold). Falls through to row 9 → approve.

Medium findings

• src/adcp/reporting/subscriptions/managed.py:104 — _MAX_PAGES bounds buys but not the roster it claims to cap