Harden documentation checks and docs-only CI detection

.github/workflows/0-ci-build-and-test.yml

@@ -57,46 +57,12 @@ jobs:
         env:
           BASE_SHA: ${{ github.event.pull_request.base.sha }}
           HEAD_SHA: ${{ github.event.pull_request.head.sha }}
+          BEFORE_SHA: ${{ github.event.before }}
+          SHA: ${{ github.sha }}
         run: |
-          # Fail-safe by construction: this step always exits 0 and always emits
-          # docs_only, defaulting to 'false' (= run the full suite). Any
-          # uncertainty - no base to diff, an unreadable diff, a single non-doc
-          # path - runs everything. It can over-run CI; it can never skip a
-          # Gradle gate on a change that touches real code.
-          docs_only="false"
-          emit() { echo "docs_only=$docs_only" >> "$GITHUB_OUTPUT"; }
-          trap emit EXIT
-          set -uo pipefail
-
-          # Strict allowlist. A path is docs only if it carries a Markdown
-          # extension. Deliberately NOT docs: *.txt (requirements.txt controls
-          # dependencies), and any code file merely named README/CHANGELOG.
-          if [ -n "${BASE_SHA:-}" ] && [ -n "${HEAD_SHA:-}" ]; then
-            RANGE="$BASE_SHA...$HEAD_SHA"
-          else
-            RANGE="${{ github.event.before }}...${{ github.sha }}"
-          fi
-
-          # --no-renames so `git mv MainActivity.kt notes.md` still reveals the
-          # source-file deletion instead of collapsing to a docs destination.
-          if ! FILES="$(git diff --name-only --no-renames "$RANGE" 2>/dev/null)"; then
-            echo "Could not diff $RANGE - running the full suite."; exit 0
-          fi
-          if [ -z "$FILES" ]; then
-            echo "Empty diff - running the full suite."; exit 0
-          fi
-
-          echo "Changed files:"; echo "$FILES"
-          result="true"
-          while IFS= read -r file; do
-            [ -z "$file" ] && continue
-            case "$(printf '%s' "$file" | tr '[:upper:]' '[:lower:]')" in
-              *.md) ;;
-              *) echo "Non-docs path -> full suite required: $file"; result="false"; break ;;
-            esac
-          done <<< "$FILES"
-          docs_only="$result"
+          docs_only="$(python3 tools/detect_docs_only.py)"
           echo "docs_only=$docs_only"
+          echo "docs_only=$docs_only" >> "$GITHUB_OUTPUT"
 
   # Not a Markdown style linter. `docs-render` blocks only on breaks that render
   # as literal garbage or point at a file that does not exist; `docs-links`

ARCHITECTURE.md

@@ -70,6 +70,7 @@ Recent cleanup keeps `MainActivity` as the Android boundary while moving cluster
 - After artifacts are uploaded, `.github/workflows/1-orchestration-release.yml` publishes both targets in the same run: GitHub (`hermes-webui-v<version>-github.apk`) and Play production (`hermes-webui-v<version>.aab`) with the configured Play service account.
 - The beta workflow `.github/workflows/play-store-beta-manual.yml` remains available for manual/open-testing runs and is no longer part of default orchestration.
 - The build job generates release metadata once from `.github/release.yml` categories and bundles it with both artifacts. GitHub notes preserve clickable PR links; Play's brief `whatsnew-en-US` keeps compact PR/issue URLs, caps output below the Play limit, and appends the in-app bug-report reminder. Retry publishers validate version, tag, commit, build-run SHA, exact artifact name, and bundled metadata before publishing.
+- CI's `tools/check_markdown.py` rejects rendering breaks, missing in-repository targets, and links whose resolved files escape the repository, including through symlinks. `tools/detect_docs_only.py` owns the docs-only Gradle-gate short-circuit and returns `false` for empty, unreadable, invalid, renamed, or mixed diffs.
 - Release workflows use concurrency groups so duplicate runs for the same ref or target version do not publish over each other.
 - The build and publish workflows validate that exactly one matching APK or AAB exists before upload or publication.
 - The publish workflows also support manual dispatch with the build run ID and artifact metadata so a failed GitHub or Play publish can be retried without rebuilding both release artifacts.

README.md

