fix(docs): address review feedback on architecture clarity and path consistency

- Clarify SPAWN exits vs HANDOFF continues in architecture diagram (S1)
- Add 'read' to toolsAllow in architecture snippet to match cron config (G2)
- Rephrase safety invariant 6 to clarify workers may push/manage labels (G3)
- Add reserved Rule 1 placeholder to explain numbering gap (S2)
- Clarify Rule 10 skip behavior for already-assigned PRs (S3)
- Standardize invariants checker path to full workspace path (G4/G5)
- Add note explaining SKILL.md deployment to workspace path (G1)

SKILL.md

@@ -0,0 +1,129 @@
+# Dev-Loop Skill: review-bot
+
+This file documents the dev-loop architecture for the `review-bot` project.
+It lives in the repo so changes are version-controlled alongside the code.
+
+## Architecture
+
+Dispatch is a **pure shell script** — no model reasoning.
+
+```
+Cron (agentTurn, toolsAllow: [exec, sessions_spawn, read])
+  → runs dispatch script
+  → reads output for SPAWN or HANDOFF lines
+  → spawns worker if instructed
+
+Dispatch script  (~/.openclaw/workspace/scripts/dev-loop-dispatch.sh)
+  → pure bash, all decisions are curl API calls + branches
+  → exits after emitting one SPAWN line (at most one worker per run)
+  → emits HANDOFF for each qualifying PR (does not exit after HANDOFF)
+
+Workers (Opus, spawned by cron model)
+  → receive precise task description
+  → do one job: self-review, fix CI, address feedback, or implement
+  → remove wip label when done, reply NO_REPLY
+```
+
+The cron model's **only** job: run script, read output, spawn worker if told to.
+The model **never** assesses project state or makes dispatch decisions.
+
+## Safety Invariants
+
+1. **NEVER MERGE** — no merge API call exists anywhere in the script or worker templates
+2. **REQUEST_CHANGES always blocks** — checked first, before CI, before self-review, before handoff
+3. **WIP mutex** — one active worker per repo; WIP label gates new issue pickup
+4. **One SPAWN per run** — script emits at most one SPAWN line per execution
+5. **set -euo pipefail** — any curl failure aborts immediately, no partial actions
+6. **Workers reply NO_REPLY** — no dispatch-level side effects (workers may push changes and manage labels as part of their task)
+
+## Dispatch Rules (in order)
+
+| Rule | Condition | Action |
+|------|-----------|--------|
+| 0 | WIP label > 1hr old | Remove stale WIP, continue |
+| 0b | WIP label ≤ 1hr old | Mark ACTIVE_WIP=1, continue (only gates Rule 10) |
+| _(1)_ | _(reserved — intentionally unused)_ | — |
+| 2 | Any reviewer has REQUEST_CHANGES | SPAWN:findings |
+| 3 | PR not mergeable | SPAWN:rebase |
+| 4 | CI failure, no fix plan | SPAWN:ci-fix |
+| 4b | CI failure, fix plan exists | Skip (worker in progress) |
+| 5 | Bot review missing | Wait |
+| 6 | CI pending/unknown | Wait |
+| 7 | No clean self-review, no fix plan | SPAWN:self-review |
+| 7b | Self-review needs attention, no fix plan | SPAWN:sr-fix |
+| 8 | Unacknowledged bot review findings | SPAWN:address-feedback |
+| 9 | Unresolved inline diff comments | SPAWN:address-feedback |
+| 10 | All checks pass | HANDOFF |
+| 11 | No open PRs + no ACTIVE_WIP | SPAWN:impl (next issue) |
+
+## Files
+
+| File | Description |
+|------|-------------|
+| `~/.openclaw/workspace/scripts/dev-loop-dispatch.sh` | Dispatch script — pure bash |
+| `~/.openclaw/workspace/scripts/worker-tasks/self-review.md` | Self-review worker template |
+| `~/.openclaw/workspace/scripts/worker-tasks/sr-fix.md` | Fix findings from self-review |
+| `~/.openclaw/workspace/scripts/worker-tasks/ci-fix.md` | CI fix worker template |
+| `~/.openclaw/workspace/scripts/worker-tasks/address-feedback.md` | Address feedback worker template |
+| `~/.openclaw/workspace/scripts/worker-tasks/findings.md` | Address REQUEST_CHANGES findings |
+| `~/.openclaw/workspace/scripts/worker-tasks/rebase.md` | Rebase worker template |
+| `~/.openclaw/workspace/scripts/worker-tasks/impl.md` | Issue implementation worker template |
+| `~/.openclaw/workspace/scripts/test/dispatch.bats` | Unit tests (bats) |
+| `~/.openclaw/workspace/scripts/test/check-invariants.sh` | Static invariant checks |
+| `~/.openclaw/workspace/memory/projects/review-bot.yaml` | Project config |
+
+## Project Config
+
+Config is at `~/.openclaw/workspace/memory/projects/review-bot.yaml`.
+
+Key fields:
+- `repo`: `rodin/review-bot`
+- `api_base`: `https://gitea.weiker.me/api/v1`
+- `user`: `rodin` (bot Gitea username)
+- `labels.wip`: WIP label ID
+- `labels.ready`: ready label ID
+- `review_bots`: list of bot sentinel names
+
+## Cron Config
+
+```yaml
+- label: review-bot-dev-loop
+  schedule: "*/15 * * * *"
+  prompt: |
+    Run: bash ~/.openclaw/workspace/scripts/dev-loop-dispatch.sh review-bot
+
+    Read the output. If it contains a SPAWN line, load the matching template from
+    ~/.openclaw/workspace/scripts/worker-tasks/<type>.md, substitute {{PROJECT}},
+    {{PR_NUM}}, and {{HEAD_SHA}}, then spawn with sessions_spawn(mode: "run",
+    model: "hai-anthropic/anthropic--claude-4.6-opus", thinking: "high").
+
+    If no SPAWN line in output, reply NO_REPLY.
+
+    See ~/.openclaw/workspace/skills/dev-loop/SKILL.md for full instructions.
+    (This repo's SKILL.md is deployed to that workspace path.)
+  model: hai-anthropic/anthropic--claude-4.5-haiku
+  toolsAllow: [exec, sessions_spawn, read]
+```
+
+## Tests
+
+```bash
+# Unit tests (no real API calls):
+bats ~/.openclaw/workspace/scripts/test/dispatch.bats
+
+# Invariant checks (static analysis):
+bash ~/.openclaw/workspace/scripts/test/check-invariants.sh
+
+# Dry-run against real API:
+DRY_RUN=1 bash ~/.openclaw/workspace/scripts/dev-loop-dispatch.sh review-bot
+```
+
+## Related Issues
+
+- **#144** — autonomous merge: eliminated by removing all merge API calls from dispatch
+- **#145** — merged despite REQUEST_CHANGES: eliminated by checking REQUEST_CHANGES first, unconditionally
+- **#148** — this redesign
+
+## Spec
+
+Full design spec: `docs/dev-loop-spec.md`

