feat(desktop): port v1 projects + workspaces into v2#3670
Merged
saddlepaddle merged 2 commits intomainfrom Apr 23, 2026
Merged
Conversation
First-time v2 launch now migrates the user's v1 local data into v2 cloud + local stores and surfaces a branded summary modal. Covers projects (dedup via GitHub remote, link-or-create), worktrees (adopt into v2_workspaces with legacy-path support), sections (including empty ones), and sidebar ordering with v1→v2 normalization. Idempotent across runs via a new v1_migration_state table. Followups tracked: SUPER-469, SUPER-470, SUPER-471.
Contributor
📝 WalkthroughWalkthroughIntroduces a V1→V2 migration system: migration spec doc, TRPC migration router, migration hook and UI modal, host-service adopt verification, DB table to track per-org migration state, sidebar-order translation, and extensive tests covering control flow and idempotency. Changes
Sequence DiagramsequenceDiagram
participant React as React Hook<br/>(useMigrateV1DataToV2)
participant TRPC as TRPC Server<br/>(migration router)
participant DB as Local DB<br/>(v1_migration_state)
participant HostSvc as Host Service<br/>(workspace-creation)
participant Modal as React Modal<br/>(V1MigrationSummaryModal)
React->>React: Check session marker and org context
React->>TRPC: Fetch v1 projects, workspaces, worktrees, sections
TRPC-->>React: v1 entities
React->>TRPC: Fetch migration state for org
TRPC->>DB: Query v1_migration_state
DB-->>TRPC: existing state rows
TRPC-->>React: state by v1 ID
rect rgba(100, 150, 255, 0.5)
Note over React: Per-project loop
React->>React: Match v1 project to v2 by path
React->>HostSvc: Project setup or create (import-local)
HostSvc-->>React: v2 projectId or error
React->>TRPC: Upsert migration state (project)
TRPC->>DB: Insert/update v1_migration_state
end
rect rgba(150, 200, 100, 0.5)
Note over React: Per-workspace loop
React->>HostSvc: Workspace adopt (maybe with worktreePath)
HostSvc-->>React: adopted or error
React->>TRPC: Upsert migration state (workspace)
TRPC->>DB: Insert/update v1_migration_state
end
React->>React: Normalize v1→v2 tab ordering
React->>React: Write v2 sidebar collections (idempotent)
React->>React: Persist MigrationSummary to localStorage
React->>Modal: Dispatch v1-migration-summary-updated
Modal->>Modal: Read localStorage and render summary
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 introduces a one-time, idempotent v1→v2 migration that runs automatically on first Electron launch when v2 is enabled. It reads the user's v1 local-db (projects, workspaces, worktrees, sections), creates or links the corresponding v2 cloud + host-service records, translates the sidebar layout into three PGlite collections, and presents a branded summary modal. Two P1 issues need fixes before this ships to avoid user-facing sidebar corruption and silent fatal failures. Confidence Score: 3/5Not safe to merge — the section-duplication bug silently corrupts the v2 sidebar on every app relaunch after migration. The P1 section-duplication issue is a data-integrity problem that worsens with each launch (ghost sidebar sections multiply). The silent org-conflict failure leaves users with no actionable recovery path. Both are straightforward to fix but must land before this ships. writeSidebarState.ts (section idempotency) and useMigrateV1DataToV2.ts (fatal error surfacing)
|
| Filename | Overview |
|---|---|
| apps/desktop/src/renderer/routes/_authenticated/hooks/useMigrateV1DataToV2/writeSidebarState.ts | Translates v1 sidebar state into v2 PGlite collections — section idempotency is broken because fresh UUIDs are generated on every call, causing duplicate sections on every app relaunch |
| apps/desktop/src/renderer/routes/_authenticated/hooks/useMigrateV1DataToV2/useMigrateV1DataToV2.ts | Hook orchestrator with good session-marker dedup logic, but fatal org-conflict errors are silently swallowed with no user feedback |
| apps/desktop/src/renderer/routes/_authenticated/hooks/useMigrateV1DataToV2/migrate.ts | Core migration orchestrator — handles project/workspace dedup, orphan detection, and per-row error recording correctly; well-tested |
| apps/desktop/src/lib/trpc/routers/migration/index.ts | New tRPC router for v1 reads and migration_state CRUD; findMigrationByOtherOrg does an in-process filter but the table will be tiny in practice |
| packages/host-service/src/trpc/router/workspace-creation/workspace-creation.ts | Adds optional worktreePath to adopt; new findWorktreeAtPath helper correctly validates git worktree registration but may misparse on Windows CRLF output |
| packages/local-db/src/schema/schema.ts | Adds v1_migration_state table with composite PK on (v1_id, kind) and appropriate indexes; migration SQL and snapshot match the schema definition |
| apps/desktop/src/renderer/routes/_authenticated/hooks/useMigrateV1DataToV2/normalize.ts | Tab-order normalization (workspaces-before-sections) is clean, well-commented, and thoroughly tested |
| apps/desktop/src/renderer/routes/_authenticated/components/V1MigrationSummaryModal/V1MigrationSummaryModal.tsx | Branded summary modal with expandable sections; non-unique React list keys for project/workspace entries could cause reconciliation issues with duplicate names |
Sequence Diagram
sequenceDiagram
participant Layout as DashboardLayout
participant Hook as useMigrateV1DataToV2
participant Migrate as migrateV1DataToV2
participant ElecTRPC as electronTrpcClient (main)
participant HS as HostService
participant PGlite as PGlite Collections
Layout->>Hook: mount
Hook->>Hook: check sessionStorage marker
Hook->>ElecTRPC: readV1Projects / readV1Workspaces / readV1Worktrees / readV1WorkspaceSections
Hook->>ElecTRPC: listState + findMigrationByOtherOrg
ElecTRPC-->>Hook: v1 data + existing migration_state
alt other org already migrated
Migrate-->>Hook: throw Error (silently caught — no UI feedback)
else normal path
loop each v1 project skip success/linked
Migrate->>HS: project.findByPath then create or setup
Migrate->>ElecTRPC: upsertState success/linked/error
end
loop each v1 workspace skip if parent errored or orphan
Migrate->>HS: workspaceCreation.adopt with optional worktreePath
Migrate->>ElecTRPC: upsertState success/skipped/error
end
Migrate->>PGlite: writeV2SidebarState projects + sections + workspaces
Note over PGlite: sections get fresh UUIDs every call — duplicates on relaunch
Migrate-->>Hook: MigrationSummary
Hook->>Hook: persistSummary to localStorage if didAnything
Hook->>Layout: dispatchEvent V1_MIGRATION_SUMMARY_EVENT
Layout->>Layout: V1MigrationSummaryModal shown
end
Comments Outside Diff (4)
-
apps/desktop/src/renderer/routes/_authenticated/hooks/useMigrateV1DataToV2/writeSidebarState.ts, line 2488-2504 (link)Section entries duplicated on every app relaunch
Each call to
writeV2SidebarStategenerates a freshcrypto.randomUUID()for every v1 section, then uses that brand-new UUID as the idempotency guard key — so the guard can never fire. The function is called unconditionally at the end ofmigrateV1DataToV2, including relaunches where all projects/workspaces are already insuccess/linkedstate. Since PGlite sidebar collections persist in localStorage across Electron sessions, each relaunch appends another full set of duplicate section entries with new UUIDs, while workspacesidebarState.sectionIdreferences remain pinned to the first migration's UUIDs — rendering the duplicates as ghost sections.A straightforward fix is to gate the
writeV2SidebarStatecall on whether anything was newly written (skip when all state rows are alreadysuccess/linked), or persist the v1→v2 section-ID mapping inmigration_stateso subsequent calls can use stable keys.Prompt To Fix With AI
This is a comment left during a code review. Path: apps/desktop/src/renderer/routes/_authenticated/hooks/useMigrateV1DataToV2/writeSidebarState.ts Line: 2488-2504 Comment: **Section entries duplicated on every app relaunch** Each call to `writeV2SidebarState` generates a fresh `crypto.randomUUID()` for every v1 section, then uses that brand-new UUID as the idempotency guard key — so the guard can never fire. The function is called unconditionally at the end of `migrateV1DataToV2`, including relaunches where all projects/workspaces are already in `success/linked` state. Since PGlite sidebar collections persist in localStorage across Electron sessions, each relaunch appends another full set of duplicate section entries with new UUIDs, while workspace `sidebarState.sectionId` references remain pinned to the first migration's UUIDs — rendering the duplicates as ghost sections. A straightforward fix is to gate the `writeV2SidebarState` call on whether anything was newly written (skip when all state rows are already `success/linked`), or persist the v1→v2 section-ID mapping in `migration_state` so subsequent calls can use stable keys. How can I resolve this? If you propose a fix, please make it concise.
-
apps/desktop/src/renderer/routes/_authenticated/hooks/useMigrateV1DataToV2/useMigrateV1DataToV2.ts, line 2017-2023 (link)When
findMigrationByOtherOrgreturns a non-null value,migrateV1DataToV2throws an error that propagates into the outercatchblock here, which only callsconsole.errorand clears the session marker. The user gets no visible feedback that migration is permanently blocked, and because the session marker is cleared, the hook retries silently on every component re-mount. Consider surfacing this specific error class to the UI (e.g. via a toast or by storing the error message alongside the summary in localStorage).Prompt To Fix With AI
This is a comment left during a code review. Path: apps/desktop/src/renderer/routes/_authenticated/hooks/useMigrateV1DataToV2/useMigrateV1DataToV2.ts Line: 2017-2023 Comment: **Fatal org-conflict error is silently swallowed** When `findMigrationByOtherOrg` returns a non-null value, `migrateV1DataToV2` throws an error that propagates into the outer `catch` block here, which only calls `console.error` and clears the session marker. The user gets no visible feedback that migration is permanently blocked, and because the session marker is cleared, the hook retries silently on every component re-mount. Consider surfacing this specific error class to the UI (e.g. via a toast or by storing the error message alongside the summary in localStorage). How can I resolve this? If you propose a fix, please make it concise.
-
packages/host-service/src/trpc/router/workspace-creation/workspace-creation.ts, line 2578-2580 (link)findWorktreeAtPathblank-line detection may fail on WindowsThe stanza separator in
git worktree list --porcelainis an empty line. The parser resetscurrentPathonline === "", but on Windows git uses CRLF. After splitting on"\n", the separator becomes"\r", not"", socurrentPathis never reset and path state can bleed between stanzas. Consider usingline.trim() === ""instead.Prompt To Fix With AI
This is a comment left during a code review. Path: packages/host-service/src/trpc/router/workspace-creation/workspace-creation.ts Line: 2578-2580 Comment: **`findWorktreeAtPath` blank-line detection may fail on Windows** The stanza separator in `git worktree list --porcelain` is an empty line. The parser resets `currentPath` on `line === ""`, but on Windows git uses CRLF. After splitting on `"\n"`, the separator becomes `"\r"`, not `""`, so `currentPath` is never reset and path state can bleed between stanzas. Consider using `line.trim() === ""` instead. How can I resolve this? If you propose a fix, please make it concise.
-
apps/desktop/src/renderer/routes/_authenticated/components/V1MigrationSummaryModal/V1MigrationSummaryModal.tsx, line 607-616 (link)Non-unique React keys for list entries
Project entries are keyed on
p.name, which collides when two v1 projects share the same name. The same issue affects workspace entries at lines 635–644, where the key combinesw.nameandw.branch. Using a stable index or including the v1 id in the summary would avoid reconciliation warnings and potential rendering glitches.Prompt To Fix With AI
This is a comment left during a code review. Path: apps/desktop/src/renderer/routes/_authenticated/components/V1MigrationSummaryModal/V1MigrationSummaryModal.tsx Line: 607-616 Comment: **Non-unique React keys for list entries** Project entries are keyed on `p.name`, which collides when two v1 projects share the same name. The same issue affects workspace entries at lines 635–644, where the key combines `w.name` and `w.branch`. Using a stable index or including the v1 id in the summary would avoid reconciliation warnings and potential rendering glitches. 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/hooks/useMigrateV1DataToV2/writeSidebarState.ts
Line: 2488-2504
Comment:
**Section entries duplicated on every app relaunch**
Each call to `writeV2SidebarState` generates a fresh `crypto.randomUUID()` for every v1 section, then uses that brand-new UUID as the idempotency guard key — so the guard can never fire. The function is called unconditionally at the end of `migrateV1DataToV2`, including relaunches where all projects/workspaces are already in `success/linked` state. Since PGlite sidebar collections persist in localStorage across Electron sessions, each relaunch appends another full set of duplicate section entries with new UUIDs, while workspace `sidebarState.sectionId` references remain pinned to the first migration's UUIDs — rendering the duplicates as ghost sections.
A straightforward fix is to gate the `writeV2SidebarState` call on whether anything was newly written (skip when all state rows are already `success/linked`), or persist the v1→v2 section-ID mapping in `migration_state` so subsequent calls can use stable keys.
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/hooks/useMigrateV1DataToV2/useMigrateV1DataToV2.ts
Line: 2017-2023
Comment:
**Fatal org-conflict error is silently swallowed**
When `findMigrationByOtherOrg` returns a non-null value, `migrateV1DataToV2` throws an error that propagates into the outer `catch` block here, which only calls `console.error` and clears the session marker. The user gets no visible feedback that migration is permanently blocked, and because the session marker is cleared, the hook retries silently on every component re-mount. Consider surfacing this specific error class to the UI (e.g. via a toast or by storing the error message alongside the summary in localStorage).
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/workspace-creation/workspace-creation.ts
Line: 2578-2580
Comment:
**`findWorktreeAtPath` blank-line detection may fail on Windows**
The stanza separator in `git worktree list --porcelain` is an empty line. The parser resets `currentPath` on `line === ""`, but on Windows git uses CRLF. After splitting on `"\n"`, the separator becomes `"\r"`, not `""`, so `currentPath` is never reset and path state can bleed between stanzas. Consider using `line.trim() === ""` instead.
```suggestion
} else if (line.trim() === "") {
currentPath = null;
}
```
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/components/V1MigrationSummaryModal/V1MigrationSummaryModal.tsx
Line: 607-616
Comment:
**Non-unique React keys for list entries**
Project entries are keyed on `p.name`, which collides when two v1 projects share the same name. The same issue affects workspace entries at lines 635–644, where the key combines `w.name` and `w.branch`. Using a stable index or including the v1 id in the summary would avoid reconciliation warnings and potential rendering glitches.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "feat(desktop): port v1 projects + worksp..." | Re-trigger Greptile
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (5)
apps/desktop/src/renderer/routes/_authenticated/components/V1MigrationSummaryModal/V1MigrationSummaryModal.tsx (1)
250-394: Split helper components out of this TSX file.
SummaryRow,ExpandableSummaryRow,EntryList, andEntrymake this a multi-component file. Move them into component folders/barrel exports or co-located child component files. As per coding guidelines,**/*.{tsx,ts}: Do not create multi-component files; use one component per file.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/components/V1MigrationSummaryModal/V1MigrationSummaryModal.tsx` around lines 250 - 394, This file contains multiple React components (SummaryRow, ExpandableSummaryRow, EntryList, Entry) that must be split into single-component files; create a new component file for each (e.g., SummaryRow.tsx, ExpandableSummaryRow.tsx, EntryList.tsx, Entry.tsx) exporting the default component and any needed prop types, move the corresponding function implementation and its props interface into each file, update the original V1MigrationSummaryModal.tsx to import these components from their new module paths and remove the in-file definitions, and ensure any shared utilities (cn, Badge, LuChevronDown/Right) remain imported where used and that types like SummaryRowProps/EntryProps are exported or redefined as needed for typing.apps/desktop/src/renderer/routes/_authenticated/hooks/useMigrateV1DataToV2/migrate.test.ts (3)
112-112: Nit:Omit<StateRow, "migratedAt">omits a nonexistent field.
StateRow(lines 42-49) has nomigratedAtproperty, so theOmitis a no-op. Either add the field toStateRowto mirror the real schema, or drop theOmit.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/hooks/useMigrateV1DataToV2/migrate.test.ts` at line 112, The test's mutate signature uses Omit<StateRow, "migratedAt"> but StateRow (in this file) does not declare a migratedAt field, so the Omit is a no-op; either add migratedAt to the StateRow type definition to mirror the real schema (e.g., add migratedAt?: string | number | Date depending on production schema) or simplify the test by removing the Omit and declare mutate as async (row: StateRow) => { ... }; update the mutate declaration in migrate.test.ts accordingly (referencing the mutate function and the StateRow type).
88-95: Deadvoidlocals insidemakeElectronTrpc.
createdV2s,hostProjects,hostWorkspacesare declared, voided, and never used — they look like leftovers from an earlier stub shape. Safe to drop.🧹 Proposed cleanup
function makeElectronTrpc(env: FakeEnv): ElectronTrpcClient { - const createdV2s: string[] = []; - const hostProjects = new Set<string>(); - const hostWorkspaces = new Set<string>(); - void createdV2s; - void hostProjects; - void hostWorkspaces; - const stub = {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/hooks/useMigrateV1DataToV2/migrate.test.ts` around lines 88 - 95, The test helper makeElectronTrpc declares unused locals createdV2s, hostProjects, and hostWorkspaces and immediately voids them; remove these dead variables and their void statements from makeElectronTrpc to clean up the function body and avoid leftover stubs.
230-509: Consider adding atype: "branch"workspace test.All adoption scenarios use
type: "worktree". Themigrate.tsbranch-path (line 229 onward) skips the worktree-presence check fortype: "branch"and passesworktreePath: undefinedtoadopt. A small test pinning that call shape would guard against future regressions in how branch-type workspaces flow through.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/hooks/useMigrateV1DataToV2/migrate.test.ts` around lines 230 - 509, Add a unit test exercising a v1 workspace with type: "branch" so the migrateV1DataToV2 branch-path that skips the worktree lookup is covered: create a fake env via makeFakeEnv with a v1Project and a workspace(...) whose metadata includes type: "branch" (and a branch name), invoke migrateV1DataToV2 (using makeElectronTrpc and makeHostService from the test helpers), and assert the migration summary indicates the workspace was handled (e.g., workspacesCreated or workspacesSkipped as appropriate) and that hostService.adopt was invoked for that workspace with worktreePath === undefined (or equivalent captured call args) to lock in the expected call shape through adopt.apps/desktop/src/renderer/routes/_authenticated/hooks/useMigrateV1DataToV2/migrate.ts (1)
99-119: Optional: fold the threeexistingStatepasses into one.Lines 99-119 iterate
existingStatethree times to buildstateByKey,projectV1ToV2, andworkspaceV1ToV2. A single pass would be marginally cheaper and easier to keep consistent (e.g., the status filter on workspaces at line 116 is absent for projects at line 108 — intentional, but the asymmetry is easier to spot inline).♻️ Proposed refactor
- const stateByKey = new Map<string, (typeof existingState)[number]>(); - for (const row of existingState) { - stateByKey.set(`${row.kind}:${row.v1Id}`, row); - } - - const worktreesById = new Map<string, (typeof v1Worktrees)[number]>(); - for (const wt of v1Worktrees) worktreesById.set(wt.id, wt); - - const projectV1ToV2 = new Map<string, string>(); - for (const row of existingState) { - if (row.kind === "project" && row.v2Id) { - projectV1ToV2.set(row.v1Id, row.v2Id); - } - } - - const workspaceV1ToV2 = new Map<string, string>(); - for (const row of existingState) { - if (row.kind === "workspace" && row.v2Id && row.status === "success") { - workspaceV1ToV2.set(row.v1Id, row.v2Id); - } - } + const stateByKey = new Map<string, (typeof existingState)[number]>(); + const projectV1ToV2 = new Map<string, string>(); + const workspaceV1ToV2 = new Map<string, string>(); + for (const row of existingState) { + stateByKey.set(`${row.kind}:${row.v1Id}`, row); + if (row.v2Id) { + if (row.kind === "project") projectV1ToV2.set(row.v1Id, row.v2Id); + else if (row.kind === "workspace" && row.status === "success") { + workspaceV1ToV2.set(row.v1Id, row.v2Id); + } + } + } + + const worktreesById = new Map<string, (typeof v1Worktrees)[number]>(); + for (const wt of v1Worktrees) worktreesById.set(wt.id, wt);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/hooks/useMigrateV1DataToV2/migrate.ts` around lines 99 - 119, Replace the three passes over existingState with a single loop that populates stateByKey, projectV1ToV2 and workspaceV1ToV2 in one iteration: iterate existingState once, always set stateByKey.set(`${row.kind}:${row.v1Id}`, row), and inside that same loop, if row.kind === "project" && row.v2Id then set projectV1ToV2.set(row.v1Id, row.v2Id), and if row.kind === "workspace" && row.v2Id && row.status === "success" then set workspaceV1ToV2.set(row.v1Id, row.v2Id); keep worktreesById initialization separate as it uses v1Worktrees.
🤖 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/plans/20260422-2100-v1-to-v2-port.md`:
- Around line 91-114: The fenced pseudocode blocks in this file are missing a
language tag and trigger markdownlint MD040; update each triple-backtick fence
that contains the pseudocode (for example the block starting with parsed_url =
parseGitHubRemote(origin_url_from_main_repo_path) and the later block showing
apps/desktop/.../MigrateV1Data/) to use a language tag such as text (e.g.,
```text) so the fences are correctly labeled; ensure all other pseudocode fences
(including the block covering the v2Project.create /
v2Project.findByGitHubRemote / migration_state logic and the later block
mentioned in the comment) are similarly updated.
- Around line 21-23: Update the migration plan text under the "**Dropped (no v2
destination or no user value):**" section and the other outdated sections that
describe earlier scope so it matches the shipped behavior: state that migration
runs automatically on first-time V2 launch, that workspace sections and
sidebar/tab order are preserved and migrated, and that the summary modal is
shown; remove or revise statements claiming "no auto-run", "no section
migration", or "no tab/sidebar-order migration" and align rationale with the
actual implementation tracked in SUPER-469/470 where branch-prefix/branch-base
handling is discussed.
In `@apps/desktop/src/lib/trpc/routers/migration/index.ts`:
- Around line 75-83: The upsert's conflict target currently uses
(v1MigrationState.v1Id, v1MigrationState.kind) which is global and can cause
cross-organization overwrites; update the onConflictDoUpdate target to include
organizationId (e.g., [v1MigrationState.v1Id, v1MigrationState.kind,
v1MigrationState.organizationId]) and ensure the underlying v1_migration_state
table has a unique constraint/primary key that includes organizationId so the
database-level uniqueness matches the application upsert semantics (adjust any
migration/schema where the constraint is defined).
In
`@apps/desktop/src/renderer/routes/_authenticated/components/V1MigrationSummaryModal/V1MigrationSummaryModal.tsx`:
- Around line 172-203: The mapping for summary.projects and summary.workspaces
uses unstable keys (key={`project-${p.name}`} and
key={`workspace-${w.name}-${w.branch}`}) that can collide and cause React to
reuse wrong rows; update the Entry key generation in the map callbacks to use a
stable unique identifier from each item (e.g., p.id or w.id) when present, and
if those ids are not available fall back to a deterministic indexed key (e.g.,
include the array index or a generated stable hash) so keys remain unique and
stable across renders for the Entry components inside the summary.projects.map
and summary.workspaces.map loops.
- Around line 123-145: The dialog in V1MigrationSummaryModal renders the title
and subtitle visually but lacks semantic accessibility; import DialogTitle and
DialogDescription from `@superset/ui/dialog` and replace or wrap the visual title
("Welcome to Superset v2") with <DialogTitle> and the subtitle ("We imported
your v1 data") with <DialogDescription>, preserving existing styling (or apply
className="sr-only" on those components if the strings are meant to be purely
visual); ensure the DialogTitle/DialogDescription appear inside the same
DialogContent container so assistive tech can associate them with the Dialog.
In
`@apps/desktop/src/renderer/routes/_authenticated/hooks/useMigrateV1DataToV2/migrate.ts`:
- Around line 136-148: The code currently silently uses found.candidates[0] when
multiple v2 projects match a v1 path; update the block that sets v2ProjectId and
calls hostService.project.setup.mutate to detect when found.candidates.length >
1 and record/emit a warning instead of silently picking the first: log a warning
including the list of candidate ids (or call the existing logger) and append a
descriptive reason to the migration_state record (or set a `reason` field
alongside status="linked") before proceeding; keep the existing fallback
behavior (attempt setup on candidate.id) but ensure the multiple-candidates
condition is recorded so it’s diagnosable later.
In
`@apps/desktop/src/renderer/routes/_authenticated/hooks/useMigrateV1DataToV2/writeSidebarState.ts`:
- Around line 97-104: The current loop uses crypto.randomUUID() to create
v2SectionId which prevents reuse on reruns; instead derive or load a stable
v2SectionId for each v1Section so the lookup against
collections.v2SidebarSections works and duplicates are not created. Replace
crypto.randomUUID() in the sectionV1ToV2 assignment with a deterministic id (for
example a hash of `${v2ProjectId}:${v1Section.id}`) or first load a persisted
v1→v2 mapping before the loop and reuse any existing mapping; keep using
sectionV1ToV2, input.v1Sections, input.projectV1ToV2 and
collections.v2SidebarSections.insert but ensure the id is stable across runs.
In `@packages/local-db/drizzle/0041_v1_migration_state.sql`:
- Around line 5-9: The PRIMARY KEY currently uses (v1_id, kind) which allows
different organizations to collide; update the table definition to include
organization_id in the PK by changing PRIMARY KEY(`v1_id`, `kind`) to PRIMARY
KEY(`v1_id`, `kind`, `organization_id`) (and ensure the `organization_id` column
remains NOT NULL), then mirror this key change in the Drizzle schema definition
and its snapshot so code-level migrations and queries use the composite key of
v1_id + kind + organization_id.
In `@packages/local-db/src/schema/schema.ts`:
- Around line 258-260: The migration-state table primary key currently uses
(table.v1Id, table.kind) which is global; include table.organizationId in that
key so rows are namespaced per org: change primaryKey to use
[table.organizationId, table.v1Id, table.kind], and adjust any related indexes
(e.g., v1_migration_state_v2_id_idx) if they need organization scoping; then
regenerate the SQL migration/snapshot to reflect the new PK and update the
router conflict target to use the same column set (organizationId, v1Id, kind)
so UPSERTs and conflict handling align with the new schema.
---
Nitpick comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/components/V1MigrationSummaryModal/V1MigrationSummaryModal.tsx`:
- Around line 250-394: This file contains multiple React components (SummaryRow,
ExpandableSummaryRow, EntryList, Entry) that must be split into single-component
files; create a new component file for each (e.g., SummaryRow.tsx,
ExpandableSummaryRow.tsx, EntryList.tsx, Entry.tsx) exporting the default
component and any needed prop types, move the corresponding function
implementation and its props interface into each file, update the original
V1MigrationSummaryModal.tsx to import these components from their new module
paths and remove the in-file definitions, and ensure any shared utilities (cn,
Badge, LuChevronDown/Right) remain imported where used and that types like
SummaryRowProps/EntryProps are exported or redefined as needed for typing.
In
`@apps/desktop/src/renderer/routes/_authenticated/hooks/useMigrateV1DataToV2/migrate.test.ts`:
- Line 112: The test's mutate signature uses Omit<StateRow, "migratedAt"> but
StateRow (in this file) does not declare a migratedAt field, so the Omit is a
no-op; either add migratedAt to the StateRow type definition to mirror the real
schema (e.g., add migratedAt?: string | number | Date depending on production
schema) or simplify the test by removing the Omit and declare mutate as async
(row: StateRow) => { ... }; update the mutate declaration in migrate.test.ts
accordingly (referencing the mutate function and the StateRow type).
- Around line 88-95: The test helper makeElectronTrpc declares unused locals
createdV2s, hostProjects, and hostWorkspaces and immediately voids them; remove
these dead variables and their void statements from makeElectronTrpc to clean up
the function body and avoid leftover stubs.
- Around line 230-509: Add a unit test exercising a v1 workspace with type:
"branch" so the migrateV1DataToV2 branch-path that skips the worktree lookup is
covered: create a fake env via makeFakeEnv with a v1Project and a workspace(...)
whose metadata includes type: "branch" (and a branch name), invoke
migrateV1DataToV2 (using makeElectronTrpc and makeHostService from the test
helpers), and assert the migration summary indicates the workspace was handled
(e.g., workspacesCreated or workspacesSkipped as appropriate) and that
hostService.adopt was invoked for that workspace with worktreePath === undefined
(or equivalent captured call args) to lock in the expected call shape through
adopt.
In
`@apps/desktop/src/renderer/routes/_authenticated/hooks/useMigrateV1DataToV2/migrate.ts`:
- Around line 99-119: Replace the three passes over existingState with a single
loop that populates stateByKey, projectV1ToV2 and workspaceV1ToV2 in one
iteration: iterate existingState once, always set
stateByKey.set(`${row.kind}:${row.v1Id}`, row), and inside that same loop, if
row.kind === "project" && row.v2Id then set projectV1ToV2.set(row.v1Id,
row.v2Id), and if row.kind === "workspace" && row.v2Id && row.status ===
"success" then set workspaceV1ToV2.set(row.v1Id, row.v2Id); keep worktreesById
initialization separate as it uses v1Worktrees.
🪄 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: 300deecc-18aa-4fef-8365-fc3d441dd533
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (19)
apps/desktop/plans/20260422-2100-v1-to-v2-port.mdapps/desktop/src/lib/trpc/routers/index.tsapps/desktop/src/lib/trpc/routers/migration/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/layout.tsxapps/desktop/src/renderer/routes/_authenticated/components/V1MigrationSummaryModal/V1MigrationSummaryModal.tsxapps/desktop/src/renderer/routes/_authenticated/components/V1MigrationSummaryModal/index.tsapps/desktop/src/renderer/routes/_authenticated/hooks/useMigrateV1DataToV2/index.tsapps/desktop/src/renderer/routes/_authenticated/hooks/useMigrateV1DataToV2/migrate.test.tsapps/desktop/src/renderer/routes/_authenticated/hooks/useMigrateV1DataToV2/migrate.tsapps/desktop/src/renderer/routes/_authenticated/hooks/useMigrateV1DataToV2/normalize.test.tsapps/desktop/src/renderer/routes/_authenticated/hooks/useMigrateV1DataToV2/normalize.tsapps/desktop/src/renderer/routes/_authenticated/hooks/useMigrateV1DataToV2/useMigrateV1DataToV2.tsapps/desktop/src/renderer/routes/_authenticated/hooks/useMigrateV1DataToV2/writeSidebarState.test.tsapps/desktop/src/renderer/routes/_authenticated/hooks/useMigrateV1DataToV2/writeSidebarState.tspackages/host-service/src/trpc/router/workspace-creation/workspace-creation.tspackages/local-db/drizzle/0041_v1_migration_state.sqlpackages/local-db/drizzle/meta/0041_snapshot.jsonpackages/local-db/drizzle/meta/_journal.jsonpackages/local-db/src/schema/schema.ts
Comment on lines
+21
to
+23
| **Dropped (no v2 destination or no user value):** | ||
|
|
||
| `color`, `tab_order`, `section_id`, `default_branch`, `workspace_base_branch`, `branch_prefix_mode`, `branch_prefix_custom`, `worktree_base_dir`, `neon_project_id`, `github_owner`, `icon_url`, `hide_image`, `config_toast_dismissed`, `is_unread`, `is_unnamed`, `deleting_at`, `port_base`, all timestamps, all `workspace_sections` rows. Rationale per field is in the sanity-check exploration (2026-04-22) — most are features v2 doesn't have yet; two (`workspace_base_branch`, `branch_prefix_*`) are tracked in SUPER-469/470. |
Contributor
There was a problem hiding this comment.
Update the plan to match the shipped migration behavior.
These sections still describe the earlier scope: no auto-run, no section migration, and no tab/sidebar-order migration. The PR now migrates on first-time V2 launch, preserves sections/sidebar order, and shows the summary modal, so this doc will mislead future maintenance unless refreshed.
Also applies to: 57-79, 219-224
🧰 Tools
🪛 LanguageTool
[uncategorized] ~23-~23: The official name of this software platform is spelled with a capital “H”.
Context: ...worktree_base_dir, neon_project_id, github_owner, icon_url, hide_image, `conf...
(GITHUB)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/plans/20260422-2100-v1-to-v2-port.md` around lines 21 - 23,
Update the migration plan text under the "**Dropped (no v2 destination or no
user value):**" section and the other outdated sections that describe earlier
scope so it matches the shipped behavior: state that migration runs
automatically on first-time V2 launch, that workspace sections and sidebar/tab
order are preserved and migrated, and that the summary modal is shown; remove or
revise statements claiming "no auto-run", "no section migration", or "no
tab/sidebar-order migration" and align rationale with the actual implementation
tracked in SUPER-469/470 where branch-prefix/branch-base handling is discussed.
Comment on lines
+91
to
+114
| ``` | ||
| parsed_url = parseGitHubRemote(origin_url_from_main_repo_path) | ||
|
|
||
| if parsed_url: | ||
| existing = v2Project.findByGitHubRemote({ organizationId, cloneUrl: parsed_url }) | ||
| if existing: | ||
| link: record (v1_id -> existing.id) in migration_state | ||
| if existing.repoCloneUrl is null: | ||
| v2Project.linkRepoCloneUrl({ id: existing.id, cloneUrl: parsed_url }) | ||
| continue | ||
| else: | ||
| try v2Project.create({ orgId, name, slug, repoCloneUrl: parsed_url }) | ||
| on success: record mapping, continue | ||
| on UNIQUE_VIOLATION(repo_clone_url): | ||
| # race: someone else in the org just created it | ||
| existing = v2Project.findByGitHubRemote(...) # should now hit | ||
| link to existing | ||
| on UNIQUE_VIOLATION(slug): | ||
| retry with slug-2, slug-3, ... (bounded: 10 attempts) | ||
| else: | ||
| # local-only repo, no dedup signal | ||
| v2Project.create({ orgId, name, slug, repoCloneUrl: null }) | ||
| on UNIQUE_VIOLATION(slug): retry with slug-N | ||
| ``` |
Contributor
There was a problem hiding this comment.
Add languages to fenced code blocks.
markdownlint flags these fences with MD040. Use text for the pseudocode/tree blocks unless a more specific language applies.
📝 Proposed doc lint fix
-```
+```text
parsed_url = parseGitHubRemote(origin_url_from_main_repo_path)
...@@
- +text
apps/desktop/src/renderer/routes/_authenticated/settings/.../MigrateV1Data/
...
Also applies to: 134-160
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 91-91: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/plans/20260422-2100-v1-to-v2-port.md` around lines 91 - 114, The
fenced pseudocode blocks in this file are missing a language tag and trigger
markdownlint MD040; update each triple-backtick fence that contains the
pseudocode (for example the block starting with parsed_url =
parseGitHubRemote(origin_url_from_main_repo_path) and the later block showing
apps/desktop/.../MigrateV1Data/) to use a language tag such as text (e.g.,
```text) so the fences are correctly labeled; ensure all other pseudocode fences
(including the block covering the v2Project.create /
v2Project.findByGitHubRemote / migration_state logic and the later block
mentioned in the comment) are similarly updated.
Comment on lines
+136
to
+148
| if (found.candidates.length > 0) { | ||
| const candidate = found.candidates[0]; | ||
| if (!candidate) throw new Error("findByPath returned empty candidate"); | ||
| v2ProjectId = candidate.id; | ||
| status = "linked"; | ||
| try { | ||
| await hostService.project.setup.mutate({ | ||
| projectId: candidate.id, | ||
| mode: { kind: "import", repoPath: project.mainRepoPath }, | ||
| }); | ||
| } catch (err) { | ||
| if (trpcCode(err) !== "CONFLICT") throw err; | ||
| } |
Contributor
There was a problem hiding this comment.
findByPath returning multiple candidates silently picks the first.
If a v1 repo path is registered under more than one v2 project, candidates[0] is a coin flip — and we then setup against that arbitrary project. Worth at least logging a warning (or recording a reason on the migration_state row) when candidates.length > 1 so this is diagnosable post-hoc.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/routes/_authenticated/hooks/useMigrateV1DataToV2/migrate.ts`
around lines 136 - 148, The code currently silently uses found.candidates[0]
when multiple v2 projects match a v1 path; update the block that sets
v2ProjectId and calls hostService.project.setup.mutate to detect when
found.candidates.length > 1 and record/emit a warning instead of silently
picking the first: log a warning including the list of candidate ids (or call
the existing logger) and append a descriptive reason to the migration_state
record (or set a `reason` field alongside status="linked") before proceeding;
keep the existing fallback behavior (attempt setup on candidate.id) but ensure
the multiple-candidates condition is recorded so it’s diagnosable later.
Comment on lines
+5
to
+9
| `organization_id` text NOT NULL, | ||
| `status` text NOT NULL, | ||
| `reason` text, | ||
| `migrated_at` integer NOT NULL, | ||
| PRIMARY KEY(`v1_id`, `kind`) |
Contributor
There was a problem hiding this comment.
Include organization_id in the migration-state primary key.
Line 9 keys rows only by (v1_id, kind), so the same V1 project/workspace cannot have independent migration state per organization. That conflicts with the per-org idempotency model and can cause an upsert for Org B to overwrite/block Org A’s state. Mirror the same key change in the Drizzle schema and snapshot.
🛠️ Proposed schema fix
- PRIMARY KEY(`v1_id`, `kind`)
+ PRIMARY KEY(`organization_id`, `v1_id`, `kind`)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| `organization_id` text NOT NULL, | |
| `status` text NOT NULL, | |
| `reason` text, | |
| `migrated_at` integer NOT NULL, | |
| PRIMARY KEY(`v1_id`, `kind`) | |
| `organization_id` text NOT NULL, | |
| `status` text NOT NULL, | |
| `reason` text, | |
| `migrated_at` integer NOT NULL, | |
| PRIMARY KEY(`organization_id`, `v1_id`, `kind`) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/local-db/drizzle/0041_v1_migration_state.sql` around lines 5 - 9,
The PRIMARY KEY currently uses (v1_id, kind) which allows different
organizations to collide; update the table definition to include organization_id
in the PK by changing PRIMARY KEY(`v1_id`, `kind`) to PRIMARY KEY(`v1_id`,
`kind`, `organization_id`) (and ensure the `organization_id` column remains NOT
NULL), then mirror this key change in the Drizzle schema definition and its
snapshot so code-level migrations and queries use the composite key of v1_id +
kind + organization_id.
Contributor
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
5 issues found across 20 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/lib/trpc/routers/migration/index.ts">
<violation number="1" location="apps/desktop/src/lib/trpc/routers/migration/index.ts:106">
P1: Cross-org migration guard ignores `linked` project states, so machines that only linked existing projects can migrate again into another organization.</violation>
</file>
<file name="apps/desktop/src/renderer/routes/_authenticated/hooks/useMigrateV1DataToV2/writeSidebarState.ts">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/hooks/useMigrateV1DataToV2/writeSidebarState.ts:101">
P1: Section migration is not idempotent: generating a new UUID per run guarantees duplicate section inserts on rerun.</violation>
</file>
<file name="apps/desktop/src/renderer/routes/_authenticated/hooks/useMigrateV1DataToV2/migrate.ts">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/hooks/useMigrateV1DataToV2/migrate.ts:162">
P2: The `upsertState` call inside the project `catch` block is not itself guarded. If this persistence fails, the exception escapes the loop and aborts migration for all remaining items. Wrap it in a try/catch (same applies to the two analogous sites in the workspace `catch` block).</violation>
</file>
<file name="packages/local-db/src/schema/schema.ts">
<violation number="1" location="packages/local-db/src/schema/schema.ts:259">
P1: Primary key for `v1_migration_state` is missing `organizationId`, which can cause cross-organization migration state collisions.</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:1408">
P2: Persist the normalized worktree path after explicit-path validation; storing raw `input.worktreePath` can save relative/non-canonical paths that later path-based workspace operations may mis-handle.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| } | ||
|
|
||
| projectV1ToV2.set(project.id, v2ProjectId); | ||
| await electronTrpc.migration.upsertState.mutate({ |
Contributor
There was a problem hiding this comment.
P2: The upsertState call inside the project catch block is not itself guarded. If this persistence fails, the exception escapes the loop and aborts migration for all remaining items. Wrap it in a try/catch (same applies to the two analogous sites in the workspace catch block).
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/hooks/useMigrateV1DataToV2/migrate.ts, line 162:
<comment>The `upsertState` call inside the project `catch` block is not itself guarded. If this persistence fails, the exception escapes the loop and aborts migration for all remaining items. Wrap it in a try/catch (same applies to the two analogous sites in the workspace `catch` block).</comment>
<file context>
@@ -0,0 +1,333 @@
+ }
+
+ projectV1ToV2.set(project.id, v2ProjectId);
+ await electronTrpc.migration.upsertState.mutate({
+ v1Id: project.id,
+ kind: "project",
</file context>
| message: `No git worktree registered at "${input.worktreePath}" on branch "${branch}"`, | ||
| }); | ||
| } | ||
| worktreePath = input.worktreePath; |
Contributor
There was a problem hiding this comment.
P2: Persist the normalized worktree path after explicit-path validation; storing raw input.worktreePath can save relative/non-canonical paths that later path-based workspace operations may mis-handle.
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/workspace-creation/workspace-creation.ts, line 1408:
<comment>Persist the normalized worktree path after explicit-path validation; storing raw `input.worktreePath` can save relative/non-canonical paths that later path-based workspace operations may mis-handle.</comment>
<file context>
@@ -1354,16 +1395,30 @@ export const workspaceCreationRouter = router({
+ message: `No git worktree registered at "${input.worktreePath}" on branch "${branch}"`,
+ });
+ }
+ worktreePath = input.worktreePath;
+ } else {
+ const { worktreeMap } = await listWorktreeBranches(
</file context>
Suggested change
| worktreePath = input.worktreePath; | |
| worktreePath = resolve(input.worktreePath); |
- Scope v1_migration_state PK to (organization_id, v1_id, kind) so migrating the same v1 row under different orgs keeps independent state. Regenerated 0041 in place (pre-ship, no chained migration needed). - Reuse v1 section id as v2 section id — deterministic mapping makes the rerun guard in writeV2SidebarState actually dedup; prior crypto.randomUUID would silently duplicate sections on every rerun. - Add sr-only DialogTitle + DialogDescription to V1MigrationSummaryModal for assistive tech; keep the visual welcome header as-is. - Stable React keys in summary entry lists (include array index). - Log warning when v2Project.findByGitHubRemote returns multiple candidates so the constraint-slip case is diagnosable post-hoc.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/desktop/src/renderer/routes/_authenticated/components/V1MigrationSummaryModal/V1MigrationSummaryModal.tsx (1)
28-39:⚠️ Potential issue | 🟡 MinorUse stable entry IDs instead of array indexes for keys.
Biome flags Lines 184 and 212. Add stable ids to the migration summary entries and key from those ids; index-based keys will keep failing lint and can reuse rows incorrectly if the lists change order.
Proposed direction
interface ProjectEntry { + id: string; name: string; status: ProjectStatus; reason?: string; } interface WorkspaceEntry { + id: string; name: string; branch: string; status: WorkspaceStatus; reason?: string; }- {summary.projects.map((p, index) => ( + {summary.projects.map((p) => ( <Entry - key={`project-${index}-${p.name}`} + key={`project-${p.id}`}- {summary.workspaces.map((w, index) => ( + {summary.workspaces.map((w) => ( <Entry - key={`workspace-${index}-${w.name}-${w.branch}`} + key={`workspace-${w.id}`}Also populate
idfrom the migration summary producer.Also applies to: 181-184, 210-212
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/components/V1MigrationSummaryModal/V1MigrationSummaryModal.tsx` around lines 28 - 39, The list rendering uses array indexes as React keys; update the types ProjectEntry and WorkspaceEntry to include a stable id field (e.g., id: string), change V1MigrationSummaryModal to use that id for key props instead of the index at the places referenced (around the rows at lines ~181-184 and ~210-212), and ensure the migration summary producer that builds these entries populates the id (stable unique value per project/workspace) so keys remain stable when order or contents change.
🧹 Nitpick comments (1)
apps/desktop/src/renderer/routes/_authenticated/components/V1MigrationSummaryModal/V1MigrationSummaryModal.tsx (1)
260-404: Split local components into separate component files.
SummaryRow,ExpandableSummaryRow,EntryList, andEntrymake this a multi-component TSX file. Please move them into co-located component files/barrels under this component folder.As per coding guidelines,
**/*.{tsx,ts}: Do not create multi-component files.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/components/V1MigrationSummaryModal/V1MigrationSummaryModal.tsx` around lines 260 - 404, This file contains multiple local React components (SummaryRow, ExpandableSummaryRow, EntryList, Entry) that must be split into separate co-located component files; create individual TSX files for each component (e.g., SummaryRow.tsx, ExpandableSummaryRow.tsx, EntryList.tsx, Entry.tsx) exporting their component and prop types, move the corresponding component code (preserving prop interfaces SummaryRowProps, ExpandableSummaryRowProps, EntryProps) into those files, update the original V1MigrationSummaryModal component to import these named exports and remove the local definitions, and ensure any used icons/utilities (cn, Badge, LuChevronDown/Right) remain imported where needed.
🤖 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/lib/trpc/routers/migration/index.ts`:
- Around line 100-114: The query in findMigrationByOtherOrg currently only
checks for v1MigrationState.status === "success", missing successful "linked"
project migrations; update the where/filter to include both statuses (e.g.
status is "success" OR "linked") while keeping the existing kind === "project"
check so v1MigrationState rows with status "linked" are treated as blocking the
migration; modify the v1MigrationState.where clause (used in
findMigrationByOtherOrg) to combine eq(v1MigrationState.kind, "project") with an
or(...) over eq(v1MigrationState.status, "success") and
eq(v1MigrationState.status, "linked").
---
Duplicate comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/components/V1MigrationSummaryModal/V1MigrationSummaryModal.tsx`:
- Around line 28-39: The list rendering uses array indexes as React keys; update
the types ProjectEntry and WorkspaceEntry to include a stable id field (e.g.,
id: string), change V1MigrationSummaryModal to use that id for key props instead
of the index at the places referenced (around the rows at lines ~181-184 and
~210-212), and ensure the migration summary producer that builds these entries
populates the id (stable unique value per project/workspace) so keys remain
stable when order or contents change.
---
Nitpick comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/components/V1MigrationSummaryModal/V1MigrationSummaryModal.tsx`:
- Around line 260-404: This file contains multiple local React components
(SummaryRow, ExpandableSummaryRow, EntryList, Entry) that must be split into
separate co-located component files; create individual TSX files for each
component (e.g., SummaryRow.tsx, ExpandableSummaryRow.tsx, EntryList.tsx,
Entry.tsx) exporting their component and prop types, move the corresponding
component code (preserving prop interfaces SummaryRowProps,
ExpandableSummaryRowProps, EntryProps) into those files, update the original
V1MigrationSummaryModal component to import these named exports and remove the
local definitions, and ensure any used icons/utilities (cn, Badge,
LuChevronDown/Right) remain imported where needed.
🪄 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: 9062bd2a-74cd-4588-8c1a-8a808d6f494d
📒 Files selected for processing (9)
apps/desktop/src/lib/trpc/routers/migration/index.tsapps/desktop/src/renderer/routes/_authenticated/components/V1MigrationSummaryModal/V1MigrationSummaryModal.tsxapps/desktop/src/renderer/routes/_authenticated/hooks/useMigrateV1DataToV2/migrate.tsapps/desktop/src/renderer/routes/_authenticated/hooks/useMigrateV1DataToV2/writeSidebarState.test.tsapps/desktop/src/renderer/routes/_authenticated/hooks/useMigrateV1DataToV2/writeSidebarState.tspackages/local-db/drizzle/0041_v1_migration_state.sqlpackages/local-db/drizzle/meta/0041_snapshot.jsonpackages/local-db/drizzle/meta/_journal.jsonpackages/local-db/src/schema/schema.ts
✅ Files skipped from review due to trivial changes (2)
- packages/local-db/drizzle/meta/_journal.json
- packages/local-db/drizzle/0041_v1_migration_state.sql
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/local-db/drizzle/meta/0041_snapshot.json
- apps/desktop/src/renderer/routes/_authenticated/hooks/useMigrateV1DataToV2/writeSidebarState.test.ts
- packages/local-db/src/schema/schema.ts
Comment on lines
+100
to
+114
| findMigrationByOtherOrg: publicProcedure | ||
| .input(z.object({ organizationId: z.string().min(1) })) | ||
| .query(({ input }) => { | ||
| const other = localDb | ||
| .select({ organizationId: v1MigrationState.organizationId }) | ||
| .from(v1MigrationState) | ||
| .where( | ||
| and( | ||
| eq(v1MigrationState.kind, "project"), | ||
| eq(v1MigrationState.status, "success"), | ||
| ), | ||
| ) | ||
| .all() | ||
| .find((row) => row.organizationId !== input.organizationId); | ||
| return other?.organizationId ?? null; |
Contributor
There was a problem hiding this comment.
Include linked project migrations in the cross-org guard.
linked is a successful project migration outcome, but this query only blocks prior "success" rows. A v1 project linked into another org can be migrated again under the current org.
Proposed fix
const other = localDb
- .select({ organizationId: v1MigrationState.organizationId })
+ .select({
+ organizationId: v1MigrationState.organizationId,
+ status: v1MigrationState.status,
+ })
.from(v1MigrationState)
.where(
and(
eq(v1MigrationState.kind, "project"),
- eq(v1MigrationState.status, "success"),
),
)
.all()
- .find((row) => row.organizationId !== input.organizationId);
+ .find(
+ (row) =>
+ row.organizationId !== input.organizationId &&
+ (row.status === "success" || row.status === "linked"),
+ );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/lib/trpc/routers/migration/index.ts` around lines 100 - 114,
The query in findMigrationByOtherOrg currently only checks for
v1MigrationState.status === "success", missing successful "linked" project
migrations; update the where/filter to include both statuses (e.g. status is
"success" OR "linked") while keeping the existing kind === "project" check so
v1MigrationState rows with status "linked" are treated as blocking the
migration; modify the v1MigrationState.where clause (used in
findMigrationByOtherOrg) to combine eq(v1MigrationState.kind, "project") with an
or(...) over eq(v1MigrationState.status, "success") and
eq(v1MigrationState.status, "linked").
3 tasks
saddlepaddle
added a commit
that referenced
this pull request
Apr 23, 2026
Forces the desktop coordinator to kill any adopted host-service older than
0.2.0 and respawn from the current app bundle. Prevents the renderer from
talking to a stale host-service that's missing newly-added procedures or
params — specifically the `workspaceCreation.adopt({ worktreePath })`
parameter introduced in #3670 for the v1→v2 migration.
Observed symptom this prevents: a new Superset.app connects to a zombie
host-service from an older Superset Canary install via the shared manifest
at ~/.superset/host/<org>/manifest.json. The old service silently strips
unknown zod fields, falls through to the pre-worktreePath adopt logic, and
returns NOT_FOUND for every legacy-path worktree.
13 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
First-time v2 launch now migrates the user's v1 local data into v2 cloud + local stores and shows a branded summary modal.
Migrated:
tab_order IS NOT NULL) — dedup via GitHub remote (v2Project.findByGitHubRemote), then either link to existing v2 project or create new. Host-service registration handled viaproject.create importLocal/project.setup import.deleting_atset) — adopted into v2 via a newworktreePathargument onworkspaceCreation.adopt, so legacy v1 worktree paths (~/.superset/worktrees/...) work in addition to v2's.worktrees/convention.workspace_sectionsunder a migrated project, fresh UUIDs, empty sections preserved.projects.default_app→v2SidebarProjects.defaultOpenInApp.Architecture:
useMigrateV1DataToV2hook) — follows theuseMigrateV1PresetsToV2precedent since PGlite sidebar collections are localStorage-only.migrationtRPC router on the main process for v1 reads + migration_state CRUD.writeV2SidebarStatefunction owns all sidebar collection writes; the main orchestrator only does cloud + host-service calls.v1_migration_statetable inpackages/local-db(migration0041) tracks per-row outcome for idempotent reruns.V1MigrationSummaryModalshows counts + expandable per-row breakdown on first-run only. Non-dismissable (must click "Got it"), persisted via localStorage + custom event.Tab-order translation: v2 doesn't support interleaving sections and top-level workspaces at arbitrary positions (post-section workspaces get visually absorbed). Migration normalizes v1 layouts to
[top-level workspaces] → [sections]with relative order preserved within each group.Followups tracked:
workspace_base_branchsettingapps/desktop/plans/20260422-2100-v1-to-v2-port.mdTest plan
Summary by cubic
On first v2 launch, migrate v1 pinned projects and workspaces into v2 cloud and local stores, preserve sections and sidebar order, and show a one-time summary modal. Migration is idempotent, dedupes projects by GitHub remote, and tracks state per org.
New Features
worktreePathonworkspaceCreation.adopt(supports v1 paths and v2.worktrees/).[top-level workspaces] → [sections], preserving relative order within each group.migrationtRPC router for v1 reads and state CRUD, a renderer orchestratoruseMigrateV1DataToV2, and a singlewriteV2SidebarStatewriter for sidebar collections.v1_migration_statetable inpackages/local-dbfor idempotent reruns; show results inV1MigrationSummaryModalon first run only.Bug Fixes
v1_migration_stateprimary key to(organization_id, v1_id, kind)for per-org idempotence.Follow-ups: SUPER-469 (workspace base branch), SUPER-470 (branch prefix mode), SUPER-471 (phase-2 settings).
Written for commit b77991e. Summary will update on new commits.
Summary by CodeRabbit
New Features
Tests
Chores
Documentation