@@ -249,6 +249,17 @@ Device check:
 .\gradlew.bat connectedDebugAndroidTest --no-daemon
 ```
 
+Repository tooling checks can be run with:
+
+```powershell
+python -m unittest discover -s tools/tests -p 'test_*.py' -v
+python tools/check_markdown.py
+```
+
+The docs-only CI short-circuit is fail-safe: only a non-empty diff containing
+Markdown files exclusively can skip the Android Gradle gates. Unreadable,
+empty, invalid, renamed, or mixed diffs run the full suite.
+
 Release automation is centered on:
 
 - `.github/workflows/1-orchestration-release.yml`

tools/check_markdown.py

@@ -66,6 +66,7 @@ def check_rendering_breaks(path: Path, lines: list[tuple[int, str]]) -> list[Fin
 
 def check_repo_links(root: Path, path: Path, lines: list[tuple[int, str]]) -> list[Finding]:
     findings = []
+    root = root.resolve()
     for number, line in lines:
         without_code = re.sub(r"`[^`]*`", "", line)
         for match in INLINE_LINK_RE.finditer(without_code):
@@ -79,6 +80,13 @@ def check_repo_links(root: Path, path: Path, lines: list[tuple[int, str]]) -> li
                 continue
             base = root if target_path.startswith("/") else path.parent
             target = (base / target_path.lstrip("/")).resolve()
+            try:
+                target.relative_to(root)
+            except ValueError:
+                findings.append(
+                    Finding(path, number, f"link target outside repository: {dest}")
+                )
+                continue
             if not target.exists():
                 findings.append(Finding(path, number, f"link target not found: {dest}"))
     return findings

tools/detect_docs_only.py

@@ -0,0 +1,68 @@
+#!/usr/bin/env python3
+"""Report whether the current Git diff contains Markdown files only."""
+
+from __future__ import annotations
+
+import argparse
+import os
+import subprocess
+import sys
+from collections.abc import Iterable
+
+
+def classify_paths(paths: Iterable[str]) -> bool:
+    paths = list(paths)
+    return bool(paths) and all(path.lower().endswith(".md") for path in paths)
+
+
+def changed_paths(base: str, head: str, before: str, sha: str) -> list[str] | None:
+    if base and head:
+        revision_range = f"{base}...{head}"
+    elif before and sha:
+        revision_range = f"{before}...{sha}"
+    else:
+        return None
+
+    if "0" * 40 in revision_range:
+        return None
+
+    try:
+        result = subprocess.run(
+            ["git", "diff", "--name-only", "--no-renames", revision_range],
+            capture_output=True,
+            text=True,
+            check=False,
+        )
+    except OSError:
+        print("Could not run git diff - running the full suite.", file=sys.stderr)
+        return None
+    if result.returncode != 0:
+        print(f"Could not diff {revision_range} - running the full suite.", file=sys.stderr)
+        return None
+    return result.stdout.splitlines()
+
+
+def main(argv: list[str] | None = None) -> int:
+    parser = argparse.ArgumentParser(description=__doc__)
+    parser.add_argument("--base", default=os.environ.get("BASE_SHA", ""))
+    parser.add_argument("--head", default=os.environ.get("HEAD_SHA", ""))
+    parser.add_argument("--before", default=os.environ.get("BEFORE_SHA", ""))
+    parser.add_argument("--sha", default=os.environ.get("SHA", ""))
+    args = parser.parse_args(argv)
+
+    paths = changed_paths(args.base, args.head, args.before, args.sha)
+    if paths is None:
+        print("false")
+        return 0
+
+    if not paths:
+        print("Empty diff - running the full suite.", file=sys.stderr)
+        print("false")
+        return 0
+
+    print("true" if classify_paths(paths) else "false")
+    return 0
+
+
+if __name__ == "__main__":
+    raise SystemExit(main())

tools/tests/test_check_markdown.py

@@ -44,6 +44,48 @@ def test_missing_relative_target_is_reported(self) -> None:
     def test_existing_relative_target_passes(self) -> None:
         self.assertEqual(self._check("See [the guide](guide.md).", ("guide.md",)), [])
 
+    def test_existing_target_outside_repository_is_reported(self) -> None:
+        with TemporaryDirectory() as tmp, TemporaryDirectory() as outside_tmp:
+            root = Path(tmp)
+            outside = Path(outside_tmp) / "guide.md"
+            outside.write_text("", encoding="utf-8")
+            doc = root / "docs" / "doc.md"
+            doc.parent.mkdir()
+            doc.write_text("See [the guide](../../" + outside.name + ").", encoding="utf-8")
+            findings = check_file(root, doc)
+            self.assertEqual(
+                [finding.message for finding in findings],
+                ["link target outside repository: ../../" + outside.name],
+            )
+
+    def test_symlink_target_outside_repository_is_reported(self) -> None:
+        with TemporaryDirectory() as tmp, TemporaryDirectory() as outside_tmp:
+            root = Path(tmp)
+            outside = Path(outside_tmp) / "guide.md"
+            outside.write_text("", encoding="utf-8")
+            link = root / "guide.md"
+            try:
+                link.symlink_to(outside)
+            except (NotImplementedError, OSError) as error:
+                self.skipTest(f"symlinks unavailable: {error}")
+            doc = root / "doc.md"
+            doc.write_text("See [the guide](guide.md).", encoding="utf-8")
+            findings = check_file(root, doc)
+            self.assertEqual(
+                [finding.message for finding in findings],
+                ["link target outside repository: guide.md"],
+            )
+
+    def test_parent_target_inside_repository_passes(self) -> None:
+        with TemporaryDirectory() as tmp:
+            root = Path(tmp)
+            guide = root / "guide.md"
+            guide.write_text("", encoding="utf-8")
+            doc = root / "docs" / "doc.md"
+            doc.parent.mkdir()
+            doc.write_text("See [the guide](../guide.md).", encoding="utf-8")
+            self.assertEqual(check_file(root, doc), [])
+
     def test_anchor_on_a_real_file_passes(self) -> None:
         self.assertEqual(
             self._check("See [setup](guide.md#setup).", ("guide.md",)),

tools/tests/test_detect_docs_only.py

@@ -0,0 +1,72 @@
+import io
+import sys
+import unittest
+from contextlib import redirect_stdout
+from pathlib import Path
+from unittest.mock import patch
+
+ROOT = Path(__file__).resolve().parents[2]
+sys.path.insert(0, str(ROOT / "tools"))
+
+from detect_docs_only import main  # noqa: E402
+
+
+class DocsOnlyDetectorTests(unittest.TestCase):
+    def _run(self, paths: str, **arguments: str) -> str:
+        completed = type("Completed", (), {"returncode": 0, "stdout": paths})()
+        output = io.StringIO()
+        with patch("detect_docs_only.subprocess.run", return_value=completed), redirect_stdout(output):
+            self.assertEqual(
+                main(sum(([f"--{key}", value] for key, value in arguments.items()), [])),
+                0,
+            )
+        return output.getvalue().strip()
+
+    def test_markdown_paths_are_docs_only(self) -> None:
+        self.assertEqual(self._run("README.md\nUPPER.MD\n", base="base", head="head"), "true")
+
+    def test_mixed_paths_require_full_suite(self) -> None:
+        self.assertEqual(self._run("README.md\napp/src/MainActivity.kt\n", base="base", head="head"), "false")
+
+    def test_empty_diff_requires_full_suite(self) -> None:
+        self.assertEqual(self._run("", base="base", head="head"), "false")
+
+    def test_missing_revision_range_requires_full_suite(self) -> None:
+        output = io.StringIO()
+        with redirect_stdout(output):
+            self.assertEqual(main([]), 0)
+        self.assertEqual(output.getvalue().strip(), "false")
+
+    def test_invalid_diff_requires_full_suite(self) -> None:
+        completed = type("Completed", (), {"returncode": 128, "stdout": ""})()
+        output = io.StringIO()
+        with patch("detect_docs_only.subprocess.run", return_value=completed), redirect_stdout(output):
+            self.assertEqual(main(["--base", "base", "--head", "head"]), 0)
+        self.assertEqual(output.getvalue().strip(), "false")
+
+    def test_unavailable_git_requires_full_suite(self) -> None:
+        output = io.StringIO()
+        with patch("detect_docs_only.subprocess.run", side_effect=OSError), redirect_stdout(output):
+            self.assertEqual(main(["--base", "base", "--head", "head"]), 0)
+        self.assertEqual(output.getvalue().strip(), "false")
+
+    def test_push_event_revision_fallback_is_supported(self) -> None:
+        self.assertEqual(self._run("README.md\n", before="before", sha="head"), "true")
+
+    def test_all_zero_revision_requires_full_suite(self) -> None:
+        zero = "0" * 40
+        self.assertEqual(self._run("README.md\n", base=zero, head="head"), "false")
+
+    def test_diff_disables_rename_detection(self) -> None:
+        completed = type("Completed", (), {"returncode": 0, "stdout": "README.md\n"})()
+        output = io.StringIO()
+        with patch("detect_docs_only.subprocess.run", return_value=completed) as run, redirect_stdout(output):
+            self.assertEqual(main(["--base", "base", "--head", "head"]), 0)
+        self.assertEqual(
+            run.call_args.args[0],
+            ["git", "diff", "--name-only", "--no-renames", "base...head"],
+        )
+
+
+if __name__ == "__main__":
+    unittest.main()

tools/tests/test_workflow_contracts.py

@@ -111,8 +111,8 @@ def test_docs_only_change_sets_skip_the_gradle_gates_but_fail_safe(self) -> None
         # change is exactly when they matter most.
         self.assertNotIn("docs_only", blocks["release-tooling-tests"])
         # The detector must default to running everything.
-        self.assertIn('docs_only="false"', blocks["changes"])
-        self.assertIn("trap emit EXIT", blocks["changes"])
+        self.assertIn("python3 tools/detect_docs_only.py", blocks["changes"])
+        self.assertTrue((ROOT / "tools" / "detect_docs_only.py").exists())
 
     def test_required_check_candidates_always_report_a_conclusion(self) -> None:
         """Jobs intended as required status checks must never be skipped.