Conversation
There was a problem hiding this comment.
Ladon verdict: Approve
Approve.
No blocking findings. Reviewer verified the schema-proof single-flight cache (support.py), payload-free receipt diagnostics (_diagnostics.py leaks only class name + source coordinates, confirmed by SECRETS greps), the boundary-parameterized _storage_errors seam (pg.py/handler.py), and both 500+-line adversarial test files against their base versions. Proof-key/epoch/cancellation logic is fail-closed, exception translation keeps context empty, and the single new public method is additive.
Two non-blocking follow-ups noted (not blocking): no explicit catalog-scan timeout, and brittle hardcoded counts in _hardening_installed.py. Neither rises to medium.
Gate checks: no critical/high/medium findings (rows 1, 4, 5, 6, 8 don't fire); gated_paths false (row 2 n/a); high_risk false (rows 3, 5 n/a); no no-auto-approve team match (row 7 n/a); first review (row 6 n/a). Falls through to row 9 → approve.
Independent review found that only `handler` and `store.ingest_receipt_batch` had committed boundary-label assertions, leaving `store.create_schema`, `store.receipt_ingestion_ready` and `store.read_receipt_boundaries` — all documented parts of the diagnostic contract — with no regression protection. A mislabelled decoration or a label missing from the runtime allowlist would silently reattribute an operator diagnostic with nothing failing. Exercise all four decorated raw boundaries on real PostgreSQL with genuine driver failures, asserting the injection fired, the exact safe code/message, empty cause/context, each boundary's own literal label, allowlist membership, exactly one record and a payload-free serialized LogRecord. Add an exhaustiveness guard so a new allowlisted store boundary cannot ship without its own executed regression, two mutation negative controls proving the label assertions are load-bearing, and a pin that a driver failure inside `validate_receipt_schema` stays the deliberately silent RECEIPT_SCHEMA_UNREADY path. Tests only; no production, documentation or workflow behaviour changes. The installed negative and preservation selections still resolve to exactly 30 and 48 cases, and the PostgreSQL-only additions skip correctly when the driver is absent. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Ladon verdict: Approve
Approve — clean test-only delta.
This PR adds ~161 lines of boundary-diagnostics regression coverage to tests/conformance/reporting/test_reporting_receipt_diagnostics.py, exercising the parameterized _storage_errors(boundary) decorator, the _BOUNDARIES fallback-to-"handler" negative control, and the deliberately-silent RECEIPT_SCHEMA_UNREADY validator path — all consistent with production in _diagnostics.py/pg.py.
Checks:
- No critical/high/medium findings from the reviewer.
- high_risk: false; gated_paths: false; review_decision: APPROVED.
- No no-auto-approve team match.
- Prior decision was also approve with no findings; nothing to re-surface.
None of decision-table rows 1–8 fire, so this falls through to row 9 → approve.
|
Issue #1196 ("test(reporting): shorten and instrument installed production gates") targets the same harness files this PR touches — Generated by Claude Code |
The base branch was changed.
There was a problem hiding this comment.
Ladon verdict: Approve
Approve — subsequent pass on PR #1191, no blocking findings.
The core feature source (outbox/support.py, _schema.py, status_schema.py, receipts/pg.py, _diagnostics.py) is byte-identical to the prior clean-approved head a09878f and is absent from this delta. The delta's genuine contribution is CI lane ownership (feed_hardening_installed moved to the installed lane, covered by exactly one lane), integration-baseline SHA/collation pins to integrated main, and a mechanical await-extraction test hardening — all consistent and correct.
Gate checks:
- No critical/high/medium findings → decision table rows 1, 4-8 do not fire.
- gated_paths: false → row 2 does not apply.
- high_risk: false → rows 3 and 5 do not apply.
- Prior decision was
approve→ row 6 (sticky escalate) does not apply. - No no-auto-approve team match → row 7 does not apply.
- Falls through to row 9 → approve.
Note: review_decision is REVIEW_REQUIRED but gated_paths is false, so the hard approval gate does not bind. The installed-feed and Postgres conformance CI lanes were IN_PROGRESS at review time and should be confirmed green before merge.
Activity capability discovery currently repeats the same live schema proof, and unexpected receipt-storage failures return a safe buyer error without a useful operator diagnostic. This change caches only a completed positive proof per support instance and records bounded, payload-free diagnostics at the original failure boundary.
Exact integration scope
ab12e1511058f0a9e75a8228f650d9a302423640, treec043d1e14c5071859f566e07cc9980058fa6ee07.e00dae171bff6eaf4e7f22342097f13ef00dc998, with ordered parents[a09878f67ab397a4b51b3314e3e8a5e87cf96da5, 2d777ace7b4bf8be519ce0abd4fd0a25ed4f1da7].2d777ace7b4bf8be519ce0abd4fd0a25ed4f1da7, tree2f71a273c0218e7ffc490fb4df243d02711cff7b.--binary --abbrev=8SHA25617385a7487bd513f0db40b0a8a9ffbb93a4d9e4d1257b60da0baf23ffab77914.37ce140d156369080a3db34e51603262592fdfaef2ac2e6be316c0c06de73daa. No corrective production, SQL, cache or manifest changes.This is an ordinary composition and appended correction, not a rewrite. Earlier
a09878f6results and reviews remain historical and are not attributed to this integrated head. PR1189 remains closed as superseded, with its branch and failed run preserved; the integrated feed content arrived through PR1215.Schema proof and receipt diagnostics
ReportingActivitySupportretains its frozen constructor and public wiring. A private cell holds only a completed positive schema proof, keyed by the support instance, exact B versus B+C wiring identities and packaged manifest digests. Every request still checks dynamic claims, concrete component types, pool and worker relationships, scheduling and account-listing topology. Capability responses and authorization are not cached. Cold callers share one scan; B+C validators use one physical catalog capture. Failures and cancellations never become positive evidence. Completed state is loop-neutral, and invalidation advances an epoch so an older scan cannot republish into it.Unexpected PostgreSQL failures and unexpected handler resolver/custom-store failures produce one structured ERROR with a static code, allowlisted boundary, exception class and sanitized module/function/line coordinates. The record retains no exception text, arguments, traceback, chain, locals, request payload, credentials or dynamically named task/thread context. The handler does not log an already-translated store error again. Expected domain errors and cancellation remain silent; the buyer's safe code/message and deliberate exception-chain suppression remain unchanged. A failing logging sink cannot replace the buyer error.
The historical catalog-performance concern from PR1183 remains separately recorded; integration credit for this correction requires this head's fresh gates. Existing historical thread dispositions are not rewritten by this PR body.
Composition corrections
2d777aceand verifies its tree, source bytes and installed origins. Its fetch has a one-minute limit. Earlier B2.3 snapshots are not qualified by this comparison.always(),permissions: {}and success-only checks.Fresh local evidence
These are implementer results, not independent review or remote CI acceptance. All raw logs, source identities, original failures and cleanup records are retained locally. Normal commit hooks passed.
Collection: core 2020, process 7, status 251, materializer 47, receipts 724, receipt compatibility 8, native feed 375, feed compatibility 9, installed feed 13. Collection is not execution credit. The new installed parent controls and full frozen/installed matrix were not run locally on this head; fresh CI must supply their complete lifecycles and Python 3.10 VCS/sdist origins. Prior successful artifact runs supply no credit here.
Private orchestration stops are preserved separately: a two-region merge-completeness assertion and refused unmerged commit, a relative evidence-path setup error before SDK execution, and a source-proof recorder comparison against a file absent from the old feature. None is represented as an SDK or CI outcome.
Security and retained limits
Fresh exact-head Gitleaks report SHA256
d34db299c7314018f205557d1825ecb2a6452d75593ea6faa6b59d10607516b0: all 17 changed files/256491 bytes, zero matches; all four commit/merge-parent additions/145726333 bytes, 82 matches individually checked at their exact Git objects (68 schema example identifiers, 13 patterned documentation credentials, one verified file digest), zero private or unadjudicated matches. Fresh large-positive and negative controls passed. Rules retain documented entropy/value-allowlist limits; no repository ignore/baseline, inline exemption or size cap was used. This is not GitGuardian success. Any substitution under the owner's standing train authorization remains a separate disposition after fresh scanner output.Arbitrary serving-time DDL is not detected automatically: drain callers and invalidate before controlled DDL, or use a fresh support instance after migration. Do not change database/search-path configuration behind a serving instance. Cold catalog statements retain adopter/database time bounds; this change adds no total cold-scan deadline. Overlapping cold validation on another loop fails closed. Driver errors translated by the readiness validator remain the contractually silent
RECEIPT_SCHEMA_UNREADYpath. Mounted local evidence is in-process ASGI, not live TCP.No new listener/auth surface or wildcard setting is introduced. The inherited wildcard listener, optional authentication and Bandit suppressions remain a separately documented exposure; no zero-alert or genuine-CodeQL-exception claim is made.
All six earlier rolling release-note exclusions remain open, with the pre-
2d777aceB2.3 hardening-comparison limit added. The inherited fail-closed pagination limitation remains, without a snapshot-retention promise. Source-only B2.3, epoch-0/quarantine/advertisement veto, B2.4 future work, #1172 delivery scope, #1199/TS holds and the translator's partial 8-pass/47-skip result remain separate. No artifact, release, activation or downstream acceptance is implied. Guard1201 stays draft/LAST and the legacy workflow stays disabled.