[NIT] The comment on ValidateDocPath says "Defense-in-depth: the VCS API should already scope paths to the repo" — this is accurate for the VCS-fetch path, but ValidateDocPath is now also used by the local-filesystem stale-docs check where the VCS API is not involved. The comment is still broadly correct but could be generalized: "Defense-in-depth: callers should also confine the joined path to the repo root via filepath.Rel before filesystem access."
[NIT] TODO.md is committed in its pre-PR-submission state ("Next Steps: create PR via gitea-rodin token"). This file appears to be a dev-loop artifact rather than project documentation. Committing intermediate dev-loop state into the main branch adds noise; consider either removing this file from version control or updating it post-merge.
[MINOR] The stale-docs check calls filepath.EvalSymlinks(absRoot) on the --repo-root path, which will fail with an error if the directory does not yet exist (e.g. a new repo checkout where the root hasn't been created). In practice --repo-root defaults to . (cwd) which always exists, so this is low-risk, but an explicit os.Stat check before EvalSymlinks would give a clearer error message when the root is genuinely missing vs. a symlink resolution failure.
[NIT] TestCheckStaleDocs_PathTraversal constructs the YAML by concatenating tc.docPath directly into the string literal. A path like /etc/passwd happens to be valid YAML, but paths containing YAML-special characters (e.g. :, {, #) could produce a parse error (exit 2) rather than the expected stale-doc error (exit 1). Adding a YAML quoting step or restricting the test vectors would make the test more robust.
[MINOR] The error message for stale docs reads "ERROR: stale docmap docs: entries (paths do not exist):" — the docs: YAML key embedded in the prose makes it slightly awkward to read ("stale docmap docs: entries"). Consider "ERROR: stale docs: entries in docmap do not exist on disk:" for clarity.
[MINOR] The EvalSymlinks call on --repo-root (line ~91) will fail with an error if the directory does not yet exist, which gives a confusing error message at startup rather than a clear 'directory not found' message. The error message just says 'failed to resolve --repo-root' without distinguishing a non-existent directory from a permission error. Not a functional bug, but a UX rough edge.
[MINOR] The coverage check reports all uncovered files even when the docmap has zero mappings (empty config). This is technically correct behaviour, but an empty docmap + any changed files will fail with a coverage error. There is no documented special case for an empty docmap meaning 'coverage not required'. This is a design choice but could surprise users who have the --docmap flag set but an empty file. Consider documenting this in the usage text or treating an empty docmap as 'coverage not enforced'.
[NIT] stdinValidateDocmap defers f.Close() but the file is opened read-write and os.Stdin is set to f. The defer will close the file descriptor while it may still be in use by runValidateDocmap if Go's test runner parallelises subtests. In practice this is safe because the function runs synchronously and the defer fires after the return, but it is slightly fragile. The existing validateurl pattern likely has the same structure so this is not a regression, just a note.
[NIT] The comment on ValidateDocPath says 'Backslashes are rejected explicitly to prevent Windows platform edge cases.' — the sentence could be clearer that the tool itself may run on Windows and that backslashes in YAML doc paths could be misinterpreted by filepath.Join on that platform. Minor documentation clarity.
[MINOR] readLines reads from os.Stdin directly rather than accepting an io.Reader parameter. This forces tests to swap os.Stdin (which is fragile and not goroutine-safe), whereas accepting io.Reader would let callers pass any reader. The existing test helper works around this by redirecting os.Stdin, but the function signature itself is less testable than it could be. Compare with validateurl.go pattern — if that also uses os.Stdin directly this is consistent, but it's still worth noting.
[NIT] stdinValidateDocmap leaks the temp file handle — it calls defer f.Close() but the file is passed to os.Stdin before the function returns. Because os.Stdin is restored in the deferred restore and runValidateDocmap reads synchronously, this works in practice, but a reader of the test may be confused. A comment explaining the ordering would help.
[NIT] The error message 'ERROR: stale docmap docs: entries (paths do not exist):' has an awkward colon after 'docs:' — this reads as if 'docs:' is YAML syntax leaking into the message. Consider 'ERROR: stale docmap entries (docs: paths that do not exist on disk):' or simply 'ERROR: docs: entries not found on disk:'.
[NIT] The flag is named --docmap (no hyphen between doc and map) but the README, function name, and error messages consistently spell it doc-map. Minor inconsistency — --doc-map would match the established naming convention of --doc-map-max-bytes and the config file name doc-map.yml.
[NIT] The captureOutput helper mutates package-level outWriter/errWriter variables without any synchronisation. This is fine for sequential tests but would race if tests run in parallel (t.Parallel()). No parallel calls exist currently, but a comment noting this limitation would be defensive documentation.
[MINOR] The stale-docs check (checkStaleDocs) runs even when no changed files were provided on stdin (empty stdin case). This means a docmap with stale docs: entries will always fail, even on runs where no files changed — which is arguably the correct behavior, but worth a comment clarifying the intent. Currently the empty-stdin test passes because the test fixture has the doc file present, so it's fine, but the behavior asymmetry (coverage check is vacuously true on empty input, stale-docs check is not) is undocumented.
[NIT] TestRunValidateDocmap_Clean has a long comment block explaining why it relies on process-level stdin instead of the stdinValidateDocmap helper, which is defined later in the same file. The test would be cleaner and more consistent if it simply used stdinValidateDocmap with an empty string — which is exactly what TestRunValidateDocmap_EmptyStdin does. These two tests appear to be testing the same case (empty stdin → clean). Consider removing TestRunValidateDocmap_Clean or consolidating it with TestRunValidateDocmap_EmptyStdin.