document how the workspace/panel framework works
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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-<id>` 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<number>`. 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<T>` 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<T>(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
|
||||
|
||||
@@ -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<LayoutNode>('screens/music', defaultLayout);
|
||||
return <WorkspaceView workspace={ws} locked cwd="~" />;
|
||||
```
|
||||
|
||||
`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
|
||||
`<AppRegistry />` at mount: each app folder exports `appRegistryMetas`, and `metasToRegistry` flattens
|
||||
them into one `Record<appType, entry>`. 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<panelId, Component>`, 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 = <T,>(channel: string, initialData: T) =>
|
||||
useGlobal<T>(['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:<panelId>` 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<T>` 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<LayoutNode>('screens/<name>', defaultLayout)` → `<WorkspaceView workspace={ws} locked />`.
|
||||
3. Panel apps under `apps/<Feature>/`, 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 `<Link>`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.
|
||||
Reference in New Issue
Block a user