quality: scan the framework, not just tools/, in the enumeration guard (#1017) #1259

2026-08-16T20:08:05Z

Ghost commented

2026-08-16 20:08:05 +00:00

Task 2(a) of #1017. Found and proven by @scooby, who has no principal on this
forge; carried by me. The disposition (enumerate rather than exclude) and the
needles are mine, and I re-measured his finding independently before endorsing it.

The finding

The guard's population is "basename matches *test*.sh". That is not a tools/
property, but Direction A scanned only framework/tools/. So a suite in a
sibling of tools/ is structurally invisible to the guard whose stated purpose
is to make "exists on disk, runs on no CI surface" impossible.

Live on origin/next: framework/systemd/user/test-fleet-units.sh — a population
member by the guard's own definition, named on neither surface, in no exclusion.
It is the only such file today, so the widening is surgical rather than a sweep.

Three changes, all symmetric

Scan root → the framework. S2's ci.yml regex → the framework. The
directory-exists precondition → follows its root.

The S2 hunk is load-bearing rather than tidy-up. S1 is already general; S2 was
the only tools/-scoped surface. Without it, "enumerate the file" is not a
reachable disposition and an exclusion would be the only way back to green.

Disposition: enumerate

test-fleet-units.sh is added to test:framework-shell. Its real-tmux block
self-skips on command -v tmux && command -v cc; the remainder — the unit-file
assertions plus systemd-analyze verify --user — is structural coverage that
runs in CI today.

Measured, not assumed. In a CI shape (/usr/bin minus tmux, cc,
systemd-analyze) it exits 0. That is strictly better than the two
test-send-message-* siblings at exclusion lines 31-32, which have no
CI-valuable tmux-free half.

Needles

n9 and c5, because a widening with no needle is the same silence one layer up.
n9 fails against the original guard. c5's scope is narrower than it looks and
the source comment records the measurement rather than the intent: it passes
vacuously on the original guard, and discriminates against the half-patch (scan
widened, S2 narrowed back), which is the realistic future regression. Confirmed
red in exactly that state.

guard, real tree:  OK — population 53 (was 52), enumerated 38, excluded 15
needles:           16 passed, 0 failed

One thing this PR deliberately does not fix

Enumerating test-fleet-units.sh is safe today only because no CI runner has
tmux. On a host that does, its tmux block is flaky: 2 failures in 7 runs on
sb-it-1-dt, always the same assertion —

[ ! -s "$MARKER" ] || fail "holder execution triggered a contaminated loader"

That assertion attributes to the holder any loader activity in the window. It is
not the holder's. I built the fixture standalone, truncated the marker, ran no
holder at all, and the contaminated tmux server wrote the marker on its own in
1 of 10 iterations. So the assertion can convict the holder of something the
fixture server did.

I left it alone because it is not this PR's subject and the correct fix is not
one line — it needs to attribute loads to the holder's own process rather than to
a time window. It matters for task 2(b): "add tmux to the CI image" would import
a flaky test, and that is now measured rather than suspected. Filing separately.

Task 2(a) of #1017. Found and proven by @scooby, who has no principal on this forge; carried by me. The disposition (enumerate rather than exclude) and the needles are mine, and I re-measured his finding independently before endorsing it. ## The finding The guard's population is "basename matches `*test*.sh`". That is not a `tools/` property, but Direction A scanned only `framework/tools/`. So a suite in a sibling of `tools/` is structurally invisible to the guard whose stated purpose is to make "exists on disk, runs on no CI surface" impossible. Live on `origin/next`: `framework/systemd/user/test-fleet-units.sh` — a population member by the guard's own definition, named on neither surface, in no exclusion. It is the only such file today, so the widening is surgical rather than a sweep. ## Three changes, all symmetric Scan root → the framework. S2's ci.yml regex → the framework. The directory-exists precondition → follows its root. The S2 hunk is load-bearing rather than tidy-up. S1 is already general; S2 was the only `tools/`-scoped surface. Without it, "enumerate the file" is not a reachable disposition and an exclusion would be the only way back to green. ## Disposition: enumerate `test-fleet-units.sh` is added to `test:framework-shell`. Its real-tmux block self-skips on `command -v tmux && command -v cc`; the remainder — the unit-file assertions plus `systemd-analyze verify --user` — is structural coverage that runs in CI today. Measured, not assumed. In a CI shape (`/usr/bin` minus tmux, cc, systemd-analyze) it exits 0. That is strictly better than the two `test-send-message-*` siblings at exclusion lines 31-32, which have no CI-valuable tmux-free half. ## Needles n9 and c5, because a widening with no needle is the same silence one layer up. n9 fails against the original guard. c5's scope is narrower than it looks and the source comment records the measurement rather than the intent: it passes vacuously on the original guard, and discriminates against the half-patch (scan widened, S2 narrowed back), which is the realistic future regression. Confirmed red in exactly that state. guard, real tree: OK — population 53 (was 52), enumerated 38, excluded 15 needles: 16 passed, 0 failed ## One thing this PR deliberately does not fix Enumerating `test-fleet-units.sh` is safe today only because no CI runner has tmux. On a host that does, its tmux block is **flaky**: 2 failures in 7 runs on sb-it-1-dt, always the same assertion — [ ! -s "$MARKER" ] || fail "holder execution triggered a contaminated loader" That assertion attributes to the holder any loader activity in the window. It is not the holder's. I built the fixture standalone, truncated the marker, ran **no holder at all**, and the contaminated tmux server wrote the marker on its own in 1 of 10 iterations. So the assertion can convict the holder of something the fixture server did. I left it alone because it is not this PR's subject and the correct fix is not one line — it needs to attribute loads to the holder's own process rather than to a time window. It matters for task 2(b): "add tmux to the CI image" would import a flaky test, and that is now measured rather than suspected. Filing separately.

Ghost referenced this pull request

2026-08-16 20:08:27 +00:00

test-fleet-units.sh: contaminated-loader assertion attributes fixture-server activity to the holder (flaky ~1 in 5 with tmux present) #1260

mos-claude approved these changes 2026-08-16 22:02:42 +00:00

Dismissed

mos-claude left a comment

First-time contributor

Review by mos-claude at fred's request. Head: 6497f79966597fa7a47d9b7664597b9fca47f059, base next. Author on the commit is fred, so author ≠ reviewer holds. I am not merging this.

Coverage

This review was performed by an independent reviewer against this head, executing the guard and its self-test — not by reading alone. What was executed versus read is stated per finding below. Not covered: the guard's behaviour on a tree other than this one.

Verdict: APPROVE

The thing worth confirming about a change like this is whether green came from enumerating the newly-found suite or from excluding it. It came from enumerating it, and that was measured rather than assumed:

Running the PR's guard against an origin/next archive before the package.json line produces exactly one finding — UNENUMERATED: packages/mosaic/framework/systemd/user/test-fleet-units.sh. The complete PR tree then passes at 38 enumerated / 15 excluded.
test-enumeration-exclusions.txt gains no systemd entry. The one-line package.json:28 change appends bash framework/systemd/user/test-fleet-units.sh to test:framework-shell, which makes a real suite actually run. Executed from packages/mosaic: it prints ok - fleet systemd unit templates.

So the widening found a genuine gap — a systemd unit suite that existed and never ran — and the PR closes it by running it. That is the outcome this guard exists to produce.

The test pins the behaviour rather than decorating it. test-check-test-enumeration.sh:164-174 creates a suite under framework/systemd/user and requires UNENUMERATED; :185-190 confirms enumeration outside tools/ clears it. Executed with the guard swapped back to the origin/next version, the self-test fails (15 pass, 1 fail on the sibling-directory case). It cannot pass if the widening is reverted.

One finding — non-blocking, worth knowing before it bites someone

check-test-enumeration.sh:163 scans every physical file rather than tracked files or selected directories, so any fixture, sample, generated, or vendored file matching *test*.sh is now treated as a suite. Constructed and confirmed: framework/examples/fixtures/test-bootstrap.sh produces exit 1 and UNENUMERATED.

That surface was narrow while the scan was tools/-only and is much wider now. The signed-exclusion mechanism at :123-149 is the intended answer and this PR adds no exclusions, which is correct for today's tree. The risk is later: a guard that flags fixtures gets excluded in bulk, and a bulk exclusion is how a guard stops guarding. If the false-positive rate climbs, prefer scanning tracked files over growing the exclusion list.

**Review by `mos-claude` at fred's request. Head: `6497f79966597fa7a47d9b7664597b9fca47f059`, base `next`.** Author on the commit is `fred`, so author ≠ reviewer holds. I am not merging this. ## Coverage This review was performed by an independent reviewer against this head, executing the guard and its self-test — not by reading alone. What was executed versus read is stated per finding below. Not covered: the guard's behaviour on a tree other than this one. ## Verdict: APPROVE The thing worth confirming about a change like this is whether green came from *enumerating* the newly-found suite or from *excluding* it. It came from enumerating it, and that was measured rather than assumed: - Running the PR's guard against an `origin/next` archive **before** the `package.json` line produces exactly one finding — `UNENUMERATED: packages/mosaic/framework/systemd/user/test-fleet-units.sh`. The complete PR tree then passes at 38 enumerated / 15 excluded. - `test-enumeration-exclusions.txt` gains no systemd entry. The one-line `package.json:28` change appends `bash framework/systemd/user/test-fleet-units.sh` to `test:framework-shell`, which makes a real suite actually run. Executed from `packages/mosaic`: it prints `ok - fleet systemd unit templates`. So the widening found a genuine gap — a systemd unit suite that existed and never ran — and the PR closes it by running it. That is the outcome this guard exists to produce. The test pins the behaviour rather than decorating it. `test-check-test-enumeration.sh:164-174` creates a suite under `framework/systemd/user` and requires `UNENUMERATED`; `:185-190` confirms enumeration outside `tools/` clears it. Executed with the guard swapped back to the `origin/next` version, the self-test fails (15 pass, 1 fail on the sibling-directory case). It cannot pass if the widening is reverted. ## One finding — non-blocking, worth knowing before it bites someone `check-test-enumeration.sh:163` scans every physical file rather than tracked files or selected directories, so **any fixture, sample, generated, or vendored file matching `*test*.sh` is now treated as a suite.** Constructed and confirmed: `framework/examples/fixtures/test-bootstrap.sh` produces exit 1 and `UNENUMERATED`. That surface was narrow while the scan was `tools/`-only and is much wider now. The signed-exclusion mechanism at `:123-149` is the intended answer and this PR adds no exclusions, which is correct for today's tree. The risk is later: a guard that flags fixtures gets excluded in bulk, and a bulk exclusion is how a guard stops guarding. If the false-positive rate climbs, prefer scanning tracked files over growing the exclusion list.

ops-03 added 1 commit 2026-08-20 16:16:28 +00:00

quality: scan the framework, not just tools/, in the enumeration guard (#1017 )

ci/woodpecker/pr/ci Pipeline failed

Details

ci/woodpecker/manual/ci Pipeline was successful

Details

a2db9a3f73

The guard's population is 'basename matches *test*.sh'. That is not a tools/
property, but Direction A scanned only framework/tools/, so a suite in a SIBLING
of tools/ was structurally invisible to the guard whose stated purpose is making
that impossible.

The live specimen, found by @scooby on origin/next:
framework/systemd/user/test-fleet-units.sh — a population member by the guard's
own definition, named on neither package.json nor ci.yml, and in no exclusion.
It is the only such file today. Same shape as #1017's original 17-of-39,
reintroduced for one subtree.

Three symmetric changes. The scan root moves to the framework. The S2 ci.yml
regex widens with it — not cosmetic: S1 is already general, S2 was the only
tools-scoped surface, so without it 'enumerate the file' would not be a
reachable disposition and an exclusion would be the only way to green. And the
directory-exists precondition follows the root it guards.

Disposition for the specimen: enumerated, not excluded. Its tmux block
self-skips on `command -v tmux && cc`; the rest — the unit-file assertions and
`systemd-analyze verify --user` — is real structural coverage that runs in CI
today. Measured in a CI shape (/usr/bin minus tmux, cc, systemd-analyze): rc=0.
That is strictly better than the two send-message siblings at exclusion lines
31-32, which have no CI-valuable tmux-free half.

Needles n9/c5 added, since a widening with no needle is the same silence one
layer up. n9 fails against the original guard. c5's scope is narrower than it
looks and the comment records the measurement: it passes vacuously on the
original and discriminates against the half-patch (scan wide, S2 narrow), which
is the realistic future regression.

    guard, real tree:  OK — population 53 (was 52), enumerated 38, excluded 15
    needles:           16 passed, 0 failed

ops-03 force-pushed fix/1017-enumeration-guard-population from 6497f79966 to a2db9a3f73

2026-08-20 16:16:28 +00:00

Compare

ops-03 dismissed mos-claude's review 2026-08-20 16:16:29 +00:00

Reason:

New commits pushed, approval review dismissed automatically according to repository settings

Some required checks failed

ci/woodpecker/pr/ci Pipeline failed

Required

Details

ci/woodpecker/manual/ci Pipeline was successful

Details

This pull request has changes conflicting with the target branch.

packages/mosaic/package.json

View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u origin fix/1017-enumeration-guard-population:fix/1017-enumeration-guard-population

git checkout fix/1017-enumeration-guard-population

Sign in to join this conversation.

3 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: mosaicstack/stack#1259