Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ualtinok merged 1 commit into from
Aug 29, 2026
Merged

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

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 45 additions & 1 deletion scripts/gate.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,42 @@ run_check "clippy (seam features)" \
run_expect() {
local min="$1" label="$2"; shift 2
printf '\n=== %s ===\n' "$label"

# THE SKIP CHECK BELOW DEPENDS ON A FLAG IT DOES NOT CONTROL, so enforce the
# dependency here rather than describing it in a comment and hoping. `--nocapture`
# is what makes a skip notice visible to this function; without it the guard reads
# an empty stream, finds no notice, and reports the arm clean -- identical to a run
# where nothing skipped. Nothing about the green says which happened. That flag is
# exactly what a later tidy-up deletes: it is noisy, it looks like debug residue,
# and removing it breaks no test. It silently disarms the guard instead.
#
# WHICH ARMS NEED IT IS DERIVED FROM SOURCE, NOT ASSUMED. The first version of this
# check keyed on `--ignored` and was WRONG -- it fired on the release-artifact arm,
# which is `#[ignore]` because it builds a release binary, not because it can skip.
# `--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 instead of fixed.
#
# The real discriminator is whether the target's source can EMIT a skip notice, so
# ask the source. A new skip path in any test file arms this automatically; a
# removed one disarms it. Neither requires anyone to remember this function exists.
local a want_target=0 target="" has_nocapture=0
for a in "$@"; do
[ "$a" = "--nocapture" ] && has_nocapture=1
[ "$want_target" = "1" ] && { target="$a"; want_target=0; }
[ "$a" = "--test" ] && want_target=1

@cubic-dev-ai cubic-dev-ai Bot Aug 29, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>

done
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

@cubic-dev-ai cubic-dev-ai Bot Aug 29, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>

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 \
stream and pass the arm without ever seeing it skip."
fi
fi

local out
out="$("$@" 2>&1)" || { printf '%s\n' "$out"; fail "$label"; }
printf '%s\n' "$out" | grep -E '^test result:' || true
Expand Down Expand Up @@ -211,7 +247,15 @@ run_expect 442 "workspace unit + integration" \
# what needs capturing is the SUPERVISOR's own stderr at that moment -- the arm
# reports only that the file never appeared, which is an absence and says nothing
# about why. Do not let this comment become a claim that the flag settled it.
CRED_REQUIRE_DAEMON=1 run_expect 8 "real-daemon e2e (ship gate)" \
# THE FLOOR MUST MATCH THE POPULATION, and this one had drifted under it. It read 8
# against 9 live arms, so the ninth could vanish -- deleted, renamed out of the
# harness, gated behind an absent feature -- and the gate would still pass, because
# 8 of 9 clears a floor of 8. A floor below its population silently stops being a
# floor for the difference.
#
# Raise this WITH the arm count. A floor that trails is worse than no floor: it
# reports a bound it is not enforcing, and the gap is invisible from a green run.
CRED_REQUIRE_DAEMON=1 run_expect 9 "real-daemon e2e (ship gate)" \
cargo test --locked -p credentials-module --test real_daemon_e2e -- \
--ignored --nocapture --test-threads=1

Expand Down
Loading