From ce7968ac9051df1e04cc6ab987d05c2b665fec2c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Padez?= Date: Fri, 7 Aug 2026 04:10:36 +0000 Subject: [PATCH] document how the workspace/panel framework works --- CLAUDE.md | 4 + docs/workspace-panel-todo.md | 81 ++++++++++++++- docs/workspace-panels.md | 193 +++++++++++++++++++++++++++++++++++ 3 files changed, 274 insertions(+), 4 deletions(-) create mode 100644 docs/workspace-panels.md diff --git a/CLAUDE.md b/CLAUDE.md index 7d9ae012..13e5dd3f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -280,6 +280,10 @@ link-focusable). Half the app still does this; none of the new code should. - `docs/navigation-audit.md` — **authoritative** on routing/navigation: the opaque-click anti-pattern, a severity-ranked findings table, the channel-selection map and the four-phase plan +- `docs/workspace-panels.md` — how the Workspace/Panel framework works: the layout tree, how a panel is + mounted and what it knows, the URL-vs-channel split for panel-to-panel communication, and the + persistence key families. Read before building a panel app. Its defect list is + `docs/workspace-panel-todo.md`. - `docs/sidecar-topology.md` — where the sidecar architecture is going, and what was considered and dropped - `docs/working-on-officer.md` — how to run, restart and check your work on this machine - `docs/wallet-key-custody.md` — what the platform can and cannot see of the wallet diff --git a/docs/workspace-panel-todo.md b/docs/workspace-panel-todo.md index 85a822f2..2f4906d9 100644 --- a/docs/workspace-panel-todo.md +++ b/docs/workspace-panel-todo.md @@ -4,6 +4,9 @@ Living list. Add items as they are found, tick them as they land, and write the item rather than deleting it — the reason a thing was done is worth more later than a clean list. Move anything fully settled to §7. +**How the framework actually works is documented separately, in `workspace-panels.md`** — read that +first if you are new to it. This file is only the defect list and the work queue. + Findings and full reasoning: `COMMS/workspace-panel-framework-analysis-2026-08-07.md`. Every `file:line` below was opened; DB claims were run against live `officer_dev`. Paths are relative to `platform/src/workspaces/officerdev/src/` unless they start with `servers/`, `databases/` or `sidecars/`. @@ -57,6 +60,19 @@ Both are two-line fixes, and without them you cannot tell whether any later fix branches do not. A null falls to the UPDATE branch and sets a `NOT NULL` column (`databases/officer_db/src/queries/dashboards.ts:70`) → 23502. +- [ ] **`HostTerminalWrapper` never strips the prefix, so it mints phantom dashboards.** *(found + 2026-08-08, latent — `dashboards` is still 0 rows)* + `apps/Terminal/HostTerminalWrapper.tsx:12` is `` `ws-host-terminals-${dashboardId}` `` with **no + regex**, while its sibling `TerminalWrapper.tsx:13-14` correctly matches `^ws-layout-(.+)$` first. + So the key becomes `ws-host-terminals-ws-layout-` or `ws-host-terminals-screens/terminal`, the + dispatcher's `^ws-host-terminals-(.+)$` branch captures that whole string as an id, and + `upsertDashboard` **inserts a row when the id is unknown** (`queries/dashboards.ts:75-86`, + `name: data.name ?? id`). `getAllDashboardState` maps every `dashboards` row into `workspaces`, so + the garbage id surfaces in the Dashboards list as a real dashboard. + Two apps deriving a state key from the same string by two different rules is the actual defect; + the id-shaped-string-as-a-bag problem behind it is §5.9. + Latent only because `officerdev/terminal-host` is in no default layout and hidden from the picker. + - [ ] **Renaming a dashboard resurrects it as a zombie row.** `apps/Dashboards/DashboardPreview.tsx:316-318` PATCHes `ws-layout-old: null` *and* `ws-terminals-old: null` together. The first deletes the row; the second then calls @@ -116,12 +132,16 @@ these. `databases/CLAUDE.md`'s stance that the schema is the source of truth for *contents*, a CHECK on `jsonb_typeof(layout) = 'object'` is the cheap half. -- [ ] **A bare number is squatting in a framework namespace.** *(verified in the live DB)* - `apps/Soulseek/shared.ts:12` builds `screens/soulseek-zoom/${panelId}` and passes it to +- [x] **A bare number is squatting in a framework namespace.** *(verified in the live DB)* + `apps/Soulseek/shared.ts:12` built `screens/soulseek-zoom/${panelId}` and passed it to `useDashboardState`. The server routes `^screens/(.+)$` into `screens.layout`, so: `user_id 1 | soulseek-zoom/soulseek-view | jsonb_typeof = number`. That namespace belongs to - layouts. Give per-panel scalar prefs their own key family (and a matching dispatcher branch, which - the §2 `else` will otherwise start rejecting). + layouts. + **Resolved `ba664dc`** — Soulseek's private zoom became a framework feature (`LayoutPanel.zoom`), + so the scalar now rides on the layout node inside `screens/soulseek-v2` and needs no key of its + own. The squatting row is gone: `select name from screens` returns 15 rows, none `soulseek-zoom/*`. + This is also the general answer for per-panel scalar prefs — put them on the node, not in a key, + because `useDashboardState` seeds a row per key on mount. --- @@ -307,6 +327,59 @@ All the same bug: an app guessing "am I being closed?" from an unmount, or payin (`frontend.tsx:15-21`) with no per-route scoping, and `reset` is called nowhere. Authority: `docs/navigation-audit.md`. +### 5.9 The context has grown an app-config section — *(found 2026-08-08)* + +`WorkspaceContext` is 18 fields, of which the framework itself reads none of the first six. Apps never +touch the framework half, so the abstraction holds in one direction; the leak is entirely outbound. + +- [ ] **Delete `initialFilePath` and `defaultFileSort`.** Declared `WorkspaceContext.ts:14-15`, plumbed + through `WorkspaceView.tsx:19-20,30`, read only by + `apps/FileBrowser/FileBrowserApp/FileBrowserPanelWrapper.tsx:7,9,10` — and **set by zero callers**. + `DefaultFileSort` (`{field: 'name'|'size'|'type'|'date'}`) is file-browser vocabulary living in the + framework's type file, and it is re-exported from the barrel (`Workspace/index.ts:27`). Pure leak, + no payoff, entirely deletable. +- [ ] **Move `promptPrefix` onto the component, not the context.** `WorkspaceContext.ts:16` → + `Chat/ChatPanelWrapper.tsx:78` → `useEmbeddableChat.ts:107`. Set by `EmailScreen.tsx:53` and + `BrowserScreen.tsx:35`, each a screen-local ~40-word system prompt. The framework is a courier for + a string only one app understands, and the `components` prop already exists for exactly this — + Email can supply a pre-configured chat by panel id. +- [ ] **`dashboardId` is a bag whose *format* three apps parse.** It is literally `workspace.key` + (`WorkspaceView.tsx:163`). Consumers reverse-engineer meaning from its shape: + `Chat/ChatPanelWrapper.tsx:51-56` does `dashboardId === 'email' || dashboardId === 'screens/email'` + → email context, and `!startsWith('screens/')` → dashboard context — **so renaming a screen key + silently changes the agent's system context**. `TerminalWrapper.tsx:13-16` regexes it; + `HostTerminalWrapper.tsx:12` doesn't (§2). Give the context the parsed facts + (`{ kind: 'screen'|'dashboard', id }`) instead of the raw key, and the three parsers collapse. +- [ ] **`WorkspaceLayout.tsx:36` silently omits `root`, `initialFilePath`, `defaultFileSort`** — apps + inside a `DashboardPreview` fall through to the `createContext` defaults. Whatever survives the + three items above should be constructed in one place, not twice by hand. + +### 5.10 Channel hygiene — *(found 2026-08-08)* + +Cheap to fix, and prerequisites for the 5.8 migration rather than alternatives to it. + +- [ ] **Four channels are bare string literals with no constant.** `'files:refresh-signal'` (repeated in + 4 files), `'chat:selected-session'` (3 files — plus a module-private `CHANNEL` const in a 4th place + that only one of them uses), `'chat:active-session'`, `'preview:refresh'`. A typo silently yields a + fresh channel pinned to `initialData`; nothing errors. Export a constant per channel, next to its + payload type. +- [ ] **Payload types are per-call-site, not per-channel.** `usePanelChannel` takes `T` from each + caller, so a publisher and a subscriber can disagree and nothing checks. + `'files:refresh-signal'` is `number` in all four places by convention only. A + `defineChannel(name)` helper returning a typed hook would fix both this and the item above. +- [ ] **Two write idioms disagree on the same channel.** `files:refresh-signal` is bumped with + `Date.now()` at the Chat sites and `setRefreshSignal((n) => n + 1)` at the FileViewer sites — and + `useGlobal`'s functional form applies against the **render-time** snapshot (`useGlobal.ts:18`), so + two increments in one render window collapse into one. Standardise on the nonce. + (`useLyricsOpen.ts:19-23` already documents avoiding the functional form for this reason.) +- [ ] **`system-settings:run-command` has no writer.** `run-command-channel.ts:6`; the only two writes + (`SystemSettings.tsx:87,133`) are both *clears*, and the sibling ServerSettings sections never + import it. The panel it drives — a terminal that opens with a command pre-loaded — appears + unreachable. Wire it or delete it; add to §8 either way. +- [ ] **`PanelComponentEntry.component` is typed with no props** (`types.ts:60-62`) but + `PanelSlot.tsx:521` passes `panelId` at runtime. `components`-supplied panels get a prop they + cannot see; registry apps get the honest `{ panelId: string }`. One-line type fix. + --- ## 6. Decisions needed — not defects, don't guess diff --git a/docs/workspace-panels.md b/docs/workspace-panels.md new file mode 100644 index 00000000..923869cd --- /dev/null +++ b/docs/workspace-panels.md @@ -0,0 +1,193 @@ +# The Workspace/Panel framework — how it works + +Orientation for anyone building a panel app or a dashboard route. This describes the framework **as it +is**, not as it should be; the defect list and the work queue live in `workspace-panel-todo.md`, and the +full reasoning behind them in `COMMS/workspace-panel-framework-analysis-2026-08-07.md`. + +Code: `src/workspaces/officerdev/src/components/Workspace/` — 12 files, ~1,700 lines, no tests. + +--- + +## 1. The data model + +One serialisable tree, persisted verbatim as Postgres `jsonb`: + +```ts +type LayoutGroup = { type: 'group'; id: string; direction: 'horizontal' | 'vertical'; + children: { node: LayoutNode; size: number }[] }; +type LayoutPanel = { type: 'panel'; id: string; appType: string | null; zoom?: number; fitContent?: boolean }; +type LayoutNode = LayoutGroup | LayoutPanel; +``` + +That is the whole model, and it is the good part — small, immutable, and transformed only by the pure +functions in `layout-utils.ts` (`splitPanel`, `removePanel`, `setApp`, `setZoom`, `updateSizes`, +`swapPanels`). Every mutator returns a new tree, or the identical reference when nothing changed. + +A **group** is a resizable split. A **panel** is a leaf that names an app by string (`appType`). + +## 2. The three layers + +| layer | file | what it does | +|---|---|---| +| the public API | `WorkspaceView.tsx` | owns swap/drag/maximize/zoom state, wires mutators to `workspace.setValue`, builds the context | +| the recursion | `WorkspaceRenderer.tsx` | tree → `ResizablePanelGroup`; the mobile collapse; the 500 ms resize debounce | +| one panel | `PanelSlot.tsx` | chrome, header, traffic lights, zoom, context menus, maximize; **mounts the app** | + +`WorkspaceLayout.tsx` is the read-only variant (no swap, no maximize, no persistence) used by the +dashboard-creation preview. `AppPicker.tsx` is the chip grid in an empty panel. `DragOverlay.tsx`, +`LayoutEditor.tsx` and `WorkspaceHeader.tsx` are currently unreachable. + +A screen wires it up in four lines: + +```tsx +const ws = useDashboardState('screens/music', defaultLayout); +return ; +``` + +`locked` means the user cannot split, close or re-app panels — the screen's layout is the product. Every +route screen is locked; only user-created dashboards are not. + +## 3. What a panel knows + +A panel app is `ComponentType<{ panelId: string }>`. **`panelId` is the only prop it gets.** Everything +else arrives through one of four doors, and picking the right door is most of the design work: + +1. **`useWorkspace()`** — ambient *environment*: `dashboardId`, `cwd`, `root`, `promptPrefix`, plus the + framework's own maximize/swap/zoom state. Read-mostly, set once per screen. +2. **react-router** — `useParams` / `useSearchParams`. Panels render inside the Route element tree, so + they can read the URL directly. **This is where selection belongs.** See §4. +3. **`useDashboardState(key, default)`** — durable, per-user, server-persisted state. See §5. +4. **`usePanelChannel(name, initial)`** — the cross-panel bus. See §4. + +`appType` → component resolution happens at `PanelSlot.tsx`, against a global registry seeded by +`` at mount: each app folder exports `appRegistryMetas`, and `metasToRegistry` flattens +them into one `Record`. Registry entry fields: + +| field | effect | +|---|---| +| `name`, `icon` | label in the picker and the default header | +| `component` | the body, gets `panelId` | +| `header?` | replaces the default icon+name header, gets `panelId` | +| `provider?` | wraps header **and** body, so the header can read the provider's context | +| `transparent?` | skips all chrome — no card, no header, no zoom wrapper | +| `availableOnPanel?` | `false` hides it from the picker. **Not access control** — a saved layout naming it still mounts it | +| `zoomable?` | `false` opts out of per-panel CSS `zoom` (the xterm apps and Remote Desktop, whose geometry is negotiated with a server) | + +Two escape hatches exist for components that cannot be in the global registry: + +- **`components` prop** — `Record`, keyed by *panel id*, taking precedence over the + registry. This is how a screen mounts a component that lives in `officer-web` rather than `officerdev` + (Email's list/reader, Browser's tabs/preview). +- **`ephemeral` prop** — a second, independent layout tree rendered in a sibling collapsible pane, driven + entirely from search params and deliberately *never* persisted. One producer: `useFileViewerPanels`, + which is how FileBrowser opens a viewer/player/side-chat without writing it into your saved layout. + +## 4. How panels talk to each other + +**Two mechanisms, and the split between them is the single most important convention here.** + +### The URL — for "which thing is open" + +Anything addressable — a session, an album, a section, a torrent — belongs in `useParams` or +`useSearchParams`. Every panel reads the URL independently; nobody passes a selection to anybody. This +gives you deep links, back/forward, cmd-click, and refresh-survival for free. + +This is the post-refactor pattern, and about two thirds of the app now does it. `docs/navigation-audit.md` +is the authority. + +### `usePanelChannel` — for signals + +```ts +export const usePanelChannel = (channel: string, initialData: T) => + useGlobal(['PANEL_CHANNEL', channel], initialData); +``` + +Five lines. It is a **slot in the app-wide React Query cache** — not a subscription, not an event +emitter. Writing sets the cache entry; every component reading that key re-renders. It is +process-global: nothing in the key comes from a panel, a dashboard or a route, so two workspaces on the +same screen share every channel. Lifetime is the SPA session — it does not survive a reload. + +Legitimate use is **notification**, not state: "something changed, refetch." The honest examples: + +| channel | who publishes | who subscribes | +|---|---|---| +| `files:refresh-signal` | Chat on turn-complete, FileViewer after a write | FileBrowser re-lists the directory | +| `music:resync` | MusicBrowser after a reindex | MusicDetail refetches | +| `soulseek:refresh` | SearchResults on enqueue | Transfers list refetches now instead of on its poll tick | + +The one correct *scoped* use of the same primitive is `terminal-conn-${panelId}`: a panel's body and its +registry `header` are separately-mounted components with no shared parent, so the body publishes its +connection status and the header renders it. Panel-scoped by construction, which is exactly what +`panelId` is for. + +### The thirteen channels, honestly + +13 channels, 26 call sites. Six carry selection that should be in the URL — `music:cwd` (the entire +library location, so no album is linkable), `soulseek:section` and `soulseek:user` (the whole Soulseek +workspace has no URL), `chat:selected-session`, `monitor:scope`. Two are view toggles where a channel is +genuinely fine (`music:favorites`, `music:lyrics`). + +Three are wired to nobody: `preview:refresh` and `chat:active-session` have publishers and no +subscribers; `file-viewer:` has a subscriber and no publisher, which is why the registry-mounted +`officerdev/file-viewer` app renders "No file selected" forever. + +Four channel names are **bare string literals with no shared constant**, repeated across up to four +files. A typo does not error — you silently get a fresh channel stuck on its initial value. And because +`usePanelChannel` takes `T` from each caller, nothing enforces that a publisher and a subscriber agree +on the payload type. + +**Rule of thumb:** if you can write down what a channel's value *is* ("the open session", "the current +folder"), it is selection and belongs in the URL. If the only thing it means is "something happened", +it is a signal and a channel is right — give it an exported constant and a nonce payload. + +## 5. Persistence + +One hook, one endpoint, three tables: + +``` +useDashboardState(key, default) → GET/PATCH /api/dashboards → dashboards | screens | dashboard_defaults +``` + +`useDashboardState` keeps the **entire** user state blob under one React Query key with +`staleTime: Infinity`, reads `state[key] ?? default`, and on mount **seeds its default to the server** if +the key is absent. Writes are optimistic: set the cache, fire a PATCH, ignore the result. + +The server dispatches each PATCH key by *family*: `workspaces`, `ws-layout-*`, `ws-terminals-*`, +`ws-host-terminals-*`, `screens/*`. Two consequences worth knowing before you invent a key: + +- **There is no `else`.** A key matching no family is dropped, and the request still returns 200. The + client's error path never fires, so an unknown key looks exactly like a successful save and fails only + on reload. Three key families in active use today (`ws-tmux-*`, `ws-nvim-*`, `ws-claude-code-*`) are in + precisely this state. +- **`null` only means "delete" in two of the branches.** Elsewhere it reaches a `NOT NULL` jsonb column. + +Because `useDashboardState` seeds one row per key on mount, per-panel scalar preferences should **not** +get their own key — that mints a row per panel. Put them on the layout node instead, the way `zoom` is +(`layout-utils.setZoom` drops the field at its default so untouched panels stay clean). + +Not everything belongs on the server. Per-tab UI state — which panel is maximized, what you named the +tab — uses `useSessionState` (`hooks/useSessionState`): `useGlobal` for the live value, sessionStorage as +the write-through copy the next document reads back. + +## 6. Building a new screen — the checklist + +1. `defaultLayout.ts` next to the screen, plus a `normalizeLayout` guard pinning `appType`s to an + allow-list (this guard is copy-pasted 14 times today and should be framework; until it is, copy it). +2. `useDashboardState('screens/', defaultLayout)` → ``. +3. Panel apps under `apps//`, exporting `appRegistryMetas` with `availableOnPanel: false`, + registered in `AppRegistry.tsx`. +4. Selection in the URL — route pair `/x` + `/x/:param`, or `?selected=` for a master list with a live + preview. Rows are real ``s. +5. A title rule in `usePageTitle.ts`. +6. If two panels must coordinate a *refresh*, export a channel constant. If they must coordinate a + *selection*, you want the URL. + +## 7. What is deliberately good — don't regress it + +- **Maximize is a CSS state toggle on the same element** — no portal, no remount, so scroll position and + media playback survive it. Every mutator should be held to this standard. +- **`layout-utils` is pure and short-circuits on referential equality**, so identity churn is bounded. +- **The framework contains zero route navigation**, which is why fixing a channel-driven app is a local + change to that app and never a framework change. +- **The resize debounce** — 500 ms trailing, with a guard that drops the spurious first `onLayout`. +- **`WorkspaceContext` carries environment, never selection**, and only 4 of ~30 apps read any of it.