From 94bebb3b245bee8e6751f5b13cd7d73331ce958c Mon Sep 17 00:00:00 2001 From: Jiong Gong Date: Wed, 23 Sep 2026 07:43:51 +0000 Subject: [PATCH 1/2] compass(gates): gate_cpu.sh verdict line carries its reason through a pipe A pipeline's status is its last command's, so `gate_cpu.sh | tail -6` reports tail's 0 on a run that exited 98. No script can change the status its caller's shell reports; what it controls is the text. The final line, GATE_CPU_RC=, now says PASSED or NOT PASSED with the reason, so every pipe that keeps the verdict keeps why -- `2>/dev/null | tail -6` used to leave a bare 98 under pytest's green summary. A printed header line names the last line as the verdict. Refusing to run when stdout is a pipe was weighed and rejected: under `docker exec` on the CPU node stdout is a FIFO too (measured), so every normal run would need an override. The `gpu: UNKNOWN` line now states which of its three causes held. A git checkout whose integration ref does not resolve, or shares no commit with HEAD, is no longer told it was never stamped. Closes #191 Co-Authored-By: Claude Opus 5.5 (1M context) --- scripts/compass/gate_cpu.sh | 75 ++++++++---- tests/compass/test_gate_cpu_pipe_identity.py | 2 +- .../test_gate_cpu_verdict_through_pipe.py | 109 ++++++++++++++++++ 3 files changed, 161 insertions(+), 25 deletions(-) create mode 100644 tests/compass/test_gate_cpu_verdict_through_pipe.py diff --git a/scripts/compass/gate_cpu.sh b/scripts/compass/gate_cpu.sh index 0e96f9b45d..d348f30090 100755 --- a/scripts/compass/gate_cpu.sh +++ b/scripts/compass/gate_cpu.sh @@ -39,10 +39,30 @@ set -uo pipefail # not failed safely. # The rule this encodes: the verdict is the script's only output contract, so no # path may skip it, including the ones that give up before measuring anything. +# +# The verdict line also carries its reason, because it is the only line every +# pipe keeps. `gate_cpu.sh | tail -6` hands the caller tail's exit status, not +# this script's, and nothing a script does can change that: the status of a +# pipeline belongs to the caller's shell. What survives is the text, and the +# three common pipes keep different parts of it -- `2>/dev/null | tail -6` drops +# every stderr line, so a bare `GATE_CPU_RC=98` there sits under pytest's green +# summary with nothing to say why it is not a pass. Stated on the verdict line, +# the reason survives any pipe that keeps the verdict at all. +# +# Refusing to run when stdout is a pipe was weighed and rejected. It cannot tell +# `| tail` from the callers that keep the status: under `docker exec`, which is +# how the gate runs on the CPU node, stdout is a FIFO too, as it is under any +# harness that captures output. Every normal run would need an override, and an +# override set by habit lets `| tail` through with it. finish() { - printf 'GATE_CPU_RC=%s\n' "$1" + if [ "$1" -eq 0 ]; then + printf 'GATE_CPU_RC=0 PASSED\n' + else + printf 'GATE_CPU_RC=%s NOT PASSED -- %s\n' "$1" "$2" + fi exit "$1" } +printf 'verdict: the last line, GATE_CPU_RC=; if this run is piped, $? is the pipe'"'"'s\n' # pytest's -r is store-last-wins, so a caller's -rE silently replaces the -rf # below. This gate's own verdict is pytest's exit code and survives that, but @@ -58,23 +78,23 @@ for arg in "$@"; do printf 'REFUSED: %s sets pytest -r, which this gate owns. It selects the\n' "$arg" >&2 printf ' report lines that name the failing tests, and -r is\n' >&2 printf ' store-last-wins, so your flag would replace them silently.\n' >&2 - finish 95 + finish 95 "refused a -r argument; nothing was run" ;; esac done INTEGRATION=${COMPASS_INTEGRATION_REF:-feature/atomcompass_new} -ROOT=$(compass_tree_root) || finish $? -cd "$ROOT" || finish 90 +ROOT=$(compass_tree_root) || finish $? "the tree to measure could not be resolved; nothing was run" +cd "$ROOT" || finish 90 "cannot cd into the tree; nothing was run" compass_env "$ROOT" -compass_require_tree "$ROOT" || finish $? +compass_require_tree "$ROOT" || finish $? "atom does not import from this tree; nothing was run" compass_describe "$ROOT" EXCLUDE=$ROOT/scripts/compass/cpu_gate_exclude.txt TRIGGERS=$ROOT/scripts/compass/gpu_gate_triggers.txt -[ -r "$EXCLUDE" ] || { printf 'FATAL: cannot read %s\n' "$EXCLUDE" >&2; finish 94; } -[ -r "$TRIGGERS" ] || { printf 'FATAL: cannot read %s\n' "$TRIGGERS" >&2; finish 94; } +[ -r "$EXCLUDE" ] || { printf 'FATAL: cannot read %s\n' "$EXCLUDE" >&2; finish 94 "exclusion list unreadable; nothing was run"; } +[ -r "$TRIGGERS" ] || { printf 'FATAL: cannot read %s\n' "$TRIGGERS" >&2; finish 94 "trigger list unreadable; nothing was run"; } # Parse defensively: strip a trailing CR so a list that has been through a # Windows editor does not turn into --ignore=tests/foo.py^M, which pytest @@ -92,7 +112,7 @@ while IFS= read -r line || [ -n "$line" ]; do [ -e "$ROOT/$line" ] || { printf 'FATAL: %s lists %s, which does not exist.\n' "$EXCLUDE" "$line" >&2 printf ' Re-run regen_cpu_gate_exclude.sh.\n' >&2 - finish 94 + finish 94 "exclusion list names a missing file; nothing was run" } IGN+=(--ignore="$line") NEX=$((NEX + 1)) @@ -109,7 +129,7 @@ SRC= GPU_SRC_UNKNOWN= if [ -n "${COMPASS_CHANGED_FILES:-}" ]; then [ -r "$COMPASS_CHANGED_FILES" ] || - { printf 'FATAL: COMPASS_CHANGED_FILES=%s unreadable\n' "$COMPASS_CHANGED_FILES" >&2; finish 94; } + { printf 'FATAL: COMPASS_CHANGED_FILES=%s unreadable\n' "$COMPASS_CHANGED_FILES" >&2; finish 94 "COMPASS_CHANGED_FILES unreadable; nothing was run"; } CHANGED=$(cat "$COMPASS_CHANGED_FILES") SRC="COMPASS_CHANGED_FILES" elif git -C "$ROOT" rev-parse --git-dir >/dev/null 2>&1 && @@ -139,10 +159,19 @@ elif [ -r "$ROOT/.compass-changed" ]; then CHANGED=$(cat "$ROOT/.compass-changed") SRC=".compass-changed stamp" else - # No git, no supplied list, no stamp: the question cannot be answered. It is - # recorded as unanswered, and the run below exits 98 rather than passing -- - # reporting "GPU not required" here would be a guess that reads as a clear. - GPU_SRC_UNKNOWN=1 + # No supplied list, no usable diff, no stamp: the question cannot be + # answered. It is recorded as unanswered, with which of the three ways in + # failed, and the run below exits 98 rather than passing -- reporting "GPU + # not required" here would be a guess that reads as a clear. Only a tree with + # no .git is a staging omission; a git checkout reaches here because the + # integration ref does not resolve, or shares no commit with HEAD. + if ! git -C "$ROOT" rev-parse --git-dir >/dev/null 2>&1; then + GPU_SRC_UNKNOWN="this tree was never stamped (no .git, no .compass-changed); a staging omission, see snapshot.sh" + elif REF=$(compass_resolve_ref "$ROOT" "$INTEGRATION"); then + GPU_SRC_UNKNOWN="a git checkout with no .compass-changed, and $REF shares no commit with HEAD" + else + GPU_SRC_UNKNOWN="a git checkout with no .compass-changed, and $INTEGRATION resolves to no commit here" + fi fi if [ -z "$GPU_SRC_UNKNOWN" ]; then @@ -160,7 +189,7 @@ if [ -z "$GPU_SRC_UNKNOWN" ]; then fi if [ -n "$GPU_SRC_UNKNOWN" ]; then - printf 'gpu: UNKNOWN -- this tree was never stamped (no .compass-changed); a staging omission, see snapshot.sh\n' + printf 'gpu: UNKNOWN -- %s\n' "$GPU_SRC_UNKNOWN" elif [ -n "$GPU_NEEDED" ]; then printf 'gpu: REQUIRED (%s)\n' "$SRC" printf '%s' "$GPU_NEEDED" | sed 's/^/ /' @@ -197,19 +226,17 @@ if [ "$RC" -ne 0 ]; then printf 'timing property and fails intermittently on a loaded box. The\n' >&2 printf 'scripts/compass/README.md section names it, what it was measured to do, and\n' >&2 printf 'to run gates one at a time. It is not a Compass defect and is not excluded.\n' >&2 - finish "$RC" + finish "$RC" "pytest failed; the FAILED lines above name the tests" fi if [ -n "$GPU_SRC_UNKNOWN" ]; then - printf 'The CPU tier is green, but the test gate is not passed: this tree was\n' >&2 - printf 'never stamped -- it carries no .compass-changed, which a bare `git\n' >&2 - printf 'archive` does not write and snapshot.sh does. So this run cannot tell\n' >&2 - printf 'whether the diff needs the GPU tier, and an unanswered question is not a\n' >&2 - printf '"no". That is a staging omission, not a finding about the tree.\n' >&2 + printf 'The CPU tier is green, but the test gate is not passed: %s.\n' "$GPU_SRC_UNKNOWN" >&2 + printf 'So this run cannot tell whether the diff needs the GPU tier, and an\n' >&2 + printf 'unanswered question is not a "no". It is not a finding about the tree.\n' >&2 printf ' Fix by any one of: stage with scripts/compass/snapshot.sh, which stamps\n' >&2 printf ' .compass-changed; set COMPASS_CHANGED_FILES=; or\n' >&2 printf ' run in a tree where %s resolves.\n' "$INTEGRATION" >&2 - finish 98 + finish 98 "unknown whether this diff needs the GPU tier: $GPU_SRC_UNKNOWN" fi # A green CPU tier is not a passed gate when the diff lands in the blind spot: @@ -219,7 +246,7 @@ if [ -n "$GPU_NEEDED" ]; then if [ -z "$DONE" ]; then printf 'this diff touches the CPU tier'"'"'s blind spot; the GPU tier has not run.\n' >&2 printf ' Run gate_gpu.sh, then re-run with COMPASS_GPU_GATE_DONE=\n' >&2 - finish 98 + finish 98 "this diff needs the GPU tier, which has not run; run gate_gpu.sh" fi # An attestation names a tree. Without a commit we cannot check it names # THIS tree, and accepting it unchecked would make the whole rule @@ -229,12 +256,12 @@ if [ -n "$GPU_NEEDED" ]; then printf 'COMPASS_GPU_GATE_DONE=%s cannot be checked: this tree has no commit.\n' "$DONE" >&2 printf ' No .git and no .compass-commit stamp, so the attestation could name\n' >&2 printf ' any tree. Rebuild the snapshot with scripts/compass/snapshot.sh.\n' >&2 - finish 98 + finish 98 "COMPASS_GPU_GATE_DONE cannot be checked; this tree has no commit" fi if [ "$DONE" != "$HEAD_SHA" ]; then printf 'COMPASS_GPU_GATE_DONE=%s but this tree is %s.\n' "$DONE" "$HEAD_SHA" >&2 printf ' The GPU tier passed on a different tree. Re-run it here.\n' >&2 - finish 98 + finish 98 "COMPASS_GPU_GATE_DONE names a different tree; run gate_gpu.sh here" fi printf 'gpu: satisfied by COMPASS_GPU_GATE_DONE=%s\n' "${DONE:0:9}" fi diff --git a/tests/compass/test_gate_cpu_pipe_identity.py b/tests/compass/test_gate_cpu_pipe_identity.py index 3f07c987b9..3c6732a677 100644 --- a/tests/compass/test_gate_cpu_pipe_identity.py +++ b/tests/compass/test_gate_cpu_pipe_identity.py @@ -45,7 +45,7 @@ def _rendered(): for line in GATE.read_text().splitlines(): if line == 'if [ "$RC" -ne 0 ]; then': inside = True - elif inside and line.strip() == 'finish "$RC"': + elif inside and line.strip().startswith('finish "$RC"'): break elif inside and line.strip().startswith("printf "): body.append(line) diff --git a/tests/compass/test_gate_cpu_verdict_through_pipe.py b/tests/compass/test_gate_cpu_verdict_through_pipe.py new file mode 100644 index 0000000000..f1d6349c3e --- /dev/null +++ b/tests/compass/test_gate_cpu_verdict_through_pipe.py @@ -0,0 +1,109 @@ +# SPDX-License-Identifier: MIT +"""`gate_cpu.sh`'s verdict has to survive being piped, and say why. + +A pipeline's status is its last command's, so `gate_cpu.sh | tail -6` hands +the caller tail's 0 whatever the gate exited. The script cannot change that. +What it controls is the text: its last line is the verdict, and that line has +to carry the reason, because the three common pipes keep different halves of +the output -- `2>/dev/null | tail -6` drops every stderr line, which on a run +that exits 98 leaves the verdict number under pytest's green summary. + +Each case runs the real script over a throwaway tree: a one-test suite, an +empty exclusion list, and a trigger list that the stamped diff touches, so the +run is green and then exits 98 for the GPU tier. Nothing needs a driver. +""" + +import os +import shutil +import subprocess +import sys +from pathlib import Path + +import pytest + +REPO = Path(__file__).resolve().parents[2] +BASH = shutil.which("bash") +BLIND = "atom/blind.py" + +pytestmark = pytest.mark.skipif( + BASH is None or shutil.which("git") is None, reason="needs bash and git" +) + + +def _tree(root, stamped): + """A minimal checkout the gate accepts, green, touching one GPU trigger.""" + shutil.copytree(REPO / "scripts" / "compass", root / "scripts" / "compass") + (root / "scripts" / "compass" / "cpu_gate_exclude.txt").write_text("") + (root / "scripts" / "compass" / "gpu_gate_triggers.txt").write_text(BLIND + "\n") + (root / "atom").mkdir() + (root / "atom" / "__init__.py").write_text("") + (root / "tests").mkdir() + (root / "tests" / "test_ok.py").write_text("def test_ok():\n pass\n") + if stamped: + (root / ".compass-changed").write_text(BLIND + "\n") + (root / ".compass-commit").write_text("0" * 40 + "\n") + return root + + +def _run(root, command): + env = {k: v for k, v in os.environ.items() if not k.startswith("COMPASS_")} + env["PATH"] = os.path.dirname(sys.executable) + os.pathsep + env["PATH"] + # A checkout above the temp dir must not make the bare tree look like git. + env["GIT_CEILING_DIRECTORIES"] = str(root.parent) + return subprocess.run( + [BASH, "-c", command.format(gate="scripts/compass/gate_cpu.sh")], + cwd=root, + env=env, + capture_output=True, + text=True, + check=False, + timeout=300, + ) + + +@pytest.fixture(scope="module") +def gpu_tree(tmp_path_factory): + return _tree(tmp_path_factory.mktemp("gate") / "ATOM", stamped=True) + + +def test_unpiped_the_gate_exits_98(gpu_tree): + # The control: the status every piped case below loses. + out = _run(gpu_tree, "{gate}") + assert out.returncode == 98, out.stdout + out.stderr + + +@pytest.mark.parametrize( + "pipe", ["| tail -6", "2>&1 | tail -6", "2>/dev/null | tail -6"] +) +def test_a_piped_run_still_reads_as_not_passed_and_says_why(gpu_tree, pipe): + out = _run(gpu_tree, "{gate} " + pipe) + assert out.returncode == 0, "the pipe kept the gate's status; recheck the premise" + kept = out.stdout.splitlines() + assert kept, "the pipe kept nothing" + verdict = kept[-1] + assert verdict.startswith("GATE_CPU_RC=98 NOT PASSED"), kept + assert "gate_gpu.sh" in verdict, ( + f"`{pipe}` keeps a verdict with no reason: {verdict!r}. Beside the " + "pipe's own 0 and pytest's green summary, a bare 98 is all the reader " + "has, so the reason has to travel on the verdict line." + ) + + +def test_a_git_checkout_is_not_called_unstamped(tmp_path): + root = _tree(tmp_path / "ATOM", stamped=False) + git = ["git", "-C", str(root), "-c", "user.name=t", "-c", "user.email=t@t"] + subprocess.run(git + ["init", "-q"], check=True) + subprocess.run(git + ["add", "-A"], check=True) + subprocess.run(git + ["commit", "-qm", "t"], check=True) + out = _run(root, "{gate}") + gpu = [line for line in out.stdout.splitlines() if line.startswith("gpu:")] + assert out.returncode == 98 and gpu, out.stdout + out.stderr + assert "never stamped" not in gpu[0], gpu[0] + assert "feature/atomcompass_new resolves to no commit" in gpu[0], gpu[0] + + +def test_a_tree_with_no_git_and_no_stamp_is_still_called_unstamped(tmp_path): + out = _run(_tree(tmp_path / "ATOM", stamped=False), "{gate}") + gpu = [line for line in out.stdout.splitlines() if line.startswith("gpu:")] + assert out.returncode == 98 and gpu, out.stdout + out.stderr + assert "never stamped" in gpu[0], gpu[0] From c4509d1d9c4f0cf8119e32691fd2da30d26cd315 Mon Sep 17 00:00:00 2001 From: Jiong Gong Date: Wed, 23 Sep 2026 11:06:00 +0000 Subject: [PATCH 2/2] compass(gates): keep GATE_CPU_RC= off the header; test the orphan-ref arm The printed header named the verdict as `GATE_CPU_RC=`, which put the key on stdout twice and made a first-match reader land on the header. It now says "the last line of stdout". A test asserts the key appears on exactly one line of a run's output. The `gpu: UNKNOWN` arm for an integration ref that shares no commit with HEAD had no test; the git-checkout test now covers it with an orphan branch as well as an unresolvable ref. Co-Authored-By: Claude Opus 5.5 (1M context) --- scripts/compass/gate_cpu.sh | 2 +- .../test_gate_cpu_verdict_through_pipe.py | 22 +++++++++++++++++-- 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/scripts/compass/gate_cpu.sh b/scripts/compass/gate_cpu.sh index d348f30090..6b3cb49085 100755 --- a/scripts/compass/gate_cpu.sh +++ b/scripts/compass/gate_cpu.sh @@ -62,7 +62,7 @@ finish() { fi exit "$1" } -printf 'verdict: the last line, GATE_CPU_RC=; if this run is piped, $? is the pipe'"'"'s\n' +printf 'verdict: the last line of stdout; if this run is piped, $? is the pipe'"'"'s\n' # pytest's -r is store-last-wins, so a caller's -rE silently replaces the -rf # below. This gate's own verdict is pytest's exit code and survives that, but diff --git a/tests/compass/test_gate_cpu_verdict_through_pipe.py b/tests/compass/test_gate_cpu_verdict_through_pipe.py index f1d6349c3e..8eaf15cd42 100644 --- a/tests/compass/test_gate_cpu_verdict_through_pipe.py +++ b/tests/compass/test_gate_cpu_verdict_through_pipe.py @@ -72,6 +72,14 @@ def test_unpiped_the_gate_exits_98(gpu_tree): assert out.returncode == 98, out.stdout + out.stderr +def test_the_verdict_key_is_printed_exactly_once(gpu_tree): + # A reader taking the first match must land on the verdict, not on prose + # that mentions the key. + out = _run(gpu_tree, "{gate} 2>&1") + keyed = [line for line in out.stdout.splitlines() if "GATE_CPU_RC=" in line] + assert len(keyed) == 1, keyed + + @pytest.mark.parametrize( "pipe", ["| tail -6", "2>&1 | tail -6", "2>/dev/null | tail -6"] ) @@ -89,17 +97,27 @@ def test_a_piped_run_still_reads_as_not_passed_and_says_why(gpu_tree, pipe): ) -def test_a_git_checkout_is_not_called_unstamped(tmp_path): +@pytest.mark.parametrize( + "orphan, cause", + [(False, "resolves to no commit"), (True, "shares no commit with HEAD")], +) +def test_a_git_checkout_is_not_called_unstamped(tmp_path, orphan, cause): root = _tree(tmp_path / "ATOM", stamped=False) git = ["git", "-C", str(root), "-c", "user.name=t", "-c", "user.email=t@t"] subprocess.run(git + ["init", "-q"], check=True) subprocess.run(git + ["add", "-A"], check=True) subprocess.run(git + ["commit", "-qm", "t"], check=True) + if orphan: + # An integration branch with no history in common with HEAD. + tree = git + ["commit-tree", "HEAD^{tree}", "-m", "o"] + sha = subprocess.run(tree, check=True, capture_output=True, text=True) + branch = ["branch", "feature/atomcompass_new", sha.stdout.strip()] + subprocess.run(git + branch, check=True) out = _run(root, "{gate}") gpu = [line for line in out.stdout.splitlines() if line.startswith("gpu:")] assert out.returncode == 98 and gpu, out.stdout + out.stderr assert "never stamped" not in gpu[0], gpu[0] - assert "feature/atomcompass_new resolves to no commit" in gpu[0], gpu[0] + assert f"feature/atomcompass_new {cause}" in gpu[0], gpu[0] def test_a_tree_with_no_git_and_no_stamp_is_still_called_unstamped(tmp_path):