From a7c9dab22c78b155ccb4594b7c4403c6da30e907 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Padez?= Date: Wed, 26 Aug 2026 04:17:30 +0000 Subject: [PATCH] a repair that cannot stick must not be written MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit useAuthoredLayout turned a harmless fallback into a hard crash, and /browser on fofos is where it showed: "This screen failed to render", react #185, maximum update depth exceeded. WorkspaceView's appTypes guard repairs an unrecognised appType and WRITES THE REPAIR BACK. that was self-limiting while the structure was persisted — the repair stuck and the next render agreed with it. with the structure coming from source, it can never stick: recomputed, rewritten, discarded, recomputed, every render, until react gives up. the browser plugin's layout names officerdev/chat, which is not one of its own panel keys, so every single render of that screen produced a repair. the same trap proxy-manager hit last night, except that one only rendered the wrong panel — this one loops. so a write whose result equals the current value is dropped. not an optimisation: it is the thing that terminates the loop. persisting an identical tree was pointless anyway. tested by grafting a repaired structure and asserting it comes back identical to the current value — which is the exact condition setValue now checks, so if it ever writes there, the loop is back. Co-Authored-By: Claude Opus 5 Officer-Session: 656f5c85-e49a-4f1f-b9f3-042a04b4d043 --- .../state/src/useAuthoredLayout.test.ts | 31 +++++++++++++++++++ src/workspaces/state/src/useAuthoredLayout.ts | 22 ++++++++++++- 2 files changed, 52 insertions(+), 1 deletion(-) diff --git a/src/workspaces/state/src/useAuthoredLayout.test.ts b/src/workspaces/state/src/useAuthoredLayout.test.ts index 4f4db107..c71b3d75 100644 --- a/src/workspaces/state/src/useAuthoredLayout.test.ts +++ b/src/workspaces/state/src/useAuthoredLayout.test.ts @@ -96,3 +96,34 @@ describe('applySizes', () => { expect(JSON.stringify(source)).toBe(before); }); }); + +/** + * The loop that broke `/browser`. + * + * `WorkspaceView` repairs an appType it does not recognise and writes the repair back. With the structure + * coming from source, that repair can never persist — so unless an identical write is dropped, it is + * recomputed and rewritten every render until React gives up (#185). + */ +describe('a repair that cannot stick must not be written', () => { + test('grafting a repaired structure yields exactly the current value', () => { + // What the guard produces: the same tree with a disallowed appType swapped for the fallback. + const repaired = group('root', [ + { + node: group('lists', [ + { node: panel('hosts', 'proxy-manager-hosts'), size: 62 }, + { node: panel('certs', 'proxy-manager-certificates'), size: 38 }, + ]), + size: 66, + }, + // `chat` replaced by the fallback — the substitution the allow-list makes every render. + { node: panel('chat', 'proxy-manager-hosts'), size: 34 }, + ]); + + const current = applySizes(source, collectSizes(source)); + const afterRepair = applySizes(source, collectSizes(repaired)); + + // Identical, because only sizes survive and the repair changed none. `setValue` compares these and + // returns without writing; if it ever writes, the render loop is back. + expect(JSON.stringify(afterRepair)).toBe(JSON.stringify(current)); + }); +}); diff --git a/src/workspaces/state/src/useAuthoredLayout.ts b/src/workspaces/state/src/useAuthoredLayout.ts index 8759f824..977b6985 100644 --- a/src/workspaces/state/src/useAuthoredLayout.ts +++ b/src/workspaces/state/src/useAuthoredLayout.ts @@ -93,7 +93,27 @@ export function useAuthoredLayout(key: string, source: LayoutNode) { const setValue = useCallback( (update: LayoutNode | ((prev: LayoutNode) => LayoutNode)) => { const next = typeof update === 'function' ? update(value) : update; - stored.setValue(applySizes(source, collectSizes(next))); + const merged = applySizes(source, collectSizes(next)); + + /* + A write that changes nothing is dropped, and this is not an optimisation — it is what stops an + infinite render loop. + + `WorkspaceView`'s `appTypes` guard repairs an unrecognised appType and WRITES THE REPAIR BACK. That + was self-limiting while the structure was persisted: the repair stuck and the next render agreed + with it. Here the structure always comes from `source`, so the repair can never stick — it is + recomputed, rewritten, discarded and recomputed, every render, forever. React gives up with + "maximum update depth exceeded" (#185) and the screen fails to render. + + Not hypothetical: the `browser` plugin's layout names `officerdev/chat`, which is not one of its own + panel keys, so every render of `/browser` produced a repair. Shipping the sizes-only hook turned a + silent, harmless fallback into a hard crash on that screen. + + Comparing the serialised value is enough — these trees are small, this runs only on a real change, + and it is the same check `setValue` would otherwise force React Query to make against a new object. + */ + if (JSON.stringify(merged) === JSON.stringify(value)) return; + stored.setValue(merged); }, // `stored.setValue` is stable per key; `value` is memoised above. [source, value, stored],