Decide the Rule Set From the Repository Being Scanned, Not the Caller's Directory

[ptr727]

prose_lint.py chose whether to run home-path by asking whether the current directory was an operational repository, rather than the path it was asked to scan.

Stand in an operational repo, scan a release repo, and the check is discarded. The skip prints to stderr and the run exits 0.

That silences the one rule written because real paths carrying family names reached a public comment. A gate that switches itself off based on where the caller happens to stand is answering a different question than the one it was asked — and answering it as a pass.

Reproduced, and measured against a real violation

A release repo carrying /home/someuser/project/output.log, scanned from an operational checkout:

Before — silent skip, exit 0, violation missed:

note: home-path is not checked in an operational repository, where an absolute
path is the operator instruction rather than observed data.
exit=0

After — found, exit 1, and identical to what running from inside the repo gives:

README.md:3: home-path: absolute home path '/home/someuser' -> use a constructed path, not an observed one
exit=1

The fix

The scanned path decides. Paths spanning two repositories refuse rather than pick one, since each declares its own workflow model and no single rule set can be right for both. A path git cannot place falls back to itself rather than to ., which would quietly put the caller's directory back in charge of the verdict.

The --diff same-root guard is untouched and still correct — git diff genuinely does run in the current directory. But it only ever ran under --diff, so it never covered this path at all.

Why the existing suite missed it

TestOperationalExemption already covers the exemption in five cases. It mocks repo_root to return one value for every argument, which cannot tell the caller's repository from the scanned one — the exact distinction the defect lives in.

The four new cases set those two roots to different models. Three of them fail against the previous code. The fourth guards the fallback against a future regression rather than reproducing the defect, and I am flagging that rather than counting it as four.

Provenance

Reported by the ESPHome-Config agent, from a symptom I could not reproduce — its scenario exits 2 here on the guard that landed in #520. I went looking anyway and found this adjacent defect, which is real.

Independently hit by the HomeAutomation-Config agent within the hour, which had run the gate from a scratch directory and got a clean result worth nothing. It re-ran from inside the checkout and found 190 violations in prose it was about to submit.

Two agents, two different wrong directories, same hour. The tool gave both of them a pass.

Verification

Full local suite green: all five self-tests, audit --selftest, gh-write-guard --selftest, repo_gate.py, prose_lint.py tree-wide, editorconfig-checker.

[Copilot]

Pull request overview

This pull request fixes scripts/prose_lint.py so the active rule set (notably the home-path operational-repo exemption) is determined by the repository being scanned, rather than by the caller’s current working directory. This prevents a false-clean outcome where running the gate from an operational checkout could silently disable home-path while scanning a release repository.

Changes:

• Determine the scan “root” from the provided scan paths and use it (not .) when deciding whether to exempt home-path.
• Refuse scans that span multiple repositories/roots to avoid applying a single rule set across incompatible workflow models.
• Add focused regression tests covering differing caller-vs-scanned repository roots and the “git can’t place this path” fallback.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
scripts/prose_lint.py	Computes scan_root from the requested scan paths and uses it to decide the operational exemption; refuses multi-root scans.
scripts/test_prose_lint.py	Adds regression tests ensuring the scanned repository (not caller cwd) decides exemptions and that multi-root scans refuse.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (1)

scripts/prose_lint.py:1121

• scan_roots currently falls back to Path(p).resolve() per requested path, and then refuses whenever there is more than one distinct resolved path. That creates two concrete problems:

1. It breaks the existing multi-path CLI behavior for non-git inputs (e.g. prose_lint.py README.md docs/guide.md outside a repo): each path resolves to a different absolute path, so the run exits 2 even though there is no workflow model ambiguity.

2. It can make the operational exemption decision inconsistent with what discover() will actually scan. discover() treats a path that is neither a file nor a dir as . (see discover(): root = p if p.is_dir() else Path('.')), but scan_roots will treat that same raw argument as its own resolved path, potentially changing whether home-path is exempted (or triggering the new multi-root refusal) before discovery runs.

Consider deriving the scan anchor the same way discovery does (dir -> itself, file -> parent, missing -> .), and only refusing when there are multiple distinct non-empty git roots (i.e., multiple repositories), not merely multiple filesystem paths.

 # A path git cannot place resolves to itself, so the roots are not all repositories.
# The refusal below names which is which rather than calling every one of them a repository.
scan_roots = {}
for raw in (a.paths or ['.']):
found = repo_root(Path(raw))
scan_roots[found or str(Path(raw).resolve())] = bool(found)
if len(scan_roots) > 1:
named = ', '.join(root + ('' if is_repo else ' (no git repository)')
for root, is_repo in sorted(scan_roots.items()))

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

