Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion scripts/prose_lint.py
Original file line number Diff line number Diff line change
Expand Up @@ -1106,11 +1106,25 @@ def main(argv: list[str] | None = None) -> int:

rules = set(a.checks or DEFAULT_RULES)

# 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.
scan_paths = a.paths or ['.']
scan_roots = {repo_root(Path(p)) or str(Path(p).resolve()) for p in scan_paths}
if len(scan_roots) > 1:
print('error: the requested paths span more than one repository (' +
', '.join(sorted(scan_roots)) + '). Each declares its own workflow model, so one '
'rule set cannot be correct for both. Run the gate once per repository.',
file=sys.stderr)
Comment thread
ptr727 marked this conversation as resolved.
Outdated
return 2
scan_root = Path(scan_roots.pop())

# An operational repository's runbook carries the literal path an operator types.
# That is the repository's own content, not an agent quoting an environment it observed.
# The skip is announced, since a rule that silently stops running reads as one that passed.
# That is the same failure the diff-scope floor below exists to prevent.
if 'home-path' in rules and operational_checkout(Path(repo_root(Path('.')) or '.')):
if 'home-path' in rules and operational_checkout(scan_root):
rules.discard('home-path')
print('note: home-path is not checked in an operational repository, where an absolute '
'path is the operator instruction rather than observed data.', file=sys.stderr)
Expand Down
74 changes: 74 additions & 0 deletions scripts/test_prose_lint.py
Original file line number Diff line number Diff line change
Expand Up @@ -1794,6 +1794,80 @@ def test_a_release_repository_still_reports_the_finding(self) -> None:
self.assertEqual(1, prose_lint.main(['--check', 'home-path']))


class TestScanRootDecidesTheRuleSet(unittest.TestCase):
"""The rule set follows the repository being scanned, never the directory the caller stands in.

Read from `.`, the exemption discarded home-path on a release repository whenever the caller
happened to stand in an operational one, announced the skip on stderr, and exited 0. That
silences the rule that exists because real paths reached a public comment.

Every case here gives the two roots *different* models. `TestOperationalExemption` above mocks
`repo_root` to one value for every argument, which cannot tell the caller's repository from the
scanned one, and is why this defect survived it.
"""

def setUp(self) -> None:
self.tmp = Path(self.enterContext(tempfile.TemporaryDirectory()))
self.enterContext(contextlib.redirect_stdout(io.StringIO()))
self.err = self.enterContext(contextlib.redirect_stderr(io.StringIO()))
self.operational = self._repo('operational', ('repo-config', 'operational', 'develop.json'))
self.release = self._repo('release', ('repo-config', 'develop.json'))
self.bait = self.release / 'runbook.md'
self.bait.write_text(f'Deploy into {NIX_HOME}/stack here.\n', encoding='utf-8')

def _repo(self, name: str, payload: tuple[str, ...]) -> Path:
root = self.tmp / name
target = root.joinpath(*payload)
target.parent.mkdir(parents=True, exist_ok=True)
target.write_text('{}\n', encoding='utf-8')
return root

def _roots(self, cwd: Path, scanned: Path):
"""Resolve `.` to one repository and every other path to another."""
return lambda p: str(cwd) if str(p) == '.' else str(scanned)

def test_standing_in_an_operational_repo_does_not_exempt_a_release_repo(self) -> None:
"""The defect: the caller's directory turned the rule off on the repository being scanned."""
with mock.patch.object(prose_lint, 'repo_root',
side_effect=self._roots(self.operational, self.release)), \
mock.patch.object(prose_lint, 'discover', return_value=[self.bait]):
self.assertEqual(1, prose_lint.main([str(self.release), '--check', 'home-path']))
self.assertNotIn('operational repository', self.err.getvalue())

def test_standing_in_a_release_repo_does_not_un_exempt_an_operational_repo(self) -> None:
"""The inverse error, which reports a finding the exemption exists to suppress."""
runbook = self.operational / 'runbook.md'
runbook.write_text(f'Deploy into {NIX_HOME}/stack here.\n', encoding='utf-8')
with mock.patch.object(prose_lint, 'repo_root',
side_effect=self._roots(self.release, self.operational)), \
mock.patch.object(prose_lint, 'discover', return_value=[runbook]):
self.assertEqual(0, prose_lint.main([str(self.operational), '--check', 'home-path']))
self.assertIn('operational repository', self.err.getvalue())

def test_paths_spanning_two_repositories_refuse_rather_than_pick_one(self) -> None:
"""Two repositories declare two models, and one rule set cannot be correct for both."""
with mock.patch.object(prose_lint, 'repo_root', side_effect=lambda p: str(Path(p))):
self.assertEqual(2, prose_lint.main(
[str(self.release), str(self.operational), '--check', 'home-path']))
self.assertIn('span more than one repository', self.err.getvalue())

def test_a_path_under_no_repository_falls_back_to_itself_rather_than_the_caller(self) -> None:
"""The fallback must not reintroduce the dependency the fix removes.

`repo_root` returns '' for a path git cannot place, and resolving that to `.` would put the
caller's directory back in charge of the verdict.
"""
loose = self.tmp / 'loose'
loose.mkdir()
bait = loose / 'notes.md'
bait.write_text(f'Deploy into {NIX_HOME}/stack here.\n', encoding='utf-8')
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())


class TestDiffScopeFloor(unittest.TestCase):
"""The assertion every `--diff` verdict rests on, that the run matched something it was given.

Expand Down