Skip to content

Add local md-link-checker agent and dedupe link-checking rules#16131

Merged
azat-msft merged 9 commits into
mainfrom
dev/azatm/link-checker-agent
Jun 19, 2026
Merged

Add local md-link-checker agent and dedupe link-checking rules#16131
azat-msft merged 9 commits into
mainfrom
dev/azatm/link-checker-agent

Conversation

@azat-msft

@azat-msft azat-msft commented Jun 18, 2026

Copy link
Copy Markdown
Member

Summary

Adds a repo-specific @md-link-checker agent that runs the same documentation link-checking and fixing logic locally. Today that logic only runs in the weekly md-link-checker pipeline, so a contributor who edits a .md file has to push and wait for CI to learn whether a relative link or heading anchor is broken. With this agent they can validate (and auto-fix) changed docs on their machine first.

It also removes duplicated instructions and logic: the link extraction/testing bash is now a single shared script, and the agent is the single source of the fixing rules that the pipeline workflow delegates to.

What changed

  • .github/workflows/scripts/check-md-links.py (new) — the one shared implementation of link extraction + testing, factored out of the workflow's previously-inlined ~130-line bash block. It accepts an explicit file list (fast local runs over changed docs) or, with no args, scans the pipeline default scope (docs/ + README.md). It writes broken-links.md and link-check-results.md under OUT_DIR (default /tmp/gh-aw/agent) and prints a working/broken summary.
  • .github/agents/md-link-checker.md (new) — canonical, authoritative rules for checking and fixing broken relative .md links and heading anchors. The agent runs the shared script to produce the broken-links list (so it genuinely extracts links, not just describes them), then applies the fix rules. Two modes:
    • Local (default) — picks changed/untracked docs, runs the script, fixes in-place, and reports to the console. No PR/issue/commit.
    • Delegated — consumes the pipeline's broken-links.md (already produced by the same script) and only applies the fix rules.
    • Rules: scope is relative .md links + heading anchors only (absolute URLs ignored); GitHub heading-slug + <a name/id> anchor matching; strict ≤2-grep-per-anchor token budget, never read a whole target file.
  • .github/workflows/md-link-checker.md — the bash step now calls check-md-links.py instead of the inlined block; Step 3 delegates fixing to @md-link-checker rather than restating the rules.
  • .github/workflows/md-link-checker.lock.yml — recompiled so the workflow invokes the shared script.

Pattern followed

This mirrors the existing pr-expert-reviewer workflow, which delegates its deep review rules to the @expert-reviewer agent rather than duplicating them. The result: the link-checking logic lives in exactly one place (the shared script + the agent) and is reused both locally and by CI.

Notes

  • Docs-tooling only; no source/product code is touched.
  • Incidental gh-aw compiler-version churn (actions-lock.json, unrelated .lock.yml files, generated maintenance workflow) was intentionally excluded to keep the change scoped.

Introduce a repo-specific `@md-link-checker` agent that runs the same
documentation link-checking and fixing logic locally, so contributors can
validate changed .md files before pushing instead of waiting for the
weekly pipeline.

To avoid duplicating instructions, the agent is now the single source of
the checking/fixing rules (scope, GitHub anchor-slug matching, the
two-grep-per-anchor token budget, and the fix logic). The md-link-checker
workflow delegates its fixing step to the agent rather than restating the
rules, mirroring how pr-expert-reviewer delegates to @expert-reviewer.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 18, 2026 14:03

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces a repo-specific @md-link-checker agent intended to centralize the markdown relative-link/anchor fixing rules and enable running the same logic locally, while updating the existing weekly md-link-checker workflow to delegate its fixing step to that agent.

Changes:

  • Added a new .github/agents/md-link-checker.md agent that defines the canonical link/anchor checking + fixing rules and supports “local” and “delegated” modes.
  • Updated .github/workflows/md-link-checker.md to defer the fixing logic to @md-link-checker rather than duplicating instructions in the workflow prompt.
  • Regenerated .github/workflows/md-link-checker.lock.yml to reflect the workflow body change.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
