fix(changelog-monotonic): report containment vacuous when the base IS HEAD
Dropping the pull_request gate made merge_base == HEAD a routine path rather than a degradation, and the success line did not follow. On every push to main the step printed "all N release heading(s) at the merge base are still present" — a containment claim on the one event where deletion is undetectable, since the comparison is the file against itself. That is the dishonesty this PR fixed in the skip messages, surviving in the success message. The line now has two forms: containment vacuous, naming uniqueness as the half that ran, or the existing containment wording when a real base exists. Both pinned, including that they do not collapse. Also scopes the ci.yml negative pin to the monotonic step's own block. As a file-wide assertion it forbade any FUTURE step in ci.yml from being pull_request-gated and would have failed citing #133 when one legitimately was; a companion assert keeps the extractor from silently matching nothing and turning the negative into a tautology. Ported from heavy-duty/box#144, where the defect was found after this PR's approvals had landed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
parent
0bd531042e
commit
84c592e961
3 changed files with 78 additions and 3 deletions
15
.github/scripts/changelog-monotonic.sh
vendored
15
.github/scripts/changelog-monotonic.sh
vendored
|
|
@ -234,4 +234,17 @@ EOF
|
||||||
fi
|
fi
|
||||||
|
|
||||||
count="$(printf '%s\n' "$base_headings" | grep -c . || true)"
|
count="$(printf '%s\n' "$base_headings" | grep -c . || true)"
|
||||||
echo "changelog-monotonic: all $count release heading(s) at the merge base ($(git rev-parse --short "$merge_base")) are still present in $changelog"
|
head_count="$(printf '%s\n' "$head_headings" | grep -c . || true)"
|
||||||
|
|
||||||
|
# The success line has two honest forms, because this step now runs on two
|
||||||
|
# shapes of event. On a push to main the merge base IS HEAD: containment
|
||||||
|
# compared the file against itself and asserted nothing, and deletion is
|
||||||
|
# undetectable on that event by construction. Reporting "all N still present"
|
||||||
|
# there would be the same dishonesty the skip messages were fixed for (#133) —
|
||||||
|
# a log claiming a check that did no work. Uniqueness is the half that actually
|
||||||
|
# ran, so that is the half the line names.
|
||||||
|
if [ "$merge_base" = "$(git rev-parse HEAD)" ]; then
|
||||||
|
echo "changelog-monotonic: containment vacuous (the merge base IS HEAD, so nothing could have been deleted between them) — uniqueness on HEAD checked $head_count release heading(s)."
|
||||||
|
else
|
||||||
|
echo "changelog-monotonic: all $count release heading(s) at the merge base ($(git rev-parse --short "$merge_base")) are still present in $changelog"
|
||||||
|
fi
|
||||||
|
|
|
||||||
|
|
@ -82,7 +82,14 @@ actually cutting it, and this file starts there.
|
||||||
Fixed by moving, not rewriting: uniqueness now runs directly after the file
|
Fixed by moving, not rewriting: uniqueness now runs directly after the file
|
||||||
exists, before any git access. The skip messages say *containment* skipped
|
exists, before any git access. The skip messages say *containment* skipped
|
||||||
and that uniqueness already passed, so a skip no longer claims nothing was
|
and that uniqueness already passed, so a skip no longer claims nothing was
|
||||||
checked. The guard is also no longer gated to `pull_request` — deletion is
|
checked — and the success line got the same treatment, because dropping the
|
||||||
|
gate made `merge_base == HEAD` a routine path rather than a degradation. On a
|
||||||
|
push to main containment compares the file against itself and asserts
|
||||||
|
nothing, so the line now reports containment *vacuous* and names uniqueness
|
||||||
|
as the half that ran, instead of claiming N headings were verified present by
|
||||||
|
a comparison that could not have detected their absence.
|
||||||
|
|
||||||
|
The guard is also no longer gated to `pull_request` — deletion is
|
||||||
vacuous on a push to main, but duplication is vacuous on no tree, so a
|
vacuous on a push to main, but duplication is vacuous on no tree, so a
|
||||||
duplicate reaching main by any other route went unasserted. That gate could
|
duplicate reaching main by any other route went unasserted. That gate could
|
||||||
not simply be dropped: `github.base_ref` is empty on a push, and a bare
|
not simply be dropped: `github.base_ref` is empty on a push, and a bare
|
||||||
|
|
|
||||||
|
|
@ -435,9 +435,53 @@ describe("changelog-monotonic.sh — release headings are append-only (#133)", (
|
||||||
) => run("bash", [MONOTONIC, base], env, repo);
|
) => run("bash", [MONOTONIC, base], env, repo);
|
||||||
|
|
||||||
it("a branch that touches nothing passes, and says how many headings it checked", async () => {
|
it("a branch that touches nothing passes, and says how many headings it checked", async () => {
|
||||||
|
// A branch that touches nothing has HEAD as its own merge base, which is
|
||||||
|
// now the VACUOUS-containment path (#133), so the count this asserts moved
|
||||||
|
// to uniqueness's — which serves the stated intent better anyway: it says
|
||||||
|
// the parser read the file and found real headings in it, rather than that
|
||||||
|
// a comparison of the file against itself came out equal.
|
||||||
const r = await check(repoWith());
|
const r = await check(repoWith());
|
||||||
expect(r.code).toBe(0);
|
expect(r.code).toBe(0);
|
||||||
|
expect(r.output).toContain(
|
||||||
|
"uniqueness on HEAD checked 2 release heading(s)",
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
// --- the push-to-main shape: containment vacuous, uniqueness real --------
|
||||||
|
// With the pull_request gate gone (#133), merge_base == HEAD is a ROUTINE
|
||||||
|
// path, not a degradation. Containment compares the file against itself and
|
||||||
|
// asserts nothing, so a line reading "all N still present" would claim a
|
||||||
|
// check that did no work — the same dishonesty the skip messages were fixed
|
||||||
|
// for. The success line therefore has two forms, and these pin which one
|
||||||
|
// each event shape gets, including that they do not collapse into one.
|
||||||
|
|
||||||
|
it("HEAD as its own base reports containment VACUOUS, not verified", async () => {
|
||||||
|
const r = await check(repoWith(), {}, "HEAD");
|
||||||
|
expect(r.code).toBe(0);
|
||||||
|
expect(r.output).toContain("containment vacuous");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("...and names uniqueness as the half that actually ran", async () => {
|
||||||
|
const r = await check(repoWith(), {}, "HEAD");
|
||||||
|
expect(r.output).toContain("uniqueness on HEAD checked");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("...and does NOT claim the headings were still present", async () => {
|
||||||
|
const r = await check(repoWith(), {}, "HEAD");
|
||||||
|
expect(r.output).not.toContain("are still present");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("...while a REAL base still reports containment, naming the count", async () => {
|
||||||
|
// The PR shape. The two wordings must not collapse into one.
|
||||||
|
const good = BASE.replace(
|
||||||
|
"## Unreleased\n",
|
||||||
|
"## Unreleased\n\n### Fixed\n\n- **A new entry**\n",
|
||||||
|
);
|
||||||
|
const r = await check(repoWith(good));
|
||||||
|
expect(r.code).toBe(0);
|
||||||
expect(r.output).toContain("all 2 release heading(s)");
|
expect(r.output).toContain("all 2 release heading(s)");
|
||||||
|
expect(r.output).toContain("are still present");
|
||||||
|
expect(r.output).not.toContain("containment vacuous");
|
||||||
});
|
});
|
||||||
|
|
||||||
it("adding an entry the CORRECT way — above the heading, never over it — passes", async () => {
|
it("adding an entry the CORRECT way — above the heading, never over it — passes", async () => {
|
||||||
|
|
@ -645,7 +689,18 @@ describe("changelog-monotonic.sh — release headings are append-only (#133)", (
|
||||||
// #133: NOT pull-request-only. Deletion is vacuous on a push to main, but
|
// #133: NOT pull-request-only. Deletion is vacuous on a push to main, but
|
||||||
// duplication is vacuous on no tree — gating the whole script left a
|
// duplication is vacuous on no tree — gating the whole script left a
|
||||||
// duplicate that reached main by any other route unasserted forever.
|
// duplicate that reached main by any other route unasserted forever.
|
||||||
expect(CI).not.toContain("if: github.event_name == 'pull_request'");
|
//
|
||||||
|
// Scoped to the step's OWN block, deliberately. As a file-wide negative it
|
||||||
|
// would forbid any FUTURE step in ci.yml from being pull_request-gated and
|
||||||
|
// would fail citing #133 when one legitimately is — #133 constrains this
|
||||||
|
// step, not the file. The companion assert below keeps the extractor from
|
||||||
|
// silently matching nothing and turning the negative into a tautology.
|
||||||
|
const monoBlock = CI.split(/^ {6}- name: /m).find((b) =>
|
||||||
|
b.startsWith("no shipped changelog heading"),
|
||||||
|
);
|
||||||
|
expect(monoBlock).toBeDefined();
|
||||||
|
expect(monoBlock).toContain("changelog-monotonic.sh");
|
||||||
|
expect(monoBlock).not.toContain("if:");
|
||||||
// ...and dropping that gate is only safe WITH the fallback: on a push
|
// ...and dropping that gate is only safe WITH the fallback: on a push
|
||||||
// `github.base_ref` is empty, a bare `origin/` does not resolve, and
|
// `github.base_ref` is empty, a bare `origin/` does not resolve, and
|
||||||
// STRICT promotes that to a hard failure on every push to main.
|
// STRICT promotes that to a hard failure on every push to main.
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue