diff --git a/.github/workflows/browser-smoke.yml b/.github/workflows/browser-smoke.yml index b0b2eb11..d3c83753 100644 --- a/.github/workflows/browser-smoke.yml +++ b/.github/workflows/browser-smoke.yml @@ -8,6 +8,12 @@ name: Browser smoke # # No secrets, no credentials: the server boots agent-free and the smoke script # strips every *_API_KEY from the environment before launch. +# +# Docs-only fast path: `browser-smoke` is a REQUIRED status check in the master +# ruleset, and a skipped required check reports as pending forever (auto-merge +# would hang). So on a docs/CHANGELOG/README-only change we keep the job running +# and reporting green but short-circuit the expensive browser install + smoke. +# Detection FAILS SAFE: any uncertainty or any non-doc path -> full smoke runs. on: pull_request: @@ -16,13 +22,85 @@ on: branches: [master] jobs: + changes: + runs-on: ubuntu-latest + outputs: + docs_only: ${{ steps.detect.outputs.docs_only }} + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Detect docs-only change set + id: detect + env: + BASE_SHA: ${{ github.event.pull_request.base.sha }} + HEAD_SHA: ${{ github.event.pull_request.head.sha }} + run: | + # Fail-safe: ALWAYS exits 0 and ALWAYS emits docs_only (default 'false' = + # run full smoke), so a detection hiccup can never wedge the required + # browser-smoke context into perpetual pending. + docs_only="false" + emit() { echo "docs_only=$docs_only" >> "$GITHUB_OUTPUT"; } + trap emit EXIT + set -uo pipefail + # STRICT ALLOWLIST — docs iff doc extension (*.md/.markdown/.rst) or an + # extensionless doc basename. NOT docs: *.txt (requirements.txt!), anything + # under docs/ that isn't a doc extension, and code files merely NAMED + # README/CHANGELOG. See tests.yml for the full rationale. + is_docs() { + f="$1"; low="$(printf '%s' "$f" | tr '[:upper:]' '[:lower:]')"; base="${f##*/}" + case "$low" in + *.md|*.markdown|*.rst) return 0 ;; + esac + case "$base" in + LICENSE|README|CHANGELOG|NOTICE|AUTHORS|CONTRIBUTING) return 0 ;; + esac + return 1 + } + if [ -n "${BASE_SHA:-}" ] && [ -n "${HEAD_SHA:-}" ]; then + git fetch --no-tags --depth=1 origin "$BASE_SHA" 2>/dev/null || git fetch --no-tags origin master || true + RANGE="$BASE_SHA...$HEAD_SHA" + else + RANGE="${{ github.event.before }}...${{ github.sha }}" + fi + # --no-renames so a rename reveals BOTH sides (a renamed-away code file is visible). + if ! FILES="$(git diff --name-only --no-renames "$RANGE" 2>/dev/null)"; then + echo "Could not diff $RANGE — running full smoke (fail-safe)."; exit 0 + fi + if [ -z "$FILES" ]; then + echo "Empty diff — running full smoke (fail-safe)."; exit 0 + fi + echo "Changed files:"; echo "$FILES" + result="true" + while IFS= read -r f; do + [ -z "$f" ] && continue + if ! is_docs "$f"; then + echo "Non-docs path -> full smoke required: $f" + result="false" + break + fi + done <<< "$FILES" + docs_only="$result" + echo "docs_only=$docs_only" + browser-smoke: + needs: changes + # Belt-and-suspenders (see tests.yml): run even if `changes` errored/was skipped + # so this required context can never wedge; step guards degrade to full smoke. + if: ${{ always() }} runs-on: ubuntu-latest timeout-minutes: 10 steps: + - name: Docs-only short-circuit + if: needs.changes.outputs.docs_only == 'true' + run: echo "Docs-only change set — skipping browser smoke (no page-executable change)." + - uses: actions/checkout@v4 + if: needs.changes.outputs.docs_only != 'true' - name: Set up Python + if: needs.changes.outputs.docs_only != 'true' uses: actions/setup-python@v5 with: python-version: '3.12' @@ -32,6 +110,7 @@ jobs: **/pyproject.toml - name: Install dependencies + if: needs.changes.outputs.docs_only != 'true' run: | python -m pip install --upgrade pip pip install "pyyaml>=6.0" playwright @@ -39,4 +118,5 @@ jobs: python -m playwright install --with-deps chromium - name: Run browser smoke + if: needs.changes.outputs.docs_only != 'true' run: python tests/browser_smoke.py diff --git a/.github/workflows/docs-ci.yml b/.github/workflows/docs-ci.yml new file mode 100644 index 00000000..5ba7f868 --- /dev/null +++ b/.github/workflows/docs-ci.yml @@ -0,0 +1,116 @@ +name: Docs CI + +# Lightweight checks for documentation changes. Intentionally MINIMAL (Nathan, +# 2026-07-18): NOT a Markdown style linter — no line length, heading levels, list +# markers, or cosmetic rules. It runs only two things, and only when docs change: +# 1. critical_markdown_check.py — flags ONLY a rendering break (a link whose +# destination token is split across a newline, or an unclosed inline link). +# 2. lychee link check — flags broken/dead links (informational, non-blocking). +# +# Runs on docs paths only; does NOT gate code PRs. This job is NOT a required +# status check — it's an informational quality gate; promote it via the ruleset +# later if desired. +# +# SECURITY: changed filenames on a fork PR are UNTRUSTED. They are never +# interpolated into `run:` source via ${{ }} (which Bash would re-evaluate, +# executing $(...) in a crafted filename). The file list is passed through an +# `env:` variable and referenced as "$DOC_FILES", and lychee is handed a FIXED +# glob rather than an attacker-controlled explicit list. + +on: + pull_request: + branches: [master] + paths: + - '**/*.md' + - '**/*.markdown' + - '**/*.rst' + - 'docs/**' + - 'CHANGELOG*' + - '**/README*' + push: + branches: [master] + paths: + - '**/*.md' + - '**/*.markdown' + - '**/*.rst' + - 'docs/**' + - 'CHANGELOG*' + - '**/README*' + +# Read-only; no secrets needed. Fork PRs already get a read-only token, but pin it. +permissions: + contents: read + +jobs: + docs-ci: + runs-on: ubuntu-latest + timeout-minutes: 8 + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Set up Python + uses: actions/setup-python@v5 + with: + python-version: '3.12' + + - name: Resolve changed doc files (NUL-safe, untrusted names) + id: docs + env: + BASE_SHA: ${{ github.event.pull_request.base.sha }} + HEAD_SHA: ${{ github.event.pull_request.head.sha }} + run: | + set -uo pipefail + # Compute the changed markdown/doc files and write them to a file, ONE PER + # LINE. We never echo untrusted names into another shell; downstream steps + # read this file. Fall back to all tracked docs if the diff can't compute. + if [ -n "${BASE_SHA:-}" ] && [ -n "${HEAD_SHA:-}" ]; then + git fetch --no-tags --depth=1 origin "$BASE_SHA" 2>/dev/null || git fetch --no-tags origin master || true + RANGE="$BASE_SHA...$HEAD_SHA" + else + RANGE="${{ github.event.before }}...${{ github.sha }}" + fi + : > doc_files.txt + if CHANGED="$(git diff --name-only --no-renames --diff-filter=ACMR "$RANGE" 2>/dev/null)"; then + printf '%s\n' "$CHANGED" | grep -Ei '\.(md|markdown|rst)$' > doc_files.txt || true + fi + if [ ! -s doc_files.txt ]; then + git ls-files '*.md' '*.markdown' '*.rst' > doc_files.txt 2>/dev/null || true + fi + # Keep only files that still exist (skip deletions), preserving exact names. + : > doc_files_existing.txt + while IFS= read -r f; do + [ -n "$f" ] && [ -f "$f" ] && printf '%s\n' "$f" >> doc_files_existing.txt + done < doc_files.txt + mv doc_files_existing.txt doc_files.txt + echo "Doc files to check:"; cat doc_files.txt + echo "count=$(wc -l < doc_files.txt)" >> "$GITHUB_OUTPUT" + + - name: Critical Markdown check (rendering-breaks only, not style) + if: steps.docs.outputs.count != '0' + run: | + set -uo pipefail + # Read the NUL-safe-ish file list (one path per line) into an array. Names + # are passed as ARGUMENTS to python, never evaluated as shell. + mapfile -t FILES < doc_files.txt + if [ "${#FILES[@]}" -eq 0 ]; then echo "No doc files."; exit 0; fi + python3 scripts/critical_markdown_check.py "${FILES[@]}" + + # Broken-link check. Handed a FIXED glob (not the attacker-controlled list) so + # no untrusted filename ever reaches lychee's `eval`. Over-checking docs is + # harmless. Informational only: continue-on-error keeps it from blocking. + - name: Broken-link check (lychee) + uses: lycheeverse/lychee-action@v2 + with: + args: >- + --no-progress + --max-concurrency 4 + --accept 200,206,301,302,303,307,308,401,403,429 + --timeout 20 + --max-retries 2 + "**/*.md" + "**/*.markdown" + "**/*.rst" + fail: true + continue-on-error: true diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 78b9b01b..a34c5e8c 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -6,28 +6,120 @@ on: push: branches: [master] +# Docs-only fast path: a CHANGELOG/README/docs-only PR does not need the full pytest +# matrix or the JS/lint runtime guards — nothing executable changed. But the `test (…)` +# shards and `lint` are REQUIRED status checks in the master ruleset, and a *skipped* +# required check reports as pending forever (auto-merge would hang). So we keep the jobs +# running and reporting green, but SHORT-CIRCUIT their expensive work when every changed +# path is documentation. Detection FAILS SAFE: if anything is uncertain (no base to diff, +# any non-doc path touched), docs_only=false and the full suite runs. jobs: - # Forward-looking Python lint gate (ruff). The Python twin of the ESLint runtime - # guard. Runs the curated [tool.ruff] ruleset (E9 + F + B) but only on lines this - # PR adds/modifies vs the merge-base — so it keeps NEW code clean without demanding - # a reformat of the existing tree's cosmetic backlog (#3273). Fast, fails early. - lint: + changes: runs-on: ubuntu-latest + outputs: + docs_only: ${{ steps.detect.outputs.docs_only }} steps: - uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Detect docs-only change set + id: detect + env: + BASE_SHA: ${{ github.event.pull_request.base.sha }} + HEAD_SHA: ${{ github.event.pull_request.head.sha }} + run: | + # Fail-safe: this step ALWAYS exits 0 and ALWAYS emits docs_only (default + # 'false' = run the full suite). If the `changes` job ever failed, the + # required test/browser jobs (needs: changes) would be SKIPPED and their + # required contexts would never report — wedging the PR in perpetual + # pending. So the detector can never fail; worst case it over-runs CI. + docs_only="false" + emit() { echo "docs_only=$docs_only" >> "$GITHUB_OUTPUT"; } + trap emit EXIT + set -uo pipefail + # STRICT ALLOWLIST — a path counts as "docs" ONLY if it is unambiguously + # documentation. Everything else runs the full suite (fail-open to safety). + # A file is docs iff: + # * it has a doc extension: *.md / *.markdown / *.rst, OR + # * its basename is an extensionless doc file (LICENSE/README/CHANGELOG/…). + # NOT docs (deliberately): *.txt (requirements.txt controls deps!), anything + # under docs/ that isn't a doc extension (docs/Makefile, docs/build.py, + # docs/*.json), and any code file whose NAME merely contains README/CHANGELOG + # (scripts/CHANGELOG_stamp.py, static/README_renderer.js). Extension/type + # wins over name, and a non-doc extension is never docs. + is_docs() { + f="$1"; low="$(printf '%s' "$f" | tr '[:upper:]' '[:lower:]')"; base="${f##*/}" + case "$low" in + *.md|*.markdown|*.rst) return 0 ;; + esac + case "$base" in + LICENSE|README|CHANGELOG|NOTICE|AUTHORS|CONTRIBUTING) return 0 ;; + esac + return 1 + } + + # Resolve the changed-file list. PR event = base..head; push = before..after. + if [ -n "${BASE_SHA:-}" ] && [ -n "${HEAD_SHA:-}" ]; then + git fetch --no-tags --depth=1 origin "$BASE_SHA" 2>/dev/null || git fetch --no-tags origin master || true + RANGE="$BASE_SHA...$HEAD_SHA" + else + RANGE="${{ github.event.before }}...${{ github.sha }}" + fi + + # --no-renames so a rename shows BOTH sides: `git mv src/app.py docs.md` must + # reveal the src/app.py deletion (a code change) instead of collapsing to the + # docs.md destination and being mis-classified as docs-only. + if ! FILES="$(git diff --name-only --no-renames "$RANGE" 2>/dev/null)"; then + echo "Could not diff $RANGE — running full suite (fail-safe)."; exit 0 + fi + if [ -z "$FILES" ]; then + echo "Empty diff — running full suite (fail-safe)."; exit 0 + fi + + echo "Changed files:"; echo "$FILES" + result="true" + while IFS= read -r f; do + [ -z "$f" ] && continue + if ! is_docs "$f"; then + echo "Non-docs path -> full suite required: $f" + result="false" + break + fi + done <<< "$FILES" + docs_only="$result" + echo "docs_only=$docs_only" + + lint: + needs: changes + # `lint` is not currently a required check, but add always() anyway so it can + # never wedge if it's promoted to required later — same rationale as the test/ + # browser-smoke jobs; a failed `changes` degrades to running the full lint. + if: ${{ always() }} + runs-on: ubuntu-latest + steps: + - name: Docs-only short-circuit + if: needs.changes.outputs.docs_only == 'true' + run: echo "Docs-only change set — skipping lint/runtime guards (nothing executable changed)." + + - uses: actions/checkout@v4 + if: needs.changes.outputs.docs_only != 'true' with: # Need history so the gate can diff against the merge-base with master. fetch-depth: 0 - name: Set up Python + if: needs.changes.outputs.docs_only != 'true' uses: actions/setup-python@v5 with: python-version: '3.12' - name: Install ruff + if: needs.changes.outputs.docs_only != 'true' run: pip install ruff - name: Ensure origin/master ref is available for the diff gate + if: needs.changes.outputs.docs_only != 'true' run: git fetch --no-tags --depth=1 origin master || true # Whole-tree Python syntax gate. The Python analog of the JS `node --check` @@ -39,13 +131,15 @@ jobs: # file:line, before the matrix suite runs. Whole-tree + unconditional so it # cannot be bypassed by the diff scope. (#4641) - name: Python syntax gate (whole tree — fails fast on any unparseable .py) + if: needs.changes.outputs.docs_only != 'true' run: python3 -m compileall -q api server.py bootstrap.py mcp_server.py tests scripts - name: Ruff forward gate (new/changed lines only) + if: needs.changes.outputs.docs_only != 'true' run: python3 scripts/ruff_lint.py --diff origin/master - name: Ruff whole-tree report (informational — never blocks) - if: always() + if: needs.changes.outputs.docs_only != 'true' && always() run: python3 scripts/ruff_lint.py --all # Static-JS runtime-error guards. These catch brick-class bugs that throw only @@ -56,20 +150,31 @@ jobs: # scope, catching a function defined nested but called from a sibling # scope (#3696 — ReferenceError: _sessionAttentionState is not defined). - name: Set up Node for ESLint runtime guards + if: needs.changes.outputs.docs_only != 'true' uses: actions/setup-node@v4 with: node-version: '20' - name: Install ESLint + if: needs.changes.outputs.docs_only != 'true' run: npm install --no-save eslint@^10 - name: ESLint runtime-error gate (no-const-assign / no-import-assign, #3162) + if: needs.changes.outputs.docs_only != 'true' run: npx eslint --no-config-lookup -c eslint.runtime-guard.config.mjs "static/**/*.js" - name: Scope / undefined-reference gate (#3696) + if: needs.changes.outputs.docs_only != 'true' run: python3 scripts/scope_undef_gate.py test: + needs: changes + # Belt-and-suspenders: run even if `changes` errored/was skipped. A required job + # that gets SKIPPED reports its context as pending forever (wedge). With always(), + # the job runs regardless; the per-step `if: … != 'true'` guards then treat a + # missing/empty docs_only (which is what a failed `changes` yields) as "run full + # CI" — so a detector failure degrades to running everything, never to a wedge. + if: ${{ always() }} runs-on: ubuntu-latest strategy: # Don't cancel the other shards/versions when one fails — we want the full @@ -87,9 +192,15 @@ jobs: shard: [0, 1, 2, 3, 4] steps: + - name: Docs-only short-circuit + if: needs.changes.outputs.docs_only == 'true' + run: echo "Docs-only change set — skipping pytest shard ${{ matrix.shard }} (no executable change)." + - uses: actions/checkout@v4 + if: needs.changes.outputs.docs_only != 'true' - name: Set up Python ${{ matrix.python-version }} + if: needs.changes.outputs.docs_only != 'true' uses: actions/setup-python@v5 with: python-version: ${{ matrix.python-version }} @@ -100,6 +211,7 @@ jobs: **/pyproject.toml - name: Install dependencies + if: needs.changes.outputs.docs_only != 'true' run: | python -m pip install --upgrade pip # Office parsers stay optional at runtime; CI installs them explicitly for the workspace preview tests. @@ -118,10 +230,12 @@ jobs: pip install mcp || echo "mcp install failed — test_mcp_server.py will importorskip" - name: Get Playwright version + if: needs.changes.outputs.docs_only != 'true' id: pw-version run: echo "version=$(pip show playwright | grep '^Version:' | cut -d' ' -f2)" >> "$GITHUB_OUTPUT" - name: Cache Playwright browsers + if: needs.changes.outputs.docs_only != 'true' uses: actions/cache@v4 with: path: ~/.cache/ms-playwright @@ -130,7 +244,9 @@ jobs: ${{ runner.os }}-playwright- - name: Install Playwright browsers + if: needs.changes.outputs.docs_only != 'true' run: python -m playwright install --with-deps chromium - name: Run tests (shard ${{ matrix.shard }} of 5) + if: needs.changes.outputs.docs_only != 'true' run: pytest tests/ -v --timeout=60 --shard-id=${{ matrix.shard }} --num-shards=5 diff --git a/scripts/critical_markdown_check.py b/scripts/critical_markdown_check.py new file mode 100644 index 00000000..8e65c97c --- /dev/null +++ b/scripts/critical_markdown_check.py @@ -0,0 +1,224 @@ +#!/usr/bin/env python3 +"""critical_markdown_check.py — catch ONLY catastrophic Markdown rendering breaks. + +Deliberately minimal (Nathan, 2026-07-18): "only pretty critical, borderline +catastrophic Markdown issues, like there is a new line in a link that would prevent +the link from rendering." This is NOT a style linter and NOT a prose scanner. It +flags ONLY two constructs that genuinely break how the document renders, verified +against the CommonMark reference parser (markdown-it-py), and which do not occur in +normal prose: + + 1. A newline INSIDE an inline-link destination TOKEN — `[label](https://exa\\nmple.com)`. + The destination is a single token; a newline splitting it (non-space, newline, + non-space, before any closing `)`) makes CommonMark NOT render the link. + IMPORTANT: a newline in the *whitespace* around the destination — + `[label](\\nhttps://x)`, `[label](https://x\\n)`, or inside a `"title"` — is + LEGAL CommonMark and renders fine, so those are NOT flagged (verified). + 2. A single-line inline link/image whose destination is never closed — + `[label](https://x` with no `)` before end of line. Renders broken. + +Code spans (fenced ``` / ~~~ blocks, 4-space indented code, and inline `code`) are +blanked first so an example of "bad" Markdown shown inside code is never flagged. + +Exit 1 only on one of the two defects above. Anything ambiguous is NOT flagged — a +false positive on a docs PR is worse than missing a cosmetic issue. Dead external +links are handled separately by the lychee step, not here. +""" +from __future__ import annotations +import re +import sys +from pathlib import Path + + +def _blank_code(text: str) -> str: + """Blank fenced blocks, 4-space indented code, and inline code — keep line count.""" + out = [] + in_fence = False + fence_marker = "" + for line in text.split("\n"): + # Fence open/close. A closing fence must be at least as long and same char. + m = re.match(r"^(\s*)(`{3,}|~{3,})", line) + if not in_fence and m: + in_fence = True + fence_marker = m.group(2)[0] * len(m.group(2)) + out.append("") + continue + if in_fence: + cm = re.match(r"^\s*(`{3,}|~{3,})", line) + if cm and cm.group(1)[0] == fence_marker[0] and len(cm.group(1)) >= len(fence_marker): + in_fence = False + out.append("") + continue + # 4-space (or tab) indented code line → blank it. + if re.match(r"^(?: {4}|\t)", line): + out.append("") + continue + # inline `code` (including multi-backtick) → blank spans on this line. + out.append(re.sub(r"(`+)(?:.*?)\1", lambda mm: " " * len(mm.group(0)), line)) + return "\n".join(out) + + +def _scan_inline_dest(text: str, i: int) -> str: + """Given the index right after a link/image `[label](`, decide whether the inline + destination renders. Models the CommonMark inline-link grammar closely enough to + catch the two catastrophic shapes without false positives: + after '(': [ws] destination [ws title] [ws] ')' + where destination is either `<...>` (angle, no raw newline) or a bare run of + non-whitespace with BALANCED parens, and title is "...", '...', or (...) (may span + newlines). Returns 'ok', 'split' (a newline breaks the destination token), or + 'unclosed' (no closing ')'). + """ + n = len(text) + + def skip_ws(j: int) -> int: + while j < n and text[j] in " \t\n": + j += 1 + return j + + j = skip_ws(i) # leading ws (incl newline) before dest is legal + if j >= n: + return "unclosed" + + # Angle-bracket destination <...> — a raw newline inside it is invalid, but a + # backslash-escaped '>' does NOT terminate it. Scan honoring escapes. + if text[j] == "<": + k = j + 1 + while k < n and text[k] not in ">\n": + if text[k] == "\\" and k + 1 < n: + # An escaped char doesn't terminate the angle dest — but an escaped + # NEWLINE is still a raw newline inside <...>, which is invalid. + if text[k + 1] == "\n": + return "split" + k += 2 + else: + k += 1 + if k >= n: + return "unclosed" + if text[k] == "\n": + return "split" + j = k + 1 + else: + # Bare destination: run of non-whitespace with balanced parens. It ends at the + # first whitespace, or at the ')' that closes the link (depth 0). + depth = 0 + while j < n: + c = text[j] + if c == "\\" and j + 1 < n: + # An escaped char doesn't end the destination — but an escaped NEWLINE + # is still a raw line ending, which GFM/CommonMark forbid in a bare + # destination (GitHub's cmark-gfm won't render it). Treat as split. + if text[j + 1] == "\n": + return "split" + j += 2 + continue + if c in " \t\n": + break # whitespace terminates the destination token + if c == "(": + depth += 1 + elif c == ")": + if depth == 0: + return "ok" # this ')' closes the link — complete + depth -= 1 + j += 1 + else: + return "unclosed" # ran off the end with no closing ')' + # Whitespace ended the bare destination. A bare dest must have BALANCED parens; + # if a '(' is still open, the destination is malformed and won't render. + if depth > 0: + return "split" + + # Destination token ended at whitespace. Skip it (a single newline here is legal), + # then the next non-whitespace must be the close ')' or a title opener. + j = skip_ws(j) + if j >= n: + return "unclosed" + c = text[j] + if c == ")": + return "ok" + if c in "\"'(": + # A title is present. It ends at its matching delimiter — `"..."`, `'...'`, or + # a balanced `(...)`. Titles may span newlines. After the title, whitespace is + # allowed and then the link's OWN closing ')' MUST follow; otherwise the link + # is unclosed (e.g. `[x](foo (title)` — the sole ')' closes the title, not the + # link). Walk the title body honoring escapes. + openc = c + k = j + 1 + if openc == "(": + # A parenthesized title must NOT contain an unescaped '(' — CommonMark + # forbids it, so `(a (b) c)` is not a valid title and the link won't render. + while k < n: + ch = text[k] + if ch == "\\" and k + 1 < n: + k += 2 + continue + if ch == "(": + return "unclosed" # nested unescaped '(' → invalid title + if ch == ")": + break + k += 1 + if k >= n: + return "unclosed" # parenthesized title never closed + else: + closec = openc # matching " or ' + while k < n and text[k] != closec: + if text[k] == "\\" and k + 1 < n: + k += 2 + else: + k += 1 + if k >= n: + return "unclosed" # quoted title never closed + # k is at the title's closing delimiter. After it: optional ws then the link ')'. + k = skip_ws(k + 1) + if k < n and text[k] == ")": + return "ok" + return "unclosed" + # Anything else after the destination + whitespace is bare text → the inline link is + # malformed and renders as literal characters. This is the catastrophic split, e.g. + # `[x](https://exa\nmple.com)` or `[x](url\nmore)`. + return "split" + + +def check_file(path: Path) -> list[str]: + problems: list[str] = [] + raw = path.read_text(encoding="utf-8", errors="replace") + text = _blank_code(raw) + + # Locate each inline link/image opener `[label](` (label has no unescaped ] or newline). + for m in re.finditer(r"!?\[(?:[^\]\n\\]|\\.)*\]\(", text): + line_no = text.count("\n", 0, m.start()) + 1 + verdict = _scan_inline_dest(text, m.end()) + if verdict == "split": + problems.append( + f"{path}:{line_no}: newline inside a link destination — the link will not render") + elif verdict == "unclosed": + problems.append( + f"{path}:{line_no}: link/image destination never closed with ')' — renders broken") + return problems + + +def main(argv: list[str]) -> int: + files = [Path(a) for a in argv[1:] if a.strip()] + if not files: + print("critical_markdown_check: no files to check (ok)") + return 0 + all_problems: list[str] = [] + scanned = 0 + for f in files: + if not f.is_file(): + continue + scanned += 1 + try: + all_problems.extend(check_file(f)) + except Exception as e: # never crash the gate on an odd file + print(f"warning: could not scan {f}: {e!r}") + if all_problems: + print("Critical Markdown problems (fix these — they break rendering):") + for p in all_problems: + print(f" ✗ {p}") + return 1 + print(f"critical_markdown_check: {scanned} file(s) OK — no catastrophic Markdown issues") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main(sys.argv)) diff --git a/scripts/test_critical_markdown_check.py b/scripts/test_critical_markdown_check.py new file mode 100644 index 00000000..01946432 --- /dev/null +++ b/scripts/test_critical_markdown_check.py @@ -0,0 +1,135 @@ +#!/usr/bin/env python3 +"""Tests for scripts/critical_markdown_check.py — the docs-CI critical-markdown gate. + +The checker must flag ONLY catastrophic rendering breaks (a newline that splits an +inline-link destination, or an unclosed inline link) and nothing that renders fine. +Each case is asserted against the intended CommonMark behavior. Where markdown-it-py +is installed the test additionally cross-checks the checker's verdict against the +reference parser; when it isn't, the hand-labeled expectations still run. + +Run: pytest scripts/test_critical_markdown_check.py -v +""" +import importlib.util +import tempfile +from pathlib import Path + +import pytest + +_spec = importlib.util.spec_from_file_location( + "critical_markdown_check", + str(Path(__file__).with_name("critical_markdown_check.py")), +) +cmc = importlib.util.module_from_spec(_spec) +_spec.loader.exec_module(cmc) + +try: + from markdown_it import MarkdownIt + _MD = MarkdownIt("commonmark") +except Exception: # optional; hand labels still assert + _MD = None + + +def _flags(src: str) -> bool: + with tempfile.NamedTemporaryFile("w", suffix=".md", delete=False) as tf: + tf.write(src) + p = Path(tf.name) + try: + return len(cmc.check_file(p)) > 0 + finally: + p.unlink() + + +def _renders(src: str) -> bool: + html = _MD.render(src) + return ")", False), + ("angle-escaped-gt", "[x](bar>)", False), + ("dq-title-inline", '[x](url "title")', False), + ("paren-title-closed", "[x](foo (title))", False), + ("dq-title-then-close", '[x](foo "title")', False), + ("empty-dest", "[x]()", False), + ("plain-good", "[l](https://x.com)", False), + ("two-links-line", "[a](x) and [b](y)\n", False), + # catastrophic breaks — MUST flag (do not render as links) + ("newline-inside-dest", "[l](https://ex\nam.com)", True), + ("img-newline-inside", "![a](img\nf.png)", True), + ("bare-text-after-nl", "[x](url\nmore)", True), + ("quote-glued-in-url", '[x](exa"part\nmple.com)', True), + ("bare-unbalanced-paren", "[x](foo(\n))", True), + ("unclosed-at-eof", "[x](https://a", True), + ("unclosed-newline-term", "[x](https://a\n", True), + ("angle-dest-split", "[x]()", True), + ("angle-escaped-newline", "[x]()", True), + ("paren-title-unclosed", "[x](foo (title)", True), + ("angle-title-unclosed", "[x]( (title)", True), + ("dq-title-unclosed", '[x](foo "title"', True), + ("nested-paren-title", "[x](foo (a (b) c))", True), +] + +# Bad markdown INSIDE code must never be flagged (it's an intentional example). +CODE_SAFE = [ + ("fenced-example", "```\n[x](\ny)\n```\n"), + ("indented-example", "text\n\n [x](\n y)\n"), + ("inline-code", "Use `[x](` in prose\n"), +] + +# Cases where the FORMAL CommonMark/GFM grammar (what GitHub's cmark-gfm renders) +# disagrees with markdown-it-py, which is permissive. We follow the formal spec / +# GitHub behavior (these docs are GitHub-rendered), so these are verdict-asserted but +# excluded from the markdown-it-py cross-check. +SPEC_DIVERGENT = [ + # An escaped newline inside a bare destination is a raw line ending — GFM forbids + # it (GitHub won't render the link) though markdown-it-py permissively renders it. + ("bare-escaped-newline", "[x](foo\\\nbar)", True), +] + + +@pytest.mark.parametrize("name,src,should_flag", CASES, ids=[c[0] for c in CASES]) +def test_checker_verdict(name, src, should_flag): + assert _flags(src) is should_flag + + +@pytest.mark.parametrize("name,src,should_flag", SPEC_DIVERGENT, + ids=[c[0] for c in SPEC_DIVERGENT]) +def test_spec_divergent_verdict(name, src, should_flag): + # Follow the formal GFM grammar / GitHub rendering, not markdown-it-py's permissive + # behavior — so these are asserted by hand, not cross-checked against the parser. + assert _flags(src) is should_flag + + +@pytest.mark.parametrize("name,src,should_flag", CASES, ids=[c[0] for c in CASES]) +def test_agrees_with_commonmark(name, src, should_flag): + if _MD is None: + pytest.skip("markdown-it-py not installed") + # The checker should flag exactly the cases that do NOT render as a link/image. + assert _flags(src) is (not _renders(src)) + + +@pytest.mark.parametrize("name,src", CODE_SAFE, ids=[c[0] for c in CODE_SAFE]) +def test_code_spans_never_flagged(name, src): + assert _flags(src) is False + + +def test_empty_and_missing_inputs(): + # No files -> ok (exit 0 path). + assert cmc.main(["prog"]) == 0 + # A file with no links -> no problems. + with tempfile.NamedTemporaryFile("w", suffix=".md", delete=False) as tf: + tf.write("# Title\n\nJust prose, no links.\n") + p = Path(tf.name) + try: + assert cmc.check_file(p) == [] + finally: + p.unlink()