Repository navigation
fix(compliance): qualify independent catalog reporting scopes - #8336
Conversation
8ccc1d6 to
5ad539b
Compare
There was a problem hiding this comment.
Ladon verdict: Approve
Approve — clean diff, no blocking or medium findings.
Checked:
- Changeset: patch changeset present (
.changeset/catalog-optional-delivery-reporting.md), correctly scoped to compliance assets. No schema understatic/schemas/source/**touched, so no schema↔docs coherence concern and no oneOf discriminator regression. - Released artifacts: no file under any existing
dist/<semver>/artifact modified or deleted — immutability preserved. - New adapter (
server/src/compliance/catalog-reporting-runner.ts): fail-closed; capability-discovery failures throw a stable public message with remote diagnostics confined to a non-enumerableError.cause(verified not to leak into Addie model context); budget derived from the exact selected price snapshot; non-reporting storyboards pass through the unchanged SDK path. Well-tested across the added.test.cjs/.test.tssuites. - Addie/registry edits (
member-tools.ts,registry-api.ts,config-version.ts): no REST self-report-score path introduced; certification remains Addie-tool-gated. - Docs (
known-limitations.mdx) coherent with the scenario rewrite; one disclosed, tracked limitation (hosted full-run viacomply()— SDK #3382/#3381) is a tracked follow-up, not a defect.
Decision-table trace: no critical/high/medium findings (rows 1,4,5,6,8 n/a); gated_paths false (row 2 n/a); high_risk true but all reasons are (modified) with no medium finding (rows 3,5 n/a); no no-auto-approve team match (row 7 n/a); no prior decision (row 6 n/a). Falls through to row 9 → approve. high_risk is a heuristic LOOK signal only; modified high-risk files with no medium-or-higher concern are presumed safe.
There was a problem hiding this comment.
Ladon verdict: Approve
Approve — subsequent pass, net clean.
This is a follow-up push on a PR whose prior decision was already approve with no findings. The delta (5 files, 273 lines) adds strictly additive fixture_resolutions producer-evidence retention in catalog-reporting-runner.ts:380-396, with coherent docs/memo/test updates.
Checks confirmed:
- Enrichment is additive only — no grade, phase, count, or wire field overwritten;
=== undefinedguard preserves native evidence. - Public security test confirms host-denial throws before enrichment, no private-fixture leakage.
- Full/standalone tests pin failed/passed/skipped counts and phase length, proving no graded rows added.
- No wire/schema change (compliance tooling only, not static/schemas/source or dist artifacts); changeset already present and in scope.
No Critical/High/Medium findings. high_risk is true only via (modified) entries, but with zero medium-or-higher findings the modifications are presumed safe (row 5 does not fire). gated_paths is false. No no-auto-approve team match. Rows 1–8 do not fire → row 9 approve.
093b7a4 to
aec14f6
Compare
aec14f6 to
82e6944
Compare
There was a problem hiding this comment.
Ladon verdict: Approve
Approve — clean pass, no blocking or medium findings.
Reviewed the new 451-line catalog-reporting-runner.ts, the two reworked media-buy reporting scenarios, the member-tools/registry-api/sdk-safe-fetch wiring, and the test suite. Runner control flow is sound: failed discovery delegates execute(originalOptions) preserving the trusted sandbox rewrite without a partial profile; budget derives as max(1000, declared minimum) with the fixed price never reshaped into an auction bid; reserved/protected caller maps and supplied prepared state are refused before seller contact; PreparedFixturesDeniedError discloses only machine codes. Operational readiness holds — every hosted path carries an AbortSignal deadline composed with per-request signals at the safe-fetch boundary, enforced even when the SDK omits the run signal; the 1 MiB cap only tightens. Changeset is a correctly-scoped patch on compliance assets with no static/schemas/source change — no schema↔docs drift, oneOf regression, or immutable-artifact concern.
Decision path: no critical/high findings (rows 1 skipped); gated_paths is false (row 2 n/a); high_risk is true but all reasons are (modified) with zero medium findings, so rows 3–5 do not fire; prior decision was approve (row 6 n/a); no no-auto-approve team match (row 7 n/a); zero medium findings (row 8 n/a). Falls through to row 9.
Problem and behavior
Optional reporting support was inferred from generic wholesale/controller gates and a seeded video product. This patch independently discovers native format-breakdown and time-view products through fully paginated, schema-validated wholesale sandbox discovery. A completed valid no-match is
fixture_unsatisfied, projected not applicable: feature not tested, not a pass or proof of nonsupport. Fixture discovery/schema/protocol/pagination failures and broken selected support remain graded failures.advanced_delivery_reportingselects non-guaranteed fixed-price CPM products advertising format breakdown. Newtime_based_delivery_reporting(introduced in 3.3) independently selects the same commercial slice listingtime_based_views. Required response fields, legal returned measurements, accepted impression/click totals and metric narrowing remain graded. No specific threshold, basis, format kind, truncation=true or nonempty observations are invented. A pass does not attest that the seller returns time-based views: the controller cannot inject those observations.Repository full/standalone callers prepare the graph once using SDK 16.3, capture the SAME selected price, and derive budget
max(1000, declared minimum when present). Absence/0/5000 stay distinct in that protected snapshot. Purchase has no invented top-level currency; its selected pricing option determines currency. Standalone preparation is invocation-local, not a durable grant/state service. Collection storyboards are unchanged. Refs #8254; the separate collections follow-up closes the remaining scopes.Review fixes
discovery_failed; some native capability-read failures still become synthetic-profile capability skips (SDK Spec inconsistency:Submittedarm in per-tool responseoneOffor 4 of 6 HITL tools #3392), explicitly not qualified as conformant discovery.{ code, refusal_code }; the endpoint OpenAPI response union and actual handler/schema regression match that body. Only the authorized audit sink receives refusal records; no result/capture/binding leaks to the refused caller. A pre-preparation refusal honestly has no seller resolution or exchange to invent.prepared_fixtures_authorizeris refused before all seller contact, with typedauthorizer_deniedand private zero-exchange evidence. The hook is called zero times, because SDK 16.3 exposes no truthful public pre-preparation binding builder. This is fail-closed availability, not implementation of the exact-once callback requirement. No invented binding/reference/profile or alternate hook is used. Ordinary invocation-local reuse keeps every-lookup checks, closes infinally, and isolates concurrent grants.Verification
Review-fix head:
82e6944430088ac39aa7bb4047b3e09929270ada, rebased ontomainate2f2d3a4f837a1dc2f923390ea4f819fa7d44ce3. The final upstream rebase added only the unrelated display-tag schema refactor #8418; all reviewed implementation/test bytes are unchanged. Existing approval and earlier CI are not carried forward as exact-head qualification. The local advisory storyboard matrix was explicitly skipped at push; non-draft GitHub storyboard CI is required.Published npm @adcp/sdk@16.3.0, gitHead
765aa8986a93cdab00da873cee47b98f9678bd75, containing foundation37d30876; ordinarynpm ciand all 6,398 installed payload files match the registry archive (SHA25624dace3ec5babd5b380e34f697cc9ee73f05353c0055f8bf768366a70a76205d). No active local pack, SDK shim or dependency/lock delta in this review fix.Current final reporting matrix: 174/174, 0 failures or skips and natural exit 0, plus 27/27 context-lint checks. This includes 3 explicitly labeled longstanding known-gap characterizations and native capability-compatibility controls; it is not 174 repaired seller-conformance claims. Actual public transport exercises independent reporting scope, absence/0/5000 selected snapshots and derived budgets, preserved failures, two concurrent real invocations, denied/late grants and response-budget enforcement. Hosted unit regressions cover the actual registry handler/schema, served-artifact selection, safe-fetch deadline composition, Addie labels and Socket option forwarding.
Ordinary hooks passed 1,272 root tests and all 794 server test files (13,141 passed, 31 existing skips), plus canonical typecheck. The final OpenAPI response-contract correction passed the same ordinary hooks again: 1,272 root tests, 794 server test files (13,141 passed, 31 existing skips), and canonical typechecking, with natural exit 0. The policy-supported four server shards were used; live Anthropic keys were removed from the test environment. No hook or timeout was disabled. Historical failed runs remain preserved: live model keys caused five HTTP-route fixture failures, corrected only by removing those live keys from the test environment; overlapping SDK/server runs had SIGKILL/OOM events; six added snapshot assertions initially read protected data from a run projection and were corrected to inspect the public prepared state. No hook, timeout, grading guard or failure expectation was weakened.
Native training runs currently find no qualifying product and execute zero reporting rows; these are coverage gaps, not positive feature evidence. Earlier 155/170 counts are historical and are not substituted for final-byte checks.
Root independently reviewed the exact pushed helper/source delta and found no additional source blocker. Its separate ignored npm 16.3 consumer run produced 123 passes and one missing frozen-schema setup failure; after restoring 1,625 Git-verified schema files without changing tests, the one failed case passed on rerun. Those are 123 plus 1 passes across two runs, not a single green 124-test run.
Independent protocol, code, security and product delta reviews found no remaining source blocker after their concrete findings were corrected. They are agent reviews, not eligible human/WG approval. Root will re-review this delta and owns ordinary merge disposition. Exact-head non-draft CI is complete: 63 successful checks, one normal skip, zero failures. All five required checks passed. Build Check and Training Agent Storyboards both completed successfully on
82e6944430088ac39aa7bb4047b3e09929270ada; canonical shard 2 executed all 174 reporting regression checks with zero failures. Both native reporting training rows explicitly loggedfixture_unsatisfiedand executed zero graded feature rows; neither supplies native positive feature evidence. No inline comments or unresolved threads remained. The reviewed head was squashed as3e94f5e376bae2911fe26373a8ff506908f1a765by the maintainer.Bounded follow-ups and release scope
simulate_deliveryobservations, following merged feat(compliance): gate property-grain delivery conformance on a seller rollup #7982. #8412 remains general maturity/value-or-disclosure accountability.Guaranteed, auction-priced, brief-only and seeded-only support is not tested. The scheduled heartbeat still invokes bare comply without per-storyboard budget/brand derivation until #3381/#3382. No bundle, durable cross-invocation or fully interruptible Socket Mode parity claim. Details:
specs/optional-product-reporting-qualification.md.Main-first patch; any 3.2.x backport needs separate actual-pin qualification and review. No release/publication/VP write/dispatch, immutable artifact edits, production action or maintenance merge is performed here. Main-merge side effect was separately routed to the maintainer before their merge: existing
release.ymlcan refresh draft Version Packages #8419 and generate/register/sign candidates. The #8302-only permission does not extend. Root owns final merge authorization; this PR does not bypass it.