.github/workflows/md-link-checker.md Rewords the workflow prompt to delegate fixing to @md-link-checker and avoid duplicating fix instructions.
.github/workflows/md-link-checker.lock.yml Updates the compiled lock metadata (body_hash) after the workflow prompt change.
.github/agents/md-link-checker.md Adds the new canonical agent prompt defining link/anchor scope and fix behavior for local + delegated runs.

Comment thread .github/workflows/md-link-checker.md Outdated
Comment thread .github/agents/md-link-checker.md
Comment thread .github/agents/md-link-checker.md Outdated
azat-msft and others added 2 commits June 18, 2026 16:27
Move the markdown link extraction/testing bash (previously inlined in the
md-link-checker workflow) into a single reusable script,
.github/workflows/scripts/check-md-links.sh. Both consumers now run it:

- the workflow step calls the script instead of an inline 130-line block;
- the local @md-link-checker agent runs the same script to produce the
  broken-links list, so it actually extracts links rather than only
  describing the rules.

The script accepts an explicit file list (fast local runs over changed
docs) or defaults to the pipeline scope (docs/ + README.md), and writes
broken-links.md / link-check-results.md under OUT_DIR.

Recompiled md-link-checker.lock.yml so the workflow invokes the script.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 18, 2026 15:19

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

Comment thread .github/workflows/scripts/check-md-links.sh Outdated
Comment thread .github/workflows/scripts/check-md-links.sh Outdated
Comment thread .github/workflows/scripts/check-md-links.sh Outdated
Comment thread .github/agents/md-link-checker.md
azat-msft and others added 2 commits June 18, 2026 17:24
The heading-extraction command was switched to the portable
sed -nE form, but a second reference still said 'two-grep token
budget'. Make the wording consistent.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace the bash check-md-links.sh with a pure-Python check-md-links.py
so the shared link checker runs natively on Windows, macOS, and Linux
(and on the ubuntu-latest pipeline runner) without requiring bash,
GNU grep -P, or GNU realpath.

The port reproduces the original behavior exactly: markdown link
extraction, the GitHub heading-slug algorithm, <a name/id> anchors,
absolute-URL skipping, and realpath --relative-to semantics (including
its parent-missing fallback). Verified byte-for-byte identical output to
the previous bash script over the full docs scope, scoped file lists,
HTML-anchor files, the no-files early exit, and GITHUB_OUTPUT.

- workflow step now runs: python3 .github/workflows/scripts/check-md-links.py
  (gh aw compile added a Setup Python step in the lock)
- @md-link-checker agent doc updated to invoke the Python script, with a
  PowerShell note for Windows.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

Comment thread .github/workflows/scripts/check-md-links.py
Comment thread .github/workflows/scripts/check-md-links.py
Comment thread .github/workflows/scripts/check-md-links.py Outdated
Comment thread .github/workflows/scripts/check-md-links.py
Comment thread .github/workflows/md-link-checker.lock.yml

@nohwnd nohwnd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Description] Script extension mismatch: .sh.py

The PR description refers to the new shared script as check-md-links.sh (a bash script), but the actual file added in this PR is check-md-links.py (a Python script). The agent definition itself correctly names it check-md-links.py and says "It is plain Python (no bash), so it works the same on Windows, macOS, and Linux" — the description was apparently not updated after the bash → Python refactor.

Specific stale references in the description:

  1. Bullet point header**`.github/workflows/scripts/check-md-links.sh`** (new) should be check-md-links.py.
  2. Notes section"The shared script is enforced to LF via .gitattributes (*.sh text eol=lf)" — no .gitattributes change is in the diff, and even if there were, the *.sh glob would not match a .py file.

No vstest product code is touched by this PR; all changes are .github/-scoped tooling infrastructure. The Python script itself is a faithful port of the original bash logic and looks algorithmically correct.


🧠 Reviewed by pr-expert-reviewer

🧠 Reviewed by Expert Code Reviewer 🧠

