fix(ui): Help settings deep-links land on the right section + agent split/tile keeps every view - #11840
Conversation
… + WS split/tile layout fields dropped
1. Help deep-links to a Settings section always landed on the generic hub.
HelpView.navigate wrote the section into window.location.hash and then
called setTab("settings"), whose pushState replaces the URL with the bare
/settings path — clearing the fragment BEFORE SettingsView mounts and reads
it (hash-nav mode clobbered it the same way with "#/settings"). Route the
section through the sanctioned eliza:navigate:view `subview` channel
instead (App.tsx maps it into SettingsView's initialSection — the same path
the agent + slash-command flows use). "Open AI Model settings" now opens
the ai-model section, not the hub.
2. The WS shell:navigate:view → DOM eliza:navigate:view bridge
(startup-phase-hydrate) dropped the server's `views`/`layout`/`placement`
fields, so an agent-driven split-view/tile-views action degraded to a
single view: createNavigateViewHandler saw only viewId and laid out one
pane. Forward the three fields with the same untrusted-input sanitization
as the rest of the frame.
Tests: HelpView.test.tsx (4) pins the subview dispatch, the untouched
plain-tab + tutorial paths, and that the fragment stays clean;
startup-phase-hydrate.navigate-frame.test.ts (+3) pins layout-field
forwarding, sanitization, and single-view omission. Both mutation-checked
(reverting each fix reds its tests).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
Two demo-blocking view/navigation bugs in @elizaos/ui, found by tracing the deep-link plumbing end to end, each fixed minimally and pinned by mutation-checked tests. Branch:
fix/ui-view-polishofforigin/develop.Bug 1 — Help's "Open … settings" deep-links dead-end on the generic Settings hub
Before: Open Help → expand "How do I change the AI model?" → tap Open AI Model settings → → you land on the generic Settings hub. The section is silently lost. Every Help entry with a
settingsSection(ai-model ×6, runtime ×2) had the same dead-end.Root cause:
HelpView.navigatewrote the section intowindow.location.hash, then calledsetTab("settings").setTab(useNavigationState) doeshistory.pushState(null, "", "/settings")— a full URL replacement with no fragment — clearing the hash beforeSettingsViewmounts and reads it (readSettingsHashSection()in itsuseStateinitializer). Hash-routing mode clobbered it identically (#ai-model→#/settings). App.tsx's navigate handler even documents that the generic path nav "would drop the requested section" — which is exactly what Help was doing.After: the link dispatches
eliza:navigate:viewwith{ viewId: "settings", viewPath: "/settings", subview: "<section>" }— the sanctioned channel the agent and slash-command flows already use; App.tsx maps it toSettingsView'sinitialSection. Plain-tab links ("Open Settings", "Open Launcher") and the tutorial link are untouched.Tests (
HelpView.test.tsx, new, 4 passing): subview dispatch shape; URL fragment stays clean; plain-tab path still usessetTab; tutorial path still starts the tour + returns to chat. Mutation check: reverting the fix reds 2/4.Bug 2 — WS navigate bridge drops
views/layout/placement: agent split/tile degrades to one viewBefore: ask the agent to "tile the browser and wallet" → only one view opens. The server (
packages/agent/src/api/views-routes.ts) broadcastsviews: string[],layout,placementin theshell:navigate:viewframe for the documentedsplit-view/tile-viewsactions, but the client bridge (startup-phase-hydrate.ts) forwarded only viewId/viewPath/viewLabel/viewType/action/subview/alwaysOnTop.createNavigateViewHandlerthen sawdetail.views === undefinedand laid out a single pane fromviewIdalone; the layout/placement hints were discarded.After: the bridge forwards all three fields with the same untrusted-input sanitization the rest of the frame gets (string-array filter, non-empty-string gates).
Tests (
startup-phase-hydrate.navigate-frame.test.ts, +3 in the existing raw-WS-frame harness, 12 passing): tile-views frame forwards views/layout/placement; wrong-typed entries are sanitized; plain single-view frames omit the fields; the 9 pre-existing frame tests stay green. Mutation check: reverting the fix reds 2/12.Verification
bunx vitest run(packages/ui) on both files: 16/16 pass.biome check --writeon all 4 files: clean.tsgo --noEmit: no errors in touched files (one pre-existing, unrelatediwerdev-dep error inspatial/__e2e__, present without this change).packages/ui/src. No money/cloud-Worker surfaces, nouseCloudState.ts/ProviderSwitcher.tsx.Evidence rows: unit/component tests + mutation results above. Video walkthrough / before-after screenshots: N/A — headless workflow agent (no display); both behaviors are pinned by DOM-level component tests that assert the exact user-facing navigation contract (event detail, URL fragment, tab switch). Real-LLM trajectories: N/A — no agent/action/prompt/model behavior changed; this is client-side navigation plumbing.
[phone-ui]
Authored end-to-end by a Fable-5 agent (find + fix + tests + mutation-checks + push; transcript verified 100% claude-fable-5). Independently re-verified: 4-file merge-base diff, 16/16 tests fresh, HelpView mutation reproduced (revert → 2/4 red; restore → green). Conservative view/nav fixes; no money/cloud/routing changes. Part of the app demo-readiness push (#11157). — [phone-ui]