Skip to content

feat(desktop): headless v1→v2 migrator core + first migration-ledger writers - #5814

Merged
Kitenite merged 2 commits into
mainfrom
v1-to-v2-auto-migrate
Jul 21, 2026
Merged

Kitenite merged 2 commits into
mainfrom
v1-to-v2-auto-migrate

Conversation

@Kitenite

@Kitenite Kitenite commented Jul 20, 2026 •

Copy link
Copy Markdown
Member

Summary

PR 1 of the v1→v2 auto-migration plan (SUPER-1534 / SUPER-1577, design doc plans/20260716-v1-to-v2-auto-migration.md): extract the V1ImportModal wizard's import logic into a shared headless module and give the dormant v1_migration_state ledger its first readers/writers.

  • New renderer/lib/v1-migration/ module: runV1Migration() orchestrator (projects → workspaces → presets, every outcome ledger-recorded, idempotent per boot) plus the extracted pure logic: decideProjectImport, planWorkspaceAdoptions, resolvePresetImport (keep-v2 preset collision policy), importV1Project, adoptV1Workspace (worktree-path NOT_FOUND fallback preserved).
  • Ledger live: migration.ledgerList / migration.ledgerRecord electron tRPC procedures (upsert on (org, v1Id, kind)); V1MigrationKind extended with settings + terminal for the follow-up PRs.
  • Wizard fixes: pages consume the shared module and record ledger outcomes; ImportWorkspacesPage dedups against host-local workspace.list instead of the stale cloud cloudList (post-local-first, newly adopted workspaces never appear in cloud, so the old "Imported" badge misreported); removed the now-moot cloud tiebreak in v1→v2 project mapping.

The boot trigger + flip gate land in a later PR — nothing runs automatically yet; the wizard behavior is unchanged apart from the badge fix.

Test Plan

  • bun test src/renderer/lib/v1-migration/ — 12 tests over the extracted decision logic (each mutation-verified to fail on a seeded regression)
  • bun run typecheck (desktop) and root bun run lint clean
  • Manual wizard pass on a machine with v1 data: import project + workspace + preset, confirm v1_migration_state rows land and re-running shows Imported/Linked

Summary by cubic

Built the headless v1→v2 migrator core and activated the migration ledger to prepare desktop for automated migration (SUPER-1534, SUPER-1577). Added per-stage ledger flushing and guarded UI callbacks; the wizard now uses the shared module with fixed workspace dedup. Nothing runs automatically yet.

  • New Features

    • Added renderer/lib/v1-migration/ with runV1Migration() (projects → workspaces → presets; idempotent; records outcomes).
    • Extracted pure logic: decideProjectImport, planWorkspaceAdoptions, resolvePresetImport, importV1Project, adoptV1Workspace.
    • Enabled migration.ledgerList and migration.ledgerRecord tRPC procedures; upsert on (org, v1Id, kind).
    • Extended V1MigrationKind with settings and terminal for follow-up migrations.
  • Bug Fixes

    • Flush ledger outcomes between stages and guard onProjectImported/onWorkspaceAdopted callbacks to avoid dropping committed results on later errors.
    • Wizard pages consume the shared module and record outcomes to the ledger; ImportWorkspacesPage dedups against host-local workspace.list, and ImportPresetsPage uses the shared resolver and writes ledger results.

Written for commit 9aae067. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • New Features
    • Added resumable v1 → v2 migration support for projects, workspaces, and terminal presets, with ledger-based progress tracking to safely skip completed items and retry partial runs.
    • Introduced an idempotent, headless migration runner with clearer per-item results and an overall completion gate.
    • Updated the import screens to use the shared migration flow, improving project eligibility decisions, preset collision handling, and workspace adoption behavior.
  • Documentation
    • Added a seamless auto-migration plan and v1 sunset roadmap.
  • Tests
    • Added coverage for project, workspace, and preset migration decision logic.

@coderabbitai

coderabbitai Bot commented Jul 20, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Adds a ledger-backed v1-to-v2 migration library with project, workspace, and preset helpers, a headless migration runner, organization-scoped persistence APIs, and integration into the existing import wizard.

Changes

V1 migration flow

