fix: resume an import whose workspace was already created

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HtYCxXEn2WujwVvh5aZRCa

frontend/src/lib/importWizard/execution.svelte.ts

@@ -13,6 +13,7 @@ import type {
 	ProjectMigration
 } from '$lib/components/workspaceSettings/projectBundle'
 import { planWorkspaceId, type ImportPlan } from './plan'
+import { clearParkedImport, parkImport, resumableImport } from './parking'
 
 /**
  * The only thing in the wizard that changes anything. It takes a finished plan and
@@ -44,6 +45,30 @@ export interface TaskView {
 	detail?: string
 }
 
+/**
+ * What a plan will do, as the same task list the run reports against. Exported so the
+ * last step can show it before the run starts: the checklist is what the step says it
+ * is going to do, and the run then fills in the same rows rather than replacing them.
+ *
+ * Derived from the plan alone — no network — so it is safe to call while rendering.
+ */
+export function plannedTasks(plan: ImportPlan): TaskView[] {
+	const d = plan.destination
+	const tasks: TaskView[] = []
+	if (d?.kind === 'new') {
+		tasks.push({ key: 'create', label: `Create workspace ${d.id}`, status: 'pending' })
+	}
+	tasks.push({ key: 'fetch', label: 'Fetch the project from the hub', status: 'pending' })
+	// The destination rides on this row when nothing creates it, so the list still says
+	// where the items are going in the existing-workspace case.
+	tasks.push({
+		key: 'import',
+		label: d?.kind === 'existing' ? `Import the items into ${d.workspaceId}` : 'Import the items',
+		status: 'pending'
+	})
+	return tasks
+}
+
 export interface ExecutionDeps {
 	/**
 	 * Chooses which data table migrations to run. Returns the migrations to apply,
@@ -118,9 +143,19 @@ export class ImportExecution {
 	// `$workspaceStore` already holds the workspace being entered.
 	#priorWorkspace = get(workspaceStore)
 
+	/**
+	 * A workspace this plan created before the page was reloaded. Only ever true for a run
+	 * whose create already succeeded: `resumableImport` requires the parked project *and*
+	 * workspace to be this plan's, so an entry left by another import cannot make this run
+	 * skip a create it has not done.
+	 */
+	#resumed: boolean
+
 	constructor(plan: ImportPlan, deps: ExecutionDeps) {
 		this.#plan = plan
 		this.#deps = deps
+		const d = plan.destination
+		this.#resumed = d?.kind === 'new' && resumableImport(plan.slug, d.id)
 		this.tasks = this.#initialTasks()
 	}
 
@@ -128,7 +163,17 @@ export class ImportExecution {
 		return planWorkspaceId(this.#plan)
 	}
 
-	/** True once this run created a workspace — the only case where deleting is ours to offer. */
+	/**
+	 * True once this run created a workspace — the only case where deleting is ours to offer.
+	 *
+	 * Deliberately not satisfied by `#resumed`. A parked entry is enough to skip a create,
+	 * because entering the wrong workspace is recoverable; it is not enough to delete one,
+	 * because that is not. Verifying would need a discriminator to compare the live
+	 * workspace against, and a workspace has none — no `created_at`, nothing that moves
+	 * when someone else writes — so a parked id could name a workspace another admin made
+	 * at that id after ours was removed. A resumed run therefore finishes the import and
+	 * leaves the undo to the run that actually did the creating.
+	 */
 	get createdWorkspace(): boolean {
 		return this.#workspaceCreated
 	}
@@ -137,15 +182,19 @@ export class ImportExecution {
 		return this.results.filter((r) => !r.ok).length
 	}
 
+	/** `installProject` reports migrations through the same channel as items, tagged by this
+	 *  prefix. Split so the import row counts what it imported and the migrate row counts
+	 *  what it migrated — one failure should not be attributed to both. */
+	static readonly MIGRATION_PREFIX = 'data table: '
+	get itemResults(): InstallResult[] {
+		return this.results.filter((r) => !r.path.startsWith(ImportExecution.MIGRATION_PREFIX))
+	}
+	get migrationResults(): InstallResult[] {
+		return this.results.filter((r) => r.path.startsWith(ImportExecution.MIGRATION_PREFIX))
+	}
+
 	#initialTasks(): TaskView[] {
-		const d = this.#plan.destination
-		const tasks: TaskView[] = []
-		if (d?.kind === 'new') {
-			tasks.push({ key: 'create', label: `Create workspace ${d.id}`, status: 'pending' })
-		}
-		tasks.push({ key: 'fetch', label: 'Fetch the project from the hub', status: 'pending' })
-		tasks.push({ key: 'import', label: 'Import the items', status: 'pending' })
-		return tasks
+		return plannedTasks(this.#plan)
 	}
 
 	#set(key: string, status: TaskStatus, detail?: string) {
@@ -202,8 +251,9 @@ export class ImportExecution {
 		}
 		// Keyed on the workspace existing rather than on the task being green: a retry
 		// after entering it failed must not run the create again, which would only
-		// report the id as taken by the workspace this run just made.
-		if (!this.#workspaceCreated) {
+		// report the id as taken by the workspace this run just made. `#resumed` covers
+		// the same ground across a reload, where the field starts false again.
+		if (!this.#workspaceCreated && !this.#resumed) {
 			this.#set('create', 'running')
 			try {
 				await WorkspaceService.createWorkspace({
@@ -216,6 +266,9 @@ export class ImportExecution {
 				return undefined
 			}
 			this.#workspaceCreated = true
+			// From here a reload can no longer tell that this id is ours, so record it
+			// before anything else can fail.
+			parkImport({ slug: this.#plan.slug, workspaceId: d.id })
 		}
 		try {
 			await enterNewWorkspace(d.id)
@@ -275,6 +328,20 @@ export class ImportExecution {
 			return
 		}
 
+		// Appended only once the review has settled: until then nothing knows whether any
+		// migration is runnable here, and a row that might not apply is worse than none.
+		if (migrations.length && !this.tasks.some((t) => t.key === 'migrate')) {
+			const n = migrations.length
+			this.tasks = [
+				...this.tasks,
+				{
+					key: 'migrate',
+					label: `Run ${n} data table migration${n === 1 ? '' : 's'}`,
+					status: 'pending'
+				}
+			]
+		}
+
 		this.results = []
 		try {
 			await installProject({
@@ -283,7 +350,8 @@ export class ImportExecution {
 				folder,
 				migrations,
 				hasEeLicense: this.#deps.hasEeLicense,
-				onResult: (r) => (this.results = [...this.results, r])
+				onResult: (r) => (this.results = [...this.results, r]),
+				onMigrationsStart: () => this.#set('migrate', 'running')
 			})
 		} catch (e: any) {
 			this.#set('import', 'failed', String(e))
@@ -291,17 +359,29 @@ export class ImportExecution {
 			return
 		}
 
-		const failed = this.failedCount
+		const items = this.itemResults
+		const failed = items.filter((r) => !r.ok).length
 		this.#set(
 			'import',
 			failed > 0 ? 'failed' : 'done',
-			failed > 0
-				? `${this.results.length - failed} of ${this.results.length} imported`
-				: `${this.results.length} items`
+			failed > 0 ? `${items.length - failed} of ${items.length} imported` : `${items.length} items`
 		)
+
+		const migrated = this.migrationResults
+		if (migrated.length) {
+			const badly = migrated.filter((r) => !r.ok)
+			this.#set(
+				'migrate',
+				badly.length ? 'failed' : 'done',
+				badly.length ? badly.map((r) => r.error).join('; ') : undefined
+			)
+		}
 		// A partial import is finished, not broken: the items that landed are real,
 		// and the failures are listed. Only a hard stop leaves `done` false.
 		this.done = true
+		// Nothing left to resume. A later import of the same project must reach its
+		// create rather than adopt this one.
+		clearParkedImport()
 		if (failed > 0) this.error = `${failed} item${failed === 1 ? '' : 's'} failed to import.`
 	}
 
@@ -334,6 +414,8 @@ export class ImportExecution {
 		switchWorkspace(this.#priorWorkspace)
 		await refreshWorkspaceList()
 		this.#workspaceCreated = false
+		// The id is free again, so a retry has to create it rather than adopt it.
+		clearParkedImport()
 		this.#set('create', 'pending')
 		this.done = false
 		this.results = []

frontend/src/lib/importWizard/parking.test.ts

@@ -0,0 +1,39 @@
+import { beforeEach, describe, expect, it } from 'vitest'
+
+import { clearParkedImport, parkImport, readParkedImport, resumableImport } from './parking'
+
+// A run that skips its create when it should not have imports a project into a workspace
+// somebody else owns, so the match has to be exact and a damaged entry has to read as
+// nothing parked rather than as a partial match.
+
+describe('resumableImport', () => {
+	beforeEach(() => clearParkedImport())
+
+	it('resumes the run that parked it', () => {
+		parkImport({ slug: 'calendly', workspaceId: 'calendly-7' })
+		expect(resumableImport('calendly', 'calendly-7')).toBe(true)
+	})
+
+	it('does not resume another project parked at the same workspace', () => {
+		parkImport({ slug: 'calendly', workspaceId: 'calendly-7' })
+		expect(resumableImport('bitly', 'calendly-7')).toBe(false)
+	})
+
+	it('does not resume the same project aimed at another workspace', () => {
+		parkImport({ slug: 'calendly', workspaceId: 'calendly-7' })
+		expect(resumableImport('calendly', 'calendly-8')).toBe(false)
+	})
+
+	it('does not resume once cleared', () => {
+		parkImport({ slug: 'calendly', workspaceId: 'calendly-7' })
+		clearParkedImport()
+		expect(resumableImport('calendly', 'calendly-7')).toBe(false)
+	})
+
+	it('reads a damaged entry as nothing parked', () => {
+		sessionStorage.setItem('import_wizard_parked', '{"slug":"calendly"')
+		expect(readParkedImport()).toBeUndefined()
+		sessionStorage.setItem('import_wizard_parked', '{"slug":"calendly"}')
+		expect(readParkedImport()).toBeUndefined()
+	})
+})

frontend/src/lib/importWizard/parking.ts

@@ -0,0 +1,62 @@
+/**
+ * What an import run has already created, kept across a reload.
+ *
+ * The plan lives in the URL (`./plan`) and everything the first two steps decide belongs
+ * there — that is what makes the back button and shareable links work. This file is for the
+ * one thing the URL cannot hold: a fact the run produced rather than the user, which the
+ * next page load has no other way to learn. Creating the workspace is currently the only
+ * one. Anything a user chose goes in the plan, not here.
+ *
+ * Kept out of the executor so a caller can ask what is parked without pulling the run in.
+ */
+
+const PARKED_KEY = 'import_wizard_parked'
+
+export type ParkedImport = {
+	/** The project this run is importing. */
+	slug: string
+	/** The workspace the run created, which a resumed run must not try to create again. */
+	workspaceId: string
+}
+
+export function parkImport(parked: ParkedImport): void {
+	try {
+		sessionStorage.setItem(PARKED_KEY, JSON.stringify(parked))
+	} catch {
+		// Storage disabled or full. The run continues; only the resume is lost.
+	}
+}
+
+export function clearParkedImport(): void {
+	try {
+		sessionStorage.removeItem(PARKED_KEY)
+	} catch {}
+}
+
+/**
+ * The parked run, when it is the one being asked about. Both fields have to match: an entry
+ * left by another project would otherwise make this run skip a create it has not done, and
+ * enter a workspace that belongs to a different import.
+ */
+export function resumableImport(slug: string, workspaceId: string): boolean {
+	const parked = readParkedImport()
+	return parked?.slug === slug && parked?.workspaceId === workspaceId
+}
+
+export function readParkedImport(): ParkedImport | undefined {
+	let raw: string | null = null
+	try {
+		raw = sessionStorage.getItem(PARKED_KEY)
+	} catch {
+		return undefined
+	}
+	if (!raw) return undefined
+	try {
+		const parsed = JSON.parse(raw)
+		return typeof parsed?.slug === 'string' && typeof parsed?.workspaceId === 'string'
+			? { slug: parsed.slug, workspaceId: parsed.workspaceId }
+			: undefined
+	} catch {
+		return undefined
+	}
+}