Skip to content

ci: classify-paths.sh reads a grep error as "no match", silently answering wrong #545

Description

@EricAndrechek

What

scripts/classify-paths.sh turns a grep failure into a grep "no match", silently answering the wrong classification instead of failing.

Both decisions have this shape:

if printf '%s\n' "$files" | grep -qE '^(docs/|clients/ts/|…)'; then
  echo "docs=true"
else
  echo "docs=false"
fi

grep exits 0 on match, 1 on no match, and 2 on error (can't fork/exec, read error, bad pattern). The else branch collapses 1 and 2 into the same answer. set -euo pipefail is on but does not help: set -e is suppressed for a command used as an if condition.

Observed

One make ci run (static checks run at -j 14) failed test-classify-paths on a single case:

FAIL mixed-docs-go      want code=true docs=true, got code=true docs=false

Every other case passed in the same run. mixed-docs-go feeds docs/x.md internal/a.go: the first grep matched (code=true, correct), the second did not (docs=false, wrong) — for input where docs/x.md plainly matches ^docs/.

The script and its test are byte-identical to main, the test passes standalone on both, and it has passed in ~20 other local make ci runs on the same branch. That points at a transient grep failure under parallel load rather than a logic error in the patterns — and a transient grep failure is exactly what produces this signature.

Why it matters beyond the flake

In CI this script drives the changes job that decides whether the docs pipeline runs. A docs=false produced by an errored grep doesn't fail the run — it skips the docs build and reports success. The test caught it here only because the test asserts an expected value; the production path has no such check.

Fix direction

Distinguish the exit codes:

set +e
printf '%s\n' "$files" | grep -qE ''
rc=$?
set -e
case $rc in
  0) echo "docs=true" ;;
  1) echo "docs=false" ;;
  *) echo "classify-paths: grep failed (exit $rc)" >&2; exit 2 ;;
esac

A helper taking the pattern would keep both call sites honest. Callers already have to handle a non-zero exit from the script itself, so failing loudly is strictly better than a confidently wrong answer.

Spotted during #540's pre-push gate.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions