Files
platform/docs/navigation-audit.md
T
pastilhasandClaude Opus 5 829b034d13 docs: fold the workspace-root docs into the repo, with honest status
The root held four Markdown files that were not in any git repo and were being read as
current. CLAUDE_SIDECAR_ISOLATION.md is the one that prompted this: it describes, in the
present tense, an agent that dies whenever officer restarts. That was true when it was
written and has not been true for two days.

Rather than delete analysis that version control was not holding, the two substantial ones
moved into platform/docs/ with headers that say what has since happened:

- claude-sidecar-isolation.md — stages 0-2 are done and running (R1, R2, R4, R5 all
  satisfied); stages 3-5 are the only live part.
- sidecar-audit-2026-07.md — a snapshot audit, largely executed. Email, pty, music and vnc
  have been done since; claude, opencode and the cross-cutting notes are still open. The
  vault stays off-limits.

MUSIC_IMAGES_SPEC.md is deleted outright: the sidecar serves /image and /poster, so the
spec is the feature. The workspace root now holds one Markdown file, CLAUDE.md, which is
where cross-cutting operational reality belongs.

Also corrected, in the same pass:
- root CLAUDE.md listed email as "the big one, and untouched" and pty stage 4 as
  outstanding; both are done. It now names what actually remains (claude stages 3-5, the
  terminal orphan leak) and what landed.
- docs/sidecar-topology.md said "nothing built yet". Two sidecars now serve their own
  transport; what has NOT happened is the part the document is about — fixed ports, the
  shared table, .env toggles — so it says exactly that rather than implying the design is
  underway. Migration order updated: pty and email went first, not music.
- docs/navigation-audit.md is cited as authoritative but still planned work on Projects,
  a feature since deleted. A status header marks H3 void, H1/H2 done and H4 the one open
  item, so nobody follows it into a directory that no longer exists.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-31 16:45:26 +00:00

