[NIT] Rule 10 states 'If already assigned to aweiker: skip.' — it's unclear whether this means skip the HANDOFF emission only, or skip applying the ready label too. The spec would benefit from a clarifying sentence, e.g. 'If the PR is already assigned to aweiker, assume handoff was already performed and continue to the next PR without emitting another HANDOFF.'
[NIT] Section 9 references scripts/check-deps.sh as the tool that verifies invariant S1, but the script is described as test/check-invariants.sh in Section 6 and throughout SKILL.md. This appears to be a stale reference — should be test/check-invariants.sh.
[NIT] The dispatch rules table has two rows labeled '10' — one for 'All checks pass → HANDOFF' and one for 'No open PRs + no ACTIVE_WIP → SPAWN:impl (next issue)'. The second should be labeled '11' to match the spec in docs/dev-loop-spec.md which correctly uses Rule 11 for new issue pickup.
[NIT] The usage message prints flag help manually (fmt.Fprintln chains) rather than calling fs.Usage(). This is inconsistent with how flag.FlagSet normally surfaces help and could get out of sync with the registered flags. Minor since this is a short flag set, but worth noting.
[NIT] The comment on stdinValidateDocmap says 'Tests must not call t.Parallel() while sharing the global os.Stdin' but there is no t.Skip or assertion to enforce this. Since os.Stdin mutation is inherently process-global, this is a latent footgun if someone later adds t.Parallel() to these tests. Consider adding a brief comment directing future maintainers to use TestMain to serialize tests instead, or document that this constraint is enforced by convention.
[NIT] The maxDocmapBytes constant is typed as int64 solely to compare against fi.Size() (which returns int64). Consider using an untyped constant or keeping it typed but adding a brief comment explaining why int64 was chosen, since the type is not otherwise obvious from context.
[NIT] The comment on ValidateDocPath says 'Defense-in-depth: callers must also confine the joined path to the repo root via filepath.Rel before any filesystem access.' This is good documentation but the function's contract doesn't verify the caller satisfies that postcondition. This is fine (it's defense-in-depth by definition), just making sure the comment is understood as advisory rather than enforced.
[NIT] In TestCheckStaleDocs_PathTraversal, the inner table test creates a docmap using string concatenation with the docPath directly embedded in YAML (- +tc.docPath+``). For paths like /etc/passwd and ../../etc/passwd, this creates valid YAML. However, for paths containing special YAML characters (:, #, {), this could produce invalid YAML that would fail at parse time rather than at the stale-docs check, making the test misleading. The current test cases are safe, but using fmt.Sprintf with YAML quoting or writing the file programmatically would be more robust.
[MINOR] The readLines function is defined in validatedocmap.go but is a generic utility. If another subcommand file ever needs to read lines from stdin, there will be a naming conflict since both would be in package main. This is fine for now but worth noting — consider a brief comment that this is intentionally scoped to the validate-docmap use case, or move it to a shared helpers file if reuse is anticipated.
[NIT] The validateDocmapPath function documents that resolvedRoot must be an absolute, symlink-free path, but this is only enforced by convention (the caller must pass the right value). An internal assertion or a brief check if !filepath.IsAbs(resolvedRoot) at the top of the function would make the contract self-enforcing. Low priority since the caller in runValidateDocmap does this correctly.
[NIT] The comment on ValidateDocPath mentions "Finding #3" which appears to be a reference to an internal issue/finding number that won't be meaningful to future readers of this file. This should be reworded to be self-contained (e.g., remove the parenthetical or rephrase as 'to prevent OS-specific path separator normalization issues').
[MINOR] The comment on stdinValidateDocmap says "Tests must not call t.Parallel() while sharing the global os.Stdin" — this is a valid concern, but none of the tests call t.Parallel() anyway, so the guard is informational only. The real risk is if a future contributor adds t.Parallel() without reading this comment. Consider adding a // WARNING: prefix or an explicit // do not add t.Parallel() to callers note to make it more prominent.
[NIT] The usage message emitted via multiple fmt.Fprintln(errWriter, "") calls (blank line) could use a single fmt.Fprintf with \n\n for readability. Minor style issue only.
[NIT] FileCoveredByDocMap doc comment says 'It is used by static validation tooling (e.g. the validate-docmap subcommand)'. Per the documentation pattern, the doc comment should start with the function name: // FileCoveredByDocMap reports whether... — the current phrasing already does start correctly but the second sentence adds implementation context that per the patterns belongs in a separate paragraph or omitted from the primary sentence. Very minor.
[MINOR] The comment on stdinValidateDocmap says "Tests must not call t.Parallel() while sharing the global os.Stdin" but this constraint is not enforced (e.g. via a test-level mutex or a serial subtest runner). The tests currently do not call t.Parallel(), so this is safe today, but the constraint is easy to violate silently when new tests are added. Consider documenting this with a t.Setenv-style workaround or a note that tests in this file must remain serial.
[MINOR] The stale-docs check runs even when --repo-root resolution fails, but only coverage failures are accumulated before the repo-root resolution block. If filepath.Abs or filepath.EvalSymlinks fails the function returns exit code 2 immediately — meaning a coverage failure that was already accumulated (and stored in failed = true) is silently swallowed, and the user sees exit 2 with only the repo-root error rather than exit 1 with both failures. Consider either: (a) emitting the accumulated coverage failures before returning 2, or (b) resolving repo-root before the coverage check so all errors are consistently collected.