Skip to content

Add repo picker to spawn dialog and auto-worktree on project collision#119

Merged
kjgbot merged 7 commits into
mainfrom
claude/maestro-app-comparison-9EQ7H
Jun 6, 2026
Merged

Add repo picker to spawn dialog and auto-worktree on project collision#119
kjgbot merged 7 commits into
mainfrom
claude/maestro-app-comparison-9EQ7H

Conversation

@khaliqgant
Copy link
Copy Markdown
Member

@khaliqgant khaliqgant commented Jun 6, 2026

User description

  • SpawnAgentDialog: show root selector dropdown when project has multiple roots,
    passing the chosen root as cwd to spawnProjectAgent/spawnProjectPersona
  • spawn-agent: accept rootOverride parameter to spawn into any root, not just
    the active one
  • project:add-root IPC: detect when selected path already belongs to another
    project and return a conflict descriptor instead of silently reusing it
  • project:create-worktree-root IPC: given a repo path, discovers the git root,
    creates a new worktree at ~/.pear/worktrees/{projectId}/{repo} on a fresh
    pear/{project-slug} branch, and registers it as a root
  • ProjectSettings: render an inline conflict banner with "Create worktree" and
    "Go to existing project" buttons when a collision is detected
  • project-store: expose pendingRootConflict state, clearRootConflict, and
    createWorktreeRoot actions
  • git: add getGitRoot and createWorktree helpers

https://claude.ai/code/session_01MhKe5JsErX12X8Wbjv9aU2


CodeAnt-AI Description

Let users choose the spawn folder and avoid terminal input clashes with multiple agents

What Changed

  • The agent spawn dialog now lets users pick which project root to spawn into when a project has more than one root.
  • If a user tries to add a folder that already belongs to another project, the app now warns them instead of reusing it silently, and offers to create an isolated git worktree or open the existing project.
  • Typing in a terminal with multiple agents running now automatically puts that terminal into hold mode so input is queued until Enter is pressed or the terminal loses focus.

Impact

✅ Fewer wrong-folder agent spawns
✅ Safer project setup when repos are shared
✅ Fewer accidental terminal commands sent to the wrong agent

💡 Usage Guide

Checking Your Pull Request

Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.

Talking to CodeAnt AI

Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:

@codeant-ai ask: Your question here

This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.

Example

@codeant-ai ask: Can you suggest a safer alternative to storing this secret?

Preserve Org Learnings with CodeAnt

You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:

@codeant-ai: Your feedback here

This helps CodeAnt AI learn and adapt to your team's coding style and standards.

Example

@codeant-ai: Do not flag unused imports.

Retrigger review

Ask CodeAnt AI to review the PR again, by typing:

@codeant-ai: review

Check Your Repository Health

To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.

claude added 2 commits June 6, 2026 07:32
- SpawnAgentDialog: show root selector dropdown when project has multiple roots,
  passing the chosen root as cwd to spawnProjectAgent/spawnProjectPersona
- spawn-agent: accept rootOverride parameter to spawn into any root, not just
  the active one
- project:add-root IPC: detect when selected path already belongs to another
  project and return a conflict descriptor instead of silently reusing it
- project:create-worktree-root IPC: given a repo path, discovers the git root,
  creates a new worktree at ~/.pear/worktrees/{projectId}/{repo} on a fresh
  pear/{project-slug} branch, and registers it as a root
- ProjectSettings: render an inline conflict banner with "Create worktree" and
  "Go to existing project" buttons when a collision is detected
- project-store: expose pendingRootConflict state, clearRootConflict, and
  createWorktreeRoot actions
- git: add getGitRoot and createWorktree helpers

https://claude.ai/code/session_01MhKe5JsErX12X8Wbjv9aU2
When a user starts typing in any terminal and more than one agent is running
in the project, the terminal automatically switches to drive (hold) mode so
keystrokes are queued rather than immediately injected into the agent. This
prevents accidental input interference across agents.

- useTerminal: accept autoHold flag plus onAutoHoldStart/onAutoHoldRelease
  callbacks; on first keypress in passthrough mode fires onAutoHoldStart; on
  Enter fires onAutoHoldRelease(flush:true); on container blur fires
  onAutoHoldRelease(flush:false) to release hold without flushing
- TerminalInstance: threads autoHold + callback props through to useTerminal
- TerminalProject + SplitTerminalTile: thread autoHold props
- SplitTerminalPage: passes makeAutoHoldHandlers factory through to tiles
- TerminalPane: computes autoHold = runningAgents > 1; makeAutoHoldHandlers
  creates per-agent callbacks that call handleDeliveryModeChange and
  flushPending on Enter release

https://claude.ai/code/session_01MhKe5JsErX12X8Wbjv9aU2
@codeant-ai
Copy link
Copy Markdown

codeant-ai Bot commented Jun 6, 2026

CodeAnt AI is reviewing your PR.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jun 6, 2026

Ready to act? Review this PR in Change Stack to turn feedback into patch suggestions you can inspect and refine.

Review Change Stack

Warning

Review limit reached

@kjgbot, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 26 minutes and 44 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: af84a2ee-0340-4596-b3e8-387ed416c1c5

📥 Commits

Reviewing files that changed from the base of the PR and between 8af5696 and 47394a5.

📒 Files selected for processing (10)
  • src/main/ipc-handlers.ts
  • src/main/store.ts
  • src/preload/index.ts
  • src/renderer/src/components/settings/ProjectSettings.tsx
  • src/renderer/src/components/sidebar/SpawnAgentDialog.tsx
  • src/renderer/src/components/terminal/TerminalPane.tsx
  • src/renderer/src/hooks/use-terminal.ts
  • src/renderer/src/lib/spawn-agent.ts
  • src/renderer/src/stores/project-store.ts
  • src/shared/types/ipc.ts
📝 Walkthrough

Walkthrough

Adds Git worktree helpers and IPC for creating isolated worktree roots with conflict detection, plus renderer/store/UI support for conflict resolution and root-aware agent spawning. Introduces terminal auto-hold across terminal components and the useTerminal hook to hold/release input when multiple agents run.

Changes

Multi-Root Worktree Support

Layer / File(s) Summary
Git Worktree Foundation
src/main/git.ts
Import mkdirSync, dirname, join; add getGitRoot(path) to resolve repository root, createWorktree(repoPath, worktreePath, branchName) to create worktrees, and worktreeExists(worktreePath) to detect existing worktrees.
IPC Contracts and Preload API
src/shared/types/ipc.ts, src/preload/index.ts
Add ProjectRootConflict interface; extend PearAPI.project with createWorktreeRoot(...) and PearAPI.broker with sendInput(...); expose preload wrappers for project:create-worktree-root and broker:send-input.
Main Process - Handlers & Conflict Detection
src/main/store.ts, src/main/ipc-handlers.ts
Add findProjectsWithPath usage; update project:add-root to detect existing-project conflicts and return { kind: 'conflict', ... }; implement project:create-worktree-root to resolve repo root, construct deterministic branch/worktree names, create worktree under app.getPath('userData'), and register the new root; add broker:send-input IPC handler.
Renderer Store & Conflict UI
src/renderer/src/stores/project-store.ts, src/renderer/src/components/settings/ProjectSettings.tsx
Re-export ProjectRootConflict; add pendingRootConflict state and clearRootConflict(); handle conflict-shaped responses in addRoot(...); add createWorktreeRoot(...) action to create and activate worktree roots via IPC; ProjectSettings renders a banner when pendingRootConflict is set offering create worktree, go to existing project, or cancel.
Agent Spawning with Root Override
src/renderer/src/components/sidebar/SpawnAgentDialog.tsx, src/renderer/src/lib/spawn-agent.ts
Spawn dialog adds selectedRootId and "Spawn into" dropdown for multi-root projects; passes selected root to spawnProjectAgent / spawnProjectPersona; spawn-agent functions accept optional rootOverride and use it when provided.

