Files
platform/docs/navigation-audit.md
T
pastilhas b419af32de sweep the dead code in section 8
Each item re-verified before deleting; three of the ten entries were stale
and are corrected in place rather than silently fixed.

- WorkspaceLayout's isMobile/mobilePanelId/onMobileBack: none of its ten
  callers set them, so the mobile collapse they fed was permanently off in
  that renderer. WorkspaceView passes the same props to WorkspaceRenderer
  itself, where they are live.
- fixedHeight on AppRegistryEntry, and getFixedHeight with it: no app has
  ever declared one, so it only contributed undefined. The flex-column
  branch it shared with fitContent stays, keyed on fitContent alone.
- getDefaults: getAllDashboardState already folds the defaults row into the
  one payload the client fetches, which is why it never got a caller.
- upsertScreen's terminals/hostTerminals: never read is right, never
  written was not — it inserted them, which is why all 15 rows hold {}.
  The columns are left in place; dropping them needs a db:push, and this
  tree holds another agent's uncommitted schema file.
- SELECTED_DASHBOARD_KEY: H2 (01365cb) replaced it with ?selected= four
  months ago and it has had no reader since.
- ui/sidebar.tsx and the stray ui/hooks/ beside it. use-mobile was not
  orphaned as claimed — the sidebar imported it — and the use-toast in
  there was a near-identical copy of the live one.
- findChildById's unreachable duplicate condition, and the doc comment that
  described the wrong behaviour rather than the code being wrong.

Left deliberately: DragOverlay/LayoutEditor (gated on 5.3, an owner
decision) and the two chat-owned channels, whose docs are fixed here even
though the publishers are not mine to delete.
2026-08-07 11:26:33 +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 /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

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

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