Fix review agent and hook working directory for worktree commits

[ryan-mahoney]

Summary

When commits are made in git worktrees, roborev resolved the path to the main repository root before passing it to review agents and hooks. This caused two problems: review agents searched the main checkout (wrong branch) instead of the worktree, producing false positives for files that only existed on the feature branch; and post-review hooks like roborev refine ran in the main repo root, refusing to operate because it detected the main branch instead of the feature branch.

• Adds worktree_path column to review_jobs to persist the originating worktree path per job
• Detects worktree divergence in handleEnqueue by comparing GetRepoRoot vs GetMainRepoRoot
• Worker uses WorktreePath as the agent's working directory and prompt builder path, with fallback to the main repo root if the worktree no longer exists
• Hooks now run in the worktree directory, so roborev refine operates on the correct branch
• Config resolution, event broadcasting, and fix-job worktree creation continue to use the main repo root

Changes

• Add WorktreePath field to ReviewJob and EnqueueOpts
• feat(storage): add worktree_path column migration (Add roborev install-hook uninstall to uninstall the hook #1)
• Persist WorktreePath in EnqueueJob INSERT and returned struct (Add roborev install-hook uninstall to uninstall the hook #1)
• Read worktree_path in ClaimJob, GetJobByID, and ListJobs (Add roborev install-hook uninstall to uninstall the hook #1)
• Detect worktree and pass WorktreePath in handleEnqueue (Add roborev install-hook uninstall to uninstall the hook #1)
• feat(daemon): use worktree path for agent cwd and prompt building (Add roborev install-hook uninstall to uninstall the hook #1)
• test(storage): verify ReenqueueJob preserves worktree_path (Add roborev install-hook uninstall to uninstall the hook #1)
• feat(daemon): use worktree path for hook working directory (Add roborev install-hook uninstall to uninstall the hook #1)
• fix(daemon): address PR review feedback for worktree path handling (Add roborev install-hook uninstall to uninstall the hook #1)

[roborev-ci]

roborev: Combined Review (9f1246e)

Verdict: The PR adds worktree_path persistence but contains a medium-severity bug affecting directory resolution for non-worktree users.

Medium

• Location: internal/daemon/server.go:710
• Problem: worktreePath is set whenever gitCwd != repoRoot, which also happens when the CLI is run from a subdirectory inside a normal checkout. In that case the code stores the subdirectory path as
  WorktreePath, and later the worker and hook runner use that subdirectory as the review cwd. That changes behavior for non-worktree users and can break prompt-building or hooks that expect the repository root.
• Fix: Resolve and compare the actual checkout root for the current worktree against the main
  repo root, and only persist WorktreePath when those two roots differ. Add a test that enqueues from a repo subdirectory and verifies no worktree path is stored.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[ryan-mahoney]

roborev: Combined Review (9f1246e)

Verdict: The PR adds worktree_path persistence but contains a medium-severity bug affecting directory resolution for non-worktree users.

Medium

• Location: internal/daemon/server.go:710
• Problem: worktreePath is set whenever gitCwd != repoRoot, which also happens when the CLI is run from a subdirectory inside a normal checkout. In that case the code stores the subdirectory path as
  WorktreePath, and later the worker and hook runner use that subdirectory as the review cwd. That changes behavior for non-worktree users and can break prompt-building or hooks that expect the repository root.
• Fix: Resolve and compare the actual checkout root for the current worktree against the main
  repo root, and only persist WorktreePath when those two roots differ. Add a test that enqueues from a repo subdirectory and verifies no worktree path is stored.

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

No, this specific bug report is not true for the current code.

• In handleEnqueue, gitCwd is computed with git.GetRepoRoot(req.RepoPath), which resolves to the checkout root (not the caller’s subdirectory).
• repoRoot comes from git.GetMainRepoRoot, and for normal repos it falls back to GetRepoRoot (same value) in internal/git/git.go.
• The CLI already normalizes to repo root before enqueueing in review.go and sends that root as RepoPath in review.go.

So “run from a subdirectory in a normal checkout stores subdirectory as WorktreePath” does not match this implementation.

The suggested regression test is still a reasonable hardening test, but this specific medium-severity finding appears to be a false positive.

[roborev-ci]

roborev: Combined Review (4821cd0)

Verdict: The pull request successfully propagates worktree paths but introduces a high-severity security risk and a medium-severity correctness issue related to deleted worktrees.

High Severity

• Location: internal/daemon/hooks.go :171, internal/daemon/worker.go:447
• Problem: handleEvent changes the hook working directory from the stable repo root to the reviewed worktree when the path exists. Any hook command that uses a relative executable or reads helper files from the current directory can
  resolve and execute attacker-controlled content from an untrusted branch/worktree. In a shared repo, reviewing a contributor's worktree could trigger arbitrary local code execution through an otherwise trusted hook configuration.
• Fix: Keep hook execution rooted at event.Repo by default, and pass Worktree Path to hooks as data (e.g., via an environment variable or explicit argument) instead of changing cwd. If worktree-based execution is required, make it an explicit opt-in and resolve hook executables to absolute trusted paths before launching them.

Medium Severity

• Location:
  internal/daemon/worker.go:362, internal/daemon/worker.go:394
• Problem: When a queued dirty review came from a worktree that has since been removed, processJob silently falls back to job.RepoPath. BuildDirty will then pull file context from the main checkout instead of the captured worktree state, meaning the generated review can be based on the wrong files.
• Fix: For dirty jobs with a missing WorktreePath, fail the job explicitly, or persist the needed file context at enqueue time so prompt construction does not
  depend on the worktree still existing.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (db8e5eb)

The PR successfully propagates worktree_path for review jobs, but introduces medium-severity issues regarding path resolution and validation
that need addressing.

Findings

Medium

• Location: internal/daemon/server.go (lines 710-713)

  • Problem: Worktree detection uses if gitCwd != repoRoot { worktreePath = gitCwd }. This
    will misclassify any invocation from a subdirectory of the main checkout as a worktree job, causing prompt building, config loading, and hooks to run from the subdirectory instead of the repository root.
  • Fix: Resolve and store the actual worktree root explicitly, then compare that root to the canonical repo root.
    Do not use an arbitrary cwd/subdirectory as worktree_path.

• Location: internal/daemon/worker.go (lines 362-372), internal/daemon/hooks.go (lines 137-147, 181
  -186), internal/storage/sync.go (line 579)

  • Problem: worktree_path is persisted, synchronized, and then trusted by the worker and hook runner after only an os.Stat existence check. A stale or crafted synced job from a shared
    database could point worktree_path at any existing local directory. This crosses a trust boundary and could lead to reviewing the wrong repository, leaking unrelated local contents to the model, or executing hook commands from an attacker-chosen local checkout.
  • Fix: Do not trust or sync raw absolute worktree _path values across machines without validation. Before using a stored/synced worktree path, canonicalize it (filepath.Abs + filepath.EvalSymlinks) and verify it is a real Git worktree attached to RepoPath (e.g., via git worktree list --porcelain). If
    validation fails, clear the field and fall back to RepoPath.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (5b4b4cc)

Verdict: The PR successfully adds worktree tracking support, but contains a logic flaw in worktree path resolution when executing from subdirectories.

Medium

• Location: internal/daemon/server.go:710-716
• Problem: worktreePath is derived by directly comparing gitCwd to repoRoot. If the CLI is invoked from a subdirectory, gitCwd will
  be that subdirectory rather than the checkout root. This causes main-repo subdirectories to be falsely identified as worktrees, and worktree subdirectories to be saved incorrectly. Later validation will silently reject these paths and fall back to the main repository, breaking the intended behavior.
• Fix: Resolve the top-level
  checkout directory before comparing or persisting it. For example, resolve git.GetRepoRoot(gitCwd) and compare that against repoRoot instead of comparing the raw gitCwd.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[wesm]

False positive. Will merge this once CI green

[ryan-mahoney]

Thank you @wesm - I've been enjoying using this service and was delighted to be able to contribute

[roborev-ci]

roborev: Combined Review (4c9510b)

Verdict: The PR successfully propagates worktree_path throughout the application, but introduces medium-severity issues regarding configuration resolution and session reuse in worktrees.

Medium

• Location: internal/daemon/ server.go (around config.IsBranchExcluded(repoRoot, currentBranch) in handleEnqueue)

  • Problem: Enqueue-time branch exclusion resolves config from the main repo root instead of the worktree root. A review started from a worktree might use different .roborev. toml rules during enqueueing than those used later for the same job.
  • Fix: Resolve branch-exclusion rules from the effective checkout root (checkoutRoot / validated worktreePath), or intentionally normalize all config lookups to a single path.

• Location: internal/daemon/server.go (at findReusableSessionID(...) calls for dirty/range/single-commit jobs)

  • Problem: Session reuse keys jobs only by repo/branch/SHA and ignores the new worktree_path. Two reviews from different checkouts of the same repository might reuse the same agent session, risking
    the leakage of stale conversation context into the wrong checkout because prompt construction and repo config can differ by worktree.
  • Fix: Include the effective checkout root or worktree_path in session-reuse matching, or disable session reuse when the checkout root differs.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (71fd194)

Verdict: The code is clean; no medium or higher severity issues were found across the reviewed changes.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)