[ptr727]

Answering the suppressed finding here, since it carries no thread. It was correct, it named a regression this PR introduced, and it is fixed in 492957a.

It breaks the existing multi-path CLI behavior for non-git inputs (e.g. prose_lint.py README.md docs/guide.md outside a repo): each path resolves to a different absolute path, so the run exits 2 even though there is no workflow model ambiguity.

Reproduced exactly as described, against both versions, from a directory under no repository:

$ prose_lint.py README.md docs/guide.md --check home-path
this PR: exit=2 error: ...resolve to more than one root...
develop: exit=0

My fallback — resolving a path git cannot place to the path itself, added so the caller's directory could not creep back in — made every argument its own root. Two files in one loose directory became "two repositories". That is worse than the defect I was fixing, because it turns a working invocation into a hard refusal.

The refusal now keys on non-empty git roots, as you suggested. Only a repository declares a workflow model, so only distinct repositories are ambiguous. Loose paths conflict neither with each other nor with a single repository.

That also dissolves the thread finding above it. With only git roots in the message, no non-repository root can appear in it, so the marking I added an hour ago is gone rather than corrected. The message went back to naming repositories because it now only ever names repositories.

It can make the operational exemption decision inconsistent with what discover() will actually scan.

Taken. With no repository in play, the anchor now follows discovery's own rule rather than a second rule invented here, so the rule set is decided over the tree that will actually be read.

Cases: the one asserting the marked-up message is replaced by two — several loose paths are not a conflict, and one repository beside a loose path is not either. Three of the six in this class still fail against develop, so the original defect stays covered. Checked live as well: two real repositories still refuse, and standing in an operational repo while scanning a release repo still finds the planted violation.

Worth saying plainly: this is the second time in this PR that a fix of mine introduced a narrower version of the problem it was fixing. Both were caught here rather than by me.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (2)

scripts/prose_lint.py:1130

• When none of the scan paths are in a git repo, scan_root falls back to Path('.') for file arguments (scan_root = first if first.is_dir() else Path('.')). That reintroduces the original bug for loose files: if the caller happens to be in an operational repo, home-path will be exempted even though the scanned file is not in that repo. Using the file's parent directory keeps the rule-set decision anchored to what is actually being scanned.

 if git_roots:
scan_root = Path(next(iter(git_roots)))
else:
first = Path(scan_paths[0])
scan_root = first if first.is_dir() else Path('.')

scripts/test_prose_lint.py:1898

• test_a_path_under_no_repository_falls_back_to_itself_rather_than_the_caller currently passes the directory (loose) into prose_lint.main(...), so it never exercises the file-argument path where scan_root previously fell back to Path('.'). This means the regression (caller CWD influencing home-path when scanning a loose file) can slip through. Updating the test to pass the file path and mocking operational_checkout to treat Path('.') as operational will fail on the buggy behavior and pass once scan_root uses the scanned path’s parent.

 with mock.patch.object(prose_lint, 'repo_root', return_value=''), \
mock.patch.object(prose_lint, 'discover', return_value=[bait]):
# Not a repository, so it carries no operational payload and the rule stays on.
self.assertEqual(1, prose_lint.main([str(loose), '--check', 'home-path']))
self.assertNotIn('operational repository', self.err.getvalue())

[ptr727]

Both suppressed findings on the head are correct and both are fixed in 3939f7c. The first is this PR's own defect for the third time, in a narrower form each round.

When none of the scan paths are in a git repo, scan_root falls back to Path('.') for file arguments. That reintroduces the original bug for loose files: if the caller happens to be in an operational repo, home-path will be exempted even though the scanned file is not in that repo.

Correct. I chose Path('.') last round to match discover(), and that was the wrong thing to match. discover() uses . as a root for a tracked-file lookup; the rule-set decision has to anchor on what is being scanned. Matching a function because it is nearby is not the same as matching it because it answers the same question.

A file now anchors on its own parent. A bare filename still anchors on ., and that stays correct, since . is the directory holding it.

Verified live, from an operational checkout scanning a loose file elsewhere:

$ prose_lint.py /tmp/.../loose2/notes.md --check home-path
notes.md:3: home-path: absolute home path '/home/someuser' -> use a constructed path, not an observed one
exit=1

test_a_path_under_no_repository_falls_back_to_itself_rather_than_the_caller currently passes the directory into prose_lint.main(...), so it never exercises the file-argument path.

This is the more useful of the two, because it explains why the first one reached the head at all. I wrote that test specifically to guard the fallback, and it guarded the branch that was not broken. A regression guard aimed at the wrong branch reads exactly like coverage.

