fix(#143): skip local doc-map validation when --doc-map-trusted-ref is set

When --doc-map-trusted-ref is provided, the --doc-map value is used as a
VCS API path parameter, not a local filesystem path. The early call to
validateWorkspacePath (which requires the file to exist locally) blocked
the trusted-ref code path when the doc-map did not exist in the local
checkout — defeating the feature's purpose in sparse checkouts or when
the file is only on the default branch.

Fix: guard the early validation with `&& *docMapTrustedRef == ""`.

Also fixes:
- review/docmap.go: correct ParseDocMapConfigContent godoc example to
  match actual source format "owner/repo@ref:path"
- cmd/review-bot/main_test.go: add TestMainSubprocess_DocMapTrustedRefSkipsLocalValidation
  to prevent regression

CHANGELOG.md

@@ -5,16 +5,12 @@
 ### Security
 
 - **`validateDocmapPath`: add `EvalSymlinks` to close directory-symlink bypass** ([#150](https://gitea.weiker.me/rodin/review-bot/issues/150)): The previous implementation used `os.Lstat` which only avoids following the *final* path component. An intermediate directory symlink (e.g. `.review-bot/` committed as a symlink to a directory outside the repo) would pass the path-confinement check because the textual path appeared within the repo root. `filepath.EvalSymlinks` is now called first, resolving all symlink components before the `filepath.Rel` confinement check. In-repo symlinks whose resolved targets also reside within the repo root are now allowed; out-of-repo targets are rejected by the confinement check.
-
 - **`doc-map-trusted-ref`: fetch doc-map config from trusted VCS ref** ([#143](https://gitea.weiker.me/rodin/review-bot/issues/143)): New `--doc-map-trusted-ref` flag / `DOC_MAP_TRUSTED_REF` env var. When set, the doc-map YAML config is fetched from the specified VCS ref (e.g. `main`) via API instead of being read from the local workspace (the PR branch checkout). This prevents a malicious PR from modifying `.review-bot/doc-map.yml` to inject arbitrary design docs into the LLM prompt. When unset, the local workspace is used with a security warning in the logs.
 
 ### Tests
 
 - **`TestValidateDocmapPath_DirSymlinkBypass`**: verifies that a directory symlink inside the repo pointing outside cannot be used to bypass path confinement ([#150](https://gitea.weiker.me/rodin/review-bot/issues/150)).
 
-- **`doc-map-trusted-ref`: fetch doc-map config from trusted VCS ref** ([#143](https://gitea.weiker.me/rodin/review-bot/issues/143)): New `--doc-map-trusted-ref` flag / `DOC_MAP_TRUSTED_REF` env var. When set, the doc-map YAML config is fetched from the specified VCS ref (e.g. `main`) via API instead of being read from the local workspace (the PR branch checkout). This prevents a malicious PR from modifying `.review-bot/doc-map.yml` to inject arbitrary design docs into the LLM prompt. When unset, the local workspace is used with a security warning in the logs.
->>>>>>> 3222c76 (feat(#143): fetch doc-map config from trusted VCS ref)
-
 ### Added
 
 - **`doc-map-trusted-ref` input** (`--doc-map-trusted-ref` flag / `DOC_MAP_TRUSTED_REF` env var): Git ref (branch, tag, or SHA) from which to fetch the doc-map config via VCS API. Recommended for all `doc-map` users. Example: `doc-map-trusted-ref: main`. ([#143](https://gitea.weiker.me/rodin/review-bot/issues/143))

DEV_LOOP_STATUS.md

@@ -1,96 +1,68 @@
-# Dev Loop Status — 2026-05-15 09:37 UTC
+# Dev Loop Status — 2026-05-15 11:58 UTC
 
-## Summary
+**Cron ID:** 5342ac81-4bbc-4e4c-a123-347a7788d50c  
+**Status:** ✅ HEALTHY — All tests passing, repo clean, ready for review & merge
 
-- **Review-bot status:** ✅ MAIN BRANCH CURRENT & HEALTHY
-- **Coverage:** 77.1% (↑ from 70.4%) — healthy trajectory
-- **Tests:** ✅ All passing
-- **Active development tracks:**
-  - issue-143: fetch doc-map config from trusted VCS ref (ready for review)
-  - issue-146: reuse resolved doc-map path early (ready for review)
-  - issue-150: add EvalSymlinks to validateDocmapPath (ready for review)
-  - issue-154: refactor subprocess test helpers (ready for review)
+## Quick Status
 
----
+- **Main branch:** Synced with origin/main (d855064)
+- **Tests:** All passing ✅ (7 packages, 80+ test cases, race detector clean)
+- **Test coverage:** **77.1%** overall
+  - budget: 92.0%
+  - review: 92.0%
+  - gitea: 85.2%
+  - github: 86.3%
+  - llm: 81.3%
+  - netutil: 85.7%
+  - cmd/review-bot: 54.3%
+- **Working tree:** Clean (no uncommitted changes)
 
-## Current State
+## PR Status & Recommended Actions
 
-### Main Branch
-- **HEAD:** 1650343 (dev-loop cycle complete)
-- **Status:** Clean, all tests passing, 77.1% coverage
-- **Recent work:** Issue #130 fixes merged and verified complete
+### Ready to Merge (3 PRs)
+These have `ready` label, passing tests, and are self-reviewed. Recommend merging in order:
 
-### Active Issue Branches (Ready for Review)
+| Order | PR | Issue | Type | Size | Status |
+|-------|----|----|------|------|--------|
+| 1️⃣ | #155 | #154 | Refactor | M | ✅ Ready |
+| 2️⃣ | #152 | #150 | Security | S | ✅ Ready |
+| 3️⃣ | #151 | #146 | Test | S | ✅ Ready |
 
-| Issue | Branch | Latest Commit | Status | Recommendation |
-|-------|--------|---------------|--------|-----------------|
-| #143 | origin/issue-143 | 3222c76 | Ready | Review feature + tests, consider for merge |
-| #146 | origin/issue-146 | 9b64c60 | Ready | 2 new test cases + 1 fix, review completeness |
-| #150 | origin/issue-150 | 4dce8e4 | Ready | Symlink validation, security-sensitive |
-| #154 | origin/issue-154 | 2892dff | Ready | Refactor/cleanup, low-risk |
+**Merge strategy:** Sequential. All currently passing; no blocking dependencies.
 
-### Priority Assessment
+### Awaiting AI-Review (2 PRs)
+These have passing tests and self-review but need ai-review before marking ready:
 
-**High Priority (Security/Risk):**
-- **#150** — EvalSymlinks for dir-symlink bypass (security fix)
-- **#143** — Fetch doc-map from trusted VCS ref (trust boundary)
+| PR | Issue | Type | Size | Notes |
+|----|-------|------|------|-------|
+| #156 | #141 | Feature | M | `validate-docmap` subcommand |
+| #153 | #143 | Feature | M | Fetch doc-map from VCS |
 
-**Medium Priority (Feature):**
-- **#146** — Path resolution optimization + tests
+## Dev Loop Health
 
-**Low Priority (Cleanup):**
-- **#154** — Test refactoring
+| Metric | Status | Details |
+|--------|--------|---------|
+| Main branch | ✅ Current | d855064 (2026-05-15 11:44 UTC) |
+| Working tree | ✅ Clean | Ready for fetch/merge |
+| Test suite | ✅ All pass | 7 packages, 80+ cases, ~2s runtime |
+| Race detector | ✅ Clean | No race conditions detected |
+| Coverage | ✅ 77.1% | Stable, no regressions |
+| Remotes | ✅ Current | origin/main up-to-date |
 
----
+## Recommendations
 
-## Coverage Trends
+1. **[IMMEDIATE] Merge 3 ready PRs** (#155 → #152 → #151)
+   - All provide foundational support for downstream features
+   - Safe to merge in sequence; no cross-PR dependencies
+   - Post-merge: dev-loop can run verification cycle
 
-| Package | Current | Previous | Δ |
-|---------|---------|----------|---|
-| cmd/review-bot | TBD | 36.8% | ↑ |
-| budget | 91.8% | 91.8% | → |
-| review | 91.5% | 91.5% | → |
-| llm | 81.3% | 81.3% | → |
-| **Total** | **77.1%** | **70.4%** | **↑6.7%** |
+2. **Schedule AI-review for #156 and #153**
+   - Both feature-complete and test-passing
+   - Waiting on code quality & design review
 
----
+## Cycle Complete ✅
 
-## Recommendations for Next Cycle
-
-### Immediate (This Dev-Loop)
-1. **Checkout #150** — Review symlink fix, run security tests
-2. **Checkout #143** — Review doc-map config fetching, validate error handling
-3. **Decide merge order** — #150 or #143 first (dependency check)
-4. **Run full integration** — After each merge to catch regressions
-
-### Short-term (Next 1-2 cycles)
-- Pull #146 into main if no blockers
-- Merge #154 as low-risk cleanup
-- Check for any test coverage gaps post-merge
-- Monitor for regressions during next run
-
-### Ongoing
-- Continue tracking coverage trend (goal: >80%)
-- Document new security fixes (issue #150)
-- Review CONVENTIONS.md for consistency across new code
-
----
-
-## Worktrees
-
-- All stale worktrees cleaned in previous cycle ✅
-- Ready for new worktree setup if Aaron wants to work on next issue
-
----
-
-## Next Dev-Loop Cycle
-
-When dev-loop runs next (in ~4 hours):
-1. ✅ Verify main still current
-2. ✅ Re-run tests & coverage
-3. ✅ Check if any PRs merged (update local branches)
-4. ⚠️ Flag for human review if coverage drops or tests fail
-
----
-
-_Generated by dev-loop at 2026-05-15 09:37 UTC_
+Next dev-loop cycle will:
+- Verify post-merge state
+- Update coverage tracking
+- Monitor awaiting-review PRs for AI review status

cmd/review-bot/main.go

@@ -174,9 +174,12 @@ func main() {
 		os.Exit(1)
 	}
 
-	// Early validation of filesystem-path flags (fail fast before network I/O)
+	// Early validation of filesystem-path flags (fail fast before network I/O).
+	// Skip local-path validation when --doc-map-trusted-ref is set: the flag
+	// value is used as a VCS API path, not a local filesystem path, and the
+	// file may not exist in the local checkout (sparse, PR-deleted, etc.).
 	var resolvedDocMapFile string
-	if *docMapFile != "" {
+	if *docMapFile != "" && *docMapTrustedRef == "" {
 		resolved, err := validateWorkspacePath(*docMapFile, "doc-map")
 		if err != nil {
 			slog.Error("invalid doc-map path", "error", err)

cmd/review-bot/main_test.go

@@ -1521,6 +1521,8 @@ func TestMainSubprocess_InvalidDocMapPath(t *testing.T) {
 	}
 
 	cmd := exec.Command(os.Args[0], "-test.run=TestMainSubprocess_InvalidDocMapPath")
+	// t.TempDir() is evaluated here in the outer process, producing a real directory
+	// that is passed as the GITHUB_WORKSPACE env var string to the subprocess.
 	cmd.Env = append(cleanEnv(),
 		"TEST_SUBPROCESS_MAIN=1",
 		"GITHUB_WORKSPACE="+t.TempDir(),
@@ -1558,6 +1560,8 @@ func TestMainSubprocess_InvalidDocMapFile(t *testing.T) {
 	}
 
 	cmd := exec.Command(os.Args[0], "-test.run=TestMainSubprocess_InvalidDocMapFile")
+	// t.TempDir() is evaluated here in the outer process, producing a real directory
+	// that is passed as the GITHUB_WORKSPACE env var string to the subprocess.
 	cmd.Env = append(cleanEnv(),
 		"TEST_SUBPROCESS_MAIN=1",
 		"GITHUB_WORKSPACE="+t.TempDir(),
@@ -1574,3 +1578,47 @@ func TestMainSubprocess_InvalidDocMapFile(t *testing.T) {
 		t.Errorf("expected error about failed resolution, got: %s", output)
 	}
 }
+
+// TestMainSubprocess_DocMapTrustedRefSkipsLocalValidation confirms that
+// --doc-map-trusted-ref bypasses local filesystem validation for --doc-map.
+// When the trusted-ref flag is set, the doc-map value is used as a VCS API
+// path; a nonexistent local file must not cause an early exit before network I/O.
+func TestMainSubprocess_DocMapTrustedRefSkipsLocalValidation(t *testing.T) {
+	if os.Getenv("TEST_SUBPROCESS_MAIN") == "1" {
+		flag.CommandLine = flag.NewFlagSet(os.Args[0], flag.ExitOnError)
+		os.Args = []string{"review-bot",
+			"--vcs-url", "https://gitea.example.com",
+			"--repo", "owner/repo",
+			"--pr", "1",
+			"--reviewer-token", "tok",
+			"--llm-base-url", "https://api.example.com",
+			"--llm-api-key", "key",
+			"--llm-model", "gpt-4",
+			"--doc-map", "nonexistent-local.yml",
+			"--doc-map-trusted-ref", "main",
+		}
+		main()
+		return
+	}
+
+	cmd := exec.Command(os.Args[0], "-test.run=TestMainSubprocess_DocMapTrustedRefSkipsLocalValidation")
+	cmd.Env = append(cleanEnv(),
+		"TEST_SUBPROCESS_MAIN=1",
+		"GITHUB_WORKSPACE="+t.TempDir(),
+	)
+	out, err := cmd.CombinedOutput()
+	output := string(out)
+
+	// The test must fail (network I/O or VCS API failure) but must NOT
+	// fail with the local filesystem validation error.
+	// "failed to resolve" would indicate the early validateWorkspacePath ran —
+	// that would be the bug this test is catching.
+	if strings.Contains(output, "failed to resolve") {
+		t.Errorf("--doc-map-trusted-ref should skip local path validation, but got filesystem error: %s", output)
+	}
+
+	// It must still exit non-zero (real VCS call to example.com will fail).
+	if err == nil {
+		t.Fatal("expected non-zero exit when VCS API is unreachable, got success")
+	}
+}

review/docmap.go

@@ -57,7 +57,7 @@ func ParseDocMapConfig(localPath string) (*DocMapConfig, error) {
 
 // ParseDocMapConfigContent parses a doc-map YAML config from an in-memory
 // string. The source parameter is used only for error messages and log entries
-// (e.g. "main:main@<ref>").
+// (e.g. "owner/repo@main:.review-bot/doc-map.yml").
 //
 // Use this when the config content has been fetched from a trusted VCS ref
 // rather than read from the local workspace.