feat(link-health): add stdlib Python link checker with unchecked egress bucket by jimcadden · Pull Request #123 · rossoctl/automation

jimcadden · 2026-10-29T16:00:11Z

Replace the lychee-only path with LINK_CHECKER=python|lychee (default lychee) and a stdlib link-check.py that emits lychee-compatible JSON. Egress-proxy refusals are classified as unchecked so they never open or auto-close issues.

Assisted-By: Claude (Anthropic AI) noreply@anthropic.com

…ss bucket Replace the lychee-only path with LINK_CHECKER=python|lychee (default lychee) and a stdlib link-check.py that emits lychee-compatible JSON. Egress-proxy refusals are classified as unchecked so they never open or auto-close issues. Signed-off-by: Jim Cadden <jcadden@ibm.com>

clawgenti

Solid design (default-off flag, shadow mode, unchecked bucket) with thorough tests, but the macOS CI leg is red and the new tests never run in CI.

must-fix scripts/link-health-scanner.sh:157 — the empty-array expansion "${CONFIG_ARGS[@]}" under set -u is fatal on the macOS runner's bash 3.2, so the scanner dies for any repo without .lychee.toml; test-link-health-dryrun-baseline.sh fails on macOS (ubuntu's bash 5 passes).
must-fix tests/test-link-health-unchecked.sh:1 — this test and tests/test-link-check.py are missing from .github/workflows/tests.yml's explicit list (and the shellcheck step), so they silently never run in CI.
suggestion tests/test-link-check.py:412 — the optional lychee-parity test fails on any host with lychee installed (reference-definition URLs).
suggestion scripts/link-check.py:326 — with a proxy configured, any connection reset is classified "unchecked", including origin-side resets after the CONNECT tunnel is up.
nit scripts/link-check.py:543 — --exclude-all-private filters the original host only; redirect targets are not re-checked.

Reviewed by clawgenti using the github-pr-review skill

clawgenti · 2026-10-29T16:36:09Z

+
+  if [ "$LINK_CHECKER" = "python" ]; then
+    python3 "$SCRIPT_DIR/link-check.py" "${LINK_CHECKER_ARGS[@]}" \
+      "${CONFIG_ARGS[@]}" "$repo_dir" > "$CHECK_OUTPUT" 2>/dev/null || true


