diff --git a/src/apps/officer-web/Screens/Dashboard/PluginScreen.tsx b/src/apps/officer-web/Screens/Dashboard/PluginScreen.tsx index db69dac2..30074852 100644 --- a/src/apps/officer-web/Screens/Dashboard/PluginScreen.tsx +++ b/src/apps/officer-web/Screens/Dashboard/PluginScreen.tsx @@ -1,6 +1,6 @@ import type { LayoutNode, AppRegistryMeta } from 'officerdev'; import { WorkspaceView } from 'officerdev'; -import { useDashboardState } from 'state/useDashboardState'; +import { useAuthoredLayout } from 'state/useAuthoredLayout'; // The screen every plugin route renders. THE PLUGIN DOES NOT RENDER A SCREEN. // @@ -27,7 +27,12 @@ export function PluginScreen({ layout: LayoutNode; }) { // Per-user and per-plugin, so two plugins never share a layout and a user's arrangement is their own. - const workspace = useDashboardState(`screens/plugin/${appName}`, layout); + // + // `useAuthoredLayout`, not `useDashboardState`: the STRUCTURE here is the plugin author's and comes from + // `layout` on every render, while only the sizes are the user's and persisted. That is what lets a plugin + // ship a new layout and have it reach people who have already opened the screen — and it is what stops + // the `appTypes` repair below from being written back as though the user had chosen it. See the hook. + const workspace = useAuthoredLayout(`screens/plugin/${appName}`, layout); const allowed = panels.map((panel) => panel.key); return ( diff --git a/src/workspaces/state/src/useAuthoredLayout.test.ts b/src/workspaces/state/src/useAuthoredLayout.test.ts new file mode 100644 index 00000000..4f4db107 --- /dev/null +++ b/src/workspaces/state/src/useAuthoredLayout.test.ts @@ -0,0 +1,98 @@ +import { describe, expect, test } from 'bun:test'; +import type { LayoutNode } from 'officerdev'; +import { __testing } from './useAuthoredLayout'; + +const { collectSizes, applySizes } = __testing; + +/** + * The grafting rules, which are the whole hook. + * + * Tested at this level deliberately: the React half is `useDashboardState` with a memo around it, and a + * test of that would be a test of React Query. What can actually be wrong is the tree walking — and every + * way it can be wrong is silent, because a layout with the wrong numbers in it still renders. + */ + +const panel = (id: string, appType: string): LayoutNode => ({ type: 'panel', id, appType }); + +const group = (id: string, children: { node: LayoutNode; size: number }[]): LayoutNode => ({ + type: 'group', + id, + direction: 'horizontal', + children, +}); + +/** The shape that produced the bug: two lists over a chat. */ +const source = group('root', [ + { + node: group('lists', [ + { node: panel('hosts', 'proxy-manager-hosts'), size: 62 }, + { node: panel('certs', 'proxy-manager-certificates'), size: 38 }, + ]), + size: 66, + }, + { node: panel('chat', 'proxy-manager-chat'), size: 34 }, +]); + +describe('collectSizes', () => { + test('records every group, nested ones included', () => { + expect(collectSizes(source)).toEqual({ root: [66, 34], lists: [62, 38] }); + }); + + test('a lone panel has no sizes at all', () => { + expect(collectSizes(panel('only', 'x'))).toEqual({}); + }); +}); + +describe('applySizes', () => { + test("the user's sizes win over the author's", () => { + const result = applySizes(source, { root: [50, 50], lists: [70, 30] }); + expect(collectSizes(result)).toEqual({ root: [50, 50], lists: [70, 30] }); + }); + + test("a group with no saved entry keeps the author's numbers", () => { + const result = applySizes(source, { lists: [70, 30] }); + expect(collectSizes(result)).toEqual({ root: [66, 34], lists: [70, 30] }); + }); + + // The case that makes shipping a layout change safe. Two numbers cannot describe three panels, and + // applying them would leave the new one with whatever was left over. + test("a group whose child count changed keeps the author's numbers", () => { + const result = applySizes(source, { root: [10, 20, 70], lists: [70, 30] }); + expect(collectSizes(result)).toEqual({ root: [66, 34], lists: [70, 30] }); + }); + + test('sizes saved for a group that no longer exists are ignored, not an error', () => { + const result = applySizes(source, { 'a-group-from-last-year': [1, 99] }); + expect(collectSizes(result)).toEqual({ root: [66, 34], lists: [62, 38] }); + }); + + // The whole point: whatever structure comes back from the view — normalised, repaired, rewritten — is + // discarded, and only its numbers survive. This is the corruption becoming unrepresentable. + test('a repaired structure cannot write itself back', () => { + const repaired = group('root', [ + { + node: group('lists', [ + { node: panel('hosts', 'proxy-manager-hosts'), size: 50 }, + { node: panel('certs', 'proxy-manager-certificates'), size: 50 }, + ]), + size: 80, + }, + // What the fallback did: a second copy of the first panel, in the chat's place. + { node: panel('chat', 'proxy-manager-hosts'), size: 20 }, + ]); + + const stored = applySizes(source, collectSizes(repaired)); + + // The numbers are kept, because the user may genuinely have dragged them. + expect(collectSizes(stored)).toEqual({ root: [80, 20], lists: [50, 50] }); + // The appType is not, because it was never the user's to change. + const chat = (stored as { children: { node: LayoutNode }[] }).children[1]!.node; + expect((chat as { appType: string }).appType).toBe('proxy-manager-chat'); + }); + + test('the source is not mutated', () => { + const before = JSON.stringify(source); + applySizes(source, { root: [1, 99], lists: [1, 99] }); + expect(JSON.stringify(source)).toBe(before); + }); +}); diff --git a/src/workspaces/state/src/useAuthoredLayout.ts b/src/workspaces/state/src/useAuthoredLayout.ts new file mode 100644 index 00000000..8759f824 --- /dev/null +++ b/src/workspaces/state/src/useAuthoredLayout.ts @@ -0,0 +1,106 @@ +import type { LayoutNode, LayoutGroup } from 'officerdev'; +import { useCallback, useMemo } from 'react'; +import { useDashboardState } from './useDashboardState'; + +/** + * A layout whose STRUCTURE belongs to whoever wrote the screen, and whose SIZES belong to whoever uses it. + * + * ── The bug this ends ── + * + * A locked screen persisted its whole layout tree, and that tree is almost entirely the author's work: + * `locked` means the user cannot add, move or close a panel. The only thing they can actually change is + * how big things are. So the saved blob was a copy of the author's design sitting in the user's slot, + * where it could only go stale or get damaged, and it did both: + * + * - **Stale.** Ship a new `defaultLayout` and nobody who had opened the screen ever saw it. Their copy + * won, forever, with no way back that a person could reach. + * - **Damaged.** `WorkspaceView`'s `appTypes` guard replaces an unrecognised `appType` with the fallback + * and writes the repair back — correct for a user-authored dashboard, catastrophic here. On + * 2026-08-25 a plugin layout naming a panel that did not exist yet was rewritten to two copies of its + * first panel and SAVED, so the corruption survived every reload, every fix, and every restart. The + * row had to be deleted from Postgres, which is not a thing a user can do. + * + * ── The fix, and why it is a shape rather than a rule ── + * + * The structure comes from `source` on every render and is never read back from storage. Only sizes are + * persisted. So a stale structure cannot exist, a repair has nothing to write itself into, and a plugin + * that changes its layout reaches everybody on the next load — with no version to bump, no migration, and + * nothing for an author to remember. + * + * The user keeps the one thing that was theirs. + * + * ── What is stored ── + * + * `source` with the user's sizes applied — a valid layout, because the storage validates one on both the + * route and a CHECK constraint, and because a value that is always freshly derived from source cannot rot. + * Its structure is written and never read. That asymmetry is the whole design and is why it is safe. + * + * ── Not for unlocked workspaces ── + * + * A real dashboard is the opposite case: the user genuinely authors the tree, and there the saved + * structure IS the thing worth keeping. Use `useDashboardState` directly for those. The line is exactly + * where `locked` already falls. + */ + +type Sizes = Record; + +const isGroup = (node: LayoutNode): node is LayoutGroup => node.type === 'group'; + +/** Every group's child sizes, by group id. */ +function collectSizes(node: LayoutNode, into: Sizes = {}): Sizes { + if (!isGroup(node)) return into; + into[node.id] = node.children.map((child) => child.size); + for (const child of node.children) collectSizes(child.node, into); + return into; +} + +/** + * Put saved sizes back onto the author's structure. + * + * **A group whose child count no longer matches keeps the author's sizes**, which is the case that makes + * this safe to ship a layout change into: adding a third panel to a two-panel group means the old pair of + * numbers describes a layout that no longer exists, and applying them would leave the new panel at + * whatever was left over. Dropping them silently is right — the author's proportions are a better guess + * than arithmetic on a stale pair. + */ +function applySizes(node: LayoutNode, sizes: Sizes): LayoutNode { + if (!isGroup(node)) return node; + + const saved = sizes[node.id]; + const usable = saved?.length === node.children.length ? saved : null; + + return { + ...node, + children: node.children.map((child, index) => ({ + node: applySizes(child.node, sizes), + size: usable ? usable[index]! : child.size, + })), + }; +} + +/** + * Same shape as `useDashboardState`, so a screen swaps one call and changes nothing else. + * + * `setValue` accepts a whole layout because that is what `WorkspaceView` reports — a resize hands back the + * entire tree. Everything but its sizes is discarded here, which is precisely the point: the view is free + * to report a normalised, repaired, or otherwise rewritten structure and none of it can be stored. + */ +export function useAuthoredLayout(key: string, source: LayoutNode) { + const stored = useDashboardState(key, source); + + const value = useMemo(() => applySizes(source, collectSizes(stored.value)), [source, stored.value]); + + const setValue = useCallback( + (update: LayoutNode | ((prev: LayoutNode) => LayoutNode)) => { + const next = typeof update === 'function' ? update(value) : update; + stored.setValue(applySizes(source, collectSizes(next))); + }, + // `stored.setValue` is stable per key; `value` is memoised above. + [source, value, stored], + ); + + return { key: stored.key, value, setValue, isLoaded: stored.isLoaded, loadFailed: stored.loadFailed }; +} + +/** The tree walking, exposed for its test. Not part of the hook's API — import the hook. */ +export const __testing = { collectSizes, applySizes };