Terminal Auto-Hold Mode

Layer / File(s) Summary
Terminal Components - Auto-Hold Props
src/renderer/src/components/terminal/TerminalInstance.tsx, src/renderer/src/components/terminal/TerminalPane.tsx
Extend TerminalInstance props with autoHold, onAutoHoldStart, onAutoHoldRelease(flush); propagate these props through TerminalProject, SplitTerminalTile, SplitTerminalPage; TerminalPane computes autoHold from running agent count and wires makeAutoHoldHandlers to switch delivery modes and optionally flush on release.
Terminal Hook - Auto-Hold Logic
src/renderer/src/hooks/use-terminal.ts
Extend useTerminal signature with autoHold, onAutoHoldStart, onAutoHoldRelease; keep refs in sync; make sendInput async and implement auto-hold flow (start on first keystroke, route held input through pear.broker.sendInput, release on Enter with onAutoHoldRelease(true)); add capturing-phase blur handler to cancel typing and call onAutoHoldRelease(false); update call sites to use void sendInput(...).

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

🐰 I dug a path where roots once crossed,
A quiet branch to save the lost.
When agents chatter, I softly hold—
Then let the stream release, all bold.
Hop prance, new worktrees take root!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The PR title clearly describes the two main features being added: repo picker for spawn dialog and auto-worktree handling for project collision scenarios, matching the core changes across multiple files.
Description check ✅ Passed The PR description comprehensively covers all major changes including spawn dialog root selection, conflict detection, worktree creation, and terminal input handling, directly aligned with the changeset scope.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/maestro-app-comparison-9EQ7H

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codeant-ai codeant-ai Bot added the size:L This PR changes 100-499 lines, ignoring generated files label Jun 6, 2026
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces support for creating isolated git worktrees when adding a project root that conflicts with an existing project, alongside an 'auto-hold' terminal input queuing mechanism when multiple agents are running. The review feedback identifies a critical race condition in the terminal's auto-hold implementation where the first keystroke can bypass the hold mode; it is recommended to make the input sending and auto-hold handlers asynchronous. Additionally, the feedback advises using asynchronous file system operations (mkdir instead of mkdirSync) in the main process to prevent blocking the thread, and handling cases where the target git branch already exists during worktree creation.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread src/renderer/src/hooks/use-terminal.ts Outdated
Comment on lines 201 to 222
const sendInput = (data: string): void => {
if (terminalModeRef.current === 'view') return

// Auto-hold: on first keystroke with multiple agents running, switch to
// drive mode so input is queued rather than immediately injected.
if (autoHoldRef.current && !typingActiveRef.current && terminalModeRef.current === 'passthrough') {
typingActiveRef.current = true
onAutoHoldStartRef.current?.()
}

// Optimistically echo before the round trip; the engine reconciles
// against authoritative output and stays dormant on fast local links.
predictiveEchoRef.current?.onUserInput(data)
recordKeystrokeSent(data)
pear.broker.sendInputFast(projectId, agentName!, data)

// On Enter, flush the queued input and return to live mode.
if (typingActiveRef.current && data.includes('\r')) {
typingActiveRef.current = false
onAutoHoldReleaseRef.current?.(true)
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

There is a race condition when autoHold is triggered on the first keystroke. Because onAutoHoldStart is called asynchronously to switch the broker's mode to hold (i.e., drive), but pear.broker.sendInputFast is called immediately afterwards, the first keystroke is highly likely to be sent and executed in passthrough mode before the broker has finished switching modes. To prevent this, sendInput should be an async function that awaits onAutoHoldStart before sending the input.

    const sendInput = async (data: string): Promise<void> => {
      if (terminalModeRef.current === 'view') return

      // Auto-hold: on first keystroke with multiple agents running, switch to
      // drive mode so input is queued rather than immediately injected.
      if (autoHoldRef.current && !typingActiveRef.current && terminalModeRef.current === 'passthrough') {
        typingActiveRef.current = true
        try {
          await onAutoHoldStartRef.current?.()
        } catch (err) {
          typingActiveRef.current = false
          console.error('[terminal] Failed to start auto-hold:', err)
          return
        }
      }

      // Optimistically echo before the round trip; the engine reconciles
      // against authoritative output and stays dormant on fast local links.
      predictiveEchoRef.current?.onUserInput(data)
      recordKeystrokeSent(data)
      pear.broker.sendInputFast(projectId, agentName!, data)

      // On Enter, flush the queued input and return to live mode.
      if (typingActiveRef.current && data.includes('\r')) {
        typingActiveRef.current = false
        onAutoHoldReleaseRef.current?.(true)
      }
    }

Comment thread src/main/git.ts
Comment on lines +1597 to +1600
export async function createWorktree(repoPath: string, worktreePath: string, branchName: string): Promise<void> {
mkdirSync(dirname(worktreePath), { recursive: true })
await runGit(['worktree', 'add', '-b', branchName, worktreePath], repoPath)
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

If the branch specified by branchName already exists in the repository, git worktree add -b <branch> will fail. To make this more robust, check if the branch already exists using listBranches and omit the -b flag if it does. Also, use the asynchronous mkdir from fs/promises instead of mkdirSync to avoid blocking the main thread.

Suggested change
export async function createWorktree(repoPath: string, worktreePath: string, branchName: string): Promise<void> {
mkdirSync(dirname(worktreePath), { recursive: true })
await runGit(['worktree', 'add', '-b', branchName, worktreePath], repoPath)
}
export async function createWorktree(repoPath: string, worktreePath: string, branchName: string): Promise<void> {
await mkdir(dirname(worktreePath), { recursive: true })
const branches = await listBranches(repoPath)
if (branches.includes(branchName)) {
await runGit(['worktree', 'add', worktreePath, branchName], repoPath)
} else {
await runGit(['worktree', 'add', '-b', branchName, worktreePath], repoPath)
}
}

Comment thread src/main/git.ts
Comment on lines +3 to 4
import { existsSync, mkdirSync } from 'fs'
import { appendFile, mkdtemp, readFile, rm, stat } from 'fs/promises'
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Avoid importing and using synchronous file system methods like mkdirSync in asynchronous functions within the Electron main process, as they can block the main thread. Instead, import mkdir from fs/promises.

Suggested change
import { existsSync, mkdirSync } from 'fs'
import { appendFile, mkdtemp, readFile, rm, stat } from 'fs/promises'
import { existsSync } from 'fs'
import { appendFile, mkdir, mkdtemp, readFile, rm, stat } from 'fs/promises'

Comment thread src/renderer/src/hooks/use-terminal.ts Outdated
Comment on lines +148 to +150
autoHold = false,
onAutoHoldStart?: () => void,
onAutoHoldRelease?: (flush: boolean) => void
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Update the type signatures of onAutoHoldStart and onAutoHoldRelease to allow returning a Promise<void> so that they can be properly awaited in sendInput.

Suggested change
autoHold = false,
onAutoHoldStart?: () => void,
onAutoHoldRelease?: (flush: boolean) => void
autoHold = false,
onAutoHoldStart?: () => void | Promise<void>,
onAutoHoldRelease?: (flush: boolean) => void | Promise<void>

Comment on lines +13 to +15
autoHold?: boolean
onAutoHoldStart?: () => void
onAutoHoldRelease?: (flush: boolean) => void
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Update the prop types of TerminalInstance to support Promise<void> return types for the auto-hold handlers.

Suggested change
autoHold?: boolean
onAutoHoldStart?: () => void
onAutoHoldRelease?: (flush: boolean) => void
autoHold?: boolean
onAutoHoldStart?: () => void | Promise<void>
onAutoHoldRelease?: (flush: boolean) => void | Promise<void>

Comment on lines +175 to +177
autoHold?: boolean
onAutoHoldStart?: () => void
onAutoHoldRelease?: (flush: boolean) => void
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Update the prop types of TerminalProject to support Promise<void> return types for the auto-hold handlers.

  autoHold?: boolean
  onAutoHoldStart?: () => void | Promise<void>
  onAutoHoldRelease?: (flush: boolean) => void | Promise<void>

Comment on lines +219 to +221
autoHold?: boolean
onAutoHoldStart?: () => void
onAutoHoldRelease?: (flush: boolean) => void
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Update the prop types of SplitTerminalTile to support Promise<void> return types for the auto-hold handlers.

  autoHold?: boolean
  onAutoHoldStart?: () => void | Promise<void>
  onAutoHoldRelease?: (flush: boolean) => void | Promise<void>

Comment on lines +367 to +368
autoHold: boolean
makeAutoHoldHandlers: (agent: Agent) => { onAutoHoldStart: () => void; onAutoHoldRelease: (flush: boolean) => void }
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Update the prop types of SplitTerminalPage to support Promise<void> return types for the auto-hold handlers.

  autoHold: boolean
  makeAutoHoldHandlers: (agent: Agent) => { onAutoHoldStart: () => void | Promise<void>; onAutoHoldRelease: (flush: boolean) => void | Promise<void> }

Comment on lines +524 to +537
const makeAutoHoldHandlers = (agent: Agent): {
onAutoHoldStart: () => void
onAutoHoldRelease: (flush: boolean) => void
} => ({
onAutoHoldStart: () => {
void handleDeliveryModeChange(agent, 'hold')
},
onAutoHoldRelease: (flush: boolean) => {
if (flush) {
void pear.broker.flushPending(agent.projectId, agent.name).catch(() => {})
}
void handleDeliveryModeChange(agent, 'auto')
}
})
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Update makeAutoHoldHandlers to return promises and await the asynchronous delivery mode changes and broker flushes.

  const makeAutoHoldHandlers = (agent: Agent): {
    onAutoHoldStart: () => Promise<void>
    onAutoHoldRelease: (flush: boolean) => Promise<void>
  } => ({
    onAutoHoldStart: async () => {
      await handleDeliveryModeChange(agent, 'hold')
    },
    onAutoHoldRelease: async (flush: boolean) => {
      if (flush) {
        await pear.broker.flushPending(agent.projectId, agent.name).catch(() => {})
      }
      await handleDeliveryModeChange(agent, 'auto')
    }
  })

Comment on lines +234 to +238
const rootData = result.kind === 'added' ? (result as { root: unknown }).root : raw
const root = parseRoot(rootData)
await get().load()
if (root) get().setActiveRoot(root.id)
return root
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: Successful root additions do not clear the previously stored conflict state, so after a conflict the UI can keep showing an outdated collision banner even when a later addRoot succeeds. Clear pendingRootConflict on the success path (before returning) to prevent stale conflict actions from being shown against the wrong state. [incomplete implementation]

Severity Level: Major ⚠️
- ⚠️ Project Settings Roots banner shows outdated conflict message.
- ⚠️ Users see wrong existing-project name after success.
- ⚠️ Conflicts appear active despite successful root addition.
Steps of Reproduction ✅
1. Open the Project Settings UI, which is implemented in
`src/renderer/src/components/settings/ProjectSettings.tsx` lines 838–907 and uses
`useProjectStore((s) => s.addRoot)` and `useProjectStore((s) => s.pendingRootConflict)`
(lines 848–851) to drive the Roots section.

2. In the Roots section (`ProjectSettings.tsx` lines 999–1072), click the "Add root"
button, which calls `run(async () => { await addRoot() })` (lines 1000–1007), and select a
directory that belongs to a different project so that `pear.project.addRoot` returns a
conflict descriptor.

3. The store's `addRoot` implementation in `src/renderer/src/stores/project-store.ts`
lines 222–231 receives this conflict result, detects `result.kind === 'conflict'`, and
sets `pendingRootConflict` via `set({ pendingRootConflict: raw as unknown as
ProjectRootConflict })`, causing the conflict banner in `ProjectSettings.tsx` lines
1030–1069 to render, but `pendingRootConflict` remains non-null in the store.

4. Without clicking any of the banner actions that call `clearRootConflict()` (`Create
worktree`, `Go to existing project`, or `Cancel` in `ProjectSettings.tsx` lines
1039–1064), click "Add root" again and now select a non-conflicting path so that
`pear.project.addRoot` returns a successful result; `addRoot` then executes the success
path at `project-store.ts` lines 234–238 (parsing the root, calling `get().load()`, and
`setActiveRoot`), but never clears `pendingRootConflict`, so `pendingRootConflict` stays
set and the collision banner continues to show stale `path`/`existingProjectName` data
even though the latest add succeeded.

Fix in Cursor | Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** src/renderer/src/stores/project-store.ts
**Line:** 234:238
**Comment:**
	*Incomplete Implementation: Successful root additions do not clear the previously stored conflict state, so after a conflict the UI can keep showing an outdated collision banner even when a later `addRoot` succeeds. Clear `pendingRootConflict` on the success path (before returning) to prevent stale conflict actions from being shown against the wrong state.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
👍 | 👎

Comment on lines +111 to 117
export async function spawnProjectPersona(project: Project, personaId: string, rootOverride?: ProjectRoot): Promise<string> {
await ensureLocalBroker(project)

const root = useProjectStore.getState().getActiveRoot()
const root = rootOverride ?? useProjectStore.getState().getActiveRoot()
if (!root?.pathExists) {
throw new Error(`Project root not found: ${root?.path || project.rootPath}`)
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Architect Review — HIGH

The new rootOverride for personas does not affect the broker workspace: listProjectPersonas and spawnProjectPersona still operate against the broker session started for the active root, so selecting a different root in SpawnAgentDialog does not actually make personas run from that selected root.

Suggestion: Thread the selected root through the persona flow so that broker context is aligned with it (either by starting/reattaching the broker for that root or by passing an explicit cwd into spawnPersona), mirroring the root-selection behavior used for regular agent spawn.

Fix in Cursor | Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖
This is an **Architect / Logical Review** comment left during a code review. These reviews are first-class, important findings — not optional suggestions. Do NOT dismiss this as a 'big architectural change' just because the title says architect review; most of these can be resolved with a small, localized fix once the intent is understood.

**Path:** src/renderer/src/lib/spawn-agent.ts
**Line:** 111:117
**Comment:**
	*HIGH: The new rootOverride for personas does not affect the broker workspace: listProjectPersonas and spawnProjectPersona still operate against the broker session started for the active root, so selecting a different root in SpawnAgentDialog does not actually make personas run from that selected root.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
If a suggested approach is provided above, use it as the authoritative instruction. If no explicit code suggestion is given, you MUST still draft and apply your own minimal, localized fix — do not punt back with 'no suggestion provided, review manually'. Keep the change as small as possible: add a guard clause, gate on a loading state, reorder an await, wrap in a conditional, etc. Do not refactor surrounding code or expand scope beyond the finding.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix

Comment on lines +1043 to +1044
clearRootConflict()
await createWorktreeRoot(pendingRootConflict.path)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: The conflict state is cleared before attempting worktree creation, so if createWorktreeRoot fails (for example non-git roots), the user loses the conflict context and cannot retry from the same banner or jump to the existing project. Only clear the conflict after a successful worktree creation (or restore it on failure). [logic error]

Severity Level: Major ⚠️
- ⚠️ Root collision banner disappears after failed worktree creation.
- ⚠️ Users lose one-click navigation to existing project.
- ⚠️ Users must re-trigger Add root flow to retry.
Steps of Reproduction ✅
1. In `ProjectSettings`
(src/renderer/src/components/settings/ProjectSettings.tsx:200–217), click the "Add root"
button, which calls `addRoot()` via `run(...)` (lines ~203–209).

2. When `pear.project.addRoot(...)` returns a `{ kind: 'conflict' }` result,
`project-store` sets `pendingRootConflict`
(src/renderer/src/stores/project-store.ts:23–32) and `ProjectSettings` renders the yellow
conflict banner when `pendingRootConflict` is truthy
(src/renderer/src/components/settings/ProjectSettings.tsx:231–271).

3. Click "Create worktree" in that banner; the handler invokes `void run(async () => {
clearRootConflict(); await createWorktreeRoot(pendingRootConflict.path) })`
(ProjectSettings.tsx:243–246). `clearRootConflict()` immediately sets
`pendingRootConflict` to `null` (project-store.ts:42–44) before `createWorktreeRoot` runs.

4. If `createWorktreeRoot` fails (e.g. `pear.project.createWorktreeRoot(...)` throws
inside project-store.ts:46–55 due to a non‑git path or IPC error), `run(...)` catches and
surfaces the error in the `error` banner (ProjectSettings.tsx:81–88), but the conflict
banner no longer renders because `pendingRootConflict` was cleared and
`createWorktreeRoot` never restores it. The user loses the "Create worktree / Go to
existing project" options without resolving the conflict.

Fix in Cursor | Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** src/renderer/src/components/settings/ProjectSettings.tsx
**Line:** 1043:1044
**Comment:**
	*Logic Error: The conflict state is cleared before attempting worktree creation, so if `createWorktreeRoot` fails (for example non-git roots), the user loses the conflict context and cannot retry from the same banner or jump to the existing project. Only clear the conflict after a successful worktree creation (or restore it on failure).

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
👍 | 👎

Comment on lines +1054 to +1055
clearRootConflict()
void setActiveProject(pendingRootConflict.existingProjectId)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: setActiveProject is async in the project store, but this click handler fire-and-forgets it without error handling. If IPC/hydration fails, the promise rejection is unhandled and the UI may remain in an inconsistent state. Route this through run(...) (or await with try/catch) so failures surface properly. [possible bug]

Severity Level: Major ⚠️
- ❌ Project switch failures surface as unhandled promise rejections.
- ⚠️ Root conflict banner cleared even when switch fails.
- ⚠️ Users see inconsistent state with no visible error message.
Steps of Reproduction ✅
1. Produce a root conflict as in suggestion 1: `addRoot()` in `ProjectSettings` triggers a
`{ kind: 'conflict' }` result, which sets `pendingRootConflict` in `project-store`
(src/renderer/src/stores/project-store.ts:23–32) and causes the conflict banner to render
(ProjectSettings.tsx:231–271).

2. Click the "Go to \"\"" button in the conflict banner; its onClick handler runs
`clearRootConflict()` and then `void
setActiveProject(pendingRootConflict.existingProjectId)`
(src/renderer/src/components/settings/ProjectSettings.tsx:253–257).

3. `setActiveProject` is asynchronous (project-store.ts:202–213): it awaits
`pear.project.setActive(id)`, then updates
`activeProjectId`/`activeRootId`/`activeChannelName`, and finally awaits `ensureBroker()`
when `id` is truthy. Because the click handler calls it with `void` and does not wrap it
in `run(...)` or a try/catch, any rejection from `pear.project.setActive` or
`ensureBroker` becomes an unhandled promise rejection.

4. On failure, the UI remains on the original project (since the store never completes the
state update), but `clearRootConflict()` has already nulled `pendingRootConflict`
(project-store.ts:42–44), so the banner disappears and the user has no in-UI feedback that
switching projects failed; the only indication is the unhandled rejection in the
console/runtime.

Fix in Cursor | Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** src/renderer/src/components/settings/ProjectSettings.tsx
**Line:** 1054:1055
**Comment:**
	*Possible Bug: `setActiveProject` is async in the project store, but this click handler fire-and-forgets it without error handling. If IPC/hydration fails, the promise rejection is unhandled and the UI may remain in an inconsistent state. Route this through `run(...)` (or await with try/catch) so failures surface properly.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
👍 | 👎

Comment on lines +528 to +530
onAutoHoldStart: () => {
void handleDeliveryModeChange(agent, 'hold')
},
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: onAutoHoldStart passes 'hold' into handleDeliveryModeChange, but that function only maps 'drive' to hold/manual-flush and treats every other value as passthrough/live mode. This means auto-hold never actually enables hold behavior, so messages keep auto-injecting while typing. Use the hold-equivalent mode that maps to manual flush ('drive') here. [logic error]

Severity Level: Major ⚠️
- ⚠️ Auto-hold never transitions terminals into held delivery.
- ⚠️ Incoming messages keep streaming while user types.
- ⚠️ Held-message flush logic is effectively never exercised.
Steps of Reproduction ✅
1. Run two or more agents so `runningAgentCount > 1`; `TerminalPane` computes `autoHold =
runningAgentCount > 1` and passes `autoHold` plus `makeAutoHoldHandlers(agent)` into
`SplitTerminalPage` and `TerminalProject`
(src/renderer/src/components/terminal/TerminalPane.tsx:540–586, 646–650).

2. `TerminalProject` forwards these to `TerminalInstance` (TerminalPane.tsx:194–203), and
`TerminalInstance` passes `autoHold`, `onAutoHoldStart`, and `onAutoHoldRelease` to
`useTerminal` (src/renderer/src/components/terminal/TerminalInstance.tsx:6–20).

3. In `useTerminal`, on the first keystroke while `terminalModeRef.current ===
'passthrough'`, auto-hold is supposed to switch the agent into drive/held mode; this is
implemented as `if (autoHoldRef.current && !typingActiveRef.current &&
terminalModeRef.current === 'passthrough') { typingActiveRef.current = true;
onAutoHoldStartRef.current?.() }` (src/renderer/src/hooks/use-terminal.ts:203–209).

4. However, `makeAutoHoldHandlers` defines `onAutoHoldStart` as `() => { void
handleDeliveryModeChange(agent, 'hold') }` (TerminalPane.tsx:524–531), but
`QueueDeliveryMode` is `'drive' | 'auto'` (PendingMessagesPane.tsx:15) and
`toTerminalMode(mode)` maps only `'drive'` to `'drive'` and all other values to
`'passthrough'` (TerminalPane.tsx:88–93). Passing `'hold'` therefore results in
`terminalMode` staying `'passthrough'`, so the broker never enters held/manual mode and
incoming messages continue to inject live while the user is typing, defeating the
auto-hold behavior.

Fix in Cursor | Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** src/renderer/src/components/terminal/TerminalPane.tsx
**Line:** 528:530
**Comment:**
	*Logic Error: `onAutoHoldStart` passes `'hold'` into `handleDeliveryModeChange`, but that function only maps `'drive'` to hold/manual-flush and treats every other value as passthrough/live mode. This means auto-hold never actually enables hold behavior, so messages keep auto-injecting while typing. Use the hold-equivalent mode that maps to manual flush (`'drive'`) here.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
👍 | 👎

Comment on lines +532 to +535
if (flush) {
void pear.broker.flushPending(agent.projectId, agent.name).catch(() => {})
}
void handleDeliveryModeChange(agent, 'auto')
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: flushPending is fired and ignored while mode is switched immediately afterward, so flush and mode-change race each other. That can release live injection before queued messages are deterministically flushed, causing inconsistent delivery ordering. Await flush completion before switching back to auto mode. [race condition]

Severity Level: Major ⚠️
- ⚠️ Auto-hold flush may interleave with new live messages.
- ⚠️ Message ordering differs from manual flush semantics.
- ⚠️ Harder to reason about queued vs live delivery behavior.
Steps of Reproduction ✅
1. With auto-hold active (multiple agents running and the auto-hold path in `useTerminal`
engaged as in suggestion 3), the first keystroke calls `onAutoHoldStartRef.current?.()`
and subsequent typing sets `typingActiveRef.current = true`
(src/renderer/src/hooks/use-terminal.ts:203–209).

2. When the user presses Enter, `sendInput` in `useTerminal` detects
`data.includes('\r')`, resets `typingActiveRef.current` to false, and calls
`onAutoHoldReleaseRef.current?.(true)` (use-terminal.ts:217–220), signalling that queued
messages should be flushed and live mode resumed.

3. `makeAutoHoldHandlers` implements `onAutoHoldRelease` as: if `flush` is true, it calls
`pear.broker.flushPending(agent.projectId, agent.name).catch(() => {})` and then
immediately `void handleDeliveryModeChange(agent, 'auto')`
(src/renderer/src/components/terminal/TerminalPane.tsx:531–537). Both the flush and the
mode change are fired-and-forgotten; neither Promise is awaited.

4. Because `flushPending` and `setTerminalMode` are issued concurrently with no
sequencing, the broker can observe `setTerminalMode(..., 'passthrough')` before or during
`flushPending(...)`. This differs from manual flush in `PendingMessagesMenu`, which awaits
`pear.broker.flushPending(projectId, agentName)` before updating state
(PendingMessagesPane.tsx:117–124). Under auto-hold, the race can cause held messages to
interleave with or trail new live messages instead of "flush then resume live", leading to
non-deterministic delivery ordering for queued messages.

Fix in Cursor | Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** src/renderer/src/components/terminal/TerminalPane.tsx
**Line:** 532:535
**Comment:**
	*Race Condition: `flushPending` is fired and ignored while mode is switched immediately afterward, so flush and mode-change race each other. That can release live injection before queued messages are deterministically flushed, causing inconsistent delivery ordering. Await flush completion before switching back to auto mode.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
👍 | 👎

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
src/renderer/src/hooks/use-terminal.ts (1)

544-548: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Auto-hold state machine is bypassed for global key/paste forwarding.

This sendInput path skips the new auto-hold start/release logic, so keystrokes captured via global listeners won’t consistently enter hold mode before injection.

Suggested fix
     const sendInput = (data: string): void => {
       if (terminalModeRef.current === 'view') return
+      if (autoHoldRef.current && !typingActiveRef.current && terminalModeRef.current === 'passthrough') {
+        typingActiveRef.current = true
+        onAutoHoldStartRef.current?.()
+      }
       predictiveEchoRef.current?.onUserInput(data)
       pear.broker.sendInputFast(projectId, agentName, data)
+      if (typingActiveRef.current && data.includes('\r')) {
+        typingActiveRef.current = false
+        onAutoHoldReleaseRef.current?.(true)
+      }
     }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/renderer/src/hooks/use-terminal.ts` around lines 544 - 548, sendInput
currently bypasses the auto-hold start/release logic causing global key/paste
injections to skip entering hold mode; modify sendInput to early-return for
terminalModeRef.current === 'view' as before but then invoke the same auto-hold
start helper used elsewhere (e.g., call the project's startAutoHold /
scheduleAutoHoldRelease or the existing auto-hold start function) before calling
predictiveEchoRef.current?.onUserInput and pear.broker.sendInputFast(projectId,
agentName, data), and ensure the corresponding auto-hold release is
scheduled/triggered after the injection so global listeners behave the same as
local input paths.
src/renderer/src/lib/spawn-agent.ts (1)

46-60: ⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Avoid reassigning all existing live agents to the currently selected root.

In both spawn paths, the live-agent reconciliation loop uses the current root.id/root.path for every liveAgent. With multi-root spawning, this can overwrite correct root association for already-running agents.

Also applies to: 114-128

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/renderer/src/lib/spawn-agent.ts` around lines 46 - 60, The reconciliation
loop incorrectly assigns the currently selected root (root.id/root.path) to
every live agent; update the logic in the spawn-agent reconciliation (where
liveAgents are iterated and agentStore.trackSpawnedAgent is called) to use each
liveAgent's own root metadata when available (e.g., liveAgent.rootId and
liveAgent.rootPath or equivalent properties) and only fall back to the current
rootOverride/root; apply the same fix in the second spawn path referenced (the
block around lines 114-128) so existing running agents keep their original root
association instead of being overwritten by the selected root.
🧹 Nitpick comments (1)
src/shared/types/ipc.ts (1)

811-814: ⚡ Quick win

Tighten these IPC return types to eliminate unsafe renderer casts.

At Line 811 and Line 813, Promise<unknown> pushes shape validation/casting into callers. Defining explicit result unions here (e.g., addRoot conflict/added union and a concrete worktree-root return type) would prevent unsafe as casts downstream and make contract drift compile-time visible.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/shared/types/ipc.ts` around lines 811 - 814, Replace the Promise<unknown>
signatures with explicit result types for addRoot, createWorktreeRoot and
addIntegration: define clear return unions (e.g., AddRootResult = { status:
'added' | 'conflict' | 'error'; rootId?: string; conflictReason?: string } and
WorktreeRoot = { id: string; path: string; name: string; repoPath: string } and
a typed IntegrationResult) and use them in the IPC interface instead of
Promise<unknown>; update the signatures addRoot(projectId: string, name?:
string, rootPath?: string): Promise<AddRootResult>,
createWorktreeRoot(projectId: string, repoPath: string, projectName: string,
name?: string): Promise<WorktreeRoot | { status: 'error'; message: string }>,
and addIntegration(projectId: string, name: string, type?: string):
Promise<IntegrationResult>, then update callers to rely on these discriminated
unions rather than casting with as.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/main/ipc-handlers.ts`:
- Around line 175-177: repoBasename + projectId alone can collide for different
repos with the same folder name; change worktreePath construction to incorporate
a stable, short fingerprint of the full gitRoot (e.g., a SHA1/MD5 or
base64-encoded hash of gitRoot) so the path becomes projectId + repoBasename +
"-" + shortHash, and apply the same pattern to the other occurrence(s) around
where worktreePath is set (the second instance referenced in the comment).
Update the code that computes worktreePath (and any related cleanup/lookup
logic) to use this deterministic short hash so different absolute repo locations
produce distinct worktree directories while keeping names readable.

In `@src/main/store.ts`:
- Around line 196-200: The comparison in findProjectsWithPath uses raw strings
(rootPath and r.path) which lets equivalent paths slip through; normalize both
sides before comparing (e.g., use Node's path.normalize/path.resolve) so compare
normalizedRoot = normalize(rootPath) against normalize(r.path). Update
findProjectsWithPath to normalize rootPath once and normalize each r.path inside
the .some check, and ensure the path module is imported where needed.

In `@src/renderer/src/components/settings/ProjectSettings.tsx`:
- Around line 1041-1045: The code currently calls clearRootConflict() before
awaiting createWorktreeRoot(pendingRootConflict.path) so the conflict UI is
cleared even if creation fails; change the call order and error handling so
clearRootConflict() is invoked only after createWorktreeRoot(...) resolves
successfully: inside the async callback passed to run(), await
createWorktreeRoot(pendingRootConflict.path) in a try/catch and call
clearRootConflict() after the await on success (handle/log errors in the catch
and do not clear the conflict state).

In `@src/renderer/src/components/sidebar/SpawnAgentDialog.tsx`:
- Around line 172-173: Ensure the select's value can't be a stale id by
validating selectedRootId against the current project.roots before rendering; in
SpawnAgentDialog compute a safe value (e.g. use selectedRootId only if
project.roots contains an item with that id, otherwise use root?.id or ''), use
that safe value for the select value, and when the selected id becomes invalid
call setSelectedRootId(root?.id ?? '') so state and the effective fallback stay
in sync (referencing selectedRootId, setSelectedRootId, root, and
project.roots).

In `@src/renderer/src/lib/spawn-agent.ts`:
- Around line 111-115: spawnProjectPersona currently ignores the rootOverride
when launching the persona: ensureLocalBroker and the call to
spawnPersona(project.id, personaId) use the active root instead of the provided
rootOverride. Update spawnProjectPersona to derive const root = rootOverride ??
useProjectStore.getState().getActiveRoot() (already present) and pass that root
into both the broker startup and the spawnPersona invocation (e.g.,
ensureLocalBroker(project, root) or passing root.path as cwd and
spawnPersona(project.id, personaId, { cwd: root.path })), and ensure any broker
startup logic in ensureLocalBroker or calls inside spawnProjectPersona respect
the provided root variable rather than re-reading the active root.

In `@src/renderer/src/stores/project-store.ts`:
- Around line 229-238: The success path after adding a root doesn't clear the
previously set pendingRootConflict, so the conflict banner can persist; update
the success branch in the function that handles the add result to reset
pendingRootConflict to null (or undefined) when result.kind !== 'conflict' and
before returning the parsed root. Specifically, in the block where you compute
root via parseRoot(rootData), call the store setter to clear pendingRootConflict
(the same state key set in the conflict branch) prior to await get().load() and
get().setActiveRoot(root.id) so any stale conflict state is removed after a
successful add.

---

Outside diff comments:
In `@src/renderer/src/hooks/use-terminal.ts`:
- Around line 544-548: sendInput currently bypasses the auto-hold start/release
logic causing global key/paste injections to skip entering hold mode; modify
sendInput to early-return for terminalModeRef.current === 'view' as before but
then invoke the same auto-hold start helper used elsewhere (e.g., call the
project's startAutoHold / scheduleAutoHoldRelease or the existing auto-hold
start function) before calling predictiveEchoRef.current?.onUserInput and
pear.broker.sendInputFast(projectId, agentName, data), and ensure the
corresponding auto-hold release is scheduled/triggered after the injection so
global listeners behave the same as local input paths.

In `@src/renderer/src/lib/spawn-agent.ts`:
- Around line 46-60: The reconciliation loop incorrectly assigns the currently
selected root (root.id/root.path) to every live agent; update the logic in the
spawn-agent reconciliation (where liveAgents are iterated and
agentStore.trackSpawnedAgent is called) to use each liveAgent's own root
metadata when available (e.g., liveAgent.rootId and liveAgent.rootPath or
equivalent properties) and only fall back to the current rootOverride/root;
apply the same fix in the second spawn path referenced (the block around lines
114-128) so existing running agents keep their original root association instead
of being overwritten by the selected root.

---

Nitpick comments:
In `@src/shared/types/ipc.ts`:
- Around line 811-814: Replace the Promise<unknown> signatures with explicit
result types for addRoot, createWorktreeRoot and addIntegration: define clear
return unions (e.g., AddRootResult = { status: 'added' | 'conflict' | 'error';
rootId?: string; conflictReason?: string } and WorktreeRoot = { id: string;
path: string; name: string; repoPath: string } and a typed IntegrationResult)
and use them in the IPC interface instead of Promise<unknown>; update the
signatures addRoot(projectId: string, name?: string, rootPath?: string):
Promise<AddRootResult>, createWorktreeRoot(projectId: string, repoPath: string,
projectName: string, name?: string): Promise<WorktreeRoot | { status: 'error';
message: string }>, and addIntegration(projectId: string, name: string, type?:
string): Promise<IntegrationResult>, then update callers to rely on these
discriminated unions rather than casting with as.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 15f72026-8838-4b95-8026-ea5e83aa6031

📥 Commits

Reviewing files that changed from the base of the PR and between b9c5d20 and 682fb80.

📒 Files selected for processing (12)
  • src/main/git.ts
  • src/main/ipc-handlers.ts
  • src/main/store.ts
  • src/preload/index.ts
  • src/renderer/src/components/settings/ProjectSettings.tsx
  • src/renderer/src/components/sidebar/SpawnAgentDialog.tsx
  • src/renderer/src/components/terminal/TerminalInstance.tsx
  • src/renderer/src/components/terminal/TerminalPane.tsx
  • src/renderer/src/hooks/use-terminal.ts
  • src/renderer/src/lib/spawn-agent.ts
  • src/renderer/src/stores/project-store.ts
  • src/shared/types/ipc.ts

Comment thread src/main/ipc-handlers.ts
Comment thread src/main/store.ts
Comment thread src/renderer/src/components/settings/ProjectSettings.tsx
Comment thread src/renderer/src/components/sidebar/SpawnAgentDialog.tsx Outdated
Comment thread src/renderer/src/lib/spawn-agent.ts
Comment thread src/renderer/src/stores/project-store.ts
@codeant-ai
Copy link
Copy Markdown

codeant-ai Bot commented Jun 6, 2026

CodeAnt AI finished reviewing your PR.

@agent-relay-code
Copy link
Copy Markdown
Contributor

Fixed the PR issues I validated:

  • Auto-hold input now awaits the switch to drive mode before sending held input, uses an awaited IPC input path, and flushes before returning to auto mode.
  • Global terminal key/paste forwarding now uses the same auto-hold path instead of bypassing it.
  • Worktree creation is now safer for duplicate attempts: existing target worktrees are reused, and generated branch names include a stable project-id suffix to avoid same-name project collisions.
  • Verified the generated git worktree add -b ... command against a throwaway fixture.

Validation:

  • npm run build passed.
  • npm test passed, 77 tests.
  • npx tsc -b tsconfig.json --pretty false still reports broad unrelated existing type errors outside the PR path; the build succeeds.

@agent-relay-code
Copy link
Copy Markdown
Contributor

pr-reviewer applied fixes — committed and pushed 8af5696 to this PR. The notes below describe what changed.

Fixed the PR issues I validated:

  • Auto-hold input now awaits the switch to drive mode before sending held input, uses an awaited IPC input path, and flushes before returning to auto mode.
  • Global terminal key/paste forwarding now uses the same auto-hold path instead of bypassing it.
  • Worktree creation is now safer for duplicate attempts: existing target worktrees are reused, and generated branch names include a stable project-id suffix to avoid same-name project collisions.
  • Verified the generated git worktree add -b ... command against a throwaway fixture.

Validation:

  • npm run build passed.
  • npm test passed, 77 tests.
  • npx tsc -b tsconfig.json --pretty false still reports broad unrelated existing type errors outside the PR path; the build succeeds.

@agent-relay-code
Copy link
Copy Markdown
Contributor

Reviewed and fixed the validated PR issues.

Changed:

  • Sequenced terminal input sends so auto-hold keystrokes cannot overtake the first mode-switching input.
  • Added deterministic hashed worktree paths to avoid same-project repo basename collisions.
  • Normalized stored root path comparisons for conflict detection.
  • Kept root conflict UI visible if worktree creation fails, and cleared stale conflict state after successful root creation.
  • Made spawn root selection resilient to stale root IDs.
  • Ensured persona spawning applies the selected root before broker startup.
  • Preserved existing tracked root metadata when reconciling already-running agents.
  • Tightened IPC return types for root and integration APIs.

Validated locally:

  • npx tsc --noEmit -p tsconfig.json
  • npm test passed, 77 tests
  • Throwaway git worktree add -b ... fixture passed
  • npm run build passed

@agent-relay-code
Copy link
Copy Markdown
Contributor

pr-reviewer applied fixes — committed and pushed 189c5b6 to this PR. The notes below describe what changed.

Reviewed and fixed the validated PR issues.

Changed:

  • Sequenced terminal input sends so auto-hold keystrokes cannot overtake the first mode-switching input.
  • Added deterministic hashed worktree paths to avoid same-project repo basename collisions.
  • Normalized stored root path comparisons for conflict detection.
  • Kept root conflict UI visible if worktree creation fails, and cleared stale conflict state after successful root creation.
  • Made spawn root selection resilient to stale root IDs.
  • Ensured persona spawning applies the selected root before broker startup.
  • Preserved existing tracked root metadata when reconciling already-running agents.
  • Tightened IPC return types for root and integration APIs.

Validated locally:

  • npx tsc --noEmit -p tsconfig.json
  • npm test passed, 77 tests
  • Throwaway git worktree add -b ... fixture passed
  • npm run build passed

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/main/ipc-handlers.ts (1)

171-192: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Validate repoPath before running git worktree operations.

This handler is accepting a renderer-supplied path and then creating branches/worktrees without the assertPathWithinProjects(...) guard used across the rest of the git/fs IPC surface. That means a compromised renderer can mutate any local repo the user can access, not just registered project roots.

🔒 Minimal guard
 ipcMain.handle('project:create-worktree-root', async (_, projectId: string, repoPath: string, projectName: string, name?: string) => {
-  const gitRoot = await git.getGitRoot(repoPath)
+  const resolvedRepoPath = resolve(repoPath)
+  assertPathWithinProjects(resolvedRepoPath)
+
+  const gitRoot = await git.getGitRoot(resolvedRepoPath)
   if (!gitRoot) throw new Error(`Not a git repository: ${repoPath}`)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/main/ipc-handlers.ts` around lines 171 - 192, The handler for
'project:create-worktree-root' uses renderer-supplied repoPath to call
git.getGitRoot/git.createWorktree without the usual guard; add a check using the
existing assertPathWithinProjects (or equivalent guard used elsewhere) before
any git operations (ideally right after computing gitRoot or immediately after
receiving repoPath) and throw/return an error if the path is not allowed, so
that git.getGitRoot, git.worktreeExists and git.createWorktree only run for
validated project paths, then continue to call addProjectRoot as before.
🧹 Nitpick comments (1)
src/preload/index.ts (1)

200-201: ⚡ Quick win

Use concrete shared result types for the root IPC wrappers.

These methods now return structured payloads, but Promise<unknown> erases that contract at the preload boundary. Please promote the add-root/worktree-root results into named shared IPC types so the renderer/store can branch on them exhaustively instead of casting.

Also applies to: 204-205

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/preload/index.ts` around lines 200 - 201, The preload IPC wrappers
currently return Promise<unknown> (e.g., addRoot and the similar worktree root
method), which erases the structured result; change the generic on invoke to a
concrete shared IPC result type (for example replace invoke<unknown> with
invoke<AddRootResult> and invoke<AddWorktreeRootResult>), import those named
types from the shared IPC/types module where shared payloads live, and update
the method signatures (addRoot, addWorktreeRoot) to return
Promise<AddRootResult>/Promise<AddWorktreeRootResult> so the renderer/store can
exhaustively pattern-match the payloads without casting.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/renderer/src/components/terminal/TerminalPane.tsx`:
- Around line 534-537: The empty catch on pear.broker.flushPending inside
onAutoHoldRelease masks failures; change the call to await
pear.broker.flushPending(agent.projectId, agent.name) without swallowing errors,
and if it rejects, do NOT proceed to reset the agent back to auto—propagate or
rethrow the error so the caller can surface it and keep the agent in held/drive
mode; update onAutoHoldRelease to either throw the caught error or return a
failure result so the caller (use-terminal.ts) can handle showing the error and
avoid flipping the mode.
- Around line 527-539: makeAutoHoldHandlers captures a stale agent object which
causes handleDeliveryModeChange to mis-dedupe via getTerminalMode(agent) and
also swallows pear.broker.flushPending errors; fix by changing the handlers to
use stable identifiers (agent.projectId and agent.name) instead of the captured
agent, resolve the current terminal mode from the store at callback time (call
getTerminalMode with projectId/name or otherwise fetch latest agent state inside
onAutoHoldStart/onAutoHoldRelease) to decide whether to call
handleDeliveryModeChange, and change onAutoHoldRelease so that
pear.broker.flushPending(projectId, name) errors are not silently ignored
(propagate or handle the error and only proceed to call
handleDeliveryModeChange('auto') on successful flush, or log/return the error
instead of swallowing it).

---

Outside diff comments:
In `@src/main/ipc-handlers.ts`:
- Around line 171-192: The handler for 'project:create-worktree-root' uses
renderer-supplied repoPath to call git.getGitRoot/git.createWorktree without the
usual guard; add a check using the existing assertPathWithinProjects (or
equivalent guard used elsewhere) before any git operations (ideally right after
computing gitRoot or immediately after receiving repoPath) and throw/return an
error if the path is not allowed, so that git.getGitRoot, git.worktreeExists and
git.createWorktree only run for validated project paths, then continue to call
addProjectRoot as before.

---

Nitpick comments:
In `@src/preload/index.ts`:
- Around line 200-201: The preload IPC wrappers currently return
Promise<unknown> (e.g., addRoot and the similar worktree root method), which
erases the structured result; change the generic on invoke to a concrete shared
IPC result type (for example replace invoke<unknown> with invoke<AddRootResult>
and invoke<AddWorktreeRootResult>), import those named types from the shared
IPC/types module where shared payloads live, and update the method signatures
(addRoot, addWorktreeRoot) to return
Promise<AddRootResult>/Promise<AddWorktreeRootResult> so the renderer/store can
exhaustively pattern-match the payloads without casting.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 7a8a3847-0e22-460d-9e4f-f2540c1ac441

📥 Commits

Reviewing files that changed from the base of the PR and between 682fb80 and 8af5696.

📒 Files selected for processing (7)
  • src/main/git.ts
  • src/main/ipc-handlers.ts
  • src/preload/index.ts
  • src/renderer/src/components/terminal/TerminalInstance.tsx
  • src/renderer/src/components/terminal/TerminalPane.tsx
  • src/renderer/src/hooks/use-terminal.ts
  • src/shared/types/ipc.ts
🚧 Files skipped from review as they are similar to previous changes (4)
  • src/main/git.ts
  • src/shared/types/ipc.ts
  • src/renderer/src/hooks/use-terminal.ts
  • src/renderer/src/components/terminal/TerminalInstance.tsx

Comment on lines +527 to +539
const makeAutoHoldHandlers = (agent: Agent): {
onAutoHoldStart: () => Promise<void>
onAutoHoldRelease: (flush: boolean) => Promise<void>
} => ({
onAutoHoldStart: async () => {
await handleDeliveryModeChange(agent, 'drive')
},
onAutoHoldRelease: async (flush: boolean) => {
if (flush) {
await pear.broker.flushPending(agent.projectId, agent.name).catch(() => {})
}
await handleDeliveryModeChange(agent, 'auto')
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Fix auto-hold mode switching to avoid stale-agent dedupe (TerminalPane.tsx)
makeAutoHoldHandlers(agent) captures a render-time agent, but handleDeliveryModeChange dedupes by reading getTerminalMode(agent). Because setAgentTerminalMode replaces agent objects immutably, the captured agent can have stale terminalMode, so onAutoHoldRelease can incorrectly no-op and leave the agent in the previous mode (e.g., still 'drive') depending on timing.
Also, onAutoHoldRelease swallows pear.broker.flushPending(...) failures while still switching back to 'auto', so a failed flush doesn’t prevent/reflect the delivery-mode transition.
Update the callbacks to use stable identifiers and resolve the latest terminal mode from the store at callback time (or pass explicit current/target modes), and don’t unconditionally ignore flushPending failures.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/renderer/src/components/terminal/TerminalPane.tsx` around lines 527 -
539, makeAutoHoldHandlers captures a stale agent object which causes
handleDeliveryModeChange to mis-dedupe via getTerminalMode(agent) and also
swallows pear.broker.flushPending errors; fix by changing the handlers to use
stable identifiers (agent.projectId and agent.name) instead of the captured
agent, resolve the current terminal mode from the store at callback time (call
getTerminalMode with projectId/name or otherwise fetch latest agent state inside
onAutoHoldStart/onAutoHoldRelease) to decide whether to call
handleDeliveryModeChange, and change onAutoHoldRelease so that
pear.broker.flushPending(projectId, name) errors are not silently ignored
(propagate or handle the error and only proceed to call
handleDeliveryModeChange('auto') on successful flush, or log/return the error
instead of swallowing it).

Comment thread src/renderer/src/components/terminal/TerminalPane.tsx
@agent-relay-code
Copy link
Copy Markdown
Contributor

Fixed issues found in PR #119:

  • Auto-hold release now checks the current agent state before deciding whether to switch back to live mode, avoiding a stale-agent no-op that could leave terminals stuck in drive.
  • Auto-hold startup now gates held input behind the mode-change promise, so rapid keystrokes do not race ahead before the broker is in hold mode.
  • Added IPC regression coverage for duplicate root conflicts, worktree creation/reuse, and broker:send-input.

Validated locally:

  • npx tsc --noEmit -p tsconfig.json
  • npx vitest run src/main/ipc-handlers.test.ts
  • npm test
  • npm run build passed after installing missing dependencies; generated/install artifacts were cleaned up afterward.

@agent-relay-code
Copy link
Copy Markdown
Contributor

⚠️ pr-reviewer push failed (exit 1) — fixes were not applied to the PR. The notes below are advisory and were not pushed.

Fixed issues found in PR #119:

  • Auto-hold release now checks the current agent state before deciding whether to switch back to live mode, avoiding a stale-agent no-op that could leave terminals stuck in drive.
  • Auto-hold startup now gates held input behind the mode-change promise, so rapid keystrokes do not race ahead before the broker is in hold mode.
  • Added IPC regression coverage for duplicate root conflicts, worktree creation/reuse, and broker:send-input.

Validated locally:

  • npx tsc --noEmit -p tsconfig.json
  • npx vitest run src/main/ipc-handlers.test.ts
  • npm test
  • npm run build passed after installing missing dependencies; generated/install artifacts were cleaned up afterward.

Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 12 files (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread src/renderer/src/components/sidebar/SpawnAgentDialog.tsx
Comment thread src/renderer/src/components/terminal/TerminalPane.tsx Outdated
@agent-relay-code
Copy link
Copy Markdown
Contributor

Reviewed PR #119’s diff and touched call paths. I fixed one verified issue in use-terminal.ts: held terminal input failures now rethrow after logging, so auto-hold does not flush/release as if a failed queued write succeeded. Applied the same fix to the global-key input path.

Local validation run:
npx tsc --noEmit
npm test -- --test-reporter=spec
npm run build
npm run verify:mcp-resources-drift

I also exercised the new git worktree add -b ... command shape against a throwaway git repo successfully.

@agent-relay-code
Copy link
Copy Markdown
Contributor

pr-reviewer applied fixes — committed and pushed 5f81145 to this PR. The notes below describe what changed.

Reviewed PR #119’s diff and touched call paths. I fixed one verified issue in use-terminal.ts: held terminal input failures now rethrow after logging, so auto-hold does not flush/release as if a failed queued write succeeded. Applied the same fix to the global-key input path.

Local validation run:
npx tsc --noEmit
npm test -- --test-reporter=spec
npm run build
npm run verify:mcp-resources-drift

I also exercised the new git worktree add -b ... command shape against a throwaway git repo successfully.

Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 12 files

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread src/renderer/src/hooks/use-terminal.ts
Comment thread src/renderer/src/components/settings/ProjectSettings.tsx Outdated
@agent-relay-code
Copy link
Copy Markdown
Contributor

Fixed the validated PR issues, including cubic-identified findings:

  • Scoped root-conflict state to the project that produced it, so the banner cannot be acted on from the wrong project.
  • Made persona discovery use the selected spawn root before listing personas.
  • Refactored terminal input sending so local xterm input and global keyboard input share the same path, including recordKeystrokeSent.
  • Stopped swallowing auto-hold flushPending failures before switching back to live mode.

Validation run locally:

  • npm test
  • npx vitest run src/main/ipc-handlers.test.ts
  • npm run verify:mcp-resources-drift
  • npm run build

@agent-relay-code
Copy link
Copy Markdown
Contributor

pr-reviewer applied fixes — committed and pushed 8dc02fa to this PR. The notes below describe what changed.

Fixed the validated PR issues, including cubic-identified findings:

  • Scoped root-conflict state to the project that produced it, so the banner cannot be acted on from the wrong project.
  • Made persona discovery use the selected spawn root before listing personas.
  • Refactored terminal input sending so local xterm input and global keyboard input share the same path, including recordKeystrokeSent.
  • Stopped swallowing auto-hold flushPending failures before switching back to live mode.

Validation run locally:

  • npm test
  • npx vitest run src/main/ipc-handlers.test.ts
  • npm run verify:mcp-resources-drift
  • npm run build

Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 7 files (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread src/renderer/src/components/terminal/TerminalPane.tsx Outdated
Comment thread src/renderer/src/lib/spawn-agent.ts Outdated
@kjgbot kjgbot merged commit 08d06f8 into main Jun 6, 2026
4 checks passed
@kjgbot kjgbot deleted the claude/maestro-app-comparison-9EQ7H branch June 6, 2026 09:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L This PR changes 100-499 lines, ignoring generated files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants