From a536efe68a7c95fd9127848c167b2a9ef53e448e Mon Sep 17 00:00:00 2001 From: Guilhem Lemouel Date: Fri, 14 Aug 2026 19:07:36 +0200 Subject: [PATCH] fix(frontend): keep a failed setup's claims across the redirect and rollback Co-Authored-By: Claude Opus 5 (1M context) --- .../AddDataTableWizard.svelte | 17 ++++++- .../addDataTableModel.test.ts | 46 ++++++++++++++++++- .../workspaceSettings/addDataTableModel.ts | 7 +++ .../workspaceSettings/wizardParking.ts | 16 ++++++- 4 files changed, 82 insertions(+), 4 deletions(-) diff --git a/frontend/src/lib/components/workspaceSettings/AddDataTableWizard.svelte b/frontend/src/lib/components/workspaceSettings/AddDataTableWizard.svelte index 582403bb7d..74036d03b8 100644 --- a/frontend/src/lib/components/workspaceSettings/AddDataTableWizard.svelte +++ b/frontend/src/lib/components/workspaceSettings/AddDataTableWizard.svelte @@ -338,7 +338,11 @@ parkWizard({ name: wiz.review.name, region: wiz.supabase.region, - projectName: wiz.supabase.projectName + projectName: wiz.supabase.projectName, + resourcePath, + claimedPath, + createdProjectName, + createdProjectPath }) }) @@ -365,6 +369,17 @@ poolerUnavailable = undefined if (resume) { wiz.provider = 'supabase' + // The clears above are for a fresh run. This one is the same run coming back from the + // redirect, so what it had already created is still its own to write over. + claimedPath = resume.claimedPath + createdProjectName = resume.createdProjectName + createdProjectPath = resume.createdProjectPath + leftBehind = !!(resume.claimedPath || resume.createdProjectPath) + const cut = resume.resourcePath?.lastIndexOf('/') ?? -1 + if (resume.resourcePath && cut > 0) { + wiz.review.folder = resume.resourcePath.slice(0, cut) + wiz.review.resourceName = resume.resourcePath.slice(cut + 1) + } enterStep(2) } } diff --git a/frontend/src/lib/components/workspaceSettings/addDataTableModel.test.ts b/frontend/src/lib/components/workspaceSettings/addDataTableModel.test.ts index c67eb83290..ac68d2acb7 100644 --- a/frontend/src/lib/components/workspaceSettings/addDataTableModel.test.ts +++ b/frontend/src/lib/components/workspaceSettings/addDataTableModel.test.ts @@ -17,6 +17,10 @@ vi.mock('./supabaseProvisioning', async (importOriginal) => ({ const existsVariableMock = vi.fn() const createVariableMock = vi.fn() +const getSettingsMock = vi.fn() +const editDataTableConfigMock = vi.fn() +const testDataTableConnectionMock = vi.fn() +const setupCustomInstanceDbMock = vi.fn() vi.mock('$lib/gen', () => ({ VariableService: { existsVariable: (...a: any[]) => existsVariableMock(...a), @@ -24,8 +28,12 @@ vi.mock('$lib/gen', () => ({ updateVariable: vi.fn() }, ResourceService: { existsResource: vi.fn(), createResource: vi.fn(), updateResource: vi.fn() }, - SettingService: { setupCustomInstanceDb: vi.fn() }, - WorkspaceService: { getSettings: vi.fn(), editDataTableConfig: vi.fn() } + SettingService: { setupCustomInstanceDb: (...a: any[]) => setupCustomInstanceDbMock(...a) }, + WorkspaceService: { + getSettings: (...a: any[]) => getSettingsMock(...a), + editDataTableConfig: (...a: any[]) => editDataTableConfigMock(...a), + testDataTableConnection: (...a: any[]) => testDataTableConnectionMock(...a) + } })) import { newWizardState, runSetup, type WizardState } from './addDataTableModel' @@ -114,3 +122,37 @@ describe('runSetup refusing to mint over a project it already created', () => { expect(createSupabaseProjectMock).not.toHaveBeenCalled() }) }) + +// The instance branch is the one that has to write its row before it can probe it, since the +// probe is by data table name. A database Windmill cannot store data in must not stay in the +// config -- and a probe that throws leaves exactly the same unusable row as one that says no. +describe('runSetup rolling the instance row back', () => { + function usingInstanceDb(): WizardState { + const state = newWizardState({ name: 'main', projectName: 'x', folder: 'f/team' }) + state.provider = 'instance' + state.instance = { mode: 'existing', dbName: 'shared' } + return state + } + + beforeEach(() => { + vi.clearAllMocks() + getSettingsMock.mockResolvedValue({ datatable: { datatables: {} } }) + editDataTableConfigMock.mockResolvedValue(undefined) + setupCustomInstanceDbMock.mockResolvedValue({ success: true, logs: {} }) + }) + + it('takes the row back out when the probe never answers', async () => { + testDataTableConnectionMock.mockRejectedValue(new Error('connection refused')) + const result = await runSetup(usingInstanceDb(), { + workspace: 'w', + supabaseToken: undefined, + onProgress: () => {} + } as any) + expect(result.ok).toBe(false) + expect(result.error).toContain('connection refused') + expect(result.rowRolledBack).toBe(true) + expect(result.rowWritten).toBe(false) + const lastWrite = editDataTableConfigMock.mock.calls.at(-1)?.[0] + expect(lastWrite.requestBody.settings.datatables).not.toHaveProperty('main') + }) +}) diff --git a/frontend/src/lib/components/workspaceSettings/addDataTableModel.ts b/frontend/src/lib/components/workspaceSettings/addDataTableModel.ts index ffd14baf84..353dac5b18 100644 --- a/frontend/src/lib/components/workspaceSettings/addDataTableModel.ts +++ b/frontend/src/lib/components/workspaceSettings/addDataTableModel.ts @@ -680,6 +680,13 @@ export async function runSetup(state: WizardState, deps: RunDeps): Promise { + // A probe that never answered leaves the same unusable row behind as one that + // answered no -- an unreachable database or a timeout lands here -- so it takes + // the same way out rather than the bare outer catch. + rowRolledBack = await removeRow(deps, name) + rowWritten = !rowRolledBack + throw err }) if (!report.can_create_table) { rowRolledBack = await removeRow(deps, name) diff --git a/frontend/src/lib/components/workspaceSettings/wizardParking.ts b/frontend/src/lib/components/workspaceSettings/wizardParking.ts index 3373245dee..4836fd3e4c 100644 --- a/frontend/src/lib/components/workspaceSettings/wizardParking.ts +++ b/frontend/src/lib/components/workspaceSettings/wizardParking.ts @@ -9,7 +9,21 @@ const RESUME_KEY = 'datatable_wizard_resume' -export type WizardResume = { name: string; region: string; projectName: string } +export type WizardResume = { + name: string + region: string + projectName: string + /** + * What the interrupted run had already created. Without these the resumed run meets its + * own secret variable and resource as somebody else's and refuses to write over them, + * which strands the Supabase project it just paid for. No secret is parked -- these are + * paths, and the password they name is already in the workspace. + */ + resourcePath?: string + claimedPath?: string + createdProjectName?: string + createdProjectPath?: string +} /** True while a wizard run is waiting on the Supabase redirect to come back. */ export function hasParkedWizard(): boolean {