Skip to content

[codex] Harden v1 to v2 migration idempotency #3781

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
saddlepaddle merged 1 commit into from
Apr 27, 2026
Merged

[codex] Harden v1 to v2 migration idempotency #3781

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
155 changes: 149 additions & 6 deletions apps/desktop/src/renderer/routes/_authenticated/hooks/useMigrateV1DataToV2/migrate.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ interface FakeEnv {
setupThrowsFor: Set<string>;
createThrowsFor: Set<string>;
adoptThrowsFor: Map<string, { code: string; message?: string }>;
adoptThrowsForPath: Map<string, { code: string; message?: string }>;
createCalls: Array<{ name: string; repoPath: string }>;
createdProjectIds: string[];
setupCalls: Array<{ projectId: string; repoPath?: string }>;
Expand Down Expand Up @@ -95,6 +96,7 @@ function makeFakeEnv(overrides: Partial<FakeEnv> = {}): FakeEnv {
setupThrowsFor: new Set(),
createThrowsFor: new Set(),
adoptThrowsFor: new Map(),
adoptThrowsForPath: new Map(),
createCalls: [],
createdProjectIds: [],
setupCalls: [],
Expand Down Expand Up @@ -178,14 +180,13 @@ function makeHostService(env: FakeEnv): HostServiceClient {
mode,
}: {
projectId: string;
mode: { repoPath?: string };
mode: { repoPath: string; allowRelocate?: boolean };
}) => {
env.setupCalls.push({ projectId, repoPath: mode.repoPath });
if (env.setupThrowsFor.has(projectId)) {
throw trpcErr("CONFLICT", "already set up elsewhere");
}
if (mode.repoPath)
env.hostProjectsByPath.set(mode.repoPath, projectId);
env.hostProjectsByPath.set(mode.repoPath, projectId);
return { repoPath: "/fake" };
},
},
Expand Down Expand Up @@ -242,6 +243,11 @@ function makeHostService(env: FakeEnv): HostServiceClient {
if (existingWorkspaceId)
call.existingWorkspaceId = existingWorkspaceId;
env.adoptCalls.push(call);
const pathBehavior = worktreePath
? env.adoptThrowsForPath.get(worktreePath)
: undefined;
if (pathBehavior)
throw trpcErr(pathBehavior.code, pathBehavior.message);
const behavior = env.adoptThrowsFor.get(branch);
if (behavior) throw trpcErr(behavior.code, behavior.message);
const key = `${projectId}:${worktreePath ?? branch}`;
Expand Down Expand Up @@ -441,6 +447,7 @@ describe("migrateV1DataToV2", () => {
}),
],
v1Worktrees: [],
adoptThrowsFor: new Map([["branch-w-orphan", { code: "NOT_FOUND" }]]),
});

