From 7c63408e864fceb412c0791ba5990306b8269f8a Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Fri, 11 Sep 2026 21:25:46 -0700 Subject: [PATCH] Read an Empty prose_lint --diff as a Failed Base, Not as No Diff MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `prose_lint.py` decided whether a run was diff-scoped by the truthiness of `--diff` rather than by its presence, so an empty value read as no `--diff` at all. The run then widened to the whole tree and reported the existing backlog as though the change had introduced it, which is the exact outcome the refusal on the next line exists to prevent. An unresolvable ref took that refusal while an empty string walked past it. The exposed path is a local run whose caller substitutes a command's output, `--diff "$(git merge-base origin/develop HEAD)"`. Where `origin/develop` does not exist, as in a single-branch clone of `main`, `git merge-base` writes only to stderr and the script receives `--diff ""`. The composite action is unaffected, since it verifies `$BASE` resolves before calling the script. Testing for presence routes the empty value into `changed_lines`, where `git diff ''` fails with `fatal: bad revision ''`, so the existing refusal fires and names the value it could not resolve. The reproduction in the issue now exits 2 with `cannot diff against ''` where it previously exited 1 on a line the change never touched. ## What changed - `main` tests `a.diff is not None` in both the scope computation and the refusal below it, with a comment naming the caller shape that produces an empty value, so a later edit does not read the presence test as a stylistic choice. - A regression test asserts the empty value reaches the resolver and takes the refusal, rather than asserting only the exit code, which the whole-tree path could also produce for its own reasons. - A second test runs the same case against a real repository, since the first mocks the resolver and would stay green if git ever resolved an empty revision. Resolving one to an empty scope reads as a clean tree, which is the other half of what the routing closes. Fixes #1537 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) --- .github/actions/prose-gate/prose_lint.py | 6 +++-- scripts/tests/test_prose_lint.py | 33 ++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/.github/actions/prose-gate/prose_lint.py b/.github/actions/prose-gate/prose_lint.py index e84dd271..72a24a54 100755 --- a/.github/actions/prose-gate/prose_lint.py +++ b/.github/actions/prose-gate/prose_lint.py @@ -1846,8 +1846,10 @@ def main(argv: list[str] | None = None) -> int: return 0 discovered = len(files) - scope = changed_lines(a.diff, scan_root) if a.diff else None - if a.diff and scope is None: + # Presence rather than truthiness, since an empty value is a base that failed to compute. + # Reading it as no diff at all is the silent widening the refusal below exists to prevent. + scope = changed_lines(a.diff, scan_root) if a.diff is not None else None + if a.diff is not None and scope is None: # Widening to the whole tree answers a different question, and answers it silently. # A caller scoping to a change gets the backlog reported as though the change made it. # A CI adoption hits this first, where an unresolvable base walls off the first run. diff --git a/scripts/tests/test_prose_lint.py b/scripts/tests/test_prose_lint.py index 4fecba9f..79509863 100755 --- a/scripts/tests/test_prose_lint.py +++ b/scripts/tests/test_prose_lint.py @@ -1899,6 +1899,24 @@ def test_a_failed_diff_is_an_error_rather_than_a_wider_or_narrower_scan(self) -> ): self.assertEqual(2, prose_lint.main(["--check", "dupword", "--diff", "HEAD"])) + def test_an_empty_diff_value_takes_the_refusal_rather_than_widening(self) -> None: + """An empty value is a base that failed to compute, never a request for a whole tree. + + A caller substituting a command's output, `--diff "$(git merge-base origin/develop HEAD)"`, + passes an empty string whenever that command fails. Read as truthiness the option + vanishes, the refusal `main` holds for a base it cannot resolve is never reached, and the + run reports the whole tree's backlog as though the change introduced it. + """ + bait = self.tmp / "bait.md" + bait.write_text(f"{DUP} thing\n", encoding="utf-8") + with ( + mock.patch.object(prose_lint, "discover", return_value=[bait]), + mock.patch.object(prose_lint, "changed_lines", return_value=None) as changed, + ): + self.assertEqual(2, prose_lint.main(["--check", "dupword", "--diff", ""])) + # Pins that the resolver received the empty value itself, rather than a coerced stand-in. + self.assertEqual("", changed.call_args.args[0]) + def test_list_files_prints_the_scope_and_reports_nothing(self) -> None: """The audit path for the sweep scope exits 0 even on a tree full of findings.""" bait = self.tmp / "bait.md" @@ -2672,6 +2690,21 @@ def test_scanning_one_repository_while_standing_in_another_diffs_the_one_scanned self.git(there, "commit", "-qam", "change") self.assertEqual(1, self.run_in(here, str(there), "--diff", "HEAD~1")) + def test_an_empty_base_is_refused_by_a_real_git_rather_than_by_a_mock(self) -> None: + """The precondition the empty-value refusal rests on, asserted against git itself. + + `main` routes an empty `--diff` into the resolver rather than treating it as no diff at + all, which is only a refusal because git rejects an empty revision. + `TestCli.test_an_empty_diff_value_takes_the_refusal_rather_than_widening` mocks the + resolver, so it would stay green if git ever resolved one. Resolving one to an empty scope + reads as a clean tree, which is the false pass the routing exists to close. + """ + root = self.repo() + (root / "DOC.md").write_text(self.BAIT, encoding="utf-8") + self.git(root, "commit", "-qam", "change") + self.assertEqual(1, self.run_in(root, ".", "--diff", "HEAD~1")) + self.assertEqual(2, self.run_in(root, ".", "--diff", "")) + def test_a_subtree_argument_reads_the_untracked_files_inside_it(self) -> None: """`git ls-files` prints names relative to its `-C` directory, for `--others` as well.