diff --git a/crates/devflow-cli/tests/fixtures/plan-bashisms/legit-PLAN.md b/crates/devflow-cli/tests/fixtures/plan-bashisms/legit-PLAN.md
index 3996f71b..1d8754a3 100644
--- a/crates/devflow-cli/tests/fixtures/plan-bashisms/legit-PLAN.md
+++ b/crates/devflow-cli/tests/fixtures/plan-bashisms/legit-PLAN.md
@@ -5,5 +5,5 @@ does run under bash. A scanner that rejected this would make plan authoring impo
and would satisfy every must-fail case here.
- bash -c 'cargo test 2>&1 | tail -3; echo "x=${PIPESTATUS[0]}"'
+ bash -c 'cargo build 2>&1 | tail -3; echo "x=${PIPESTATUS[0]}"'
diff --git a/crates/devflow-cli/tests/plan_bashism_scanner.rs b/crates/devflow-cli/tests/plan_bashism_scanner.rs
index c9e3a9e7..d0b187d7 100644
--- a/crates/devflow-cli/tests/plan_bashism_scanner.rs
+++ b/crates/devflow-cli/tests/plan_bashism_scanner.rs
@@ -488,56 +488,218 @@ fn assert_inline_accepted(file_name: &str, contents: &str) {
);
}
-/// `rg -c` prints nothing and exits 1 when nothing matches, so piping it into
-/// `rg '^0$'` can never observe the zero it claims to check.
+/// Wrap one command in a single `` block.
+fn gate(command: &str) -> String {
+ format!("\n {command}\n\n")
+}
+
+/// `rg -c` prints nothing and exits 1 when nothing matches, so a count compared
+/// in shell misreads the zero case. The old `| rg '^0$'` shape is still caught.
#[test]
fn rg_count_piped_to_zero_match_is_blocked() {
assert_inline_blocked(
"rg-count-zero-PLAN.md",
- "\n rg -c needle src/lib.rs | rg '^0$'\n\n",
- "rg -c ... | rg '^0$' trap",
+ &gate("rg -c needle src/lib.rs | rg '^0$'"),
+ "rg count flag -c",
+ );
+}
+
+#[test]
+fn rg_count_in_a_flag_cluster_is_blocked() {
+ assert_inline_blocked(
+ "rg-count-cluster-PLAN.md",
+ &gate("test \"$(rg -nc needle src/lib.rs)\" = 3"),
+ "rg count flag -nc",
);
}
-/// `devflow` is binary-only, so `cargo test -p devflow --lib` selects no target
-/// and verifies nothing.
#[test]
-fn devflow_lib_target_is_blocked() {
+fn rg_count_matches_is_blocked() {
assert_inline_blocked(
- "devflow-lib-PLAN.md",
- "\n cargo test -p devflow --lib stop::tests::x\n\n",
- "cargo test -p devflow --lib verifies nothing",
+ "rg-count-matches-PLAN.md",
+ &gate("rg --count-matches -F needle src/lib.rs || true"),
+ "rg count flag --count-matches",
);
}
-/// Negative control for the `--lib` rule: `devflow-core` has a library target,
-/// and its package name merely starts with `devflow`.
+/// Negative control for the count rule: `-c` as the value of `-e`, inside a
+/// quoted pattern, and the context flags `-C`/`-A` are not counts.
#[test]
-fn devflow_core_lib_target_is_accepted() {
+fn rg_without_a_count_flag_is_accepted() {
assert_inline_accepted(
- "devflow-core-lib-PLAN.md",
- "\n cargo test -p devflow-core --lib verify::tests::parser -- --exact\n\n",
+ "rg-no-count-PLAN.md",
+ &gate("rg -q -e '-c' f && ! rg -n \"bash -c\" g && rg -C 2 x f && rg -A1 -e x f"),
+ );
+}
+
+/// A `cargo test` name filter that matches nothing still exits 0, including a
+/// bare name after `--exact` and a filter on a package with no lib target.
+#[test]
+fn cargo_test_in_a_gate_is_blocked() {
+ for (file, command) in [
+ (
+ "cargo-test-suite-PLAN.md",
+ "cargo test -p devflow-core --lib",
+ ),
+ (
+ "exact-bare-name-PLAN.md",
+ "cargo test -p devflow-core --lib -- --exact holder_status",
+ ),
+ (
+ "devflow-lib-PLAN.md",
+ "cargo test -p devflow --lib stop::tests::x",
+ ),
+ (
+ "cargo-test-toolchain-PLAN.md",
+ "cargo +1.97.1 test -p devflow --bin devflow lock::tests::x",
+ ),
+ ] {
+ assert_inline_blocked(file, &gate(command), "cargo test in a verify gate");
+ }
+}
+
+#[test]
+fn nextest_without_no_tests_fail_is_blocked() {
+ assert_inline_blocked(
+ "nextest-no-fail-PLAN.md",
+ &gate("cargo nextest run -p devflow-core --lib -E 'test(=a::b)'"),
+ "without --no-tests=fail",
+ );
+}
+
+/// `-E 'test(=real) | test(=wrong)'` runs the real test, skips the wrong name
+/// silently and exits 0 (verified on cargo-nextest 0.9.148).
+#[test]
+fn nextest_filter_naming_two_tests_is_blocked() {
+ assert_inline_blocked(
+ "nextest-pipe-PLAN.md",
+ &gate(
+ "cargo nextest run -p devflow-core --lib --no-tests=fail -E 'test(=a::b) | test(=c::d)'",
+ ),
+ "filter is not exactly test(=module::path::name)",
+ );
+}
+
+#[test]
+fn nextest_with_two_filter_flags_is_blocked() {
+ assert_inline_blocked(
+ "nextest-two-e-PLAN.md",
+ &gate(
+ "cargo nextest run -p devflow-core --lib --no-tests=fail -E 'test(=a::b)' -E 'test(=c::d)'",
+ ),
+ "with 2 test filters",
+ );
+}
+
+#[test]
+fn nextest_substring_filter_is_blocked() {
+ assert_inline_blocked(
+ "nextest-substring-PLAN.md",
+ &gate("cargo nextest run -p devflow-core --lib --no-tests=fail -E 'test(a::b)'"),
+ "filter is not exactly test(=module::path::name)",
);
}
-/// A bare name after `--exact` matches no test inside a module, and
-/// `cargo test` still exits 0.
#[test]
-fn bare_test_name_after_exact_is_blocked() {
+fn nextest_positional_filters_are_blocked() {
+ for (file, command) in [
+ (
+ "nextest-positional-PLAN.md",
+ "cargo nextest run -p devflow-core --lib --no-tests=fail a::b",
+ ),
+ (
+ "nextest-dashdash-PLAN.md",
+ "cargo nextest run -p devflow-core --lib --no-tests=fail -- --exact a::b",
+ ),
+ ] {
+ assert_inline_blocked(file, &gate(command), "positional test filter");
+ }
+}
+
+/// The rules read the command inside a `bash -c '...'` region too, where its
+/// own quotes are double quotes.
+#[test]
+fn nextest_inside_bash_c_without_no_tests_fail_is_blocked() {
assert_inline_blocked(
- "exact-bare-name-PLAN.md",
- "\n cargo test -p devflow-core --lib -- --exact holder_status\n\n",
- "cargo test --exact bare-name trap",
+ "nextest-bash-c-PLAN.md",
+ &gate(
+ "bash -c 'cargo nextest run -p devflow-core --lib -E \"test(=a::b)\" 2>&1 | tail -3; echo \"x=${PIPESTATUS[0]}\"'",
+ ),
+ "without --no-tests=fail",
+ );
+}
+
+#[test]
+fn devflow_cli_package_is_blocked() {
+ assert_inline_blocked(
+ "devflow-cli-package-PLAN.md",
+ &gate("cargo nextest run -p devflow-cli --lib --no-tests=fail -E 'test(=a::b)'"),
+ "-p devflow-cli names no package",
+ );
+}
+
+/// A quote that never closes leaves the command undecidable; refuse it.
+#[test]
+fn unparsable_command_is_blocked() {
+ assert_inline_blocked(
+ "nextest-unbalanced-PLAN.md",
+ &gate("cargo nextest run -p devflow-core --lib --no-tests=fail -E 'test(=a::b)"),
+ "cannot parse this cargo command",
+ );
+}
+
+/// An escaped quote inside double quotes does not end them, so the command is
+/// still read and its count flag found.
+#[test]
+fn escaped_quote_inside_double_quotes_is_parsed() {
+ assert_inline_blocked(
+ "rg-escaped-quote-PLAN.md",
+ &gate("bash -c 'n=$(rg -U --count-matches \"token = \\\"$t\\\"\" f || true)'"),
+ "rg count flag --count-matches",
+ );
+ assert_inline_accepted(
+ "rg-escaped-quote-ok-PLAN.md",
+ &gate("bash -c 'rg -q \"token = \\\"$t\\\"\" f'"),
+ );
+}
+
+/// The form the rules ask for: one exact test per command, chained with `&&`.
+#[test]
+fn exact_nextest_commands_chained_are_accepted() {
+ assert_inline_accepted(
+ "nextest-chain-PLAN.md",
+ &gate(
+ "cargo nextest run -p devflow --bin devflow --no-tests=fail -E 'test(=lock::tests::holder_status)' && cargo nextest run -p devflow-core --lib --no-tests=fail -E 'test(=verify::tests::parser)'",
+ ),
+ );
+}
+
+/// A whole suite with no filter, the spaced `--no-tests fail` form, and the `2`
+/// of `2>&1`, which is a file descriptor and not a positional filter.
+#[test]
+fn nextest_suite_run_is_accepted() {
+ assert_inline_accepted(
+ "nextest-suite-PLAN.md",
+ &gate("cargo nextest run -p devflow --test start_lock_e2e --no-tests fail 2>&1"),
+ );
+}
+
+#[test]
+fn exact_nextest_inside_bash_c_is_accepted() {
+ assert_inline_accepted(
+ "nextest-bash-c-ok-PLAN.md",
+ &gate(
+ "bash -c 'cargo nextest run -p devflow-core --lib --no-tests=fail -E \"test(=a::b)\" 2>&1 | tail -3; echo \"x=${PIPESTATUS[0]}\"'",
+ ),
);
}
-/// Negative control for the bare-name rule: a libtest flag after `--exact` is
-/// not a test name. The rule once read `--nocapture` as one and refused
-/// correct plans.
+/// The rules read only `` blocks: plan prose may name the shapes.
#[test]
-fn libtest_flag_after_exact_is_accepted() {
+fn command_shapes_in_prose_are_accepted() {
assert_inline_accepted(
- "exact-then-flag-PLAN.md",
- "\n cargo test -p devflow --bin devflow lock::tests::holder_status -- --exact --nocapture\n\n",
+ "prose-PLAN.md",
+ "Prose: cargo test -p devflow-core --lib and rg -c x are refused in gates.\n\
+ \n cargo fmt --check\n\n",
);
}
diff --git a/scripts/lint-plan-bashisms.sh b/scripts/lint-plan-bashisms.sh
index 251efeba..b3dc6d3d 100755
--- a/scripts/lint-plan-bashisms.sh
+++ b/scripts/lint-plan-bashisms.sh
@@ -52,6 +52,26 @@
# An opening tag with no closing tag is reported as a malformed block rather
# than ignored: leaving it unhandled would be a bypass by construction.
#
+# ---------------------------------------------------------------------------
+# COMMAND RULES. Each command in a block is split into words (quotes removed,
+# stopping at a separator or at the end of its bash -c region) and refused when
+# it has a shape that has shipped a false green or a gate that fails correct
+# work on this repository:
+#
+# - `cargo test`: a name filter that matches nothing still exits 0. Gates run
+# one named test as `cargo nextest run -p PKG --lib --no-tests=fail
+# -E 'test(=module::path::name)'`, or a suite with `--test NAME`.
+# - `cargo nextest run` without `--no-tests=fail`, with a positional or `--`
+# filter, with more than one `-E`/`--skip`, or with a filter that is not
+# exactly `test(=...)`: `-E 'test(=real) | test(=wrong)'` runs one test and
+# exits 0. One test per command, chained with `&&`.
+# - `rg -c` / `--count` / `--count-matches`: zero matches print nothing, not
+# 0, and exit 1. Count source structure in a Rust source-scan test instead.
+# - `-p devflow-cli`: no such package; the CLI package is `devflow`.
+#
+# A command whose quote does not close before it ends is refused as
+# unparsable, for the same fail-closed reason as above.
+#
# The regression cases live in crates/devflow-cli/tests/plan_bashism_scanner.rs,
# which EXECUTES this script rather than reading it.
# ---------------------------------------------------------------------------
@@ -237,22 +257,203 @@ function check_block(body, base, rest, off, o, txt) {
rest = substr(rest, RSTART + RLENGTH)
}
- # Trap: cargo test -p devflow --lib (devflow has no lib target)
- if (match(body, /cargo test[^\n]*(-p[ \t]+devflow[ \t]+--lib|--lib[ \t]+-p[ \t]+devflow)/)) {
- printf "%s:%d: cargo test -p devflow --lib verifies nothing (devflow is binary-only; use -p devflow --bin devflow)\n", fname, line_of(base + RSTART - 1)
- violations++
+ scan_commands(body, base)
+}
+
+# ---- Command rules (no apostrophes anywhere below: this is inside the
+# single-quoted awk program). Each rule names a gate shape that passes on
+# broken work or fails on correct work.
+
+function report(base, o, msg) {
+ printf "%s:%d: %s\n", fname, line_of(base + o - 1), msg
+ violations++
+}
+
+# End of the bash -c region holding offset o, or 0 when o is in none.
+function region_end(o, n) {
+ for (n = 1; n <= nregions; n++) {
+ if (o >= rstart[n] && o <= rend[n]) return rend[n]
+ }
+ return 0
+}
+
+function is_word_start(body, o, c) {
+ if (o == 1) return 1
+ c = substr(body, o - 1, 1)
+ return index(" \t\n(|;&!`" SQ DQ, c) > 0
+}
+
+# Split the command starting at offset o into tok[1..ntok], quotes removed. It
+# stops at an unquoted separator or redirect, or at the end of the bash -c
+# region holding o. Returns -1 when a quote does not close inside that span.
+function tokenize(body, o, limit, c, cur, have, inq) {
+ limit = region_end(o)
+ if (limit == 0) limit = length(body)
+ delete tok
+ ntok = 0
+ cur = ""
+ have = 0
+ inq = ""
+ for (; o <= limit; o++) {
+ c = substr(body, o, 1)
+ if (inq != "") {
+ # Inside double quotes a backslash escapes a quote, a backslash, a
+ # dollar sign or a backtick, as the shell reads it.
+ if (inq == DQ && c == "\\" && index(DQ "\\$`", substr(body, o + 1, 1)) > 0) {
+ o++
+ cur = cur substr(body, o, 1)
+ } else if (c == inq) {
+ inq = ""
+ } else {
+ cur = cur c
+ }
+ continue
+ }
+ if (c == "\\" && o < limit && substr(body, o + 1, 1) != "\n") {
+ o++
+ cur = cur substr(body, o, 1)
+ have = 1
+ continue
+ }
+ if (c == SQ || c == DQ) {
+ inq = c
+ have = 1
+ continue
+ }
+ if (c == "\\" && substr(body, o + 1, 1) == "\n") {
+ o++
+ c = " "
+ }
+ if (c == " " || c == "\t") {
+ if (have) tok[++ntok] = cur
+ cur = ""
+ have = 0
+ continue
+ }
+ if (index("\n;&|<>)`", c) > 0) {
+ # The 2 of 2>&1 is a file descriptor, not an argument.
+ if ((c == ">" || c == "<") && cur ~ /^[0-9]+$/) have = 0
+ break
+ }
+ cur = cur c
+ have = 1
+ }
+ if (inq != "") return -1
+ if (have) tok[++ntok] = cur
+ return ntok
+}
+
+function scan_commands(body, base, rest, off, o) {
+ off = 0
+ rest = body
+ while (match(rest, /(rg|cargo)[ \t]/)) {
+ o = off + RSTART
+ if (is_word_start(body, o)) {
+ if (substr(body, o, 2) == "rg") check_rg(body, base, o)
+ else check_cargo(body, base, o)
+ }
+ off = o
+ rest = substr(body, o + 1)
+ }
+}
+
+# rg -c and --count print nothing (not 0) on zero matches and exit 1, so a
+# count compared in shell misreads the zero case.
+function check_rg(body, base, o, n, i, j, t, ch, hit) {
+ n = tokenize(body, o)
+ if (n < 0) {
+ report(base, o, "cannot parse this rg command (a quote does not close before the command ends)")
+ return
+ }
+ for (i = 2; i <= n && hit == ""; i++) {
+ t = tok[i]
+ if (t == "--") break
+ if (t ~ /^--count(-matches)?(=|$)/) {
+ hit = t
+ } else if (t ~ /^--/) {
+ if (t !~ /=/ && (substr(t, 3) in RG_LONGVAL)) i++
+ } else if (t ~ /^-./) {
+ for (j = 2; j <= length(t); j++) {
+ ch = substr(t, j, 1)
+ if (ch == "c") {
+ hit = t
+ break
+ }
+ if (index(RG_SHORTVAL, ch) > 0) {
+ if (j == length(t)) i++
+ break
+ }
+ }
+ }
+ }
+ if (hit != "") {
+ report(base, o, "rg count flag " hit " in a verify gate (rg prints nothing, not 0, on zero matches; count source structure in a Rust source-scan test, or use rg -q)")
}
+}
- # Trap: rg -c piped to rg ^0$ (no apostrophes: this comment is inside the single-quoted awk program)
- if (match(body, /rg[ \t]+-[a-zA-Z0-9]*c[^\n|]*\|[ \t]*rg[ \t]+[\047\042]?\^0\$/)) {
- printf "%s:%d: rg -c ... | rg %s^0$%s trap (rg -c prints nothing on zero matches and exits 1)\n", fname, line_of(base + RSTART - 1), SQ, SQ
- violations++
+function check_cargo(body, base, o, n, k, i) {
+ n = tokenize(body, o)
+ if (n < 0) {
+ report(base, o, "cannot parse this cargo command (a quote does not close before the command ends)")
+ return
+ }
+ for (i = 2; i <= n; i++) {
+ if (tok[i] == "--") break
+ if (((tok[i] == "-p" || tok[i] == "--package") && tok[i + 1] == "devflow-cli") || tok[i] == "--package=devflow-cli") {
+ report(base, o, "-p devflow-cli names no package (the CLI package is devflow)")
+ }
}
+ k = 2
+ if (tok[k] ~ /^\+/) k++
+ if (tok[k] == "test") {
+ report(base, o, "cargo test in a verify gate (a name filter that matches nothing still exits 0; run one named test as cargo nextest run -p PKG --lib --no-tests=fail -E " SQ "test(=module::path::name)" SQ ", a suite as cargo nextest run -p PKG --test NAME --no-tests=fail)")
+ } else if (tok[k] == "nextest" && tok[k + 1] == "run") {
+ check_nextest(base, o, k + 2, n)
+ }
+}
- # Trap: cargo test --exact with bare identifier (must be module-qualified)
- if (match(body, /cargo test[^\n]*--exact[ \t]+[\047\042]?[A-Za-z0-9_][A-Za-z0-9_-]*[\047\042]?([ \t\n]|$)/)) {
- printf "%s:%d: cargo test --exact bare-name trap (bare name matches nothing; use module-qualified path e.g. tests::name)\n", fname, line_of(base + RSTART - 1)
- violations++
+# One nextest command proves one named test: --no-tests=fail makes a name that
+# matches nothing exit 4, and a filter combining two names passes when only one
+# of them exists.
+function check_nextest(base, o, i, n, t, nofail, nfilt, npos, val) {
+ for (; i <= n; i++) {
+ t = tok[i]
+ if (t == "--") {
+ npos += n - i
+ break
+ }
+ if (t == "--no-tests=fail") {
+ nofail = 1
+ } else if (t == "--no-tests") {
+ if (tok[i + 1] == "fail") nofail = 1
+ i++
+ } else if (t == "-E" || t == "--filterset" || t == "--skip") {
+ nfilt++
+ val = tok[++i]
+ } else if (t ~ /^(--filterset|--skip)=/) {
+ nfilt++
+ val = substr(t, index(t, "=") + 1)
+ } else if (t ~ /^-E./) {
+ nfilt++
+ val = substr(t, 3)
+ } else if (t ~ /^--/) {
+ if (t !~ /=/ && (substr(t, 3) in NX_LONGVAL)) i++
+ } else if (t ~ /^-./) {
+ if (length(t) == 2 && index(NX_SHORTVAL, substr(t, 2, 1)) > 0) i++
+ } else {
+ npos++
+ }
+ }
+ if (!nofail) {
+ report(base, o, "cargo nextest run without --no-tests=fail (a filter that matches nothing then passes)")
+ }
+ if (npos > 0) {
+ report(base, o, "cargo nextest run with a positional test filter (a substring match; use -E " SQ "test(=module::path::name)" SQ ")")
+ }
+ if (nfilt > 1) {
+ report(base, o, "cargo nextest run with " nfilt " test filters (a wrong name is skipped silently; run one test per command and chain them with &&)")
+ } else if (nfilt == 1 && val !~ /^test\(=[A-Za-z0-9_:]+\)$/) {
+ report(base, o, "cargo nextest run filter is not exactly test(=module::path::name): " val " (one test per command, chained with &&)")
}
}
@@ -262,6 +463,13 @@ BEGIN {
OPEN = "<" "automated" ">"
CLOSE = "<" "/automated" ">"
violations = 0
+ # Flags that take a separate value, so the next token is not a filter.
+ RG_SHORTVAL = "efgtTmABCrMjEd"
+ split("regexp file glob iglob type type-not type-add type-clear max-count after-context before-context context replace max-columns threads encoding max-depth sort sortr color colors pre pre-glob path-separator context-separator field-match-separator field-context-separator dfa-size-limit regex-size-limit max-filesize ignore-file engine hostname-bin hyperlink-format", rgv, " ")
+ for (n in rgv) RG_LONGVAL[rgv[n]] = 1
+ NX_SHORTVAL = "pFZRjP"
+ split("color package exclude bin example test bench features build-jobs cargo-profile target target-dir manifest-path cargo-message-format config rerun run-ignored partition platform-filter test-threads retries flaky-result max-fail debugger tracer stress-count stress-duration failure-output success-output status-level final-status-level show-progress max-progress-running message-format message-format-version archive-file archive-format extract-to cargo-metadata workspace-remap binaries-metadata target-dir-remap build-dir-remap config-file user-config-file tool-config-file profile", nxv, " ")
+ for (n in nxv) NX_LONGVAL[nxv[n]] = 1
}
{ buf = buf $0 "\n" }
@@ -295,11 +503,14 @@ done
echo "$SELF: scanned $count file(s)"
if [ "$status" -ne 0 ]; then
- echo "$SELF: refusing a plan whose verify command uses bash-only" >&2
- echo " syntax that expands to NOTHING under the executor's zsh shell:" >&2
+ echo "$SELF: refusing a plan whose verify command has a shape" >&2
+ echo " that passes on broken work or fails on correct work:" >&2
printf '%s' "$hits" | cut -c1-200 | sed 's/^/ /' >&2
- echo " These assertions would pass without ever reading the value they claim" >&2
- echo " to check. Wrap the command so it runs under bash, e.g.:" >&2
+ echo " A cargo, nextest or rg line above names its own fix. A bash-only" >&2
+ echo " expansion (\${...} or declare -A) expands to NOTHING under the" >&2
+ echo " executor's zsh shell, so an assertion on it would pass without ever" >&2
+ echo " reading the value it claims to check. Wrap the command so it runs" >&2
+ echo " under bash, e.g.:" >&2
echo " bash -c 'cargo test ... ; echo \"exit=\${PIPESTATUS[0]}\"'" >&2
echo " The bashism must sit INSIDE the single quotes that follow bash -c;" >&2
echo " a trailing '# bash -c' comment, a closed 'bash -c \":\";' prefix, or a" >&2