feat(web): add Run-in-Worktree toggle to mobile AutoRun launch - #946
Merged
Conversation
Adds parity with desktop's WorktreeRunSection so mobile users can dispatch an Auto Run to a separate git worktree, optionally creating a PR on completion. - New WS handlers `get_git_branches` and `list_worktrees` (SSH-aware via the session's `sessionSshRemoteConfig`). - Surfaces `isGitRepo` + `worktreeBasePath` on `SessionData` so mobile can gate the section and compute the worktree path preview. - Extends `useAutoRun.launchAutoRun` to forward the optional `worktree` payload through `configure_auto_run`, and adds `loadGitBranches` / `listWorktrees` helpers. - New `AutoRunWorktreeSection` mobile component (toggle, base-branch picker, sanitized branch input + path preview, "create PR on completion" checkbox), wired into `AutoRunSetupSheet` above the Prompt section. - Tests: handler validation/payload shape, hook payload threading, and section rendering / config emission. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds Run-in-Worktree support end-to-end: new types, WebSocket message handlers and callback wiring, main-process git executors, frontend hook methods to load branches/worktrees and include worktree in Auto Run launches, a mobile UI for worktree configuration, a shared worktree spawn utility, and tests/mocks. ChangesRun-in-Worktree Feature
Sequence DiagramsequenceDiagram
actor User
participant Mobile as Mobile App
participant Sheet as AutoRunSetupSheet
participant Section as AutoRunWorktreeSection
participant Hook as useAutoRun Hook
participant WS as WebSocket Client
participant Server as WebServer
participant Git as Git Executor
User->>Section: Enable worktree dispatch
Section->>Hook: loadGitBranches(sessionId)
Hook->>WS: sendRequest get_git_branches
WS->>Server: route to messageHandlers
Server->>Git: exec "git branch -a" / "git rev-parse"
Git->>Server: branches + currentBranch
Server->>WS: git_branches response
WS->>Hook: deliver response
Hook->>Section: return {branches,currentBranch}
Section->>Hook: listWorktrees(sessionId)
Hook->>WS: sendRequest list_worktrees
WS->>Server: route to messageHandlers
Server->>Git: exec "git worktree list --porcelain"
Git->>Server: worktree entries
Server->>WS: worktrees_list response
WS->>Hook: deliver response
Hook->>Section: return worktrees
User->>Section: Configure branch, toggle PR
Section->>Sheet: onChange(LaunchWorktreeConfig)
User->>Sheet: Tap Launch
Sheet->>Hook: launchAutoRun(config with worktree)
Hook->>WS: send configure_auto_run (includes worktree)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Greptile SummaryAdds the "Dispatch to a separate worktree" toggle to the mobile Auto Run launch sheet, closing the AR-PARITY-02 feature gap with the desktop. Two new SSH-aware WebSocket messages ( The previously flagged P1 (empty Confidence Score: 5/5Safe to merge — no functional defects found in the new worktree dispatch path or the WS handlers. The change is well-scoped and follows established patterns. The previously flagged guard against empty prTargetBranch on early PR-toggle is now present and tested. The spawnWorktreeAgentAndDispatch extraction is a straight lift; startBatchRun already has the worktreeTarget-present guard that skips duplicate setupWorktree, confirmed at line 630 of useBatchProcessor.ts. SSH context resolution is consolidated into a single helper rather than copy-pasted. Only minor style observations remain. No files require special attention. Important Files Changed
Sequence DiagramsequenceDiagram
participant MobileUI as Mobile UI (AutoRunWorktreeSection)
participant WS as WebSocket (useAutoRun)
participant WSHandler as WS Handler (messageHandlers.ts)
participant Factory as web-server-factory.ts
participant RemoteListener as useAppRemoteEventListeners
participant Spawn as worktreeSpawn.ts
participant BatchProc as useBatchProcessor
MobileUI->>WS: get_git_branches / list_worktrees (sessionId)
WS->>WSHandler: handleGetGitBranches / handleListWorktrees
WSHandler->>Factory: getGitBranchesForSession(sessionId)
Factory->>Factory: resolveSessionGitContext (SSH-aware)
Factory->>Factory: execGit branch -a cwd sshRemote
Factory-->>WSHandler: GitBranchesResult
WSHandler-->>MobileUI: git_branches response
MobileUI->>WS: configure_auto_run (worktree payload)
WS->>RemoteListener: configure_auto_run event
RemoteListener->>RemoteListener: resolve parentSession if worktree child
RemoteListener->>Spawn: spawnWorktreeAgentAndDispatch(parent, spawnConfig)
Spawn->>Spawn: git worktreeSetup create-new mode
Spawn->>Spawn: buildWorktreeSession add to sessionStore
Spawn-->>RemoteListener: newSessionId
RemoteListener->>RemoteListener: set batchConfig.worktreeTarget skip flag
RemoteListener->>BatchProc: startBatchRun(newSessionId, batchConfig)
BatchProc->>BatchProc: worktreeTarget present skip setupWorktree
BatchProc->>BatchProc: run Auto Run on worktree session
Reviews (2): Last reviewed commit: "fix(web): spawn worktree child against l..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.github.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/web-server/web-server-factory.ts`:
- Around line 1433-1453: The catch block in getGitBranchesForSession (the try
that awaits execGit for ['branch', '-a'...] and ['rev-parse'...]) currently
swallows unexpected failures and returns { branches: [] }; instead, change the
behavior so that genuine execution or SSH errors are not collapsed into an
empty-branches success result: rethrow the caught error (or return an explicit
error-shaped result the client can detect) so callers can distinguish infra/SSH
failures from a real empty repo; similarly update the handling around non-zero
execGit exits in the same function (and the analogous worktree function that
uses execGit/parseGitBranches/parseGitWorktrees) to only map successful
exitCode===0 to parsed results and to propagate other failures upstream (e.g.,
via throw or an error result object) rather than returning { branches: [] } / {
worktrees: [] }.
In `@src/web/hooks/useAutoRun.ts`:
- Around line 163-193: The current implementations of loadGitBranches and
listWorktrees swallow all errors from sendRequest and return fake empty results;
instead let transport/backend failures bubble up so callers can show an error
state. Remove or change the empty-catching behavior in loadGitBranches and
listWorktrees (the try/catch blocks around sendRequest) so that unexpected
errors are rethrown (or rethrown with added context referencing the operation,
e.g., "get_git_branches" / "list_worktrees") rather than returning `{ branches:
[] }` or `[]`; keep the existing return shapes on success and ensure sendRequest
errors propagate to callers (or wrap and throw a new Error) so Sentry/upper
layers can observe failures.
In `@src/web/mobile/AutoRunSetupSheet.tsx`:
- Line 60: AutoRunSetupSheet treats a null worktreeConfig as "omit worktree"
which lets an invalid/cleared AutoRunWorktreeSection silently fall back to
launching against the main checkout; change the worktree state to an explicit
discriminated union (e.g. { status: "disabled" } | { status: "enabled", config:
LaunchWorktreeConfig } | { status: "invalid", errors?: string[] }) instead of
nullable, update AutoRunWorktreeSection's onChange to emit those three cases
(disabled, enabled-with-config, enabled-but-invalid), and update the launch
payload construction in AutoRunSetupSheet (the code paths around the current
worktreeConfig handling) to block launch when status === "invalid" and only omit
the worktree payload for status === "disabled" while using the config when
status === "enabled".
In `@src/web/mobile/AutoRunWorktreeSection.tsx`:
- Around line 88-93: The catch blocks in loadBranches and loadWorktrees silently
swallow errors; update both .catch handlers to forward the caught exception to
Sentry by importing and calling captureException from src/utils/sentry.ts (e.g.,
catch((err) => { captureException(err, { context: { function: 'loadBranches' }
}); ... }) for loadBranches and similarly for loadWorktrees), while preserving
existing UI state changes like setBranchLoadError(true) and setBranches([]).
- Around line 288-293: The select currently shows "Loading..." whenever
branches.length === 0, which also occurs after a failed fetch because branches
is reset to [], so change the condition to reflect actual loading vs error
states: render the "Loading..." option only when branchLoading is true (or when
!branchLoadError && branches.length === 0), and render an error placeholder
option (e.g., "Failed to load branches") when branchLoadError is true; update
the JSX around the branches.map block (referencing branches, branchLoading,
branchLoadError in AutoRunWorktreeSection) so the select distinguishes
in-progress vs failed fetches.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 582d86ae-e66d-4d4a-8d50-cefbeb74a659
📒 Files selected for processing (14)
src/__tests__/main/web-server/handlers/messageHandlers.test.tssrc/__tests__/web/hooks/useAutoRun.test.tssrc/__tests__/web/mobile/App.test.tsxsrc/__tests__/web/mobile/AutoRunWorktreeSection.test.tsxsrc/main/web-server/WebServer.tssrc/main/web-server/handlers/messageHandlers.tssrc/main/web-server/managers/CallbackRegistry.tssrc/main/web-server/types.tssrc/main/web-server/web-server-factory.tssrc/web/hooks/useAutoRun.tssrc/web/hooks/useWebSocket.tssrc/web/mobile/App.tsxsrc/web/mobile/AutoRunSetupSheet.tsxsrc/web/mobile/AutoRunWorktreeSection.tsx
CI failure fix:
- Add `setGetGitBranchesForSessionCallback` / `setListWorktreesForSessionCallback`
to the WebServer mock in `web-server-factory.test.ts` so the factory's
callback registration doesn't blow up the suite.
Review feedback (greptile + coderabbit):
- `messageHandlers.ts`: defensively coerce non-Error rejections in the two new
handlers (`error instanceof Error ? error.message : String(error)`).
- `web-server-factory.ts`: extract `resolveSessionGitContext()` helper to
deduplicate SSH resolution and drop the no-op `|| undefined`. Stop
swallowing exec/SSH failures into empty-result success — let them
propagate so Sentry sees real infra/SSH regressions; only `exitCode !== 0`
(legitimately "not a git repo") still maps to empty results.
- `useAutoRun.ts`: `loadGitBranches` / `listWorktrees` no longer swallow
transport errors; let rejections propagate so the section can render an
actual error state.
- `AutoRunWorktreeSection.tsx`: log caught exceptions via `webLogger.error`,
replace `branchLoadError` boolean with a `branchLoadStatus`
('idle' | 'loading' | 'error') tri-state so the picker shows
"Failed to load" instead of a stuck "Loading…" after a fetch failure.
- `AutoRunWorktreeSection.tsx` + `AutoRunSetupSheet.tsx`: `onChange` now
emits a discriminated `AutoRunWorktreeState` union
('disabled' | 'enabled-valid' | 'enabled-invalid'). The sheet blocks
launch and shows an inline warning when status is 'enabled-invalid' —
prevents an enabled-but-invalid worktree config from silently falling
through to a normal Auto Run on the main checkout.
Tests:
- Updated `AutoRunWorktreeSection.test.tsx` to assert the new state union
and added a "Failed to load" placeholder regression test.
- Updated `useAutoRun.test.ts` to assert the new error-propagation contract
for `loadGitBranches` / `listWorktrees`.
Scoped vitest: 196 pass / 0 fail. Lint, prettier, type-check clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/__tests__/main/web-server/web-server-factory.test.ts (1)
348-410: ⚡ Quick winAdd registration assertions for the new git/worktree callbacks.
The mock includes the two new setters, but callback-registration tests don’t currently assert they were wired during factory setup.
Suggested test addition
it('should register file and auto-run callbacks', () => { expect(server.setOpenFileTabCallback).toHaveBeenCalled(); expect(server.setRefreshFileTreeCallback).toHaveBeenCalled(); expect(server.setRefreshAutoRunDocsCallback).toHaveBeenCalled(); expect(server.setConfigureAutoRunCallback).toHaveBeenCalled(); + expect(server.setGetGitBranchesForSessionCallback).toHaveBeenCalled(); + expect(server.setListWorktreesForSessionCallback).toHaveBeenCalled(); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/__tests__/main/web-server/web-server-factory.test.ts` around lines 348 - 410, Add assertions that the two new git/worktree callback setters were called during factory setup: add expect(server.setGetGitStatusCallback).toHaveBeenCalled(); and expect(server.setSwitchWorktreeCallback).toHaveBeenCalled(); (or use the exact mock setter names if they differ) into the callback registrations tests—either in the existing "file and auto-run callbacks" test or a new small it block so the factory wiring for setGetGitStatusCallback and setSwitchWorktreeCallback is asserted.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/web-server/web-server-factory.ts`:
- Around line 1418-1427: resolveSessionGitContext currently falls back to local
git when session.sessionSshRemoteConfig.enabled is true but
getSshRemoteById(remoteId) returns undefined; change it to fail fast instead:
inside resolveSessionGitContext (and referencing StoredSession,
session.sessionSshRemoteConfig.remoteId and getSshRemoteById), detect the case
where enabled is true but sshRemoteId is missing or getSshRemoteById returns
undefined and throw a descriptive error (or return an explicit failure) rather
than returning a local context; ensure callers handle the thrown error/explicit
failure so remote-backed sessions don't silently use local git data.
---
Nitpick comments:
In `@src/__tests__/main/web-server/web-server-factory.test.ts`:
- Around line 348-410: Add assertions that the two new git/worktree callback
setters were called during factory setup: add
expect(server.setGetGitStatusCallback).toHaveBeenCalled(); and
expect(server.setSwitchWorktreeCallback).toHaveBeenCalled(); (or use the exact
mock setter names if they differ) into the callback registrations tests—either
in the existing "file and auto-run callbacks" test or a new small it block so
the factory wiring for setGetGitStatusCallback and setSwitchWorktreeCallback is
asserted.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b8897577-67ec-47f1-a0f0-8a47fa346d63
📒 Files selected for processing (8)
src/__tests__/main/web-server/web-server-factory.test.tssrc/__tests__/web/hooks/useAutoRun.test.tssrc/__tests__/web/mobile/AutoRunWorktreeSection.test.tsxsrc/main/web-server/handlers/messageHandlers.tssrc/main/web-server/web-server-factory.tssrc/web/hooks/useAutoRun.tssrc/web/mobile/AutoRunSetupSheet.tsxsrc/web/mobile/AutoRunWorktreeSection.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/web/hooks/useAutoRun.ts
Comment on lines
+1418
to
+1427
| const resolveSessionGitContext = ( | ||
| session: StoredSession | ||
| ): { sshRemote: ReturnType<typeof getSshRemoteById>; remoteCwd: string | undefined } => { | ||
| const sshRemoteId = session.sessionSshRemoteConfig?.enabled | ||
| ? session.sessionSshRemoteConfig.remoteId | ||
| : undefined; | ||
| const sshRemote = sshRemoteId ? getSshRemoteById(sshRemoteId) : undefined; | ||
| const remoteCwd = sshRemote ? session.cwd : undefined; | ||
| return { sshRemote, remoteCwd }; | ||
| }; |
There was a problem hiding this comment.
Fail fast when SSH is enabled but the remote cannot be resolved.
If session.sessionSshRemoteConfig.enabled is true but remoteId is missing/stale, this currently falls back to local git execution. That can return incorrect branch/worktree data for remote-backed sessions.
Suggested fix
const resolveSessionGitContext = (
session: StoredSession
): { sshRemote: ReturnType<typeof getSshRemoteById>; remoteCwd: string | undefined } => {
- const sshRemoteId = session.sessionSshRemoteConfig?.enabled
- ? session.sessionSshRemoteConfig.remoteId
- : undefined;
- const sshRemote = sshRemoteId ? getSshRemoteById(sshRemoteId) : undefined;
- const remoteCwd = sshRemote ? session.cwd : undefined;
- return { sshRemote, remoteCwd };
+ if (!session.sessionSshRemoteConfig?.enabled) {
+ return { sshRemote: undefined, remoteCwd: undefined };
+ }
+ const sshRemoteId = session.sessionSshRemoteConfig.remoteId;
+ if (!sshRemoteId) {
+ throw new Error(`SSH remote is enabled but remoteId is missing for session ${session.id}`);
+ }
+ const sshRemote = getSshRemoteById(sshRemoteId);
+ if (!sshRemote) {
+ throw new Error(`SSH remote not found: ${sshRemoteId}`);
+ }
+ return { sshRemote, remoteCwd: session.cwd };
};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/web-server/web-server-factory.ts` around lines 1418 - 1427,
resolveSessionGitContext currently falls back to local git when
session.sessionSshRemoteConfig.enabled is true but getSshRemoteById(remoteId)
returns undefined; change it to fail fast instead: inside
resolveSessionGitContext (and referencing StoredSession,
session.sessionSshRemoteConfig.remoteId and getSshRemoteById), detect the case
where enabled is true but sshRemoteId is missing or getSshRemoteById returns
undefined and throw a descriptive error (or return an explicit failure) rather
than returning a local context; ensure callers handle the thrown error/explicit
failure so remote-backed sessions don't silently use local git data.
Resolves conflicts where Run-in-Worktree extensions and the upstream Auto Run parity work both touched the same surfaces: - src/main/web-server/types.ts (SessionData / SessionBroadcastData) - src/main/web-server/web-server-factory.ts (sessions emit + new git callbacks) - src/web/hooks/useAutoRun.ts (LaunchConfig, exports, state shape) - src/web/hooks/useWebSocket.ts (SessionData) - src/web/mobile/App.tsx (useAutoRun destructuring + sheet props wiring) - src/web/mobile/AutoRunSetupSheet.tsx (imports, props, reset effect, layout) All conflicts resolved by keeping both sides' fields/handlers — Run-in- Worktree props live alongside upstream's playbook/error-recovery wiring. Also addresses one new review comment on commit 9ce0e33: - web-server-factory.ts:resolveSessionGitContext now fails loudly when SSH is enabled but the configured remote can't be resolved instead of silently falling back to local git (would have leaked the run to the wrong machine for SSH-backed sessions). Updated the App.test.tsx useAutoRun mock to expose the playbook / error-recovery / worktree methods that App.tsx now destructures. Scoped vitest: 188 pass / 0 fail. Lint, prettier, type-check clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/web-server/handlers/messageHandlers.ts`:
- Around line 3101-3104: The catch blocks in messageHandlers.ts that currently
convert backend failures into websocket-only errors use this.sendError(...) and
should instead report the error to Sentry via reportHandlerError(...) so
failures are captured; modify the two catch handlers (the one calling
this.sendError(client, `Failed to get git branches: ${msg}`) and the similar
block at the other location) to call reportHandlerError(error, context) (or
reportHandlerError(new Error(msg), { handler: 'git/worktree', client }))
before/alongside sending a websocket error, using the existing
reportHandlerError helper and preserving the original client/sendError behavior
if needed so diagnostics are recorded.
In `@src/main/web-server/web-server-factory.ts`:
- Around line 1622-1626: The current logic treats execGit failures (where
execGit returns exitCode as a string like "ENOENT"/"EPERM") as harmless empty
results by mapping branchesResult/currentBranchResult to []/undefined; instead
detect when exitCode is a non-numeric string and surface the error (throw or
return the original error) so it does not get downgraded to an empty repo state.
Update the code paths that use execGit (references: execGit, branchesResult,
currentBranchResult, parseGitBranches and the analogous worktreesResult
handling) to check typeof exitCode === "string" and rethrow a descriptive Error
(or propagate the original error object) before falling back to parsing stdout
for numeric exitCode === 0 cases. Ensure identical handling is applied to the
worktrees block mentioned (lines 1649-1653).
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 814f95e9-2e04-4d71-a7b0-f962f63cfb44
📒 Files selected for processing (12)
src/__tests__/main/web-server/handlers/messageHandlers.test.tssrc/__tests__/main/web-server/web-server-factory.test.tssrc/__tests__/web/mobile/App.test.tsxsrc/main/web-server/WebServer.tssrc/main/web-server/handlers/messageHandlers.tssrc/main/web-server/managers/CallbackRegistry.tssrc/main/web-server/types.tssrc/main/web-server/web-server-factory.tssrc/web/hooks/useAutoRun.tssrc/web/hooks/useWebSocket.tssrc/web/mobile/App.tsxsrc/web/mobile/AutoRunSetupSheet.tsx
✅ Files skipped from review due to trivial changes (5)
- src/tests/main/web-server/web-server-factory.test.ts
- src/main/web-server/types.ts
- src/tests/main/web-server/handlers/messageHandlers.test.ts
- src/tests/web/mobile/App.test.tsx
- src/web/hooks/useWebSocket.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- src/web/mobile/AutoRunSetupSheet.tsx
- src/web/hooks/useAutoRun.ts
- src/main/web-server/WebServer.ts
- src/web/mobile/App.tsx
…t spawn failures Addresses two CodeRabbit review comments on f304407: - messageHandlers.ts (handleGetGitBranches / handleListWorktrees): the new `.catch` blocks were calling `this.sendError` directly, which surfaces a websocket error to the client but never reaches Sentry. Routed both through `reportHandlerError(...)` so production diagnostics see SSH/git regressions; the user-facing message and shape are unchanged. - web-server-factory.ts (getGitBranchesForSession / listWorktreesForSession): `execGit` returns `exitCode: number | string`, where a string code (ENOENT, EPERM, …) means git never ran. The previous fallback collapsed those failures into `{ branches: [] }` / `{ worktrees: [] }`, making broken local/SSH execution look like a healthy empty repo. Now we explicitly throw on a non-numeric exitCode so the real cause propagates and only numeric non-zero codes (legitimate "not a git repo") still map to empty results. Scoped vitest: 188 pass / 0 fail. Lint, prettier, type-check clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
Author
Blocker: AR-PARITY-02 gap not actually closed — mobile/web launch skips child-session creation that desktop doesFound while soaking #946 into The PR description says this closes the AR-PARITY-02 gap with desktop's EvidenceDesktop path — } else if (
config.worktreeTarget?.mode === 'create-new' ||
config.worktreeTarget?.mode === 'existing-closed'
) {
let parentForSpawn = activeSession;
if (activeSession.parentSessionId) {
const parent = selectSessionById(activeSession.parentSessionId)(useSessionStore.getState());
if (parent) parentForSpawn = parent;
}
const newSessionId = await spawnWorktreeAgentAndDispatch(parentForSpawn, config);
if (!newSessionId) return;
targetSessionId = newSessionId;
}
// ...
startBatchRun(targetSessionId, config, activeSession.autoRunFolderPath);→ Mobile/web path — const worktree: BatchRunConfig['worktree'] | undefined =
config.worktree && config.worktree.enabled
? { enabled: true, path: ..., branchName: ..., createPROnCompletion: ..., prTargetBranch: ... }
: undefined;
const batchConfig: BatchRunConfig = {
documents,
prompt: config.prompt || DEFAULT_BATCH_PROMPT,
loopEnabled: config.loopEnabled || false,
maxLoops: config.maxLoops,
...(worktree ? { worktree } : {}),
};
// ...
startBatchRun(sessionId, batchConfig, folderPath).catch(...)→ Repro
Observed:
Suggested fixMirror desktop's path in the remote handler: before calling The minimum diff is roughly: // In useAppRemoteEventListeners.ts, before startBatchRun:
let dispatchSessionId = sessionId;
if (worktree?.enabled) {
const newSessionId = await spawnWorktreeAgentAndDispatch(parentForSpawn, batchConfig);
if (!newSessionId) {
sendRemoteConfigureAutoRunResponse(responseChannel, { success: false, error: 'Worktree spawn failed' });
return;
}
dispatchSessionId = newSessionId;
}
sendRemoteConfigureAutoRunResponse(responseChannel, { success: true, sessionId: dispatchSessionId });
startBatchRun(dispatchSessionId, batchConfig, folderPath).catch(...);…plus the corresponding mobile-client-side handling of the returned Other findings from the same soak (non-blocking, FYI)
|
Contributor
Author
Afternote — child session IS being spawned, but attached to the wrong parentCorrection to my prior comment: a child session does get added to the Left Bar after a mobile/web worktree launch — it just attaches to the agent immediately above the launching agent in the Left Bar, not to the launching agent itself. I missed this on the first read because the new entry blended in among my many existing worktree sessions; chr1syy spotted it on a closer look at the Left Bar after the run. That makes this two distinct issues, not one:
Repro for the wrong-parent attachment
Keeping the |
… AutoRun Mobile/web AutoRun's "Dispatch to a separate worktree" was relying on the chokidar watcher in useWorktreeHandlers to discover the new on-disk worktree and attach it to a parent. The watcher's find-first heuristic attached the child to whichever sibling agent's worktreeConfig.basePath matched first, producing the wrong-parent attachment reported in PR RunMaestro#946. Extract spawnWorktreeAgentAndDispatch into a shared util and call it from the remote configureAutoRun handler, mirroring desktop's useAutoRunHandlers parent resolution. The remote path now spawns the child session explicitly with the correct parentSessionId before startBatchRun runs, and forwards worktreeTarget so startBatchRun skips its redundant setupWorktree. Also closes Greptile P1: AutoRunWorktreeSection now emits enabled-invalid when createPROnCompletion is true but baseBranch is still empty (race window between loadBranches resolution and the user toggling PR creation). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/renderer/hooks/remote/useAppRemoteEventListeners.ts (1)
423-446:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
batchConfig.worktree.pathmay not match the spawned worktree whencreatePROnCompletionis false.
spawnWorktreeAgentAndDispatchonly writes back intoconfig.worktreewhentarget.createPROnCompletionistrue(seeworktreeSpawn.tslines 175–184). When the mobile launch setsworktree.enabledwithcreatePROnCompletion=false,batchConfig.worktree.path/branchNameretain the values the CLI/web client sent — which may differ from the path/branch the helper actually resolved (e.g. the user typedCue Dashboardand the helper sanitized it tocue-dashboard, or the branch already existed at a different worktree path returned viaresult.existingPath).Because dispatch happens against
targetSessionId(whosecwdis correct), the run itself is fine, but downstream consumers readingbatchConfig.worktree.path/branchNamewill see stale values. Two safe options:
- Always update
batchConfig.worktreefrom the helper's resolved values (mirror whatworktreeTargetwould tell us), or- Drop
batchConfig.worktreeentirely on thecreatePROnCompletion=falsepath and rely onworktreeTarget+ the spawned session'scwd.🤖 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/hooks/remote/useAppRemoteEventListeners.ts` around lines 423 - 446, batchConfig.worktree can contain stale path/branch values because spawnWorktreeAgentAndDispatch (see worktreeSpawn.ts) may canonicalize or return an existingPath/branchName but we never copy those back into batchConfig; update useAppRemoteEventListeners.ts so that after calling spawnWorktreeAgentAndDispatch (or wherever you receive the spawn result) you overwrite batchConfig.worktree.path and batchConfig.worktree.branchName with the resolved values from the spawn result (e.g., result.existingPath / result.branchName) whenever a worktree was created/resolved (mirror what worktreeTarget would expose); alternatively, if you choose option 2, remove batchConfig.worktree when worktree.createPROnCompletion is false so downstream consumers rely on worktreeTarget/targetSessionId cwd instead—implement one of these two fixes in the code path that calls spawnWorktreeAgentAndDispatch.
♻️ Duplicate comments (1)
src/web/mobile/AutoRunWorktreeSection.tsx (1)
101-109:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
captureExceptionstill absent from bothcatchblocks — coding guideline violation.
webLogger.errorwas added (improvement over the previous commit), butcaptureExceptionfromsrc/utils/sentry.tsis still missing in both handlers. Per the guideline, recoverable network errors should still be tracked via Sentry with context.🛡️ Proposed fix — add
captureExceptionin both catch blocks+import { captureException } from '../../utils/sentry'; // branch-load catch .catch((err: unknown) => { if (cancelled) return; webLogger.error( `Failed to load branches: ${err instanceof Error ? err.message : String(err)}`, 'AutoRunWorktreeSection' ); + captureException(err instanceof Error ? err : new Error(String(err)), { + tags: { context: 'AutoRunWorktreeSection.loadBranches' }, + }); setBranches([]); setBranchLoadStatus('error'); }); // worktree-load catch .catch((err: unknown) => { if (cancelled) return; webLogger.error( `Failed to list worktrees: ${err instanceof Error ? err.message : String(err)}`, 'AutoRunWorktreeSection' ); + captureException(err instanceof Error ? err : new Error(String(err)), { + tags: { context: 'AutoRunWorktreeSection.loadWorktrees' }, + }); setExistingWorktrees([]); });As per coding guidelines: "Use Sentry utilities (
captureException,captureMessage) fromsrc/utils/sentry.tsfor explicit error reporting with context."Also applies to: 130-137
🤖 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/web/mobile/AutoRunWorktreeSection.tsx` around lines 101 - 109, The catch blocks inside AutoRunWorktreeSection currently log errors with webLogger.error but omit Sentry reporting; import captureException from src/utils/sentry.ts and call captureException(err, { component: 'AutoRunWorktreeSection', action: 'load branches' }) in the catch that wraps the branches loading (the handler that currently calls setBranches([]) and setBranchLoadStatus('error')), and likewise add captureException(err, { component: 'AutoRunWorktreeSection', action: 'load worktrees' }) in the other catch (the handler around lines 130-137) so recoverable network errors are reported with contextual metadata while retaining the existing webLogger.error and state updates.
🧹 Nitpick comments (3)
src/renderer/utils/worktreeSpawn.ts (3)
132-135: 💤 Low valueDead fallback:
branchNameis always set before this point.In the
create-newbranch we early-return whensanitizeGitBranchNameproduces an empty string (lines 56–64), and inexisting-closedwe setbranchName = worktreePath.split('/').pop() || 'worktree'(line 111) which is always truthy. Soif (!branchName && gitBranches && ...)can never fire. Either drop the block or move it before theexisting-closedfallback so the git-detected branch is preferred over the path basename.🤖 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/utils/worktreeSpawn.ts` around lines 132 - 135, The fallback that sets branchName from gitBranches is dead because branchName is always set earlier; move the block that picks gitBranches[0] (the if (!branchName && gitBranches && gitBranches.length > 0) {...}) to execute before the existing-closed fallback that sets branchName = worktreePath.split('/').pop() || 'worktree' (or simply remove the block if you prefer path basename to win). In practice, relocate the git-detected branch selection so it runs after sanitizeGitBranchName and before the worktreePath basename assignment (references: branchName, gitBranches, sanitizeGitBranchName, worktreePath) so the git-detected branch is preferred.
65-68: 💤 Low valuePath derivation breaks on Windows local sessions.
parentSession.cwd.replace(/\/[^/]+$/, '') + '/worktrees'andworktreePath = basePath + '/' + branchNameassume POSIX separators. On a Windows local session wherecwdisC:\Users\me\repo, the regex won't match (no forward slashes),basePathbecomesC:\Users\me\repo/worktrees, and the resulting path mixes separators. Same concern at line 111 withworktreePath.split('/').pop().This isn't a regression (the logic was extracted from the pre-existing handler with the same shape), so flagging it as a follow-up rather than a blocker. SSH sessions and macOS/Linux are unaffected.
🤖 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/utils/worktreeSpawn.ts` around lines 65 - 68, The path logic assumes POSIX separators and breaks on Windows; update the worktree path derivation in worktreeSpawn.ts to use Node's path utilities: import path from 'path' and compute basePath with either path.join(parentSession.worktreeConfig.basePath, ...) when worktreeConfig.basePath exists or path.join(path.dirname(parentSession.cwd), 'worktrees') when falling back, then set worktreePath = path.join(basePath, branchName); likewise replace worktreePath.split('/').pop() with path.basename(worktreePath) so separator handling is correct on Windows and POSIX environments.
125-130: ⚡ Quick winRemove duplicate Sentry capture from
gitService.getBranchescall.
gitService.getBranchesis implemented viacreateIpcMethodwithdefaultValue: []and withoutrethrow: true. This configuration causes the IPC layer to swallow failures internally and report them to Sentry (viacaptureExceptioninipcWrapper.ts:108). The try/catch block at lines 125–130 is dead code—the call never throws, and any exception would already be captured by the IPC wrapper.Remove the try/catch and simplify:
- // Step 3: Fetch git info for the worktree - let gitBranches: string[] | undefined; - try { - gitBranches = await gitService.getBranches(worktreePath, sshRemoteId); - } catch (err) { - // Non-fatal — git info is nice-to-have - captureException(err, { extra: { worktreePath, sshRemoteId } }); - } + // Step 3: Fetch git info for the worktree (errors reported via IPC layer). + const gitBranches = await gitService.getBranches(worktreePath, sshRemoteId);🤖 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/utils/worktreeSpawn.ts` around lines 125 - 130, Remove the dead try/catch around gitService.getBranches in worktreeSpawn.ts: since createIpcMethod for getBranches uses defaultValue: [] and doesn't rethrow, the IPC wrapper already captures errors (ipcWrapper.ts captureException at the IPC layer), so delete the try/catch and just assign gitBranches = await gitService.getBranches(worktreePath, sshRemoteId); leaving the defaultValue behavior intact and avoiding a duplicate Sentry capture.
🤖 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/__tests__/web/mobile/AutoRunWorktreeSection.test.tsx`:
- Around line 235-236: The test is resolving the pending promise by calling
resolveBranches!(...) outside React's act(), which triggers state setters
(setBranches, setBaseBranch, setNewBranchName, setBranchLoadStatus) on
AutoRunWorktreeSection and can produce act() warnings; fix it by wrapping the
resolveBranches!({ branches: ['main'], currentBranch: 'main' }) call inside
act(() => { ... }) (or await act(async () => { ... }) if needed) so the
component state updates occur within React's act boundary.
In `@src/renderer/hooks/remote/useAppRemoteEventListeners.ts`:
- Around line 482-488: spawnWorktreeAgentAndDispatch currently returns null on
failure which causes sendRemoteConfigureAutoRunResponse (via
window.maestro.process.sendRemoteConfigureAutoRunResponse) to always send the
generic 'Failed to spawn worktree agent'; change spawnWorktreeAgentAndDispatch
to return a tagged result (e.g. { sessionId: string } | { error: string | Error
} or set a small ref with lastError) and then update this handler to inspect
that result and forward the specific error string (or result.error) in the
sendRemoteConfigureAutoRunResponse payload instead of the hardcoded message so
the mobile client receives the real failure reason.
- Around line 480-520: The success response currently omits the spawned agent
session ID so the mobile client cannot navigate; add an optional sessionId?:
string to the IPC response type in the renderer type declaration and preload API
(update the response type in global.d.ts and the process.ts signature), then
modify the renderer call to
window.maestro.process.sendRemoteConfigureAutoRunResponse to include sessionId:
targetSessionId when targetSessionId !== original sessionId (or when
targetSessionId is set), and update the web-server/message handler that emits
configure_auto_run_result to propagate that sessionId through the
configure_auto_run_result payload so callers receive the new session ID.
In `@src/web/mobile/AutoRunWorktreeSection.tsx`:
- Line 225: branchNameMissing is currently set to enabled &&
!newBranchName.trim() which triggers a "Branch name is required" warning while
branches are still loading; update the validation in AutoRunWorktreeSection to
also check branchLoadStatus (use the existing branchLoadStatus variable) and
only mark branchNameMissing true when branchLoadStatus indicates loading is
finished (e.g., branchLoadStatus === 'loaded' or !== 'loading') so the warning
does not flash before a default branch name is seeded.
---
Outside diff comments:
In `@src/renderer/hooks/remote/useAppRemoteEventListeners.ts`:
- Around line 423-446: batchConfig.worktree can contain stale path/branch values
because spawnWorktreeAgentAndDispatch (see worktreeSpawn.ts) may canonicalize or
return an existingPath/branchName but we never copy those back into batchConfig;
update useAppRemoteEventListeners.ts so that after calling
spawnWorktreeAgentAndDispatch (or wherever you receive the spawn result) you
overwrite batchConfig.worktree.path and batchConfig.worktree.branchName with the
resolved values from the spawn result (e.g., result.existingPath /
result.branchName) whenever a worktree was created/resolved (mirror what
worktreeTarget would expose); alternatively, if you choose option 2, remove
batchConfig.worktree when worktree.createPROnCompletion is false so downstream
consumers rely on worktreeTarget/targetSessionId cwd instead—implement one of
these two fixes in the code path that calls spawnWorktreeAgentAndDispatch.
---
Duplicate comments:
In `@src/web/mobile/AutoRunWorktreeSection.tsx`:
- Around line 101-109: The catch blocks inside AutoRunWorktreeSection currently
log errors with webLogger.error but omit Sentry reporting; import
captureException from src/utils/sentry.ts and call captureException(err, {
component: 'AutoRunWorktreeSection', action: 'load branches' }) in the catch
that wraps the branches loading (the handler that currently calls
setBranches([]) and setBranchLoadStatus('error')), and likewise add
captureException(err, { component: 'AutoRunWorktreeSection', action: 'load
worktrees' }) in the other catch (the handler around lines 130-137) so
recoverable network errors are reported with contextual metadata while retaining
the existing webLogger.error and state updates.
---
Nitpick comments:
In `@src/renderer/utils/worktreeSpawn.ts`:
- Around line 132-135: The fallback that sets branchName from gitBranches is
dead because branchName is always set earlier; move the block that picks
gitBranches[0] (the if (!branchName && gitBranches && gitBranches.length > 0)
{...}) to execute before the existing-closed fallback that sets branchName =
worktreePath.split('/').pop() || 'worktree' (or simply remove the block if you
prefer path basename to win). In practice, relocate the git-detected branch
selection so it runs after sanitizeGitBranchName and before the worktreePath
basename assignment (references: branchName, gitBranches, sanitizeGitBranchName,
worktreePath) so the git-detected branch is preferred.
- Around line 65-68: The path logic assumes POSIX separators and breaks on
Windows; update the worktree path derivation in worktreeSpawn.ts to use Node's
path utilities: import path from 'path' and compute basePath with either
path.join(parentSession.worktreeConfig.basePath, ...) when
worktreeConfig.basePath exists or path.join(path.dirname(parentSession.cwd),
'worktrees') when falling back, then set worktreePath = path.join(basePath,
branchName); likewise replace worktreePath.split('/').pop() with
path.basename(worktreePath) so separator handling is correct on Windows and
POSIX environments.
- Around line 125-130: Remove the dead try/catch around gitService.getBranches
in worktreeSpawn.ts: since createIpcMethod for getBranches uses defaultValue: []
and doesn't rethrow, the IPC wrapper already captures errors (ipcWrapper.ts
captureException at the IPC layer), so delete the try/catch and just assign
gitBranches = await gitService.getBranches(worktreePath, sshRemoteId); leaving
the defaultValue behavior intact and avoiding a duplicate Sentry capture.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 437ffd73-02c5-41e7-80dd-971368538751
📒 Files selected for processing (5)
src/__tests__/web/mobile/AutoRunWorktreeSection.test.tsxsrc/renderer/hooks/batch/useAutoRunHandlers.tssrc/renderer/hooks/remote/useAppRemoteEventListeners.tssrc/renderer/utils/worktreeSpawn.tssrc/web/mobile/AutoRunWorktreeSection.tsx
Comment on lines
+480
to
520
| try { | ||
| const newSessionId = await spawnWorktreeAgentAndDispatch(parentForSpawn, spawnConfig); | ||
| if (!newSessionId) { | ||
| window.maestro.process.sendRemoteConfigureAutoRunResponse(responseChannel, { | ||
| success: false, | ||
| error: 'Failed to spawn worktree agent', | ||
| }); | ||
| return; | ||
| } | ||
| targetSessionId = newSessionId; | ||
| // spawnWorktreeAgentAndDispatch may rewrite spawnConfig.worktree | ||
| // (e.g. when the on-disk worktree already existed at a different | ||
| // path or createPROnCompletion was requested). Reflect that on | ||
| // the batchConfig forwarded to startBatchRun. | ||
| if (spawnConfig.worktree) { | ||
| batchConfig.worktree = spawnConfig.worktree; | ||
| } | ||
| // Setting worktreeTarget tells startBatchRun to skip its own | ||
| // setupWorktree call — the spawn helper already created the | ||
| // directory and built the session. | ||
| batchConfig.worktreeTarget = spawnConfig.worktreeTarget; | ||
| } catch (err) { | ||
| logger.error('[Remote] Failed to spawn worktree agent:', undefined, err); | ||
| notifyToast({ | ||
| type: 'error', | ||
| title: 'Worktree Error', | ||
| message: err instanceof Error ? err.message : String(err), | ||
| }); | ||
| window.maestro.process.sendRemoteConfigureAutoRunResponse(responseChannel, { | ||
| success: false, | ||
| error: err instanceof Error ? err.message : String(err), | ||
| }); | ||
| return; | ||
| } | ||
| } | ||
|
|
||
| // Send success response immediately - startBatchRun is long-running | ||
| // and would exceed the IPC/CLI timeout if awaited. | ||
| window.maestro.process.sendRemoteConfigureAutoRunResponse(responseChannel, { | ||
| success: true, | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm the configure-auto-run response type and check the WS handler/mobile consumer.
rg -nP -C3 'sendRemoteConfigureAutoRunResponse' --type=ts
rg -nP -C3 'configure_auto_run_result|ConfigureAutoRunResponse|RemoteConfigureAutoRunResponse' --type=tsRepository: RunMaestro/Maestro
Length of output: 24517
Add sessionId to the success response so the mobile client can navigate to spawned agents.
After a worktree spawn, the mobile caller needs the new session ID to display and navigate to the spawned child session. Today the success path sends { success: true } (line 518–520) with no ID, leaving the mobile UI unable to switch to the agent running the batch.
The response type currently lacks this field. Update the type definition in src/renderer/global.d.ts (line 378) and src/main/preload/process.ts (line 671–672) to include sessionId?: string, then plumb it through:
- Renderer: pass
targetSessionIdwhen it differs from the originalsessionId(line 518–520) - Web-server handler: propagate the new ID in
configure_auto_run_result(messageHandlers.ts, line 1587–1590)
This is a blocking acceptance criterion per the PR objectives.
🤖 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/hooks/remote/useAppRemoteEventListeners.ts` around lines 480 -
520, The success response currently omits the spawned agent session ID so the
mobile client cannot navigate; add an optional sessionId?: string to the IPC
response type in the renderer type declaration and preload API (update the
response type in global.d.ts and the process.ts signature), then modify the
renderer call to window.maestro.process.sendRemoteConfigureAutoRunResponse to
include sessionId: targetSessionId when targetSessionId !== original sessionId
(or when targetSessionId is set), and update the web-server/message handler that
emits configure_auto_run_result to propagate that sessionId through the
configure_auto_run_result payload so callers receive the new session ID.
Comment on lines
+482
to
+488
| if (!newSessionId) { | ||
| window.maestro.process.sendRemoteConfigureAutoRunResponse(responseChannel, { | ||
| success: false, | ||
| error: 'Failed to spawn worktree agent', | ||
| }); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Generic 'Failed to spawn worktree agent' swallows the helper's specific reason.
When spawnWorktreeAgentAndDispatch returns null it has already shown a desktop toast with the actual cause (invalid branch name, busy target agent, worktreeSetup failure with result.error, etc.), but the mobile client only sees the generic string here. Consider having the helper return a tagged result (or read the most recent error from a small ref) so the WS response can propagate the real reason — otherwise the mobile UX is "Failed to spawn worktree agent" with no actionable detail.
🤖 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/hooks/remote/useAppRemoteEventListeners.ts` around lines 482 -
488, spawnWorktreeAgentAndDispatch currently returns null on failure which
causes sendRemoteConfigureAutoRunResponse (via
window.maestro.process.sendRemoteConfigureAutoRunResponse) to always send the
generic 'Failed to spawn worktree agent'; change spawnWorktreeAgentAndDispatch
to return a tagged result (e.g. { sessionId: string } | { error: string | Error
} or set a small ref with lastError) and then update this handler to inspect
that result and forward the specific error string (or result.error) in the
sendRemoteConfigureAutoRunResponse payload instead of the hardcoded message so
the mobile client receives the real failure reason.
Contributor
Author
|
@greptileai re-review please |
- useAppRemoteEventListeners: stop pre-populating batchConfig.worktree with the raw mobile payload. The user-typed branch name and computed path can drift from the values spawnWorktreeAgentAndDispatch resolves (sanitized branch, or an existingPath returned by `git worktree add`). Mirror the desktop launch shape: leave batchConfig.worktree undefined unless the spawn helper writes it back (createPROnCompletion=true), rely on worktreeTarget + the spawned session's cwd otherwise. - worktreeSpawn: drop the dead `!branchName && gitBranches[0]` fallback (branchName is always set before that point) and remove the duplicate Sentry capture from the gitService.getBranches call — the IPC wrapper already reports failures via captureException. Keep a bare try/catch to preserve the non-fatal contract. - AutoRunWorktreeSection: gate the "Branch name is required" warning on branchLoadStatus !== 'loading' so it doesn't flash before the seeded default arrives. - AutoRunWorktreeSection.test: wrap the trailing `resolveBranches!` resolution in act() so its component state updates are inside React's act boundary. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
Author
Wrong-parent fix verified — unblockingRe-tested 2026-05-07 against the What's fixed
Removing the Non-blocking follow-ups (mobile/web parity gaps)Two UI affordance gaps surfaced during the same soak — these don't block this PR but should be tracked as follow-up work:
Filed as a gist with repro and suggested fix directions: https://gist.github.com/chr1syy/67630166ff5217a97d7b1fad91201f3b Suggested fix directions in the gist: Center Flash + jump-session click action for the launch ack (Gap 1), and mirroring desktop's Other items still untested in this pass (deferred from yesterday's plan, not part of this PR's blocker history):
These can be exercised in a follow-up pass once the gist's items are addressed. |
chr1syy
added
ready to merge
This was referenced May 7, 2026
chr1syy
added a commit
to chr1syy/Maestro
that referenced
this pull request
May 9, 2026
…h indicator
- useAutoRun.ts: extend the configure_auto_run timeout to 60s when
worktree dispatch is enabled (greptile P1). Worktree launches block
on `git worktree add` plus an upstream getBranches round-trip on the
server, which routinely exceeds the default 10s and surfaces as a
spurious "Request timed out" failure.
- useAutoRun.ts: log unexpected launch failures via webLogger before
converting them into `{ success: false }` (CodeRabbit). The web bundle
has no Sentry wired up, so the logger is the equivalent surface.
- mobile/App.tsx: read the pre-launch session state from the current
sessions snapshot before scheduling the optimistic update instead of
capturing it as a side effect inside the `setSessions` updater
(greptile P2). React 18 Concurrent Mode is allowed to re-run updater
callbacks for speculative/interrupted renders.
- mobile/App.tsx: header indicator now prefers the session-level
`connecting` state over the active tab's state, and the dot animates
on both `busy` and `connecting` so the launching agent's pulsing
orange feedback actually pulses (CodeRabbit).
Note: the remaining four review comments target files introduced by
PR RunMaestro#946 (`useAppRemoteEventListeners.ts`, `worktreeSpawn.ts`,
`AutoRunSetupSheet.tsx`, `AutoRunWorktreeSection.tsx`) and belong with
that PR rather than this Gap 1 follow-up.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
chr1syy
added a commit
to chr1syy/Maestro
that referenced
this pull request
May 9, 2026
Closes Gap 3 from the PR RunMaestro#946/RunMaestro#947 mobile/web AutoRun parity follow-up. Desktop has an "Add Docs" affordance that surfaces both Create-doc and Marketplace as co-equal entry points. On mobile/web, the empty state of AutoRunInline only offered "+ Create document" — users with a fresh AutoRun folder had no in-UI path to discover existing playbooks and had to know to open the launch sheet just to find the marketplace entry. - Add onOpenMarketplace?: () => void prop to AutoRunInline. Render a "Browse Playbook Exchange" CTA next to "+ Create document" in the empty state when the prop is provided. Both buttons share the same vertical stack so the discoverability matches desktop's overview. - Plumb the prop through AutoRunPanel (full-screen overlay), AutoRunTabContent / RightDrawer (inline tab), and RightPanel. - Wire handleOpenMarketplaceSheet through from App.tsx for both AutoRunPanel and RightPanel render sites; reuses the same MarketplaceSheet and WS infrastructure introduced by PR RunMaestro#947, so no new IPC/server surface. The CTA is opt-in (prop is optional) so existing callers and tests keep working without change. Existing scoped tests pass; new AutoRunInline.test.tsx covers the empty-state CTA visibility, hidden-when-unset, and onClick behaviour. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
chr1syy
added a commit
to chr1syy/Maestro
that referenced
this pull request
May 10, 2026
pedramamini
pushed a commit
that referenced
this pull request
May 10, 2026
- useAutoRun.ts: extend the configure_auto_run timeout to 60s when
worktree dispatch is enabled (greptile P1). Worktree launches block
on `git worktree add` plus an upstream getBranches round-trip on the
server, which routinely exceeds the default 10s and surfaces as a
spurious "Request timed out" failure.
- useAutoRun.ts: log unexpected launch failures via webLogger before
converting them into `{ success: false }` (CodeRabbit). The web bundle
has no Sentry wired up, so the logger is the equivalent surface.
- mobile/App.tsx: read the pre-launch session state from the current
sessions snapshot before scheduling the optimistic update instead of
capturing it as a side effect inside the `setSessions` updater
(greptile P2). React 18 Concurrent Mode is allowed to re-run updater
callbacks for speculative/interrupted renders.
- mobile/App.tsx: header indicator now prefers the session-level
`connecting` state over the active tab's state, and the dot animates
on both `busy` and `connecting` so the launching agent's pulsing
orange feedback actually pulses (CodeRabbit).
Note: the remaining four review comments target files introduced by
PR #946 (`useAppRemoteEventListeners.ts`, `worktreeSpawn.ts`,
`AutoRunSetupSheet.tsx`, `AutoRunWorktreeSection.tsx`) and belong with
that PR rather than this Gap 1 follow-up.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
chr1syy
added a commit
to chr1syy/Maestro
that referenced
this pull request
May 10, 2026
Closes Gap 3 from the PR RunMaestro#946/RunMaestro#947 mobile/web AutoRun parity follow-up. Desktop has an "Add Docs" affordance that surfaces both Create-doc and Marketplace as co-equal entry points. On mobile/web, the empty state of AutoRunInline only offered "+ Create document" — users with a fresh AutoRun folder had no in-UI path to discover existing playbooks and had to know to open the launch sheet just to find the marketplace entry. - Add onOpenMarketplace?: () => void prop to AutoRunInline. Render a "Browse Playbook Exchange" CTA next to "+ Create document" in the empty state when the prop is provided. Both buttons share the same vertical stack so the discoverability matches desktop's overview. - Plumb the prop through AutoRunPanel (full-screen overlay), AutoRunTabContent / RightDrawer (inline tab), and RightPanel. - Wire handleOpenMarketplaceSheet through from App.tsx for both AutoRunPanel and RightPanel render sites; reuses the same MarketplaceSheet and WS infrastructure introduced by PR RunMaestro#947, so no new IPC/server surface. The CTA is opt-in (prop is optional) so existing callers and tests keep working without change. Existing scoped tests pass; new AutoRunInline.test.tsx covers the empty-state CTA visibility, hidden-when-unset, and onClick behaviour. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
pedramamini
pushed a commit
that referenced
this pull request
May 10, 2026
Closes Gap 3 from the PR #946/#947 mobile/web AutoRun parity follow-up. Desktop has an "Add Docs" affordance that surfaces both Create-doc and Marketplace as co-equal entry points. On mobile/web, the empty state of AutoRunInline only offered "+ Create document" — users with a fresh AutoRun folder had no in-UI path to discover existing playbooks and had to know to open the launch sheet just to find the marketplace entry. - Add onOpenMarketplace?: () => void prop to AutoRunInline. Render a "Browse Playbook Exchange" CTA next to "+ Create document" in the empty state when the prop is provided. Both buttons share the same vertical stack so the discoverability matches desktop's overview. - Plumb the prop through AutoRunPanel (full-screen overlay), AutoRunTabContent / RightDrawer (inline tab), and RightPanel. - Wire handleOpenMarketplaceSheet through from App.tsx for both AutoRunPanel and RightPanel render sites; reuses the same MarketplaceSheet and WS infrastructure introduced by PR #947, so no new IPC/server surface. The CTA is opt-in (prop is optional) so existing callers and tests keep working without change. Existing scoped tests pass; new AutoRunInline.test.tsx covers the empty-state CTA visibility, hidden-when-unset, and onClick behaviour. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
chr1syy
added a commit
that referenced
this pull request
Jul 11, 2026
…og (#1145) * fix(autorun): correct worktree attribution + add absolute task watchdog Two related Auto Run robustness fixes surfaced by a web-desktop launch of a two-document playbook with auto-worktree creation. 1) Worktree attribution across same-repo agents (PR #946 follow-up) - worktreeSpawn: re-mark the resolved worktree path as recently-created AFTER worktreeSetup (create-new only). The original 10s dedup TTL started before the potentially slow `git worktree add`, so the chokidar discovery could fire during the getBranches/buildWorktreeSession window after the mark aged out, letting a sibling agent watching the same basePath adopt the worktree under the wrong parent. Re-marking also covers the existingPath reassignment the original mark never did. - useWorktreeHandlers: scope onWorktreeDiscovered dedup per-parent instead of globally by cwd. Each same-repo agent runs its own watcher and fires its own discovery event, so an externally-created worktree now fans out: every agent in the cwd gets its own child instead of the first watcher claiming it globally and the others silently dropping it. Maestro-spawned worktrees still attach only to the launching agent (the dedup mark suppresses sibling watchers). 2) Absolute Auto Run task watchdog The per-document batch loop awaits processTask, whose only recovery was the silence-based inactivity watchdog. A stuck-but-chatty agent that keeps emitting output (resetting the silence timer) but never finishes defeats it, hanging the whole multi-document run with no advance to the next document. - Add an absolute wall-clock per-task cap (autoRunMaxTaskDurationMin, default 480 min, 0 = unlimited) that force-kills regardless of output and resolves the task as watchdog-timeout, which the batch loop already handles by terminating the document and moving on. - Wire the new setting through metadata, store, useSettings, the General tab UI (alongside the inactivity timeout), and searchable settings. Tests: worktree attribution (per-parent fan-out, mark-skip, idempotency, cross-repo reject), worktreeSpawn re-mark robustness, and batch watchdog (absolute cap fires on chatty-stuck, inactivity still fires on silence, both unlimited = no watchdog). Updated the existing mark-order assertion. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(autorun): address review feedback on worktree attribution + watchdog Scope worktree reuse/dedup to the launching parent and harden the Auto Run max-task-duration cap, per PR #1145 review threads. - worktreeSpawn: reuse an existing worktree session only when it is OWNED by the launching parent (new sessionOwnedByParent helper). A global cwd match let a same-repo sibling's child be adopted for this parent's Auto Run, reproducing the wrong-parent attribution the flow exists to prevent (codex + greptile P1). - useWorktreeHandlers rescan: apply the same per-parent dedup that the chokidar path already uses, so a restart/visibility rescan fans a shared worktree out to every same-repo parent instead of letting the first to iterate claim it globally (codex P2). - settingsStore: sanitize the persisted autoRunMaxTaskDurationMin on load so a corrupt (non-finite/negative/wrong-type) value can no longer silently DISABLE the cap; extract clampAutoRunMaxTaskDurationMin so setter and loader agree (greptile P2). - settingsStore: migrate existing "Unlimited" inactivity installs (persisted 0 with no cap set) to an unlimited absolute cap (0) so the new 480-min default doesn't silently start killing previously-uncapped runs (codex P2). - Replace prohibited em/en dashes introduced by this PR in copy, comments, and tests per the repo style rule (codex P2). Adds tests: parent-scoped worktree reuse, rescan per-parent fan-out, and the watchdog sanitize/clamp/migration helpers. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(autorun): tighten worktree ownership + rescan/dedup-mark robustness Follow-up review on the worktree-attribution fix: - sessionOwnedByParent: match ownership by parentSessionId ONLY. The legacy worktreeParentPath fallback compared against the parent's cwd, but that field is the scanned worktree BASE directory, which several same-repo parents can share - so it could match the wrong owner. Legacy sessions now fall through to getting a fresh new-model child for the launcher, which is safe (no misattribution) even if the legacy child isn't reused. - scanWorktreeConfigs: skip a subdir whose path is still marked recently-created by spawnWorktreeAgentAndDispatch (mirrors the chokidar listener). Without it, a startup/visibility rescan inside the spawn window would fan a Maestro-launched worktree out under every same-repo parent - and the rescan is the ONLY discovery path on SSH, where chokidar is unavailable. - worktreeSpawn: give the pre-`git worktree add` dedup mark a 60s TTL so it can't expire mid-setup on a large repo / cold disk / SSH remote (the default 10s window could lapse before the post-setup re-mark). The error/already-existed branches clear the path and the success path re-marks with the default TTL, so the long window never outlives the setup it guards. Tests: rescan skips a recently-created marked path; existing parent-scoped reuse and fan-out tests still pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(autorun): concurrency + migration + copy fixes from follow-up review - worktreeSpawn: refuse to build a second child into a worktree a BUSY sibling is already running in. worktreeSetup can resolve to a branch attached under a sibling agent; reuse stays parent-scoped, but dispatching a fresh child into that same checkout would let two agents write it concurrently. Bail if any same-root session is busy/connecting. - worktreeSpawn: re-mark the resolved path with the long setup TTL (not the default 10s). The owning session isn't committed until after the awaited getBranches, which is slow on SSH/large repos, so the mark must survive that window too. - settingsStore: make the "Unlimited inactivity" -> unlimited-cap migration one-shot by persisting the migrated value. Without writing the key back, the branch re-ran on every load and would silently reset a cap the user set later. - useWorktreeHandlers.handleSaveWorktreeConfig: scope the save-time scan's existing-child checks to the active parent (and skip recently-created marks), mirroring scanWorktreeConfigs. A global cwd match let a second same-repo agent enabling the same basePath be skipped, leaving it childless until a later rescan. - Clarify the inactivity "Unlimited" copy: it disables the inactivity watchdog only; the Max Task Duration cap still applies unless also set to Unlimited. Tests: busy same-root sibling is rejected (no duplicate child); the unlimited migration persists its value one-shot. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(autorun): expect the 60s setup TTL on the worktree dedup re-mark Follows the re-mark TTL change: the worktree Auto Run test asserted markWorktreePathAsRecentlyCreated was called with only the path, but both marks now pass WORKTREE_SETUP_MARK_TTL_MS (60000). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This was referenced Jul 25, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
WorktreeRunSection.get_git_branchesandlist_worktrees, proxy git directly from the main process using each session'scwd+sessionSshRemoteConfig.worktreepayload throughconfigure_auto_run, where the desktop's existing remote-handler already creates the worktree, checks out the branch, and (optionally) opens a PR.What changed
Backend (
src/main/web-server/)handlers/messageHandlers.ts— newget_git_branchesandlist_worktreeshandlers withsessionIdvalidation, "not configured" fallback, andrequestIdecho.types.ts—SessionData/SessionBroadcastDatagainisGitRepo+worktreeBasePath; newGitBranchesResult,WorktreeEntry,ListWorktreesResult, and the corresponding callback type aliases.managers/CallbackRegistry.ts,WebServer.ts— registry slots and setter exposure for the new callbacks.web-server-factory.ts— emitsisGitRepo+worktreeBasePathon the sessions list and wires the two new callbacks viaexecGit(SSH-aware) on the session's cwd.Frontend (
src/web/)hooks/useAutoRun.ts— newLaunchWorktreeConfig,WorktreeSummary,loadGitBranches,listWorktrees;launchAutoRunspreadsworktreeonly whenenabled === true.hooks/useWebSocket.ts—SessionDatagainsisGitRepo+worktreeBasePath.mobile/AutoRunWorktreeSection.tsx(new) — toggle, base-branch picker, sanitized branch-name input with path preview, PR checkbox; gated onisGitRepo+worktreeBasePath.mobile/AutoRunSetupSheet.tsx,mobile/App.tsx— section rendered above the Prompt, loaders bound to the active session.Tests
__tests__/main/web-server/handlers/messageHandlers.test.ts— six new cases (validation, payload shape, callback-not-configured) for the two WS handlers.__tests__/web/hooks/useAutoRun.test.ts(new) —launchAutoRunworktree forwarding/strip + branch/worktree request shape and error fallbacks.__tests__/web/mobile/AutoRunWorktreeSection.test.tsx(new) — render gating, enable→config emission, PR toggle, branch-cleared→null.__tests__/web/mobile/App.test.tsxmock to expose the new hook methods.Test plan
<basePath>/<branchName>.worktreeBasePathis configured.sessionSshRemoteConfig).npx vitest runon the scoped suite (158 pass / 0 fail).🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests