Files
platform/docs/navigation-audit.md
T
pastilhas 00332e275a put the monitor scope in the url
/system-monitor/:scope, the same shape as /photos: route pair, one Navigate guard after the
hooks, scope list as react-router NavLinks, and both panels reading useParams instead of
agreeing over a `monitor:scope` channel. The Dock's isActive is a startsWith, so its
highlight survives the redirect off the bare route.
2026-08-07 11:41:16 +00:00

21 KiB
Raw Blame History

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. 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

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.

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 NavLinks 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:

  • MusicMusic/{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
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)

  • H1 Jobs rows → <Link to={/jobs/${job.id}}> (JobsPage.tsx). Done — 46482f3. (active-row highlight already keyed off useParams().id.)
  • 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.)
  • 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 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.
  • 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).
  • Soulseek — decide the nested tree /soulseek/:section + /soulseek/{users,search,rooms,chat}/:x; S1 decides S2S7 (M-slsk).
  • 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.