Group threads by worktree#3898
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
github-actions
Bot
added
size:XXL
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
ApprovabilityVerdict: Needs human review 1 blocking correctness issue found. This PR introduces significant new sidebar grouping and workspace selection features with substantial cross-cutting changes. Multiple unresolved review comments identify high/medium severity issues including race conditions, pagination bugs, and path normalization problems that could cause threads to execute in the wrong workspace. You can customize Macroscope's approvability policy. Learn more. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9b96569856
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return ( | ||
| serverConfigs.get(newThreadProjectRef.environmentId)?.settings ?? DEFAULT_SERVER_SETTINGS |
There was a problem hiding this comment.
When starting a thread from the sidebar or the command palette's "New thread in..." flow, startNewThreadInProjectFromContext is passed the selected projectRef, but the default mode/start-from-origin values here are resolved from newThreadProjectRef (the active/default project) instead. In a multi-environment setup where the selected project lives on an environment with different defaultThreadEnvMode or newWorktreesStartFromOrigin, the new draft is created with the wrong workspace mode for that target project.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in afe1ae1e: wire target-scoped defaults through sidebar, command-palette, and shortcut entry points.
| environmentId: projectRef.environmentId, | ||
| input: { cwd: project.workspaceRoot, limit: 100 }, | ||
| }); | ||
| if (result._tag === "Failure") return activeProjectMainCheckout; |
There was a problem hiding this comment.
If listRefs fails while resolving the main checkout for a project other than the active one, this falls back to activeProjectMainCheckout. In the "New thread in..." or sidebar project button flows, that can seed a draft for project B with project A's branch/worktree path, so the first send can run in the wrong checkout instead of the selected project's checkout.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Regression coverage added in fd4f7526 to ensure failed target discovery never reuses another project's checkout.
| const workspaceEnvironmentId = renderedThreads[0]?.environmentId ?? null; | ||
| const workspaceProjectId = renderedThreads[0]?.projectId ?? null; |
There was a problem hiding this comment.
With the default repository project grouping, renderedThreads can include threads from several project ids/checkouts in the same repo, but this derives a single workspace identity from only the first thread. Threads from the other grouped checkouts that have worktreePath: null will not have their project checkout path substituted, so they are split into project-id buckets and mislabeled instead of being grouped with their actual worktree.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Regression coverage added in 0e60ec29 for grouped rows containing multiple project checkouts.
| ? renderedThreads.map(renderThread) | ||
| : null} | ||
| {shouldShowThreadPanel && threadGroupingMode === "worktree" | ||
| ? renderedThreadGroups.flatMap((group) => [ |
There was a problem hiding this comment.
In worktree grouping mode, this renders each group's threads together, so a sorted list like A/worktree1, B/worktree2, C/worktree1 appears as A, C, B. The jump labels, next/previous navigation, and shift-selection ranges are still computed from the ungrouped thread order, so those shortcuts no longer match the visual row order whenever groups are interleaved.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in b4fa9a38: navigation, jump labels, and range selection now use grouped visual order.
| const defaultRef = activeProjectBranches.data.refs.find( | ||
| (ref) => !ref.isRemote && ref.isDefault, | ||
| ); | ||
| return defaultRef ? { branch: defaultRef.name, path: null } : undefined; |
There was a problem hiding this comment.
When a normal project checkout is currently on a feature branch while the default branch is main, this fallback returns main with path: null. Local-mode drafts do not run a checkout before the first send, so the agent still runs in the current cwd on the feature branch while the thread metadata/bootstrap records main, making sidebar/status data and later worktree bases point at the wrong branch.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in 69592d82: local drafts are seeded from the branch actually checked out in the project checkout.
| setPendingServerThreadEnvMode("worktree"); | ||
| setPendingServerThreadBranch(worktree.branch); | ||
| void updateThreadMetadata({ |
There was a problem hiding this comment.
For an empty server thread, selecting an existing worktree only stores the env mode and branch as pending while the worktreePath update is sent asynchronously. Until that metadata update round-trips, the send path still sees activeThread.worktreePath === null and treats worktree mode as a request to create a brand-new worktree, so pressing Enter immediately after selecting an existing worktree can run in a different checkout than the one selected.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in 9145cbb1: pending existing-worktree selections are honored immediately by the send path.
| projectRef.environmentId === newThreadProjectRef.environmentId && | ||
| projectRef.projectId === newThreadProjectRef.projectId | ||
| ) { | ||
| return activeProjectMainCheckout; |
There was a problem hiding this comment.
For the active project, this returns the memoized activeProjectMainCheckout even while the branch query is still loading or has failed, instead of running the resolver path below. Creating a default worktree draft in that window seeds branch: null, so the first send is blocked by "Select a base branch" despite the project having a current branch; local/main-checkout starts can similarly miss the discovered main checkout until the query happens to finish.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Regression coverage added in 67955356 for starting a draft while active ref discovery is still pending.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3abce121a5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const activeWorktreePath = | ||
| activeWorktreePathOverride !== undefined | ||
| ? activeWorktreePathOverride | ||
| : (serverThread?.worktreePath ?? draftThread?.worktreePath ?? null); |
There was a problem hiding this comment.
When an empty server thread has a pending existing-worktree selection, this override makes the workspace picker show the selected checkout immediately, but BranchToolbarBranchSelector still reads serverThread.worktreePath/draft state and sees null until updateMetadata returns. If the user changes the branch before that round-trip, the branch picker treats the thread as “New worktree” from the project cwd and can write worktreePath: null instead of switching/reusing the selected checkout.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in 5e3872d3: the branch selector now uses the pending worktree path immediately.
| function normalizeWorkspacePath(path: string): string { | ||
| return path.trim().replaceAll("\\", "/").replace(/\/+$/, ""); |
There was a problem hiding this comment.
For Windows-backed environments, the same checkout can be reported with different drive-letter/path casing by thread metadata vs. git worktree list; this normalizer only fixes separators/trailing slashes, so C:/repo and c:/repo become different sidebar group keys and fail the main-checkout comparison. In that case threads from the same worktree are split or the main checkout label is missed even though the paths refer to the same checkout.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in 8b6e2257: Windows drive and UNC workspace keys are normalized case-insensitively, with regression coverage.
| threadGroupingMode === "worktree" | ||
| ? projectMembers.map((member) => ({ | ||
| member, | ||
| atom: vcsEnvironment.listRefs({ | ||
| environmentId: member.environmentId, | ||
| input: { cwd: member.workspaceRoot, limit: 100 }, | ||
| }), |
There was a problem hiding this comment.
Because SidebarProjectThreadList is mounted for every project row, default worktree grouping now builds and reads a listRefs atom for every member project even when shouldShowThreadPanel is false and no grouped rows will be rendered. In sidebars with many collapsed projects or repository-grouped members, this issues unnecessary git ref/worktree scans for hidden rows; gate these targets on the panel actually being shown.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in 9d2ed11c: hidden project rows no longer subscribe to ref scans.
| const options = deriveWorkspaceOptions( | ||
| branchState.data?.refs ?? [], | ||
| activeProject.workspaceRoot, | ||
| branchState.data?.mainCheckoutPath, |
There was a problem hiding this comment.
When a repo has more than the first 100 refs returned by useBranches, any existing worktree whose branch sorts after that page never reaches deriveWorkspaceOptions, so it is absent from the workspace picker unless it is the active fallback. The same partial page can also fabricate the main checkout as HEAD when its path is known but its branch ref was not loaded, so users with large repos cannot reliably choose existing checkouts.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in 9644cd87: the workspace picker now loads all ref pages before deriving its complete worktree list.
| }) | ||
| .pipe( | ||
| Effect.matchEffect({ | ||
| onFailure: (error) => |
There was a problem hiding this comment.
When startFromOrigin is requested, this handler catches every resolveRemoteTrackingCommit failure and creates the worktree from the local branch instead. That fallback is appropriate when the remote-tracking ref is absent, but if the lookup fails for another reason after fetchRemote succeeds (for example a transient Git/config/refs error), the thread silently starts from a potentially stale local base despite the user asking to start from origin.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in 770796f4: only a specifically classified missing remote ref falls back locally; unexpected lookup failures propagate, with regression coverage.
There was a problem hiding this comment.
Effect service conventions: one issue found. See inline comment.
Posted via Macroscope — Effect Service Conventions
There was a problem hiding this comment.
Effect Service Conventions: one finding on the new resolveRemoteTrackingCommit failure handling. See inline comment.
Posted via Macroscope — Effect Service Conventions
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3ffbe06f2e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| isServerThread && | ||
| activeThread && | ||
| activeThread.messages.length === 0 && | ||
| activeThread.worktreePath === null && |
There was a problem hiding this comment.
Once an existing-worktree selection for an empty server thread round-trips, activeThread.worktreePath becomes non-null while the thread still has no messages/session. This condition then disables the override path, and BranchToolbar also treats the persisted worktree path as locked, so a user who picked the wrong existing worktree cannot switch back to Main checkout or another worktree before the first send without abandoning the thread.
Useful? React with 👍 / 👎.
| effectiveEnvModeOverride: envMode, | ||
| activeWorktreePathOverride: activeWorktreePath, | ||
| activeThreadBranchOverride: activeThreadBranch, | ||
| onActiveThreadBranchOverrideChange: setPendingServerThreadBranch, |
There was a problem hiding this comment.
For an empty server thread, choosing a branch that is already checked out in another worktree through the branch dropdown updates server metadata asynchronously with that worktree path, but the parent only records the pending branch override here. If the user presses Enter before the metadata update round-trips, activeWorktreePath is still null, so the send path treats the thread as the main checkout instead of the selected existing worktree.
Useful? React with 👍 / 👎.
| } | ||
|
|
||
| function looksLikeWindowsPath(path: string): boolean { | ||
| return /^[a-zA-Z]:\//.test(path); |
There was a problem hiding this comment.
Fresh evidence beyond the sidebar-path finding: the branch toolbar uses this separate normalizer, and it only lowercases drive-letter Windows paths. In Windows-backed environments where the same UNC checkout is reported as \\Server\Share\Repo by thread metadata and //server/share/repo by Git, workspacePathsEqual returns false, so the workspace picker can fail to recognize the selected main/existing checkout and show duplicate or wrong workspace choices.
Useful? React with 👍 / 👎.
| new Promise<undefined>((resolve) => { | ||
| timeout = setTimeout(resolve, MAIN_CHECKOUT_RESOLUTION_TIMEOUT_MS); | ||
| }), | ||
| ])) ?? |
There was a problem hiding this comment.
Late race rejection unhandled
Low Severity
After the 500ms timeout wins Promise.race in resolveMainCheckout, a later rejection from resolveDefaultMainCheckout is no longer observed by the surrounding try/catch, so it can surface as an unhandled promise rejection instead of falling back to defaultMainCheckout.
Reviewed by Cursor Bugbot for commit 3f42da8. Configure here.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3f42da85e8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| loadRefs: async () => { | ||
| const result = await listRefs({ | ||
| environmentId: projectRef.environmentId, | ||
| input: { cwd: project.workspaceRoot, limit: 100 }, |
There was a problem hiding this comment.
When the selected project is a linked checkout and its real main checkout is on a non-default branch that falls outside the first 100 refs, this resolver never fetches the page containing the ref whose worktreePath matches mainCheckoutPath. resolveMainCheckoutTarget then falls back to branch: "HEAD" for that main checkout, so new local/default drafts are created with bogus branch metadata while running in the actual checkout; later sidebar/status data and new-worktree bases can point at HEAD instead of the checked-out branch. Load all ref pages here before resolving the main checkout, like the toolbar path does.
Useful? React with 👍 / 👎.
| await context.handleNewThread(projectRef, { | ||
| branch: mainCheckout?.branch ?? null, | ||
| worktreePath: threadEnvMode === "local" ? (mainCheckout?.path ?? null) : null, | ||
| envMode: threadEnvMode, |
There was a problem hiding this comment.
When this path starts a draft for a selected grouped project, these options are lost if useNewThreadHandler reuses an existing stored draft for the same logical repo but remaps it to a different member project/environment: that branch calls setDraftThreadContext first, then setLogicalProjectDraftThreadId(..., { threadId }), and the remap clears branch/worktree/env on project changes. In that scenario, clicking New thread for project B while an unused draft from project A exists silently drops the target main-checkout/default-mode values and creates a local/null draft for B instead; forward the same options through the remap call as the active-draft path does.
Useful? React with 👍 / 👎.
| { key: "mod+0", command: "preview.resetZoom", when: "previewFocus" }, | ||
| { key: "mod+k", command: "commandPalette.toggle", when: "!terminalFocus" }, | ||
| { key: "mod+n", command: "chat.new", when: "!terminalFocus" }, | ||
| { key: "mod+t", command: "chat.newSameWorktree", when: "!terminalFocus" }, |
There was a problem hiding this comment.
In the browser build, mod+t is the standard new-tab shortcut (Cmd+T/Ctrl+T), but this default binding now resolves to chat.newSameWorktree; ChatRouteGlobalShortcuts then calls preventDefault() for that command in chat views whenever the terminal is not focused. As a result, users typing in the composer or navigating the chat cannot open a browser tab with the normal shortcut and instead create a T3 thread, so choose a non-reserved default or gate this binding to the desktop app.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Apply project defaults when opening empty projects
When the project-search action falls through to creating a draft because the selected project has no latest thread, this still calls the raw handleNewThread instead of startNewThreadInProjectFromContext. That bypasses the newly added target defaults/main-checkout resolver used by the new-thread-in action below, so opening an empty project from the palette can create a draft with branch/worktreePath left null; in environments defaulting to New worktree the first send is blocked for a missing base branch, and linked-checkout projects defaulting to Main checkout won't switch to the actual main checkout.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| : refs.find( | ||
| (ref) => | ||
| ref.isDefault && | ||
| ref.worktreePath !== null && | ||
| !workspacePathsEqual(ref.worktreePath, projectWorkspaceRoot), |
There was a problem hiding this comment.
When mainCheckoutPath is unavailable, for example from an older environment server that still returns per-ref worktreePath, this promotes whichever default-branch worktree is not the registered project to “Main checkout”. In a repo where main is checked out in a secondary worktree while the actual primary checkout is elsewhere or on another branch, the workspace picker and local-draft seeding will use that secondary worktree path/branch, so the agent can run in the wrong checkout.
Useful? React with 👍 / 👎.
| (await Promise.race([ | ||
| context.resolveDefaultMainCheckout(projectRef), | ||
| new Promise<undefined>((resolve) => { | ||
| timeout = setTimeout(resolve, MAIN_CHECKOUT_RESOLUTION_TIMEOUT_MS); | ||
| }), |
There was a problem hiding this comment.
Fresh evidence beyond the loading-window finding: this races main-checkout discovery against a fixed 500 ms timer, and the production callers do not provide defaultMainCheckout. In remote environments or large repos where the first listRefs call takes just over 500 ms, local/default thread creation falls back to null branch/path even though discovery would have resolved shortly after, so linked-checkout drafts run in the registered checkout instead of the main checkout and default worktree drafts can still be created without a base branch.
Useful? React with 👍 / 👎.
Drive-letter paths can differ only by casing across refs and thread metadata, so workspace matching must ignore case for those paths. Co-authored-by: Cursor <cursoragent@cursor.com>
withActiveWorkspaceFallback should run even when listRefs returns data, since the first page can omit the thread's current worktree. Co-authored-by: Cursor <cursoragent@cursor.com>
Pass the effective checkout path into BranchToolbar so empty server threads show the selected worktree before metadata round-trips. Co-authored-by: Cursor <cursoragent@cursor.com>
Sidebar workspace identity should only treat an explicit main checkout path as main, not fall back to the project workspace root. Co-authored-by: Cursor <cursoragent@cursor.com>
98ba360 to
4ade26b
Compare
| const options = data | ||
| ? deriveWorkspaceOptions(data.refs, member.workspaceRoot, data.mainCheckoutPath) | ||
| : null; | ||
| const normalizedProjectCwd = member.workspaceRoot.replaceAll("\\", "/").replace(/\/+$/, ""); |
There was a problem hiding this comment.
🟡 Medium components/Sidebar.tsx:1022
projectCheckoutRef compares normalized paths case-sensitively, so on Windows a ref whose worktreePath differs only in drive or directory casing from member.workspaceRoot (e.g. C:\Repo vs c:\repo) is never matched. The project-checkout group then falls back to the thread branch or "Project checkout" instead of the actual ref name. The groupSidebarThreadsByWorktree logic already uses normalizeWorkspacePath for case-insensitive comparison — use the same helper here instead of the inline replaceAll/replace normalization.
Also found in 1 other location(s)
apps/web/src/components/BranchToolbar.logic.ts:42
workspacePathKeyonly recognizes drive-letter Windows paths, so normalized UNC paths such as//server/share/Repoare compared case-sensitively. Windows/WSL UNC paths can arrive with casing differences (for example the project root versus Git ref metadata), causing the same checkout to be treated as distinct: the picker can add duplicate fallback worktrees, fail to select the active checkout, or resolve the wrong Main checkout state. Detect normalized paths beginning with//as Windows paths too.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/Sidebar.tsx around line 1022:
`projectCheckoutRef` compares normalized paths case-sensitively, so on Windows a ref whose `worktreePath` differs only in drive or directory casing from `member.workspaceRoot` (e.g. `C:\Repo` vs `c:\repo`) is never matched. The project-checkout group then falls back to the thread branch or `"Project checkout"` instead of the actual ref name. The `groupSidebarThreadsByWorktree` logic already uses `normalizeWorkspacePath` for case-insensitive comparison — use the same helper here instead of the inline `replaceAll`/`replace` normalization.
Also found in 1 other location(s):
- apps/web/src/components/BranchToolbar.logic.ts:42 -- `workspacePathKey` only recognizes drive-letter Windows paths, so normalized UNC paths such as `//server/share/Repo` are compared case-sensitively. Windows/WSL UNC paths can arrive with casing differences (for example the project root versus Git ref metadata), causing the same checkout to be treated as distinct: the picker can add duplicate fallback worktrees, fail to select the active checkout, or resolve the wrong Main checkout state. Detect normalized paths beginning with `//` as Windows paths too.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 3 potential issues.
There are 4 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 4ade26b. Configure here.
| branch: worktree.branch, | ||
| worktreePath: worktree.path, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Workspace metadata races picker
High Severity
Selecting an existing worktree on an empty server thread immediately calls updateThreadMetadata. This prematurely updates the thread's worktreePath, causing canOverrideServerThreadEnvMode to become false. As a result, pending environment overrides are cleared, the workspace picker locks, and the first turn may execute in the project checkout instead of the selected worktree.
Reviewed by Cursor Bugbot for commit 4ade26b. Configure here.
| atom: vcsEnvironment.listRefs({ | ||
| environmentId: member.environmentId, | ||
| input: { cwd: member.workspaceRoot, limit: 100 }, | ||
| }), |
There was a problem hiding this comment.
Sidebar refs stop at page
Medium Severity
The sidebar's worktree grouping and new thread main-checkout resolution (via Cmd+N) use a single listRefs call with a 100-ref limit. This can lead to incomplete worktree identities, mislabeled groups, and incorrect default branches for new threads in large repositories, where other UI components correctly paginate all refs.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 4ade26b. Configure here.
| const normalized = path.trim().replaceAll("\\", "/").replace(/\/+$/, ""); | ||
| return /^[A-Za-z]:\//.test(normalized) || normalized.startsWith("//") | ||
| ? normalized.toLowerCase() | ||
| : normalized; |
There was a problem hiding this comment.
Duplicate path normalization logic
Low Severity
Worktree path normalization for grouping lives in a private normalizeWorkspacePath in Sidebar.logic.ts, while the workspace picker uses a separate normalizeWorkspacePath and workspacePathsEqual in BranchToolbar.logic.ts. The sidebar version lowercases UNC (//) paths; the toolbar version only lowercases drive-letter paths, so the same checkout can be treated as different workspaces in the sidebar versus the picker on UNC paths.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 4ade26b. Configure here.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4ade26bc2b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (mainCheckout && value === `main:${mainCheckout.path}`) { | ||
| onExistingWorktreeChange(mainCheckout); |
There was a problem hiding this comment.
When the selected project is a linked checkout and the real main checkout is currently on a non-default branch, routing the Main checkout option through the existing-worktree handler records only activeWorktreePath and loses that it is the main checkout. If the user then chooses the default branch in the branch picker, resolveBranchSelectionTarget treats activeWorktreePath + refName.isDefault as a request to clear worktreePath, so the checkout/sent thread moves to the registered project checkout instead of switching the real main checkout. Preserve a distinct main-checkout selection state or pass that state into branch selection so default-branch changes still target the main checkout path.
Useful? React with 👍 / 👎.
| ...(draftThread?.worktreePath | ||
| ? mode === "local" | ||
| ? { branch: null, worktreePath: null } | ||
| : { worktreePath: null } |
There was a problem hiding this comment.
When a draft is currently pointed at an existing worktree, choosing Main checkout in a project without a separate mainCheckoutPath reaches this mode === "local" branch and clears both worktreePath and branch. The UI can still display the project checkout's current branch from git status, but the draft metadata remains null, so the first send creates the server thread with branch: null while running in the project checkout, breaking sidebar/PR status and future worktree bases for that thread.
Useful? React with 👍 / 👎.
| return false; | ||
| } | ||
|
|
||
| await context.handleNewThread(projectRef, buildContextualThreadOptions(context)); |
There was a problem hiding this comment.
When an empty server thread has just selected an existing worktree, ChatView only has that path in its pending local override until the metadata update round-trips; this global chat.newSameWorktree path still copies only the persisted activeThread.worktreePath from the shared context. Pressing the new same-worktree shortcut in that window therefore creates the duplicate draft with worktreePath: null/local checkout instead of the selected checkout, so pass the effective pending branch/worktree into this action or wait for metadata before duplicating.
Useful? React with 👍 / 👎.
1 participant
Solves #3697
Reasoning/Justification
https://cap.so/s/dvaz19nmrwjh2ja
Verification
Screen.Recording.2026-07-11.at.5.22.24.PM.mov
Summary
Why
Thread creation previously conflated the registered project checkout, the true main checkout, and the active worktree. That made new drafts inherit whichever worktree happened to be active, produced misleading picker selections, and could make Cmd+N silently do nothing while resolving the main checkout.
The new flow resolves the primary checkout from Git worktree/ref data, keeps Cmd+N and Cmd+T semantics separate, and ensures checkout discovery failures cannot swallow thread creation.
User impact
Users can keep related threads grouped by worktree, deliberately create a thread in an existing worktree, and rely on Cmd+N to honor the configured Main checkout/New worktree default. Cmd+T always retains the active worktree.
Validation
vp checkvp run typecheckNote
Group sidebar threads by worktree with a new
mod+tsame-worktree shortcutsidebarThreadGroupingModeclient setting ('worktree'default,'separate'opt-out) that groups sidebar threads under labeled worktree headers, with thread counts and dynamic workspace identity resolution from VCS refs.groupSidebarThreadsByWorktreeandorderSidebarThreadsByWorktreein Sidebar.logic.ts to compute groups; handles Windows path case-insensitivity vianormalizeWorkspacePath.deriveWorkspaceOptions,resolveWorkspaceSelection, and related helpers in BranchToolbar.logic.ts; the env-mode selector and mobile selector now list Main checkout and existing worktrees as discrete choices.chat.newSameWorktreecommand (default keybindingmod+t) viastartNewThreadInSameWorktreeFromContext; new-thread creation is centralized throughstartNewThreadInProjectFromContextwith per-project env-mode and start-from-origin defaults.mainCheckoutPathtoVcsListRefsResultand aRemoteTrackingRefNotFoundErrortoGitVcsDriver; when starting from origin with a missing remote tracking branch, the server now falls back to the local base branch instead of failing.resolveEnvModeLabelnow returns'Main checkout'instead of'Local';resolveSidebarNewThreadSeedContextis removed and callers must use the new handler pipeline.Macroscope summarized 4ade26b.
Note
Medium Risk
Touches git bootstrap, worktree metadata, and default sidebar grouping behavior; incorrect checkout resolution could start threads in the wrong path, though fallbacks and tests limit blast radius.
Overview
Adds worktree-aware workspaces end to end: Git ref listing now exposes
mainCheckoutPath, the branch toolbar can pick Main checkout, New worktree, or an existing worktree, and thread creation separates Cmd+N (project default mode + resolved main checkout) from Cmd+T (chat.newSameWorktree, same checkout as the active thread).The sidebar gains a Group threads client setting (
worktreedefault,separatefor the old flat list). In worktree mode, threads render under checkout labels using VCS ref data and normalized paths (including Windows case rules).Bootstrap worktree prep no longer hard-fails when
startFromOrigincannot resolve a remote-tracking branch: missing refs yieldRemoteTrackingRefNotFoundErrorand the server logs and falls back to the local base branch; other git errors still abort.Copy and settings rename Local / Current checkout to Main checkout where relevant; pending worktree selection is honored before server metadata round-trips on empty server threads.
Reviewed by Cursor Bugbot for commit 4ade26b. Bugbot is set up for automated code reviews on this repo. Configure here.