diff --git a/frontend/src/lib/components/workspaceSettings/SimpleCreateWorkspace.svelte b/frontend/src/lib/components/workspaceSettings/SimpleCreateWorkspace.svelte index 68465190aa..35bbaf4903 100644 --- a/frontend/src/lib/components/workspaceSettings/SimpleCreateWorkspace.svelte +++ b/frontend/src/lib/components/workspaceSettings/SimpleCreateWorkspace.svelte @@ -43,30 +43,34 @@ /** * Whether the policy is settled, which is what this form may not submit without: * `automateUsername` starts at the common case, and posting that guess to an instance - * that derives no usernames sends none where one is required. A load that fails settles - * it by handing over to the full form, which asks for a username outright instead of - * inferring one — so this is never true while the answer is still a guess. + * that derives no usernames sends none where one is required. `loadUsernamePolicy` + * always answers — "ask for one" when it cannot read the setting — so this turns true + * on a known answer rather than on the attempt finishing. */ let policyLoaded = $state(false) /** Someone typed while the prefill was in flight; their name wins over the suggestion. */ let nameEdited = false async function load() { - try { - const [me, policy] = await Promise.all([UserService.globalWhoami(), loadUsernamePolicy()]) - if (!nameEdited) name = defaultWorkspaceName(me.name, me.email) - automateUsername = policy.automate - suggestedUsername = policy.suggested - if (!policy.automate && !policy.suggested) advanced = true - policyLoaded = true - } catch (error) { - console.error('Could not prefill the workspace name:', error) - if (!nameEdited) name = 'My workspace' - // The policy is what failed, so there is nothing to submit against. The full form - // carries its own username field, which is the version that needs no policy. - advanced = true - policyLoaded = true + // Settled apart: the policy decides whether this form may submit at all, the suggested + // name is cosmetic, and neither failure should decide the other. + const [me, policy] = await Promise.allSettled([ + UserService.globalWhoami(), + loadUsernamePolicy() + ]) + if (!nameEdited) { + name = + me.status === 'fulfilled' + ? defaultWorkspaceName(me.value.name, me.value.email) + : 'My workspace' } + // `loadUsernamePolicy` answers "ask for one" rather than rejecting when the setting + // cannot be read, so the fallback here is the same answer by another route. + const answer = policy.status === 'fulfilled' ? policy.value : { automate: false } + automateUsername = answer.automate + suggestedUsername = answer.suggested + if (!answer.automate && !answer.suggested) advanced = true + policyLoaded = true } void load() diff --git a/frontend/src/lib/workspaceCreation.test.ts b/frontend/src/lib/workspaceCreation.test.ts index 13d63d1fd1..a79e0d0ee5 100644 --- a/frontend/src/lib/workspaceCreation.test.ts +++ b/frontend/src/lib/workspaceCreation.test.ts @@ -1,13 +1,22 @@ import { describe, expect, it, vi } from 'vitest' // The module reaches the API for the username policy and the workspace list; the name -// helper touches neither. -vi.mock('./gen', () => ({ SettingService: {}, UserService: {}, WorkspaceService: {} })) +// helper touches neither. `getGlobal` is a spy so the policy's failure path can be driven. +const getGlobal = vi.fn() +vi.mock('./gen', () => ({ + SettingService: { + get getGlobal() { + return getGlobal + } + }, + UserService: {}, + WorkspaceService: {} +})) vi.mock('./stores', () => ({ usersWorkspaceStore: { set: () => {} } })) vi.mock('./storeUtils', () => ({ switchWorkspace: () => {} })) vi.mock('./cloud', () => ({ isCloudHosted: () => false })) -import { defaultWorkspaceName, usernameFromName } from './workspaceCreation' +import { defaultWorkspaceName, loadUsernamePolicy, usernameFromName } from './workspaceCreation' describe('defaultWorkspaceName', () => { it('names the workspace after the person, not the address', () => { @@ -60,3 +69,19 @@ describe('usernameFromName', () => { expect(usernameFromName('a'.repeat(51))).toBeUndefined() }) }) + +describe('loadUsernamePolicy', () => { + // Fail-closed matters because a caller told "automated" hides its username field and + // posts none: an instance that derives none refuses that, with nowhere to supply one. + it('asks for a username when the setting cannot be read', async () => { + getGlobal.mockRejectedValueOnce(new Error('502')) + expect(await loadUsernamePolicy()).toEqual({ automate: false }) + }) + + it('automates when the setting says so, and when it is unset', async () => { + getGlobal.mockResolvedValueOnce(true) + expect(await loadUsernamePolicy()).toEqual({ automate: true }) + getGlobal.mockResolvedValueOnce(null) + expect(await loadUsernamePolicy()).toEqual({ automate: true }) + }) +}) diff --git a/frontend/src/lib/workspaceCreation.ts b/frontend/src/lib/workspaceCreation.ts index da67afba6f..b10955135d 100644 --- a/frontend/src/lib/workspaceCreation.ts +++ b/frontend/src/lib/workspaceCreation.ts @@ -68,10 +68,20 @@ export function usernameFromName(name: string): string | undefined { * requires one when it does not, so the field only exists in the second case. */ export async function loadUsernamePolicy(): Promise { - const automate = - ((await SettingService.getGlobal({ - key: 'automate_username_creation' - })) as boolean | null) ?? true + let automate: boolean + try { + automate = + ((await SettingService.getGlobal({ + key: 'automate_username_creation' + })) as boolean | null) ?? true + } catch (error) { + // Unreadable is not "automated". A caller told yes hides its username field and posts + // none, which an instance that derives none then refuses — with nowhere on screen to + // supply what it wanted. Answering no asks for one, which is right either way: an + // instance that does automate ignores a username it was sent. + console.error('Could not read the username policy; asking for one instead:', error) + return { automate: false } + } if (automate) return { automate: true } try { const me = await UserService.globalWhoami()