must-fix — macOS CI is red because of this empty-array expansion. The macOS runner ships bash 3.2.57, where "${CONFIG_ARGS[@]}" with an empty array (any repo without a .lychee.toml) is a fatal CONFIG_ARGS[@]: unbound variable error under set -u — it only became legal in bash 4.4, so ubuntu (bash 5) passes while macOS dies in the scan loop. That is exactly the test-link-health-dryrun-baseline.sh failure: the real run writes no latest.json and files nothing (run 1's checks pass vacuously, and test-link-health-subset-carryforward.sh also "passes" vacuously because its assertions hold against the seeded baseline when the scanner exits mid-loop). Fix both branches (here and line 160) with ${CONFIG_ARGS[@]+"${CONFIG_ARGS[@]}"}, or revert to the previous conditional-inline --config style.

clawgenti · 2026-10-29T16:36:09Z

@@ -0,0 +1,182 @@
+#!/usr/bin/env bash


must-fix — this test (and tests/test-link-check.py) is not in .github/workflows/tests.yml's explicit test list, nor in the shellcheck step's list. The workflow's own comment warns: "A new test file must be added here (and to the relevant shellcheck step) or it silently never runs in CI." Right now neither new test executes on any CI leg. Notably, had this one run on macOS it would have failed non-vacuously and caught the bash 3.2 regression: case 1 asserts unchecked_count == 1 in a latest.json that the crashing scanner never writes.

clawgenti · 2026-10-29T16:36:09Z

+            # We only require our extractor URLs ⊆ lychee's for the shared formats;
+            # lychee may also find mailto:/etc. that we skip by scheme later.
+            missing = ours - lychee_urls
+            self.assertFalse(missing, f"URLs we extract that lychee did not: {missing}")


suggestion — test_optional_lychee_extraction_compare fails on any host with lychee installed (verified locally with lychee 0.24.2): lychee does not extract unused reference-definition URLs like [ref]: https://extract.example/reference, while MD_REF_DEF_RE in link-check.py does, so missing is non-empty and the assertion trips. The test skips in CI (no lychee there), so it only bites shadow-mode hosts — exactly where the parity signal matters. Either drop reference definitions from the extractor for parity, exclude them from this comparison, or document the divergence as intentional.

clawgenti · 2026-10-29T16:36:09Z

+    if "tunnel connection failed" in lower and "403" in msg:
+        return True
+    # Connection reset/close while establishing the CONNECT tunnel.
+    if isinstance(exc, (ConnectionResetError, ConnectionAbortedError, BrokenPipeError)):


suggestion — when a proxy is configured, any ConnectionResetError/ConnectionAbortedError/BrokenPipeError is classified as an egress refusal, including resets raised by the origin server after the CONNECT tunnel is established (urllib surfaces those as the same exception types). Such links are then marked "unchecked" — never retried, never reported — so a genuinely flaky origin behind a proxy silently disappears from results. Consider restricting this to failures during tunnel establishment (before any response bytes), or retrying a reset once before classifying it.

clawgenti · 2026-10-29T16:36:09Z

+        if url_excluded(url, exclude_res):
+            continue
+        host = urllib.parse.urlparse(url).hostname or ""
+        if exclude_private:


nit — --exclude-all-private filters on the original URL's hostname only; urllib then follows redirects without re-validating, so a public URL that 302s to a private/loopback address is still fetched. Worth confirming whether lychee re-checks redirect targets against exclude_all_private and matching that behavior before LINK_CHECKER=python becomes the default.

An empty config array aborted the macOS scan, and the new tests never ran. Unused reference definitions and post-tunnel resets are no longer unchecked, and a redirect to a private host is not fetched. Signed-off-by: Jim Cadden <jcadden@ibm.com> Assisted-by: Cursor <cursoragent@cursor.com>

jimcadden · 2026-10-30T14:51:47Z

Addressed the review in 7ad4a53.

bash 3.2: both checker invocations use ${CONFIG_ARGS[@]+"${CONFIG_ARGS[@]}"}, so an empty array under set -u no longer aborts the macOS scan. Verified on bash 3.2.57.
CI: tests/test-link-health-unchecked.sh is in the shellcheck list and the run loop. tests/test-link-check.py runs in the suite.
Lychee parity: unused [label]: url definitions are dropped before the bare-URL pass. A used reference is still extracted.
Proxy resets: only a Tunnel connection failed is an egress refusal. An origin Connection reset by peer behind a proxy stays a reported error.
Private redirects: with --exclude-all-private, a redirect whose host is private or loopback is not fetched.

Locally: python3 tests/test-link-check.py, bash tests/test-link-health-unchecked.sh, and shellcheck on the scanner and the new shell test.

Assisted-By: Claude Code

clawgenti

All five prior findings are addressed and verified (bash 3.2-safe CONFIG_ARGS expansion, both new tests wired into CI, egress refusal now requires tunnel-failure text, redirect targets re-checked for private hosts, unused reference definitions dropped); CI is green on both legs and the full suite — including the lychee parity test, which runs on this host with lychee 0.24.2 — passes locally.

Findings:

suggestion scripts/link-check.py:147 — 4-space-indented lines are always treated as code and blanked, but in CommonMark they are usually nested list items or lazy continuations whose URLs lychee extracts (details inline).

Reviewed by clawgenti using the github-pr-review skill

clawgenti · 2026-10-30T15:15:53Z

+            out.append("\n" if line.endswith("\n") else "")
+            continue
+        # Indented code block: line starts with 4 spaces or a tab (and is not blank).
+        if re.match(r"^(?: {4}|\t)", line) and line.strip():


suggestion — This blanks every 4-space/tab-indented non-blank line as code, but CommonMark only treats a 4-space indent as a code block outside list context: nested list items and lazy paragraph continuations are commonly indented 4 spaces, and lychee extracts URLs from them. The python checker will under-extract relative to lychee (shadow totals under-count; broken links in nested bullets go unreported), and the parity test can't catch it since it only asserts ours ⊆ lychee. Consider exempting lines that start a list item (e.g. ^\s*([-*+]|\d+[.)])\s) before treating indentation as code.

clawgenti

Follow-up on 7ad4a53: the rebase wires both new test files into the shellcheck and test-suite lists and runs tests/test-link-check.py in CI; both legs are green and DCO is signed. Two extraction-heuristic gaps remain (one carried over, one new) plus a forward-compat note on the test loader.

Findings:

suggestion scripts/link-check.py:147 — 4-space-indented lines are always blanked as code, but in CommonMark they are usually nested list items whose URLs lychee does extract (carried over from last round; verified against lychee 0.24.2).
suggestion scripts/link-check.py:101 — BARE_URL_RE's lookbehind drops any URL starting immediately after (, so prose like "(https://example.com)" is never checked (lychee extracts it; verified).
nit tests/test-link-check.py:29 — exec_module without sys.modules registration is the pattern that just broke #124's ubuntu leg on Python 3.14; worth fixing here too.

Reviewed by clawgenti using the github-pr-review skill

clawgenti · 2026-10-30T16:15:07Z

+            out.append("\n" if line.endswith("\n") else "")
+            continue
+        # Indented code block: line starts with 4 spaces or a tab (and is not blank).
+        if re.match(r"^(?: {4}|\t)", line) and line.strip():


suggestion — carried over from the last round and still open: every line indented 4 spaces (or a tab) is blanked as code, but in CommonMark a 4-space indent is usually a nested list item or lazy continuation whose URLs lychee does extract. Verified with lychee 0.24.2 on a fixture: - item followed by an indented nested line with https://example.com/x is extracted by lychee but dropped here, so the python checker under-reports relative to lychee in shadow mode. A blank-line-preceded rule (or tracking list context) would track CommonMark more closely.

clawgenti · 2026-10-30T16:15:07Z

+MD_REF_DEF_RE = re.compile(r"^\s*\[([^\]]+)\]:\s*(\S+)", re.MULTILINE)
+MD_REF_USE_RE = re.compile(r"\[([^\]]*)\]\[([^\]]*)\]")
+MD_AUTOLINK_RE = re.compile(r"<(https?://[^>\s]+)>", re.IGNORECASE)
+BARE_URL_RE = re.compile(r"(?<![(\[<\"'])(https?://[^\s<>\)\]\"']+)", re.IGNORECASE)


suggestion — the negative lookbehind skips any URL that starts immediately after (, so prose like "(https://example.com/release)" is never extracted at all. Verified with lychee 0.24.2: it extracts the parenthesized URL, so this is another shadow-mode divergence. The lookbehind exists to avoid re-capturing markdown-link targets (](url)); consider skipping only when the ( is preceded by ], or dedupe against MD_INLINE_RE matches instead.

clawgenti · 2026-10-30T16:15:07Z

+    spec = importlib.util.spec_from_file_location("link_check_mod", CHECKER)
+    mod = importlib.util.module_from_spec(spec)
+    assert spec.loader is not None
+    spec.loader.exec_module(mod)


nit — exec_module on a module that was never registered in sys.modules is the exact pattern that broke #124's ubuntu leg on Python 3.14 (sys.modules.get(cls.__module__) is None inside @dataclass processing, raising AttributeError). link-check.py has no dataclasses today so this passes, but adding sys.modules[spec.name] = mod before exec_module(mod) future-proofs the loader.

jimcadden added the ready-for-ai-review Request automated AI code review from clawgenti label Oct 29, 2026

clawgenti reviewed Oct 29, 2026

View reviewed changes

clawgenti reviewed Oct 30, 2026

View reviewed changes

jimcadden force-pushed the feat/stdlib-link-checker branch from 7ad4a53 to d8cca92 Compare October 30, 2026 15:53

clawgenti reviewed Oct 30, 2026

View reviewed changes

clawgenti mentioned this pull request Oct 30, 2026

feat: Add deterministic doc-drift program with release assist #124

Open

3 of 4 tasks

Conversation

jimcadden commented Oct 29, 2026

Uh oh!

clawgenti left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jimcadden commented Oct 30, 2026

Uh oh!

clawgenti left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

clawgenti left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants