From daf827a168081eec806856f7417d881152fe5066 Mon Sep 17 00:00:00 2001 From: Dan Lynch Date: Sun, 9 Aug 2026 07:43:43 +0000 Subject: [PATCH] fix(desktop): make user delete work, refetch panels on a project change MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Deleting a user did nothing in the most common case: the trash button was disabled for the only admin, which in a one-user project is the only row there is. The store allows it (removing the last account just disables UI logins) — only losing the last admin *while other users remain* is a lock-out. The rule now lives in one place both the store and the UI agree on, and rejected store writes are shown instead of being swallowed by an unhandled promise rejection. Panels also refetched on the project *name* only, so switching the project in use — or importing over it, or clearing the store — could leave a panel showing what it fetched on mount. Every project-scoped hook now takes a ProjectScope { project, rev }, and rev is bumped whenever the store changes underneath the panels. --- .../desktop/__tests__/access-rules.test.ts | 35 ++++++ packages/desktop/src/renderer/App.tsx | 116 ++++++++++++++---- .../desktop/src/renderer/lib/access-rules.ts | 28 +++++ .../desktop/src/renderer/lib/use-wavegrid.ts | 50 +++++--- .../src/renderer/routes/access-route.tsx | 22 ++-- 5 files changed, 198 insertions(+), 53 deletions(-) create mode 100644 packages/desktop/__tests__/access-rules.test.ts create mode 100644 packages/desktop/src/renderer/lib/access-rules.ts diff --git a/packages/desktop/__tests__/access-rules.test.ts b/packages/desktop/__tests__/access-rules.test.ts new file mode 100644 index 0000000..f6867ce --- /dev/null +++ b/packages/desktop/__tests__/access-rules.test.ts @@ -0,0 +1,35 @@ +import { roleChangeBlock, userRemovalBlock } from '@/renderer/lib/access-rules'; +import type { UserAccount } from '@/types/ipc'; + +const admin = (username: string): UserAccount => ({ username, role: 'admin' }); +const operator = (username: string): UserAccount => ({ username, role: 'operator' }); + +describe('userRemovalBlock', () => { + it('allows removing the only account, admin or not', () => { + expect(userRemovalBlock([admin('dan')], admin('dan'))).toBeNull(); + expect(userRemovalBlock([operator('dan')], operator('dan'))).toBeNull(); + }); + + it('refuses to remove the only admin while other users remain', () => { + const users = [admin('dan'), operator('crew')]; + expect(userRemovalBlock(users, admin('dan'))).toMatch(/last admin/i); + }); + + it('allows removing an operator, and an admin when another admin remains', () => { + const users = [admin('dan'), admin('sam'), operator('crew')]; + expect(userRemovalBlock(users, operator('crew'))).toBeNull(); + expect(userRemovalBlock(users, admin('dan'))).toBeNull(); + }); +}); + +describe('roleChangeBlock', () => { + it('refuses to demote the last admin even when they are the only account', () => { + expect(roleChangeBlock([admin('dan')], admin('dan'), 'operator')).toMatch(/demoted/i); + }); + + it('allows promotion and demotion while another admin remains', () => { + const users = [admin('dan'), admin('sam')]; + expect(roleChangeBlock(users, admin('dan'), 'operator')).toBeNull(); + expect(roleChangeBlock(users, operator('crew'), 'admin')).toBeNull(); + }); +}); diff --git a/packages/desktop/src/renderer/App.tsx b/packages/desktop/src/renderer/App.tsx index ee177b8..70f4dd0 100644 --- a/packages/desktop/src/renderer/App.tsx +++ b/packages/desktop/src/renderer/App.tsx @@ -1,4 +1,4 @@ -import { Activity, Cog, Cpu, FolderKanban, Lightbulb, MonitorPlay, Radio, ShieldCheck, SlidersHorizontal } from 'lucide-react'; +import { Activity, AlertTriangle, Cog, Cpu, FolderKanban, Lightbulb, MonitorPlay, Radio, ShieldCheck, SlidersHorizontal, X } from 'lucide-react'; import * as React from 'react'; import { type AppLinkRenderer } from '@/components/ui/app-bar'; @@ -75,6 +75,16 @@ export function App() { const { projects, loaded, refresh, use, create, remove } = useProjects(); const presets = usePresets(); + // Bumped whenever the store changes underneath the panels (project in use, + // import, clear-all) so every project-scoped panel refetches — a panel bound + // to the same project name would otherwise keep its mount-time data. + const [dataRev, setDataRev] = React.useState(0); + const invalidateProjectData = React.useCallback(() => setDataRev((n) => n + 1), []); + + // The action that just failed, if any. Store writes can refuse (the last admin, + // a name clash), and a silently swallowed rejection looks like a dead button. + const [actionError, setActionError] = React.useState(null); + // Boot splash: keep the cube loader up until the project registry has landed // AND one full animation cycle has played, so it never flashes for a frame. const [splashCycleDone, setSplashCycleDone] = React.useState(false); @@ -85,27 +95,35 @@ export function App() { const showSplash = !loaded || !splashCycleDone; const activeProject = projects.find((p) => p.active)?.name ?? status.project ?? null; + const activeScope = React.useMemo( + () => ({ project: activeProject, rev: dataRev }), + [activeProject, dataRev] + ); const { devices, refresh: refreshDevices, rename: renameDevice, assignShard } = - useDevices(activeProject); + useDevices(activeScope); // The project whose config the editor is bound to — defaults to the active one, // overridden when the operator clicks "Config" on a specific project row. const [configProject, setConfigProject] = React.useState(null); const editingProject = configProject ?? activeProject; + const editingScope = React.useMemo( + () => ({ project: editingProject, rev: dataRev }), + [editingProject, dataRev] + ); const { config, loading: configLoading, refresh: refreshConfig, save: saveConfig } = - useProjectConfig(editingProject); + useProjectConfig(editingScope); const { users, refresh: refreshUsers, add: addUser, remove: removeUser, setRole: setUserRole - } = useProjectUsers(editingProject); + } = useProjectUsers(editingScope); const { sessions, refresh: refreshSessions, revoke: revokeSession - } = useSessions(editingProject); + } = useSessions(editingScope); const { keys, refresh: refreshKeys, @@ -114,12 +132,12 @@ export function App() { setRole: setKeyRole, remove: removeKey, removeAll: removeAllKeys - } = useAccessKeys(editingProject); + } = useAccessKeys(editingScope); const { secrets, refresh: refreshSecrets, generate: generateSecrets - } = useProjectSecrets(editingProject); + } = useProjectSecrets(editingScope); const { view: lightMap, loading: lightMapLoading, @@ -130,17 +148,22 @@ export function App() { autoMap: autoMapLights, identify: identifyLight, identifyClear: identifyClearLights - } = useLightMap(editingProject); + } = useLightMap(editingScope); const { info: storeInfo, refresh: refreshStore, clear: clearStore } = useStore(); const { report: doctorReport, loading: doctorLoading, error: doctorError, refresh: refreshDoctor - } = useDoctor(activeProject); - const { target: oscTarget, refresh: refreshOsc, save: saveOsc } = useOscTarget(editingProject); + } = useDoctor(activeScope); + const { target: oscTarget, refresh: refreshOsc, save: saveOsc } = useOscTarget(editingScope); const discovery = useDiscovery(); - const { exportProject, importProject } = useTransfer(refresh); + const { exportProject, importProject } = useTransfer( + React.useCallback(async () => { + await refresh(); + invalidateProjectData(); + }, [refresh, invalidateProjectData]) + ); // A failed start is reported through `status.lastError` (pushed by the main // process), so the rejection here is expected and not re-thrown. @@ -177,6 +200,7 @@ export function App() { // panel (config, lights, devices, access) follows the project in use // instead of the one last inspected. setConfigProject(null); + invalidateProjectData(); if (status.running && status.project !== name) { await window.wavegrid.brain.start(name).catch(() => undefined); } @@ -184,7 +208,7 @@ export function App() { setBusy(false); } }, - [use, status.running, status.project] + [use, invalidateProjectData, status.running, status.project] ); const onCreate = React.useCallback( @@ -192,11 +216,12 @@ export function App() { setBusy(true); try { await create(input); + invalidateProjectData(); } finally { setBusy(false); } }, - [create] + [create, invalidateProjectData] ); const onRemove = React.useCallback( @@ -205,11 +230,12 @@ export function App() { try { await remove(name); setConfigProject((cur) => (cur === name ? null : cur)); + invalidateProjectData(); } finally { setBusy(false); } }, - [remove] + [remove, invalidateProjectData] ); const onEditConfig = React.useCallback((name: string) => { @@ -217,15 +243,34 @@ export function App() { setRoute('config'); }, []); + /** + * Run a store write with the busy flag held, surfacing a refusal instead of + * losing it: the store rejects some writes on purpose (removing the last + * admin, an unknown user), and an unhandled rejection here reads as a button + * that does nothing. + */ const withBusy = React.useCallback(async (fn: () => Promise): Promise => { setBusy(true); + setActionError(null); try { return await fn(); + } catch (e) { + setActionError(e instanceof Error ? e.message : String(e)); + throw e; } finally { setBusy(false); } }, []); + /** Fire-and-forget variant for buttons that don't consume a result — the + * failure is already on screen, so the rejection is deliberately dropped. */ + const runAction = React.useCallback( + (fn: () => Promise) => { + void withBusy(fn).catch(() => undefined); + }, + [withBusy] + ); + // A live brain resolved its config at startup, so a layout/port change only // reaches the artist UI (and the light map derived from it) on a restart. const onSaveConfig = React.useCallback( @@ -359,6 +404,22 @@ export function App() { }} breadcrumbs={[{ id: route, label: ROUTE_LABEL[route], current: true }]} > + {actionError && ( +
+ + + That didn’t work. {actionError} + + +
+ )} {route === 'show' && ( void refreshDoctor()} brainLive={status.running && status.project === activeProject} receiverRunning={status.receiverRunning && status.project === activeProject} - onStartReceiver={() => void withBusy(async () => { + onStartReceiver={() => runAction(async () => { await window.wavegrid.brain.startReceiver(); await refreshDoctor(); })} - onStopReceiver={() => void withBusy(async () => { + onStopReceiver={() => runAction(async () => { await window.wavegrid.brain.stopReceiver(); await refreshDoctor(); })} @@ -419,17 +480,17 @@ export function App() { sessions={sessions} secrets={secrets} onAddUser={(u, p, r) => withBusy(() => addUser(u, p, r))} - onRemoveUser={(u) => void withBusy(() => removeUser(u))} - onSetUserRole={(u, r) => void withBusy(() => setUserRole(u, r))} - onRevokeSession={(id) => void withBusy(() => revokeSession(id))} + onRemoveUser={(u) => runAction(() => removeUser(u))} + onSetUserRole={(u, r) => runAction(() => setUserRole(u, r))} + onRevokeSession={(id) => runAction(() => revokeSession(id))} onRefreshSessions={() => void refreshSessions()} keys={keys} onMintKey={(name, role) => withBusy(() => mintKey(name, role))} - onSetKeyEnabled={(name, enabled) => void withBusy(() => setKeyEnabled(name, enabled))} - onSetKeyRole={(name, role) => void withBusy(() => setKeyRole(name, role))} - onRemoveKey={(name) => void withBusy(() => removeKey(name))} - onRemoveAllKeys={() => void withBusy(() => removeAllKeys())} - onGenerateSecrets={(force) => void withBusy(() => generateSecrets(force))} + onSetKeyEnabled={(name, enabled) => runAction(() => setKeyEnabled(name, enabled))} + onSetKeyRole={(name, role) => runAction(() => setKeyRole(name, role))} + onRemoveKey={(name) => runAction(() => removeKey(name))} + onRemoveAllKeys={() => runAction(() => removeAllKeys())} + onGenerateSecrets={(force) => runAction(() => generateSecrets(force))} busy={busy} /> )} @@ -438,9 +499,9 @@ export function App() { project={editingProject} view={lightMap} loading={lightMapLoading} - onSaveMap={(name, pl) => void withBusy(() => saveLightMap(name, pl))} - onActivate={(name) => void withBusy(() => activateLightMap(name))} - onDeleteMap={(name) => void withBusy(() => deleteLightMap(name))} + onSaveMap={(name, pl) => runAction(() => saveLightMap(name, pl))} + onActivate={(name) => runAction(() => activateLightMap(name))} + onDeleteMap={(name) => runAction(() => deleteLightMap(name))} onAutoMap={autoMapLights} onIdentify={identifyLight} onIdentifyClear={identifyClearLights} @@ -482,6 +543,7 @@ export function App() { // so no route keeps showing a project that no longer exists. setConfigProject(null); await refresh(); + invalidateProjectData(); return result; } finally { setBusy(false); diff --git a/packages/desktop/src/renderer/lib/access-rules.ts b/packages/desktop/src/renderer/lib/access-rules.ts new file mode 100644 index 0000000..cf1efad --- /dev/null +++ b/packages/desktop/src/renderer/lib/access-rules.ts @@ -0,0 +1,28 @@ +import type { UserAccount, UserRole } from '@/types/ipc'; + +/** + * Why a user can't be removed, or null when they can — the same rule the store + * enforces, so the button is only ever disabled for a removal that would + * actually be refused. + * + * Losing the only admin *while other users remain* locks administration out of + * the project. Removing the very last account is fine: it leaves nobody to sign + * in, which is recoverable by adding a user, not a lock-out. + */ +export function userRemovalBlock(users: UserAccount[], target: UserAccount): string | null { + if (target.role !== 'admin') return null; + if (users.length <= 1) return null; + const admins = users.filter((u) => u.role === 'admin').length; + return admins <= 1 ? 'The last admin cannot be removed while other users exist.' : null; +} + +/** Why a user's role can't be changed, or null when it can. */ +export function roleChangeBlock( + users: UserAccount[], + target: UserAccount, + next: UserRole +): string | null { + if (target.role !== 'admin' || next === 'admin') return null; + const admins = users.filter((u) => u.role === 'admin').length; + return admins <= 1 ? 'The last admin cannot be demoted.' : null; +} diff --git a/packages/desktop/src/renderer/lib/use-wavegrid.ts b/packages/desktop/src/renderer/lib/use-wavegrid.ts index 673e2b2..dfa0db7 100644 --- a/packages/desktop/src/renderer/lib/use-wavegrid.ts +++ b/packages/desktop/src/renderer/lib/use-wavegrid.ts @@ -107,9 +107,23 @@ export function usePresets(): string[] { return presets; } +/** + * What a project-scoped panel reads: a project name plus a revision counter. + * + * Keying a refetch on the project name alone isn't enough — switching the + * project *in use* can leave a panel bound to the same name (or to a project + * whose contents changed underneath it, e.g. after an import or a clear), and + * the panel would then keep showing what it fetched on mount. Bumping `rev` + * refetches every panel regardless of the name. + */ +export interface ProjectScope { + project: string | null; + rev: number; +} + /** The active project's editable config, mirrored from the store. `save` folds * the edited fields back in (osc/sync/etc. preserved) and returns the result. */ -export function useProjectConfig(project: string | null): { +export function useProjectConfig({ project, rev }: ProjectScope): { config: EditableConfig | null; loading: boolean; refresh: () => Promise; @@ -129,7 +143,7 @@ export function useProjectConfig(project: string | null): { } finally { setLoading(false); } - }, [project]); + }, [project, rev]); const save = React.useCallback( async (next: EditableConfig) => { @@ -173,7 +187,7 @@ export function useTransfer(onChanged: () => Promise): { } /** A project's laser output target (BEYOND / FB4 / routing file / none). */ -export function useOscTarget(project: string | null): { +export function useOscTarget({ project, rev }: ProjectScope): { target: OscTarget | null; refresh: () => Promise; save: (target: OscTarget) => Promise; @@ -182,7 +196,7 @@ export function useOscTarget(project: string | null): { const refresh = React.useCallback(async () => { setTarget(project ? await window.wavegrid.osc.get(project) : null); - }, [project]); + }, [project, rev]); const save = React.useCallback( async (next: OscTarget) => { @@ -227,7 +241,7 @@ export function useDiscovery(): { /** UI login users for a project (username + role — password hashes never leave * main). add/remove/setRole write straight through to the scrypt-backed store. */ -export function useProjectUsers(project: string | null): { +export function useProjectUsers({ project, rev }: ProjectScope): { users: UserAccount[]; refresh: () => Promise; add: (username: string, password: string, role: UserRole) => Promise; @@ -242,7 +256,7 @@ export function useProjectUsers(project: string | null): { return; } setUsers(await window.wavegrid.users.list(project)); - }, [project]); + }, [project, rev]); const add = React.useCallback( async (username: string, password: string, role: UserRole) => { @@ -278,7 +292,7 @@ export function useProjectUsers(project: string | null): { /** Active UI login sessions for a project (who's logged in). Local admin reads * straight from the shared store; revoke removes the row (the client loses * access on its next token refresh — sockets are untouched). */ -export function useSessions(project: string | null): { +export function useSessions({ project, rev }: ProjectScope): { sessions: SessionInfo[]; refresh: () => Promise; revoke: (id: string) => Promise; @@ -287,7 +301,7 @@ export function useSessions(project: string | null): { const refresh = React.useCallback(async () => { setSessions(project ? await window.wavegrid.sessions.list(project) : []); - }, [project]); + }, [project, rev]); const revoke = React.useCallback( async (id: string) => { @@ -307,7 +321,7 @@ export function useSessions(project: string | null): { /** A project's access keys + controls. `mint` creates (or replaces) a named key * and returns its cleartext once, for the admin to copy; the store keeps only a * hash. Every other action just reshapes the list. */ -export function useAccessKeys(project: string | null): { +export function useAccessKeys({ project, rev }: ProjectScope): { keys: AccessKeyInfo[]; refresh: () => Promise; mint: (name: string, role: UserRole) => Promise; @@ -324,7 +338,7 @@ export function useAccessKeys(project: string | null): { return; } setKeys(await window.wavegrid.keys.list(project)); - }, [project]); + }, [project, rev]); const mint = React.useCallback( async (name: string, role: UserRole) => { @@ -374,7 +388,7 @@ export function useAccessKeys(project: string | null): { /** Required-secret status for a project (name/description/set only). `generate` * triggers one-time generation, or rotation with force=true. */ -export function useProjectSecrets(project: string | null): { +export function useProjectSecrets({ project, rev }: ProjectScope): { secrets: RequiredSecretInfo[]; refresh: () => Promise; generate: (force: boolean) => Promise; @@ -387,7 +401,7 @@ export function useProjectSecrets(project: string | null): { return; } setSecrets(await window.wavegrid.secrets.status(project)); - }, [project]); + }, [project, rev]); const generate = React.useCallback( async (force: boolean) => { @@ -408,7 +422,7 @@ export function useProjectSecrets(project: string | null): { * raw `physicalLights` the editor mutates, and the named-map library. Saving * writes a named correction map; activating one materializes it into the same * light-map.json the running brain reads (null = identity / no correction). */ -export function useLightMap(project: string | null): { +export function useLightMap({ project, rev }: ProjectScope): { view: LightMapView | null; loading: boolean; refresh: () => Promise; @@ -433,7 +447,7 @@ export function useLightMap(project: string | null): { } finally { setLoading(false); } - }, [project]); + }, [project, rev]); const saveMap = React.useCallback( async (name: string, physicalLights: number[]) => { @@ -483,7 +497,7 @@ export function useLightMap(project: string | null): { /** The project-scoped device registry, mirrored from the shared appstash store. * Renaming and shard assignment write straight through to the store — the same * records the CLI's `devices` commands manage. */ -export function useDevices(project: string | null): { +export function useDevices({ project, rev }: ProjectScope): { devices: DeviceInfo[]; refresh: () => Promise; rename: (idOrName: string, newName: string) => Promise; @@ -493,7 +507,7 @@ export function useDevices(project: string | null): { const refresh = React.useCallback(async () => { setDevices(project ? await window.wavegrid.devices.list(project) : []); - }, [project]); + }, [project, rev]); const rename = React.useCallback( async (idOrName: string, newName: string) => { @@ -523,7 +537,7 @@ export function useDevices(project: string | null): { * prints. Each collection probes the brain, so refresh is explicit (or on an * interval the Status screen owns) rather than on every render. */ -export function useDoctor(project: string | null): { +export function useDoctor({ project, rev }: ProjectScope): { report: DoctorReport | null; loading: boolean; error: string | null; @@ -547,7 +561,7 @@ export function useDoctor(project: string | null): { } finally { setLoading(false); } - }, [project]); + }, [project, rev]); return { report, loading, error, refresh }; } diff --git a/packages/desktop/src/renderer/routes/access-route.tsx b/packages/desktop/src/renderer/routes/access-route.tsx index bc7fe12..ae975a3 100644 --- a/packages/desktop/src/renderer/routes/access-route.tsx +++ b/packages/desktop/src/renderer/routes/access-route.tsx @@ -41,6 +41,7 @@ import { TableRow } from '@/components/ui/table'; import { Tabs, TabsContent, TabsList, TabsTrigger } from '@/components/ui/tabs'; +import { roleChangeBlock, userRemovalBlock } from '@/renderer/lib/access-rules'; import type { AccessKeyInfo, RequiredSecretInfo, SessionInfo, UserAccount, UserRole } from '@/types/ipc'; const ROLE_STYLE = 'border-input bg-background h-8 rounded-md border px-2 text-sm'; @@ -200,7 +201,6 @@ function UsersTab({ busy: boolean; }) { const names = users.map((u) => u.username); - const adminCount = users.filter((u) => u.role === 'admin').length; return (
@@ -231,9 +231,8 @@ function UsersTab({ {users.map((u) => { - // Don't let the last admin be demoted or removed — that would lock - // administration out of the project. - const lastAdmin = u.role === 'admin' && adminCount <= 1; + const demoteBlock = roleChangeBlock(users, u, 'operator'); + const removeBlock = userRemovalBlock(users, u); return ( {u.username} @@ -241,8 +240,8 @@ function UsersTab({