Finding #4 (ACK-NOT-VALID): os.Lstat is intentionally used here for consistency with checkStaleDocs, which also uses Lstat to avoid implicit symlink-follow semantics. Switching to Stat post-EvalSymlinks would be equivalent at runtime but would create a mixed Stat/Lstat pattern in the same file that could confuse future readers. Deliberate choice, no change needed.
Finding #2 (ACK-NOT-VALID): Acknowledged. The reviewer confirms no issue — the ModeSymlink removal is correct (the check was genuinely unreachable after filepath.EvalSymlinks), the comment is accurate, and the removal is a deliberate documented choice. No action needed.
Finding #1 (ACK-NOT-VALID): Acknowledged. The reviewer explicitly notes this is a known limitation of the os.SameFile pattern and that "no action is required" — the code comment already labels it defense-in-depth. The narrow hardlink-swap window is a theoretical limitation of this approach, not a defect introduced here. No change needed.
Finding #1 (ACK-NOT-VALID): Acknowledged. The reviewer explicitly notes this is a known limitation of the os.SameFile pattern with no action required — the code comment already labels it defense-in-depth. The narrow hardlink-swap race is a theoretical limitation of the approach, not a defect introduced here. No code change needed.
Finding #5 (ACK-NOT-VALID): User-facing error messages intentionally reference *docmapFlag (the original --docmap value) rather than the resolved path. Showing the resolved path in errors would confuse users who passed a symlink — they would see a path they never specified. Using the flag value is the correct UX. The resolved path is used internally for all I/O; the flag value is only used in error messages.
Finding #4 (ACK-NOT-VALID): os.Lstat is intentionally used here for consistency with checkStaleDocs, which also uses Lstat to avoid implicit symlink-follow semantics. Switching to Stat post-EvalSymlinks would be equivalent at runtime but would create a mixed Stat/Lstat pattern in the same file that could confuse future readers. The deliberate choice is noted in the comment.
Finding #2 (ACK-NOT-VALID): Acknowledged. The reviewer confirms no issue — the ModeSymlink removal is correct (the check was genuinely unreachable after filepath.EvalSymlinks), the comment is accurate, and the removal is a deliberate documented choice. The comment phrasing is slightly imprecise but does not affect correctness.
Finding #1 (ACK-NOT-VALID): Acknowledged. The reviewer explicitly notes this is a known limitation of the os.SameFile pattern and that no action is required — the code comment already calls it defense-in-depth. The narrow hardlink-swap race is a theoretical limitation of the approach, not a defect introduced here.
Finding #5 (ACK-NOT-VALID): User-facing error messages intentionally reference *docmapFlag (the original --docmap value) rather than the resolved path. Showing the resolved path in errors would confuse users who passed a symlink — they would see a path they never specified. Using the flag value is the correct UX. The resolved path is used internally for all I/O; the flag value is only used in error messages.
Finding #4 (ACK-NOT-VALID): os.Lstat is intentionally used here for consistency with checkStaleDocs, which also uses Lstat to avoid implicit symlink-follow semantics. Switching to Stat post-EvalSymlinks would be equivalent at runtime but would create a mixed Stat/Lstat pattern in the same file that could confuse future readers. The deliberate choice is noted in the comment.
Finding #2 (ACK-NOT-VALID): Acknowledged. The reviewer confirms no issue — the ModeSymlink removal is correct (the check was genuinely unreachable after EvalSymlinks), the comment is accurate, and the removal is a deliberate documented choice. The comment phrasing is slightly imprecise but does not affect correctness.
Finding #1 (ACK-NOT-VALID): Acknowledged. The reviewer explicitly states this is a known limitation of the os.SameFile pattern and that no action is required — the code comment already calls it defense-in-depth. The narrow hardlink-swap race is a theoretical limitation of the approach, not a defect introduced here.
Finding #4 (ACK-NOT-VALID): Acknowledged. Lstat is intentionally used here for consistency with checkStaleDocs, which also uses Lstat to avoid follow-on symlink semantics. Switching to Stat post-EvalSymlinks would be equivalent at runtime but would create an inconsistency across the file that could confuse future readers. Keeping Lstat is the more conservative and consistent choice.
Finding #2 (ACK-NOT-VALID): Acknowledged. The comment saying 'ModeSymlink can never be set' is accurate — EvalSymlinks guarantees no symlinks remain, so that check was genuinely unreachable. The removal was intentional and documented. No action needed.
Finding #5 (ACK-NOT-VALID): Acknowledged. User-facing error messages intentionally reference the original --docmap flag value, not the resolved path. Showing the resolved path in errors would confuse users who passed a symlink — they'd see a path they never specified. Using the flag value is the correct UX here.
Finding #1 (ACK-NOT-VALID): Acknowledged. The reviewer explicitly notes this is a known limitation and that no action is required — the code comment already calls it defense-in-depth. No change needed.
Fix Plan against eb0ff3aa69f152dd995de91c88227d3e32ac2917:
Addressing bot-review findings from the latest round (sonnet/4814, security/4816, gpt/4817) — all evaluated against eb0ff3aa:
Self-Review: PR #152
Self-review against eb0ff3aa69f152dd995de91c88227d3e32ac2917
Phase 1: Independent Findings
None — diff looks clean.
Reviewed: validateDocmapPath signature change…