225 lines
20 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# Frontend Navigation Audit — the "opaque click" anti-pattern
> **Status, 2026-07-31.** H1 (jobs), H2 (dashboards) and H4's groundwork are **done** — rows are real
> `<Link>`s and selection lives in `?selected=` / `useParams`. **H3 is void: the Projects feature was
> deleted**, so every reference to `Projects/`, `SELECTED_PROJECT` and `/projects/:id` below is historical
> and should be read as an example of the pattern, not as work to do. **H4 (chat detail reading
> `sessionId` from `useParams` rather than the `chat:selected-session` channel) is the one item still
> open.** The rules in this file are current and authoritative; the findings table is a snapshot.
**Date:** 2026-07-30 · **Origin:** written as exploration before any of the routing work was done.
Prep work for the upcoming **full navigation refactor**. This catalogues every place the frontend
navigates to / selects an **addressable entity** using a `<button onClick>` / `<div onClick>` /
imperative `navigate()` / global-channel setter **instead of a real `<Link to>` / `<a href>`**.
> Line numbers were captured on `sidecars-claude` at audit time and may drift as other work lands —
> treat them as anchors, re-grep the symbol if a line looks off.
---
## The anti-pattern (definition)
A clickable element selects/opens something that has (or should have) a URL, but:
- **(a)** the entity id/slug is **not in the DOM** (no `href`, no `data-*`) — it lives only in an onClick closure;
- **(b)** clicking **doesn't change the URL** (or does so only via an indirect state→URL effect);
- **(c)** selection is held in **JS state / a global channel** (`usePanelChannel`, `useGlobal`), not the URL;
- **(d)** **no anchor semantics** — can't cmd/ctrl-click to a new tab, no middle-click, not link-focusable.
**Exemplar (already fixed):** the `/chat` session list. Rows were `<button onClick={() => selectById(id)}>`
(id only in the closure) → converted to `<Link to={`/chat/${session.id}`}>` (committed to master `f35c145`).
That fix is the template for the HIGH items below. **Caveat:** the fix only did the *rows* — the chat
**detail panel** still selects via channel, not the URL (finding **C1**), so `/chat` is the model for both
"done right" (rows) and "still to do" (detail).
## Route map (from `App.tsx`)
**Existing entity routes:** `/chat/:sessionId`, `/jobs/:id`, `/dashboards/:id`, `/projects/:id`, `/email/:emailId`.
**Flat screens:** `/` `/files` `/music` `/soulseek` `/tasks` `/skills` `/processes` `/activity`
`/system-monitor` `/task-logs` `/plans` `/terminal` `/desktop` `/browser` `/code-editor` `/settings/{profile,ai,system,integrations,apps}`.
**What's already correct** (lean on these in the refactor): the **Dock**, **Header** (logo + mobile sheet),
**UserMenu**, **JobsIndicator** are all real `<Link>`s. Shared `NavLink.tsx` (query-string-appending `<Link>`
wrapper — note: *not* react-router's NavLink, gives no active state) and `BackButton.tsx` (`<Link>` back arrow)
are good building blocks. The **Workspace/Panel framework** contains **zero** route navigation — it's orthogonal.
---
## Consolidated findings — severity ranked
### 🔴 HIGH — addressable route already exists; just needs a `<Link>` / URL-as-source-of-truth
| ID | file:line | Entity | Current impl | Fix |
|----|-----------|--------|--------------|-----|
| H1 | `Screens/Dashboard/Jobs/JobsPage.tsx:108` | a job | `<button onClick={() => navigate(`/jobs/${job.id}`)}>` — id in closure | → `<Link to={`/jobs/${job.id}`}>`. Active-row already keys off `useParams().id`; keep the stop/delete button. **The exact twin of the /chat fix.** |
| H2 | `workspaces/…/apps/Dashboards/DashboardListApp.tsx:133` | a dashboard | `<div onClick={handleClick}>``useGlobal(SELECTED_DASHBOARD_KEY)` on-page (**no URL change**), `navigate()` off-page | rows → `<Link to={`/dashboards/${ws.id}`}>`; drop the global as selection source (derive from `useParams`). Header is already a `<Link>` — app is internally inconsistent. |
| H3 | `workspaces/…/apps/Projects/ProjectListApp.tsx:161` | a project | `<div onClick={handleClick}>``useGlobal(SELECTED_PROJECT)` on-page (**no URL change**), `navigate()` off-page | identical to H2 → `<Link to={`/projects/${p.id}`}>`; retire `SELECTED_PROJECT` as source of truth. |
| H4 | `workspaces/…/apps/ChatHistory/ChatDetailPanel.tsx:136` | which chat to render | reads `usePanelChannel('chat:selected-session')`, **not** `useParams` | make the route the source of truth: read `sessionId` from `useParams`, fetch by id, retire `chat:selected-session` (or make it a derived cache). **Finishes the /chat fix.** |
| H5 | `Screens/Dashboard/Email/EmailList.tsx:336` | an email | `<button data-email-id onClick={setSelectedId}>` + `EMAIL_SELECTED` channel + a state↔URL sync effect (`EmailScreen.tsx:27-33`) | rows → `<Link to={`/email/${msg.id}`}>`, drive selection **from** `useParams().emailId`, delete the two sync effects. (Already round-trips the URL + has `data-email-id`, but still isn't an anchor.) |
> **Verify:** `Dashboards/DashboardPreview.tsx:355,377` and `Projects/ProjectPreview.tsx:430,457` also
> `navigate('/…/:id')`. The nav auditor read these as HIGH "open entity"; the apps auditor read the Project
> ones as *correct* post-create/edit flows. Eyeball them during H2/H3 — convert the ones that are "open an
> existing entity from a preview", leave genuine post-mutation redirects.
### 🟠 MEDIUM — navigable entity with **no route yet** (add a route, then link)
| ID | file:line | Entity | Proposed route | Note |
|----|-----------|--------|----------------|------|
| M1 | `Screens/Dashboard/CapabilityPage.tsx:431` | task / skill / process | `/tasks/:dir`, `/skills/:dir`, `/processes/:dir` | one component backs **three** screens (Tasks/Skills/Processes). Highest-value MEDIUM. Also drops the auto-select-`items[0]` effect. |
| M2 | `Screens/Dashboard/TaskLogs/index.tsx:104` | a task-log run | `/task-logs/:id` | detail fetch already keys off the id — clean move. |
| M3 | `Screens/Dashboard/Activity/ActivityScreen.tsx:63,73` | background task / detached job | `/activity/:id` | two row types; unify under one param, screen re-derives `task=`/`path=`. |
| M4 | `FileBrowser/.../useFileBrowserApp.ts:269`, `FileItem.tsx:516`, `Breadcrumb.tsx:16`, search-hit `:249` | a folder | `/files?path=<dir>` | **files** already open via `?view=`; **folders** are pure `currentPath` state — no URL, no back/forward. Folder rows + crumbs → `<Link>` on a `?path=` param. |
| M5 | `CodeEditor/FileTree.tsx:59`, `EditorTabs.tsx:33` | open source file / active tab | `/code-editor?file=<path>` (+ `open=` for tabs) | tree file rows → `<Link>`; active file in a param. |
| M6 | `Layout/Settings/SettingsPanel.tsx:39` | a settings sub-section | `/settings/:page/:section` or `?section=` | `<button onClick={setSelectedKey}>``useGlobal`; nothing in URL, not deep-linkable. **One change covers all settings pages** (all go through `createSettingsPanelComponents`). |
| M7 | `workspaces/components/Combobox.tsx:53` | caller-supplied route | (existing) | shared widget: `href`-bearing options do `navigate(option.href)`; render them as `<Link>`. Every caller inherits the opaque click. |
| M8 | `Layout/Header/UserMenu.tsx:51` | — | — | **dead target:** `<Link to="/settings/resources">` — route doesn't exist (falls through to `/`). Add the route or remove the item. |
| M9 | `Screens/Dashboard/Plans/index.tsx:36` | a plan document | `/plans/:name` | native `<select>` → local state; a plan is a real addressable doc. Have the select `navigate()` or use a link list. |
| M10 | `SystemMonitor/ScopeList.tsx:14` | monitor scope (btop/pm2/docker) | `/system-monitor/:scope` | small fixed set; low urgency but genuinely addressable. |
**Music** (M-music) and **Soulseek** (M-slsk) are whole-workspace channel apps — pulled out below because each
is **one design decision** that cascades across many files:
- **Music** — `Music/{MusicBrowser:182,209 · MusicDetail:138,174,234 · FavoritesView:85,107}` hold the entire
library location in `usePanelChannel('music:cwd')`; every drill-in is `setCwd(path)`. No URL, no deep-link to
an album. → back `music:cwd` with `/music?path=<rel>`; rows become `<Link>`s.
- **Soulseek** — an entire workspace with **no URL at all**: `soulseek:section` nav (`SoulseekNav:40`,
`SoulseekDashboard:88/96/110/121`), `soulseek:user` peer channel (`UserMenu:32`, `SoulseekUsers:42`), and
per-panel `useState` for past-search (`SearchView:160`, a textbook `role="button"` div with `s.id` in the
closure), rooms (`SoulseekRooms:153`), conversations (`SoulseekChat:135`). → decide a nested tree
`/soulseek/:section` + `/soulseek/{users/:name, search/:id, rooms/:name, chat/:name}`. **S1 (the section nav)
decides S2S7.**
### 🟡 LOW / borderline
- **Dock / Header active styling** (`Dock.tsx:43,78` · `Header.tsx:53,112`) — destinations are already real
`<Link>`s; only the *active* class is JS-derived from `useLocation`. Switch to react-router `<NavLink>` and drop the hand-rolled `isActive`.
- **Browser tabs** (`Browser/TabList.tsx:93`) — ephemeral CDP targets (polled, come/go every few seconds); a durable `/browser/:targetId` is dubious. Probably leave as selection.
- **Jobs step/iteration** (`Jobs/JobDetail.tsx:234,255`) — intra-detail sub-selection; only if we want `/jobs/:id/:stepKey` deep links.
- **Preview slug** (`Preview/PreviewApp.tsx:23`) — Preview usually derives its target from the surrounding dashboard/project `cwd`; the manual picker is a fallback. Probably leave URL-less.
- **FileBrowser widget** (`FileBrowserWidget/.../BrowseTab.tsx:36`) — compact sidebar widget; keeping its browse cursor local is defensible.
- **Music favorites view** (`music:fav`) & **SystemMonitor scope** — view toggles over tiny fixed sets; channel-state is arguably fine.
- **"New Chat" button** (`ChatHistory/SessionList.tsx:68`) — a create-action that also `navigate('/chat/new')`; could be `<Link to="/chat/new">` if the state-set moves into the route. Its `chat:selected-session` write is part of the H4 cleanup.
---
## Channel-based selection map (the core of the refactor)
Every place an **addressable entity** is selected through a global channel / global state instead of the URL.
This is the primary surface to convert to URL-driven selection.
| Channel / global key | Entity held | Should map to | Files |
|----------------------|-------------|---------------|-------|
| `chat:selected-session` | open chat session | `/chat/:sessionId` | `ChatDetailPanel.tsx:136`, `SessionList.tsx:16` (H4) |
| `chat:active-cwd` | chat working dir | query param on `/chat` | `ChatDetailPanel.tsx:102`, `SessionList.tsx:14` |
| `SELECTED_DASHBOARD_KEY` (`useGlobal`) | selected dashboard | `/dashboards/:id` | `DashboardListApp.tsx:36`, `DashboardPreview.tsx:287` (H2) |
| `SELECTED_PROJECT` (`useGlobal`) | selected project | `/projects/:id` | `ProjectListApp.tsx:45`, `ProjectPreview.tsx:386` (H3) |
| `EMAIL_SELECTED` | open email | `/email/:emailId` | `EmailList.tsx:336`, `EmailScreen.tsx` sync (H5) |
| `music:cwd` | library location (album/artist/folder) | `/music?path=` | `MusicBrowser/MusicDetail/FavoritesView` |
| `soulseek:section` | active Soulseek section | `/soulseek/:section` | `SoulseekNav/View/Dashboard/UserMenu` |
| `soulseek:user` | selected peer | `/soulseek/users/:name` | `UserMenu.tsx:28`, `SoulseekUsers.tsx:34` |
| `monitor:scope` | monitor scope | `/system-monitor/:scope` | `ScopeList.tsx` (via `shared.ts:15`) |
| settings `useGlobal(*_SELECTED)` | settings section | `/settings/:page/:section` | `SettingsPanel.tsx:56` (M6) |
| local `useState` selections | search / room / conversation / peer / open file / preview slug | respective nested routes | `SearchView`, `SoulseekRooms`, `SoulseekChat`, `SoulseekUsers`, `CodeEditor/useEditorState`, `PreviewProvider` |
**Excluded — event-bus / refresh signals, NOT selection:** `preview:refresh`, `files:refresh-signal`,
`SLSKD_REFRESH_CHANNEL`, `MUSIC_RESYNC_CHANNEL`, `FILE_VIEWER_CHANNEL`, `chat:active-session` (embedded-chat signal).
---
## Appendix A — all `navigate()` call sites (whole frontend, 25)
"Link?" = a `<Link>`/`<NavLink>` is the right refactor.
| # | file:line | what | target | Link? | note |
|---|-----------|------|--------|-------|------|
| 1 | `Jobs/JobsPage.tsx:109` | job list row | `/jobs/:id` | **YES** | H1 |
| 2 | `Dashboards/DashboardListApp.tsx:90` | dashboard row (off-page) | `/dashboards/:id` | YES | H2 |
| 3 | `Dashboards/DashboardListApp.tsx:114` | inside "New Dashboard" | `/dashboards` | ~ | create action |
| 4 | `Dashboards/DashboardPreview.tsx:355` | open dashboard | `/dashboards/:id` | YES | verify vs H2 |
| 5 | `Dashboards/DashboardPreview.tsx:377` | open dashboard | `/dashboards/:id` | YES | verify vs H2 |
| 6 | `Projects/ProjectListApp.tsx:116` | project row (off-page) | `/projects/:id` | YES | H3 |
| 7 | `Projects/ProjectListApp.tsx:142` | inside "New Project" | `/projects` | ~ | create action |
| 8 | `Projects/ProjectPreview.tsx:430` | open project | `/projects/:id` | YES | verify vs H3 |
| 9 | `Projects/ProjectPreview.tsx:457` | open project | `/projects/:id` | YES | verify vs H3 |
| 10 | `Jobs/JobDetail.tsx:709` | back button | `/jobs` | YES | use `BackButton`/`<Link>` |
| 11 | `ChatHistory/SessionList.tsx:70` | "New Chat" | `/chat/new` | ~ | borderline (LOW) |
| 12 | `components/Combobox.tsx:54` | nav combobox option | `option.href` | YES | M7 |
| 13 | `MusicPlayer/MusicPlayerHost.tsx:251` | open current album (setCwd+nav) | `/music` | ~ | has side-effect |
| 14 | `FileBrowserWidget/useFileBrowserWidget.ts:53` | open folder in Files | `/files?view=…` | ~ | query nav |
| 15 | `FileBrowserApp/useFileBrowserApp.ts:401` | create dashboard from folder | `/dashboards/new?…` | ~ | action-nav |
| 16 | `FileBrowserApp/useFileBrowserApp.ts:427` | create dashboard from files | `/dashboards/new?…` | ~ | action-nav |
| 17 | `FileBrowserApp/.../TaskRunnerModal.tsx:1168` | after starting job | `/jobs` `/jobs/:id` | NO | post-submit |
| 18 | `FileBrowserApp/.../TaskRunnerModal.tsx:1649` | after starting pipeline | `/jobs/:id` | NO | post-submit |
| 19 | `Email/EmailScreen.tsx:29` | sync selection → URL (effect) | `/email/:emailId` | NO | delete in H5 |
| 20 | `Email/EmailScreen.tsx:31` | clear selection → URL (effect) | `/email` | NO | delete in H5 |
| 21 | `ChatHistory/index.tsx:107` | redirect when no id | `/chat` | NO | guard |
| 22 | `Projects/ProjectListScreen.tsx:66` | clear/redirect | `/projects` | NO | guard |
| 23 | `Authentication/ForgotPassword/useResetPassword.ts:48` | post-reset redirect | `/` | NO | action result |
| 24 | `Authentication/LandingPage/Bootstrap.tsx:34` | post-bootstrap redirect | `/` | NO | action result |
| 25 | `hooks/useQueryState/useQueryState.ts:39` | generic query-state writer | dynamic | NO | infra hook |
---
## ✅ TO-DO — ordered for tomorrow
> **Progress — 2026-07-30, branch `navigation-refactor` (off master; NOT yet runtime-tested):**
> H1, H2, H3 implemented and tsgo-clean. **Design correction for H2/H3:** the naive "row → `<Link to="/dashboards/:id">`"
> would destroy the *preview-on-list* feature (that route is the full page). The faithful fix — which is what
> was implemented — moves selection out of the `SELECTED_*` global into a **`?selected=<id>` URL param** read by
> the list, the screen (mobile panel), and the preview; rows are real `<Link>`s (`/…?selected=id` on-page,
> `/…/:id` off-page) with the action buttons kept as **siblings** of the anchor, not nested inside it. Same
> approach will suit any future "master list + live preview" screen. **Must verify on next restart:** preview,
> create/edit/delete(/publish), and mobile-panel flows on `/dashboards` and `/projects`.
### Phase 1 — Quick wins (routes already exist; mechanical, high value)
- [x] **H1** Jobs rows → `<Link to={`/jobs/${job.id}`}>` (`JobsPage.tsx`). Done — `46482f3`. (active-row highlight already keyed off `useParams().id`.)
- [x] **H2** Dashboards rows → `<Link>`; `SELECTED_DASHBOARD_KEY` global replaced by `?selected=` URL param across `DashboardListApp`/`DashboardsScreen`/`DashboardPreview`. Done — `01365cb`. **Needs runtime test.**
- [x] **H3** Projects rows → `<Link>`; `SELECTED_PROJECT` global replaced by `?selected=` URL param across `ProjectListApp`/`ProjectListScreen`/`ProjectPreview`. Done — `2aaacc8`. **Needs runtime test.**
- [ ] **H4** Chat detail: read `sessionId` from `useParams`, retire `chat:selected-session` as source of truth (`ChatDetailPanel.tsx:136`) — **finishes the /chat fix**. *Deferred: overlaps the in-flight `sidecars-*` chat-comms work; do after that lands.*
- [ ] **H5** Email rows → `<Link>` driven by `useParams().emailId`; delete the state↔URL sync effects (`EmailList.tsx:336`, `EmailScreen.tsx:27-33`).
- [ ] **M8** Fix or remove the dead `/settings/resources` link (`UserMenu.tsx:51`).
- [ ] Verify + convert the 4 preview "open" navigates (`DashboardPreview:355,377`, `ProjectPreview:430,457`); leave genuine post-mutation redirects.
### Phase 2 — Add a route, then link (per-entity, medium effort)
- [ ] **M6** Settings sub-sections → nested route or `?section=`, render `SectionButton` as `<NavLink>` (`SettingsPanel.tsx:39`) — one change covers all settings pages.
- [ ] **M1** Capabilities → `/tasks|skills|processes/:dir`, rows → `<Link>` (`CapabilityPage.tsx:431`) — covers 3 screens.
- [ ] **M2** TaskLogs → `/task-logs/:id` (`TaskLogs/index.tsx:104`).
- [ ] **M3** Activity → `/activity/:id` (`ActivityScreen.tsx:63,73`).
- [ ] **M4** FileBrowser folders → `/files?path=`; folder rows + breadcrumbs → `<Link>` (`useFileBrowserApp.ts:269`, `FileItem.tsx`, `Breadcrumb.tsx`).
- [ ] **M5** CodeEditor active file → `/code-editor?file=` (`FileTree.tsx:59`, `EditorTabs.tsx:33`).
- [ ] **M7** Combobox: render `href` options as `<Link>` (`Combobox.tsx:53`).
### Phase 3 — Whole-workspace routing decisions (needs a design call first)
- [ ] **Music** — decide `/music?path=` (or nested); back `music:cwd` with the URL; rows → `<Link>` (M-music).
- [ ] **Soulseek** — decide the nested tree `/soulseek/:section` + `/soulseek/{users,search,rooms,chat}/:x`; **S1 decides S2S7** (M-slsk).
- [ ] **M10** SystemMonitor scope → `/system-monitor/:scope` (`ScopeList.tsx:14`).
- [ ] **M9** Plans → `/plans/:name` (`Plans/index.tsx:36`).
### Phase 4 — Polish + borderline decisions
- [ ] Dock + Header + mobile sheet → react-router `<NavLink>` for active state; drop hand-rolled `isActive` (`Dock.tsx`, `Header.tsx`).
- [ ] "New Chat" → `<Link to="/chat/new">` (`SessionList.tsx:68`) once H4's channel cleanup lands.
- [ ] Jobs back button + any `navigate('/jobs')` → shared `BackButton` (`JobDetail.tsx:709`).
- [ ] Decide/skip: Browser tabs, Jobs step deep-link, Preview slug, FileBrowser widget, Music-favorites/Monitor-scope view toggles.
### Cross-cutting for the refactor itself
- [ ] Standardise a URL-as-source-of-truth pattern for panel selection (replace the `usePanelChannel`/`useGlobal`
selection channels in the map above with `useParams`/`useSearchParams`, keeping channels only for
genuine signals/refresh buses).
- [ ] Adopt `<NavLink>` (real react-router) for all nav chrome so active state stops being JS-derived.
- [ ] Keep `NavLink.tsx` (query-string wrapper) and `BackButton.tsx` as the standard building blocks.
---
## Coverage
- **Area 1 — Dashboard screens** (`Screens/Dashboard/**`, excl. Settings/Layout): 9 findings (2 HIGH, 4 MEDIUM, 3 LOW) + spillover to `officerdev` apps.
- **Area 2 — Workspace panel apps** (`workspaces/officerdev/src/apps/**`): 22 findings (4 HIGH, ~15 MEDIUM, 3 LOW); 11 distinct channel-selection sites.
- **Area 3 — Global nav / Layout / Settings / shared components**: 6 findings (0 HIGH — chrome is already links; 3 MEDIUM, 3 LOW) + the 25-site `navigate()` map.
Raw per-area findings (with full impl / id-in-DOM / URL-change / coverage notes) were produced in the
audit run; this document is the consolidated, de-duplicated synthesis. The three HIGH clusters
(Jobs · Dashboards · Projects) plus the two chat halves (rows done, detail = H4) are the direct siblings of
the `/chat` fix and the highest-value starting point.