feat(reporting): add destination verification and strict revision selection - #1186
Conversation
…ection B1 of B1/B2: immutable writer/resolver contracts, SDK-owned verification, a non-production reference writer, strict whole-history revision selection, and a fenced C checkpoint semantics migration. No materializer persistence or Managed readiness is added. Refs #1167
![aao-secretariat[bot]](https://avatars.githubusercontent.com/u/221690821?s=60&v=4){kind=small}
There was a problem hiding this comment.
Ladon verdict: Approve
Approve — no blocking or medium findings.
Reviewed PR #1186 (B1 of #1167): immutable destination-I/O contracts, SDK-owned source/destination verification, the strict whole-history select_reporting_revision selector, a non-production reference writer, and a fenced C selector-epoch migration. Reviewer covered the mandatory largest files (verification.py, contracts.py, reference.py, revision_selection.py), operational-readiness (deadline/cancel via ReportingIOContext), credential/import-layering/generated-code invariants, and the SQL fence. All frozen-head gates green. Additive public surface under a correct feat(reporting): prefix.
Decision-table trace: 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 at all (rows 4 & 8 n/a); no prior decision (row 6 n/a); no no-auto-approve team match (row 7 n/a). Falls through to row 9 → approve.
`commit_revision_from_manifest` chose `supersedes` from a whole-history selection, which returns the *official* revision outright once one exists. A snapshot restatement published after an official close therefore rooted itself at `None`, leaving the obligation with two snapshot roots -- `disconnected_snapshot_history` over immutable rows, which health projection, producer acquisition, consumer-status ingest and status evidence validation all park for a repair the SDK cannot perform. Take the leaf from the snapshot subset instead, and fail closed when either pass reports corruption. The replaced `_current_snapshot_leaf` chained this correctly. Two further closed-contract gaps found in the same review: - A retained Core `ReportingDefinitionBinding` is only loosely validated, so an obligation can carry a definition -- a versioned query URI, an uppercase or short digest, a legacy dialect or schema version -- that the strict `ReportingVerificationKey` cannot express. Rebinding it raised a raw `ValueError` out of `ReportingRevisionVerifierRegistry.prepare()`, `ReportingDestinationIO.write()` and `.verify()` rather than the closed `BINDING_MISMATCH` every caller of that boundary catches. - `plan_consumer_statuses`' new fail-closed history checks reported "no reading was supplied" when the missing input was `obligation_revisions`, pointing adopters at the wrong argument, and the existing revision-without-reading regression test silently stopped covering its own path. Name the obligation and the argument, document the complete-partition contract on the public function, and restore that coverage. Regression tests cover snapshot-after-official chaining on memory and PostgreSQL, all four definition coordinates through prepare/write/verify, and the absent, short, long and damaged partition shapes. Documents the new 1..500 `read_revision_rows` page-size bound. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`Test Python 3.12` on head a89f990 (run 35163238220, job 105018502311) was cancelled at the 30-minute job ceiling, not by a failing test: step timestamps put checkout, Python setup, the editable `[dev]` install, ruff, mypy, `mypy --strict` and the type-ignore contract at 1m27s, leaving the coverage suite 28m47s before the runner killed it — the log shows 99% of ~9.4k tests at 00:09:23 with no failure and no summary. The other three legs finish in 17-19 minutes because only 3.12 pays for coverage tracing on top of the same suite. A ceiling that cancels a passing job reports a red matrix and hides whatever the run was meant to tell us. Keep the complete suite and the coverage gate. Give the job 60 minutes so the step bound plus setup and cleanup still fit with margin for runner variance, and bound each pytest step at 45 minutes so a genuinely stuck suite fails that named step with its own timeout instead of silently consuming the job. Validated with actionlint 1.7.7 -- the version the workflow-security job pins -- over ci.yml and every other workflow. Workflow-only: `src/` and `tests/` are byte-identical to the previous commit, so that commit's local gate results carry over unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Ladon verdict: Approve
Approve — clean pass on the delta with no blocking or medium findings.
Checked:
producer.pynow derives snapshot supersedes from a snapshot-only whole-history selection, correctly rooting a restatement after an official close instead of splitting into two snapshot roots.verification.pyfails closed to BINDING_MISMATCH on loose Core definitions._consumer.pysharpens error messages to name obligation_revisions.- CI timeouts widened with justified step-level bounds.
Decision-table walk: no critical/high/medium findings (rows 1, 4, 5, 6, 8 do not fire). gated_paths is false and review_decision is APPROVED (rows 2). high_risk is false with no deletions (rows 3, 5). No no-auto-approve team match (row 7). Falls through to row 9 → approve. Prior decision was also approve; this is a consistent net-clean state.
The base branch was changed.
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
There was a problem hiding this comment.
Ladon verdict: Approve
Approve — subsequent pass on PR #1186, clean.
This is a re-composition of the B1 destination-verification feature onto integrated rc.6 main. The bulk of the delta is the main merge (rc.6 schema cache + regenerated generated_poc models); the only novel authored change is test/control-correction commit 157809d — semantics-preserving await-hoists (which also harden the -O lane), a fail-closed C-control repin to 967b6e2, and the require_rolling_database fixture swap. Feature core (revision_selection, materializer) is unchanged from the prior clean approval.
Checked: no credential/ctx_metadata regressions, no semver break on the public surface, type-system import layering intact, no hand-edits to generated code beyond regeneration output, CI gates remain armed. No Critical/High/Medium findings and no prior blockers to re-surface.
Gate check: no no-auto-approve team match; gated_paths is false (so row 2 does not fire despite review_decision=REVIEW_REQUIRED); high_risk is false; zero findings. Rows 1–8 do not fire → row 9 approve. Prior decision was also approve.
There was a problem hiding this comment.
Ladon verdict: Approve
Approve — clean pass, no blocking, medium, or load-bearing low findings.
Checked: capability_block() now writes SDK-owned declarations (managed_delivery, reconciled_billing, receipt_task) directly onto the returned payload instead of routing through extra=, which advertised_reporting_delivery now fail-closes on. This is the correct shape since the service is the SDK owner. Tests tighten appropriately — extra-injection rejection is parametrized over False/True, and capability_block coverage with consumer_status_enabled=False asserts consumer_status_task and receipt_task absent.
Decision-table walk:
- Row 1 (critical/high): no such findings.
- Row 2 (gated_paths): gated_paths is false — does not fire despite review_decision=REVIEW_REQUIRED.
- Rows 3–5 (high_risk): high_risk is false.
- Row 6 (sticky escalate): prior decision was approve, not escalate.
- Row 7 (no-auto-approve team): no team match.
- Row 8 (≥3 medium): zero medium findings.
- Row 9: approve.
Prior decision was also approve; this delta continues clean.
1 participant
Current revision selection could accept damaged history or choose an older materialized snapshot, while retained writer claims did not independently prove destination content. This change adds strict whole-history selection and SDK-owned verification of frozen source rows, destination rows, manifests, objects and native observations.
B1 of B1/B2; refs #1167. Composed onto current main; no aggregate release qualification.
58c82997ef82b8b5a5996eaa070c142fa488ba87; tree300e7db36c069229037c4b51c223b46c2746f25b.81eacdb8ae17e61e7c8a71ed10540cfc6637cb2b, ordered parents[1c91311ec28d25506d5db43f59d0c34936ecb8f7, 967b6e286301d7e5d089aea6fdbb90bea8ee5a16], followed by the test/control correction and a sole-parent service-capability integration repair. Original feature history is retained.Resulting behavior
Immutable public writer/resolver/session contracts carry exact tenant, consumer, generation, binding, revision and attempt identities. Write and readback independently authorize the frozen request. Credentials remain in redacted, non-persistable sessions with bounded cancellation and exactly-once close.
The installed immutable registry recomputes every source page before destination I/O, then verifies cursor-bound rows, strict recursive JSON values, typed totals, exact manifests, every object checksum and native version/path observations. A writer locator is a claim, not verification. Unsupported tuples and incompatible retained bindings fail closed before authorization. The development reference writer remains
production_eligible=False; B1 does not advertise managed delivery, reconciled billing or delivery-ready notifications.Typed
selected/not_ready/corruptselection checks the complete history: ownership, missing/duplicate/cross-finality predecessors, disconnected/forked/cyclic snapshots and multiple officials. An official revision wins only over an intact history; no artifact-based fallback occurs. Restatements after an official close still chain to the snapshot leaf. The original reviewed corrections for closed binding failures and consumer-partition diagnostics remain.Main composition and compatibility controls
Four textual conflicts were resolved by preserving both feature and integrated-main behavior: workflow baseline pins, reporting exports, producer settling helpers, and installed-package asset/schema probes. A fifth automatic-merge interaction removed an import still used by rc.6 waiver binding; normal hooks caught it and the import was restored before the composition commit. The original failed hook receipt is retained.
The only new SQL remains
reporting_status_selector_version.sql, with its separate 10-object manifest. Main's 464-object B and 249-object C manifests are byte-identical; the selector manifest is byte-identical to the original feature. All 723 combined entries matched fresh PostgreSQL catalogs and unchanged strict validation in C, libcen_US.utf8, and ICUen-US. No fingerprint regeneration or tolerance was introduced.Drain old C projectors and sweepers before selector-v2 cutover. The committed checkpoint writer floor and transaction-local marker fence old claims; the account transition drains boundaries and reprojects current plus retained scopes. Immutable rows, checkpoint identity and unchanged canonical projections are preserved. B2 separately owns durable materialization, leases, retries and atomic finish/readiness; legacy materialization writers must be drained before B2 activation.
The rolling C control now pins integrated rc.6 main
967b6e286301d7e5d089aea6fdbb90bea8ee5a16. The removed C-only fixture helper is replaced by the existing optional-driver/URL helper, which imposes no locale restriction. A/B controls remain17ee407aand0f34c666. Pre-17eeA, pre-0f34B and pre-967b6e28C rolling compatibility are unclaimed. Earlier A/B release-note obligations remain. Originalea150fabC results are historical and are not transferred.49 individually classified operational awaits were moved immediately before their assertions across four files (including the standalone frozen-C subprocess driver). Inverse AST reconstruction, repeated after formatting, recovers every original call, argument, comparison and neighboring statement. No conditional or multi-await expression was promoted; pure reads remain. Assertions-enabled behavior is preserved; retaining operations under
-Ois intentional. Assert-based preconditions under-Oare not promised.Service-capability integration correction
Original exact-
157809d6CI36003132442failed two service tests in Python 3.13; its other three Python jobs were cancelled. The producer's new reserved-field guard was correctly rejecting SDK-owned keys passed through callerextra, while main's service still used that argument for its own computed tier declarations. Both failures reproduced on unchanged source locally (2 failed, CPython 3.12); the original CI failure/cancellations and guard seal1886e7ed…remain preserved.Child
58c82997ef82b8b5a5996eaa070c142fa488ba87changes 3 files (+21/-13), patch SHA256a2b55203248a3390bba6f05335984b6d4f6ac99a2a452e6d305a447f0676a51b. The service builds the producer capability block withoutextra, then writes its same computed managed-delivery/reconciled-billing declarations and conditional receipt task. The producer rejection guard is byte-identical. All other service AST nodes and all producer-call arguments exceptextraare unchanged; computed tier expressions and receipt-task value are unchanged. No new tier implementation or managed-delivery qualification is introduced.The service schema test now covers consumer-status enabled and disabled, including absent receipt-task advertisement. All 12 reserved-key rejection controls now exercise both
TrueandFalse, protecting against relaxing the guard merely to allow the default service. Focused validation: 51 passed / 1 intended PostgreSQL skip in 14.34s (28.25s outer), under the 240s local ceiling. Normal hooks passed; workflow, SQL, schemas and all manifests are byte-identical to the prior head. Fresh exact-child CI and independent delta review remain required.The prior head's Python 3.10 log printed passes for VCS and sdist installed controls before cancellation; those are partial observations, not successful matrix acceptance. GitGuardian skipped that head at its size limit. The separate exact-
157809d6Gitleaks report was not accepted as a substitute and supplies no exact-child scan credit; fresh coverage and its disposition remain required.CI structure and validation
The workflow expands to 19 jobs. Main's core/process matrix and required PG aggregate are identical: both execution lanes remain 15 minutes; the required old-name gate always runs and requires exact success from both
pg-conformanceandpg-reporting-status. Status remains a separate 15-minute job; its only changed step adds the exact C fetch under the existing one-minute fetch ceiling. Whole-file/glob dispatch is preserved. URL-present collection on prior head157809d6found 2057 = 1799 core + 7 process + 251 status, with disjointness and complete union verified from unmasked identities; collection executed no test bodies.The original feature's
1c91311ePython matrix correction remains explicitly part of the full PR: 60-minute job / 45-minute pytest step, compared with main's 30-minute job. It is not a new response to a failure on this composition. No PG, per-case or fetch deadline was raised.Local evidence, with source boundaries retained:
157809d6, libc PostgreSQL 16.14: 432 passed / 0 skipped in 84.81s across destination contracts/I/O/lifecycle/readers, revision selection, selector epoch, rc.6 waiver and status transaction/wire/boundary controls.Capacity limit: the initial complete worktree checkout exhausted local disk and Git removed the incomplete checkout. Existing evidence/source was not deleted. The working checkout is sparse, retaining all source plus every supported distribution input (2.5, 3.0, 3.1, rc.6) and rc.3; historical development caches remain in Git. Local complete frozen-binary rolling and installed-wheel gates were not executed for this composition. Fresh CI must establish those gates and whole-job lifecycles; local results and historical feature tests do not substitute.
Fresh exact-head CI, independent composition review, ordinary review findings, security and branch-policy disposition remain required. No #1185 result transfers. The historical catalog-per-capability cost remains until #1191; no memoization credit is claimed. The inherited fail-closed status snapshot limitation remains. No artifact, release, #1199 harness, or TypeScript acceptance is implied.