feat(desktop): revamp New Workspace modal + worktree import settings#2153
Merged
saddlepaddle merged 10 commits intomainfrom Mar 7, 2026
Merged
feat(desktop): revamp New Workspace modal + worktree import settings#2153saddlepaddle merged 10 commits intomainfrom
saddlepaddle merged 10 commits intomainfrom
Conversation
…project settings - Wire PullRequestsGroup to match GitHub repos by owner+name instead of cloud project FK - Wire BranchesGroup to real tRPC data with open/create/duplicate actions - Add workspace detection across all tabs (Open vs Create CTAs) - Add platform-aware keyboard shortcuts via useHotkeysStore - Add worktree import section to project general settings (dropdown + import all) - Delete old NewWorkspaceModal and orphaned useOpenWorktree hook
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughAdds organizationId to GitHub repositories and pull requests (DB, migrations, schema, relations), persists it in API syncs/webhooks, exposes org-scoped collections, extends electric where logic, and refactors NewWorkspaceModal into a CommandDialog with new tab groups and related UI/utilities. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 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)
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 |
# Conflicts: # apps/desktop/src/renderer/react-query/workspaces/index.ts
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
9 issues found across 41 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/api/src/app/api/github/sync/route.ts">
<violation number="1" location="apps/api/src/app/api/github/sync/route.ts:73">
P2: `organizationId` is missing from the `githubPullRequests` `onConflictDoUpdate` set. For `githubRepositories`, you correctly added `organizationId` to both the insert and the upsert-on-conflict. The same should be done here so that existing PRs get backfilled on re-sync.</violation>
</file>
<file name="apps/desktop/src/renderer/components/NewWorkspaceModal/components/IssuesGroup/IssuesGroup.tsx">
<violation number="1" location="apps/desktop/src/renderer/components/NewWorkspaceModal/components/IssuesGroup/IssuesGroup.tsx:120">
P1: The `onSelect` handler always creates a new workspace, but the UI label shows "Open ↵" for issues that already have one. Clicking an issue with an existing workspace will attempt a duplicate create instead of navigating to it. The handler needs to branch on `openBranches.has(task.slug.toLowerCase())` and navigate to the existing workspace in the "Open" case.</violation>
</file>
<file name="apps/desktop/src/renderer/components/NewWorkspaceModal/components/PullRequestsGroup/PullRequestsGroup.tsx">
<violation number="1" location="apps/desktop/src/renderer/components/NewWorkspaceModal/components/PullRequestsGroup/PullRequestsGroup.tsx:133">
P1: The `onSelect` handler always creates a new workspace from the PR, but the UI labels items with existing workspaces as "Open ↵" (and shows a navigate icon). When `openBranches.has(pr.headBranch)` is true, the handler should navigate to the existing workspace instead of calling `createFromPr.mutateAsync`.</violation>
</file>
<file name="apps/api/src/app/api/github/jobs/initial-sync/route.ts">
<violation number="1" location="apps/api/src/app/api/github/jobs/initial-sync/route.ts:101">
P1: `organizationId` is added to the PR insert values but missing from the `onConflictDoUpdate` set. If the PR row already exists, the `organizationId` won't be updated on conflict — unlike the `githubRepositories` upsert which correctly includes it in both places.</violation>
</file>
<file name="apps/api/src/app/api/github/webhook/webhooks.ts">
<violation number="1" location="apps/api/src/app/api/github/webhook/webhooks.ts:162">
P1: `organizationId` is set on insert but missing from the `onConflictDoUpdate` set. Existing PRs that were created before this change will never get `organizationId` backfilled through webhook updates, leaving them with a NULL value. Add `organizationId` to the conflict-update set so the field is populated for all PRs.</violation>
</file>
<file name="apps/desktop/src/renderer/components/NewWorkspaceModal/components/ProjectSelector/ProjectSelector.tsx">
<violation number="1" location="apps/desktop/src/renderer/components/NewWorkspaceModal/components/ProjectSelector/ProjectSelector.tsx:78">
P2: `CommandItem` uses `project.name` as `value`, which cmdk treats as the item identity. If two projects share the same name, only one will be keyboard-navigable and selectable. Use a unique value (e.g. the project ID) and add `keywords` for search filtering.</violation>
</file>
<file name="apps/desktop/src/renderer/routes/_authenticated/settings/project/$projectId/components/ProjectSettings/ProjectSettings.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/settings/project/$projectId/components/ProjectSettings/ProjectSettings.tsx:472">
P2: Resetting `selectedWorktreePath` to `null` immediately switches the UI to the "Import all" button, which isn't disabled during a single-worktree import (`openExternalWorktree.isPending` isn't checked there). This lets the user trigger "Import all" while an individual import is still in progress. Either defer the reset until the import resolves, or also disable "Import all" while `openExternalWorktree.isPending`.</violation>
</file>
<file name="apps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/PromptGroup.tsx">
<violation number="1" location="apps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/PromptGroup.tsx:172">
P1: Keyboard shortcut is broken on Windows/Linux. `e.metaKey` is the ⌘ key on Mac but the Windows key (not Ctrl) on other platforms, so pressing Ctrl+Enter won't trigger `handleCreate` despite the UI label showing "Ctrl ↵".</violation>
</file>
<file name="apps/desktop/src/renderer/components/NewWorkspaceModal/components/BranchesGroup/BranchesGroup.tsx">
<violation number="1" location="apps/desktop/src/renderer/components/NewWorkspaceModal/components/BranchesGroup/BranchesGroup.tsx:141">
P2: The `Duplicate branch {modKey}↵` label advertises a keyboard shortcut that isn't wired up. `onSelect` fires on any Enter press and always calls `handleOpen` for branches with existing workspaces — there's no `onKeyDown` handler to intercept Cmd/Ctrl+Enter and route to `handleCreate`. The shortcut is only clickable via the button. Consider adding a keyboard handler similar to what `PromptGroup` does.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| {recentProjects.map((project) => ( | ||
| <CommandItem | ||
| key={project.id} | ||
| value={project.name} |
Contributor
There was a problem hiding this comment.
P2: CommandItem uses project.name as value, which cmdk treats as the item identity. If two projects share the same name, only one will be keyboard-navigable and selectable. Use a unique value (e.g. the project ID) and add keywords for search filtering.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/components/NewWorkspaceModal/components/ProjectSelector/ProjectSelector.tsx, line 78:
<comment>`CommandItem` uses `project.name` as `value`, which cmdk treats as the item identity. If two projects share the same name, only one will be keyboard-navigable and selectable. Use a unique value (e.g. the project ID) and add `keywords` for search filtering.</comment>
<file context>
@@ -29,46 +38,90 @@ export function ProjectSelector({
+ {recentProjects.map((project) => (
+ <CommandItem
+ key={project.id}
+ value={project.name}
+ onSelect={() => {
+ onSelectProject(project.id);
</file context>
Suggested change
| value={project.name} | |
| value={project.id} | |
| keywords={[project.name]} |
| <Button | ||
| size="sm" | ||
| className="w-22" | ||
| disabled={importAllWorktrees.isPending} |
Contributor
There was a problem hiding this comment.
P2: Resetting selectedWorktreePath to null immediately switches the UI to the "Import all" button, which isn't disabled during a single-worktree import (openExternalWorktree.isPending isn't checked there). This lets the user trigger "Import all" while an individual import is still in progress. Either defer the reset until the import resolves, or also disable "Import all" while openExternalWorktree.isPending.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/routes/_authenticated/settings/project/$projectId/components/ProjectSettings/ProjectSettings.tsx, line 472:
<comment>Resetting `selectedWorktreePath` to `null` immediately switches the UI to the "Import all" button, which isn't disabled during a single-worktree import (`openExternalWorktree.isPending` isn't checked there). This lets the user trigger "Import all" while an individual import is still in progress. Either defer the reset until the import resolves, or also disable "Import all" while `openExternalWorktree.isPending`.</comment>
<file context>
@@ -358,6 +415,93 @@ export function ProjectSettings({ projectId }: ProjectSettingsProps) {
+ <Button
+ size="sm"
+ className="w-22"
+ disabled={importAllWorktrees.isPending}
+ >
+ {importAllWorktrees.isPending
</file context>
Suggested change
| disabled={importAllWorktrees.isPending} | |
| disabled={importAllWorktrees.isPending || openExternalWorktree.isPending} |
| handleCreate(branch.name); | ||
| }} | ||
| > | ||
| Duplicate branch {modKey}↵ |
Contributor
There was a problem hiding this comment.
P2: The Duplicate branch {modKey}↵ label advertises a keyboard shortcut that isn't wired up. onSelect fires on any Enter press and always calls handleOpen for branches with existing workspaces — there's no onKeyDown handler to intercept Cmd/Ctrl+Enter and route to handleCreate. The shortcut is only clickable via the button. Consider adding a keyboard handler similar to what PromptGroup does.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/components/NewWorkspaceModal/components/BranchesGroup/BranchesGroup.tsx, line 141:
<comment>The `Duplicate branch {modKey}↵` label advertises a keyboard shortcut that isn't wired up. `onSelect` fires on any Enter press and always calls `handleOpen` for branches with existing workspaces — there's no `onKeyDown` handler to intercept Cmd/Ctrl+Enter and route to `handleCreate`. The shortcut is only clickable via the button. Consider adding a keyboard handler similar to what `PromptGroup` does.</comment>
<file context>
@@ -0,0 +1,161 @@
+ handleCreate(branch.name);
+ }}
+ >
+ Duplicate branch {modKey}↵
+ </Button>
+ </span>
</file context>
Contributor
🚀 Preview Deployment🔗 Preview Links
Preview updates automatically with new commits |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
apps/api/src/app/api/github/jobs/initial-sync/route.ts (2)
97-120:⚠️ Potential issue | 🟠 MajorUpdate
installationIdon repository conflict upserts.Lines 109-119 only refresh the denormalized org fields. If a repo is transferred or reconnected under a different installation, the row keeps the old
installation_id, so a laterinstallation.deletedevent can cascade-delete a repository that now belongs to another org.Suggested fix
.onConflictDoUpdate({ target: [githubRepositories.repoId], set: { + installationId: installationDbId, organizationId, owner: repo.owner.login, name: repo.name, fullName: repo.full_name, defaultBranch: repo.default_branch ?? "main",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/app/api/github/jobs/initial-sync/route.ts` around lines 97 - 120, The upsert into githubRepositories currently updates denormalized org fields on conflict but omits updating installationId, which can leave a stale installation_id after repo transfers; update the onConflictDoUpdate.set payload in the insert call (the block that references githubRepositories and .onConflictDoUpdate) to include installationId: installationDbId so the stored installation_id is refreshed on repo conflict/upsert.
188-229:⚠️ Potential issue | 🟠 MajorRefresh
organizationIdwhen a PR row already exists.Lines 218-229 update the PR payload but keep the previous
organization_id. If the repository has been re-associated to a different org, existing PR rows stay visible under the old org filter and disappear from the new one.Suggested fix
.onConflictDoUpdate({ target: [ githubPullRequests.repositoryId, githubPullRequests.prNumber, ], set: { + organizationId, headSha: pr.head.sha, title: pr.title, state: pr.state, isDraft: pr.draft ?? false,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/app/api/github/jobs/initial-sync/route.ts` around lines 188 - 229, When upserting into githubPullRequests, the onConflictDoUpdate block doesn't update the organizationId so existing PR rows retain the old org; update the conflict set to include githubPullRequests.organizationId (assign it the current organizationId variable) alongside the other fields (e.g., in the onConflictDoUpdate.set add organizationId: organizationId) so rows get reassociated when a repository moves orgs.apps/api/src/app/api/github/sync/route.ts (2)
69-92:⚠️ Potential issue | 🟠 MajorCarry
installationIdforward on repository upserts.Lines 81-91 update the repo metadata but not
installationId. If the same GitHub repo is later synced under a different installation, the row stays attached to the old installation and can be deleted by the wrong cascade path.Suggested fix
.onConflictDoUpdate({ target: [githubRepositories.repoId], set: { + installationId: installation.id, organizationId, owner: repo.owner.login, name: repo.name, fullName: repo.full_name, defaultBranch: repo.default_branch ?? "main",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/app/api/github/sync/route.ts` around lines 69 - 92, The upsert for githubRepositories omits updating installationId on conflict, so retain the installationId from the current request when a repo row already exists; modify the onConflictDoUpdate set in the db.insert(githubRepositories).values(...) call to include installationId (set to installation.id) alongside the existing fields (owner, name, fullName, defaultBranch, isPrivate, updatedAt) so the row's installation is correctly carried forward.
160-201:⚠️ Potential issue | 🟠 MajorUpdate PR org scope on conflict too.
Lines 190-200 leave
organizationIduntouched for existing rows. That makes org-scoped replication inconsistent whenever a repository record has been moved to a different organization.Suggested fix
.onConflictDoUpdate({ target: [ githubPullRequests.repositoryId, githubPullRequests.prNumber, ], set: { + organizationId, headSha: pr.head.sha, title: pr.title, state: pr.state, isDraft: pr.draft ?? false,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/app/api/github/sync/route.ts` around lines 160 - 201, The conflict update for the githubPullRequests upsert doesn’t update organizationId, so when a repository moves orgs the PR row keeps the old org; inside the .onConflictDoUpdate({ target: [...], set: { ... } }) for the insert into githubPullRequests (the block using db.insert(githubPullRequests).values(...).onConflictDoUpdate), add organizationId: organizationId to the set object so the existing row’s organizationId is updated on conflict (keep the other fields like lastSyncedAt/updatedAt as-is).apps/api/src/app/api/github/webhook/webhooks.ts (2)
158-197:⚠️ Potential issue | 🟠 MajorRe-scope existing PR rows when the repo's org changes.
The insert path sets
organizationId, but the conflict update path does not. Existing PR rows can therefore remain pinned to the previous organization and replicate to the wrong workspace.Suggested fix
.onConflictDoUpdate({ target: [githubPullRequests.repositoryId, githubPullRequests.prNumber], set: { + organizationId: repo.organizationId, headBranch: pr.head.ref, headSha: pr.head.sha, baseBranch: pr.base.ref, title: pr.title,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/app/api/github/webhook/webhooks.ts` around lines 158 - 197, The conflict update currently omits organizationId so when a repository's organization changes existing githubPullRequests rows stay pinned to the old organization; update the onConflictDoUpdate set to include organizationId: repo.organizationId (alongside lastSyncedAt/updatedAt etc.) so the existing row is re-scoped to the correct organizationId when the insert conflicts on githubPullRequests.repositoryId and githubPullRequests.prNumber.
89-101:⚠️ Potential issue | 🟠 MajorDon't keep stale repo ownership on webhook re-adds.
Using
onConflictDoNothing()here means an existing repo row never gets itsinstallationIdororganizationIdcorrected. If the repo was transferred or reinstalled under a new installation, the old row survives with the wrong tenant mapping.Suggested fix
await db .insert(githubRepositories) .values({ installationId: installation.id, organizationId: installation.organizationId, repoId: String(repo.id), owner: owner ?? "", name: name ?? repo.name, fullName: repo.full_name, defaultBranch: "main", isPrivate: repo.private, }) - .onConflictDoNothing(); + .onConflictDoUpdate({ + target: [githubRepositories.repoId], + set: { + installationId: installation.id, + organizationId: installation.organizationId, + owner: owner ?? "", + name: name ?? repo.name, + fullName: repo.full_name, + isPrivate: repo.private, + updatedAt: new Date(), + }, + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/app/api/github/webhook/webhooks.ts` around lines 89 - 101, The insert into githubRepositories currently uses onConflictDoNothing(), which leaves existing rows with stale installationId/organizationId when a repo is transferred or reinstalled; change the DB operation to an upsert/update-on-conflict instead of no-op: locate the db.insert(githubRepositories).values(...) call and replace the onConflictDoNothing() behavior with an onConflict(...) or equivalent upsert that updates at minimum installationId and organizationId (and optionally owner, name, fullName, defaultBranch, isPrivate) for the conflicting repoId/fullName key so the tenant mapping is corrected on webhook re-adds.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/desktop/src/renderer/components/NewWorkspaceModal/components/IssuesGroup/IssuesGroup.tsx`:
- Around line 70-78: The openBranches set is built from raw workspace branch
names (openBranches) but lookups use task.slug.toLowerCase(), causing
case-mismatch misses; fix by normalizing branch names when building the set (map
workspace.branch to .toLowerCase()) and ensure comparisons use the same
normalized form (compare against task.slug.toLowerCase()), updating the Set
construction that uses allWorkspaces and projectId so all entries are
lowercased.
In
`@apps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/PromptGroup.tsx`:
- Around line 171-176: The onKeyDown handler in PromptGroup currently only
checks e.metaKey so the Enter shortcut works only on macOS; update the handler
to detect platform and accept Ctrl on Windows/Linux and Meta on macOS (e.g.,
derive isMac from platform or navigator.platform), then change the condition in
the onKeyDown callback (the function that currently calls handleCreate on
Enter+meta) to use (isMac ? e.metaKey : e.ctrlKey) so users on all platforms can
trigger handleCreate via the displayed shortcut.
In
`@apps/desktop/src/renderer/components/NewWorkspaceModal/components/PullRequestsGroup/PullRequestsGroup.tsx`:
- Around line 139-151: The success toast always says "Workspace created" even
though useCreateFromPr (and its createFromPr.mutateAsync call) returns
wasExisting to indicate reopened (opened) workspaces; update the toast.promise
success handler to inspect the resolved result from createFromPr.mutateAsync and
return "Workspace opened" when result.wasExisting is true, otherwise "Workspace
created" (e.g., set the success option to a function that checks
res?.wasExisting and returns the appropriate string).
- Around line 69-80: The client is comparing PR head keys using pr.headBranch
against the Set built in openBranches (from workspace.branch) but fork PRs use
the server-side getPrLocalBranchName format ("forkOwner/headRefName"); update
the client to compute the same local-branch key before lookup: in
PullRequestsGroup.tsx where openPrs and openBranches are used, replace direct
uses of pr.headBranch in checks with a reconstructed key (e.g., combine the PR's
head repo owner/login with pr.headBranch the same way getPrLocalBranchName
does), or import/replicate getPrLocalBranchName logic so openBranches.has(...)
and icon/button logic use the matching key; ensure this change is applied for
both existence checks (the spots that currently call
openBranches.has(pr.headBranch) and any similar comparisons).
In
`@apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx`:
- Around line 46-48: The selected project resolution currently only looks up
recentProjects (variable selectedProject) which misses cases where
selectedProjectId comes from usePreSelectedProjectId(); update the logic that
builds selectedProjectName, githubOwner and repoName (the code that uses
selectedProject found from recentProjects and the block around where those props
are set) to first try recentProjects.find(p => p.id === selectedProjectId) and
if not found fall back to resolving the project via the pre-selected source
(usePreSelectedProjectId() / any preSelectedProject record) or another project
lookup (e.g., global projects store or an API call) so the component passes
non-null selectedProjectName, githubOwner and repoName to PullRequestsGroup;
ensure you reference and update the variables selectedProject,
selectedProjectId, usePreSelectedProjectId(), selectedProjectName, githubOwner,
and repoName in the NewWorkspaceModal component.
- Around line 35-44: The effect that syncs the pre-selected project on modal
open currently only depends on isOpen, so if recentProjects arrive after open
the default selection never runs; update the useEffect (the effect that
references isOpen, preSelectedProjectId, recentProjects, selectedProjectId, and
setSelectedProjectId) to also depend on recentProjects (and preSelectedProjectId
if needed) and keep the existing guards (isOpen check and only set when
selectedProjectId is falsy) so that when recents load after the modal opens it
will set recentProjects[0].id as the selected project without overriding an
already-made user selection.
In
`@apps/desktop/src/renderer/routes/_authenticated/settings/project/`$projectId/components/ProjectSettings/ProjectSettings.tsx:
- Around line 228-242: The single-import flow clears the Select and only checks
openExternalWorktree's pending state, letting the UI flip to the bulk-import
control while a mutation is still in flight; fix by deriving a shared
"isImporting" boolean from both import mutations (e.g., combine
openExternalWorktree.isLoading || <bulkImportMutation>.isLoading), use that
shared flag to disable the Select and both import buttons, and move any
selection-clearing logic out of the start of handleImportWorktree into the
mutation's onSuccess/onSettled handlers so the selection is only cleared after
the mutation completes; update the code paths around handleImportWorktree,
openExternalWorktree.mutateAsync and the bulk-import mutation to use this
combined flag.
---
Outside diff comments:
In `@apps/api/src/app/api/github/jobs/initial-sync/route.ts`:
- Around line 97-120: The upsert into githubRepositories currently updates
denormalized org fields on conflict but omits updating installationId, which can
leave a stale installation_id after repo transfers; update the
onConflictDoUpdate.set payload in the insert call (the block that references
githubRepositories and .onConflictDoUpdate) to include installationId:
installationDbId so the stored installation_id is refreshed on repo
conflict/upsert.
- Around line 188-229: When upserting into githubPullRequests, the
onConflictDoUpdate block doesn't update the organizationId so existing PR rows
retain the old org; update the conflict set to include
githubPullRequests.organizationId (assign it the current organizationId
variable) alongside the other fields (e.g., in the onConflictDoUpdate.set add
organizationId: organizationId) so rows get reassociated when a repository moves
orgs.
In `@apps/api/src/app/api/github/sync/route.ts`:
- Around line 69-92: The upsert for githubRepositories omits updating
installationId on conflict, so retain the installationId from the current
request when a repo row already exists; modify the onConflictDoUpdate set in the
db.insert(githubRepositories).values(...) call to include installationId (set to
installation.id) alongside the existing fields (owner, name, fullName,
defaultBranch, isPrivate, updatedAt) so the row's installation is correctly
carried forward.
- Around line 160-201: The conflict update for the githubPullRequests upsert
doesn’t update organizationId, so when a repository moves orgs the PR row keeps
the old org; inside the .onConflictDoUpdate({ target: [...], set: { ... } }) for
the insert into githubPullRequests (the block using
db.insert(githubPullRequests).values(...).onConflictDoUpdate), add
organizationId: organizationId to the set object so the existing row’s
organizationId is updated on conflict (keep the other fields like
lastSyncedAt/updatedAt as-is).
In `@apps/api/src/app/api/github/webhook/webhooks.ts`:
- Around line 158-197: The conflict update currently omits organizationId so
when a repository's organization changes existing githubPullRequests rows stay
pinned to the old organization; update the onConflictDoUpdate set to include
organizationId: repo.organizationId (alongside lastSyncedAt/updatedAt etc.) so
the existing row is re-scoped to the correct organizationId when the insert
conflicts on githubPullRequests.repositoryId and githubPullRequests.prNumber.
- Around line 89-101: The insert into githubRepositories currently uses
onConflictDoNothing(), which leaves existing rows with stale
installationId/organizationId when a repo is transferred or reinstalled; change
the DB operation to an upsert/update-on-conflict instead of no-op: locate the
db.insert(githubRepositories).values(...) call and replace the
onConflictDoNothing() behavior with an onConflict(...) or equivalent upsert that
updates at minimum installationId and organizationId (and optionally owner,
name, fullName, defaultBranch, isPrivate) for the conflicting repoId/fullName
key so the tenant mapping is corrected on webhook re-adds.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 20d651ad-c407-4a59-9d49-13fec5fdc01e
📒 Files selected for processing (41)
apps/api/src/app/api/electric/[...path]/utils.tsapps/api/src/app/api/github/jobs/initial-sync/route.tsapps/api/src/app/api/github/sync/route.tsapps/api/src/app/api/github/webhook/webhooks.tsapps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsxapps/desktop/src/renderer/components/NewWorkspaceModal/components/BranchesGroup/BranchesGroup.tsxapps/desktop/src/renderer/components/NewWorkspaceModal/components/BranchesGroup/index.tsapps/desktop/src/renderer/components/NewWorkspaceModal/components/ExistingWorktreesList/ExistingWorktreesList.tsxapps/desktop/src/renderer/components/NewWorkspaceModal/components/ExistingWorktreesList/components/BranchesSection.tsxapps/desktop/src/renderer/components/NewWorkspaceModal/components/ExistingWorktreesList/components/PrUrlSection.tsxapps/desktop/src/renderer/components/NewWorkspaceModal/components/ExistingWorktreesList/components/WorktreesSection.tsxapps/desktop/src/renderer/components/NewWorkspaceModal/components/ExistingWorktreesList/components/index.tsapps/desktop/src/renderer/components/NewWorkspaceModal/components/ExistingWorktreesList/index.tsapps/desktop/src/renderer/components/NewWorkspaceModal/components/ImportFlow/ImportFlow.tsxapps/desktop/src/renderer/components/NewWorkspaceModal/components/ImportFlow/index.tsapps/desktop/src/renderer/components/NewWorkspaceModal/components/IssuesGroup/IssuesGroup.tsxapps/desktop/src/renderer/components/NewWorkspaceModal/components/IssuesGroup/index.tsapps/desktop/src/renderer/components/NewWorkspaceModal/components/NewWorkspaceAdvancedOptions/NewWorkspaceAdvancedOptions.tsxapps/desktop/src/renderer/components/NewWorkspaceModal/components/NewWorkspaceAdvancedOptions/index.tsapps/desktop/src/renderer/components/NewWorkspaceModal/components/NewWorkspaceCreateFlow/NewWorkspaceCreateFlow.tsxapps/desktop/src/renderer/components/NewWorkspaceModal/components/NewWorkspaceCreateFlow/index.tsapps/desktop/src/renderer/components/NewWorkspaceModal/components/NewWorkspaceHeader/NewWorkspaceHeader.tsxapps/desktop/src/renderer/components/NewWorkspaceModal/components/NewWorkspaceHeader/index.tsapps/desktop/src/renderer/components/NewWorkspaceModal/components/ProjectSelector/ProjectSelector.tsxapps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/PromptGroup.tsxapps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/index.tsapps/desktop/src/renderer/components/NewWorkspaceModal/components/PullRequestsGroup/PullRequestsGroup.tsxapps/desktop/src/renderer/components/NewWorkspaceModal/components/PullRequestsGroup/index.tsapps/desktop/src/renderer/lib/slug-width.tsapps/desktop/src/renderer/react-query/workspaces/index.tsapps/desktop/src/renderer/react-query/workspaces/useOpenWorktree.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/hooks/useTasksTable/useTasksTable.tsxapps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/collections.tsapps/desktop/src/renderer/routes/_authenticated/settings/project/$projectId/components/ProjectSettings/ProjectSettings.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/NewWorkspaceButton.tsxapps/electric-proxy/src/where.tspackages/db/drizzle/0025_add_org_id_to_github_tables.sqlpackages/db/drizzle/meta/0025_snapshot.jsonpackages/db/drizzle/meta/_journal.jsonpackages/db/src/schema/github.tspackages/db/src/schema/relations.ts
💤 Files with no reviewable changes (17)
- apps/desktop/src/renderer/react-query/workspaces/index.ts
- apps/desktop/src/renderer/components/NewWorkspaceModal/components/ImportFlow/index.ts
- apps/desktop/src/renderer/components/NewWorkspaceModal/components/NewWorkspaceAdvancedOptions/NewWorkspaceAdvancedOptions.tsx
- apps/desktop/src/renderer/components/NewWorkspaceModal/components/NewWorkspaceAdvancedOptions/index.ts
- apps/desktop/src/renderer/components/NewWorkspaceModal/components/ExistingWorktreesList/components/PrUrlSection.tsx
- apps/desktop/src/renderer/components/NewWorkspaceModal/components/NewWorkspaceCreateFlow/index.ts
- apps/desktop/src/renderer/components/NewWorkspaceModal/components/NewWorkspaceHeader/index.ts
- apps/desktop/src/renderer/components/NewWorkspaceModal/components/ExistingWorktreesList/components/BranchesSection.tsx
- apps/desktop/src/renderer/components/NewWorkspaceModal/components/ExistingWorktreesList/index.ts
- apps/desktop/src/renderer/components/NewWorkspaceModal/components/ImportFlow/ImportFlow.tsx
- apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/NewWorkspaceButton.tsx
- apps/desktop/src/renderer/components/NewWorkspaceModal/components/ExistingWorktreesList/ExistingWorktreesList.tsx
- apps/desktop/src/renderer/components/NewWorkspaceModal/components/ExistingWorktreesList/components/index.ts
- apps/desktop/src/renderer/components/NewWorkspaceModal/components/NewWorkspaceCreateFlow/NewWorkspaceCreateFlow.tsx
- apps/desktop/src/renderer/components/NewWorkspaceModal/components/ExistingWorktreesList/components/WorktreesSection.tsx
- apps/desktop/src/renderer/react-query/workspaces/useOpenWorktree.ts
- apps/desktop/src/renderer/components/NewWorkspaceModal/components/NewWorkspaceHeader/NewWorkspaceHeader.tsx
Comment on lines
+70
to
+78
| const openBranches = useMemo( | ||
| () => | ||
| new Set( | ||
| allWorkspaces | ||
| .filter((w) => w.projectId === projectId) | ||
| .map((w) => w.branch), | ||
| ), | ||
| [allWorkspaces, projectId], | ||
| ); |
Contributor
There was a problem hiding this comment.
Potential case mismatch when checking open branches.
The openBranches set is built from w.branch without case normalization (Line 75), but the lookup at Line 144 and 173 uses task.slug.toLowerCase(). If workspaces store branches in their original case (e.g., "PROJ-123"), the check will fail to find a match.
🔧 Proposed fix: normalize branch names when building the Set
const openBranches = useMemo(
() =>
new Set(
allWorkspaces
.filter((w) => w.projectId === projectId)
- .map((w) => w.branch),
+ .map((w) => w.branch.toLowerCase()),
),
[allWorkspaces, projectId],
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const openBranches = useMemo( | |
| () => | |
| new Set( | |
| allWorkspaces | |
| .filter((w) => w.projectId === projectId) | |
| .map((w) => w.branch), | |
| ), | |
| [allWorkspaces, projectId], | |
| ); | |
| const openBranches = useMemo( | |
| () => | |
| new Set( | |
| allWorkspaces | |
| .filter((w) => w.projectId === projectId) | |
| .map((w) => w.branch.toLowerCase()), | |
| ), | |
| [allWorkspaces, projectId], | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/components/NewWorkspaceModal/components/IssuesGroup/IssuesGroup.tsx`
around lines 70 - 78, The openBranches set is built from raw workspace branch
names (openBranches) but lookups use task.slug.toLowerCase(), causing
case-mismatch misses; fix by normalizing branch names when building the set (map
workspace.branch to .toLowerCase()) and ensure comparisons use the same
normalized form (compare against task.slug.toLowerCase()), updating the Set
construction that uses allWorkspaces and projectId so all entries are
lowercased.
Comment on lines
+69
to
+80
| const openBranches = useMemo( | ||
| () => | ||
| new Set( | ||
| allWorkspaces | ||
| .filter((w) => w.projectId === projectId) | ||
| .map((w) => w.branch), | ||
| ), | ||
| [allWorkspaces, projectId], | ||
| ); | ||
|
|
||
| const openPrs = useMemo( | ||
| () => (pullRequests ?? []).filter((pr) => pr.state === "open").slice(0, 30), |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Server-side PR branch naming and dedupe:"
rg -n -A12 -B6 'getPrLocalBranchName|existingWorktree|localBranchName|headBranch' \
apps/desktop/src/lib/trpc/routers/workspaces/procedures/create.ts
echo
echo "Definition of getPrLocalBranchName:"
rg -n -A20 -B5 'function getPrLocalBranchName|const getPrLocalBranchName|export function getPrLocalBranchName|export const getPrLocalBranchName' \
apps/desktop/src
echo
echo "Client-side open/create detection:"
rg -n -A10 -B6 'openBranches|headBranch|wasExisting' \
apps/desktop/src/renderer/components/NewWorkspaceModal/components/PullRequestsGroup/PullRequestsGroup.tsx \
apps/desktop/src/renderer/react-query/workspaces/useCreateFromPr.tsRepository: superset-sh/superset
Length of output: 21680
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== PR Data Type Definition ==="
rg -n -B5 -A20 'type.*PullRequest|interface.*PullRequest' \
apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts | head -100
echo
echo "=== Check where headBranch is set/used on PR objects ==="
rg -n 'headBranch|headRefName' \
apps/desktop/src --max-count=30
echo
echo "=== useLiveQuery PR structure from GitHub ==="
rg -n -B5 -A10 'useLiveQuery|search.*pull' \
apps/desktop/src/renderer/components/NewWorkspaceModal/components/PullRequestsGroup/PullRequestsGroup.tsxRepository: superset-sh/superset
Length of output: 4045
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== GitHub PR schema/type definition ==="
rg -n 'githubPullRequests|type.*Pr|interface.*Pr' \
apps/desktop/src/lib --max-count=50 | grep -E 'interface|type|from.*github' | head -30
echo
echo "=== Database schema for PR table ==="
fd -e 'ts' -e 'sql' | xargs rg -l 'githubPullRequests|pullRequests.*table' | head -5
echo
echo "=== Search for GitHub PR data import/storage ==="
rg -n 'headBranch|isCrossRepository.*true|headRepositoryOwner' \
apps/desktop/src/lib --max-count=30Repository: superset-sh/superset
Length of output: 3850
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Find githubPullRequests table/collection definition ==="
rg -n -B5 -A30 'githubPullRequests.*table|export.*githubPullRequests|const githubPullRequests' \
apps/desktop/src
echo
echo "=== Check local-db for PR schema ==="
find apps -name '*db*' -type f | xargs rg -l 'pull.*request|github.*pr' | head -5
echo
echo "=== Check what PullRequestsGroup receives as PR data ==="
rg -n -B10 'pr\.headBranch\|pr\.head' \
apps/desktop/src/renderer/components/NewWorkspaceModal/components/PullRequestsGroup/PullRequestsGroup.tsxRepository: superset-sh/superset
Length of output: 4283
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Find SelectGithubPullRequest type ==="
rg -n -B5 -A20 'type SelectGithubPullRequest|interface SelectGithubPullRequest' \
apps
echo
echo "=== Check github_pull_requests table schema ==="
rg -n -B10 -A20 'github_pull_requests.*table|githubPullRequests.*Schema' \
apps --max-count=50
echo
echo "=== Find where PR data gets columns mapped ==="
rg -n 'columnMapper|headBranch' \
apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/collections.tsRepository: superset-sh/superset
Length of output: 105
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Search for github_pull_requests in database files ==="
find apps -name '*.ts' -type f | xargs grep -l 'github_pull_requests' | head -10
echo
echo "=== Look in packages directory for DB schema ==="
find . -path ./node_modules -prune -o -name '*.ts' -type f -print | xargs grep -l 'SelectGithubPullRequest\|githubPullRequests.*table' | head -10
echo
echo "=== Search for head_branch column ==="
rg -n 'head_branch|headBranch' apps --max-count=30
echo
echo "=== Check the actual PR type being used in PullRequestsGroup ==="
cat apps/desktop/src/renderer/components/NewWorkspaceModal/components/PullRequestsGroup/PullRequestsGroup.tsx | head -100Repository: superset-sh/superset
Length of output: 4876
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Check github schema for PR columns ==="
cat packages/db/src/schema/github.ts
echo
echo "=== Verify what headBranch value is stored in DB ==="
rg -n -B5 -A5 'headBranch.*pr\.head\|headBranch:' apps/api/src/app/api/githubRepository: superset-sh/superset
Length of output: 5900
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Check how headBranch is populated from GitHub API ==="
rg -n -B10 -A10 'headBranch:.*pr\.head' apps/api/src/app/api/github
echo
echo "=== Check if headRepositoryOwner is available in the PR sync ==="
rg -n -B15 -A15 'isCrossRepository\|headRepositoryOwner' apps/api/src/app/api/github
echo
echo "=== Verify PR data structure when synced ==="
rg -n -B20 -A5 'github_pull_requests.*insert\|InsertGithubPullRequest' apps/api/src/app/api/github | head -80Repository: superset-sh/superset
Length of output: 6905
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Check if PR API response includes fork owner information ==="
rg -n -B5 -A20 'pr\.head\|headRepository\|isCrossRepository' \
apps/api/src/app/api/github/webhook/webhooks.ts | head -60
echo
echo "=== Confirm what pr object fields are available from GitHub API ==="
rg -n 'pr\.head|pr\.repository|pr\.user' \
apps/api/src/app/api/github --max-count=20Repository: superset-sh/superset
Length of output: 128
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Look at the webhook PR structure more carefully ==="
head -200 apps/api/src/app/api/github/webhook/webhooks.ts | tail -100
echo
echo "=== Search for where PR data comes from GitHub ==="
rg -n 'head\.' apps/api/src/app/api/github/webhook/webhooks.ts | head -20Repository: superset-sh/superset
Length of output: 2929
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Verify workspace creation stores getPrLocalBranchName result ==="
rg -n -A5 'workspace.*branch.*localBranchName\|branch:.*localBranchName' \
apps/desktop/src/lib/trpc/routers/workspaces/procedures/create.ts | head -40Repository: superset-sh/superset
Length of output: 132
Client-side PR key for open/existing detection does not match server-side PR branch naming for fork PRs.
For cross-repository PRs, the server computes branch names via getPrLocalBranchName() as forkOwner/headRefName (line 945, git.ts:1524–1530), and stores this in workspace.branch. However, the database stores only pr.headBranch = pr.head.ref from GitHub API, which lacks the fork owner prefix. The client checks openBranches.has(pr.headBranch) at lines 156 and 174, causing the lookup to fail for fork workspaces: the Set contains "alice/feature" but the check uses "feature". Existing fork PR workspaces will incorrectly show the "Create" button and wrong icon state.
Sync the PR branch key used on the client with the server's getPrLocalBranchName() computation. Either store the fork owner prefix in githubPullRequests.headBranch at sync time, or reconstruct it client-side during the existence check.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/components/NewWorkspaceModal/components/PullRequestsGroup/PullRequestsGroup.tsx`
around lines 69 - 80, The client is comparing PR head keys using pr.headBranch
against the Set built in openBranches (from workspace.branch) but fork PRs use
the server-side getPrLocalBranchName format ("forkOwner/headRefName"); update
the client to compute the same local-branch key before lookup: in
PullRequestsGroup.tsx where openPrs and openBranches are used, replace direct
uses of pr.headBranch in checks with a reconstructed key (e.g., combine the PR's
head repo owner/login with pr.headBranch the same way getPrLocalBranchName
does), or import/replicate getPrLocalBranchName logic so openBranches.has(...)
and icon/button logic use the matching key; ensure this change is applied for
both existence checks (the spots that currently call
openBranches.has(pr.headBranch) and any similar comparisons).
Comment on lines
+139
to
+151
| toast.promise( | ||
| createFromPr.mutateAsync({ | ||
| projectId, | ||
| prUrl: pr.url, | ||
| }), | ||
| { | ||
| loading: "Creating workspace from PR...", | ||
| success: "Workspace created", | ||
| error: (err) => | ||
| err instanceof Error | ||
| ? err.message | ||
| : "Failed to create workspace", | ||
| }, |
Contributor
There was a problem hiding this comment.
Make the success toast match the Open/Create outcome.
useCreateFromPr already distinguishes reopened workspaces with wasExisting in apps/desktop/src/renderer/react-query/workspaces/useCreateFromPr.ts, Lines 1-47, but this toast always says "Workspace created". When the item advertises Open, that copy is misleading.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/components/NewWorkspaceModal/components/PullRequestsGroup/PullRequestsGroup.tsx`
around lines 139 - 151, The success toast always says "Workspace created" even
though useCreateFromPr (and its createFromPr.mutateAsync call) returns
wasExisting to indicate reopened (opened) workspaces; update the toast.promise
success handler to inspect the resolved result from createFromPr.mutateAsync and
return "Workspace opened" when result.wasExisting is true, otherwise "Workspace
created" (e.g., set the success option to a function that checks
res?.wasExisting and returns the appropriate string).
Comment on lines
+35
to
44
| // Sync pre-selected project when modal opens | ||
| // biome-ignore lint/correctness/useExhaustiveDependencies: reset on modal open | ||
| useEffect(() => { | ||
| if (!isOpen) return; | ||
| resetForm(); | ||
| if (preSelectedProjectId) { | ||
| setSelectedProjectId(preSelectedProjectId); | ||
| } else if (recentProjects.length > 0 && !selectedProjectId) { | ||
| setSelectedProjectId(recentProjects[0].id); | ||
| } | ||
| }, [isOpen]); |
Contributor
There was a problem hiding this comment.
Let the default selection react when recents finish loading.
If getRecents() resolves after the modal opens, Line 41 never runs again because this effect only depends on isOpen, so the modal stays unselected until the user manually picks a project.
Suggested fix
- // Sync pre-selected project when modal opens
- // biome-ignore lint/correctness/useExhaustiveDependencies: reset on modal open
- useEffect(() => {
- if (!isOpen) return;
- if (preSelectedProjectId) {
- setSelectedProjectId(preSelectedProjectId);
- } else if (recentProjects.length > 0 && !selectedProjectId) {
- setSelectedProjectId(recentProjects[0].id);
- }
- }, [isOpen]);
+ useEffect(() => {
+ if (!isOpen || !preSelectedProjectId) return;
+ setSelectedProjectId(preSelectedProjectId);
+ }, [isOpen, preSelectedProjectId]);
+
+ useEffect(() => {
+ if (!isOpen || preSelectedProjectId || selectedProjectId) return;
+ const firstRecentProject = recentProjects.find((project) =>
+ Boolean(project.id),
+ );
+ if (!firstRecentProject?.id) return;
+ setSelectedProjectId(firstRecentProject.id);
+ }, [isOpen, preSelectedProjectId, recentProjects, selectedProjectId]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Sync pre-selected project when modal opens | |
| // biome-ignore lint/correctness/useExhaustiveDependencies: reset on modal open | |
| useEffect(() => { | |
| if (!isOpen) return; | |
| resetForm(); | |
| if (preSelectedProjectId) { | |
| setSelectedProjectId(preSelectedProjectId); | |
| } else if (recentProjects.length > 0 && !selectedProjectId) { | |
| setSelectedProjectId(recentProjects[0].id); | |
| } | |
| }, [isOpen]); | |
| useEffect(() => { | |
| if (!isOpen || !preSelectedProjectId) return; | |
| setSelectedProjectId(preSelectedProjectId); | |
| }, [isOpen, preSelectedProjectId]); | |
| useEffect(() => { | |
| if (!isOpen || preSelectedProjectId || selectedProjectId) return; | |
| const firstRecentProject = recentProjects.find((project) => | |
| Boolean(project.id), | |
| ); | |
| if (!firstRecentProject?.id) return; | |
| setSelectedProjectId(firstRecentProject.id); | |
| }, [isOpen, preSelectedProjectId, recentProjects, selectedProjectId]); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx`
around lines 35 - 44, The effect that syncs the pre-selected project on modal
open currently only depends on isOpen, so if recentProjects arrive after open
the default selection never runs; update the useEffect (the effect that
references isOpen, preSelectedProjectId, recentProjects, selectedProjectId, and
setSelectedProjectId) to also depend on recentProjects (and preSelectedProjectId
if needed) and keep the existing guards (isOpen check and only set when
selectedProjectId is falsy) so that when recents load after the modal opens it
will set recentProjects[0].id as the selected project without overriding an
already-made user selection.
Comment on lines
+46
to
+48
| const selectedProject = recentProjects.find( | ||
| (p) => p.id === selectedProjectId, | ||
| ); |
Contributor
There was a problem hiding this comment.
Don't resolve the selected project from getRecents() only.
Line 46 treats recents as the source of truth for the selected project, but selectedProjectId can also come from usePreSelectedProjectId(). When that id is not in the recents payload, Lines 91-116 send selectedProjectName, githubOwner, and repoName as null, so the selector loses its label and the PR tab falls into its empty states even though a project is selected. apps/desktop/src/renderer/components/NewWorkspaceModal/components/PullRequestsGroup/PullRequestsGroup.tsx, Lines 37-52, depends on those props to resolve the repo.
Also applies to: 89-116
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx`
around lines 46 - 48, The selected project resolution currently only looks up
recentProjects (variable selectedProject) which misses cases where
selectedProjectId comes from usePreSelectedProjectId(); update the logic that
builds selectedProjectName, githubOwner and repoName (the code that uses
selectedProject found from recentProjects and the block around where those props
are set) to first try recentProjects.find(p => p.id === selectedProjectId) and
if not found fall back to resolving the project via the pre-selected source
(usePreSelectedProjectId() / any preSelectedProject record) or another project
lookup (e.g., global projects store or an API call) so the component passes
non-null selectedProjectName, githubOwner and repoName to PullRequestsGroup;
ensure you reference and update the variables selectedProject,
selectedProjectId, usePreSelectedProjectId(), selectedProjectName, githubOwner,
and repoName in the NewWorkspaceModal component.
Comment on lines
+228
to
+242
| const handleImportWorktree = async (path: string, branch: string) => { | ||
| toast.promise( | ||
| openExternalWorktree.mutateAsync({ | ||
| projectId, | ||
| worktreePath: path, | ||
| branch, | ||
| }), | ||
| { | ||
| loading: "Importing worktree...", | ||
| success: `Imported ${branch}`, | ||
| error: (err) => | ||
| err instanceof Error ? err.message : "Failed to import worktree", | ||
| }, | ||
| ); | ||
| }; |
Contributor
There was a problem hiding this comment.
Disable both import paths while either import mutation is in flight.
Line 460 clears the selection before the single-import mutation settles, which immediately swaps the UI over to the bulk-import button. Since the Select and each button only check their own pending flag, users can start overlapping single and bulk imports from the same control block.
Proposed fix
+ const isImportPending =
+ importAllWorktrees.isPending || openExternalWorktree.isPending;
+
const handleImportWorktree = async (path: string, branch: string) => {
- toast.promise(
- openExternalWorktree.mutateAsync({
+ const promise = openExternalWorktree.mutateAsync({
projectId,
worktreePath: path,
branch,
- }),
- {
- loading: "Importing worktree...",
- success: `Imported ${branch}`,
- error: (err) =>
- err instanceof Error ? err.message : "Failed to import worktree",
- },
- );
+ });
+ toast.promise(promise, {
+ loading: "Importing worktree...",
+ success: `Imported ${branch}`,
+ error: (err) =>
+ err instanceof Error ? err.message : "Failed to import worktree",
+ });
+ return promise;
};
...
<Select
value={selectedWorktreePath ?? "__all__"}
+ disabled={isImportPending}
onValueChange={(value) =>
setSelectedWorktreePath(value === "__all__" ? null : value)
}
>
...
<Button
size="sm"
className="w-22"
- disabled={openExternalWorktree.isPending}
- onClick={() => {
+ disabled={isImportPending}
+ onClick={async () => {
const wt = externalWorktrees.find(
(w) => w.path === selectedWorktreePath,
);
if (wt) {
- handleImportWorktree(wt.path, wt.branch);
- setSelectedWorktreePath(null);
+ try {
+ await handleImportWorktree(wt.path, wt.branch);
+ setSelectedWorktreePath(null);
+ } catch {
+ // toast.promise already reports the failure
+ }
}
}}
>
...
<Button
size="sm"
className="w-22"
- disabled={importAllWorktrees.isPending}
+ disabled={isImportPending}
>Also applies to: 429-500
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/routes/_authenticated/settings/project/`$projectId/components/ProjectSettings/ProjectSettings.tsx
around lines 228 - 242, The single-import flow clears the Select and only checks
openExternalWorktree's pending state, letting the UI flip to the bulk-import
control while a mutation is still in flight; fix by deriving a shared
"isImporting" boolean from both import mutations (e.g., combine
openExternalWorktree.isLoading || <bulkImportMutation>.isLoading), use that
shared flag to disable the Select and both import buttons, and move any
selection-clearing logic out of the start of handleImportWorktree into the
mutation's onSuccess/onSettled handlers so the selection is only cleared after
the mutation completes; update the code paths around handleImportWorktree,
openExternalWorktree.mutateAsync and the bulk-import mutation to use this
combined flag.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/hooks/useTasksTable/useTasksTable.tsx (1)
244-246: Addw-fullto the slug cell span for more explicit width handling.The span has
truncateandmin-w-0which should work within the flex container and grid column constraints, but explicitly settingw-fullmakes the layout intent clearer and more robust.Suggested improvement
- <span className="text-xs text-muted-foreground truncate min-w-0"> + <span className="w-full min-w-0 truncate text-xs text-muted-foreground"> {info.getValue()} </span>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/hooks/useTasksTable/useTasksTable.tsx` around lines 244 - 246, The slug cell's span rendering {info.getValue()} needs explicit full width: update the JSX span in useTasksTable (the element showing {info.getValue()} within useTasksTable.tsx) to include the "w-full" utility in its className alongside "text-xs text-muted-foreground truncate min-w-0" so the cell reliably occupies full available width for truncation behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/hooks/useTasksTable/useTasksTable.tsx`:
- Around line 244-246: The slug cell's span rendering {info.getValue()} needs
explicit full width: update the JSX span in useTasksTable (the element showing
{info.getValue()} within useTasksTable.tsx) to include the "w-full" utility in
its className alongside "text-xs text-muted-foreground truncate min-w-0" so the
cell reliably occupies full available width for truncation behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 094af6f3-e874-4dba-ba37-ee55531fc5de
📒 Files selected for processing (3)
apps/desktop/src/renderer/react-query/workspaces/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/hooks/useTasksTable/useTasksTable.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/hooks/useOrderedSections/useOrderedSections.test.tsx
💤 Files with no reviewable changes (1)
- apps/desktop/src/renderer/react-query/workspaces/index.ts
✅ Files skipped from review due to trivial changes (1)
- apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/hooks/useOrderedSections/useOrderedSections.test.tsx
- IssuesGroup/PullRequestsGroup: navigate to existing workspace on "Open" instead of creating duplicate - PromptGroup: fix Ctrl+Enter shortcut on Windows/Linux (was only checking metaKey) - GitHub sync: add organizationId to onConflictDoUpdate for PR upserts across initial-sync, sync, and webhooks
Contributor
There was a problem hiding this comment.
1 issue found across 6 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/desktop/src/renderer/components/NewWorkspaceModal/components/IssuesGroup/IssuesGroup.tsx">
<violation number="1" location="apps/desktop/src/renderer/components/NewWorkspaceModal/components/IssuesGroup/IssuesGroup.tsx:71">
P1: Duplicate `workspaceByBranch` mapping logic across three components in NewWorkspaceModal</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| const { data: allWorkspaces = [] } = | ||
| electronTrpc.workspaces.getAll.useQuery(); | ||
|
|
||
| const workspaceByBranch = useMemo(() => { |
Contributor
There was a problem hiding this comment.
P1: Duplicate workspaceByBranch mapping logic across three components in NewWorkspaceModal
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/components/NewWorkspaceModal/components/IssuesGroup/IssuesGroup.tsx, line 71:
<comment>Duplicate `workspaceByBranch` mapping logic across three components in NewWorkspaceModal</comment>
<file context>
@@ -67,15 +68,15 @@ export function IssuesGroup({ projectId, onClose }: IssuesGroupProps) {
- ),
- [allWorkspaces, projectId],
- );
+ const workspaceByBranch = useMemo(() => {
+ const map = new Map<string, string>();
+ for (const w of allWorkspaces) {
</file context>
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
owner+namefrom the local project instead of using the cloud project FK (wrong ID space fix). Added GitHub connect empty state, draft PR icons, workspace detection (Open vs Create).electronTrpc.projects.getBranchesdata. Added open/create/duplicate branch actions, local vs remote icons, loading state.progressPercentto StatusIcon.useHotkeysStore(⌘ on Mac, Ctrl on Windows/Linux).NewWorkspaceModal.old/(18 files, ~1800 lines) and orphaneduseOpenWorktreehook.Test plan
Summary by cubic
Revamps the New Workspace modal with tabbed PR/Branch/Issue/Prompt flows and adds Worktrees import in Project Settings. Scopes GitHub repos/PRs by organization and fixes “Open” to reuse existing workspaces; Prompt Ctrl/Cmd+Enter works on all platforms.
New Features
Migration
Written for commit cfe6b2d. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes