diff --git a/.github/actions/repo-gate/repo_gate.py b/.github/actions/repo-gate/repo_gate.py index 46508234..375a78df 100755 --- a/.github/actions/repo-gate/repo_gate.py +++ b/.github/actions/repo-gate/repo_gate.py @@ -127,10 +127,12 @@ def tracked(root: Path, exclude: list[str] | None = None) -> list[str]: if exclude: args += ["--", *(f":!{pattern}" for pattern in exclude)] result = subprocess.run(args, capture_output=True, text=True, check=False) - # A failed command and a valid empty result would otherwise look the same to the caller. - # The command's own stderr (a bad pathspec, an unreadable root) is surfaced rather than dropped. - if result.returncode != 0 and result.stderr.strip(): - print(f"git ls-files failed: {result.stderr.strip()}", file=sys.stderr) + if result.returncode != 0: + # A failed command's stdout is never trusted, even where it is non-empty. + # A partial listing read as complete is a scan that missed files and said nothing. + reason = result.stderr.strip() or f"exit {result.returncode}, no stderr" + print(f"git ls-files failed: {reason}", file=sys.stderr) + return [] return [l for l in result.stdout.split("\n") if l] diff --git a/scripts/tests/test_repo_gate.py b/scripts/tests/test_repo_gate.py index ec9649cb..0ce17d69 100755 --- a/scripts/tests/test_repo_gate.py +++ b/scripts/tests/test_repo_gate.py @@ -617,6 +617,28 @@ def test_a_failed_ls_files_call_prints_gits_own_error(self) -> None: self.assertIn("git ls-files failed", err.getvalue()) self.assertIn("Invalid pathspec magic", err.getvalue()) + def test_a_failed_call_is_never_trusted_even_with_nonempty_stdout(self) -> None: + """A partial listing read as a complete one is a scan that missed files silently.""" + proc = subprocess.CompletedProcess([], 128, "kept.py\n", "fatal: something went wrong\n") + with ( + mock.patch.object(repo_gate.subprocess, "run", return_value=proc), + contextlib.redirect_stderr(io.StringIO()), + ): + files = repo_gate.tracked(self.tmp, ["vendor/**"]) + self.assertEqual([], files) + + def test_a_failure_with_no_stderr_still_prints_a_reason(self) -> None: + """Empty stderr on a nonzero exit must not read as a silent, unexplained empty scan.""" + proc = subprocess.CompletedProcess([], 1, "", "") + with ( + mock.patch.object(repo_gate.subprocess, "run", return_value=proc), + contextlib.redirect_stderr(io.StringIO()) as err, + ): + files = repo_gate.tracked(self.tmp, ["vendor/**"]) + self.assertEqual([], files) + self.assertIn("git ls-files failed", err.getvalue()) + self.assertIn("exit 1", err.getvalue()) + class TestHarness(unittest.TestCase): def test_this_module_collects_a_plausible_number_of_cases(self) -> None: