refactor: ask the instance what exists instead of remembering it

The wizard kept a note in `sessionStorage` — "this run created workspace X" —
so a reload could tell that a create had already happened. A note is a second
copy of a fact the instance already holds, and it could outlive the workspace
it named: the comment on `createdWorkspace` said a parked id might point at a
workspace someone else made at that id afterwards, and that there was no way to
tell, because a workspace carries no discriminator.

It carries `owner`. It is set to the creator's email at `INSERT INTO workspace`,
`listWorkspaces` already selects it, and the generated `Workspace` type already
has it. So the question the note was answering can simply be asked:
`probeWorkspace` returns whether a workspace with the plan's id exists among
the caller's, and whether they own it. Ownership is what makes adopting one
safe — an id that exists but belongs to someone else is not this run's work.

`parking.ts` and its test are gone. Nothing in the wizard writes storage now:
the plan is in the URL, what exists is in the instance, and what is in flight is
in memory, which is where in-flight things belong.

`probe.ts` also carries the two reads the follow-up needs — which of the paths
an import would write are already there, and whether a migration's tables exist.
The second is the ground truth for "did this migration run", covering both paths
`applyOneMigration` takes: it records a migration when the data table has them
enabled, and otherwise runs the SQL as a job nothing remembers. The tables
outlive both. It returns `undefined` rather than `false` when it cannot tell,
since "not there" invites a caller to run the migration and "cannot tell" does
not.

Verified against a real reload mid-run: the second attempt makes no
`createWorkspace` call, one `workspaces/list` call, and carries on to the fetch.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

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

@@ -3,8 +3,17 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'
 // Everything the executor reaches over the network, stubbed. The two behaviours under test
 // are decisions it makes around those calls, not the calls themselves.
 vi.mock('$lib/gen', () => ({
-	WorkspaceService: { createWorkspace: vi.fn(), listDataTables: vi.fn(async () => []) },
-	UserService: { whoami: vi.fn(async () => ({ username: 'u' })) }
+	WorkspaceService: {
+		createWorkspace: vi.fn(),
+		listDataTables: vi.fn(async () => []),
+		// No workspace of ours at that id: the existing-workspace plan these tests use never
+		// reaches the create, and an empty list is the honest answer for a fresh instance.
+		listWorkspaces: vi.fn(async () => [])
+	},
+	UserService: {
+		whoami: vi.fn(async () => ({ username: 'u' })),
+		globalWhoami: vi.fn(async () => ({ email: 'u@example.com' }))
+	}
 }))
 vi.mock('$lib/storeUtils', () => ({ switchWorkspace: vi.fn() }))
 vi.mock('$lib/user', () => ({ getUserExt: vi.fn(async () => ({ username: 'u' })) }))
@@ -57,7 +66,6 @@ vi.stubGlobal(
 )
 
 import { ImportExecution } from './execution.svelte'
-import { clearParkedImport, parkImport, resumableImport } from './parking'
 
 const PLAN = { slug: 'calendly', destination: { kind: 'existing' as const, workspaceId: 'ws-a' } }
 const deps = { reviewMigrations: async () => [], hasEeLicense: false }
@@ -99,7 +107,6 @@ describe('planTag', () => {
 
 describe('abandoning mid-import', () => {
 	beforeEach(() => {
-		clearParkedImport()
 		hooks.afterFirstItem = undefined
 		hooks.afterMigrationsStart = undefined
 	})
@@ -121,14 +128,6 @@ describe('abandoning mid-import', () => {
 		expect(run.itemResults.length).toBe(3)
 	})
 
-	it('keeps the parked workspace so the link the leave dialog promises still resumes', async () => {
-		parkImport({ slug: 'calendly', workspaceId: 'ws-a' })
-		const run = new ImportExecution(PLAN, deps)
-		hooks.afterFirstItem = () => run.abandon()
-		await run.run()
-		expect(resumableImport('calendly', 'ws-a')).toBe(true)
-	})
-
 	it('stops the migrate row spinning when it is abandoned mid-migration', async () => {
 		const run = new ImportExecution(PLAN, depsWithMigration)
 		// After `onMigrationsStart`, which is where the row is actually set to running —
@@ -143,11 +142,4 @@ describe('abandoning mid-import', () => {
 		expect(migrate?.status).not.toBe('running')
 		expect(run.done).toBe(false)
 	})
-
-	it('clears it on a clean finish, so a later import reaches its own create', async () => {
-		parkImport({ slug: 'calendly', workspaceId: 'ws-a' })
-		const run = new ImportExecution(PLAN, deps)
-		await run.run()
-		expect(resumableImport('calendly', 'ws-a')).toBe(false)
-	})
 })

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

@@ -1,4 +1,4 @@
-import { WorkspaceService } from '$lib/gen'
+import { UserService, WorkspaceService } from '$lib/gen'
 import { switchWorkspace } from '$lib/storeUtils'
 import { userStore, workspaceStore } from '$lib/stores'
 import { getUserExt } from '$lib/user'
@@ -13,7 +13,7 @@ import type {
 	ProjectMigration
 } from '$lib/components/workspaceSettings/projectBundle'
 import { planWorkspaceId, type ImportPlan } from './plan'
-import { clearParkedImport, parkImport, resumableImport } from './parking'
+import { probeWorkspace } from './probe'
 
 /**
  * The only thing in the wizard that changes anything. It takes a finished plan and
@@ -143,19 +143,9 @@ 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()
 	}
 
@@ -176,13 +166,12 @@ export class ImportExecution {
 	/**
 	 * 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.
+	 * Deliberately not satisfied by having *adopted* one. `workspace.owner` is enough to know
+	 * a create can be skipped — the id is one this user made — but not enough to offer to
+	 * delete it, because `owner` is an identity, not a run: a second import by the same person
+	 * into the same id looks identical. Skipping a create wrongly is recoverable; deleting a
+	 * workspace is not, so an adopted run finishes the import and leaves the undo to the run
+	 * that did the creating.
 	 */
 	get createdWorkspace(): boolean {
 		return this.#workspaceCreated
@@ -216,9 +205,9 @@ export class ImportExecution {
 	 * in flight — `installProject` takes no signal — so this stops the run at the next phase
 	 * boundary instead, which is as far as "stops where it is" can honestly go.
 	 *
-	 * Its other job is to keep the parked workspace: a run that clears parking on its way out
-	 * would make the link the user was told to come back to create the workspace a second
-	 * time and fail with "already exists".
+	 * The workspace it created stays, and the run stays resumable: coming back to the link
+	 * re-probes the instance, finds the workspace, and carries on rather than trying to create
+	 * it a second time.
 	 */
 	#abandoned = false
 
@@ -276,11 +265,14 @@ export class ImportExecution {
 			await this.#adoptUser(d.workspaceId)
 			return d.workspaceId
 		}
-		// 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. `#resumed` covers
-		// the same ground across a reload, where the field starts false again.
-		if (!this.#workspaceCreated && !this.#resumed) {
+		// Asked of the instance, not remembered. A retry after entering it failed must not
+		// run the create again — that would only report the id as taken by the workspace this
+		// run just made — and after a reload the field is false again while the workspace is
+		// still there. `ours` is what makes adopting it safe: an id that exists but belongs to
+		// someone else is not this run's work, and importing into it would be importing into
+		// a stranger's workspace.
+		const already = await probeWorkspace(d.id, await this.#email())
+		if (!this.#workspaceCreated && !(already.exists && already.ours)) {
 			this.#set('create', 'running')
 			try {
 				await WorkspaceService.createWorkspace({
@@ -293,9 +285,6 @@ 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)
@@ -426,11 +415,6 @@ export class ImportExecution {
 		// 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 — unless the user left mid-run, in which case
-		// the workspace this created is exactly what the link they were told to return to
-		// has to find.
-		if (!this.#abandoned) clearParkedImport()
 		if (failed > 0) this.error = `${failed} item${failed === 1 ? '' : 's'} failed to import.`
 	}
 
@@ -451,6 +435,21 @@ export class ImportExecution {
 		}
 	}
 
+	/**
+	 * Who we are, for the ownership check. `$userStore` is workspace-scoped and this page is
+	 * reparented out of `(logged)`, so it is unset until a run adopts one — `globalWhoami` is
+	 * the identity that exists before any workspace does.
+	 */
+	async #email(): Promise<string | undefined> {
+		const known = get(userStore)?.email
+		if (known) return known
+		try {
+			return (await UserService.globalWhoami()).email
+		} catch {
+			return undefined
+		}
+	}
+
 	/** Undoes the one thing this run created, when the user asks for it. */
 	async deleteCreatedWorkspace(): Promise<void> {
 		const d = this.#plan.destination
@@ -463,8 +462,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()
+		// The id is free again; the next `#ensureWorkspace` asks the instance and finds it
+		// gone, so a retry creates rather than adopts.
 		this.#set('create', 'pending')
 		this.done = false
 		this.results = []

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

@@ -1,39 +0,0 @@
-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

@@ -1,62 +0,0 @@
-/**
- * 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
-	}
-}

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

@@ -0,0 +1,84 @@
+import { describe, expect, it } from 'vitest'
+
+import { expectedPaths, expectedTables } from './probe'
+
+/**
+ * These two are inference over what the export happens to say, so they are the part of the
+ * probe that can be wrong while everything still type-checks. The network reads around them
+ * either answer or throw.
+ */
+
+describe('expectedTables', () => {
+	// The shape `datatableSchemaSql.ts` emits, and what every published project carries.
+	const REAL = `BEGIN;
+CREATE SCHEMA IF NOT EXISTS "calendly";
+CREATE TABLE IF NOT EXISTS "calendly"."config" (
+  "id" SERIAL NOT NULL,
+  "host_name" text NOT NULL DEFAULT ''::text
+);
+CREATE TABLE IF NOT EXISTS "calendly"."bookings" ( "id" SERIAL NOT NULL );
+COMMIT;`
+
+	it('reads every table a migration creates', () => {
+		expect(expectedTables(REAL)).toEqual(['calendly.config', 'calendly.bookings'])
+	})
+
+	it('does not mistake the schema for a table', () => {
+		expect(expectedTables(REAL)).not.toContain('calendly')
+	})
+
+	it('reads the form without IF NOT EXISTS', () => {
+		expect(expectedTables('CREATE TABLE "bitly"."links" (id int)')).toEqual(['bitly.links'])
+	})
+
+	it('is case- and whitespace-insensitive the way SQL is', () => {
+		expect(expectedTables('create   table\n  "a" . "b" (x int)')).toEqual(['a.b'])
+	})
+
+	it('reports each table once, however many times it is named', () => {
+		const sql = 'CREATE TABLE "a"."b" (x int); CREATE TABLE IF NOT EXISTS "a"."b" (x int);'
+		expect(expectedTables(sql)).toEqual(['a.b'])
+	})
+
+	/**
+	 * The answer that keeps a caller honest. An unquoted or unqualified `CREATE TABLE` is
+	 * something this cannot resolve — the schema would come from `search_path` at run time —
+	 * so it reads as nothing expected, and the caller treats that as "cannot tell" rather than
+	 * as "no tables, so the migration must have run".
+	 */
+	it('claims nothing about SQL it cannot resolve', () => {
+		expect(expectedTables('CREATE TABLE links (id int)')).toEqual([])
+		expect(expectedTables('CREATE TABLE bitly.links (id int)')).toEqual([])
+		expect(expectedTables('')).toEqual([])
+	})
+})
+
+describe('expectedPaths', () => {
+	const EXPORT = {
+		project: { slug: 'calendly', name: 'Calendly', summary: '', readme: null },
+		scripts: [{ path: 'f/calendly/book_slot' }],
+		flows: [],
+		apps: [{ path: 'f/calendly/booking' }],
+		resources: [{ path: 'f/calendly/smtp' }],
+		triggers: [],
+		migrations: []
+	} as any
+
+	it('lists what the import will write, across every kind', () => {
+		expect(expectedPaths(EXPORT, 'calendly')).toEqual([
+			'f/calendly/book_slot',
+			'f/calendly/booking',
+			'f/calendly/smtp'
+		])
+	})
+
+	// `installProject` retargets into the chosen folder, so the paths to look for are the
+	// retargeted ones — checking the export's own would look for stubs that never landed.
+	it('follows the folder the import was pointed at', () => {
+		expect(expectedPaths(EXPORT, 'elsewhere')).toEqual([
+			'f/elsewhere/book_slot',
+			'f/elsewhere/booking',
+			'f/elsewhere/smtp'
+		])
+	})
+})

frontend/src/lib/importWizard/probe.ts

@@ -0,0 +1,136 @@
+/**
+ * What is already true in the destination, read from the destination itself.
+ *
+ * The wizard used to remember what a run had done — a note in `sessionStorage` saying "this
+ * run created workspace X" — which a reload could outlive but a stale entry could also
+ * outlive the workspace it named. Everything here is asked of the instance instead, so there
+ * is nothing to go stale and nothing to clear: the plan in the URL says what should exist,
+ * and these functions say what does.
+ *
+ * The one thing the instance cannot answer is *which tables a migration was supposed to
+ * create*. That is inferred from the SQL the project ships (`expectedTables`), because the
+ * export states only what to run, never what running it should produce.
+ */
+
+import { ResourceService, ScriptService, FlowService, AppService, WorkspaceService } from '$lib/gen'
+import type {
+	ProjectExport,
+	ProjectMigration
+} from '$lib/components/workspaceSettings/projectBundle'
+
+/**
+ * The tables a migration creates, as `schema.table`, read off its `CREATE TABLE` statements.
+ *
+ * Inference, not a contract: the export ships SQL and nothing else, so this is the only way
+ * to check a migration's work without a record of it having run. It deliberately reads only
+ * the shape this project's generator emits (`datatableSchemaSql.ts` always writes the
+ * schema-qualified, quoted form) — anything hand-edited into a different shape simply reads
+ * as no expected tables, which makes the caller fall back to "cannot tell" rather than to a
+ * confident wrong answer.
+ */
+export function expectedTables(sql: string): string[] {
+	const out: string[] = []
+	const re = /CREATE\s+TABLE\s+(?:IF\s+NOT\s+EXISTS\s+)?"([^"]+)"\s*\.\s*"([^"]+)"/gi
+	let m: RegExpExecArray | null
+	while ((m = re.exec(sql)) !== null) out.push(`${m[1]}.${m[2]}`)
+	return [...new Set(out)]
+}
+
+/** Every path the export will write, under the folder the import is targeting. */
+export function expectedPaths(exportData: ProjectExport, folder: string): string[] {
+	const from = exportData.project.slug
+	const rewrite = (p: string) => (folder === from ? p : p.replace(`f/${from}/`, `f/${folder}/`))
+	return [...exportData.scripts, ...exportData.flows, ...exportData.apps, ...exportData.resources]
+		.map((i: any) => String(i.path))
+		.map(rewrite)
+}
+
+export interface WorkspaceState {
+	/** The user is a member of a workspace with this id. */
+	exists: boolean
+	/** …and it is one they own, so a run of theirs is what made it. */
+	ours: boolean
+}
+
+/**
+ * Whether the destination workspace is there, and whether it is the caller's.
+ *
+ * `listWorkspaces` answers both in one call: it returns only workspaces the caller is a
+ * member of, each carrying the `owner` email set at creation. Ownership is what makes
+ * skipping a create safe — an id that exists but belongs to someone else is not a workspace
+ * this run made, and importing into it would be importing into a stranger's.
+ */
+export async function probeWorkspace(
+	id: string,
+	email: string | undefined
+): Promise<WorkspaceState> {
+	try {
+		const mine = await WorkspaceService.listWorkspaces()
+		const found = mine.find((w) => w.id === id)
+		if (!found) return { exists: false, ours: false }
+		return { exists: true, ours: !!email && found.owner === email }
+	} catch {
+		// Cannot tell. Reported as absent so the caller creates rather than adopts: a failed
+		// create is a clear error, adopting the wrong workspace is a silent one.
+		return { exists: false, ours: false }
+	}
+}
+
+/**
+ * Which of the paths the import would write are already there.
+ *
+ * Scoped by `pathStart` to the import's own folder, so this is four small reads rather than a
+ * workspace scan. Presence is not provenance — importing into an existing workspace that
+ * already held a path reads the same as having imported it — so callers use this to decide
+ * what is left to do, never to claim credit for what is there.
+ */
+export async function probeImportedPaths(workspace: string, folder: string): Promise<Set<string>> {
+	const pathStart = `f/${folder}/`
+	const paths = new Set<string>()
+	const collect = (rows: unknown) => {
+		for (const r of (rows as { path?: string }[] | undefined) ?? []) if (r.path) paths.add(r.path)
+	}
+	const calls = [
+		ScriptService.listScripts({ workspace, pathStart }).then(collect),
+		FlowService.listFlows({ workspace, pathStart }).then(collect),
+		AppService.listApps({ workspace, pathStart }).then(collect),
+		ResourceService.listResource({ workspace, pathStart }).then(collect)
+	]
+	// One kind failing should narrow the answer, not lose the other three: a missing path
+	// only ever means "still to do", which is the safe direction.
+	await Promise.allSettled(calls)
+	return paths
+}
+
+/**
+ * Whether a migration's tables are in the data table it targets.
+ *
+ * The ground truth for "did this run", and the only one that covers both paths
+ * `applyOneMigration` takes: it records a migration when the data table has migrations
+ * enabled, and otherwise runs the SQL once as a job that nothing remembers. The tables
+ * outlive both.
+ *
+ * `undefined` means the question could not be answered — the schema was unreadable, or the
+ * SQL named no tables this can recognise. Distinct from `false`, because "not there" invites
+ * a caller to run the migration and "cannot tell" does not.
+ */
+export async function probeMigrationApplied(
+	workspace: string,
+	migration: ProjectMigration
+): Promise<boolean | undefined> {
+	const wanted = expectedTables(migration.sql ?? '')
+	if (wanted.length === 0) return undefined
+	try {
+		const schema = (await WorkspaceService.getDatatableFullSchema({
+			workspace,
+			requestBody: { source: `datatable://${migration.datatable_name}` }
+		})) as Record<string, Record<string, unknown>>
+		const present = new Set<string>()
+		for (const [schemaName, tables] of Object.entries(schema ?? {})) {
+			for (const table of Object.keys(tables ?? {})) present.add(`${schemaName}.${table}`)
+		}
+		return wanted.every((t) => present.has(t))
+	} catch {
+		return undefined
+	}
+}