gate: arm the skip guard from source, and raise the floor to its population

[iceteaSA]

Two ways this gate could pass while not checking what it claims. Independent of #20 — that one is the wire surface, this is the harness.

The skip guard depended on a flag nothing enforced

run_expect fails an arm that prints a skip notice. But cargo captures test output unless --nocapture is passed, so the check only works when the caller remembers the flag. Without it the guard reads an empty stream, finds no notice, and reports the arm clean — indistinguishable from a run where nothing skipped.

The existing comment said exactly this and stopped there:

# and cargo CAPTURES test output unless --nocapture is passed, so callers that can
# skip must pass it or this check reads an empty stream and finds nothing.

A documented requirement with no enforcement. And --nocapture is precisely what a later cleanup deletes — it is noisy, it looks like debug residue, and removing it breaks no test. It would have silently disarmed the guard instead.

The first version of this fix was wrong, which is the useful part

I keyed it on --ignored. It fired on the release-artifact arm, which is #[ignore] because it builds a release binary, not because it can skip — that test asserts unconditionally and already carries its own positive control.

--ignored conflates "can skip at runtime" with "expensive, run explicitly", and only the first needs the flag. A guard keyed on the wrong property fails the build for arms that were never at risk, which is how a correct-sounding guard gets deleted wholesale rather than fixed.

So the discriminator is derived from source instead: if an invocation names a --test target whose file can emit a skip notice, --nocapture is required. A new skip path anywhere arms this automatically; a removed one disarms it. Neither requires anyone to remember this function exists.

The e2e floor had drifted under its population

run_expect 8 against 9 live arms. The ninth could vanish — deleted, renamed out of the harness, gated behind an absent feature — and 8 of 9 still clears a floor of 8.

A floor below its population silently stops being a floor for the difference, and the gap is invisible from a green run. Raised to 9, with a comment saying to move it with the arm count.

Verification

drop --nocapture from the skip-capable arm    RED, names the target and why
floor above the arm count                     RED, "ran 9, expected at least 10"
full run, unmodified                          GREEN — the negative control
source probe, both directions                 real_daemon_e2e greps 1, cli_admin greps 0

The green run is the control that matters: it proves the guard can stay silent, which a red-only check never establishes. A guard that has only ever fired has not shown it can distinguish.

---

^{Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.}

---

Summary by cubic

Fixes two ways the gate could pass without checking what it claims: the skip guard now enforces --nocapture based on the test target's source, and the e2e floor matches the actual arm count.

Bug Fixes

• Previously, the skip guard relied on callers passing --nocapture; without it, cargo's captured output left the guard reading an empty stream, so arms that skipped looked clean.
• The guard now probes each --test target's source for a skip notice and fails the build if --nocapture is missing; keying on --ignored was rejected because #[ignore] also marks the release-artifact arm, which never skips.
• The e2e floor was 8 against 9 live arms, so a deleted or gated arm could hide behind the old floor; it's now 9 and should move with the arm count.

^{Written for commit 9280472. Summary will update on new commits.}

[cubic-dev-ai]

2 issues found across 1 file

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="scripts/gate.sh">

<violation number="1" location="scripts/gate.sh:158">
P3: The argument loop scans the entire argv including everything after the `--` separator and silently overwrites `target` on every subsequent `--test` match (last one wins). A user narrowing a run by adding `--test <name>` after `--` would reassign `target` to a file whose source may lack `SKIPPING`, disarming the guard for the real target. Post-`--` scanning is needed for `--nocapture` (the real_daemon arm places it after `--`), but `--test` after `--` is an unguarded override. Stop the `--test`/target capture at the `--` separator.</violation>

<violation number="2" location="scripts/gate.sh:163">
P2: The source-probe `grep -q 'SKIPPING'` is a case-sensitive, whole-file literal match for a magic token, so it does not actually detect whether a target's source can emit a skip notice as the comment above it claims. In this very repo, `cli_admin.rs` (a target this guard probes) emits a real skip notice as lowercase `"skipping api_key_login_flow_integration: ..."` via `eprintln!` and never contains uppercase `SKIPPING`, so the probe stays silent for it. A skip path that does not use the exact token — or one that appears only in a comment/string that never runs — either goes unarmed (false negative) or arms spuriously. That is the same magic-string fragility the comment says it removed from the old `--ignored` check. Note the tension: if you make the probe case-insensitive so it truly "asks the source," it would then falsely fail the `cli_admin` arm, which omits `--nocapture` (its skip test is filtered out by `validation_bypass_is_absent` and does not run there, but the probe cannot see that). Standardize the skip-notice convention across the probe and run_expect's downstream `grep 'SKIPPING'` (e.g. both case-insensitive, and ensure every skip-capable arm passes `--nocapture`), or scope the comment to say the guard only covers the `SKIPPING` convention.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

