# 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` `/tasks` `/skills` `/processes` `/activity`
`/system-monitor` `/task-logs` `/plans` `/terminal` `/desktop` `/browser` `/code-editor`.
The five settings pages are route **pairs** now, not flat screens — `/settings/{profile,ai,system,integrations,user-management}`
plus a `:section` each (M6). There has never been a `/settings/apps`; that entry was wrong when this list
was written.

**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~~ | `Settings/SettingsPanel.tsx` | a settings sub-section | `/settings/:page/:section` | **Done.** `<NavLink>` + `useParams`, five `*_SELECTED` globals gone, one `SettingsRoute` guard per page. The "one change covers all settings pages" claim was *almost* right: Integrations builds its own sidebar and did not go through `createSettingsPanelComponents`, and it also held the Enterprise/Personal tab in a second global — derived from the section key now, which is what fixes deep-linking a Personal section. |
| 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` | — | — | **Done.** Removed rather than routed: nothing had ever been built behind `/settings/resources`, so the item was a bounce to `/` dressed as navigation. Its `header.userMenu.resources` locale keys went with it. |
| 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` | monitor scope (btop/pm2/docker) | `/system-monitor/:scope` | **Done.** Route pair + `Navigate` guard; the scope buttons are `NavLink`s and `useMonitorScope` reads `useParams` instead of the channel. The Dock's hand-rolled `isActive` is a `startsWith`, so its highlight survives the redirect. |

**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**~~ — **mostly done.** The section is `/soulseek/:section` (nav entries are `NavLink`s, the
  view panel reads the same URL), the peer is `?user=<name>`, and the past-search was already `?search=<id>`.
  The `soulseek:section` and `soulseek:user` channels are deleted; `soulseek:refresh` stays, which is a
  signal and not selection. The peer went in the query string rather than `/soulseek/users/:name` as
  sketched here, because the section nav has to stay a one-segment `NavLink` — a second path segment would
  need a nested route just to keep the highlight, and the query string is what `?search=` already uses.
  Still on `useState`: rooms (`SoulseekRooms`) and conversations (`SoulseekChat`).

### 🟡 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` |
| 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:** `files:refresh-signal`,
`SLSKD_REFRESH_CHANNEL`, `MUSIC_RESYNC_CHANNEL`.
(`FILE_VIEWER_CHANNEL` was listed here too; it had no publisher and has been deleted — the file viewer
reads `?view=` from the URL. `preview:refresh` and `chat:active-session` were also listed, and
`preview:refresh` was cited above as the exemplar of a *legitimate* channel — but both have a publisher
in `ChatPanelWrapper` and **no subscriber at all**, and `preview:refresh`'s reader, `PreviewProvider`, is
no longer in the repo. They are declared in `officerdev/src/channels.ts` with that stated; deleting the
publishers means changing the chat panel, which is another agent's, so it is written up in
`COMMS/chat-agent-handoff-2026-08-07.md` instead. Use `files:refresh-signal` as the exemplar.)

---

## 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.** (The constant itself outlived its last reader by four months and has now been deleted; its siblings in `Dashboards/constants.ts` are dialog form state, not selection, and stay.)
- [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`).
- [x] **M8** Dead `/settings/resources` menu item removed from `UserMenu.tsx`, along with its now-orphaned `en`/`pt` locale keys. **Needs runtime test.**
- [ ] 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)
- [x] **M6** Settings sub-sections → `/settings/:page/:section`; `SectionButton` is now a `SectionLink` (`<NavLink>`), the five `*_SELECTED` globals and `INTEGRATIONS_SETTINGS_TAB` are gone, and each page renders one `SettingsRoute` guard that canonicalises the bare route and a bogus section. **Needs runtime test.**
- [ ] **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).
- [x] **Soulseek** — `/soulseek/:section` with the peer in `?user=` and the search already in `?search=`; the two selection channels are deleted. Rooms and conversations are still `useState`. **Needs runtime test.**
- [x] **M10** SystemMonitor scope → `/system-monitor/:scope`; `monitor:scope` channel deleted. **Needs runtime test.**
- [ ] **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.