Layer / File(s) Summary
Ledger contract and persistence
packages/local-db/src/schema/schema.ts, apps/desktop/src/lib/trpc/routers/migration/index.ts, apps/desktop/src/renderer/lib/v1-migration/ledger.ts
Expands migration kinds and adds organization-scoped ledger listing, upserts, loading, terminal-status checks, and outcome recording.
Shared migration decisions and operations
apps/desktop/src/renderer/lib/v1-migration/{projects,workspaces,presets}.ts, v1-migration.test.ts
Adds reusable project import, workspace adoption, preset conversion, and helper test coverage.
Headless migration orchestration
apps/desktop/src/renderer/lib/v1-migration/{runV1Migration,index}.ts, plans/20260716-v1-to-v2-auto-migration.md
Adds ordered ledger-aware migration phases, per-entity outcomes, completion gating, exports, and the auto-migration design plan.
Wizard integration
apps/desktop/src/renderer/routes/_authenticated/components/V1ImportModal/*
Updates project, workspace, and preset import pages to use shared helpers, host-local workspace state, and ledger outcome recording.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant MigrationRunner
  participant HostServiceClient
  participant ElectronTRPC
  participant MigrationLedger
  MigrationRunner->>ElectronTRPC: Load organization ledger
  ElectronTRPC->>MigrationLedger: List migration rows
  MigrationRunner->>HostServiceClient: Resolve and import projects
  MigrationRunner->>HostServiceClient: Plan and adopt workspaces
  MigrationRunner->>ElectronTRPC: Record migration outcomes
  ElectronTRPC->>MigrationLedger: Upsert outcome rows
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title is concise, conventional, and accurately summarizes the new headless migrator and ledger writers.
Description check ✅ Passed The description includes the change summary and test plan, but it doesn't follow the template headings or checklist exactly.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch v1-to-v2-auto-migrate

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@greptile-apps

greptile-apps Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds the shared core for migrating desktop data from v1 to v2. The main changes are:

  • A headless projects, workspaces, and presets migration flow.
  • Organization-scoped migration-ledger readers and writers.
  • Shared project, workspace, and preset import helpers.
  • Wizard integration with host-local workspace deduplication.

Confidence Score: 4/5

Successful migrations can be recorded as failures, and concurrent passes can repeat side effects. These paths need fixes before merging.

  • Project and workspace callbacks run inside the host-operation catch blocks.
  • A callback exception overwrites a queued success with an error.
  • The read-check-mutate-write flow has no per-organization serialization.
  • The remaining extraction and ledger schema changes appear consistent.

apps/desktop/src/renderer/lib/v1-migration/runV1Migration.ts

Important Files Changed

Filename Overview
apps/desktop/src/renderer/lib/v1-migration/runV1Migration.ts Adds the migration orchestrator, but callback failures corrupt successful outcomes and concurrent passes are not serialized.
apps/desktop/src/lib/trpc/routers/migration/index.ts Adds organization-filtered ledger reads and conflict-keyed outcome upserts.
apps/desktop/src/renderer/lib/v1-migration/projects.ts Extracts project discovery, linking, creation, and relocation handling.
apps/desktop/src/renderer/lib/v1-migration/workspaces.ts Extracts workspace planning and adoption with the existing missing-path fallback.
apps/desktop/src/renderer/lib/v1-migration/presets.ts Extracts preset collision handling and v2 preset row construction.
apps/desktop/src/renderer/lib/v1-migration/ledger.ts Adds ledger loading, terminal-state checks, batch recording, and advisory UI recording.
apps/desktop/src/renderer/routes/_authenticated/components/V1ImportModal/ImportWorkspacesPage/ImportWorkspacesPage.tsx Moves workspace deduplication from stale cloud state to the host-local workspace list.
packages/local-db/src/schema/schema.ts Extends migration kinds for the planned settings and terminal stages.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Caller
    participant Migrator as runV1Migration
    participant Host as Host Service
    participant Ledger as Migration Ledger
    Caller->>Migrator: Run for organization
    Migrator->>Ledger: Load outcomes
    Migrator->>Host: Import projects
    Migrator->>Host: Adopt workspaces
    Migrator->>Caller: Run import callbacks
    Migrator->>Caller: Insert presets
    Migrator->>Ledger: Upsert outcomes
    Migrator-->>Caller: Return summaries and gate
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant Caller
    participant Migrator as runV1Migration
    participant Host as Host Service
    participant Ledger as Migration Ledger
    Caller->>Migrator: Run for organization
    Migrator->>Ledger: Load outcomes
    Migrator->>Host: Import projects
    Migrator->>Host: Adopt workspaces
    Migrator->>Caller: Run import callbacks
    Migrator->>Caller: Insert presets
    Migrator->>Ledger: Upsert outcomes
    Migrator-->>Caller: Return summaries and gate
Loading
Prompt To Fix All With AI
Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 3
apps/desktop/src/renderer/lib/v1-migration/runV1Migration.ts:170
**Callback Reverses Project Success**

When `onProjectImported` throws after the host mutation succeeds, the surrounding catch records an error after the queued success. The later upsert leaves the project non-terminal, counts the same project as both migrated and failed, and retries an already-created project on the next run.

```suggestion
			try {
				deps.onProjectImported?.(result);
			} catch (err) {
				console.error("[v1-migration] project callback failed", {
					v1ProjectId: project.id,
					err,
				});
			}
```

### Issue 2 of 3
apps/desktop/src/renderer/lib/v1-migration/runV1Migration.ts:288
**Callback Reverses Workspace Success**

When `onWorkspaceAdopted` throws after adoption commits, the outer catch appends an error for the same workspace. The error overwrites its success in the ledger, prevents the completion gate, and causes a later run to retry a workspace that was already adopted.

```suggestion
			try {
				deps.onWorkspaceAdopted?.(result.workspace.id, entry.v2ProjectId);
			} catch (err) {
				console.error("[v1-migration] workspace callback failed", {
					v1WorkspaceId: entry.v1WorkspaceId,
					err,
				});
			}
```

### Issue 3 of 3
apps/desktop/src/renderer/lib/v1-migration/runV1Migration.ts:86
**Concurrent Passes Share Empty Ledger**

If two app instances run this function for the same organization, both can load the same ledger state and perform mutations before either writes outcomes. The final upserts deduplicate ledger rows but not the preceding side effects, so concurrent preset inserts can create separate UUID-backed rows and callbacks can run twice.

Reviews (1): Last reviewed commit: "feat(desktop): headless v1-to-v2 migrato..." | Re-trigger Greptile

status: "success",
v2Id: result.v2ProjectId,
});
deps.onProjectImported?.(result);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Callback Reverses Project Success

When onProjectImported throws after the host mutation succeeds, the surrounding catch records an error after the queued success. The later upsert leaves the project non-terminal, counts the same project as both migrated and failed, and retries an already-created project on the next run.

Suggested change
deps.onProjectImported?.(result);
try {
deps.onProjectImported?.(result);
} catch (err) {
console.error("[v1-migration] project callback failed", {
v1ProjectId: project.id,
err,
});
}
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/lib/v1-migration/runV1Migration.ts
Line: 170

Comment:
**Callback Reverses Project Success**

When `onProjectImported` throws after the host mutation succeeds, the surrounding catch records an error after the queued success. The later upsert leaves the project non-terminal, counts the same project as both migrated and failed, and retries an already-created project on the next run.

```suggestion
			try {
				deps.onProjectImported?.(result);
			} catch (err) {
				console.error("[v1-migration] project callback failed", {
					v1ProjectId: project.id,
					err,
				});
			}
```

How can I resolve this? If you propose a fix, please make it concise.

status: "success",
v2Id: result.workspace.id,
});
deps.onWorkspaceAdopted?.(result.workspace.id, entry.v2ProjectId);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Callback Reverses Workspace Success

When onWorkspaceAdopted throws after adoption commits, the outer catch appends an error for the same workspace. The error overwrites its success in the ledger, prevents the completion gate, and causes a later run to retry a workspace that was already adopted.

Suggested change
deps.onWorkspaceAdopted?.(result.workspace.id, entry.v2ProjectId);
try {
deps.onWorkspaceAdopted?.(result.workspace.id, entry.v2ProjectId);
} catch (err) {
console.error("[v1-migration] workspace callback failed", {
v1WorkspaceId: entry.v1WorkspaceId,
err,
});
}
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/lib/v1-migration/runV1Migration.ts
Line: 288

Comment:
**Callback Reverses Workspace Success**

When `onWorkspaceAdopted` throws after adoption commits, the outer catch appends an error for the same workspace. The error overwrites its success in the ledger, prevents the completion gate, and causes a later run to retry a workspace that was already adopted.

```suggestion
			try {
				deps.onWorkspaceAdopted?.(result.workspace.id, entry.v2ProjectId);
			} catch (err) {
				console.error("[v1-migration] workspace callback failed", {
					v1WorkspaceId: entry.v1WorkspaceId,
					err,
				});
			}
```

How can I resolve this? If you propose a fix, please make it concise.

deps: RunV1MigrationDeps,
): Promise<V1MigrationSummary> {
const { organizationId } = deps;
const ledger = await loadV1MigrationLedger(organizationId);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Concurrent Passes Share Empty Ledger

If two app instances run this function for the same organization, both can load the same ledger state and perform mutations before either writes outcomes. The final upserts deduplicate ledger rows but not the preceding side effects, so concurrent preset inserts can create separate UUID-backed rows and callbacks can run twice.

Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/lib/v1-migration/runV1Migration.ts
Line: 86

Comment:
**Concurrent Passes Share Empty Ledger**

If two app instances run this function for the same organization, both can load the same ledger state and perform mutations before either writes outcomes. The final upserts deduplicate ledger rows but not the preceding side effects, so concurrent preset inserts can create separate UUID-backed rows and callbacks can run twice.

How can I resolve this? If you propose a fix, please make it concise.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Known and deliberately out of scope for this PR: the per-org single-flight lock lands with the boot trigger (PR 4 of the plan, see plans/20260716-v1-to-v2-auto-migration.md edge-case table). Until then the only caller is the manual wizard, where concurrent same-org runs aren't a realistic path. The other two findings (callback throw reversing success, per-stage ledger flush) are fixed in 9aae067.

@github-actions

github-actions Bot commented Jul 20, 2026 •

Copy link
Copy Markdown
Contributor

🧹 Preview Cleanup Complete

The following preview resources have been cleaned up:

  • ✅ Neon database branch

Thank you for your contribution! 🎉

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
apps/desktop/src/renderer/routes/_authenticated/components/V1ImportModal/ImportPresetsPage/ImportPresetsPage.tsx (1)

34-65: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

alreadyImported dedupe can race ahead of v2Presets sync, risking duplicate preset creation.

v2Presets comes from useLiveQuery (line 29-32), but isLoading (line 34) only tracks presetsQuery.isPending. Before the Electric/TanStack DB collection finishes its initial sync, v2Presets is [] by default, so resolvePresetImport (line 57) can compute alreadyImported: false for a preset that was already imported in a prior session — letting the user click "Import" again and insert a duplicate row via the non-idempotent collections.v2TerminalPresets.insert(row) at line 104.

Contrast with ImportWorkspacesPage.tsx, where the equivalent host query's isPending is folded into the page's aggregate isLoading, gating the dedupe check on readiness.

As per coding guidelines: "TanStack DB/Electric useLiveQuery rendering must be cache-first... Wait for strict readiness before deriving missing rows or writing defaults unless the write is provably idempotent." Inserting a new preset row is not idempotent, so readiness should gate this derivation.

🛡️ Proposed fix: include v2Presets readiness in isLoading
 	const { data: v2Presets = [] } = useLiveQuery(
 		(query) => query.from({ v2TerminalPresets: collections.v2TerminalPresets }),
 		[collections],
 	);
 
-	const isLoading = presetsQuery.isPending;
+	const isLoading = presetsQuery.isPending || v2Presets === undefined; // adjust based on actual useLiveQuery readiness field (e.g. isReady/isLoading)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@apps/desktop/src/renderer/routes/_authenticated/components/V1ImportModal/ImportPresetsPage/ImportPresetsPage.tsx`
around lines 34 - 65, Include the v2Presets live-query readiness state in the
aggregate isLoading value of ImportPresetsPage, matching the readiness handling
in ImportWorkspacesPage. Keep resolvePresetImport and preset rendering gated
until the v2 preset collection has completed its initial sync, so
alreadyImported is not derived from the default empty array and duplicate
non-idempotent inserts cannot be initiated.

Source: Coding guidelines

🧹 Nitpick comments (5)
apps/desktop/src/lib/trpc/routers/migration/index.ts (1)

44-89: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value

Consider using a transaction for multiple inserts.

While the current loop approach works fine for a small number of entries, wrapping the loop in a database transaction ensures atomicity and can improve performance by avoiding multiple implicit transactions in SQLite.

💡 Proposed refactor to use a transaction
-			.mutation(({ input }) => {
-				for (const entry of input.entries) {
-					localDb
-						.insert(v1MigrationState)
-						.values({
-							organizationId: input.organizationId,
-							v1Id: entry.v1Id,
-							kind: entry.kind,
-							status: entry.status,
-							v2Id: entry.v2Id ?? null,
-							reason: entry.reason ?? null,
-						})
-						.onConflictDoUpdate({
-							target: [
-								v1MigrationState.organizationId,
-								v1MigrationState.v1Id,
-								v1MigrationState.kind,
-							],
-							set: {
-								status: entry.status,
-								v2Id: entry.v2Id ?? null,
-								reason: entry.reason ?? null,
-								migratedAt: Date.now(),
-							},
-						})
-						.run();
-				}
-			}),
+			.mutation(({ input }) => {
+				localDb.transaction((tx) => {
+					for (const entry of input.entries) {
+						tx
+							.insert(v1MigrationState)
+							.values({
+								organizationId: input.organizationId,
+								v1Id: entry.v1Id,
+								kind: entry.kind,
+								status: entry.status,
+								v2Id: entry.v2Id ?? null,
+								reason: entry.reason ?? null,
+							})
+							.onConflictDoUpdate({
+								target: [
+									v1MigrationState.organizationId,
+									v1MigrationState.v1Id,
+									v1MigrationState.kind,
+								],
+								set: {
+									status: entry.status,
+									v2Id: entry.v2Id ?? null,
+									reason: entry.reason ?? null,
+									migratedAt: Date.now(),
+								},
+							})
+							.run();
+					}
+				});
+			}),
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/desktop/src/lib/trpc/routers/migration/index.ts` around lines 44 - 89,
Wrap the entry loop in the ledgerRecord mutation with a localDb transaction so
all inserts or conflict updates execute atomically as one operation. Preserve
the existing per-entry insert and onConflictDoUpdate behavior and input
validation.
apps/desktop/src/renderer/lib/v1-migration/v1-migration.test.ts (1)

1-153: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

No coverage for importV1Project/adoptV1Workspace branching logic.

Tests here only cover the pure decision helpers. The retry/fallback branches in importV1Project (relocate detection) and adoptV1Workspace (NOT_FOUND fallback) are untested; a hostClient mock with mock.fn for .mutate would let you assert the fallback path without real I/O.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/desktop/src/renderer/lib/v1-migration/v1-migration.test.ts` around lines
1 - 153, Add tests covering the branching logic in importV1Project and
adoptV1Workspace, not just the pure helpers. Use a hostClient mock with mock.fn
for mutate to verify importV1Project’s relocate-detection retry path and
adoptV1Workspace’s NOT_FOUND fallback, asserting the expected fallback mutations
and outcomes without real I/O.
apps/desktop/src/renderer/lib/v1-migration/projects.ts (1)

76-86: 🩺 Stability & Availability | 🔵 Trivial | 🏗️ Heavy lift

Emit existingPath as structured error data
apps/desktop/src/renderer/lib/v1-migration/projects.ts:76-86 still depends on the host-service message text to decide whether to prompt for relocate. packages/host-service/src/trpc/router/project/project.ts:529 already throws CONFLICT; include existingPath in the error payload so the renderer can branch on typed data instead of regexing the message.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/desktop/src/renderer/lib/v1-migration/projects.ts` around lines 76 - 86,
Update the host-service project conflict error at the existing CONFLICT throw to
include existingPath as structured error data. Then revise
isAlreadySetUpElsewhereError and extractExistingPath in the renderer to consume
the typed conflict payload instead of matching the host-service message text,
preserving the relocate prompt behavior.
apps/desktop/src/renderer/lib/v1-migration/runV1Migration.ts (2)

108-108: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Redundant readV1Projects query on every run.

migrateProjects (line 108) and migrateWorkspaces (line 193) each independently call electronTrpcClient.migration.readV1Projects.query(). Since this function is designed to run on every boot per the migration plan, threading the already-fetched v1Projects array from migrateProjects into migrateWorkspaces (e.g. via a return value or parameter) avoids a duplicate IPC round-trip on the hot path.

Also applies to: 185-198

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/desktop/src/renderer/lib/v1-migration/runV1Migration.ts` at line 108,
Reuse the single v1Projects result across migrateProjects and migrateWorkspaces
instead of querying electronTrpcClient.migration.readV1Projects.query() in both
functions. Thread the already-fetched array through the
migrateProjects-to-migrateWorkspaces call, preserving existing migration
behavior while removing the redundant IPC request.

226-243: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low value

Swallowed error hides real failures, not just "not found".

The bare catch {} treats any failure from listProjectWorktrees (auth error, timeout, etc.) the same as "no worktrees exist", silently letting planWorkspaceAdoptions treat the project as adoptable. A transient host-service error here could be indistinguishable from a legitimately empty result. Consider at least logging the error for observability, even though the fallback behavior itself may be intentional.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/desktop/src/renderer/lib/v1-migration/runV1Migration.ts` around lines
226 - 243, Update the catch block in the listProjectWorktrees flow to log the
caught error with sufficient context before leaving onDiskBranchesByV2ProjectId
unset. Preserve the existing fallback behavior used by planWorkspaceAdoptions
and adopt(), while ensuring failures such as authentication or timeout errors
are observable.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@apps/desktop/src/renderer/lib/v1-migration/runV1Migration.ts`:
- Around line 82-100: Update runV1Migration to persist accumulated outcomes in a
finally block so recordV1MigrationOutcomes runs even when migrateProjects,
migrateWorkspaces, or migratePresets throws. Preserve normal error propagation
and return behavior, while ensuring all outcomes pushed before the failure are
recorded.

---

Outside diff comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/components/V1ImportModal/ImportPresetsPage/ImportPresetsPage.tsx`:
- Around line 34-65: Include the v2Presets live-query readiness state in the
aggregate isLoading value of ImportPresetsPage, matching the readiness handling
in ImportWorkspacesPage. Keep resolvePresetImport and preset rendering gated
until the v2 preset collection has completed its initial sync, so
alreadyImported is not derived from the default empty array and duplicate
non-idempotent inserts cannot be initiated.

---

Nitpick comments:
In `@apps/desktop/src/lib/trpc/routers/migration/index.ts`:
- Around line 44-89: Wrap the entry loop in the ledgerRecord mutation with a
localDb transaction so all inserts or conflict updates execute atomically as one
operation. Preserve the existing per-entry insert and onConflictDoUpdate
behavior and input validation.

In `@apps/desktop/src/renderer/lib/v1-migration/projects.ts`:
- Around line 76-86: Update the host-service project conflict error at the
existing CONFLICT throw to include existingPath as structured error data. Then
revise isAlreadySetUpElsewhereError and extractExistingPath in the renderer to
consume the typed conflict payload instead of matching the host-service message
text, preserving the relocate prompt behavior.

In `@apps/desktop/src/renderer/lib/v1-migration/runV1Migration.ts`:
- Line 108: Reuse the single v1Projects result across migrateProjects and
migrateWorkspaces instead of querying
electronTrpcClient.migration.readV1Projects.query() in both functions. Thread
the already-fetched array through the migrateProjects-to-migrateWorkspaces call,
preserving existing migration behavior while removing the redundant IPC request.
- Around line 226-243: Update the catch block in the listProjectWorktrees flow
to log the caught error with sufficient context before leaving
onDiskBranchesByV2ProjectId unset. Preserve the existing fallback behavior used
by planWorkspaceAdoptions and adopt(), while ensuring failures such as
authentication or timeout errors are observable.

In `@apps/desktop/src/renderer/lib/v1-migration/v1-migration.test.ts`:
- Around line 1-153: Add tests covering the branching logic in importV1Project
and adoptV1Workspace, not just the pure helpers. Use a hostClient mock with
mock.fn for mutate to verify importV1Project’s relocate-detection retry path and
adoptV1Workspace’s NOT_FOUND fallback, asserting the expected fallback mutations
and outcomes without real I/O.
🪄 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: 26b33b7e-f3a9-4aa2-b057-f470dce159e4

📥 Commits

Reviewing files that changed from the base of the PR and between e3d0b1d and e4b09c5.

📒 Files selected for processing (13)
  • apps/desktop/src/lib/trpc/routers/migration/index.ts
  • apps/desktop/src/renderer/lib/v1-migration/index.ts
  • apps/desktop/src/renderer/lib/v1-migration/ledger.ts
  • apps/desktop/src/renderer/lib/v1-migration/presets.ts
  • apps/desktop/src/renderer/lib/v1-migration/projects.ts
  • apps/desktop/src/renderer/lib/v1-migration/runV1Migration.ts
  • apps/desktop/src/renderer/lib/v1-migration/v1-migration.test.ts
  • apps/desktop/src/renderer/lib/v1-migration/workspaces.ts
  • apps/desktop/src/renderer/routes/_authenticated/components/V1ImportModal/ImportPresetsPage/ImportPresetsPage.tsx
  • apps/desktop/src/renderer/routes/_authenticated/components/V1ImportModal/ImportProjectsPage/ImportProjectsPage.tsx
  • apps/desktop/src/renderer/routes/_authenticated/components/V1ImportModal/ImportWorkspacesPage/ImportWorkspacesPage.tsx
  • packages/local-db/src/schema/schema.ts
  • plans/20260716-v1-to-v2-auto-migration.md

This branch was previously deployed

1 inactive deployment
preview — 9aae0674 Deployed Jul 21, 2026 by Kitenite via Deploy Admin #6706
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant