authority(0.7.1g1P2-Step3b): base-revision certificate read + M34 accepts certified migration + M44/M47 on the same facts - #475
Conversation
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
The change modifies the mutation-ratchet authority-loading machinery that gates quality enforcement, a high-stakes governance subsystem the repository guidelines reserve for explicit human authorization.
Review effort: Balanced
Findings: None
What changed in this PR
This PR extends the mutation-ratchet base-authority loader so that the digest-migration certificate ledger is read at the base revision, rather than only from the working tree. This closes the gap where M43/M47 are judged against the certificate ledger "as it exists in the base," but the base-side read never produced certificate facts for the verifier to consult. The change is confined to build-logic quality-governance machinery and is an incremental step (3b): it wires transport now, with certificate-aware M34 consumption deferred to a follow-up.
The new certificates field is loaded through the same git show "<baseSha>:<path>"-into-a-temp-tree path already used for population, classifications, enrollments, and admissions, so a single MutationRatchetAuthority snapshot carries the whole coherent base context. Absence at base is treated like the other optional ledgers (fail-closed to NONE), while a present-but-malformed ledger still fails hard through the ledger's own loader.
Changes:
- Add an optional
certificatesfield (MutationAuthorityDigestCertificates, defaulting toNONE) toMutationRatchetAuthority. - Read the certificate ledger at the base SHA in
MutationRatchetAuthorityLoader.load, mirroring the enrollments/admissions pattern. - Add
CertificateBaseRevisionTransportTestwith two discriminators against real history: the P1M merge exposes exactly one certificate whosefromDigestmatches the base admissions'populationDigest, and a pre-ledger base exposes no certificates while still carrying admissions.
| File | Description |
|---|---|
build-logic/src/main/kotlin/dev/tramai/build/quality/MutationRatchetAuthority.kt |
Adds the optional certificates field and the base-revision read path that loads it into the hermetic temp tree before invoking the certificate loader. |
build-logic/src/test/kotlin/dev/tramai/build/quality/CertificateBaseRevisionTransportTest.kt |
New transport test proving the loader exposes certificates at the minting revision and none at a pre-ledger base, both against real immutable commits. |
I verified the referenced symbols (MutationAuthorityDigestCertificateLoader.FILE_NAME/.load, MutationAuthorityDigestCertificates.NONE, MutationPopulationAdmission.populationDigest) exist and match usage, that the temp-tree path resolves to the same file the loader reads, that the new default keeps the existing MaintainabilityBaselinePlugin call site compiling, and that CI checks out full history (fetch-depth: 0) so the hardcoded historical SHAs are reliably available. I found no issues that warrant a blocking comment.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Merge hold after review. #474 has now landed as One authority seam also needs closing before merge: the PR currently recomputes I found three production recomputation sites:
So the current claim 'M34 and M44/M47 consume the same facts' is value-equivalent but not structurally true: each path gets newly constructed Minimal correction:
Yes: add the seam test. It should pin 'produce once, consume everywhere' rather than merely value equality. A source/wiring assertion that there is one production call and the same local is threaded to both admission and certificate lifecycle paths is acceptable if a runtime Everything else I reviewed in #475 is directionally correct. This is the only authority-model blocker I found. |
…revision M43/M47 are judged against the certificate ledger as it exists in the BASE, but the only loader read the working tree. So the base side had no way to see a certificate at all, and M34 had no facts it could ever consult. This closes that gap. The base-side read now goes through the same path the population, classifications, enrollments and admissions already use: git show "<baseSha>:<path>" into the temp tree, then the ledger's own loader. One authority snapshot (MutationRatchetAuthority) therefore carries the whole base context - population, classifications, enrollments, admissions and now certificates - which is what one coherent transition context means. Absent at base is handled like the other optional ledgers: a base that predates the certificate ledger has none, which is the most restrictive state, since no certified migration exists and every raw-v1 admission still fails M34. A present ledger is validated by its own loader, so a malformed one fails hard rather than degrading into "no certificates". Two discriminators, both against real immutable history rather than fixtures: - at 9ebe7b4 (the P1M merge that minted the certificate) the authority snapshot exposes exactly one certificate, and its fromDigest is the single populationDigest the base's own admissions are bound to; - at 64d0545 (the base P1M was proposed against, which predates the ledger) the snapshot exposes no certificates, while the same snapshot's admissions are non-empty - so the test isolates the certificate ledger instead of showing that some empty snapshot is empty. The default on MutationRatchetAuthority.certificates is the conservative NONE for fixtures; the loader always passes it explicitly from the base revision. Verification: 40 tests / 0 failures (base-revision transport 2, ratchet authority, population admission suites); spotless clean; Detekt baseline 4792 -> 4792 with 0 added; verifyChangePolicy PASSED, 2 changed files, class build-logic. Next: M34 consults these facts at the verifier's decision points, then the T1-T19 matrix.
Completes the certified-migration path: the base snapshot now carries the certificate ledger, and M34 consumes the facts it proves. - MutationRatchetAuthority gains the base-revision certificate read, using the same git-show-into- temp-tree path as the population, classifications, enrollments and admissions. Absent at base is the most restrictive state, as for the other optional ledgers. - appearanceVerdict's M34 branch rejects a raw-v1 admission only when no certified consumption covers that admission's own historical digest. The facts come from one production site, certifiedConsumptions(...), which runs the real verification for each distinct historical digest against the base ledger and the exact base authorization set. A Valid exists only if M43-M42 passed, so the branch cannot be reached by asserting anything. - AdmissionAuthority carries the fresh projection plus the consumptions it certifies as one value. That is what makes the two judgment paths - the lifecycle scan and the appearing-identity path - receive the same object. Feeding one without the other would have made P2's diagnostics contradict themselves for the same 67 rows: admitted in the scan, M34-rejected in the appearing path. It also keeps both signatures inside the parameter budget without suppressing anything. Without a certificate the M34 condition is exactly as before, and AdmissionAuthority(null) is the fail-closed default everywhere. M36/M37 are untouched. Verification: 158 tests / 0 failures, including the new discriminators - certifiedConsumptions yields one fact from a real base certificate and none for an uncertified projection or no certificate; M34 accepts the certified raw-v1 admission and still fails the uncertified one. Plus the base-revision cases: the certificate is present at 9ebe7b4 (the P1M merge) and absent at 64d0545 (P1M's base) while that base's admissions remain non-empty. spotless clean; Detekt baseline 4792 -> 4792 with 0 added; verifyChangePolicy PASSED, 6 changed files, class build-logic. Four genuine Detekt findings fixed by hand - the two LongParameterList ones by introducing the carrier, not by suppressing them. Next: M44/M47 consume the same fact set, and the T1-T19 matrix at both levels.
Last row of the Step 3b path: the certificate lifecycle is now driven by the facts M34 already uses, produced at one site. - MutationRatchetCandidate gains the transition's own certificate ledger, loaded from the repository like the candidate admissions. It is validated (M45 mint binding, M46 retained immutability) but can never authorize a consumption: only the base ledger is consulted, so a candidate that both mints and consumes fails M43. - The verifier now calls MutationAuthorityDigestCertificateCeremony.checks with the base and candidate certificate ledgers and validConsumptions taken from base.admissionAuthority(...).certifiedConsumptions - the same object M34 decides on, so M44 (single use) and M47 (removal custody) cannot disagree with the admission verdicts about what was consumed. - M47's consuming half is now exercised end to end: a certificate may disappear only because a consumption was independently proven in the same transition, never because it was absent. Two existing test fixtures were corrected rather than the rules relaxed: - MutationRatchetAuthorityTest's identity-transition candidate now retains the base's certificate, exactly as it retains the base's admissions. Passing NONE asserted a transition that removes the certificate with no proven consumption, which M47 rightly refuses. This is the same class of fixture defect (and the same fix) as the admissions case in Step 1. - CertificateCustodyTransportTest drives the real committed ledger through the real ceremony: unchanged, it survives; dropped without a proven consumption, M47 fails; dropped with the facts produced from the real ledgers, removal is permitted. M47 firing on that fixture is the evidence that the consuming half is genuinely live, rather than a branch nobody reaches. Verification: 162 tests / 0 failures across the certificate, admission and ratchet suites; spotless clean; Detekt baseline 4792 -> 4792 with 0 added; verifyChangePolicy PASSED, class build-logic. A full :build-logic:test run reached 855 tests with zero genuine test failures before the console call was truncated (its only entry was an infrastructure pseudo-failure from the kill itself); CI runs the complete suite. Remaining: the T1-T19 matrix at verifier and real-task level.
… the real ledger Audited the discriminator matrix against the rules that actually have tests: every rule M01-M47 is exercised somewhere, but three certificate-lifecycle attacks had only rule-level coverage, not the real-task transport coverage the design requires. All three now run against the committed certificate ledger loaded through the real loader. - T18: a retained certificate whose enforced payload was rewritten (here its reason, which is in the payload) fails M46. The rewrite is derived from the real certificate's own fields, so the test cannot pass by accident of a fixture mismatch. - T17: a certificate introduced with a fromBaseSha other than the transition's base fails M45, even alongside the legitimate retained certificate. - T14: a certificate absent from the real base ledger cannot be consumed, however it is cited - the verifier refuses it as M43 rather than accepting the citation. Together with the earlier cases this gives the certificate lifecycle transport coverage for introduction (M45), retention (M46), consumption (M40-M43), single use (M44) and removal custody (M47), all against the real committed ledger. Verification: 165 tests / 0 failures; spotless clean; Detekt baseline 4792 -> 4792 with 0 added; verifyChangePolicy PASSED, 9 changed files, class build-logic.
…the instance Review finding: there was one fact-production function but three fact-production events. MutationRatchetVerifier recomputed base.admissionAuthority(...) for the outcome-ratchet context and again for the certificate ceremony, and MutationPopulationAdmissionCeremony recomputed it once more inside its own consumption loop for the appearing-identity verdicts. The three results were value-equal, which is not the property the contract asks for: deterministic recomputation is not shared evidence, and value equality lets a later change drift one consumer away from the others without any test noticing. Now: val admissionAuthority = base.admissionAuthority(freshAuthorityProjectionHash) // once and that exact instance is threaded to every consumer - the evolution context (M34 and the appearing-identity path), the admission lifecycle ceremony, and the certificate ceremony (M44/M47) via .certifiedConsumptions. - MutationPopulationAdmissionCeremony.checks/lifecycleChecks/consumptionChecks now take the AdmissionAuthority instead of the raw projection hash and pass it through unchanged; the internal recomputation at the consumption loop is gone. Public signature change: checks() has exactly one caller, the verifier. - The AdmissionAuthority KDoc states the invariant: produced once, never re-derived. - CertificateCustodyTransportTest pins it with a source-level wiring assertion, the form the review sanctioned where a runtime === check would require exposing internals: exactly one admissionAuthority( production call in the verifier, zero in the ceremony, and the certificate ceremony consuming the shared local. Rebased onto the post-#474 epic tip a2c6c3a; diff scope verified as exactly 9 files, and #474's merged CertificateLedgerTransportTest.kt is present at the tip and absent from this diff. Verification: 172 tests / 0 failures (certificate, admission and ratchet suites); spotless clean; Detekt baseline 4792 -> 4792 with 0 added; verifyChangePolicy PASSED against base a2c6c3a, class build-logic.
99a6b79 to
7c24f27
Compare
c4d3c84
into
epic/0.7.1-control-plane-authority
Base
d192e2ea30b0dcc2b618cbc99a030920ca85833f— the post-#473 epic tip (branch parent verified).Step 3b, complete end to end: base-revision certificate read → one fact-production site → M34 accepts certified digest migration → M44/M47 consume the same facts.
Scope (9 files, +373/−7)
1. Base-revision read of the certificate ledger (
MutationRatchetAuthority)M43/M47 are judged against the certificate ledger as it exists in the base, but the only loader read the working tree. The base-side read now uses the same
git show "<baseSha>:<path>"-into-the-temp-tree path as the population, classifications, enrollments and admissions, so one authority snapshot carries the whole base context. Absent at base = no certificates = the most restrictive state. A present ledger is validated by its own loader, so malformed fails hard.2. M34 accepts certified digest migration
The
:135branch rejects a raw-v1 admission only when no certified consumption covers that admission's own historical digest. Without a certificate the condition is byte-for-byte as before.certifiedConsumptions(...)is the single production site, running the real verification per distinct historical digest against the base ledger and the exact base authorization set. AValidexists only if M43-M42 passed, so the branch cannot be reached by asserting anything.AdmissionAuthority(freshAuthorityProjectionHash, certifiedConsumptions)carries it as one value — which is what makes the lifecycle scan and the appearing-identity path receive the same object instead of contradicting each other, and what keeps both signatures inside the parameter budget without a suppression.3. M44/M47 consume the same facts
MutationRatchetCandidategains the transition's own certificate ledger, loaded from the repository. It is validated (M45 mint binding, M46 retained immutability) but can never authorize a consumption — only the base ledger is consulted, so a candidate that both mints and consumes fails M43.MutationAuthorityDigestCertificateCeremony.checkswithvalidConsumptionsfrombase.admissionAuthority(...), so M44 (single use) and M47 (removal custody) cannot disagree with the admission verdicts about what was consumed.Two fixtures corrected, no rule relaxed
MutationRatchetAuthorityTest's identity-transition candidate now retains the base's certificate, exactly as it retains the base's admissions — passingNONEasserted a transition that removes the certificate with no proven consumption, which M47 rightly refuses. Same class of fixture defect, and same fix, as the admissions case in Step 1. M47 firing on that fixture is the evidence the consuming half is genuinely live rather than a branch nobody reaches.Invariants preserved
AdmissionAuthority(null)is the fail-closed default everywhere.checks(..., validConsumptions)still defaults to empty.4792 → 4792, 0 added. Every finding fixed by hand — including fourLongParameterList/MaxLineLengthones.Verification
--rerun-tasks— 162 tests, 0 failures:build-logic:test— 855 tests, zero genuine test failures before the console call was truncated (its lone entry was an infrastructure pseudo-failure from the kill); CI runs the complete suite, and a background run is completing it locallyspotlessKotlinCheckratchet-scoped — BUILD SUCCESSFULverifyStaticAnalysis— BUILD SUCCESSFUL,4792 → 4792, 0 removed, 0 addedverifyChangePolicy -PchangePolicyBase=d192e2ea…— PASSED, classbuild-logicDiscrimination
certifiedConsumptions: one fact from a real base certificate; none for an uncertified projection (M40); none with no certificate.9ebe7b44(the P1M merge), absent at64d05450(P1M's base) while that base's admissions stay non-empty.Remaining
The T1-T19 matrix at verifier and real-task level. No production rule is left unwired: the certificate lifecycle now runs inside the real ratchet.