check-md-links.py:
- Restrict relative-link checking to .md targets, matching the documented
  scope (non-.md targets like images and source files are skipped).
- Emit broken_count/working_count to GITHUB_OUTPUT on the no-files and
  no-links early-exit paths so downstream steps always see defined values.
- Remove an unreachable rel_path=="" && anchor branch (same-file anchors
  are already handled earlier).
- Clear all-links.txt/unique-links.txt at startup so stale intermediates
  from a prior run sharing OUT_DIR cannot be mistaken as current.

Docs:
- md-link-checker.md (workflow): clarify that extraction/testing is done
  by the shared script and only the fixing rules are delegated to the agent.
- md-link-checker.md (agent): document the expected broken-links.md format
  (header, line-per-broken-link prefixed with the cross mark, summary) for
  delegated mode.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

@nohwnd nohwnd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: docs-tooling changes only — no vstest product code affected

All four changed files are under .github/. No vstest source, test, or packaging code is touched, so IPC compatibility, binding redirects, TFM guards, and the other vstest-specific dimensions do not apply.

New finding since the last review

Setup Python ordering in md-link-checker.lock.yml (inline comment above, line 427)

The generated lock file inserts the Setup Python step after the link-check step that calls python3. The link-check therefore runs on the runner's pre-installed Python, not the pinned 3.12. It happens to work today because ubuntu-latest ships Python 3.12, but the setup step is semantically dead. The fix is to add an explicit setup-python step before link-check in the source .github/workflows/md-link-checker.md and recompile.

Carried from the previous review (still open)

[Description] Script extension mismatch — the PR description still references check-md-links.sh (the old bash name) and mentions .gitattributes (*.sh text eol=lf) enforcement, neither of which matches the actual diff. The script is check-md-links.py and no .gitattributes change is included. Worth fixing before the PR comes out of draft.

What looks good

