Require base branch selection before first send in worktree mode#99
Merged
juliusmarminge merged 5 commits intomainfrom Feb 27, 2026
Merged
Require base branch selection before first send in worktree mode#99juliusmarminge merged 5 commits intomainfrom
juliusmarminge merged 5 commits intomainfrom
Conversation
WalkthroughRefactors the worktree creation flow in ChatView to add explicit guard conditions, improved state handling, and synchronization. When in worktree mode with no messages, validates base branch selection before proceeding. Upon successful worktree creation, updates thread metadata and immediately syncs local state via dispatch. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Autofix Details
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Stale threadId after await causes wrong-thread updates
- Captured
const threadId = activeThread.idbefore any await points inonSendand replaced all subsequentactiveThread.idreferences with the capturedthreadIdto prevent stale closure references after thread switches.
- Captured
- ✅ Fixed: Setup script failure does not abort send
- Added
throw errorafter the SET_ERROR dispatch inrunProjectScript's catch block so errors propagate to the worktree creation try-catch which aborts the send, and updated fire-and-forget callers to use.catch(() => {})to avoid unhandled rejections.
- Added
Or push these changes by commenting:
@cursor push f793d6c97e
Preview (f793d6c97e)
diff --git a/apps/web/src/components/ChatView.tsx b/apps/web/src/components/ChatView.tsx
--- a/apps/web/src/components/ChatView.tsx
+++ b/apps/web/src/components/ChatView.tsx
@@ -792,6 +792,7 @@
threadId: activeThreadId,
error: error instanceof Error ? error.message : `Failed to run script "${script.name}".`,
});
+ throw error;
}
},
[activeProject, activeThread, activeThreadId, dispatch, gitCwd],
@@ -1153,7 +1154,7 @@
if (!script) return;
event.preventDefault();
event.stopPropagation();
- void runProjectScript(script);
+ runProjectScript(script).catch(() => {});
};
window.addEventListener("keydown", handler);
return () => window.removeEventListener("keydown", handler);
@@ -1336,6 +1337,7 @@
if (!trimmed && composerImages.length === 0) return;
if (!activeProject) return;
const composerImagesSnapshot = [...composerImages];
+ const threadId = activeThread.id;
const shouldCreateWorktree =
activeThread.messages.length === 0 && envMode === "worktree" && !activeThread.worktreePath;
@@ -1345,7 +1347,7 @@
if (shouldCreateWorktree && !activeThread.branch) {
dispatch({
type: "SET_ERROR",
- threadId: activeThread.id,
+ threadId,
error: "Select a base branch before sending in New worktree mode.",
});
return;
@@ -1363,7 +1365,7 @@
await api.orchestration.dispatchCommand({
type: "thread.meta.update",
commandId: newCommandId(),
- threadId: activeThread.id,
+ threadId,
branch: result.worktree.branch,
worktreePath: result.worktree.path,
});
@@ -1371,7 +1373,7 @@
// with the worktree cwd/env instead of briefly using the project root.
dispatch({
type: "SET_THREAD_BRANCH",
- threadId: activeThread.id,
+ threadId,
branch: result.worktree.branch,
worktreePath: result.worktree.path,
});
@@ -1387,7 +1389,7 @@
} catch (err) {
dispatch({
type: "SET_ERROR",
- threadId: activeThread.id,
+ threadId,
error: err instanceof Error ? err.message : "Failed to create worktree",
});
return;
@@ -1406,13 +1408,13 @@
await api.orchestration.dispatchCommand({
type: "thread.meta.update",
commandId: newCommandId(),
- threadId: activeThread.id,
+ threadId,
title,
});
}
}
- setThreadError(activeThread.id, null);
+ setThreadError(threadId, null);
promptRef.current = "";
setPrompt("");
setComposerImages([]);
@@ -1443,7 +1445,7 @@
await api.orchestration.dispatchCommand({
type: "thread.turn.start",
commandId: newCommandId(),
- threadId: activeThread.id,
+ threadId,
message: {
messageId: newMessageId(),
role: "user",
@@ -1456,7 +1458,7 @@
});
} catch (err) {
setThreadError(
- activeThread.id,
+ threadId,
err instanceof Error ? err.message : "Failed to send message.",
);
} finally {
@@ -1713,7 +1715,7 @@
gitCwd={gitCwd}
diffOpen={diffOpen}
onRunProjectScript={(script) => {
- void runProjectScript(script);
+ runProjectScript(script).catch(() => {});
}}
onAddProjectScript={saveProjectScript}
onUpdateProjectScript={updateProjectScript}![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/components/ChatView.tsx (1)
1340-1386:⚠️ Potential issue | 🟠 MajorPrevent duplicate first-send while async worktree setup is in progress.
Line 1340 introduces extra async steps before send-state is locked, but
setIsSending(true)is only set later (Line 1422). A quick second submit can re-enteronSendand issue duplicate non-idempotent calls (gitCreateWorktree/thread.turn.start).🔧 Suggested fix
@@ const composerImagesRef = useRef<ComposerImageAttachment[]>([]); + const sendInFlightRef = useRef(false); @@ const onSend = async (e: React.SubmitEvent | React.KeyboardEvent) => { e.preventDefault(); const api = readNativeApi(); - if (!api || !activeThread || isSending || isConnecting) return; + if (!api || !activeThread || isSending || isConnecting || sendInFlightRef.current) return; @@ const composerImagesSnapshot = [...composerImages]; + sendInFlightRef.current = true; + setIsSending(true); + try { @@ - setIsSending(true); - try { + // existing send logic... @@ } finally { + sendInFlightRef.current = false; setIsSending(false); } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/ChatView.tsx` around lines 1340 - 1386, When shouldCreateWorktree is true, lock send-state immediately to prevent a second concurrent onSend: at the start of the onSend path (inside the shouldCreateWorktree branch, before awaiting createWorktreeMutation.mutateAsync), call setIsSending(true) (and/or check an existing isSending guard at the very top of onSend and return early if true). Wrap the async worktree setup (createWorktreeMutation.mutateAsync, api.orchestration.dispatchCommand, runProjectScript) in the existing try/catch/finally and ensure setIsSending(false) is called in finally on error or after completion only where appropriate so you don't leave state unlocked on failures; also ensure any early returns (e.g., SET_ERROR dispatch) respect the isSending guard.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@apps/web/src/components/ChatView.tsx`:
- Around line 1340-1386: When shouldCreateWorktree is true, lock send-state
immediately to prevent a second concurrent onSend: at the start of the onSend
path (inside the shouldCreateWorktree branch, before awaiting
createWorktreeMutation.mutateAsync), call setIsSending(true) (and/or check an
existing isSending guard at the very top of onSend and return early if true).
Wrap the async worktree setup (createWorktreeMutation.mutateAsync,
api.orchestration.dispatchCommand, runProjectScript) in the existing
try/catch/finally and ensure setIsSending(false) is called in finally on error
or after completion only where appropriate so you don't leave state unlocked on
failures; also ensure any early returns (e.g., SET_ERROR dispatch) respect the
isSending guard.
juliusmarminge
force-pushed
the
codething/7144546f
branch
2 times, most recently
from
f81a72b to
451a2e7
Compare
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Double-send race during worktree setup
- Moved setIsSending(true) before the worktree creation block and added setIsSending(false) in the worktree catch path, closing the race window where a second send could pass the isSending guard during the awaited runProjectScript.
Or push these changes by commenting:
@cursor push 88202b5fca
Preview (88202b5fca)
diff --git a/apps/web/src/components/ChatView.tsx b/apps/web/src/components/ChatView.tsx
--- a/apps/web/src/components/ChatView.tsx
+++ b/apps/web/src/components/ChatView.tsx
@@ -1376,6 +1376,8 @@
return;
}
+ setIsSending(true);
+
// On first message: lock in branch + create worktree if needed.
if (shouldCreateWorktree && activeThread.branch) {
try {
@@ -1410,6 +1412,7 @@
});
}
} catch (err) {
+ setIsSending(false);
dispatch({
type: "SET_ERROR",
threadId,
@@ -1444,7 +1447,6 @@
setComposerCursor(0);
setComposerHighlightedItemId(null);
- setIsSending(true);
try {
const turnAttachments = await Promise.all(
composerImagesSnapshot.map(
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/web/src/components/ChatView.tsx (2)
1405-1409:⚠️ Potential issue | 🟠 MajorSetup script execution doesn’t currently prefer a new terminal.
Given the new flow intent, Line 1405 should pass
preferNewTerminal: true; otherwise setup may reuse the active terminal and mix output/state unexpectedly.Proposed fix
if (setupScript) { await runProjectScript(setupScript, { cwd: result.worktree.path, worktreePath: result.worktree.path, + preferNewTerminal: true, rememberAsLastInvoked: false, }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/ChatView.tsx` around lines 1405 - 1409, The call to runProjectScript in ChatView.tsx (the invocation that passes setupScript and options) doesn't set preferNewTerminal so the setup may reuse an existing terminal; update the options object passed to runProjectScript to include preferNewTerminal: true so the setup script is launched in a new terminal (locate the runProjectScript(...) call where setupScript is used and add the preferNewTerminal property to the options).
1405-1409:⚠️ Potential issue | 🟠 MajorSetup-script failures can be cleared immediately before the user sees them.
runProjectScripthandles its own errors by dispatchingSET_ERROR, but Line 1439 unconditionally clears thread errors afterward. This can hide setup failures and make first-send behavior look successful when setup actually failed.Proposed fix
const composerImagesSnapshot = [...composerImages]; + setThreadError(threadId, null); const shouldCreateWorktree = activeThread.messages.length === 0 && envMode === "worktree" && !activeThread.worktreePath; @@ - setThreadError(threadId, null); promptRef.current = "";Also applies to: 1439-1439
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/ChatView.tsx` around lines 1405 - 1409, runProjectScript currently handles failures by dispatching SET_ERROR, but the code following the await unconditionally clears thread errors which can hide setup failures; modify the call site around runProjectScript to detect success before clearing thread errors — either by changing runProjectScript to return a success/failure value (or rethrow on failure) and then only perform the "clear thread errors" action when that result indicates success, or wrap the await runProjectScript(...) in a try/catch and only clear errors in the try path while leaving the SET_ERROR state intact in the catch; reference runProjectScript and the SET_ERROR dispatch when locating where to gate the clearing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@apps/web/src/components/ChatView.tsx`:
- Around line 1405-1409: The call to runProjectScript in ChatView.tsx (the
invocation that passes setupScript and options) doesn't set preferNewTerminal so
the setup may reuse an existing terminal; update the options object passed to
runProjectScript to include preferNewTerminal: true so the setup script is
launched in a new terminal (locate the runProjectScript(...) call where
setupScript is used and add the preferNewTerminal property to the options).
- Around line 1405-1409: runProjectScript currently handles failures by
dispatching SET_ERROR, but the code following the await unconditionally clears
thread errors which can hide setup failures; modify the call site around
runProjectScript to detect success before clearing thread errors — either by
changing runProjectScript to return a success/failure value (or rethrow on
failure) and then only perform the "clear thread errors" action when that result
indicates success, or wrap the await runProjectScript(...) in a try/catch and
only clear errors in the try path while leaving the SET_ERROR state intact in
the catch; reference runProjectScript and the SET_ERROR dispatch when locating
where to gate the clearing.
6512bee to
27d233d
Compare
- show an explicit error when New worktree mode has no base branch selected - sync thread branch/worktree path immediately after creation - run setup script in the new worktree terminal before continuing Co-authored-by: codex <codex@users.noreply.github.com>
- Store `activeThread.id` once at send start and reuse it - Prevents updates/errors from targeting the wrong thread during async worktree/send steps
- Remove `preferNewTerminal` when invoking `runProjectScript` in `ChatView` - Lets setup scripts use default terminal behavior for worktree runs
- Add `sendInFlightRef` guard in `ChatView` send handler - Set sending state before async work and always clear it in `finally` - Wrap early send flow in one `try/finally` to avoid double-submit races
juliusmarminge
force-pushed
the
codething/7144546f
branch
from
27d233d to
69dbc5d
Compare
- Guard `onSend` with an in-flight ref to block double submits - Set send phase before worktree prep and always clear in-flight state in `finally` - Sync thread branch/worktree path immediately and await setup script execution
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Send state set outside try-finally can lock permanently
- Moved
sendInFlightRef.current = trueandsetSendPhase(...)inside thetryblock so thefinallyblock always resets them, preventing permanent send-lock if any code before the originaltrythrows.
- Moved
- ✅ Fixed: Redundant
setSendPhase("preparing-worktree")call inside try block- Removed the redundant
setSendPhase("preparing-worktree")call inside theif (baseBranchForWorktree)block since the phase is already set to that value by the earlier conditional assignment.
- Removed the redundant
Or push these changes by commenting:
@cursor push ba4545ceb5
Preview (ba4545ceb5)
diff --git a/apps/web/src/components/ChatView.tsx b/apps/web/src/components/ChatView.tsx
--- a/apps/web/src/components/ChatView.tsx
+++ b/apps/web/src/components/ChatView.tsx
@@ -1416,44 +1416,43 @@
return;
}
- sendInFlightRef.current = true;
- setSendPhase(baseBranchForWorktree ? "preparing-worktree" : "sending-turn");
+ let attemptedTurnStart = false;
+ try {
+ sendInFlightRef.current = true;
+ setSendPhase(baseBranchForWorktree ? "preparing-worktree" : "sending-turn");
- const composerImagesSnapshot = [...composerImages];
- const messageIdForSend = newMessageId();
- const messageCreatedAt = new Date().toISOString();
- const optimisticAttachments = composerImagesSnapshot.map((image) => ({
- type: "image" as const,
- id: image.id,
- name: image.name,
- mimeType: image.mimeType,
- sizeBytes: image.sizeBytes,
- previewUrl: image.previewUrl,
- }));
- setOptimisticUserMessages((existing) => [
- ...existing,
- {
- id: messageIdForSend,
- role: "user",
- text: trimmed,
- ...(optimisticAttachments.length > 0 ? { attachments: optimisticAttachments } : {}),
- createdAt: messageCreatedAt,
- streaming: false,
- },
- ]);
+ const composerImagesSnapshot = [...composerImages];
+ const messageIdForSend = newMessageId();
+ const messageCreatedAt = new Date().toISOString();
+ const optimisticAttachments = composerImagesSnapshot.map((image) => ({
+ type: "image" as const,
+ id: image.id,
+ name: image.name,
+ mimeType: image.mimeType,
+ sizeBytes: image.sizeBytes,
+ previewUrl: image.previewUrl,
+ }));
+ setOptimisticUserMessages((existing) => [
+ ...existing,
+ {
+ id: messageIdForSend,
+ role: "user",
+ text: trimmed,
+ ...(optimisticAttachments.length > 0 ? { attachments: optimisticAttachments } : {}),
+ createdAt: messageCreatedAt,
+ streaming: false,
+ },
+ ]);
- setThreadError(threadIdForSend, null);
- promptRef.current = "";
- setPrompt("");
- setComposerImages([]);
- setComposerCursor(0);
- setComposerHighlightedItemId(null);
+ setThreadError(threadIdForSend, null);
+ promptRef.current = "";
+ setPrompt("");
+ setComposerImages([]);
+ setComposerCursor(0);
+ setComposerHighlightedItemId(null);
- let attemptedTurnStart = false;
- try {
// On first message: lock in branch + create worktree if needed.
if (baseBranchForWorktree) {
- setSendPhase("preparing-worktree");
const newBranch = `codething/${crypto.randomUUID().slice(0, 8)}`;
const result = await createWorktreeMutation.mutateAsync({
cwd: activeProject.cwd,| } | ||
|
|
||
| sendInFlightRef.current = true; | ||
| setSendPhase(baseBranchForWorktree ? "preparing-worktree" : "sending-turn"); |
Contributor
There was a problem hiding this comment.
Send state set outside try-finally can lock permanently
Medium Severity
sendInFlightRef.current = true and setSendPhase(...) are set at lines before the try block, but only reset inside the finally block. If any code between these assignments and the try (lines 1422–1452) throws unexpectedly, the finally never runs, leaving sendInFlightRef.current stuck at true and sendPhase stuck at a non-idle value, permanently blocking all future sends for the session.
Additional Locations (1)
| }); | ||
| const setupScript = setupProjectScript(activeProject.scripts); | ||
| if (setupScript) { | ||
| void runProjectScript(setupScript, { |
Contributor
There was a problem hiding this comment.
Redundant setSendPhase("preparing-worktree") call inside try block
Low Severity
setSendPhase("preparing-worktree") at line 1456 is redundant. Line 1420 already sets sendPhase to "preparing-worktree" when baseBranchForWorktree is truthy, which is exactly the condition guarding the block at line 1455. No code between these two calls changes the phase, so the second call is a no-op.
Additional Locations (1)
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
1 participant
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
worktreemodeshouldCreateWorktreeguard to simplify first-send worktree creation flow.plans/17-claude-code.mdTesting
worktreemode without a selected base branch showsSelect a base branch before sending in New worktree mode.and aborts sendNote
Medium Risk
Changes the core send/worktree-creation path in
ChatView, which can block sending and alters sequencing around worktree setup and terminal context. Risk is moderate due to potential regressions in first-message and double-submit behavior.Overview
Prevents the first message in New worktree mode from sending unless a base branch is selected, surfacing a clear
SET_ERRORmessage instead of implicitly falling back.Tightens the send lifecycle by adding a
sendInFlightRefguard, setting send phase earlier, immediately syncing local threadbranch/worktreePathafter worktree creation, and awaiting the project setup script so terminal/worktree context is consistent before starting the turn.Written by Cursor Bugbot for commit 7b0ae43. This will update automatically on new commits. Configure here.
Note
Require base branch selection before the first send in worktree mode and block concurrent sends in
ChatView.onSendin ChatView.tsxAdd
sendInFlightRefto gate concurrent sends, enforce base branch presence on the first worktree send with a thread error on missing branch, immediately dispatchSET_THREAD_BRANCHafter worktree creation, awaitrunProjectScript, and reset send phase and in-flight flag infinally.📍Where to Start
Start with the
onSendhandler in ChatView.tsx.Macroscope summarized 7b0ae43.
Summary by CodeRabbit