Repository navigation
ci(0.7.1g1P2): authority-transport non-vacuity floor instead of a frozen count - #477
Conversation
…zen count The mutation-authority-integration step "Assert the proof was not vacuous" hard-coded `executed = 2`. Step 3c's T7 discriminator adds a third authority-transport case to the integration class that proves the real transport (verifyMutationRatchet -> RECORDED_EVOLUTION -> nested canonicalMutationProbe -> fresh PIT -> exactComparison -> proof -> consumers), which is exactly where that evidence belongs. The step then failed on its own cardinality contract: executed=3 failures=0 errors=0 ::error::expected exactly 2 authority-transport tests, got 3 The step's name is a minimum-execution invariant, not a frozen suite cardinality. Make the code say what the name says: [ "$executed" -ge 2 ] A floor and not `= 3`, because this workflow PR must be independently testable against the epic it targets - which still has two tests until #476 merges. `= 3` could not pass against its own base, which would defeat the point of separating the workflow change. Verified on this exact base, replaying the step's own commands and logic: :build-logic:canonicalProbeIntegrationTest --tests '*MutationPopulationAdmissionIntegrationTest*' -> TASK_RC=0; exactly one report matched; executed=2 failures=0 errors=0 -> executed=2: old [ = 2 ] PASS, new [ -ge 2 ] PASS -> executed=1 and executed=0 (hypothetical): PRESSERVE FAIL under BOTH guards -> new guard on the current base: PASS (independent) The step still fails loudly when the task selects nothing, which is the entire reason it exists. verifyChangePolicy PASSED, class ci-workflow. Workflow-only, per .github/AGENTS.md rule 5.
|
Two corrections to this PR description and its commit message, recorded rather than silently edited:
|
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The single-line workflow change correctly replaces an over-fitted exact-count check with the minimum-execution floor the step name describes, holds on the current base, and introduces no concrete defects.
Review effort: Balanced
Findings: None
What changed in this PR
This PR relaxes a single assertion in the mutation-authority-integration.yml CI workflow. The "Assert the proof was not vacuous" step previously required that the MutationPopulationAdmissionIntegrationTest class run exactly 2 tests. Because the step's purpose is to guarantee the authority-transport proof actually executed (a Gradle test task that selects nothing is green), the correct invariant is a minimum-execution floor, not a frozen suite cardinality. This unblocks the incoming PR #476, which legitimately adds a third authority-transport test (the T7 discriminator), while keeping the step green on the current epic base that still has two tests.
Changes:
- Replaced the exact-count check
[ "$executed" = "2" ]with a floor check[ "$executed" -ge 2 ]and updated the error message to "expected at least 2". - Updated the adjacent comment to document that this is a non-vacuity floor rather than a fixed cardinality.
| File | Description |
|---|---|
| .github/workflows/mutation-authority-integration.yml | Changes the vacuity guard from an exact = 2 test count to a -ge 2 floor and clarifies the explanatory comment. |
I verified that MutationPopulationAdmissionIntegrationTest currently defines exactly two class-level @Test methods, so the -ge 2 floor passes on this base, and that the empty/non-numeric $executed case still fails loudly through the || branch. No references to the former "exactly 2" expectation exist elsewhere in the repo. The change is scoped to a single workflow file with no production, test, or gate-semantics changes beyond the acknowledged (and intentional) loss of deletion detection by raw cardinality.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
4960cc5
into
epic/0.7.1-control-plane-authority
Base
c4d3c84a28ea8991f667f6cfddf771994557f927— the current epic tip, which still carries two authority-transport tests.Why
PR #476 adds a third authority-transport test to
MutationPopulationAdmissionIntegrationTest: the T7 discriminator that runs the realverifyMutationRatchetunderRECORDED_EVOLUTIONwith a fresh nested PIT measurement. That is the class where such evidence belongs, so the count legitimately grew — and the anti-vacuity step inmutation-authority-integration.ymlfailed on its own contract, not on a failing test. The job's own echo:The step is named "Assert the proof was not vacuous", which is a minimum-execution invariant, not a frozen suite cardinality. This PR makes the code say what the name says.
Change (workflow only, one file)
plus the adjacent comment, which now records that this is a floor rather than a cardinality. The step name is unchanged, so no check identity churns.
Why a floor and not
= 3The workflow PR must be independently testable against the epic it targets, which still has two tests until #476 merges.
= 3would fail against its own base, defeating the point of separating the workflow change at all. Sequence: this PR green on its own → merge → rebase #476 → three tests → the same floor stays green.Verification on this exact base (independent, as required)
Local equivalent of the workflow's own commands, then the step's logic replayed verbatim:
The
executed=1andexecuted=0rows are the point: the step still fails loudly when the task selects nothing, which is the entire reason it exists.verifyChangePolicy -PchangePolicyBase=c4d3c84a…— PASSED, classci-workflowScope
Workflow-only, per
.github/AGENTS.mdrule 5. No production code, no tests, no gate semantics weakened — only an over-fitted equality replaced by the invariant the step is named for.Known limitation, accepted
A floor no longer detects a deletion of one transport case purely from cardinality. An exact count was a poor proxy for identity anyway —
= 3proves three tests ran, not that T7 ran. If T7 ever needs independent CI pinning, the right form is asserting a named testcase is present in the XML, not freezing the class total. Not doing that now; T1–T19 coverage is tracked as its own ledger.