P2: The source-probe grep -q 'SKIPPING' is a case-sensitive, whole-file literal match for a magic token, so it does not actually detect whether a target's source can emit a skip notice as the comment above it claims. In this very repo, cli_admin.rs (a target this guard probes) emits a real skip notice as lowercase "skipping api_key_login_flow_integration: ..." via eprintln! and never contains uppercase SKIPPING, so the probe stays silent for it. A skip path that does not use the exact token — or one that appears only in a comment/string that never runs — either goes unarmed (false negative) or arms spuriously. That is the same magic-string fragility the comment says it removed from the old --ignored check. Note the tension: if you make the probe case-insensitive so it truly "asks the source," it would then falsely fail the cli_admin arm, which omits --nocapture (its skip test is filtered out by validation_bypass_is_absent and does not run there, but the probe cannot see that). Standardize the skip-notice convention across the probe and run_expect's downstream grep 'SKIPPING' (e.g. both case-insensitive, and ensure every skip-capable arm passes --nocapture), or scope the comment to say the guard only covers the SKIPPING convention.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At scripts/gate.sh, line 163:

<comment>The source-probe `grep -q 'SKIPPING'` is a case-sensitive, whole-file literal match for a magic token, so it does not actually detect whether a target's source can emit a skip notice as the comment above it claims. In this very repo, `cli_admin.rs` (a target this guard probes) emits a real skip notice as lowercase `"skipping api_key_login_flow_integration: ..."` via `eprintln!` and never contains uppercase `SKIPPING`, so the probe stays silent for it. A skip path that does not use the exact token — or one that appears only in a comment/string that never runs — either goes unarmed (false negative) or arms spuriously. That is the same magic-string fragility the comment says it removed from the old `--ignored` check. Note the tension: if you make the probe case-insensitive so it truly "asks the source," it would then falsely fail the `cli_admin` arm, which omits `--nocapture` (its skip test is filtered out by `validation_bypass_is_absent` and does not run there, but the probe cannot see that). Standardize the skip-notice convention across the probe and run_expect's downstream `grep 'SKIPPING'` (e.g. both case-insensitive, and ensure every skip-capable arm passes `--nocapture`), or scope the comment to say the guard only covers the `SKIPPING` convention.</comment>

<file context>
@@ -131,6 +131,42 @@ run_check "clippy (seam features)" \
+  if [ -n "$target" ] && [ "$has_nocapture" = "0" ]; then
+    local src
+    src="$(find crates -path "*/tests/${target}.rs" -print -quit 2>/dev/null || true)"
+    if [ -n "$src" ] && grep -q 'SKIPPING' "$src"; then
+      fail "$label targets ${target}, whose source can print a skip notice, but omits \
+--nocapture. cargo captures that notice, so the skip check below would read an empty \
</file context>

[cubic-dev-ai]

P3: The argument loop scans the entire argv including everything after the -- separator and silently overwrites target on every subsequent --test match (last one wins). A user narrowing a run by adding --test <name> after -- would reassign target to a file whose source may lack SKIPPING, disarming the guard for the real target. Post--- scanning is needed for --nocapture (the real_daemon arm places it after --), but --test after -- is an unguarded override. Stop the --test/target capture at the -- separator.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At scripts/gate.sh, line 158:

<comment>The argument loop scans the entire argv including everything after the `--` separator and silently overwrites `target` on every subsequent `--test` match (last one wins). A user narrowing a run by adding `--test <name>` after `--` would reassign `target` to a file whose source may lack `SKIPPING`, disarming the guard for the real target. Post-`--` scanning is needed for `--nocapture` (the real_daemon arm places it after `--`), but `--test` after `--` is an unguarded override. Stop the `--test`/target capture at the `--` separator.</comment>

<file context>
@@ -131,6 +131,42 @@ run_check "clippy (seam features)" \
+  for a in "$@"; do
+    [ "$a" = "--nocapture" ] && has_nocapture=1
+    [ "$want_target" = "1" ] && { target="$a"; want_target=0; }
+    [ "$a" = "--test" ] && want_target=1
+  done
+  if [ -n "$target" ] && [ "$has_nocapture" = "0" ]; then
</file context>