TODO.md

@@ -1,47 +0,0 @@
-## Dev Loop Status: 2026-05-15 07:02 UTC
-
-**Issue:** #141 — validate-docmap subcommand  
-**Branch:** issue-141  
-**Status:** ✅ READY FOR PR SUBMISSION
-
-### Implementation Complete
-
-- ✅ `cmd/review-bot/validatedocmap.go` — main subcommand implementation (139 lines)
-- ✅ `cmd/review-bot/validatedocmap_test.go` — 9 test cases (all passing)
-- ✅ `review/docmap.go` — exported `FileCoveredByDocMap` helper
-- ✅ `cmd/review-bot/main.go` — integrated `validate-docmap` subcommand
-- ✅ `README.md` — documented subcommand with examples and flags
-
-### Test Results
-
-```
-TestRunValidateDocmap_Clean              ✅
-TestRunValidateDocmap_MissingDocmapFlag  ✅
-TestRunValidateDocmap_BadYAML            ✅
-TestRunValidateDocmap_StaleDocs          ✅
-TestRunValidateDocmap_UncoveredFile      ✅
-TestRunValidateDocmap_BothFailures       ✅
-TestRunValidateDocmap_EmptyStdin         ✅
-TestRunValidateDocmap_BlankLinesSkipped  ✅
-TestRunValidateDocmap_DuplicateDocsDeduped ✅
-```
-
-### Pre-Push Checks
-
-- ✅ All packages pass: `go test ./...` (6/6)
-- ✅ `go vet ./...` — clean
-- ✅ `go build ./cmd/review-bot` — clean
-- ✅ `scripts/check-deps.sh` — pass
-
-### Commits
-
-1. feat(#141): add validate-docmap subcommand
-2. docs(#141): add validate-docmap subcommand to README
-3. fix(#141): use stdinValidateDocmap in Clean test — avoid real os.Stdin dependency
-
-### Next Steps
-
-- [ ] Create PR via gitea-rodin token (title: "feat(#141): add validate-docmap subcommand")
-- [ ] Labels: none yet (AI reviewers will assign)
-- [ ] Link issue #141
-

cmd/review-bot/validatedocmap.go

@@ -12,6 +12,59 @@ import (
 	"gitea.weiker.me/rodin/review-bot/review"
 )
 
+// maxDocmapBytes is the maximum size of the doc-map YAML file that will be
+// read. Files larger than this are rejected before reading to prevent memory
+// exhaustion from an oversized PR-controlled file.
+const maxDocmapBytes int64 = 10 * 1024 * 1024 // 10 MB
+
+// validateDocmapPath checks that localPath is safe to read as the doc-map
+// file. It enforces three invariants before the file is opened:
+//
+//  1. The path resolves to a regular file within resolvedRoot (path
+//     confinement): prevents a PR-controlled --docmap from reading arbitrary
+//     host files via absolute paths or ".." traversal.
+//  2. The path is not a symlink: prevents denial-of-service via /dev/zero or
+//     information disclosure via symlinks that point outside the workspace.
+//  3. The file does not exceed maxDocmapBytes: prevents memory exhaustion
+//     from an oversized but legitimately committed doc-map file.
+//
+// resolvedRoot must already be an absolute, symlink-free path (obtained from
+// filepath.Abs + filepath.EvalSymlinks).
+func validateDocmapPath(localPath, resolvedRoot string) error {
+	// Resolve the docmap path to an absolute path.
+	absPath, err := filepath.Abs(localPath)
+	if err != nil {
+		return fmt.Errorf("cannot resolve path: %w", err)
+	}
+
+	// Lstat: do NOT follow symlinks. We want to inspect the entry itself.
+	fi, err := os.Lstat(absPath)
+	if err != nil {
+		return fmt.Errorf("cannot stat file: %w", err)
+	}
+
+	// Reject symlinks outright — a symlink can point to /dev/zero or arbitrary
+	// host paths, bypassing both the confinement check and the size check.
+	if fi.Mode()&os.ModeSymlink != 0 {
+		return fmt.Errorf("symlinks are not allowed")
+	}
+
+	// Confine to resolvedRoot: the cleaned absolute path must be a descendant
+	// of the repo root. This catches paths that escaped via "..", absolute
+	// paths that happen to be outside the root, etc.
+	rel, err := filepath.Rel(resolvedRoot, absPath)
+	if err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(os.PathSeparator)) {
+		return fmt.Errorf("path must be within --repo-root")
+	}
+
+	// Enforce size cap before reading to prevent memory exhaustion.
+	if fi.Size() > maxDocmapBytes {
+		return fmt.Errorf("file size %d bytes exceeds %d-byte limit", fi.Size(), maxDocmapBytes)
+	}
+
+	return nil
+}
+
 // runValidateDocmap implements the `review-bot validate-docmap` subcommand.
 //
 // It reads changed file paths from stdin (one per line, as produced by
@@ -51,6 +104,36 @@ func runValidateDocmap(args []string) int {
 		return 2
 	}
 
+	// Resolve repoRoot first — the docmap path is validated against it below.
+	// Use an absolute, symlink-free path so a symlinked --repo-root cannot
+	// bypass the escape guard in validateDocmapPath or checkStaleDocs.
+	absRoot, err := filepath.Abs(*repoRootFlag)
+	if err != nil {
+		fmt.Fprintf(errWriter, "Error: failed to resolve --repo-root %q: %v\n", *repoRootFlag, err)
+		return 2
+	}
+	resolvedRoot, err := filepath.EvalSymlinks(absRoot)
+	if err != nil {
+		if os.IsNotExist(err) {
+			fmt.Fprintf(errWriter, "Error: --repo-root %q does not exist\n", *repoRootFlag)
+		} else {
+			fmt.Fprintf(errWriter, "Error: failed to resolve --repo-root %q: %v\n", *repoRootFlag, err)
+		}
+		return 2
+	}
+
+	// Harden the docmap file path before reading it. The --docmap flag value
+	// may reference a PR-controlled file (e.g. .review-bot/doc-map.yml).
+	// Validate that it:
+	//   1. Resolves within resolvedRoot (prevent reading arbitrary host files).
+	//   2. Is not a symlink (prevent /dev/zero or symlink-based host probing).
+	//   3. Does not exceed maxDocmapBytes (prevent memory exhaustion from an
+	//      oversized committed file).
+	if err := validateDocmapPath(*docmapFlag, resolvedRoot); err != nil {
+		fmt.Fprintf(errWriter, "Error: --docmap %q is invalid: %v\n", *docmapFlag, err)
+		return 2
+	}
+
 	// Parse docmap YAML.
 	cfg, err := review.ParseDocMapConfig(*docmapFlag)
 	if err != nil {
@@ -68,8 +151,15 @@ func runValidateDocmap(args []string) int {
 	failed := false
 
 	// --- Check 1: Coverage ---
+	// Note: an empty docmap (no mappings) means every changed file is
+	// uncovered — there are no patterns to match against. This is intentional:
+	// if you declare a doc-map, every changed file must be accounted for.
+	// On empty stdin the check is vacuously true (no files to cover).
 	var uncovered []string
 	for _, f := range changedFiles {
+		// Normalize Windows-style backslashes to forward slashes so that
+		// changed-file paths from git on Windows match doc-map globs.
+		f = strings.ReplaceAll(f, "\\", "/")
 		if !review.FileCoveredByDocMap(cfg, f) {
 			uncovered = append(uncovered, f)
 		}
@@ -83,19 +173,6 @@ func runValidateDocmap(args []string) int {
 	}
 
 	// --- Check 2: Stale docs ---
-	// Resolve repoRoot to an absolute, symlink-free path before any Rel checks
-	// so that a symlinked --repo-root cannot be used to bypass the escape
-	// guard in checkStaleDocs.
-	absRoot, err := filepath.Abs(*repoRootFlag)
-	if err != nil {
-		fmt.Fprintf(errWriter, "Error: failed to resolve --repo-root %q: %v\n", *repoRootFlag, err)
-		return 2
-	}
-	resolvedRoot, err := filepath.EvalSymlinks(absRoot)
-	if err != nil {
-		fmt.Fprintf(errWriter, "Error: failed to resolve --repo-root %q: %v\n", *repoRootFlag, err)
-		return 2
-	}
 	// checkStaleDocs validates each path before touching the filesystem; see
 	// its documentation for the path-traversal hardening applied.
 	staleDocs := checkStaleDocs(cfg, resolvedRoot)
@@ -151,7 +228,7 @@ func checkStaleDocs(cfg *review.DocMapConfig, repoRoot string) []string {
 			// filepath.Rel check confirms the path is still under repoRoot.
 			fullPath := filepath.Clean(filepath.Join(repoRoot, filepath.FromSlash(docPath)))
 			rel, err := filepath.Rel(repoRoot, fullPath)
-			if err != nil || strings.HasPrefix(rel, "..") {
+			if err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(os.PathSeparator)) {
 				stale = append(stale, docPath)
 				continue
 			}

cmd/review-bot/validatedocmap_test.go

@@ -9,6 +9,8 @@ import (
 )
 
 // makeDocmapYAML writes a YAML string to a temp file and returns its path.
+// The file is created in t.TempDir() — use makeDocmapInDir when the docmap
+// must be located inside a specific repo-root directory.
 func makeDocmapYAML(t *testing.T, content string) string {
 	t.Helper()
 	f, err := os.CreateTemp(t.TempDir(), "doc-map-*.yml")
@@ -22,6 +24,21 @@ func makeDocmapYAML(t *testing.T, content string) string {
 	return f.Name()
 }
 
+// makeDocmapInDir writes a YAML string to a file inside dir and returns the
+// file path. Use this instead of makeDocmapYAML when also passing --repo-root,
+// because validateDocmapPath requires the docmap to be within the repo root.
+func makeDocmapInDir(t *testing.T, dir, content string) string {
+	t.Helper()
+	if err := os.MkdirAll(filepath.Join(dir, ".review-bot"), 0o755); err != nil {
+		t.Fatalf("MkdirAll: %v", err)
+	}
+	path := filepath.Join(dir, ".review-bot", "doc-map.yml")
+	if err := os.WriteFile(path, []byte(content), 0o644); err != nil {
+		t.Fatalf("WriteFile: %v", err)
+	}
+	return path
+}
+
 // makeDocFile creates a file (and any parent dirs) at the given path relative to dir.
 func makeDocFile(t *testing.T, dir, rel string) {
 	t.Helper()
@@ -52,7 +69,7 @@ func TestRunValidateDocmap_Clean(t *testing.T) {
 	dir := t.TempDir()
 	makeDocFile(t, dir, "docs/foo.md")
 
-	docmap := makeDocmapYAML(t, `
+	docmap := makeDocmapInDir(t, dir, `
 mappings:
   - paths:
       - "lib/foo/**"
@@ -87,10 +104,11 @@ func TestRunValidateDocmap_MissingDocmapFlag(t *testing.T) {
 }
 
 func TestRunValidateDocmap_BadYAML(t *testing.T) {
-	docmap := makeDocmapYAML(t, "mappings: [{{invalid")
+	dir := t.TempDir()
+	docmap := makeDocmapInDir(t, dir, "mappings: [{{invalid")
 	var code int
 	_, stderr := captureOutput(func() {
-		code = runValidateDocmap([]string{"--docmap", docmap})
+		code = runValidateDocmap([]string{"--docmap", docmap, "--repo-root", dir})
 	})
 	if code != 2 {
 		t.Errorf("expected exit 2 for bad YAML, got %d", code)
@@ -104,7 +122,7 @@ func TestRunValidateDocmap_StaleDocs(t *testing.T) {
 	dir := t.TempDir()
 	// docs/foo.md does NOT exist on disk.
 
-	docmap := makeDocmapYAML(t, `
+	docmap := makeDocmapInDir(t, dir, `
 mappings:
   - paths:
       - "lib/foo/**"
@@ -131,6 +149,12 @@ mappings:
 }
 
 // stdinValidateDocmap runs runValidateDocmap with a synthetic stdin.
+//
+// Implementation note: we write stdinContent to a temp file and point
+// os.Stdin at it. The defer f.Close() fires after stdinValidateDocmap
+// returns, which is after runValidateDocmap has finished reading stdin
+// synchronously — so the file is not closed while still in use.
+// Tests must not call t.Parallel() while sharing the global os.Stdin.
 func stdinValidateDocmap(t *testing.T, stdinContent string, args []string) (code int, stdout, stderr string) {
 	t.Helper()
 	// Write stdin content to a temp file and redirect os.Stdin.
@@ -160,7 +184,7 @@ func TestRunValidateDocmap_UncoveredFile(t *testing.T) {
 	dir := t.TempDir()
 	makeDocFile(t, dir, "docs/foo.md")
 
-	docmap := makeDocmapYAML(t, `
+	docmap := makeDocmapInDir(t, dir, `
 mappings:
   - paths:
       - "lib/foo/**"
@@ -187,7 +211,7 @@ func TestRunValidateDocmap_BothFailures(t *testing.T) {
 	dir := t.TempDir()
 	// docs/foo.md intentionally missing
 
-	docmap := makeDocmapYAML(t, `
+	docmap := makeDocmapInDir(t, dir, `
 mappings:
   - paths:
       - "lib/foo/**"
@@ -214,7 +238,7 @@ func TestRunValidateDocmap_EmptyStdin(t *testing.T) {
 	dir := t.TempDir()
 	makeDocFile(t, dir, "docs/foo.md")
 
-	docmap := makeDocmapYAML(t, `
+	docmap := makeDocmapInDir(t, dir, `
 mappings:
   - paths:
       - "lib/foo/**"
@@ -238,7 +262,7 @@ func TestRunValidateDocmap_BlankLinesSkipped(t *testing.T) {
 	dir := t.TempDir()
 	makeDocFile(t, dir, "docs/foo.md")
 
-	docmap := makeDocmapYAML(t, `
+	docmap := makeDocmapInDir(t, dir, `
 mappings:
   - paths:
       - "lib/foo/**"
@@ -264,7 +288,7 @@ func TestRunValidateDocmap_DuplicateDocsDeduped(t *testing.T) {
 	// docs/shared.md intentionally missing — but it appears in TWO mappings.
 	// Should appear only once in stale list.
 
-	docmap := makeDocmapYAML(t, `
+	docmap := makeDocmapInDir(t, dir, `
 mappings:
   - paths:
       - "lib/foo/**"
@@ -311,7 +335,7 @@ func TestCheckStaleDocs_PathTraversal(t *testing.T) {
 
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
-			docmap := makeDocmapYAML(t, `
+			docmap := makeDocmapInDir(t, dir, `
 mappings:
   - paths:
       - "lib/**"
@@ -352,7 +376,7 @@ func TestCheckStaleDocs_SymlinkOutside(t *testing.T) {
 		t.Fatalf("Symlink: %v", err)
 	}
 
-	docmap := makeDocmapYAML(t, `
+	docmap := makeDocmapInDir(t, dir, `
 mappings:
   - paths:
       - "lib/**"
@@ -387,7 +411,7 @@ func TestCheckStaleDocs_SymlinkInsideRepo(t *testing.T) {
 		t.Fatalf("Symlink: %v", err)
 	}
 
-	docmap := makeDocmapYAML(t, `
+	docmap := makeDocmapInDir(t, dir, `
 mappings:
   - paths:
       - "lib/**"
@@ -405,7 +429,7 @@ mappings:
 }
 
 // TestRunValidateDocmap_SymlinkRepoRoot verifies that a --repo-root that is
-// itself a symlink to a valid directory resolves correctly (Finding #2).
+// itself a symlink to a valid directory resolves correctly.
 func TestRunValidateDocmap_SymlinkRepoRoot(t *testing.T) {
 	realDir := t.TempDir()
 	makeDocFile(t, realDir, "docs/foo.md")
@@ -416,7 +440,10 @@ func TestRunValidateDocmap_SymlinkRepoRoot(t *testing.T) {
 		t.Fatalf("Symlink: %v", err)
 	}
 
-	docmap := makeDocmapYAML(t, `
+	// Place the docmap inside realDir so it passes the confinement check.
+	// (symlinkDir resolves to realDir, so files inside realDir are also inside
+	// the resolved repo-root.)
+	docmap := makeDocmapInDir(t, realDir, `
 mappings:
   - paths:
       - "lib/**"
@@ -436,3 +463,90 @@ mappings:
 		t.Errorf("expected 'OK' in stdout, got %q", stdout)
 	}
 }
+
+// TestValidateDocmapPath_Symlink verifies that --docmap pointing at a symlink
+// is rejected before the file is read (prevents /dev/zero DOS or arbitrary
+// host-file reads via PR-controlled symlinks).
+func TestValidateDocmapPath_Symlink(t *testing.T) {
+	dir := t.TempDir()
+
+	// Create a real docmap file to serve as the symlink target.
+	realDocmap := makeDocmapInDir(t, dir, `
+mappings:
+  - paths:
+      - "lib/**"
+    docs:
+      - docs/foo.md
+`)
+
+	// Create a symlink inside dir pointing to the real docmap.
+	symlinkPath := filepath.Join(dir, ".review-bot", "doc-map-link.yml")
+	if err := os.Symlink(realDocmap, symlinkPath); err != nil {
+		t.Fatalf("Symlink: %v", err)
+	}
+
+	code, _, stderr := stdinValidateDocmap(t,
+		"",
+		[]string{"--docmap", symlinkPath, "--repo-root", dir},
+	)
+	if code != 2 {
+		t.Errorf("expected exit 2 for symlink docmap, got %d; stderr: %q", code, stderr)
+	}
+	if !strings.Contains(stderr, "symlink") && !strings.Contains(stderr, "invalid") {
+		t.Errorf("expected symlink rejection in stderr, got %q", stderr)
+	}
+}
+
+// TestValidateDocmapPath_OutsideRepoRoot verifies that --docmap pointing
+// outside --repo-root is rejected (prevents reading arbitrary host files).
+func TestValidateDocmapPath_OutsideRepoRoot(t *testing.T) {
+	repoDir := t.TempDir()
+
+	// Create a docmap in a separate temp dir (outside the repo root).
+	outside := makeDocmapYAML(t, `
+mappings:
+  - paths:
+      - "lib/**"
+    docs:
+      - docs/foo.md
+`)
+
+	code, _, stderr := stdinValidateDocmap(t,
+		"",
+		[]string{"--docmap", outside, "--repo-root", repoDir},
+	)
+	if code != 2 {
+		t.Errorf("expected exit 2 for docmap outside repo-root, got %d; stderr: %q", code, stderr)
+	}
+	if !strings.Contains(stderr, "invalid") && !strings.Contains(stderr, "repo-root") {
+		t.Errorf("expected confinement rejection in stderr, got %q", stderr)
+	}
+}
+
+// TestValidateDocmapPath_SizeLimit verifies that --docmap files exceeding
+// maxDocmapBytes are rejected before reading (prevents memory exhaustion).
+func TestValidateDocmapPath_SizeLimit(t *testing.T) {
+	dir := t.TempDir()
+
+	// Write a file larger than maxDocmapBytes.
+	bigPath := filepath.Join(dir, ".review-bot", "big-doc-map.yml")
+	if err := os.MkdirAll(filepath.Dir(bigPath), 0o755); err != nil {
+		t.Fatalf("MkdirAll: %v", err)
+	}
+	// Exceed the limit by one byte.
+	bigContent := make([]byte, maxDocmapBytes+1)
+	if err := os.WriteFile(bigPath, bigContent, 0o644); err != nil {
+		t.Fatalf("WriteFile: %v", err)
+	}
+
+	code, _, stderr := stdinValidateDocmap(t,
+		"",
+		[]string{"--docmap", bigPath, "--repo-root", dir},
+	)
+	if code != 2 {
+		t.Errorf("expected exit 2 for oversized docmap, got %d; stderr: %q", code, stderr)
+	}
+	if !strings.Contains(stderr, "limit") && !strings.Contains(stderr, "size") && !strings.Contains(stderr, "invalid") {
+		t.Errorf("expected size limit error in stderr, got %q", stderr)
+	}
+}

docs/dev-loop-spec.md

@@ -0,0 +1,278 @@
+# Dev-Loop Dispatch Spec
+
+**Version:** 1.0
+**Status:** Implemented
+**Implements:** Issue #148
+
+This document is the authoritative spec for the review-bot dev-loop dispatch architecture.
+The dispatch script (`~/.openclaw/workspace/scripts/dev-loop-dispatch.sh`) and its tests
+are validated against the rules and invariants in this document.
+
+---
+
+## 1. Overview
+
+The dev-loop is a 15-minute cron that advances the state of open pull requests and picks up
+new issues when there is nothing in review. It is designed for **zero human intervention**
+in the normal flow and **hard stops at key safety boundaries**.
+
+### Architecture
+
+```
+Cron (15-min cadence)
+  → exec: bash dev-loop-dispatch.sh <project>
+  → read stdout for SPAWN/HANDOFF lines
+  → if SPAWN: load worker template, spawn subagent
+  → if HANDOFF: log, do nothing else
+  → if neither: NO_REPLY
+```
+
+The cron model has **no ambient knowledge** of the project state. All state is derived
+from the dispatch script's output, which in turn comes from live API calls.
+
+---
+
+## 2. Inputs
+
+### Project Config
+
+```yaml
+# memory/projects/<project>.yaml
+repo: rodin/review-bot         # <owner>/<repo>
+api_base: https://gitea.../v1  # API base URL
+token_path: ~/.openclaw/...    # path to bearer token
+user: rodin                    # bot Gitea username
+labels:
+  wip: <id>
+  ready: <id>
+review_bots:                   # sentinel names in review bodies
+  - sonnet
+  - gpt
+  - security
+```
+
+### Script Arguments
+
+```bash
+bash dev-loop-dispatch.sh <project>   # normal run
+DRY_RUN=1 bash dev-loop-dispatch.sh <project>   # dry-run (no mutations)
+```
+
+---
+
+## 3. State
+
+The dispatch script is **stateless per run**. All state lives in the Gitea API:
+
+| State | API location |
+|-------|-------------|
+| Open PRs | `GET /repos/:repo/pulls?state=open` |
+| PR labels | `GET /repos/:repo/issues/:n/labels` |
+| PR reviews | `GET /repos/:repo/pulls/:n/reviews` |
+| CI status | `GET /repos/:repo/commits/:sha/status` |
+| Issue comments | `GET /repos/:repo/issues/:n/comments` |
+| Inline diff comments | `GET /repos/:repo/pulls/:n/comments` |
+| Issue timeline | `GET /repos/:repo/issues/:n/timeline` |
+
+No file-based state. No cron-to-cron carry-over.
+
+---
+
+## 4. Output Protocol
+
+The script emits structured lines to stdout. Stderr is diagnostic logging.
+
+### `SPAWN:<type>:<number>:<sha>`
+
+A worker is needed. The cron model reads this and spawns a subagent using the
+template at `worker-tasks/<type>.md`.
+
+| Field | Description |
+|-------|-------------|
+| `type` | Worker type: `self-review`, `ci-fix`, `address-feedback`, `findings`, `rebase`, `impl` |
+| `number` | PR number (or issue number for `impl`) |
+| `sha` | HEAD SHA of the PR (empty for `impl`) |
+
+At most **one SPAWN** is emitted per script run.
+
+### `HANDOFF:<pr_num>`
+
+All checks passed for `pr_num`. The script applied the `ready` label and assigned
+to the human reviewer. The cron model logs this and takes no further action.
+
+Multiple HANDOFFs may be emitted in one run (one per qualifying PR).
+
+---
+
+## 5. Dispatch Rules
+
+Rules are evaluated **in order** for each open PR. The first matching condition wins.
+Only one SPAWN is emitted per full pass.
+
+### Rule 0: WIP Cleanup
+
+For each open PR with a `wip` label:
+
+1. Find the timestamp when the label was most recently applied (via timeline events)
+2. If age > 1hr: **remove the label** (stale lock — worker likely crashed)
+3. If age ≤ 1hr: **set ACTIVE_WIP=1** (do not exit, only gates Rule 10)
+
+### Rule 2: REQUEST_CHANGES Blocks
+
+**ALWAYS evaluated before any other per-PR rule.**
+
+For each reviewer, take their **latest** review state. If any reviewer's latest
+state is `REQUEST_CHANGES`:
+
+→ Acquire WIP label on this PR
+→ Emit `SPAWN:findings:<pr_num>:<head_sha>`
+→ Continue to next PR (but only one SPAWN total)
+
+This rule cannot be bypassed by any condition. There is no waiver mechanism.
+
+### Rule 3: Merge Conflicts
+
+If `mergeable == false`:
+
+→ Acquire WIP
+→ Emit `SPAWN:rebase:<pr_num>:<head_sha>`
+
+### Rule 4: CI Failure
+
+If CI state is `failure` or `error`:
+
+- If a fix plan comment exists for this HEAD SHA: **skip** (worker in progress)
+- Otherwise:
+
+→ Acquire WIP
+→ Emit `SPAWN:ci-fix:<pr_num>:<head_sha>`
+
+### Rule 5: Bot Reviews Missing
+
+For each configured `review_bot`, check whether a review body contains the
+sentinel `<!-- review-bot:<name> -->`.
+
+If any sentinel is missing: **wait** (continue to next PR, no SPAWN).
+
+### Rule 6: CI Pending/Unknown
+
+If CI state is `pending` or `unknown`: **wait**.
+
+### Rule 7: Self-Review
+
+Check for a self-review comment from the bot user against the current HEAD SHA:
+- Comment contains `Self-review against <head_sha>`
+
+Sub-cases:
+- **Missing**: No self-review comment →
+  → Acquire WIP, emit `SPAWN:self-review:<pr_num>:<head_sha>`
+- **Needs attention** (`Assessment: ⚠️`): Found, but has findings:
+  - Fix plan exists for HEAD SHA: skip
+  - No fix plan: → Acquire WIP, emit `SPAWN:sr-fix:<pr_num>:<head_sha>`
+- **Clean** (`Assessment: ✅ Clean`): Continue to Rule 8
+
+### Rule 8: Unacknowledged Bot Review Findings
+
+For each **current** (contains `Evaluated against <head_short>`) APPROVED bot review
+that has a findings table:
+
+A finding is **unacknowledged** if it does not appear as `Finding #N` in a fix plan
+comment from the bot user for this HEAD SHA.
+
+If any unacknowledged findings exist:
+- Fix plan exists: skip
+- No fix plan: → Acquire WIP, emit `SPAWN:address-feedback:<pr_num>:<head_sha>`
+
+### Rule 9: Unresolved Inline Diff Comments
+
+An inline diff comment is **unresolved** if:
+1. `in_reply_to_id` is null (top-level comment)
+2. `resolver` is null (not formally resolved)
+3. No other comment has `in_reply_to_id` pointing to this comment (no reply)
+
+If unresolved comments exist:
+- Fix plan exists: skip
+- No fix plan: → Acquire WIP, emit `SPAWN:address-feedback:<pr_num>:<head_sha>`
+
+### Rule 10: Handoff
+
+All rules above passed. Verify all bot reviews are current (contain `Evaluated against <head_short>`).
+
+If all current:
+- Apply `ready` label
+- Assign to `aweiker`
+- Emit `HANDOFF:<pr_num>`
+- Continue evaluating remaining PRs (do NOT exit)
+
+If already assigned to `aweiker`: skip (assume handoff was already performed; continue to next PR without emitting another HANDOFF).
+
+### Rule 11: New Issue Pickup
+
+Only runs if: no open PRs exist AND `ACTIVE_WIP == 0`.
+
+Fetch open, unassigned issues. Priority: bugs first, then by number ascending.
+
+Claim the issue (assign to bot user to prevent double-pick), then:
+→ Emit `SPAWN:impl:<issue_num>:`
+
+---
+
+## 6. Safety Invariants
+
+These are statically checked by `~/.openclaw/workspace/scripts/test/check-invariants.sh` and enforced in all changes:
+
+| ID | Invariant |
+|----|-----------|
+| S1 | Zero merge API calls in dispatch script (`/merge` does not appear) |
+| S2 | REQUEST_CHANGES check (Rule 2) appears before CI check (Rule 4) |
+| S3 | REQUEST_CHANGES check (Rule 2) appears before ready label application (Rule 10) |
+| S4 | No model/AI API references in dispatch script |
+| S5 | `set -euo pipefail` present |
+| S6 | Active WIP does not cause early exit (only sets ACTIVE_WIP flag) |
+| S7 | SPAWN:impl guarded by `ACTIVE_WIP == 0` check |
+| S8 | No merge calls in any worker template |
+
+---
+
+## 7. Error Handling
+
+| Error | Behavior |
+|-------|----------|
+| `curl` returns error | `set -euo pipefail` aborts script — no partial actions |
+| `jq` parse error | Script aborts |
+| Worker crashes | WIP label left on PR; stale WIP cleanup (Rule 0) removes it after 1hr |
+| Race: two crons fire | WIP mutex prevents double-dispatch for same PR |
+| `sessions_spawn` fails | Worker not spawned; WIP label orphaned → cleaned in 1hr |
+| Config file missing | Exit code 2 with error message |
+
+---
+
+## 8. Worker Templates
+
+Each worker receives a precise task description with substituted values:
+
+| Template | Trigger | Key job |
+|----------|---------|---------|
+| `self-review.md` | No clean self-review | Post self-review comment, remove WIP |
+| `sr-fix.md` | Self-review needs attention | Address self-review findings, push, remove WIP |
+| `ci-fix.md` | CI failing | Diagnose, fix, push, remove WIP |
+| `address-feedback.md` | Unacknowledged findings or inline comments | Address feedback, push, remove WIP |
+| `findings.md` | REQUEST_CHANGES present | Address REQUEST_CHANGES, push, remove WIP |
+| `rebase.md` | Merge conflicts | Rebase on main, push, remove WIP |
+| `impl.md` | New issue | Implement feature/fix, open PR |
+
+Workers **always** remove the WIP label on completion and reply `NO_REPLY`.
+
+---
+
+## 9. Fixes for Issues #144 and #145
+
+**Issue #144** (autonomous merge):
+The dispatch script contains no merge API calls anywhere. The `~/.openclaw/workspace/scripts/test/check-invariants.sh`
+invariant `S1` verifies this. Workers do not receive merge instructions.
+
+**Issue #145** (merged despite REQUEST_CHANGES):
+Rule 2 is the **first** rule evaluated per PR. It cannot be skipped, reasoned past,
+or bypassed. It is checked before CI, before self-review, before handoff. The check
+uses latest-per-reviewer state, so a reviewer who re-approved after REQUEST_CHANGES
+is correctly handled.

review/docmap.go

@@ -310,11 +310,11 @@ func readFileBytes(path string) ([]byte, error) {
 	return os.ReadFile(path)
 }
 
-// ValidateDocPath rejects doc paths that could cause path traversal via the
-// VCS API (absolute paths, any ".." segment, backslashes). Defense-in-depth:
-// the VCS API should already scope paths to the repo, but we validate locally
-// to avoid any quirk in backend path handling. Backslashes are rejected
-// explicitly to prevent Windows platform edge cases.
+// ValidateDocPath rejects doc paths that could cause path traversal
+// (absolute paths, any ".." segment, backslashes). Defense-in-depth: callers
+// must also confine the joined path to the repo root via filepath.Rel before
+// any filesystem access. Backslashes are rejected explicitly to prevent
+// Windows platform edge cases.
 func ValidateDocPath(p string) error {
 	if strings.Contains(p, "\\") {
 		return fmt.Errorf("backslashes not allowed in doc paths")