It now passes the file, with operational_checkout mocked so . is operational, which is the arrangement that fails on the old behaviour and passes on the new one. It does fail against the previous commit. A second case keeps the directory branch covered rather than trading one for the other.

On this PR's pattern. Three rounds, three fixes of mine that each introduced a narrower version of the problem being fixed:

1. CWD decided the rule set — the original defect.
2. The fallback made every path its own root, refusing valid multi-file runs.
3. The fallback anchored a loose file on CWD, exempting it again.

Each was caught here. The common cause is that I kept fixing the case in front of me and re-deriving the fallback each time, rather than stating the invariant once: the rule set is decided by the thing being scanned, and no code path may consult the process's working directory to decide it. That sentence is now the comment above the block, so the next change has something to check itself against instead of a worked example to imitate.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (1)

scripts/prose_lint.py:1115

• The new comment block explains the full history and several edge cases, which is likely to drift over time and makes the core behavior harder to spot. Consider condensing it to 1-2 lines that capture the invariant (rule-set is based on the scanned repository root; multi-repo arguments refuse) and let the tests carry the detailed rationale.

 # The rule set is decided by the repository being scanned, never by the directory the caller stands in.
# Reading it from `.` discarded home-path on a release repository whenever the caller happened to stand in an operational one.
# That silenced the rule that exists because real paths reached a public comment, announcing it on stderr and exiting 0.
# A run spanning two repositories refuses instead of picking one, since the two can declare different models and one rule set cannot be right for both.
# Only a repository declares a workflow model, so only distinct repositories are ambiguous.
# Two loose paths under no repository are not a conflict.
# Refusing on them would break the ordinary multi-file invocation outside a checkout, where every argument resolves somewhere different.

[ptr727]

Taken, and fixed in f2037e7.

The new comment block explains the full history and several edge cases, which is likely to drift over time and makes the core behavior harder to spot. Consider condensing it to 1-2 lines that capture the invariant and let the tests carry the detailed rationale.

Right on both counts, and the first is the one that matters: seven lines of incident history sat above four lines of code, and history is exactly the part that goes stale without anything failing.

It is three lines now — the invariant, what follows from it, and where the cases live:

# The invariant: the rule set is decided by what is scanned, and no path here reads the working directory to decide it.# Only a repository declares a model, so two of them refuse and anything else resolves to one anchor.# TestScanRootDecidesTheRuleSet carries the cases and the reason each one exists.

The rationale is not dropped. Each of the seven cases in that class carries the reason it exists in its own docstring, which is where it cannot rot without a test failing — a comment claiming why is unverifiable, and a test claiming why fails when the why stops being true.

Worth noting the irony, since it is the same defect class this PR is about: I wrote that block one line at a time across three rounds, each fixing the round before, and never went back to state the rule once. Long comments assembled by accretion are a symptom of that, not a separate style problem.

This closes every finding on this PR: two threads resolved and four suppressed answered here, across five rounds.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (1)

scripts/prose_lint.py:1123

• scan_root is derived from the raw CLI arguments, but discover() treats any non-file/non-directory argument as Path('.') (scripts/prose_lint.py:264). If the user passes a typo/nonexistent path, the tool will silently scan the caller’s current directory while selecting the home-path exemption based on the nonexistent path’s parent, so the rule set and the scanned content can diverge and produce confusing/incorrect verdicts. Consider refusing early when any requested path does not exist, instead of falling back to scanning ..

 ', '.join(sorted(git_roots)) + '). Each declares its own workflow model, so no '
'single rule set is correct for all of them. Run the gate once per repository.',
file=sys.stderr)
return 2
# A file anchors on its own parent rather than on `.`, which is where the caller stands.
if git_roots:
scan_root = Path(next(iter(git_roots)))
else:

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

[ptr727]

Correct, and fixed in cbb5684.

If the user passes a typo/nonexistent path, the tool will silently scan the caller's current directory while selecting the home-path exemption based on the nonexistent path's parent, so the rule set and the scanned content can diverge.

This is the same defect this PR is about, wearing different clothes: the run describes one tree and judges it by another. Refusing early, as you suggest, is better than making the anchor agree with the fallback, because scanning . after a typo is wrong even when the rule set happens to match.

A missing path is now refused, naming each one.

Two existing cases broke, and that is worth reporting rather than smoothing over.test_a_path_under_no_repository_is_refused_too and test_list_files_still_reports_scope_across_repositories both passed fake paths (/tmp/loose, /other/tree) as a mocking convenience. The new check refused them before they reached the guard each exists to exercise.

I gave them real directories rather than exempting --list-files from the existence check. Loosening the check to accommodate a test would have preserved a green suite while removing the property, and the tests' actual subject is the diff guard, not path existence. Both still assert exactly what they did.