const summary = await migrateV1DataToV2({
Expand All @@ -452,7 +459,39 @@ describe("migrateV1DataToV2", () => {

expect(summary.workspacesSkipped).toBe(1);
expect(summary.workspacesCreated).toBe(0);
expect(env.state.get("workspace:w-orphan")?.reason).toBe("orphan_worktree");
expect(env.state.get("workspace:w-orphan")?.reason).toBe(
"worktree_not_registered",
);
});

test("missing v1 worktree row falls back to branch adoption", async () => {
const env = makeFakeEnv({
v1Projects: [project("p1")],
v1Workspaces: [
workspace("w1", "p1", {
type: "worktree",
worktreeId: "missing",
}),
],
v1Worktrees: [],
});

const summary = await migrateV1DataToV2({
organizationId: ORG,
electronTrpc: makeElectronTrpc(env),
hostService: makeHostService(env),
collections: makeCollections(),
});

expect(summary.workspacesCreated).toBe(1);
expect(summary.workspacesSkipped).toBe(0);
expect(env.adoptCalls).toContainEqual({
projectId: "v2-proj-1",
branch: "branch-w1",
worktreePath: undefined,
baseBranch: undefined,
});
expect(env.state.get("workspace:w1")?.status).toBe("success");
});

test("adopt NOT_FOUND is skipped, not errored", async () => {
Expand Down Expand Up @@ -483,6 +522,45 @@ describe("migrateV1DataToV2", () => {
);
});

test("stale v1 worktree path falls back to branch adoption", async () => {
const env = makeFakeEnv({
v1Projects: [project("p1")],
v1Workspaces: [
workspace("w1", "p1", {
worktreeId: "wt1",
type: "worktree",
}),
],
v1Worktrees: [{ id: "wt1", path: "/stale-worktree" }],
adoptThrowsForPath: new Map([["/stale-worktree", { code: "NOT_FOUND" }]]),
});

const summary = await migrateV1DataToV2({
organizationId: ORG,
electronTrpc: makeElectronTrpc(env),
hostService: makeHostService(env),
collections: makeCollections(),
});

expect(summary.workspacesCreated).toBe(1);
expect(summary.workspacesSkipped).toBe(0);
expect(env.adoptCalls).toEqual([
{
projectId: "v2-proj-1",
branch: "branch-w1",
worktreePath: "/stale-worktree",
baseBranch: undefined,
},
{
projectId: "v2-proj-1",
branch: "branch-w1",
worktreePath: undefined,
baseBranch: undefined,
},
]);
expect(env.state.get("workspace:w1")?.status).toBe("success");
});

test("adopt non-NOT_FOUND error is recorded as error", async () => {
const env = makeFakeEnv({
v1Projects: [project("p1")],
Expand Down Expand Up @@ -785,7 +863,7 @@ describe("migrateV1DataToV2", () => {
expect(env.state.get("workspace:w1")?.v2Id).toBe("v2-ws-existing");
});

test("rerun does not retry permanent skipped workspaces", async () => {
test("rerun retries previous worktree skips so old skipped state can recover", async () => {
const env = makeFakeEnv({
v1Projects: [project("p1")],
v1Workspaces: [
Expand Down Expand Up @@ -828,10 +906,75 @@ describe("migrateV1DataToV2", () => {
collections: makeCollections(),
});

expect(summary.workspacesCreated).toBe(1);
expect(summary.workspacesSkipped).toBe(0);
expect(env.adoptCalls).toContainEqual({
projectId: "v2-p1",
branch: "branch-w-orphan",
worktreePath: undefined,
baseBranch: undefined,
});
expect(env.state.get("workspace:w-orphan")?.status).toBe("success");
});

test("failed retry of previous missing-worktree skip does not count as new work", async () => {
const env = makeFakeEnv({
v1Projects: [project("p1")],
v1Workspaces: [
workspace("w-orphan", "p1", {
type: "worktree",
worktreeId: "missing",
}),
],
v1Worktrees: [],
adoptThrowsFor: new Map([["branch-w-orphan", { code: "NOT_FOUND" }]]),
state: new Map([
[
"project:p1",
{
v1Id: "p1",
v2Id: "v2-p1",
organizationId: ORG,
kind: "project",
status: "success",
reason: null,
},
],
[
"workspace:w-orphan",
{
v1Id: "w-orphan",
v2Id: null,
organizationId: ORG,
kind: "workspace",
status: "skipped",
reason: "worktree_not_registered",
},
],
]),
});

const summary = await migrateV1DataToV2({
organizationId: ORG,
electronTrpc: makeElectronTrpc(env),
hostService: makeHostService(env),
collections: makeCollections(),
});

expect(summary.workspacesCreated).toBe(0);
expect(summary.workspacesSkipped).toBe(0);
expect(env.adoptCalls).toHaveLength(0);
expect(env.adoptCalls).toHaveLength(1);
expect(summary.workspaces).toHaveLength(1);
expect(summary.workspaces).toContainEqual({
name: "workspace-w-orphan",
branch: "branch-w-orphan",
status: "skipped",
reason: "worktree no longer exists",
});
expect(env.state.get("workspace:w-orphan")?.status).toBe("skipped");
expect(env.state.get("workspace:w-orphan")?.reason).toBe(
"worktree_not_registered",
);
});

test("passes v1 worktree base branch into adoption", async () => {
Expand Down
114 changes: 67 additions & 47 deletions apps/desktop/src/renderer/routes/_authenticated/hooks/useMigrateV1DataToV2/migrate.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,9 @@ function shouldRetryWorkspace(
if (existing.status === "error") return true;
return (
existing.status === "skipped" &&
existing.reason === "parent_project_unresolved"
(existing.reason === "parent_project_unresolved" ||
existing.reason === "orphan_worktree" ||
existing.reason === "worktree_not_registered")
);
}

Expand Down Expand Up @@ -144,6 +146,16 @@ function skippedWorkspaceReason(reason: string | null | undefined): string {
}
}

function wasAlreadyMissingWorktreeSkip(
existing: { status: string; reason?: string | null } | undefined,
): boolean {
return (
existing?.status === "skipped" &&
(existing.reason === "orphan_worktree" ||
existing.reason === "worktree_not_registered")
);
}

function addWorkspaceError(
summary: MigrationSummary,
name: string,
Expand Down Expand Up @@ -398,74 +410,49 @@ export async function migrateV1DataToV2(args: Args): Promise<MigrationSummary> {
continue;
}

if (workspace.type === "worktree") {
if (!workspace.worktreeId || !worktreesById.has(workspace.worktreeId)) {
await electronTrpc.migration.upsertState.mutate({
v1Id: workspace.id,
kind: "workspace",
v2Id: null,
organizationId,
status: "skipped",
reason: "orphan_worktree",
});
addWorkspaceSkip(
summary,
workspace.name,
workspace.branch,
"worktree record missing",
);
continue;
}
}

const v1WorktreePath = workspace.worktreeId
? worktreesById.get(workspace.worktreeId)?.path
: undefined;
const v1BaseBranch = workspace.worktreeId
? worktreesById.get(workspace.worktreeId)?.baseBranch
const v1Worktree = workspace.worktreeId
? worktreesById.get(workspace.worktreeId)
: undefined;
const v1WorktreePath = v1Worktree?.path;
const v1BaseBranch = v1Worktree?.baseBranch;

try {
const result = await hostService.workspaceCreation.adopt.mutate({
const adoptWorkspace = (worktreePath: string | undefined) =>
hostService.workspaceCreation.adopt.mutate({
projectId: v2ProjectId,
workspaceName: workspace.name,
branch: workspace.branch,
baseBranch: v1BaseBranch ?? undefined,
existingWorkspaceId: existing?.v2Id ?? undefined,
worktreePath: v1WorktreePath,
});
await electronTrpc.migration.upsertState.mutate({
v1Id: workspace.id,
kind: "workspace",
v2Id: result.workspace.id,
organizationId,
status: "success",
reason: null,
worktreePath,
});
workspaceV1ToV2.set(workspace.id, result.workspace.id);
summary.workspacesCreated += 1;
summary.workspaces.push({
name: workspace.name,
branch: workspace.branch,
status: "adopted",
});
} catch (err) {

const recordAdoptFailure = async (err: unknown) => {
if (trpcCode(err) === "NOT_FOUND") {
const reason = "worktree_not_registered";
await electronTrpc.migration.upsertState.mutate({
v1Id: workspace.id,
kind: "workspace",
v2Id: null,
organizationId,
status: "skipped",
reason: "worktree_not_registered",
reason,
});
if (wasAlreadyMissingWorktreeSkip(existing)) {
summary.workspaces.push({
name: workspace.name,
branch: workspace.branch,
status: "skipped",
reason: skippedWorkspaceReason(reason),
});
return;
}
addWorkspaceSkip(
summary,
workspace.name,
workspace.branch,
"worktree no longer exists",
);
continue;
return;
}
const message = errorMessage(err);
await electronTrpc.migration.upsertState.mutate({
Expand All @@ -478,6 +465,39 @@ export async function migrateV1DataToV2(args: Args): Promise<MigrationSummary> {
});
addWorkspaceError(summary, workspace.name, workspace.branch, message);
console.error("[v1-migration] workspace failed", workspace.name, err);
};

try {
let result: Awaited<ReturnType<typeof adoptWorkspace>>;
try {
result = await adoptWorkspace(v1WorktreePath);
} catch (err) {
if (trpcCode(err) !== "NOT_FOUND" || !v1WorktreePath) {
throw err;
}

// v1 worktree rows can be stale while git still has the branch
// registered at a different path. Retry by branch before giving up.
result = await adoptWorkspace(undefined);
}

await electronTrpc.migration.upsertState.mutate({
v1Id: workspace.id,
kind: "workspace",
v2Id: result.workspace.id,
organizationId,
status: "success",
reason: null,
});
workspaceV1ToV2.set(workspace.id, result.workspace.id);
summary.workspacesCreated += 1;
summary.workspaces.push({
name: workspace.name,
branch: workspace.branch,
status: "adopted",
});
} catch (err) {
await recordAdoptFailure(err);
}
}

Expand Down
Loading