docs(desktop): v2 project create/import flow design#3521
Conversation
Adds three companion design docs for the v2 cloud-driven project flows: - v2-project-create-import.md — state matrix, host-service orchestration, backing derivation (local host-service + Electric remote-host), sidebar reconciliation with existing localStorage pins, phasing. - v2-project-create-import-types.md — type contracts, pointing at existing Drizzle/tRPC shapes and declaring only HostBacking + new host-service endpoints (project.list, project.create, project.setup extension). - v2-project-user-journeys.md — five scenarios walking the state machine (new user, joining existing org, second host, multi-host, offline host).
|
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 two new documents: a design spec for v2 project create/import flows introducing a per-host three-signal backing state, host-service RPC shapes, a new Electric-synced Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant HostService
participant CloudDB
participant HostFS
participant GitHub
Client->>HostService: project.create(params)
HostService->>CloudDB: create v2_projects row
alt provisioning supported
HostService->>GitHub: provision repo (optional)
GitHub-->>HostService: repo info
end
HostService->>HostFS: materialize local git (repoPath)
HostFS-->>HostService: pathStatus (valid / stale / missing)
HostService->>CloudDB: upsert v2_host_projects (projectId, hostId)
HostService-->>Client: respond with project id / state
Note right of Client: Later flows
Client->>HostService: project.setup (import/repair)
HostService->>HostFS: validate / repair workspace (acknowledge invalidation if needed)
HostService->>CloudDB: upsert/delete v2_host_projects as final backing signal
HostService-->>Client: setup result
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR adds three companion design documents for the v2 cloud-driven project create/import flows: a core flow doc with state matrix and RPC design, a type contracts doc, and a user journeys doc. The architecture is well-reasoned — host-service as sole orchestrator, two-source backing derivation (local host-service + Electric-synced remote hosts), and a clean Active/Available sidebar split with preserved pins. Key issues to resolve before implementation:
Confidence Score: 4/5Safe to merge as a design snapshot; the inconsistencies should be fixed before implementation begins. The overall architecture is sound and the three docs cover the design space well. The issues found are cross-doc inconsistencies (return type signatures, ensureMainWorkspace phasing, cell 4 repair scope) that don't block merging the docs themselves but do need to be reconciled before any engineer starts implementing against them — otherwise the two conflicting RPC contracts will cause confusion at the first implementation review. docs/design/v2-project-create-import.md requires the most attention — it is the source of truth for RPC shapes and the phasing plan, and currently disagrees with the types doc on return types and ensureMainWorkspace timing.
|
| Filename | Overview |
|---|---|
| docs/design/v2-project-create-import.md | Core flow design: state matrix, backing derivation, and RPC shapes — has internal inconsistencies: return types for project.create/project.setup omit mainWorkspaceId (contradicts types doc), ensureMainWorkspace phasing is ambiguous, and cell 4 repair contradicts "deferred" classification. |
| docs/design/v2-project-create-import-types.md | Type contracts doc: well-specified, reuses existing Drizzle/tRPC shapes correctly; mainWorkspaceId is correctly present in both endpoint return types, though this contradicts the flow doc's abbreviated signatures. |
| docs/design/v2-project-user-journeys.md | Five user journeys traced against the state machine; generally consistent with the design, though Journey 4 references an undesigned "switch host or set up here" UI page. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A([User Action]) --> B{Cloud v2_projects row?}
B -- No --> C["project.create\n(name, visibility, localPath, mode)"]
C --> D[Create cloud v2_projects row\n+ GitHub repo provisioning]
D --> E[Local git op\nclone / init+push / link+push]
E --> F[Upsert host-service.projects row\nwith repoPath + remote metadata]
F --> G[Create main workspace\nensureMainWorkspace]
G --> H([Cell 2: Fully Backed\nreturns projectId, repoPath, mainWorkspaceId])
B -- Yes --> I{host-service.projects row?}
I -- No --> J[Cell 1: Cloud-only\nAvailable list shows project]
J --> K["project.setup\n(projectId, mode, localPath)"]
K --> L[Clone or import\nlocal git op]
L --> M[Upsert host-service.projects row]
M --> N[Create main workspace\nif none exists]
N --> H
I -- Yes --> O{Disk repoPath valid?}
O -- Valid + match --> H
O -- Missing --> P[Cell 3: Stale path\nWarning dot in Active sidebar]
P --> Q["project.setup\n(acknowledgeWorkspaceInvalidation: true)"]
Q --> M
O -- Valid + mismatch --> R[Cell 4: Wrong remote\nDeferred — not surfaced Phase 1]
H --> S{Backing sources}
S --> T["Source A: local host-service\nproject.list → Set of projectIds"]
S --> U["Source B: Electric v2_workspaces ⋈ v2_hosts\nwhere isOnline=true AND machineId≠current"]
T --> V[backedProjectIds = union A ∪ B]
U --> V
V --> W{In v2SidebarProjects pin?}
W -- Yes + backed --> X([Active Sidebar])
W -- Yes + not backed --> Y([Available — pin preserved])
W -- No --> Z([Available — no pin])
Prompt To Fix All With AI
This is a comment left during a code review.
Path: docs/design/v2-project-create-import.md
Line: 95
Comment:
**Return type mismatch with types doc**
The flow doc specifies `project.create` returning `{ projectId, repoPath }`, but the companion types doc (`v2-project-create-import-types.md:94`) specifies `→ { projectId: string; repoPath: string; mainWorkspaceId: string }`.
The types doc explicitly documents that `project.create` creates the main workspace and returns `mainWorkspaceId` as part of the contract (backing derivation source B depends on this workspace being there). The flow doc's abbreviated signature omits this. One of the two should be updated to match, or the discrepancy will cause confusion when implementing against these specs.
```suggestion
→ { projectId, repoPath, mainWorkspaceId }
```
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: docs/design/v2-project-create-import.md
Line: 113
Comment:
**Return type mismatch with types doc**
Same issue as `project.create`: the flow doc shows `→ { repoPath }` but the types doc (`v2-project-create-import-types.md:105`) specifies `→ { repoPath: string; mainWorkspaceId: string }`.
Since `project.setup` is described as upholding `ensureMainWorkspace` ("Also creates the main workspace if none exists"), the `mainWorkspaceId` is part of the intended contract and should appear here too.
```suggestion
→ { repoPath, mainWorkspaceId }
```
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: docs/design/v2-project-create-import.md
Line: 235-237
Comment:
**`ensureMainWorkspace` enforcement deferred to Phase 2, but Source B relies on it in Phase 1**
Phase 2 is where you plan to enforce `ensureMainWorkspace` in `project.create`/`project.setup`, but the Phase 1 backing derivation for Source B already depends on this invariant being in place:
> _"Source B leans on a load-bearing invariant: every backing has ≥1 workspace… Without this, a freshly-backed remote host with no workspaces yet would read as unbacked."_ (main doc, Source B section)
The types doc also describes `project.create` as creating the main workspace unconditionally as part of its core logic (not as a Phase 2 addition). If `ensureMainWorkspace` enforcement is truly deferred to Phase 2, Phase 1's remote-backing detection via Source B will be unreliable for any backing that hasn't yet accumulated a workspace through other means. The phasing should be reconciled — either move the invariant enforcement into Phase 1 (as the types doc implies), or explicitly document a Phase 1 fallback for Source B.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: docs/design/v2-project-create-import.md
Line: 196
Comment:
**Cell 4 repair contradicts "deferred" classification in state matrix**
The state matrix at line 72 marks cell 4 (valid path, remote mismatch) as `"deferred — not surfaced in Phase 1"`, but this repair section states: _"Cells 3 and 4 live in Active with a warning dot."_
This creates an ambiguity: will cell 4 show a warning dot in Phase 1 or not? If detection of the mismatch condition itself is deferred (which seems to be the intent, since `project.setup` only prevents entering cell 4 at setup time, not detecting it retroactively), the repair description should read "Cell 3" only, with a note that cell 4 repair is planned for a future phase.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: docs/design/v2-project-user-journeys.md
Line: 60
Comment:
**"Switch host or set up here" page referenced but not designed**
Journey 4 introduces a "switch host or set up here" page triggered when a user clicks a workspace row labelled "remote," but this UI surface isn't described in any of the three companion docs. Given that this page is a decision point in the multi-host workflow (a key use case), it would be worth either stubbing out its design here or adding a reference to a follow-up design issue so it doesn't slip through the cracks during implementation.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "docs(desktop): v2 project create/import ..." | Re-trigger Greptile
🚀 Preview Deployment🔗 Preview Links
Preview updates automatically with new commits |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
docs/design/v2-project-user-journeys.md (2)
31-31: Consider the UX implications of the shared-workstation edge case.The edge case where a user has access to a teammate's host creates a potentially confusing state: the project appears in the sidebar (via remote backing) but clicking "New workspace" triggers an inline setup flow rather than direct creation.
Consider adding a visual indicator or tooltip to distinguish "backed on my machines" vs "backed on shared/teammate machines" to set appropriate expectations before the user clicks.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/design/v2-project-user-journeys.md` at line 31, The sidebar/project item UX should indicate when a project's remote backing is on a shared teammate host to avoid surprise inline setup; update the Project sidebar item (the UI component that renders the project in the sidebar) to show a distinct badge/icon plus tooltip that differentiates "backed on my machines" vs "backed on teammate/shared host", and update the "New workspace" button/handler (the logic that launches the inline setup flow) to surface that same explanatory state in the setup modal or confirmation copy so users know why inline setup will run; add the tooltip text and adjust the click path to surface the rationale before launching the inline setup.
3-3: Clarify "online flag" terminology.Line 3 mentions tracking an "online flag" per host, but doesn't specify the field name. Based on the schema discussion in the companion type contracts document, this should explicitly reference the field name (which needs verification—see related comment on the types doc).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/design/v2-project-user-journeys.md` at line 3, The phrase "online flag" is ambiguous—replace it with the concrete schema field name used in our types (e.g., the field on the host-service projects row) and verify that exact identifier against the companion type contracts document; update the sentence that currently lists "cloud `v2_projects` row, host-service `projects` row, localStorage pin, online flag" to name the exact field (for example `projects.online` or whatever the types doc defines) so readers know which property is tracked and add a brief parenthetical noting you verified it against the types doc.docs/design/v2-project-create-import.md (1)
108-108: Consider documenting cleanup for failedproject.createattempts.The "no rollback" design (line 108) means if cloud row creation succeeds but local git operations fail, the project remains in the cloud with no local backing. While the design treats this as "cell 1" (cloud-only), repeated failures could clutter the Available list with projects the user never successfully created.
Consider:
- Adding a
creationStatusorsetupStatusfield to track incomplete creates- Providing a way to delete/cleanup cloud projects that were never successfully backed
- Documenting the expected user flow for recovering from mid-create failures
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/design/v2-project-create-import.md` at line 108, The docs note "no rollback" for project.create which can leave cloud-only "cell 1" projects; add a lightweight creationStatus (or setupStatus) enum to the Project model and set it in project.create (e.g., "creating" -> "created" or "failed"), update project.setup to check and transition from "creating"/"failed" states, implement a cleanup path (an API endpoint or scheduled job) that can remove cloud-only projects with creationStatus="failed" older than a threshold, and update the doc to describe the user recovery flow using project.setup and the cleanup/deletion option for orphaned cloud rows.docs/design/v2-project-create-import-types.md (1)
64-70: Clarify the "online-only" filtering mechanism forHostBacking.Line 65 comments that
hostis "online-only" but the type definition doesn't show how offline hosts are filtered out. Since the actual schema uses astatusfield (not a simple boolean), specify:
- Which status values qualify as "online"
- Where the filtering occurs (in the Electric query from lines 48-52, or in the view model composition)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/design/v2-project-create-import-types.md` around lines 64 - 70, The comment on HostBacking is ambiguous about "online-only": update the comment for HostBacking.host to explicitly list which status values count as online (e.g., specify that only status === "healthy" qualifies, or if "unknown" should also be treated as online, state that explicitly) and state where the filtering actually happens (point to the Electric query that fetches v2Hosts by status and/or the view-model composition step that maps v2Hosts into HostBacking and filters by status). Reference the HostBacking type, the host property (SelectV2Host & { isCurrent: boolean }), the status field, the v2Workspaces source, and the Electric query or view-model composition that performs the filtering so readers can find the implementation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/design/v2-project-create-import.md`:
- Around line 233-238: The phasing doc risks a deployment gap where the backing
derivation (used by useDashboardSidebarData) is active in Phase 1 but the
load-bearing ensureMainWorkspace invariant isn't enforced until Phase 2; update
the plan to remove the window by moving ensureMainWorkspace enforcement into
Phase 1 (i.e., ensure project.create and project.setup call ensureMainWorkspace
before writing backing state) or explicitly state that existing
project.create/project.setup already enforce it and Phase 2 only adds extra
defensive checks; reference ensureMainWorkspace, project.create, project.setup,
backing derivation, and useDashboardSidebarData so reviewers can find and adjust
the relevant implementation notes.
In `@docs/design/v2-project-user-journeys.md`:
- Line 16: The table row showing "+ New project" currently marks Pin as "auto"
but the main design still lists open question `#5` about Auto-pin on
project.create / project.setup; update the table entry to reconcile this by
either changing the Pin cell to "auto (assumed)" or adding a short note in the
same row referencing open question `#5`, or if the decision has been made, update
the main doc to reflect that Auto-pin on project.create / project.setup is
confirmed and keep "auto"; ensure you reference the `project.create` and
`project.setup` actions when making the change so reviewers can trace the
decision.
---
Nitpick comments:
In `@docs/design/v2-project-create-import-types.md`:
- Around line 64-70: The comment on HostBacking is ambiguous about
"online-only": update the comment for HostBacking.host to explicitly list which
status values count as online (e.g., specify that only status === "healthy"
qualifies, or if "unknown" should also be treated as online, state that
explicitly) and state where the filtering actually happens (point to the
Electric query that fetches v2Hosts by status and/or the view-model composition
step that maps v2Hosts into HostBacking and filters by status). Reference the
HostBacking type, the host property (SelectV2Host & { isCurrent: boolean }), the
status field, the v2Workspaces source, and the Electric query or view-model
composition that performs the filtering so readers can find the implementation.
In `@docs/design/v2-project-create-import.md`:
- Line 108: The docs note "no rollback" for project.create which can leave
cloud-only "cell 1" projects; add a lightweight creationStatus (or setupStatus)
enum to the Project model and set it in project.create (e.g., "creating" ->
"created" or "failed"), update project.setup to check and transition from
"creating"/"failed" states, implement a cleanup path (an API endpoint or
scheduled job) that can remove cloud-only projects with creationStatus="failed"
older than a threshold, and update the doc to describe the user recovery flow
using project.setup and the cleanup/deletion option for orphaned cloud rows.
In `@docs/design/v2-project-user-journeys.md`:
- Line 31: The sidebar/project item UX should indicate when a project's remote
backing is on a shared teammate host to avoid surprise inline setup; update the
Project sidebar item (the UI component that renders the project in the sidebar)
to show a distinct badge/icon plus tooltip that differentiates "backed on my
machines" vs "backed on teammate/shared host", and update the "New workspace"
button/handler (the logic that launches the inline setup flow) to surface that
same explanatory state in the setup modal or confirmation copy so users know why
inline setup will run; add the tooltip text and adjust the click path to surface
the rationale before launching the inline setup.
- Line 3: The phrase "online flag" is ambiguous—replace it with the concrete
schema field name used in our types (e.g., the field on the host-service
projects row) and verify that exact identifier against the companion type
contracts document; update the sentence that currently lists "cloud
`v2_projects` row, host-service `projects` row, localStorage pin, online flag"
to name the exact field (for example `projects.online` or whatever the types doc
defines) so readers know which property is tracked and add a brief parenthetical
noting you verified it against the types doc.
🪄 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: f132646a-c9f6-45a5-a838-32e2d090107c
📒 Files selected for processing (3)
docs/design/v2-project-create-import-types.mddocs/design/v2-project-create-import.mddocs/design/v2-project-user-journeys.md
There was a problem hiding this comment.
4 issues found across 3 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="docs/design/v2-project-create-import.md">
<violation number="1" location="docs/design/v2-project-create-import.md:95">
P2: Return type is missing `mainWorkspaceId` — the companion types doc (`v2-project-create-import-types.md:94`) specifies `→ { projectId: string; repoPath: string; mainWorkspaceId: string }`. Since `project.create` enforces `ensureMainWorkspace` and backing derivation Source B depends on that workspace existing, the return type here should match.</violation>
<violation number="2" location="docs/design/v2-project-create-import.md:114">
P2: Return type is missing `mainWorkspaceId` — the companion types doc (`v2-project-create-import-types.md:105`) specifies `→ { repoPath: string; mainWorkspaceId: string }`. Since `project.setup` upholds `ensureMainWorkspace`, the return type should include it here as well.</violation>
<violation number="3" location="docs/design/v2-project-create-import.md:196">
P2: Cell 4 is classified as "deferred — not surfaced in Phase 1" in the state matrix (line 72), but this repair section includes it: "Cells 3 and 4 live in Active with a warning dot." Since cell 4 detection is deferred and `project.setup` only prevents *entering* cell 4 (not detecting it retroactively), this should reference only Cell 3, with a note that cell 4 repair is planned for a future phase.</violation>
<violation number="4" location="docs/design/v2-project-create-import.md:236">
P1: `ensureMainWorkspace` invariant enforcement is deferred to Phase 2 here, but Phase 1's backing derivation Source B explicitly depends on it: "every backing has ≥1 workspace… Without this, a freshly-backed remote host with no workspaces yet would read as unbacked." The types doc also describes `project.create` as creating the main workspace unconditionally (not as a Phase 2 addition). Either move `ensureMainWorkspace` enforcement into Phase 1 (as the types doc implies), or document a Phase 1 fallback for Source B when no workspace exists yet.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
docs/design/v2-project-create-import.md (3)
90-92: Clarify the "no cloud-only mode" statement in relation to failure states.Line 90 states "
project.createalways materializes on the calling host. No 'cloud-only' mode." However, line 92 immediately describes a failure scenario where "Cloud row created but local clone fails → project is in cell 1," and cell 1 is labeled "Cloud-only" (line 38).This is potentially confusing. The intent appears to be: there is no intentional cloud-only mode (you can't call
project.createwith a flag to skip local materialization), but failure can leave you in cell 1.✏️ Suggested rewording
-**Always materializes on the calling host.** No "cloud-only" mode. Other hosts use `project.setup`. +**Always attempts local materialization.** There is no intentional "cloud-only" mode; `project.create` always tries to back the project on the calling host. Other hosts use `project.setup` to materialize their own backing. **No rollback on mid-flow failure.** Cloud row created but local clone fails → project is in cell 1. User retries via `project.setup`. Cell 1 is a first-class state, not a failure mode.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/design/v2-project-create-import.md` around lines 90 - 92, Reword the section to distinguish intentional modes from failure states: clarify that project.create always attempts local materialization on the calling host (there is no supported "cloud-only" flag or operational mode), but transient failures during create can leave the record in "cell 1" (the cloud-only state) until the user retries project.setup to complete local cloning; reference project.create, project.setup and "cell 1"/Cloud-only state explicitly so readers understand the difference between intentional cloud-only behavior (none) and accidental cloud-only state caused by failures.
34-41: Document the missing cell 4 in the state matrix.The state table jumps from cell 3 to cell 5, skipping cell 4. Cell 4 would represent the invalid state where cloud
v2_projectsrow is missing buthost-service.projectsrow exists. While this is presumably impossible by design (projects originate in cloud), explicitly documenting why cell 4 is omitted would improve clarity.📝 Suggested addition
After line 41, add:
| 5 | ✗ | — | — | Brand new | `project.create` | + +Cell 4 (cloud missing, host row exists) is impossible: all projects originate via cloud row creation in `project.create`, and `project.setup` validates cloud presence.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/design/v2-project-create-import.md` around lines 34 - 41, Add an explicit entry or explanatory note for the missing "cell 4" in the Cells state matrix: state where cloud v2_projects row is missing but host-service.projects row exists. Update the table or add a sentence after the table (referencing "Cells", "v2_projects", and "host-service.projects") clarifying that this state is impossible by design because projects originate in the cloud, so a host-only row cannot exist independently; therefore cell 4 is intentionally omitted and requires no repair action.
120-120: Consider whetherpathStatusshould distinguish "exists but not a git root" from "missing".The cell 3 definition (line 40) specifies that
repoPathon disk should be a "valid git root" vs "missing". However,pathStatusonly checks "healthy" vs "missing" viastatSync, which doesn't validate git-ness.If a path exists but is not (or no longer) a valid git repository, is that state "healthy" or "missing"? The current design treats it as "healthy" (path exists), but cell 3's repair flow expects "valid git root" semantics.
Options:
- Accept that
statSyncis sufficient (filesystem presence is enough; git validity checked later during actual operations)- Extend
pathStatusto"healthy" | "invalid" | "missing"where "invalid" = exists but not a git root- Clarify in comments that "healthy" only means "path exists", not "valid git root"
💡 Suggested clarification
If option 1 (current behavior), add clarification:
- pathStatus: "healthy" | "missing" // statSync(repoPath) at read time + pathStatus: "healthy" | "missing" // statSync(repoPath) at read time; "healthy" = path exists (git validity checked lazily during operations)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/design/v2-project-create-import.md` at line 120, The current pathStatus only reflects filesystem presence but cell 3 expects git-root semantics; update the design to distinguish these states by changing pathStatus to "healthy" | "invalid" | "missing" and modify the statSync logic (where pathStatus is set) to run a git-root validation (e.g., a helper like isGitRoot or git rev-parse check) so that "invalid" means the path exists but is not a valid git repository; keep repoPath and cell 3 repair flow aligned to use the new "invalid" state.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/design/v2-project-create-import.md`:
- Line 105: The param acknowledgeWorkspaceInvalidation is declared optional but
the comment and repair flow require an explicit acknowledgement when a projects
row already exists; change the declaration to a required boolean
(acknowledgeWorkspaceInvalidation: boolean) and add runtime validation in the
create/import routine (the repair discriminator logic that checks for existing
projects row) to throw or return a clear error if
acknowledgeWorkspaceInvalidation is false or missing, or alternatively update
the comment to precisely state "must be true when projects row already exists
and path is being re-pointed" if you choose to keep it optional—reference the
acknowledgeWorkspaceInvalidation symbol and the repair-discriminator check that
enforces caller acknowledgement.
- Line 85: The document references an important invariant called
ensureMainWorkspace but never formally defines it; add a clear formal definition
(e.g., a short subsection titled "ensureMainWorkspace invariant") that states:
for every backed project row in host-service.projects there MUST exist at least
one workspace row and one of those workspaces is designated as the main
workspace (include how the main workspace is identified—e.g., a boolean flag or
a specific workspace.role value—and the expected constraints/behavior on
creation, deletion, and uniqueness), and reference the invariant name
ensureMainWorkspace throughout the doc so implementers can rely on that exact
contract.
- Around line 184-191: The row-state conditions are ambiguous and can overlap;
update the doc to state that conditions are evaluated in a first-match-wins
order and declare the priority explicitly (e.g., Stale path > Host offline > Not
set up here > Normal), or rewrite the Normal condition to be mutually exclusive
(e.g., require local backing healthy OR no local missing and any online remote)
so entries like `pathStatus: "missing"` with an online remote fall under Stale
path rather than Normal; reference the table headers and the specific states
"Normal", "Stale path", "Host offline", and "Not set up here" when making the
change.
---
Nitpick comments:
In `@docs/design/v2-project-create-import.md`:
- Around line 90-92: Reword the section to distinguish intentional modes from
failure states: clarify that project.create always attempts local
materialization on the calling host (there is no supported "cloud-only" flag or
operational mode), but transient failures during create can leave the record in
"cell 1" (the cloud-only state) until the user retries project.setup to complete
local cloning; reference project.create, project.setup and "cell 1"/Cloud-only
state explicitly so readers understand the difference between intentional
cloud-only behavior (none) and accidental cloud-only state caused by failures.
- Around line 34-41: Add an explicit entry or explanatory note for the missing
"cell 4" in the Cells state matrix: state where cloud v2_projects row is missing
but host-service.projects row exists. Update the table or add a sentence after
the table (referencing "Cells", "v2_projects", and "host-service.projects")
clarifying that this state is impossible by design because projects originate in
the cloud, so a host-only row cannot exist independently; therefore cell 4 is
intentionally omitted and requires no repair action.
- Line 120: The current pathStatus only reflects filesystem presence but cell 3
expects git-root semantics; update the design to distinguish these states by
changing pathStatus to "healthy" | "invalid" | "missing" and modify the statSync
logic (where pathStatus is set) to run a git-root validation (e.g., a helper
like isGitRoot or git rev-parse check) so that "invalid" means the path exists
but is not a valid git repository; keep repoPath and cell 3 repair flow aligned
to use the new "invalid" state.
🪄 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: f0ef1ff7-31ab-4136-b83c-be1a0176cd11
📒 Files selected for processing (1)
docs/design/v2-project-create-import.md
…to-workspace - Drop ensureMainWorkspace invariant: neither project.create nor project.setup auto-seeds a main workspace. Workspace creation is always explicit user action. - Replace workspace-derived remote backing with a direct cloud-synced v2_host_projects table (one row per projectId × hostId that backs it). Electric-synced to the desktop, written by project.create/setup/remove. - User-facing intent split: project.create = clone new project, project.setup = import existing or repair stale path. - Phase 1 punch list now leads with the new table + Electric sync + cloud router.
There was a problem hiding this comment.
2 issues found across 24 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/hooks/useFolderFirstImport/useFolderFirstImport.ts">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/hooks/useFolderFirstImport/useFolderFirstImport.ts:117">
P2: Handle errors from `selectDirectory.mutateAsync` to avoid unhandled promise rejections and ensure users see an actionable error.
(Based on your team's feedback about handling async rejections explicitly.) [FEEDBACK_USED]</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:39">
P1: `visibility` is accepted in the input schema but never forwarded to `createFromClone` or `createFromImportLocal` (and neither handler passes it to the cloud `v2Project.create` API). The user's private/public choice is silently discarded.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| .input( | ||
| z.object({ | ||
| name: z.string().min(1), | ||
| visibility: z.enum(["private", "public"]), |
There was a problem hiding this comment.
P1: visibility is accepted in the input schema but never forwarded to createFromClone or createFromImportLocal (and neither handler passes it to the cloud v2Project.create API). The user's private/public choice is silently discarded.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/host-service/src/trpc/router/project/project.ts, line 39:
<comment>`visibility` is accepted in the input schema but never forwarded to `createFromClone` or `createFromImportLocal` (and neither handler passes it to the cloud `v2Project.create` API). The user's private/public choice is silently discarded.</comment>
<file context>
@@ -1,53 +1,131 @@
+ .input(
+ z.object({
+ name: z.string().min(1),
+ visibility: z.enum(["private", "public"]),
+ mode: z.discriminatedUnion("kind", [
+ z.object({
</file context>
| return; | ||
| } | ||
|
|
||
| const picked = await selectDirectory.mutateAsync({ |
There was a problem hiding this comment.
P2: Handle errors from selectDirectory.mutateAsync to avoid unhandled promise rejections and ensure users see an actionable error.
(Based on your team's feedback about handling async rejections explicitly.)
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/_dashboard/v2-workspaces/hooks/useFolderFirstImport/useFolderFirstImport.ts, line 117:
<comment>Handle errors from `selectDirectory.mutateAsync` to avoid unhandled promise rejections and ensure users see an actionable error.
(Based on your team's feedback about handling async rejections explicitly.) </comment>
<file context>
@@ -0,0 +1,215 @@
+ return;
+ }
+
+ const picked = await selectDirectory.mutateAsync({
+ title: "Import existing folder",
+ });
</file context>
Both the folder-first import flow and the Pin & set up modal call project.setup, which throws TRPCError CONFLICT when a host-service.projects row for the target projectId already exists (re-pointing is destructive — it invalidates existing workspace worktrees under the old path). Surface that as a second-step confirmation instead of a raw error toast. useFolderFirstImport: adds a "confirm-repoint" state branched from the 1-match auto-advance and the pick-candidate path. Modal renders a destructive "Re-point anyway" step with the project name and target path. Retrying passes acknowledgeWorkspaceInvalidation: true. PinAndSetupModal: same form stays open but flips to re-point mode — title and description switch, submit becomes a destructive "Re-point anyway" that retries with the ack flag. Both paths use TRPCClientError.data?.code === "CONFLICT" to detect the server-side guard. Any other error still bubbles out as a toast.
Biome flagged aria-label on a bare <span> in the dot-only backing indicator — added role="img" so the label is valid on the element. Also dropped two noNonNullAssertion warnings: - useFolderFirstImport: destructure candidates[0] via [only, ...rest] with a runtime check instead of `!`. - resolve-repo: iterate the remotes map via iterator.next() and throw an INTERNAL_SERVER_ERROR on the unreachable empty case rather than asserting non-null. Plus Biome's import-order auto-fixes across the add-repository modal files.
There was a problem hiding this comment.
3 issues found across 20 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/components/AddRepositoryModals/AddRepositoryModals.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/AddRepositoryModals/AddRepositoryModals.tsx:47">
P1: The effect dependency list includes `folderImport.start`, which is not stable here and can repeatedly re-trigger `start()` after a single trigger bump. Depend only on `folderImportTrigger` for this counter-based pulse effect.
(Based on your team's feedback about narrowing React effect dependencies to required fields.) [FEEDBACK_USED]</violation>
</file>
<file name="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/components/ParentDirectoryPicker/ParentDirectoryPicker.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/components/ParentDirectoryPicker/ParentDirectoryPicker.tsx:24">
P2: Wrap the `mutateAsync` call in `try/catch` to prevent unhandled promise rejections when directory selection RPC fails.
(Based on your team's feedback about handling async calls/errors explicitly and avoiding silent failures.) [FEEDBACK_USED]</violation>
</file>
<file name="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/components/NewProjectModal/NewProjectModal.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/components/NewProjectModal/NewProjectModal.tsx:47">
P2: Include host availability in `canSubmit`; otherwise submit can appear enabled but do nothing when no active host is selected.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| // 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, folderImport.start]); |
There was a problem hiding this comment.
P1: The effect dependency list includes folderImport.start, which is not stable here and can repeatedly re-trigger start() after a single trigger bump. Depend only on folderImportTrigger for this counter-based pulse effect.
(Based on your team's feedback about narrowing React effect dependencies to required fields.)
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/_dashboard/components/AddRepositoryModals/AddRepositoryModals.tsx, line 47:
<comment>The effect dependency list includes `folderImport.start`, which is not stable here and can repeatedly re-trigger `start()` after a single trigger bump. Depend only on `folderImportTrigger` for this counter-based pulse effect.
(Based on your team's feedback about narrowing React effect dependencies to required fields.) </comment>
<file context>
@@ -0,0 +1,76 @@
+ // 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, folderImport.start]);
+
+ return (
</file context>
| const selectDirectory = electronTrpc.window.selectDirectory.useMutation(); | ||
|
|
||
| const handleBrowse = async () => { | ||
| const result = await selectDirectory.mutateAsync({ |
There was a problem hiding this comment.
P2: Wrap the mutateAsync call in try/catch to prevent unhandled promise rejections when directory selection RPC fails.
(Based on your team's feedback about handling async calls/errors explicitly and avoiding silent failures.)
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/_dashboard/v2-workspaces/components/ParentDirectoryPicker/ParentDirectoryPicker.tsx, line 24:
<comment>Wrap the `mutateAsync` call in `try/catch` to prevent unhandled promise rejections when directory selection RPC fails.
(Based on your team's feedback about handling async calls/errors explicitly and avoiding silent failures.) </comment>
<file context>
@@ -0,0 +1,49 @@
+ const selectDirectory = electronTrpc.window.selectDirectory.useMutation();
+
+ const handleBrowse = async () => {
+ const result = await selectDirectory.mutateAsync({
+ title: dialogTitle,
+ defaultPath: value ?? undefined,
</file context>
| trimmedName.length > 0 && | ||
| trimmedUrl.length > 0 && | ||
| parentDir !== null && | ||
| !working; |
There was a problem hiding this comment.
P2: Include host availability in canSubmit; otherwise submit can appear enabled but do nothing when no active host is selected.
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/_dashboard/v2-workspaces/components/NewProjectModal/NewProjectModal.tsx, line 47:
<comment>Include host availability in `canSubmit`; otherwise submit can appear enabled but do nothing when no active host is selected.</comment>
<file context>
@@ -0,0 +1,147 @@
+ trimmedName.length > 0 &&
+ trimmedUrl.length > 0 &&
+ parentDir !== null &&
+ !working;
+
+ const reset = () => {
</file context>
Three cleanups for sidebar project rows:
1. "Not set up here" gets an inline Set up CTA. Expanded rows replace
the "+ new workspace" button with a compact amber "Set up" button
that opens the Pin & set up modal with the project pre-filled via
useOpenPinAndSetupModal. Collapsed rows route the thumbnail button
to the same modal when the project is unbacked, with an updated
tooltip explaining the behavior. The "Not here" indicator label is
suppressed in this state so the CTA is the only visual signal —
button and label together were redundant.
2. Host-type chip on workspace rows. Small muted pill ("Cloud" /
"Other device") rendered next to the branch on the secondary line,
only for non-local workspaces. local-device stays unlabelled to
keep the common case visually quiet.
3. Host-offline and Not-set-up-here tooltip copy tightened to be
single-sentence and directly explain either the passive resolution
or the Set up action.
Phase 2 per plans/20260417-v2-project-create-import-impl.md — the
remote-device workspace row "switch host or set up here" stub is
deferred to Phase 3.
| <DashboardSidebarProjectRow | ||
| projectName={project.name} | ||
| githubOwner={project.githubOwner} | ||
| backingState={project.backingState} |
There was a problem hiding this comment.
WE should be able to remove this backing state as a concept from most places - instead, we can query if a host is online to see if the project is already set up in the new workspace creation modal and the eventual cron logic (which'll probably also share the project selector, so maybe we can consolidate some of this in a project selector component)?
| // Remote backing — v2_host_projects ⋈ v2_hosts, excluding rows for the | ||
| // current machine (current host's backing is covered by localProjectList | ||
| // above, which is authoritative and lag-free). | ||
| const { data: remoteBackingRows = EMPTY_REMOTE_BACKING_ROWS } = useLiveQuery( |
There was a problem hiding this comment.
the v2HostProjects can probably be removed
There was a problem hiding this comment.
along with most of this logic, in favor of just doing an innerJoin on v2Hosts on workspaces
| @@ -0,0 +1,280 @@ | |||
| import { Button } from "@superset/ui/button"; | |||
There was a problem hiding this comment.
seems like this component is added to the wrong directory? should be unrelated to v2-workspaces
| onSuccess, | ||
| onError, | ||
| }: NewProjectModalProps) { | ||
| const { activeHostUrl } = useLocalHostService(); |
There was a problem hiding this comment.
nit: may or may not need to provide an escape hatch to users for setting up projects on remote machines
| @@ -0,0 +1,49 @@ | |||
| import { Button } from "@superset/ui/button"; | |||
There was a problem hiding this comment.
another one where we may need to move the file location to match where it's mounted
| }); | ||
| const hasAnyAccessible = pinned.length > 0 || others.length > 0; | ||
|
|
||
| const openNewProject = useOpenNewProjectModal(); |
There was a problem hiding this comment.
all probably needs to be mounted at the dashboard level
| }), | ||
|
|
||
| create: protectedProcedure | ||
| findByRemote: jwtProcedure |
There was a problem hiding this comment.
nit: name might need to be clearer? maybe find by githubRemote?
| return !!row; | ||
| } | ||
|
|
||
| export async function requireHostAccess( |
There was a problem hiding this comment.
nit, this pattern kinda results in extra queries so we should probably start moving away from it (unless it encapsulates complex logic like it probably will later)
There was a problem hiding this comment.
or actually jk maybe it's fine, we can just pass in a txid when we need it to be in one network round trip or w/e
Phase 3
- Host-service workspaceCreation.{create,checkout,adopt} now throw a
structured PROJECT_NOT_SETUP error (TRPCError PRECONDITION_FAILED
with cause { kind, projectId }) instead of silently auto-cloning
into ~/.superset/repos/. The error formatter in packages/host-service
trpc/index.ts surfaces the cause as data.projectNotSetup so the
renderer can read it off TRPCClientError. Error type lives in
trpc/error-types.ts alongside TeardownFailureCause.
- Pending workspace-create page catches data.projectNotSetup and opens
the Pin & set up modal via useOpenPinAndSetupModal, registering a
one-shot onSuccess callback that retries the intent automatically.
The store gained an `onSuccess` slot on the pin-and-setup active
modal to support this, invoked by AddRepositoryModals host.
- Remote-device workspace rows route to a new stub page
WorkspaceNotOnThisHostState that explains "workspace lives on <host>"
and offers "Set up here" (opens Pin & set up) or "Browse workspaces".
The V2 workspace page checks the workspace's host.machineId against
the current machine via a live query and renders the stub before
mounting the pane tree, which would otherwise crash on a foreign
worktree.
Phase 4
- Host-service project.list now returns pathStatus: "healthy" | "missing"
via a cheap statSync probe on each row. The renderer's
useDashboardSidebarData adds a 30s refetchInterval so out-of-band
directory deletions surface without requiring the user to trigger an
operation that fails.
- Fourth backing state "stale-path" added to DashboardSidebarProjectBackingState.
deriveBackingState prioritizes local pathStatus over remote backing: a
stale local path wins over a teammate's copy because the user clearly
wanted this project set up here and should fix it.
- Sidebar project rows render a destructive "Repair" button for
stale-path (replacing the "+ new workspace" action). Collapsed rows
route the thumbnail click to the repair action. Both open the Pin &
set up modal with a new forceRepoint flag that pre-sets the
destructive re-point state so the first submit sends
acknowledgeWorkspaceInvalidation: true (skips the usual two-step
conflict → confirm flow).
- Biome also auto-formatted some touched files; no functional changes.
There was a problem hiding this comment.
2 issues found across 17 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/pending/$pendingId/page.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/pending/$pendingId/page.tsx:179">
P2: Canceling the Pin & set up modal leaves the pending workspace stuck in `creating` because this flow only retries on success and never marks failure on cancel.</violation>
</file>
<file name="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/components/PinAndSetupModal/PinAndSetupModal.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/components/PinAndSetupModal/PinAndSetupModal.tsx:54">
P2: `conflict` is initialized from `forceRepoint` only once, so re-point mode can become stale across modal reopens when `forceRepoint` changes.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| githubOwner: repo?.owner ?? null, | ||
| githubRepoName: repo?.name ?? null, | ||
| }, | ||
| { onSuccess: () => void fire() }, |
There was a problem hiding this comment.
P2: Canceling the Pin & set up modal leaves the pending workspace stuck in creating because this flow only retries on success and never marks failure on cancel.
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/_dashboard/pending/$pendingId/page.tsx, line 179:
<comment>Canceling the Pin & set up modal leaves the pending workspace stuck in `creating` because this flow only retries on success and never marks failure on cancel.</comment>
<file context>
@@ -144,6 +147,39 @@ function useFireIntent(pendingId: string, pending: PendingWorkspaceRow | null) {
+ githubOwner: repo?.owner ?? null,
+ githubRepoName: repo?.name ?? null,
+ },
+ { onSuccess: () => void fire() },
+ );
+ return;
</file context>
| // destructive submit button that retries with the ack flag set. | ||
| // `forceRepoint` pre-sets this for the stale-path repair flow so the user | ||
| // doesn't have to submit once just to see the CONFLICT and re-submit. | ||
| const [conflict, setConflict] = useState(forceRepoint); |
There was a problem hiding this comment.
P2: conflict is initialized from forceRepoint only once, so re-point mode can become stale across modal reopens when forceRepoint changes.
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/_dashboard/v2-workspaces/components/PinAndSetupModal/PinAndSetupModal.tsx, line 54:
<comment>`conflict` is initialized from `forceRepoint` only once, so re-point mode can become stale across modal reopens when `forceRepoint` changes.</comment>
<file context>
@@ -46,14 +49,16 @@ export function PinAndSetupModal({
- const [conflict, setConflict] = useState(false);
+ // `forceRepoint` pre-sets this for the stale-path repair flow so the user
+ // doesn't have to submit once just to see the CONFLICT and re-submit.
+ const [conflict, setConflict] = useState(forceRepoint);
const canSubmit = project !== null && parentDir !== null && !working;
</file context>
Summary
Three companion design docs for the v2 cloud-driven project create/import flows:
v2-project-create-import.md— state matrix (cloud / host-service / disk / remote), host-service as the single orchestrator, backing derivation via two sources (local host-serviceproject.list+ Electricv2_workspaces ⋈ v2_hostsfor remote online hosts), sidebar reconciliation with existing localStorage pin collections, phased rollout.v2-project-create-import-types.md— type contracts. Reuses existing Drizzle/tRPC shapes; only introducesHostBackingas a client-side composition plus new host-service endpoints (project.list,project.create, additiveacknowledgeWorkspaceInvalidationonproject.setup).v2-project-user-journeys.md— five scenarios walked against the state machine: new user in new org, joining an existing org, adding a second host, multi-host workflow, host going offline.No code changes — design only.
Test plan
Summary by cubic
Implements Phases 1–4 of the v2 cloud-driven project create/import: backing-aware sidebar and workspaces, centralized create/import/setup modals, inline Set up/Repair flows, and a remote‑host workspace stub. Projects derive backing from the local host-service and the cloud
v2_host_projectstable; workspace actions prompt setup when needed and retry automatically.New Features
v2_host_projects(schema, migration, relations, Electric sync) with tRPCv2HostProject.upsert/delete;v2Project.findByRemote; host-serviceproject.create/setupsupport clone/import, persist local project, upsert cloud host backing, and surfacePROJECT_NOT_SETUP;project.listreturnspathStatusfor stale‑path detection.AddRepositoryModalshost for New project, Folder‑first Import, and Pin & set up (supports conflict re‑point and force re‑point); sidebar shows backing viaProjectBackingStateIndicatorwith inline “Set up” for unbacked and “Repair” for stale paths (collapsed rows route clicks accordingly); Workspaces tab adds “Available projects” and host‑type chips; remote‑device workspaces render a “Not on this device” stub with a Set up CTA; pending workspace creation catchesPROJECT_NOT_SETUP, opens Pin & set up, and retries on success.Bug Fixes
v2Project.createandv2Project.findByRemote.Written for commit 610706b. Summary will update on new commits.
Summary by CodeRabbit