Worth noting for its own sake: --list-files is deliberately not exempt here. It is exempt from the diff guard because it never consults the diff, but reporting a scan scope for a path that does not exist has the same confusion at its root.

Two cases added, one failing against the previous commit and one guarding the ordinary path.

That closes every finding on this PR: two threads resolved, five suppressed answered here, across six rounds.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (2)

scripts/prose_lint.py:1126

• The multi-repository refusal message concatenates raw paths without quoting. If a repository root path includes spaces, the message is hard to interpret; quoting each root avoids ambiguity.

 print('error: the requested paths span more than one repository (' +
', '.join(sorted(git_roots)) + '). Each declares its own workflow model, so no '
'single rule set is correct for all of them. Run the gate once per repository.',
file=sys.stderr)

scripts/prose_lint.py:1119

• The nonexistent-path refusal message prints raw path tokens joined by commas, which is ambiguous when a path contains a comma or whitespace. Quoting each path (e.g., via repr) makes the error actionable and copy/pasteable.

This issue also appears on line 1123 of the same file.

 print(f"error: requested path(s) do not exist: {', '.join(sorted(absent))}. Refusing "
'rather than falling back to the current directory, which would scan one tree and '
'choose the rule set from another.', file=sys.stderr)

[ptr727]

Both suppressed findings on the head are the same point on two messages, and both are fixed in 8756f3a.

The nonexistent-path refusal message prints raw path tokens joined by commas, which is ambiguous when a path contains a comma or whitespace.

The multi-repository refusal message concatenates raw paths without quoting.

Correct, and the timing is what makes it worth fixing rather than filing: both messages fire precisely when a path is not what the caller expected, so that is the worst moment for the message to be ambiguous about how many paths there even are.

One quoted() helper serves both rather than a repr at each site — two messages formatting the same thing differently is the small version of the problem this whole PR is about.

error: requested path(s) do not exist: '/tmp/.../no such, dir'. Refusing rather than falling back...
error: the requested paths span more than one repository ('/home/pieter/Blog', '/home/pieter/ProjectTemplate'). ...

Two cases added, one per message, each asserting the quoted form appears.

That closes every finding on this PR: two threads resolved and seven suppressed answered in the conversation, across nine rounds. 209 cases in the file.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (1)

scripts/prose_lint.py:1130

• The new scan-root selection refuses missing paths, but it still accepts existing arguments that are neither a file nor a directory (e.g., FIFO/socket/device). In that case discover() falls back to Path('.'), which can reintroduce the original bug: scanning the caller’s directory while selecting the rule set from the argument’s parent. Consider refusing any path that isn’t a file or directory, so the “scan target decides” invariant always holds.

 scan_paths = a.paths or ['.']
# A path that does not exist is refused rather than absorbed.
# `discover` reads a non-file, non-directory argument as `.`, so a typo scanned the caller's directory while the rule set anchored on the missing path's parent.
absent = [p for p in scan_paths if not Path(p).exists()]
if absent:
# Quoted, since a path holding a space or a comma is unreadable in a bare comma-joined list.
print(f"error: requested path(s) do not exist: {quoted(absent)}. Refusing rather than "
'falling back to the current directory, which would scan one tree and choose the '
'rule set from another.', file=sys.stderr)

[ptr727]

Correct, and fixed in 3124758.

The new scan-root selection refuses missing paths, but it still accepts existing arguments that are neither a file nor a directory (e.g., FIFO/socket/device). In that case discover() falls back to Path('.'), which can reintroduce the original bug.

Right, and it is the same mistake I have now made repeatedly on this PR: I checked for the case I pictured — a typo — rather than for the property the invariant needs. Existence was never the question. discover cares whether the argument is a file or a directory, so that is what the guard has to ask.

A FIFO, a socket, and a device all pass exists() and all fall through to ..

unusable= [pforpinscan_pathsifnot (Path(p).is_file() orPath(p).is_dir())]

Two cases: the missing path, and a real FIFO created with os.mkfifo in the test, skipped where that is unavailable. The FIFO case asserts exists() is true first, so it is testing the gap rather than restating the typo case. It fails against the previous commit.

The pattern, stated once rather than round by round. Every finding on this PR after the first has been a sibling of the one before it: CWD instead of the scanned path, then every path its own root, then a loose file anchored on CWD, then a missing path absorbed, and now a FIFO absorbed. Each fix asked "does this handle the case in front of me" instead of "does the invariant hold for every input shape". The invariant was even written in the comment above the code by round four, and I still did not check the new guard against it.

209 to 210 cases. Every finding on this PR is now answered: two threads resolved, eight suppressed answered here, across ten rounds.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.