The Python script is a faithful port of the original bash logic: slugify() correctly reimplements the sed -E/tr/sed/sed pipeline; check_anchor() correctly matches both HTML <a name/id> attributes and GitHub heading slugs; link extraction via _LINK_RE + _PAREN_RE reproduces the multi-paren grep behaviour; and _SCHEME_RE.match() correctly anchors at the start (Python's re.match anchors by default, equivalent to the bash =~^... check).


🧠 Reviewed by Expert Code Reviewer 🧠

🧠 Reviewed by Expert Code Reviewer 🧠

Comment thread .github/workflows/md-link-checker.lock.yml
Copilot AI review requested due to automatic review settings June 18, 2026 18:00

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

Comment thread .github/workflows/md-link-checker.lock.yml
Comment thread .github/agents/md-link-checker.md

@nohwnd nohwnd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: two open items remain after "resolving comments" commit

All four changed files remain in .github/ — no vstest source, test, or packaging code is touched, so IPC compatibility, binding redirects, TFM guards, and other vstest-specific dimensions do not apply.

The latest commit (18:00 UTC, "resolving comments") made progress on both previously raised items but neither is fully resolved yet.


1. Setup Python ordering — source fixed, lock not recompiled

The source .github/workflows/md-link-checker.md now correctly declares Setup Python before link-check — that's the right direction. However, the lock file was not recompiled in this commit: only 1 line was changed in md-link-checker.lock.yml, whereas a full gh aw compile regeneration would touch dozens of lines. As a result the compiled lock still has the same execution order:

# md-link-checker.lock.yml (current HEAD)
- id: link-check                              # line 422 — runs python3 first
  run: python3 .github/workflows/scripts/check-md-links.py
  shell: bash

- name: Setup Python                          # line 427 — still after link-check
  uses: actions/setup-python@...
  with:
    python-version: '3.12'

Next step: run gh aw compile from the repo root to regenerate the lock file from the corrected source. The inline comment at lock line 427 remains open and unresolved.


2. PR description — check-md-links.sh reference still present

The "What changed" bullet for md-link-checker.md still reads:

"the bash step now calls check-md-links.sh instead of the inlined block"

The actual file is check-md-links.py. This should be updated to match.


What looks clean

The Python script, agent document, and all other changes are algorithmically sound (previously confirmed). Once the lock file is regenerated and the description updated, the PR should be ready.


🧠 Reviewed by Expert Code Reviewer 🧠

🧠 Reviewed by Expert Code Reviewer 🧠

🧠 Reviewed by Expert Code Reviewer 🧠

@azat-msft

Copy link
Copy Markdown
Member Author

The step that sets up python version was removed, because either gh aw compile does not preserve the order of steps, or it pulls the step that sets up python version from other place. Either way, it does not affect the workflow, because the script can run on the pre-installed version of python.

@azat-msft azat-msft marked this pull request as ready for review June 18, 2026 21:01
Copilot AI review requested due to automatic review settings June 18, 2026 21:01

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

Comment on lines 422 to 425
- id: link-check
name: Check and test all documentation links
run: "echo \"# Link Check Results\" > /tmp/gh-aw/agent/link-check-results.md\necho \"\" >> /tmp/gh-aw/agent/link-check-results.md\necho \"# Broken Links\" > /tmp/gh-aw/agent/broken-links.md\necho \"\" >> /tmp/gh-aw/agent/broken-links.md\n\n# Find all markdown files in docs directory and README\necho \"Finding all markdown files...\"\n\nif ! find docs README.md -type f -name \"*.md\" -print0 2>/dev/null | grep -qz .; then\n echo \"No markdown files found\"\n echo \"no_files=true\" >> $GITHUB_OUTPUT\n exit 0\nfi\n\n# Extract all links from markdown files\necho \"## Links Found\" >> /tmp/gh-aw/agent/link-check-results.md\necho \"\" >> /tmp/gh-aw/agent/link-check-results.md\n\n# Use grep to find markdown links\n# Format for relative links: \"source_file|url\" to allow path resolution\nfind docs README.md -type f -name \"*.md\" -print0 2>/dev/null | while IFS= read -r -d '' file; do\n echo \"Checking $file...\"\n # Extract markdown links [text](url)\n grep -oP '\\[([^\\]]+)\\]\\(([^\\)]+)\\)' \"$file\" | grep -oP '\\(([^\\)]+)\\)' | tr -d '()' | while IFS= read -r link; do\n echo \"$file|$link\" >> /tmp/gh-aw/agent/all-links.txt\n done 2>/dev/null || true\ndone\n\n# Remove duplicates and sort\nif [ -f /tmp/gh-aw/agent/all-links.txt ]; then\n sort -u /tmp/gh-aw/agent/all-links.txt > /tmp/gh-aw/agent/unique-links.txt\n LINK_COUNT=$(wc -l < /tmp/gh-aw/agent/unique-links.txt)\n echo \"Found $LINK_COUNT unique links\" >> /tmp/gh-aw/agent/link-check-results.md\n echo \"\" >> /tmp/gh-aw/agent/link-check-results.md\nelse\n echo \"No links found\" >> /tmp/gh-aw/agent/link-check-results.md\n echo \"no_links=true\" >> $GITHUB_OUTPUT\n exit 0\nfi\n\n# Helper: check if an explicit HTML anchor or markdown heading anchor exists in a file\ncheck_anchor() {\n local file=\"$1\"\n local anchor=\"$2\"\n local html_anchor heading generated\n\n while IFS= read -r html_anchor; do\n if [[ \"$html_anchor\" == \"$anchor\" ]]; then\n return 0\n fi\n done < <(grep -oiP \"<a\\\\b[^>]*\\\\b(?:name|id)\\\\s*=\\\\s*['\\\"]\\\\K[^'\\\"]+(?=['\\\"])\" \"$file\" 2>/dev/null)\n\n while IFS= read -r heading; do\n generated=$(printf '%s' \"$heading\" | sed -E 's/<[^>]*>//g' | sed -E 's/[[:space:]]+/ /g; s/^ //; s/ $//' | tr '[:upper:]' '[:lower:]' | sed 's/ /-/g' | sed 's/[^a-z0-9_-]//g')\n if [[ \"$generated\" == \"$anchor\" ]]; then\n return 0\n fi\n done < <(grep -oP '^#{1,6}\\s+\\K.*' \"$file\" 2>/dev/null)\n\n return 1\n}\n# Test each link\necho \"## Link Test Results\" >> /tmp/gh-aw/agent/link-check-results.md\necho \"\" >> /tmp/gh-aw/agent/link-check-results.md\necho \"Testing links...\" >> /tmp/gh-aw/agent/link-check-results.md\n\nBROKEN_COUNT=0\nWORKING_COUNT=0\nwhile IFS='|' read -r source_file url; do\n if [[ \"$url\" == \"#\"* ]]; then\n # Same-file anchor link\n ANCHOR=\"${url#\\#}\"\n if check_anchor \"$source_file\" \"$ANCHOR\"; then\n WORKING_COUNT=$((WORKING_COUNT + 1))\n echo \"✅ $url (anchor in $source_file)\" >> /tmp/gh-aw/agent/link-check-results.md\n else\n BROKEN_COUNT=$((BROKEN_COUNT + 1))\n echo \"❌ $url (anchor not found in $source_file)\" >> /tmp/gh-aw/agent/link-check-results.md\n echo \"❌ $url (anchor not found in $source_file)\" >> /tmp/gh-aw/agent/broken-links.md\n fi\n elif [[ \"$url\" =~ ^[a-zA-Z][a-zA-Z0-9+.-]*: ]]; then\n # Skip absolute URLs (http, https, mailto, etc.) — we only check links to other .md files and anchors\n continue\n else\n # Relative file link, possibly with anchor\n # Split into file path and optional anchor\n REL_PATH=\"${url%%#*}\"\n ANCHOR=\"\"\n if [[ \"$url\" == *\"#\"* ]]; then\n ANCHOR=\"${url#*#}\"\n fi\n # Resolve relative to the source file's directory\n SOURCE_DIR=$(dirname \"$source_file\")\n TARGET_PATH=\"$SOURCE_DIR/$REL_PATH\"\n # Normalize the path\n TARGET_PATH=$(realpath --relative-to=. \"$TARGET_PATH\" 2>/dev/null || echo \"$TARGET_PATH\")\n if [[ -z \"$REL_PATH\" ]] && [[ -n \"$ANCHOR\" ]]; then\n # Link is just \"#anchor\" handled above, but in case of edge cases\n WORKING_COUNT=$((WORKING_COUNT + 1))\n elif [[ ! -f \"$TARGET_PATH\" ]]; then\n BROKEN_COUNT=$((BROKEN_COUNT + 1))\n echo \"❌ $url (file not found: $TARGET_PATH) in $source_file\" >> /tmp/gh-aw/agent/link-check-results.md\n echo \"❌ $url (file not found: $TARGET_PATH) in $source_file\" >> /tmp/gh-aw/agent/broken-links.md\n elif [[ -n \"$ANCHOR\" ]]; then\n # File exists, check the anchor\n if check_anchor \"$TARGET_PATH\" \"$ANCHOR\"; then\n WORKING_COUNT=$((WORKING_COUNT + 1))\n echo \"✅ $url (file + anchor OK) in $source_file\" >> /tmp/gh-aw/agent/link-check-results.md\n else\n BROKEN_COUNT=$((BROKEN_COUNT + 1))\n echo \"❌ $url (file exists but anchor '#$ANCHOR' not found in $TARGET_PATH) in $source_file\" >> /tmp/gh-aw/agent/link-check-results.md\n echo \"❌ $url (file exists but anchor '#$ANCHOR' not found in $TARGET_PATH) in $source_file\" >> /tmp/gh-aw/agent/broken-links.md\n fi\n else\n WORKING_COUNT=$((WORKING_COUNT + 1))\n echo \"✅ $url (file exists: $TARGET_PATH)\" >> /tmp/gh-aw/agent/link-check-results.md\n fi\n fi\ndone < /tmp/gh-aw/agent/unique-links.txt\n\necho \"\" >> /tmp/gh-aw/agent/link-check-results.md\necho \"**Summary:** $WORKING_COUNT working, $BROKEN_COUNT broken\" >> /tmp/gh-aw/agent/link-check-results.md\necho \"\" >> /tmp/gh-aw/agent/broken-links.md\necho \"**Summary:** $BROKEN_COUNT broken links\" >> /tmp/gh-aw/agent/broken-links.md\n# Output results\necho \"broken_count=$BROKEN_COUNT\" >> $GITHUB_OUTPUT\necho \"working_count=$WORKING_COUNT\" >> $GITHUB_OUTPUT\n\ncat /tmp/gh-aw/agent/link-check-results.md\n"
run: python3 .github/workflows/scripts/check-md-links.py
shell: bash
Comment on lines +67 to +69
**Token budget:** to find candidate headings extract only the headings with
`sed -nE 's/^#{1,6}[[:space:]]+//p' <file>` — at most two such extraction calls per broken anchor.
**Never read an entire target file** just to validate or fix an anchor.

@nohwnd nohwnd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: all previously-raised items addressed — one observation remains

All four changed files remain in .github/ — no vstest source, test, or packaging code is touched, so IPC compatibility, binding redirects, TFM guards, and other vstest-specific dimensions do not apply.

Resolved since the last review

[Description] Script extension mismatch — ✅ Fixed. The PR description now correctly references check-md-links.py throughout (including the What changed bullet for md-link-checker.md). The previous check-md-links.sh stale reference is gone.

Setup Python ordering thread — ✅ Marked resolved by author. The lock file was fully recompiled (compiler v0.77.5v0.79.8), which confirms gh aw compile ran.

Remaining observation (informational)

Setup Python ordering in the compiled lock — After the full recompile, Setup Python (actions/setup-python@v6.2.0) is still at lock line 427, after the link-check step that runs python3 at line 422. This is because the source md-link-checker.md does not declare an explicit setup-python step before link-check; the compiler injects setup-python automatically at a fixed position. The workflow functions correctly today because ubuntu-latest ships Python 3.12 pre-installed, making the setup-python step effectively a no-op in its current position. If the intent is for setup-python to actually pin the Python version (rather than rely on the runner default), the fix is to add an explicit setup-python step before link-check in the source .md and recompile. This is informational — not blocking given current runner behavior.

What looks good

The Python script, agent rules document, and workflow source are algorithmically sound (confirmed in prior reviews). The lock is now fully regenerated with the updated compiler version.


🧠 Reviewed by Expert Code Reviewer 🧠

🧠 Reviewed by Expert Code Reviewer 🧠

🧠 Reviewed by Expert Code Reviewer 🧠

@azat-msft azat-msft enabled auto-merge (squash) June 19, 2026 09:10

@Evangelink Evangelink left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expert review — automated

Overall: Comment / minor changes requested. This is a sound, net-positive refactor (dedup, cross-platform Python, removes the bash shell-injection surface). Not a blocker, but one behavioral change deserves an explicit decision.

1. MODERATE — Silent scope narrowing: broken non-.md relative links are no longer detected (parity regression)

In .github/workflows/scripts/check-md-links.py, the relative-link branch skips anything not ending in .md:

if not rel_path.lower().endswith(".md"):
    continue

The old inlined bash had no extension filter — it resolved TARGET_PATH and checked existence for every relative link. So links the weekly job used to flag are now silently skipped:

  • broken images: ![diagram](images/moved.png)
  • relative links to source/scripts: [runner](../scripts/run.sh), [sample](samples/Foo.csproj)
  • relative links to a directory or a renamed non-md asset

This may be intentional (it matches the documented "only .md files are checked" intent), but the implementation being replaced did not behave that way, and it contradicts the normalize_target docstring's "byte-for-byte" parity claim. Please confirm intent and either (a) document it explicitly in the PR description as a behavior change + soften the "byte-for-byte" wording, or (b) drop the .md-only continue and keep checking existence for all relative targets (still skipping anchor validation for non-md files).

2. LOW / INFO — Dropped no_files / no_links step outputs

The old bash emitted no_files=true / no_links=true to $GITHUB_OUTPUT in the empty cases; write_github_output() now always emits broken_count / working_count and never those two. Verified harmless today — neither the recompiled .lock.yml nor the workflow .md references steps.link-check.outputs.* (the agent consumes broken-links.md directly). Just noting it in case any external dashboard/gating keys on those outputs.

3. LOW — Windows python3 invocation

The agent doc instructs local users (incl. Windows) to run python3 .github/workflows/scripts/check-md-links.py. On Windows the interpreter is usually python or py -3 and python3 is often not on PATH. The doc otherwise gives a Windows PowerShell OUT_DIR example, so this is inconsistent with the "runs unchanged on Windows" framing. Human-facing docs only — the script needs no change.

Verified correct (no action)

slugify() (ASCII-only lowercase, ordering matches the bash pipeline), check_anchor() (heading slugs + <a name=|id=>), link extraction multi-paren quirk, start-anchored scheme detection, normalize_target realpath emulation, early-exit branches, broken-links.md format matches the agent's Delegated parser, and .lock.yml consistency (run: python3 ...check-md-links.py). No subprocess/shell-injection — a security improvement over the bash version.

Restore parity with the original bash link checker: every relative link target is checked for existence (broken images, source/script links, renamed assets), while heading-anchor validation stays scoped to in-file and .md targets. Update the agent doc scope and fix the Windows interpreter name (python/py -3 instead of python3).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

@nohwnd nohwnd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: new commit fully addresses Evangelink's review findings

All changed files remain in .github/ — no vstest source, test, or packaging code is touched. IPC compatibility, binding redirects, TFM guards, and all other vstest-specific dimensions remain inapplicable.

Addressed since Evangelink's review (commit bdf41de)

[Finding #1 resolved] Parity regression: non-.md relative links are now checked
The .md-only continue guard is gone. Every relative link target is now checked for existence via os.path.isfile(target_path) — broken images, source/script links, and renamed non-.md assets are all flagged, matching the original bash behavior. Anchor validation is correctly kept scoped to .md targets only (elif anchor != "" and rel_path.lower().endswith(".md")); anchors on non-.md targets are intentionally ignored and the link counted as working (correct — anchors in images don't apply).

[Finding #3 resolved] Windows interpreter documentation
The agent doc now reads: "on macOS/Linux the interpreter is python3; on Windows use python (or py -3)" and the PowerShell example uses python. Consistent with the "runs unchanged on Windows" framing.

Carrying from prior reviews (no change required)

[Finding #2 — harmless] Dropped no_files/no_links step outputs
The script still emits only broken_count/working_count. Verified harmless in Evangelink's review — neither the recompiled .lock.yml nor the workflow .md references steps.link-check.outputs.*.

[Informational] Setup Python ordering in compiled lock
Still injected after link-check by the compiler. Functionally correct today because ubuntu-latest ships Python 3.12. Known compiler constraint, previously documented as informational-only.

Code correctness spot-check on the new logic

  • Links to directories are correctly flagged (os.path.isfile returns False for dirs, consistent with bash -f).
  • Links with #anchor on non-.md targets (e.g. image.png#section) are checked for file existence, anchor ignored — correct.
  • The url.startswith("#") same-file anchor branch is unchanged and unreachable from the new non-.md path — no interference.
  • normalize_target path construction {source_dir}/{rel_path} is unchanged and works for all file types.

What looks good

All previously verified-correct items remain unchanged: slugify(), check_anchor(), link extraction, scheme detection, broken-links.md format, and .lock.yml consistency. All issues raised across all prior reviews are now resolved.


🧠 Reviewed by Expert Code Reviewer 🧠

🧠 Reviewed by Expert Code Reviewer 🧠

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants