diff --git a/packages/webapp/src/AgentRulesPicker.tsx b/packages/webapp/src/AgentRulesPicker.tsx index 4739de43..fbaaeb39 100644 --- a/packages/webapp/src/AgentRulesPicker.tsx +++ b/packages/webapp/src/AgentRulesPicker.tsx @@ -76,48 +76,66 @@ export function AgentRulesPicker({ }); }; - const save = async () => { + const save = () => { if (draft === null || busy) return; const name = draft.name.trim(); if (name === '' || draft.content.trim() === '') return; + const precedingRules = rules; + const precedingValue = value; + const precedingDraft = draft; + const id = draft.id ?? crypto.randomUUID(); + const optimistic: AgentRuleView = { + id, + name, + content: draft.content, + updatedAt: Date.now(), + builtIn: false, + }; setBusy(true); setError(null); - try { - const id = draft.id ?? crypto.randomUUID(); - // The PUT returns the canonical row, so the list is updated from it - // rather than re-fetched: one round trip, and no window where the save - // succeeded but the select cannot name what it just selected. - const { rule } = await client.putAgentRule(id, { name, content: draft.content }); - setRules((current) => current.some((entry) => entry.id === rule.id) - ? current.map((entry) => entry.id === rule.id ? rule : entry) - : [...current, rule]); - onChange(rule.id); - setDraft(null); - } catch (caught) { - setError(caught instanceof Error ? caught.message : 'The rule could not be saved.'); - } finally { - setBusy(false); - } + setRules((current) => current.some((entry) => entry.id === id) + ? current.map((entry) => entry.id === id ? optimistic : entry) + : [...current, optimistic]); + onChange(id); + setDraft(null); + void client.putAgentRule(id, { name, content: draft.content }) + .then(({ rule }) => { + // The route answers with the normalized row, so the placeholder never + // becomes a second source of truth. + setRules((current) => current.map((entry) => entry.id === id ? rule : entry)); + onChange(rule.id); + }) + .catch((caught) => { + setRules(precedingRules); + onChange(precedingValue); + setDraft(precedingDraft); + setError(caught instanceof Error ? caught.message : 'The rule could not be saved.'); + }) + .finally(() => setBusy(false)); }; - const remove = async () => { + const remove = () => { if (draft === null || draft.id === null || busy) return; + const precedingRules = rules; + const precedingValue = value; + const precedingDraft = draft; + const removed = draft.id; setBusy(true); setError(null); - // The confirmation has done its job; a failure is reported in the editor - // behind it, not under a dialog still asking the same question. + // The confirmation has done its job; a rejection reopens the editor with + // its exact draft instead of leaving the destructive prompt on screen. setConfirmingDelete(false); - try { - const removed = draft.id; - await client.deleteAgentRule(removed); - setRules((current) => current.filter((entry) => entry.id !== removed)); - if (value === removed) onChange(null); - setDraft(null); - } catch (caught) { - setError(caught instanceof Error ? caught.message : 'The rule could not be deleted.'); - } finally { - setBusy(false); - } + setRules((current) => current.filter((entry) => entry.id !== removed)); + if (value === removed) onChange(null); + setDraft(null); + void client.deleteAgentRule(removed) + .catch((caught) => { + setRules(precedingRules); + onChange(precedingValue); + setDraft(precedingDraft); + setError(caught instanceof Error ? caught.message : 'The rule could not be deleted.'); + }) + .finally(() => setBusy(false)); }; return ( @@ -137,7 +155,7 @@ export function AgentRulesPicker({ id={selectId} aria-label="Agent rules document" value={value ?? ''} - disabled={disabled} + disabled={disabled || busy} onChange={(event) => { const next = event.currentTarget.value; if (next === NEW_RULE_OPTION) { @@ -156,7 +174,7 @@ export function AgentRulesPicker({ diff --git a/packages/webapp/test/WorkspaceDetailsDialog.test.tsx b/packages/webapp/test/WorkspaceDetailsDialog.test.tsx index c7bcf19a..c04fcd96 100644 --- a/packages/webapp/test/WorkspaceDetailsDialog.test.tsx +++ b/packages/webapp/test/WorkspaceDetailsDialog.test.tsx @@ -1,10 +1,13 @@ import { act, useCallback, useState } from 'react'; import type { + AgentRuleView, MachineState, MachineResponse, MachineType, MachineView, OrgCredentialView, + PutAgentRuleRequest, + PutAgentRuleResponse, WorkspaceMemberView, WorkspaceMemberResponse, } from '@blitzos/schema'; @@ -80,6 +83,22 @@ const workspace = workspaceModelFixture({ members: [ada, grace], }); +const builtInRule: AgentRuleView = { + id: null, + name: 'Default (built-in)', + content: '# Blitz box — agent rules\n', + updatedAt: null, + builtIn: true, +}; + +const orgRule: AgentRuleView = { + id: 'rule-1', + name: 'House rules', + content: '# House rules\n', + updatedAt: 3, + builtIn: false, +}; + function client(overrides: Partial = {}): ControlPlaneClient { return { listWorkspaceRepos: vi.fn().mockResolvedValue({ repos: [] }), @@ -566,8 +585,9 @@ describe('WorkspaceDetailsDialog', () => { expect(save()?.disabled).toBe(false); const saveButton = save(); await act(async () => saveButton?.click()); - expect(saveButton?.textContent).toBe('Saving…'); - expect(name.disabled).toBe(true); + expect(saveButton?.textContent).toBe('Save settings'); + expect(saveButton?.disabled).toBe(true); + expect(name.disabled).toBe(false); // The default machine type and the agent rule were never touched, so they // travel as absent fields rather than as a restatement of what is stored. expect(updateWorkspace).toHaveBeenCalledWith(workspace.id, { @@ -588,6 +608,48 @@ describe('WorkspaceDetailsDialog', () => { await view.unmount(); }); + it('commits settings immediately and restores the snapshot with an error on rejection', async () => { + const request = deferred>>(); + const updateWorkspace = vi.fn(() => request.promise); + const view = await render(dialog({ client: client({ updateWorkspace }) })); + await settle(); + await act(async () => tab(view.container, 'Settings')?.click()); + + const name = view.container.querySelector('[aria-label="Workspace name"]'); + const toggle = view.container.querySelector( + '[aria-label="Provision a machine when a member is added"]', + ); + if (name === null || toggle === null) throw new Error('the settings fields are missing'); + await act(async () => { + typeInto(name, 'renamed-workspace'); + toggle.click(); + }); + const save = [...view.container.querySelectorAll('button')] + .find((button) => button.textContent === 'Save settings'); + await act(async () => { + save?.click(); + await Promise.resolve(); + }); + + expect(updateWorkspace).toHaveBeenCalledWith(workspace.id, { + name: 'renamed-workspace', + autoProvision: false, + }); + expect(save?.textContent).toBe('Save settings'); + expect(save?.disabled).toBe(true); + expect(name.disabled).toBe(false); + + request.reject(new ApiRequestError('settings conflict', 409, 'poll')); + await settle(); + expect(name.value).toBe(workspace.serverName); + expect(toggle.checked).toBe(workspace.autoProvision); + expect(view.container.querySelector('.webapp-error-dialog')?.textContent) + .toContain('Couldn’t save workspace settings'); + expect(view.container.querySelector('.webapp-error-dialog')?.textContent) + .toContain('Status: HTTP 409'); + await view.unmount(); + }); + it('adds and removes a repository from Settings', async () => { const addWorkspaceRepo = vi.fn().mockResolvedValue({ repos: [{ repo: 'acme/tools', private: false }], @@ -624,6 +686,234 @@ describe('WorkspaceDetailsDialog', () => { await view.unmount(); }); + it('adds a pending repository immediately and removes it with an error on rejection', async () => { + const request = deferred>>(); + const addWorkspaceRepo = vi.fn(() => request.promise); + const view = await render(dialog({ client: client({ addWorkspaceRepo }) })); + await settle(); + await act(async () => tab(view.container, 'Settings')?.click()); + + const field = view.container.querySelector('[aria-label="Repository"]'); + if (field === null) throw new Error('the settings tab has no repository field'); + await act(async () => typeInto(field, 'acme/tools')); + const add = [...view.container.querySelectorAll('button')] + .find((button) => button.textContent === 'Add repository'); + await act(async () => { + add?.click(); + await Promise.resolve(); + }); + + expect(addWorkspaceRepo).toHaveBeenCalledWith(workspace.id, { repo: 'acme/tools' }); + expect(view.container.querySelector('button[aria-label="Remove acme/tools"]')) + .not.toBeNull(); + + request.reject(new ApiRequestError('repository refused', 422, null)); + await settle(); + expect(view.container.querySelector('button[aria-label="Remove acme/tools"]')).toBeNull(); + expect(view.container.querySelector('.webapp-error-dialog')?.textContent) + .toContain('Couldn’t add repository'); + expect(view.container.querySelector('.webapp-error-dialog')?.textContent) + .toContain('Status: HTTP 422'); + await view.unmount(); + }); + + it('removes a repository immediately and restores its index with an error on rejection', async () => { + const request = deferred(); + const removeWorkspaceRepo = vi.fn(() => request.promise); + const repos = [ + { repo: 'acme/first', private: false }, + { repo: 'acme/tools', private: true }, + { repo: 'acme/last', private: false }, + ]; + const view = await render(dialog({ + client: client({ + listWorkspaceRepos: vi.fn().mockResolvedValue({ repos }), + removeWorkspaceRepo, + }), + })); + await settle(); + await act(async () => tab(view.container, 'Settings')?.click()); + + const remove = view.container.querySelector( + 'button[aria-label="Remove acme/tools"]', + ); + await act(async () => { + remove?.click(); + await Promise.resolve(); + }); + expect(removeWorkspaceRepo).toHaveBeenCalledWith(workspace.id, 'acme/tools'); + expect(view.container.querySelector('button[aria-label="Remove acme/tools"]')).toBeNull(); + + request.reject(new ApiRequestError('repository is required', 409, null)); + await settle(); + expect([...view.container.querySelectorAll('.workspace-repo-name strong')] + .map((entry) => entry.textContent)).toEqual(repos.map(({ repo }) => repo)); + expect(view.container.querySelector('.webapp-error-dialog')?.textContent) + .toContain('Couldn’t remove repository'); + expect(view.container.querySelector('.webapp-error-dialog')?.textContent) + .toContain('Status: HTTP 409'); + await view.unmount(); + }); + + it('creates and selects an agent rule immediately, then restores the editor on rejection', async () => { + const request = deferred(); + const putAgentRule = vi.fn((_id: string, _input: PutAgentRuleRequest) => request.promise); + const view = await render(dialog({ + client: client({ + listAgentRules: vi.fn().mockResolvedValue({ rules: [builtInRule, orgRule] }), + putAgentRule, + }), + })); + await settle(); + await act(async () => tab(view.container, 'Settings')?.click()); + + const select = view.container.querySelector( + 'select[aria-label="Agent rules document"]', + ); + const selectSetter = Object.getOwnPropertyDescriptor( + HTMLSelectElement.prototype, + 'value', + )?.set; + const newRuleValue = [...(select?.options ?? [])] + .find((option) => option.textContent === 'New rule…')?.value; + if (select === null || selectSetter === undefined || newRuleValue === undefined) { + throw new Error('the new-rule option is missing'); + } + await act(async () => { + selectSetter.call(select, newRuleValue); + select.dispatchEvent(new Event('change', { bubbles: true })); + }); + const name = view.container.querySelector( + 'input[aria-label="Agent rules name"]', + ); + const content = view.container.querySelector( + 'textarea[aria-label="Agent rules content"]', + ); + if (name === null || content === null) throw new Error('the rules editor is missing'); + await act(async () => { + typeInto(name, 'Review rules'); + typeInto(content, '# Review rules\n'); + }); + const save = [...view.container.querySelectorAll('button')] + .find((button) => button.textContent === 'Save rules'); + await act(async () => { + save?.click(); + await Promise.resolve(); + }); + + const pendingId = putAgentRule.mock.calls[0]?.[0]; + expect(pendingId).toEqual(expect.any(String)); + expect(view.container.querySelector('.blueprint-agent-rules-dialog')).toBeNull(); + expect(select.value).toBe(pendingId); + expect([...select.options].map((option) => option.textContent)).toContain('Review rules'); + + request.reject(new Error('rule create refused')); + await settle(); + expect(select.value).toBe(''); + expect([...select.options].map((option) => option.textContent)).not.toContain('Review rules'); + expect(view.container.querySelector( + 'input[aria-label="Agent rules name"]', + )?.value).toBe('Review rules'); + expect(view.container.querySelector('.blueprint-agent-rules-dialog [role="alert"]') + ?.textContent).toContain('rule create refused'); + await view.unmount(); + }); + + it('updates an agent rule immediately, then restores the canonical snapshot on rejection', async () => { + const request = deferred(); + const putAgentRule = vi.fn((_id: string, _input: PutAgentRuleRequest) => request.promise); + const view = await render(dialog({ + workspace: { ...workspace, agentRuleId: orgRule.id }, + client: client({ + listAgentRules: vi.fn().mockResolvedValue({ rules: [builtInRule, orgRule] }), + putAgentRule, + }), + })); + await settle(); + await act(async () => tab(view.container, 'Settings')?.click()); + const select = view.container.querySelector( + 'select[aria-label="Agent rules document"]', + ); + const edit = view.container.querySelector('.blueprint-agent-rules-edit'); + await act(async () => edit?.click()); + const name = view.container.querySelector( + 'input[aria-label="Agent rules name"]', + ); + if (select === null || name === null) throw new Error('the rules editor is missing'); + await act(async () => typeInto(name, 'Revised rules')); + const save = [...view.container.querySelectorAll('button')] + .find((button) => button.textContent === 'Save rules'); + await act(async () => { + save?.click(); + await Promise.resolve(); + }); + + expect(putAgentRule).toHaveBeenCalledWith(orgRule.id, { + name: 'Revised rules', + content: orgRule.content, + }); + expect(view.container.querySelector('.blueprint-agent-rules-dialog')).toBeNull(); + expect([...select.options].find((option) => option.value === orgRule.id)?.textContent) + .toBe('Revised rules'); + + request.reject(new Error('rule update refused')); + await settle(); + expect(select.value).toBe(orgRule.id); + expect([...select.options].find((option) => option.value === orgRule.id)?.textContent) + .toBe(orgRule.name); + expect(view.container.querySelector( + 'input[aria-label="Agent rules name"]', + )?.value).toBe('Revised rules'); + expect(view.container.querySelector('.blueprint-agent-rules-dialog [role="alert"]') + ?.textContent).toContain('rule update refused'); + await view.unmount(); + }); + + it('deletes an agent rule immediately, then restores its selection and editor on rejection', async () => { + const request = deferred(); + const deleteAgentRule = vi.fn(() => request.promise); + const view = await render(dialog({ + workspace: { ...workspace, agentRuleId: orgRule.id }, + client: client({ + listAgentRules: vi.fn().mockResolvedValue({ rules: [builtInRule, orgRule] }), + deleteAgentRule, + }), + })); + await settle(); + await act(async () => tab(view.container, 'Settings')?.click()); + const select = view.container.querySelector( + 'select[aria-label="Agent rules document"]', + ); + if (select === null) throw new Error('the rules picker is missing'); + await act(async () => { + view.container.querySelector('.blueprint-agent-rules-edit')?.click(); + }); + await act(async () => { + view.container.querySelector('.blueprint-agent-rules-delete')?.click(); + }); + await act(async () => { + view.container.querySelector('.webapp-confirmation-confirm')?.click(); + await Promise.resolve(); + }); + + expect(deleteAgentRule).toHaveBeenCalledWith(orgRule.id); + expect(view.container.querySelector('.webapp-confirmation-dialog')).toBeNull(); + expect(view.container.querySelector('.blueprint-agent-rules-dialog')).toBeNull(); + expect(select.value).toBe(''); + expect([...select.options].map((option) => option.textContent)).not.toContain(orgRule.name); + + request.reject(new Error('rule delete refused')); + await settle(); + expect(select.value).toBe(orgRule.id); + expect([...select.options].map((option) => option.textContent)).toContain(orgRule.name); + expect(view.container.querySelector( + 'input[aria-label="Agent rules name"]', + )?.value).toBe(orgRule.name); + expect(view.container.querySelector('.blueprint-agent-rules-dialog [role="alert"]') + ?.textContent).toContain('rule delete refused'); + await view.unmount(); + }); + it('shows a member the settings without the controls that write them', async () => { const view = await render(dialog({ workspace: { ...workspace, myRole: 'member' },