Summary
PR #99 added a useful fail-safe documentation-only CI path and a blocking in-repo Markdown link checker. The exact-head workflow was green and merged as 2b3c9c1874fd38bbc5f8fc98483bce4377fd1480, but two proof gaps from the final review remain on main.
1. Reject relative links that resolve outside the repository
tools/check_markdown.py currently does:
base = root if target_path.startswith("/") else path.parent
target = (base / target_path.lstrip("/")).resolve()
if not target.exists():
...It checks existence but never requires the resolved target to stay under root.
Reproduction against the merged checker:
- Create
repo/doc.md containing [outside](../outside.md).
- Create the sibling
outside.md beside repo.
- Run
python tools/check_markdown.py --root repo.
- Actual: exit
0, 0 problem(s).
The result depends on the runner’s surrounding filesystem rather than repository authority. A symlink inside the repository that resolves outside has the same problem.
Fix: after resolve(), require target.relative_to(root) to succeed (or equivalent). If it escapes, emit an explicit finding before the existence check.
Required tests:
- existing sibling outside
root is rejected;
- in-repo symlink to an outside target is rejected;
- a nested document’s
../guide.md that resolves inside root passes;
- ordinary relative, root-relative, image, and fragment cases remain green.
2. Exercise the docs-only classifier behavior, not just YAML strings
test_docs_only_change_sets_skip_the_gradle_gates_but_fail_safe currently checks that the workflow text contains docs_only="false", trap emit EXIT, and the job condition. It never executes the classifier. A change can preserve those strings while breaking range selection, rename handling, or output settlement.
Move the classifier into a small checked-in helper invoked by the workflow and test its actual output for:
- Markdown-only diff → docs-only true;
- mixed source + docs → false;
- source deletion plus
.md addition with rename detection disabled → false;
- uppercase
.MD → true;
- empty diff → false;
- missing/all-zero base → false;
- invalid/unreadable range → false;
- source renamed to Markdown → false.
Every uncertain or failed classification must keep the existing fail-safe behavior: run the full suite.
Verification
Run:
python3 -m unittest discover -s tools/tests
python3 tools/check_markdown.py
Also prove a docs-only PR skips only the intended Gradle jobs, while one mixed or invalid-range fixture runs them.
Follow-up from #99 exact-head review: #99 (comment)
Summary
PR #99 added a useful fail-safe documentation-only CI path and a blocking in-repo Markdown link checker. The exact-head workflow was green and merged as
2b3c9c1874fd38bbc5f8fc98483bce4377fd1480, but two proof gaps from the final review remain onmain.1. Reject relative links that resolve outside the repository
tools/check_markdown.pycurrently does:It checks existence but never requires the resolved target to stay under
root.Reproduction against the merged checker:
repo/doc.mdcontaining[outside](../outside.md).outside.mdbesiderepo.python tools/check_markdown.py --root repo.0,0 problem(s).The result depends on the runner’s surrounding filesystem rather than repository authority. A symlink inside the repository that resolves outside has the same problem.
Fix: after
resolve(), requiretarget.relative_to(root)to succeed (or equivalent). If it escapes, emit an explicit finding before the existence check.Required tests:
rootis rejected;../guide.mdthat resolves insiderootpasses;2. Exercise the docs-only classifier behavior, not just YAML strings
test_docs_only_change_sets_skip_the_gradle_gates_but_fail_safecurrently checks that the workflow text containsdocs_only="false",trap emit EXIT, and the job condition. It never executes the classifier. A change can preserve those strings while breaking range selection, rename handling, or output settlement.Move the classifier into a small checked-in helper invoked by the workflow and test its actual output for:
.mdaddition with rename detection disabled → false;.MD→ true;Every uncertain or failed classification must keep the existing fail-safe behavior: run the full suite.
Verification
Run:
Also prove a docs-only PR skips only the intended Gradle jobs, while one mixed or invalid-range fixture runs them.
Follow-up from #99 exact-head review: #99 (comment)