From 397f464bb893cb13cb9694ad6e21206512062062 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Andr=C3=A9=20Padez?= <andre.padez@gmail.com>
Date: Thu, 30 Jul 2026 05:21:18 +0000
Subject: [PATCH] add navigation audit doc as reference for the routing work

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---
 docs/navigation-audit.md | 207 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 207 insertions(+)
 create mode 100644 docs/navigation-audit.md

diff --git a/docs/navigation-audit.md b/docs/navigation-audit.md
new file mode 100644
index 00000000..6207f530
--- /dev/null
+++ b/docs/navigation-audit.md
@@ -0,0 +1,207 @@
+# Frontend Navigation Audit — the "opaque click" anti-pattern
+
+**Date:** 2026-07-30 · **Branch:** `sidecars-claude` · **Status:** exploration + documentation only (no code changed)
+
+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 S2–S7.**
+
+### 🟡 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
+
+### Phase 1 — Quick wins (routes already exist; mechanical, high value)
+- [ ] **H1** Jobs rows → `<Link to={`/jobs/${job.id}`}>` (`JobsPage.tsx:108`). The clean twin of the /chat fix.
+- [ ] **H2** Dashboards rows → `<Link>`; retire `SELECTED_DASHBOARD_KEY` as selection source (`DashboardListApp.tsx:133`).
+- [ ] **H3** Projects rows → `<Link>`; retire `SELECTED_PROJECT` (`ProjectListApp.tsx:161`).
+- [ ] **H4** Chat detail: read `sessionId` from `useParams`, retire `chat:selected-session` as source of truth (`ChatDetailPanel.tsx:136`) — **finishes the /chat fix**.
+- [ ] **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 S2–S7** (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.