feat(v2): minimal project create/import for workspaces#3566
Conversation
Simplified redesign after PR review. Collapses the earlier three-signal backing model (cloud + per-host cloud signal + local) into two signals (cloud + local-only), removes the v2_host_projects cloud table and Electric sync, drops per-row state decoration on the sidebar, and moves backing checks to action time (workspace-create modal, error paths).
Adds the cloud-side matcher used by host-service's folder-first import flow: given a clone URL, returns candidate projects the user has access to whose GitHub repo matches (case-insensitively). Named findByGitHubRemote (not findByRemote) because the match is GitHub-specific. v2Projects.create switches to jwtProcedure with an explicit organizationId + repoCloneUrl, matching the shape host-service needs to call from project.create. No existing callers. parseGitHubRemote moves from packages/host-service to packages/shared so both cloud tRPC and host-service consume the same implementation.
Full create/import lifecycle in host-service: - project.list — DB read of host-service.projects. Pure, no filesystem probing. Stale paths surface via operation errors, not proactive checks. - project.findByPath — validate git root, read remote, forward to cloud v2Projects.findByGitHubRemote. Backs the folder-first import picker. - project.create — discriminated-union mode (empty/clone/importLocal/ template); Phase 1 ships clone + importLocal only, empty and template throw NOT_IMPLEMENTED. - project.setup — discriminated-union mode (clone/import) with acknowledgeWorkspaceInvalidation gate on the re-point case. - project.remove — local worktree + repo dir teardown. Cloud backing (v2_host_projects) is intentionally absent: there is no per-host cloud signal in this design. Backing is a local-only concept, checked at action time. Adds ProjectNotSetupCause to the error formatter so the renderer can catch throws from workspace.create (next commit) and open the Pin & Set Up modal inline.
Three flows for getting projects onto this device: - New project — clone a GitHub URL into a chosen parent directory. Drives project.create(mode=clone). - Import existing folder — native picker → project.findByPath branches on candidate count. 0 → name + create (importLocal). 1, not set up here → auto-advance to project.setup. 1, already set up → destructive re-point confirmation. >1 → picker modal. - Pin & set up — clone an existing cloud project onto this device. Drives project.setup(mode=clone), with forceRepoint entry for repair. All three modals are mounted once at the dashboard layout level via AddRepositoryModals, and opened through a small zustand store. Sidebar header "Add repository" dropdown triggers New project / Import folder.
… trigger
Lists cloud projects in the user's active org that aren't pinned
locally. Pin & set up per row runs project.setup. Header dropdown
("Add repository") mirrors the sidebar — "+ New project" +
"Import existing folder." Entry points route through the dashboard-level
AddRepositoryModals via the shared zustand store.
useAvailableV2Projects powers the section: antijoin
v2Projects ∖ v2SidebarProjects scoped to the active organization,
with the existing v2-workspaces search filter applied.
- Host-service workspaceCreation.{create,checkout,adopt} throw
PROJECT_NOT_SETUP (PRECONDITION_FAILED + cause { kind, projectId })
when this host has no local project row. No more silent auto-clone
into ~/.superset/repos/ — the user explicitly picks where to clone.
- Pending workspace-create page intercepts data.projectNotSetup on the
error, opens the Pin & set up modal pre-filled with the project, and
registers a one-shot onSuccess callback to retry the original intent
once setup resolves. The pending row stays in "creating" through the
modal so the UI doesn't flicker to failed.
- Clicking a remote-device workspace row lands on the new
WorkspaceNotOnThisHostState stub: explains the workspace lives on
another host, offers "Set up here" (opens Pin & set up for the
project) or "Browse workspaces." V2 workspace page checks
host.machineId via live query and renders the stub before mounting
the pane tree, which would otherwise crash on a foreign worktree.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds dashboard-level add-repository modals and store triggers, a folder-first import state machine and UI, host-service project create/setup/find handlers and error shape, cloud v2 project discovery, workspace UI integrations for available projects, shared GitHub-remote export, docs/plans, and several UI/prop refactors across prompt/attachment and chat components. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant UI as Desktop UI (modals)
participant Native as Native Picker
participant Host as Host Service
participant Cloud as Cloud API
participant Sidebar as Dashboard Sidebar
User->>UI: Open "Import existing folder"
UI->>Native: Open directory picker
Native-->>UI: repoPath
UI->>Host: project.findByPath(repoPath)
Host->>Host: resolveWithPrimaryRemote / parse remotes
Host->>Cloud: v2Project.findByGitHubRemote(repoCloneUrl)
Cloud-->>Host: candidates[]
alt single candidate
Host-->>UI: auto-transition to setup
UI->>Host: project.setup(projectId, mode: import)
Host-->>UI: success (repoPath)
else multiple candidates
UI-->>User: show candidate picker
User->>UI: select candidate
UI->>Host: project.setup(projectId, mode: import)
Host-->>UI: success or CONFLICT
alt CONFLICT
UI-->>User: show re-point confirmation
User->>UI: confirm re-point
UI->>Host: project.setup(projectId, acknowledgeWorkspaceInvalidation: true)
Host-->>UI: success
end
else no candidates
UI-->>User: show "Create project" form
User->>UI: submit name
UI->>Cloud: v2Project.create(name, repoCloneUrl, organizationId)
Cloud-->>UI: projectId
UI->>Host: project.create(mode: importLocal, repoPath)
Host-->>UI: success
end
UI->>Sidebar: ensureProjectInSidebar(projectId)
Sidebar-->>Sidebar: update pins / invalidate queries
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
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 |
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (1)
packages/host-service/src/trpc/error-types.ts (1)
37-46: Consider tightening the type guard to validateprojectId.The guard narrows to
ProjectNotSetupCausebut never checks thatprojectIdis a string. If a thrower ever omits it or passes a non-string, the formatter intrpc/index.tswill silently emit an invalid shape to the renderer. Consistent withisTeardownFailureCause(which also only checkskind), so this is optional — but cheap to harden since the renderer relies onprojectIdto open the Pin & Setup modal.🛡️ Proposed hardening
export function isProjectNotSetupCause( value: unknown, ): value is ProjectNotSetupCause { return ( !!value && typeof value === "object" && "kind" in value && - (value as { kind: unknown }).kind === "PROJECT_NOT_SETUP" + (value as { kind: unknown }).kind === "PROJECT_NOT_SETUP" && + "projectId" in value && + typeof (value as { projectId: unknown }).projectId === "string" ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/host-service/src/trpc/error-types.ts` around lines 37 - 46, The type guard isProjectNotSetupCause currently only checks kind and can let through values missing or with a non-string projectId; update isProjectNotSetupCause to also verify that the object has a "projectId" property and that typeof (value as { projectId: unknown }).projectId === "string" so it truly narrows to ProjectNotSetupCause used by the trpc renderer (see trpc/index.ts) and prevents emitting an invalid shape to the renderer.
🤖 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/routes/_authenticated/_dashboard/components/AddRepositoryModals/AddRepositoryModals.tsx`:
- Around line 40-47: The effect currently depends on folderImport.start which
changes identity every render due to the inline options (used by
useFolderFirstImport -> reportError/reportSuccess), causing the effect to re-run
unexpectedly; update the useEffect dependencies to only include
folderImportTrigger (remove folderImport.start) so the effect runs solely when
the counter changes, and keep or adjust the eslint-disable comment for
react-hooks/exhaustive-deps to reflect the deliberate omission; locate this in
the useEffect that references folderImportTrigger, folderImport.start and the
useFolderFirstImport options/reportError/reportSuccess.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/AddRepositoryModals/components/FolderFirstImportModal/FolderFirstImportModal.tsx`:
- Around line 73-280: CandidatePickerContent currently lives alongside other
modal bodies and retains a stale selectedId when its candidates prop changes;
split each modal body into its own component folder (e.g.,
CandidatePickerContent/CandidatePickerContent.tsx + index.ts) to follow the
one-component-per-file rule, and update CandidatePickerContent to reset or
recompute selection when candidates change (e.g., useEffect to set selectedId to
candidates[0]?.id or derive selectedId from a prop), or key the component by
repoPath/candidates to remount it when the candidate set changes; ensure exports
match existing imports so callers of CandidatePickerContent, NoMatchContent, and
ConfirmRepointContent keep the same API.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/AddRepositoryModals/components/PinAndSetupModal/PinAndSetupModal.tsx`:
- Around line 47-62: The local state variable conflict is initialized with
useState(forceRepoint) but never resynced when the modal is reopened with a new
forceRepoint value; add an effect to watch forceRepoint and call
setConflict(forceRepoint) so the conflict flag is updated whenever forceRepoint
changes (keep reset using setConflict(forceRepoint) as-is); reference the
conflict state, setConflict setter, and the forceRepoint prop when adding this
useEffect.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/AddRepositoryModals/hooks/useFolderFirstImport/useFolderFirstImport.ts`:
- Around line 112-129: runSetup is losing the
FolderImportCandidate.organizationId and calling project.setup under the active
org; update the runSetup signature to accept an organizationId (e.g.
runSetup(projectId, repoPath, { organizationId, acknowledgeWorkspaceInvalidation
})) and pass that organizationId into the host call (project.setup.mutate
payload) so the setup runs in the candidate's org context, and update any
callers (the places that pass the candidate to runSetup) to forward
candidate.organizationId; alternatively (or additionally) ensure findByPath
filters candidates to the active organization before presenting choices to avoid
cross-org selection.
- Around line 145-158: The directory picker call (selectDirectory.mutateAsync)
can throw before the existing try/catch around client.project.findByPath.query,
so wrap the picker call in its own try/catch and funnel failures to the same
error path: catch the error, call reportError(err instanceof Error ? err.message
: String(err)), and return to prevent start() rejecting; keep the existing
canceled/!path guard and then continue to getHostServiceClientByUrl and call
project.findByPath.query inside the existing try/catch.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarHeader/DashboardSidebarHeader.tsx`:
- Around line 105-110: The icon-only buttons in DashboardSidebarHeader (the
button that renders <LuFolderPlus className="size-4" /> and the other icon-only
trigger around lines rendering the secondary icon) lack accessible names; add
aria-label="Add repository" to those <button> elements in the
DashboardSidebarHeader component so screen readers announce the action (locate
the buttons that render the LuFolderPlus icon and the other icon-only trigger
and add the aria-label prop).
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/pending/`$pendingId/page.tsx:
- Around line 211-222: The openPinAndSetup call only provides onSuccess so
closing the modal leaves the pending row stuck in "creating"; update the modal
API in add-repository-modal.ts to accept an optional onCancel callback, ensure
the modal component invokes that onCancel when the user closes/cancels the
modal, and then update the caller in page.tsx (the openPinAndSetup invocation)
to pass an onCancel that flips the pending workspace to failed (the same place
that would mark failure—i.e., call the failure handler currently used for the
pending row) so the UI isn't stuck on the spinner.
In `@packages/host-service/src/trpc/router/project/project.ts`:
- Around line 105-116: The current conflict guard throws whenever a project row
exists (variable existing) even if the caller is re-asserting the same git root;
change the logic to first read the existing.projects.repoPath and compare it to
the resolved target path (e.g., resolvedTargetPath / resolvedRepoPath) before
erroring: if they match, return success/idempotent response; only when the
repoPath differs require input.acknowledgeWorkspaceInvalidation and throw the
CONFLICT message (or proceed to update the path) — update the check around the
existing lookup, projects table usage, and
input.projectId/acknowledgeWorkspaceInvalidation accordingly.
- Around line 34-80: The input schema for the create protectedProcedure includes
a visibility field but that value is never forwarded—update or remove it: either
remove visibility from the z.object input in project.create (so callers cannot
request it), or thread visibility end-to-end by adding a visibility column to
the v2Projects DB, updating the cloud v2Project.create.mutate() schema to accept
visibility, and passing input.visibility into the handlers createFromClone and
createFromImportLocal (and onward into v2Project.create.mutate()); reference the
create protectedProcedure input, createFromClone, createFromImportLocal, and the
v2Project.create.mutate() call to implement the change consistently.
In `@packages/host-service/src/trpc/router/project/utils/resolve-repo.ts`:
- Around line 137-150: Replace the pre-clone existsSync check with an atomic
directory claim using mkdirSync(targetPath) (or mkdirSync(targetPath, {
recursive: false }) and catch EEXIST), set a local flag (e.g., ownsDir = true)
only when mkdirSync succeeds so we know we created the directory, then call
simpleGit().clone(repoCloneUrl, targetPath); in the catch block only remove the
directory with rmSync(targetPath, { recursive: true, force: true }) if ownsDir
is true; update references: targetPath, simpleGit().clone, existsSync ->
mkdirSync, rmSync, and the TRPCError thrown on EEXIST.
---
Nitpick comments:
In `@packages/host-service/src/trpc/error-types.ts`:
- Around line 37-46: The type guard isProjectNotSetupCause currently only checks
kind and can let through values missing or with a non-string projectId; update
isProjectNotSetupCause to also verify that the object has a "projectId" property
and that typeof (value as { projectId: unknown }).projectId === "string" so it
truly narrows to ProjectNotSetupCause used by the trpc renderer (see
trpc/index.ts) and prevents emitting an invalid shape to the renderer.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: db89b6da-6a84-4936-9c4b-384f2dc85ff5
📒 Files selected for processing (40)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/AddRepositoryModals/AddRepositoryModals.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/AddRepositoryModals/components/FolderFirstImportModal/FolderFirstImportModal.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/AddRepositoryModals/components/FolderFirstImportModal/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/AddRepositoryModals/components/NewProjectModal/NewProjectModal.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/AddRepositoryModals/components/NewProjectModal/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/AddRepositoryModals/components/ParentDirectoryPicker/ParentDirectoryPicker.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/AddRepositoryModals/components/ParentDirectoryPicker/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/AddRepositoryModals/components/PinAndSetupModal/PinAndSetupModal.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/AddRepositoryModals/components/PinAndSetupModal/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/AddRepositoryModals/hooks/useFolderFirstImport/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/AddRepositoryModals/hooks/useFolderFirstImport/useFolderFirstImport.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/AddRepositoryModals/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarHeader/DashboardSidebarHeader.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/layout.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/pending/$pendingId/page.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/page.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/components/WorkspaceNotOnThisHostState/WorkspaceNotOnThisHostState.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/components/WorkspaceNotOnThisHostState/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/components/V2AvailableProjectsSection/V2AvailableProjectsSection.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/components/V2AvailableProjectsSection/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/components/V2WorkspacesList/V2WorkspacesList.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/hooks/useAvailableV2Projects/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/hooks/useAvailableV2Projects/useAvailableV2Projects.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/page.tsxapps/desktop/src/renderer/stores/add-repository-modal.tsdocs/design/v2-project-create-import.mdpackages/host-service/src/runtime/pull-requests/pull-requests.tspackages/host-service/src/runtime/pull-requests/utils/parse-github-remote/index.tspackages/host-service/src/trpc/error-types.tspackages/host-service/src/trpc/index.tspackages/host-service/src/trpc/router/project/handlers.tspackages/host-service/src/trpc/router/project/project.tspackages/host-service/src/trpc/router/project/utils/git-remote.tspackages/host-service/src/trpc/router/project/utils/persist-project.tspackages/host-service/src/trpc/router/project/utils/resolve-repo.tspackages/host-service/src/trpc/router/workspace-creation/workspace-creation.tspackages/shared/package.jsonpackages/shared/src/github-remote.tspackages/trpc/src/router/v2-project/v2-project.tsplans/20260417-v2-project-create-import-impl.md
💤 Files with no reviewable changes (1)
- packages/host-service/src/runtime/pull-requests/utils/parse-github-remote/index.ts
There was a problem hiding this comment.
10 issues found across 40 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/desktop/src/renderer/routes/_authenticated/_dashboard/components/AddRepositoryModals/hooks/useFolderFirstImport/useFolderFirstImport.ts">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/AddRepositoryModals/hooks/useFolderFirstImport/useFolderFirstImport.ts:145">
P2: Handle picker mutation failures with `try/catch` so dialog/IPC errors don't escape as unhandled promise rejections.
(Based on your team's feedback about handling async rejections explicitly.) [FEEDBACK_USED]</violation>
</file>
<file name="apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/AddRepositoryModals/components/ParentDirectoryPicker/ParentDirectoryPicker.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/AddRepositoryModals/components/ParentDirectoryPicker/ParentDirectoryPicker.tsx:24">
P2: Handle `mutateAsync` failures in `handleBrowse` to avoid unhandled promise rejections from the click handler.
(Based on your team's feedback about handling async errors explicitly.) [FEEDBACK_USED]</violation>
</file>
<file name="packages/host-service/src/trpc/router/workspace-creation/workspace-creation.ts">
<violation number="1" location="packages/host-service/src/trpc/router/workspace-creation/workspace-creation.ts:734">
P2: Clear pending progress before throwing `PROJECT_NOT_SETUP`; otherwise failed requests can leave stale progress state visible to polling clients.</violation>
</file>
<file name="apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/AddRepositoryModals/AddRepositoryModals.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/AddRepositoryModals/AddRepositoryModals.tsx:42">
P2: Handle rejections from `folderImport.start()` in the effect to avoid unhandled promise rejections.
(Based on your team's feedback about handling async errors explicitly and avoiding unhandled promise rejections.) [FEEDBACK_USED]</violation>
<violation number="2" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/AddRepositoryModals/AddRepositoryModals.tsx:47">
P1: The dependency array includes `folderImport.start` but the comment above explicitly states the intent is to depend only on the counter. Since the `options` object passed to `useFolderFirstImport` is recreated every render, `start`'s identity changes every render (through its dependency chain on `reportSuccess`/`reportError`). After the first trigger, this effect will re-fire on every subsequent render, potentially reopening the folder picker.</violation>
</file>
<file name="docs/design/v2-project-create-import.md">
<violation number="1" location="docs/design/v2-project-create-import.md:154">
P2: The design doc documents an outdated `v2Projects.create` input shape. Update it to match the current API so implementers don’t call the endpoint with invalid parameters.</violation>
</file>
<file name="apps/desktop/src/renderer/routes/_authenticated/_dashboard/pending/$pendingId/page.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/pending/$pendingId/page.tsx:221">
P2: Canceling the Pin & set up modal leaves the pending workspace in `creating` because only `onSuccess` is handled, so this flow can get stuck on an indefinite spinner.</violation>
</file>
<file name="apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/AddRepositoryModals/components/PinAndSetupModal/PinAndSetupModal.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/AddRepositoryModals/components/PinAndSetupModal/PinAndSetupModal.tsx:54">
P2: `useState(forceRepoint)` only captures the prop value on initial mount. Since this modal is permanently mounted at the layout level, reopening it with `forceRepoint=true` (e.g. for stale-path repair) after a prior normal open won't update `conflict` state. Add a `useEffect` to sync `conflict` when `project`/`forceRepoint` changes.</violation>
</file>
<file name="packages/host-service/src/trpc/router/project/project.ts">
<violation number="1" location="packages/host-service/src/trpc/router/project/project.ts:38">
P2: `visibility` is accepted in the input schema but silently dropped — neither `createFromClone` nor `createFromImportLocal` passes it to the cloud `v2Project.create.mutate()` call, and the v2Projects table has no visibility column. Either thread it end-to-end or remove it from the contract to avoid misleading callers.</violation>
<violation number="2" location="packages/host-service/src/trpc/router/project/project.ts:110">
P2: The conflict guard fires for any existing project row, including idempotent setup/import of the same path. This forces the user through a destructive-workspace warning even when no re-point happens. Compare the existing `repoPath` with the resolved target path first and skip the `acknowledgeWorkspaceInvalidation` requirement when they match.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Greptile SummaryThis PR ships Phase 1 of the v2 project lifecycle — three explicit entry points (New project, Pin & set up, Folder-first import) replace the silent auto-clone-into- Key findings:
Confidence Score: 3/5Not safe to merge as-is: the useEffect dep-array bug causes the native folder picker to reopen repeatedly during an active import, and cancelling Pin & set up leaves the pending workspace permanently stuck in "creating". The host-service router, workspace-creation changes, cloud tRPC additions, and modal UIs are all well-structured and architecturally sound. However two issues in the renderer bridge logic are functional regressions on primary user paths: (1) the P0 useEffect dep bug causes an immediately broken folder-first import experience, and (2) the missing onCancel callback produces an unrecoverable "creating" state if the user bails out of Pin & set up after a PROJECT_NOT_SETUP error. Both are narrow, targeted fixes that don't require architectural changes.
|
| Filename | Overview |
|---|---|
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/AddRepositoryModals/AddRepositoryModals.tsx | Layout-level host for the three add-repository flows; contains a P0 bug where folderImport.start in the useEffect dep array causes re-invocation of the native folder picker on every state change during an active import |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/pending/$pendingId/page.tsx | Correctly intercepts PROJECT_NOT_SETUP and opens Pin & set up, but lacks an onCancel callback — if the user dismisses the modal the pending row stays stuck in "creating" state indefinitely |
| packages/host-service/src/trpc/router/project/handlers.ts | Clean extraction of create/setup handlers; known limitation of orphaned cloud row on clone failure is acknowledged in follow-ups but no compensating delete is attempted |
| packages/host-service/src/trpc/router/project/project.ts | Full rewrite of project router with discriminated-union modes, acknowledgeWorkspaceInvalidation guard for re-point, and pure-DB list; logic is sound |
| packages/host-service/src/trpc/router/workspace-creation/workspace-creation.ts | Silent auto-clone removed in favour of typed PROJECT_NOT_SETUP error across create/checkout/adopt paths; consistent and clean |
| packages/trpc/src/router/v2-project/v2-project.ts | Adds findByGitHubRemote (case-insensitive slug match) and switches create to jwtProcedure; missing slug uniqueness guard within an org could surface as an unfriendly DB error |
| apps/desktop/src/renderer/stores/add-repository-modal.ts | Clean Zustand store with trigger-counter pattern for folder import; openPinAndSetup opts only expose onSuccess — no onCancel hook to signal pending-page cancellation |
| packages/shared/src/github-remote.ts | Moved from host-service to shared; covers HTTPS, SSH, and git@ URL formats with normalised output URL; canonical and correct |
| packages/host-service/src/trpc/index.ts | Error formatter correctly re-serialises ProjectNotSetupCause as a plain object so the renderer can read data.projectNotSetup.projectId without superjson stripping fields |
Sequence Diagram
sequenceDiagram
participant U as User
participant R as Renderer
participant Store as Zustand Store
participant Modals as AddRepositoryModals
participant HS as Host-Service
participant Cloud as Cloud tRPC
Note over U,Cloud: New Project (clone) flow
U->>Store: openNewProject()
Store-->>Modals: active = new-project
U->>Modals: Fill name + URL + parentDir → submit
Modals->>Cloud: v2Projects.create(orgId, name, slug, repoCloneUrl)
Cloud-->>Modals: { id: projectId }
Modals->>HS: project.create(mode=clone, parentDir, url)
HS->>HS: cloneRepoInto(url, parentDir)
HS->>HS: persistLocalProject(ctx, projectId, resolved)
HS-->>Modals: { projectId, repoPath }
Modals->>Store: close()
Note over U,Cloud: Folder-first import flow
U->>Store: triggerFolderImport()
Store-->>Modals: folderImportTrigger++
Modals->>U: native folder picker
U->>Modals: picks /path/to/repo
Modals->>HS: project.findByPath({ repoPath })
HS->>HS: resolveWithPrimaryRemote(repoPath)
HS->>Cloud: v2Projects.findByGitHubRemote({ repoCloneUrl })
Cloud-->>HS: { candidates }
HS-->>Modals: { candidates }
alt 0 candidates
Modals->>U: NoMatch dialog
U->>Modals: confirm name
Modals->>HS: project.create(mode=importLocal, repoPath)
else 1 candidate no conflict
Modals->>HS: project.setup(mode=import, repoPath)
else 1 candidate CONFLICT
Modals->>U: ConfirmRepoint dialog
U->>Modals: confirm
Modals->>HS: project.setup(mode=import, acknowledgeWorkspaceInvalidation=true)
else more than 1 candidate
Modals->>U: CandidatePicker dialog
U->>Modals: select candidate
Modals->>HS: project.setup(mode=import, repoPath)
end
Note over U,Cloud: Inline workspace setup PROJECT_NOT_SETUP
U->>R: create workspace for project
R->>HS: workspaceCreation.create(projectId)
HS-->>R: PRECONDITION_FAILED cause PROJECT_NOT_SETUP
R->>Store: openPinAndSetup(target, onSuccess=retry)
Store-->>Modals: active = pin-and-setup
U->>Modals: select parentDir → submit
Modals->>HS: project.setup(mode=clone, parentDir)
HS->>HS: cloneRepoInto + persistLocalProject
HS-->>Modals: { repoPath }
Modals->>R: onSuccess() retry workspaceCreation.create
R->>HS: workspaceCreation.create(projectId) retry
HS-->>R: { workspace }
Comments Outside Diff (1)
-
packages/trpc/src/router/v2-project/v2-project.ts, line 158-215 (link)No slug uniqueness check within an organization
v2Projects.createinserts a row without verifying thatslugis unique withinorganizationId. If the DB schema has a unique constraint on(organizationId, slug), a duplicate will surface as a raw constraint violation rather than a user-friendly message. If there's no constraint, duplicate slugs can accumulate silently.Consider an explicit pre-flight check:
const existing = await dbWs.query.v2Projects.findFirst({ columns: { id: true }, where: and( eq(v2Projects.organizationId, input.organizationId), eq(v2Projects.slug, input.slug), ), }); if (existing) { throw new TRPCError({ code: "CONFLICT", message: `A project with slug "${input.slug}" already exists in this organization`, }); }
Prompt To Fix With AI
This is a comment left during a code review. Path: packages/trpc/src/router/v2-project/v2-project.ts Line: 158-215 Comment: **No slug uniqueness check within an organization** `v2Projects.create` inserts a row without verifying that `slug` is unique within `organizationId`. If the DB schema has a unique constraint on `(organizationId, slug)`, a duplicate will surface as a raw constraint violation rather than a user-friendly message. If there's no constraint, duplicate slugs can accumulate silently. Consider an explicit pre-flight check: ```typescript const existing = await dbWs.query.v2Projects.findFirst({ columns: { id: true }, where: and( eq(v2Projects.organizationId, input.organizationId), eq(v2Projects.slug, input.slug), ), }); if (existing) { throw new TRPCError({ code: "CONFLICT", message: `A project with slug "${input.slug}" already exists in this organization`, }); } ``` How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/AddRepositoryModals/AddRepositoryModals.tsx
Line: 42-48
Comment:
**`folderImport.start` in the dependency array causes unintended re-invocations**
The comment says "We intentionally depend only on the counter," but `folderImport.start` is still listed in the dep array. Because `options` (`onSuccess`/`onError` lambdas) is recreated on every render of `AddRepositoryModals`, and `reportSuccess` → `start` are memoized with `options` as a dep, `folderImport.start` gets a new identity on every render. Whenever `useFolderFirstImport`'s internal state changes (e.g., `no-match`, `pick`, `confirm-repoint`) it causes `AddRepositoryModals` to re-render, which recreates `start`, triggering this effect again — while `folderImportTrigger` is still non-zero — and calls `start()` a second time (opening a new native picker mid-flow).
The `// eslint-disable-next-line` comment exists precisely to allow removing `folderImport.start` from the dep list. The fix is to omit it:
```suggestion
useEffect(() => {
if (folderImportTrigger === 0) return;
void folderImport.start();
// We intentionally depend only on the counter — folderImport.start's
// identity changes every render (new hook instance per render) and
// we don't want to restart the flow on those changes.
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [folderImportTrigger]);
```
Additionally, to prevent `start` from changing on every render, consider memoizing the `options` object passed to `useFolderFirstImport`:
```typescript
const folderImportOptions = useMemo(() => ({
onSuccess: () => toast.success("Project ready — open it from the sidebar."),
onError: (message: string) => toast.error(`Import failed: ${message}`),
}), []);
const folderImport = useFolderFirstImport(folderImportOptions);
```
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: apps/desktop/src/renderer/routes/_authenticated/_dashboard/pending/$pendingId/page.tsx
Line: 199-224
Comment:
**Pending row stays stuck in "creating" state if the user cancels the Pin & set up modal**
When `PROJECT_NOT_SETUP` is caught, the pending row is left in `"creating"` state and `openPinAndSetup` is called with only an `onSuccess` callback. If the user cancels the modal (closes it without completing setup), the `AddRepositoryModals` host component calls `close()`, which resets the Zustand store's `active` to `none` — but nothing ever flips the pending row to `"failed"`. The user is then stuck on the spinning "Creating workspace..." view with no automatic way out except the "Dismiss" button.
The QA checklist states: _"On setup cancel, pending row resolves to a clean terminal state."_ The current code does not satisfy this.
The store's `openPinAndSetup` signature only accepts `onSuccess`. An `onCancel` option needs to be threaded through:
1. Add `onCancel?: () => void` to `AddRepositoryModalState.openPinAndSetup` opts and the `pin-and-setup` active kind.
2. In `AddRepositoryModals`, call `active.onCancel?.()` inside the `onOpenChange` handler when `!open`.
3. At the call site in `pending/page.tsx`, provide an `onCancel` that flips the pending row to `"failed"`:
```typescript
openPinAndSetup(
{ id: projectId, ... },
{
onSuccess: () => void fire(),
onCancel: () => {
collections.pendingWorkspaces.update(pendingId, (draft) => {
draft.status = "failed";
draft.error = "Project setup cancelled — workspace creation was not completed.";
});
},
},
);
```
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: packages/host-service/src/trpc/router/project/handlers.ts
Line: 42-57
Comment:
**Cloud project row created before local clone — no rollback on clone failure**
`createFromClone` calls `ctx.api.v2Project.create.mutate(...)` first, then `cloneRepoInto(...)`. If the clone fails (directory collision, network error, auth failure), the cloud `v2Projects` row is permanently orphaned. The comment says this is "recoverable via `project.setup`," but there is no compensating delete call.
Consider reversing the order — clone first, verify the remote, then create the cloud row — so a clone failure leaves no cloud-side state. If cloud-create must come first, at minimum attempt a best-effort delete on clone failure:
```typescript
try {
const resolved = await cloneRepoInto(args.url, args.parentDir);
persistLocalProject(ctx, cloudProject.id, resolved);
return { projectId: cloudProject.id, repoPath: resolved.repoPath };
} catch (err) {
await ctx.api.v2Project.delete?.mutate({ id: cloudProject.id }).catch(() => {});
throw err;
}
```
(Noted in follow-ups as "Orphaned cloud rows TTL cleanup" — flagging here so it's on the reviewer's radar.)
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: packages/trpc/src/router/v2-project/v2-project.ts
Line: 158-215
Comment:
**No slug uniqueness check within an organization**
`v2Projects.create` inserts a row without verifying that `slug` is unique within `organizationId`. If the DB schema has a unique constraint on `(organizationId, slug)`, a duplicate will surface as a raw constraint violation rather than a user-friendly message. If there's no constraint, duplicate slugs can accumulate silently.
Consider an explicit pre-flight check:
```typescript
const existing = await dbWs.query.v2Projects.findFirst({
columns: { id: true },
where: and(
eq(v2Projects.organizationId, input.organizationId),
eq(v2Projects.slug, input.slug),
),
});
if (existing) {
throw new TRPCError({
code: "CONFLICT",
message: `A project with slug "${input.slug}" already exists in this organization`,
});
}
```
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "feat(desktop): workspace-create inline s..." | Re-trigger Greptile
- notification-manager.test: update expected strings to match source (strings changed in #3039; test wasn't updated, CI was red on main too) - DashboardSidebarHeader: aria-label="Add repository" on icon-only dropdown triggers so screen readers announce them (tooltips don't count as accessible names) - docs/design/v2-project-create-import: correct v2Projects.create input shape (jwt-scoped { organizationId, name, slug, repoCloneUrl })
There was a problem hiding this comment.
2 issues found across 14 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/routes/_authenticated/_dashboard/v2-workspaces/components/V2WorkspacesList/V2WorkspacesList.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/components/V2WorkspacesList/V2WorkspacesList.tsx:117">
P2: The new `hasAnyWorkspaces` check uses search-filtered data, so empty search results are mislabeled as "No workspaces yet" instead of a filter/no-match state.</violation>
</file>
<file name="docs/design/v2-project-create-import.md">
<violation number="1" location="docs/design/v2-project-create-import.md:152">
P2: This doc now describes Available/inline-setup flows as out-of-scope, which conflicts with the PR’s shipped behavior and makes the design spec inaccurate.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Previously any workspace whose hostMachineId didn't match the local machine landed on a WorkspaceNotOnThisHostState stub. That hid the workspace from the user entirely when the whole point is to let them see it. Delete the gate, delete the stub component, and let the workspace page render for any host. Operations that assume local filesystem (terminal spawn, local git) fail at the point they run. Also slims the page's live query — projectGithubOwner, projectName, hostMachineId etc. were only fed into the stub. Design doc + plan updated to reflect the no-gating posture. Resolves saddlepaddle CTvC.
|
You're iterating quickly on this pull request. To help protect your rate limits, cubic has paused automatic reviews on new pushes for now—when you're ready for another review, comment |
Addresses saddlepaddle CWlq — the shared style for the three top-of-modal pickers (Device / Project / Branch) lived as a string constant in types.ts, which is an odd place for a className and doesn't compose. Promote it to a named FormPickerTrigger component that encapsulates the base button styles and accepts extra className + native button props. The three call sites lose their raw <button type="button"> + backtick-composed classNames. Drops FORM_PICKER_TRIGGER_CLASS from types.ts.
PinAndSetupModal had zero remaining callers after the MVP cut — the pending-page PROJECT_NOT_SETUP interceptor and the Available-section "Pin & set up" button were the only two. Delete the whole modal, its store action, useOpenPinAndSetupModal hook, PinAndSetupTarget type, and the forceRepoint plumbing that existed only to support it. Also addresses the async-hygiene nits on the surviving surfaces: - useFolderFirstImport.start wraps selectDirectory.mutateAsync in try/catch → reportError (coderabbit nmS, cubic op5). - ParentDirectoryPicker.handleBrowse wraps the same (cubic op8). - AddRepositoryModals effect adds .catch on startRef.current() (cubic oqE). - FolderFirstImportModal keys CandidatePickerContent on repoPath so selectedId resets per import (coderabbit nmM). Docs + plan updated to reflect the removed modal + ENOENT recovery deferral. Net −205 lines. Typecheck + lint clean.
v1 has no re-point UX. project.setup now treats an existing row as: - same resolved path → no-op success (idempotent; fixes the false CONFLICT that cubic/coderabbit flagged on same-path setup). - different path → CONFLICT with the existing path in the message, no escape hatch. User must project.remove first if they genuinely want to move the project. Drops `acknowledgeWorkspaceInvalidation` from the input, the ack branch of the CONFLICT guard, and the setupFromClone/setupFromImport helpers + SetupContext type in handlers.ts (the setup path is small enough to inline). Client drops the confirm-repoint state, confirmRepoint method, ConfirmRepointContent component, and the conflict branch in SetupInvokeResult — none of which have anything to retry against. Also fixes the TOCTOU race in cloneRepoInto: replaces existsSync + rmSync-on-error with mkdirSync (atomic claim) + rmSync-on-error, so clone failure can't delete a directory this process didn't create. Resolves coderabbit nmb, nmd, and cubic oqN.
The onboarding "No workspaces yet" check was reading already-filtered pinned/others counts, so a search that matched nothing landed on the onboarding copy instead of the clear-filters UI. Collapse to a single !hasAnyMatches branch that picks copy + icon based on hasActiveFilters. Drops the bogus hasAnyWorkspaces check. Resolves cubic CrwE.
Replace the bespoke name + clone-URL + parent-picker form with v1's new-project page layout: a Location row (text input + browse button), three mode tiles (Clone/Empty/Template), and a per-mode form. Only Clone is wired up; Empty + Template carry "(coming soon)" since v2 project.create throws NOT_IMPLEMENTED for them. Location auto-populates to ~/.superset/projects via window.getHomeDir. Project name is derived from the clone URL's last segment so the form matches v1 (no explicit name field). ParentDirectoryPicker deleted — the inline Input + folder button replaces it and there's no other caller.
v2Projects previously required a non-null githubRepositoryId, which gated project creation on the org having installed the repo via the GitHub App. Cloning any other repo (public, not installed, or non- matching) failed at the cloud step after a successful local clone. Changes: - githubRepositoryId becomes nullable with ON DELETE SET NULL, matching v1's projects table. - repoCloneUrl is added as the canonical source of truth for the remote URL. Also nullable so empty-mode / local-only projects without a remote can coexist. - UNIQUE(organization_id, lower(repo_clone_url)) prevents two projects from claiming the same repo in one org. NULLs don't collide, so URL-less projects still work. - v2Project.create accepts an optional repoCloneUrl, canonicalizes via parseGitHubRemote, and links a matching github_repositories row case-insensitively when one exists. Unique-violation (23505) surfaces as CONFLICT with per-constraint messaging. - v2Project.findByGitHubRemote matches on v2Projects.repoCloneUrl directly instead of joining through the installation table, so unlinked projects are discoverable. - v2Project.get drops the derived repoCloneUrl — consumers read the stored column or the joined githubRepository directly. Migration 0034 bundles all five schema changes. Nullable-safe: no backfill required for existing rows.
…pace modal After picking a host in DevicePicker, each project in ProjectPickerPill shows an amber warning triangle when that host doesn't have the project set up locally. A matching "Project needs to be set up" note appears next to the ⌘↵ hint when the currently-selected project needs setup, so the user sees the blocker before submitting. Setup state comes from a per-host project.list query (re-added to the host-service router). The RPC is resolved through the standard getHostServiceClientByUrl path — local uses activeHostUrl, remote/cloud goes through the relay. If the host is unreachable we treat setup as unknown and hide the indicator rather than falsely flagging everything. Submit path is unchanged: picking a not-set-up project still fires workspace.create, which throws PROJECT_NOT_SETUP and surfaces as the existing toast. Inline setup UX is still deferred.
- git.ts: biome wants the ghMsg ternary wrapped; main's 27e243b added the catch block and the CI biome check caught it post-merge. - design doc: the workspaces tab code filters to hosts the user is linked to via v2_users_hosts, not every workspace in the org. Update wording to match what shipped; note teammate workspaces on unshared hosts are not surfaced in v1.
Plan is shipped — move per AGENTS.md rule 7 and drop the rewrite/history notes in both plan and design doc since the PR body is the canonical record of what was cut.
Captured in PR body instead.
Links
docs/design/v2-project-create-import.mdplans/20260417-v2-project-create-import-impl.mdSummary
v1 of the v2 project lifecycle. Two entry points (sidebar
+dropdown): New project (clone a GitHub URL) and Import existing folder. The workspaces tab lists the workspaces you're linked to across hosts, with chips for local/remote/cloud. The new-workspace modal now flags projects that aren't set up on the selected host with an amber warning.Several ambitious items from the original plan were cut to ship the least complex surface that still lets a user create and open a workspace. See "Design decisions" below.
How it works
Host-service (
packages/host-service/src/trpc/router/project/)project.create— discriminated-unionmode; shipsclone+importLocal.empty/templatethrowNOT_IMPLEMENTED. Clone is clone-first then cloud-register, with rollback on cloud failure.project.setup— discriminated-unionclone/import. Same-path = idempotent no-op; different-path =CONFLICT(v1 has no re-point escape hatch —project.removefirst).project.findByPath— validates git root, reads remote, forwards to cloudv2Projects.findByGitHubRemote.project.list,project.remove— standard.workspace.create/checkout/adoptthrowPROJECT_NOT_SETUPwhen this host has no row for the project. Renderer surfaces as a plain toast (no recovery modal).Cloud (
packages/trpc/src/router/v2-project/)v2Projects.githubRepositoryIdis now nullable (ON DELETE SET NULL).v2_projects.repo_clone_url(nullable) holds the canonical clone URL. Source of truth for cloning on other hosts; GitHub App install is optional enrichment that the cloud links opportunistically.UNIQUE (organization_id, lower(repo_clone_url))prevents duplicate projects per repo per org. Unique-violation on create →CONFLICTwith per-constraint messaging.v2Projects.findByGitHubRemotematches onrepoCloneUrl(not the installation join), so unlinked projects are discoverable.v2Projects.createaccepts an optionalrepoCloneUrl; canonicalizes viaparseGitHubRemote; links a matching install when one exists. Previously threw when the repo wasn't installed via GitHub App — this no longer gates.Desktop
AddRepositoryModals/— mounted at dashboard layout level.NewProjectModal(v1 new-project UI as a modal — Location + Clone tile; Empty/Template "coming soon") andFolderFirstImportModal(picker state machine:idle/no-match).LuTriangleAlertwhen the host doesn't have it set up. "Project needs to be set up" hint appears next to ⌘↵ when the selected project has the warning. Submit path unchanged.v2_users_hosts.Manual QA
UNIQUEindex; picker code removed)PROJECT_NOT_SETUPtoast when submitting against unsetup projectTesting
bun run typecheck— greenbun run lint— greenDesign decisions
v2_host_projectscloud signal. Earlier drafts tracked per-host project backing in cloud. Cut; backing is local-only and checked at action time.project.setupstep inside New Workspace modal. Ifworkspace.createthrowsPROJECT_NOT_SETUP, surface as plain toast — no modal recovery loop.create/setup.repoCloneUrlas single column, not denormalized owner/name. v1 had all three; derivable on read viaparseGitHubRemote. Less drift.project.setupnever re-points in v1. Same-path no-op, different-path CONFLICT. Moving a project requiresproject.remove+ re-import.Known limitations
v2_users_hoststo avoid teammate-workspace noise for users who aren't linked to those hosts. Doc updated to match.findByGitHubRemotescope. If multi-org scoping lands later, the picker needs to be re-added.Follow-ups (deferred)
project.setuprecovery inside New Workspace modal.project.createempty/templatemodes (GitHub App provisioning).v2_projectsrow cleanup.Risks / Rollout
0034_v2_projects_decouple_github_install_add_clone_url.sql— nullablegithubRepositoryId, new nullablerepo_clone_url,ON DELETE SET NULL,UNIQUE (organization_id, lower(repo_clone_url)). Safe on any existing data (nullable add).githubRepositoryIdgoing back to NOT NULL would require a backfill if any rows were inserted with NULL. In practice all current rows have a linkedgithub_repositories, so rollback is just a reverse migration.