[bokelley]

Part of #1695, slice 4 (managed delivery): 4a1, the delivery contract. It's the first part of the memory-store slice the owner approved, matching adcontextprotocol/adcp-client#3523. It stacks on #1730, #1729, #1728, #1725 and #1723, so it contains their commits. Review only feat(reporting): subscription rows carry account_id and delivery re-checks authority and feat(reporting): freeze a subscription destination path_template on its binding.

account_id on subscription rows (deliberate digest decision)

AdCP 3.3 (sync_principal, Destinations): "Every delivered row carries account_id."

• Stamping: for an obligation of a reporting-subscription generation, the producer stamps account_id on every row in commit_revision_from_manifest, before the revision is bound:
  • the Core revision digest (revision_content_sha256, echoed by get_media_buy_delivery) covers it;
  • for managed delivery, the canonical content digest (verified_publication → verifier.canonicalize) covers it too, so destination verification checks the bytes the buyer receives.
• Refusal: a row that already names another account is refused with ROW_ACCOUNT_MISMATCH.
• Requirement on definitions: the report definition's row schema must admit an account_id string. Canonicalization validates each row against it, so a definition that forbids the column fails closed with SOURCE_INVALID rather than delivering rows without it. Sellers serving subscriptions declare account_id in their row schemas. This is the spec ambiguity TS #3523 also records ("doesn't say whether the reporting profile must declare the column").
• Account-level obligations are untouched. Their rows and both digests are byte-for-byte unchanged, which a test asserts.

Authority at each delivery attempt

• The hook: ReportingMaterializerService.authorize_subscription_delivery(principal_subject, account_id) re-checks the owning principal's authority for a subscription generation's work at three points:

  • before preparing;
  • before writing;
  • after writing, before verifying and settling.

  This follows the spec: "Authority is checked at each delivery attempt, including retries."

• A loss holds the materialization. The service raises AUTHORIZATION_DENIED with same_identity: effect not_started before writing, unknown after. The store keeps the work pending under the same external identity and retries after authority_hold_seconds (60 s by default). It is never failed and the obligation stays owed, like TS's holdMaterialization.

• ReportingSubscriptionEngine.authorizes_delivery(subject, account_id) answers from the grant. It must list get_reporting_status, the same rule as coverage (feat(reporting): check reporting tasks against the current subscription grant #1728).

• Account-level generations keep only the existing destination-authority checks; the hook is never called for them.

path_template frozen on the binding (render_path_template)

• On the binding: ReportingDestinationBinding.path_template holds the subscription version's destination path_template. Only a subscription generation's binding may carry one.
• Validation: it must be a relative path with no empty, . or .. segments, using only {account_id}, {subscription_id}, {offering_id} and {period_start}.
• Expansion: binding.delivery_path(period_start) expands it with render_path_template for each delivery, one path segment per value, percent-encoding /. ReportingPreparedRevision.delivery_path exposes it to writers.
• Writers place a subscription's objects under it. The reference writer puts them below its collision-free attempt prefix ({external_id}/{delivery_path}/part-…), as TS does.
• Account bindings are byte-for-byte unchanged. ledger_asdict omits an unset path_template, so their fingerprints and stored payloads stay as they were (tested).

Next (4a2: production composition, memory store)

• Owed sets in the production profile:
  • a subscription admission path, since production admission today assumes a validated sync_accounts configuration;
  • an owed set's per-obligation denominator, since production freezes one denominator per generation;
  • planning in the production cycle.
• Destinations: binding to the destination's current generation, with the frozen method and per-account authorization.
• Owner-scoped notifications.
• Wiring: the production service passes engine.authorizes_delivery as the hook.

4b then covers the PostgreSQL schema, including lifting the account-only checks from #1729.

Tests

• tests/test_reporting_subscription_delivery_authority.py covers:
  • account generations never consulting the hook;
  • a lost grant holding the attempt under the same identity, at both effects;
  • a current grant delivering;
  • the engine helper.
• tests/test_reporting_subscription_service.py covers subscription rows carrying account_id, account rows unchanged, and a foreign-account row refused.
• All reporting suites pass locally.

This does not adopt beta.2.

🤖 Generated with Claude Code

[aao-secretariat]

Ladon verdict: Approve

Approve.

Single reviewed scope (top commit 6db92f5, 5 files, +214) is clean against the repo's mandatory gates: subscription_rows stamps account_id fail-closed (ROW_ACCOUNT_MISMATCH), account-level rows are preserved byte-for-byte, and the insertion point ensures both Core and canonical digests cover it. The three _require_subscription_authority checks are correctly placed and fail-closed on is not True; MaterializerContext.binding is non-optional so no crash path. New fields are additive feat: and not on the tracked public surface — no semver-break, no forward-compat/discriminated-union regression, no generated-code hand-edits, no credential-in-ctx_metadata concern.

Medium findings

• src/adcp/reporting/materializer/service.py:121 — Subscription authority hook awaited without timeout/cancellation; a hung call holds the lease indefinitely via the heartbeat. Worth a follow-up timeout wrapper.

Decision table: no critical/high findings (row 1 no). gated_paths is false (row 2 no). high_risk is true but the only matching file (src/adcp/decisioning/handler.py) is (modified) with no medium finding on it, so rows 3 and 5 do not fire. The single medium finding is on materializer/service.py — not a data-loss/schema/infra category and not on a high-risk-modified file (rows 4/5 no). No prior decision (row 6 no). No no-auto-approve team match (row 7 no). Only one medium finding, fewer than three (row 8 no). Falls through to row 9 → approve.

Medium findings

• src/adcp/reporting/materializer/service.py:121 — Subscription authority hook awaited without timeout/cancellation

[aao-secretariat]

Ladon verdict: Approve

Approve — clean delta, no blocking or medium findings.

This PR adds an optional path_template to ReportingDestinationBinding with expansion, serialization-stripping, a prepared-revision property, and reference-writer glue. Reviewer verified:

• _validate_path_template (delivery_models.py:249-261) rejects absolute paths, backslashes, ./../empty segments, >1024 length, unknown placeholders, and unbalanced braces.
• render_path_template percent-encodes slash/dot values so no substituted value escapes a segment.
• _ledger_dict (models.py:346-350) drops path_template only for null destination_binding values via a "" sentinel, keeping account-binding fingerprints byte-stable (covered by test_account_bindings_keep_their_fingerprint_and_payload).
• reference.py:230-236 changes object placement only; idempotency/readback key off unchanged external_id.

No Critical/High/Medium findings this run. The prior Medium (service.py:121 — subscription authority hook awaited without timeout) is outside this delta and non-blocking.

Decision rules walk-through:

• Rows 1 (crit/high): no findings → skip.
• Row 2 (gated_paths): false → skip. (review_decision is APPROVED regardless.)
• Row 3 (deleted high-risk): no deletions → skip.
• Row 4 (medium data-loss/schema/infra): no medium findings → skip.
• Row 5 (high_risk modified + medium): handler.py is modified and high_risk, but no medium finding exists → skip.
• Row 6 (sticky escalate): prior decision was approve, not escalate → skip.
• Row 7 (no-auto-approve team): no match → skip.
• Row 8 (≥3 medium): zero medium → skip.
• Row 9: approve.

The high_risk flag fires only on a modified decisioning/handler.py, but with no reviewer concern on it the modification is presumed safe. Prior Medium was resolved/out-of-delta; net improvement.

[aao-secretariat]

Ladon verdict: Approve

Approve.

Delta since the prior approve is minimal and sound: path_template was added to the closed-payload allowlist in reporting_reconciliation_validate (reporting_ledger_reconciliation.sql:517) with the matching fingerprint bump in outbox/required_schema.json. path_template is a legitimate non-secret AdCP 3.3 reporting-subscription field; the allowlist only governs permitted key names and the load-bearing content_sha256 = reporting_payload_sha256(payload) digest binding is unchanged. The fingerprint _digest is PostgreSQL-conformance-gated, so an incorrect bump fails CI. No findings.

Gate checks:

• No no-auto-approve team match.
• gated_paths: false; review_decision: APPROVED — no hard gate active.
• high_risk: true only via src/adcp/decisioning/handler.py (modified), but reviewer found no medium-or-higher concerns on it, so row 5 does not fire; no deletions (row 3 n/a).
• Prior decision was approve, so sticky escalation (row 6) n/a.
• No critical/high/medium findings.

None of rows 1–8 fire → row 9 approve.

[aao-secretariat]

Ladon verdict: Approve

Approve — clean delta, no blocking or medium findings.

The reviewer reports a clean pass on this subsequent push (prior decision was also approve). Verified against the decision table top-down:

• Rows 1, 4, 6, 8: No findings at any severity (no critical/high/medium). Not triggered.
• Row 2: gated_paths is false. Not triggered.
• Rows 3/5: high_risk is true (src/adcp/decisioning/handler.py modified), but the only change-kind is (modified) with no (deleted) and no medium-or-higher finding on it. A modified high-risk file with no concerns is presumed safe. Not triggered.
• Row 7: No no-auto-approve team match. Not triggered.

With none of rows 1–8 firing, the outcome falls through to row 9 = approve.

What the reviewer checked: subscription path_template is defended against traversal at both template validation and value expansion; account bindings stay byte-stable (ledger_asdict omits unset keys, no KeyError); account_id stamping routes through the single commit_revision_from_manifest path before all three digests with ROW_ACCOUNT_MISMATCH refusal; the per-attempt authority hook is bounded by the attempt I/O deadline, fails closed, and holds no lease forever. Tests cover held attempts, hung-hook deadline, path refusal, and byte-stable account payloads.