Multi-repo run scanner

[shahar-caura]

FORGE-16

Closes #61

Effort: S

Problem

Forge only knows about runs in the current repo's .forge/runs/ directory. Users working across multiple repos have no unified view of all their forge activity. The dashboard needs a way to discover and aggregate runs from every repo on disk.

Solution

Add a scanner package in internal/scanner/ that discovers .forge/runs/ directories across configured repo paths. It accepts a list of root directories (e.g., ~/code) and recursively finds repos containing .forge/runs/. Returns a unified list of RunState objects tagged with their source repo path and repo name.

File Manifest

Action	File
Create	internal/scanner/scanner.go
Create	internal/scanner/scanner_test.go

Key Changes

• ScanRepos(roots []string) ([]RepoRuns, error) — walks directories, finds .forge/runs/*.yaml files
• RepoRuns struct: RepoPath string, RepoName string, Runs []state.RunState
• Skips hidden directories (except .forge), respects symlinks, handles permission errors gracefully
• Uses state.Load() to deserialize each run YAML

Interface Changes

New public API:

type RepoRuns struct {
    RepoPath string
    RepoName string
    Runs     []state.RunState
}

func ScanRepos(roots []string) ([]RepoRuns, error)

Acceptance Criteria

• Discovers .forge/runs/ in nested repo directories
• Returns runs tagged with repo path and derived repo name
• Handles missing/empty .forge/runs/ directories gracefully
• Skips directories it can't read (permission errors) without failing
• make test passes

Verify

go test ./internal/scanner/... -v

Anti-Goals

• Do NOT add CLI flags or config for repo roots yet — that comes with forge serve
• Do NOT watch for filesystem changes — that's a separate concern (fsnotify)
• Do NOT aggregate or deduplicate runs — return raw data, consumers filter

Context for Agent

Read internal/state/state.go for the RunState struct and Load() function. The scanner is a pure discovery layer — it reads state files and returns them, nothing more.

[github-actions]

Claude encountered an error —— View job

---

I'll analyze this and get back to you.

[github-actions]

Claude encountered an error —— View job

---

I'll analyze this and get back to you.

[github-actions]

Claude encountered an error —— View job

---

I'll analyze this and get back to you.

[github-actions]

Claude encountered an error —— View job

---

I'll analyze this and get back to you.

[github-actions]

Claude finished @shahar-caura's task in 3m 45s —— View job

---

PR Review: Multi-repo run scanner

• Gather context (read changed files)
• Review scanner implementation
• Review supporting changes
• Post full review

---

Summary

This PR adds a scanner package that walks configured root directories to discover .forge/runs/ directories and load their RunState objects. The scanner is well-structured, handles the key edge cases (hidden dirs, symlinks, permission errors), and comes with solid test coverage. The supporting changes are mostly cosmetic formatting/grouping fixes.

There is one significant correctness issue (global state race) and a few medium/low issues to address before merge.

---

1. Race condition: scanner's SetRunsDir races with HTTP server handlers

Severity: High
File: internal/scanner/scanner.go:145-153
Problem: stateLoadMu only serializes calls within the scanner package, but internal/server/handlers.go:94 and internal/server/sse.go:73 call state.Load() without holding that lock, so the temporary SetRunsDir() mutation races with concurrent HTTP requests in forge serve.

The scanner temporarily changes state.runsDir (a package-level var with no internal protection) to point at an external repo's runs dir. If a server handler calls state.Load() during that window, it reads from the wrong directory — a silent data corruption bug.

// scanner.go:145
func loadRun(runsDir, id string) (*state.RunState, error) {
    stateLoadMu.Lock()
    defer stateLoadMu.Unlock()
    state.SetRunsDir(runsDir)        // mutates global state.runsDir
    defer state.SetRunsDir(defaultRunsDir)
    return state.Load(id)            // server handlers can race here
}

🤖 Claude Code Prompt (click to copy)

Fix the race condition in internal/scanner/scanner.go between the scanner's loadRun function and HTTP server handlers that call state.Load() concurrently.

The root cause: loadRun temporarily mutates state.runsDir (a package-level var in internal/state/state.go) via state.SetRunsDir(), but server handlers in internal/server/handlers.go and internal/server/sse.go call state.Load() without holding the scanner's mutex.

The fix: bypass state.SetRunsDir entirely. Instead of mutating global state to point Load() at a different directory, read the YAML file directly in the scanner.

In internal/scanner/scanner.go, replace the loadRun function:

Current:
```go
var stateLoadMu sync.Mutex

func loadRun(runsDir, id string) (*state.RunState, error) {
	stateLoadMu.Lock()
	defer stateLoadMu.Unlock()
	state.SetRunsDir(runsDir)
	defer state.SetRunsDir(defaultRunsDir)
	return state.Load(id)
}

Replace with a self-contained YAML reader that doesn't touch global state:

func loadRun(runsDir, id string) (*state.RunState, error) {
	path := filepath.Join(runsDir, id+".yaml")
	data, err := os.ReadFile(path)
	if err != nil {
		return nil, err
	}
	var rs state.RunState
	if err := yaml.Unmarshal(data, &rs); err != nil {
		return nil, fmt.Errorf("unmarshal %s: %w", path, err)
	}
	return &rs, nil
}

Also remove the stateLoadMu variable and the "sync" import since they're no longer needed. Add "gopkg.in/yaml.v3" to the imports in internal/scanner/scanner.go (check go.mod to confirm the import path).

Do not modify internal/state/state.go.

</details>

---

### 2. `defer state.SetRunsDir(defaultRunsDir)` doesn't restore previous value
**Severity**: Medium
**File**: `internal/scanner/scanner.go:150`
**Problem**: The deferred restore always resets to the hardcoded constant `".forge/runs"`, so if any caller previously set a different `runsDir` (e.g., via `cmd_serve.go:22`), the scanner silently clobbers that setting.

```go
state.SetRunsDir(runsDir)
defer state.SetRunsDir(defaultRunsDir)  // never reads prior value

This issue becomes moot if issue #1 is fixed by removing the SetRunsDir usage entirely.

🤖 Claude Code Prompt (click to copy)

In internal/scanner/scanner.go, the loadRun function (lines 145-153) uses defer state.SetRunsDir(defaultRunsDir) to restore the global state after temporarily changing it, but it hardcodes the restore value instead of capturing the prior value.

The best fix is to remove the SetRunsDir usage entirely by reading YAML directly (see race condition fix). But if you want a minimal fix for this specific issue:

Replace:
```go
state.SetRunsDir(runsDir)
defer state.SetRunsDir(defaultRunsDir)

With:

prev := state.GetRunsDir() // you'd need to add GetRunsDir() to internal/state/state.go
state.SetRunsDir(runsDir)
defer state.SetRunsDir(prev)

Add to internal/state/state.go:

// GetRunsDir returns the current runs directory path.
func GetRunsDir() string { return runsDir }

</details>

---

### 3. Silent error suppression in `loadRuns`
**Severity**: Medium
**File**: `internal/scanner/scanner.go:135-139`
**Problem**: When `loadRun` fails for an individual YAML file (e.g., corrupted YAML, unexpected format), the error is silently swallowed with `continue`. There's no way for an operator to know that a run file was skipped.

```go
rs, err := loadRun(runsDir, id)
if err != nil {
    continue  // no log, no error collection — run silently disappears
}

🤖 Claude Code Prompt (click to copy)

In internal/scanner/scanner.go, the loadRuns function (lines 113-143) silently drops individual run YAML files that fail to load. Corrupted or malformed YAML files disappear without any trace.

Fix: return a slice of errors alongside the successful runs, or at minimum collect skipped-file paths into a warnings list. Since the scanner is a pure discovery layer, the lightweight fix is to return skipped files in a new field.

Alternatively (simpler): change the signature of loadRuns to accept a slog.Logger and log at Warn level when a run file fails to parse:

Change loadRuns to accept a logger parameter, then update the call site in ScanRepos to pass a logger. Add a logger field or parameter to ScanRepos if needed, or accept nil and use slog.Default().

Example:
```go
func loadRuns(runsDir string, logger *slog.Logger) ([]state.RunState, error) {
    // ... existing code ...
    rs, err := loadRun(runsDir, id)
    if err != nil {
        if logger != nil {
            logger.Warn("skipping unreadable run file", "file", name, "error", err)
        }
        continue
    }
    // ...
}

Update the ScanRepos function signature to accept an optional logger, or use slog.Default() internally. Keep the function signature change minimal.

</details>

---

### 4. Dead code inside symlink check
**Severity**: Low
**File**: `internal/scanner/scanner.go:52-57`
**Problem**: `filepath.WalkDir` uses `os.Lstat` internally, so a symlink's `DirEntry` always has `ModeSymlink` set and `ModeDir` unset. `d.IsDir()` inside the symlink branch always returns `false`, making `filepath.SkipDir` unreachable.

```go
if d.Type()&os.ModeSymlink != 0 {
    if d.IsDir() {           // always false for symlinks via WalkDir
        return filepath.SkipDir  // unreachable
    }
    return nil
}

🤖 Claude Code Prompt (click to copy)

In internal/scanner/scanner.go lines 52-57, the symlink guard has dead code. filepath.WalkDir uses os.Lstat, so any DirEntry with ModeSymlink set will never also have IsDir() return true — the inner branch is unreachable.

Simplify to:
```go
if d.Type()&os.ModeSymlink != 0 {
    return nil
}

This preserves the intended behavior (skip all symlinks) while removing the dead inner branch.

</details>

---

### 5. Pre-commit hook runs full test suite — too slow
**Severity**: Low
**File**: `lefthook.yml:13-15`
**Problem**: Adding `go test -race ./...` to the pre-commit hook means every `git commit` runs the full test suite. Pre-commit hooks should be fast (formatting, linting). `make test` already runs on pre-push.

```yaml
pre-commit:
  commands:
    test:
      glob: "*.go"
      run: go test -race ./...  # runs on every commit — slow

🤖 Claude Code Prompt (click to copy)

In lefthook.yml, remove the `test` command from the pre-commit section. The full test suite (go test -race ./...) is already run by the pre-push hook (via make test in .githooks/pre-push). Running it on pre-commit slows down every commit unnecessarily.

Remove these lines from lefthook.yml:
```yaml
    test:
      glob: "*.go"
      run: go test -race ./...

Keep the fmt and lint commands in pre-commit. Tests belong in pre-push.

</details>

---

### 6. Committed test fixture YAMLs contain volatile paths and timestamps
**Severity**: Low
**File**: `internal/pipeline/.forge/runs/20260217-120000-test.yaml`, `internal/pipeline/.forge/runs/20260219-120000-push-test.yaml`
**Problem**: These files contain local machine temp paths (random test suffixes) and timestamps that change on every test run, generating noisy diffs. This is pre-existing but gets worse as more test runs happen.

<details>
<summary>🤖 Claude Code Prompt (click to copy)</summary>

The files internal/pipeline/.forge/runs/20260217-120000-test.yaml and internal/pipeline/.forge/runs/20260219-120000-push-test.yaml contain machine-specific temp paths (e.g., /var/folders/tx/.../TestRun_LocalCR_SeparateReviewAgent3223185930/...) and timestamps that change on every test run.

Find where these files are written by the test suite (likely in internal/pipeline tests). The fix is to either:

1. Gitignore internal/pipeline/.forge/runs/*.yaml, OR
2. Make the tests that write these fixtures use t.TempDir() for the runs directory instead of committing files to the repo, OR
3. Replace the machine-specific values with stable placeholder values (e.g., plan_path: /tmp/test.md, timestamps: fixed values)

Option 1 is simplest: add internal/pipeline/.forge/runs/*.yaml to .gitignore. Check that no other test depends on these committed files being present on disk.

</details>

---

## 🚀 Fix All Prompts

### 🟠 Fix All High Priority Issues

Please fix the following HIGH PRIORITY issue:

1. Fix the race condition in internal/scanner/scanner.go between the scanner's loadRun function and HTTP server handlers that call state.Load() concurrently.

The root cause: loadRun temporarily mutates state.runsDir (a package-level var in internal/state/state.go) via state.SetRunsDir(), but server handlers in internal/server/handlers.go and internal/server/sse.go call state.Load() without holding the scanner's mutex.

The fix: bypass state.SetRunsDir entirely. Instead of mutating global state to point Load() at a different directory, read the YAML file directly in the scanner.

In internal/scanner/scanner.go, replace the loadRun function and remove stateLoadMu:

Current:

var stateLoadMu sync.Mutex

func loadRun(runsDir, id string) (*state.RunState, error) {
	stateLoadMu.Lock()
	defer stateLoadMu.Unlock()
	state.SetRunsDir(runsDir)
	defer state.SetRunsDir(defaultRunsDir)
	return state.Load(id)
}

Replace with:

func loadRun(runsDir, id string) (*state.RunState, error) {
	path := filepath.Join(runsDir, id+".yaml")
	data, err := os.ReadFile(path)
	if err != nil {
		return nil, err
	}
	var rs state.RunState
	if err := yaml.Unmarshal(data, &rs); err != nil {
		return nil, fmt.Errorf("unmarshal %s: %w", path, err)
	}
	return &rs, nil
}

Remove the stateLoadMu variable and the "sync" import. Add "gopkg.in/yaml.v3" to imports. Do not modify internal/state/state.go. Run make test to verify.

### 🟡 Fix All Medium/Low Issues

Please fix the following issues in the forge codebase on branch FORGE-16-multi-repo-run-scanner:

1. Silent error suppression in internal/scanner/scanner.go loadRuns function (lines 135-139).
   Change the loadRuns call site to log skipped run files at Warn level. Add a slog.Logger parameter to loadRuns, or use slog.Default(). Example:

rs, err := loadRun(runsDir, id)
if err != nil {
    slog.Default().Warn("skipping unreadable run file", "file", name, "error", err)
    continue
}

2. Dead code in symlink check in internal/scanner/scanner.go lines 52-57.
   filepath.WalkDir uses os.Lstat so d.IsDir() is always false for symlinks. Simplify to:

if d.Type()&os.ModeSymlink != 0 {
    return nil
}

3. Pre-commit hook runs full test suite — too slow. In lefthook.yml, remove the test command from pre-commit (go test -race ./...). Tests already run in pre-push via make test. Remove:

    test:
      glob: "*.go"
      run: go test -race ./...

4. Committed test fixture YAMLs in internal/pipeline/.forge/runs/ contain volatile machine-specific temp paths and timestamps. Add internal/pipeline/.forge/runs/*.yaml to .gitignore and verify no tests depend on these committed files. Run make test to confirm all tests still pass.