Add zen_review and zen_review_resume MCP tools#5
Conversation
eslerm
commented
Mar 20, 2026
eslerm
commented
- Auto-install /review-pr Claude command before launching sessions
- Extract worktree creation logic into shared internal/review package
- Add MCP zen_review and zen_review_resume tools
- Refactor cmd/review.go to use shared review.CreateWorktree()
Prevent indefinite hangs when GitHub is slow or unreachable by setting a 30s timeout on the OAuth2 HTTP client and all exec.CommandContext calls. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
When `zen review` or `zen review resume` launches Claude with `/review-pr`, the command file may not exist if the user skipped `zen setup`. This adds `ensureClaudeCommand()` which silently installs from the embedded FS when missing, so the user never hits an unknown-command error. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Extract worktree creation logic from cmd/review.go into shared internal/review package so both the CLI and MCP server can use it. - internal/review: CreateWorktree() and DetectRepo() shared functions - MCP zen_review: creates worktree for a PR (not read-only, open-world) - MCP zen_review_resume: returns worktree path + sessions (read-only) - Refactor cmd/review.go runReview() to use review.CreateWorktree() - Tests for missing params and non-existent worktree cases Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The codebase had a smart lock removal function (removeStaleLock) that checks if the holding PID is still alive before removing, but three call sites were using a bare os.Remove which could race with concurrent git operations. Export RemoveStaleLock and use it everywhere. Affected: internal/review, cmd/work, internal/reconciler/setup Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- ensureClaudeCommand: use os.UserHomeDir() instead of os.Getenv("HOME")
for robustness in unusual environments
- DetectRepo: replace single-field match struct with plain []string
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
mgreau
left a comment
mgreau
left a comment
There was a problem hiding this comment.
internal/review/review.go lines 81 & 89: git commands ignore context cancellation
The git fetch and git worktree add calls use exec.Command instead of exec.CommandContext, so a cancelled context (e.g. MCP client disconnects) won't interrupt them. Since GitMu is still held while they run, a hung git operation blocks all other worktree operations indefinitely.
Suggested fix:
fetchCmd := exec.CommandContext(ctx, "git", "fetch", "origin", fmt.Sprintf("+pull/%d/head:%s", prNumber, branchName))
// ...
wtCmd := exec.CommandContext(ctx, "git", "worktree", "add", worktreePath, branchName)
mgreau
left a comment
mgreau
left a comment
There was a problem hiding this comment.
internal/mcp/server.go: zen_review_resume missing repo parameter
The tool searches all worktrees by pr_number only. If two configured repos both have a PR #N (e.g. a fork and upstream), handleReviewResume returns whichever ListAll surfaces first — silently wrong.
Suggest adding an optional repo parameter to match the pattern already established by zen_review:
mcpgo.WithString("repo", mcpgo.Description("Short repo name (auto-detected if omitted)")),Then filter worktrees by repo name before matching on PRNumber.
mgreau
left a comment
mgreau
left a comment
There was a problem hiding this comment.
internal/review/review.go + internal/mcp/tools.go: Result doesn't distinguish new vs resumed
When CreateWorktree returns early because a worktree already exists, the Result is identical to a freshly-created one. MCP consumers (e.g. an agent calling zen_review) can't tell whether to say "created" or "resumed", and can't know they skipped the fetch/inject steps.
Suggest adding a boolean field to Result:
type Result struct {
WorktreePath string `json:"worktree_path"`
PRNumber int `json:"pr_number"`
Title string `json:"title"`
Author string `json:"author"`
Resumed bool `json:"resumed"` // true if worktree already existed
}Set Resumed: true in the early-return path.
2 participants
