fix(trusty-search,trusty-memory,trusty-analyze): derive UI API base from baseURI pathname, not the raw href - #4985
Open
mac-duetto wants to merge 1 commit into
Open
fix(trusty-search,trusty-memory,trusty-analyze): derive UI API base from baseURI pathname, not the raw href#4985mac-duetto wants to merge 1 commit into
mac-duetto wants to merge 1 commit into
Conversation
…rom baseURI pathname, not the raw href computeBase() ran its `$`-anchored `index.html` / `ui/` strips against the raw `document.baseURI`, which includes the URL fragment. On a hash-routed load ending in `ui/#/` the mount-segment strip silently no-opped, so every API call resolved under `/ui/` and hit the SPA catch-all, which answers 200 text/html with index.html. Nothing failed at the HTTP layer: request() fell back to res.text() and handed callers an HTML string, while EventSource hard-failed on the wrong Content-Type. The router writes the hash on the first sidebar click, so every reload, restored tab, or bookmark after that was broken. A query string (`/ui/?tab=1`) triggered the same misrouting. The strips now run against `new URL(document.baseURI).pathname`, which carries neither the fragment nor the query string, re-joined to the origin. The `window.__*_BASE__` override branch and the non-browser guard are unchanged. Applied to all three KEEP IN SYNC copies. trusty-search and trusty-analyze mount their SPA at `/ui/` and were both broken; trusty-memory is root-mounted, so its `ui/` strip is a no-op and apiUrl() stayed correct via relative resolution, but apiBase() returned a fragment-bearing string. Three regression cases added to trusty-search's base.test.js, the only UI package with a test harness. Verified red against the unfixed code (3 failed, 10 passed) and green after (13 passed); no existing case was modified. The committed ui-dist / ui/dist bundles are regenerated with the pinned toolchain, since CI and release always set SKIP_UI_BUILD=1 and ship whatever is committed. Attribution was verified by round-trip: rebuilding from the unfixed base.js reproduces the committed baseline bundle byte-for-byte, and the build is deterministic across runs, so the entire bundle delta is downstream of the computeBase change alone. Closes #4980 🤖🤖🤖 Generated with trusty-mpm — https://github.com/bobmatnyc/trusty-tools
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #4980
Problem
computeBase()derived the SPA's API base by running two$-anchored regex strips against the rawdocument.baseURIhref.baseURIcarries the URL fragment, so on a hash-routed load ending inui/#/the mount-segment strip silently no-ops. Every API call is then misrouted under/ui/, hits the SPA catch-all, and returnsindex.htmlas200 text/htmlinstead of JSON/SSE. Nothing fails at the HTTP layer, which is why this went unnoticed.API_BASEis snapshotted once at module load, so the wrong base is fixed for the life of the page.Not a recent regression. Introduced in d087b88, first shipped in trusty-search-v0.24.10 — roughly 18 minor versions ago. Reported after a v0.42.2 upgrade only because the upgrade restarted the daemon and the tab reloaded at
/ui/#/.Wider than it looks.
router.svelte.jsnever writeslocation.hashat init, so a first navigation to bare/ui/works. Butnavigate()writes the hash on the first sidebar click and it stays in the URL. Every reload, restored tab, or bookmarked visit after that is broken — the steady state after a minute of normal use.Fix
Run the same strips against
new URL(document.baseURI).pathname, which never carries the fragment or query, then returnorigin + strippedPath. Thewindow.__SEARCH_BASE__override branch and thetypeof document === 'undefined'guard are untouched.Applied to all three KEEP IN SYNC copies:
trusty-search,trusty-memory,trusty-analyze.trusty-memorywas not user-visibly broken — its SPA is root-mounted, so the strip is a no-op andapiUrl()stayed correct; onlyapiBase()returned a fragment-bearing string and nothing calls it. Fixed for the sync contract, stated as such in its changelog rather than overclaimed.Rejected alternative
base.jshonours awindow.__SEARCH_BASE__override andbase.test.jstests it, but no Rust code ever injects it. Wiring it up server-side looks like the obvious fix and is wrong:serve_indexhas onlystate.daemon_port(a bareu16) — no scheme, no host, no proxy prefix. Any absolute base it builds is a guess, and the override is checked first, so it wins. Behind the trusty-console proxy the browser would bypass the proxy entirely; underssh -L, Docker port mapping, or a non-loopback bind the injected origin is simply wrong. It would trade a reload bug for a topology bug in exactly the deployments the current code handles correctly. This was caught by an adversarial review pass, not by the first-pass diagnosis.Test ladder — rung 6 (UI / API surface)
Built with the pinned toolchain — Node 20.20.2 + pnpm 9.15.9, matching the
ui-checksCI job.Red first. The three new cases fail against unfixed code on this base:
Green after:
Test Files 2 passed (2)/Tests 13 passed (13). All pre-existing cases pass unmodified.Binary smoke run — before/after against real shipped bundles
A binary built from this branch was run isolated on port 17878. The bundle was fetched over the wire and its real
computeBase/apiUrlexecuted in Node against stubbedbaseURIvalues, contrasted against the pre-fix bundle fromdd712709:document.baseURIapiUrl('/health')/ui//health/health/ui/#//ui/health✗/health✓/ui/#/indexes/ui/health✗/health✓/ui/?tab=1/ui/health✗/health✓https://console.local/proxy/search/ui/#/.../ui/health✗.../search/health✓Served
index.htmlreferences the new hashindex-D8eMcIuN.js; the oldindex-DHZwUpbb.jsis absent from the binary.Bundle diff isolation
trusty-memoryandtrusty-analyzeisolate cleanly — 110,013/110,171 and 139,723/139,883 bytes identical, differing region iscomputeBaseonly.trusty-searchdid not: only 1,826 of 106,144 bytes match, because adding two local bindings shifted esbuild's identifier-frequency ranking and re-mapped names bundle-wide. Proven benign rather than assumed: the build is deterministic (same source twice → identical md5), and revertingbase.jsand rebuilding reproduces the exact committed baselineindex-DHZwUpbb.js, while restoring the fix givesindex-D8eMcIuN.js.base.jsis the only input delta. Zero.rsfiles and no lockfile or dependency changed.Not in scope
ui_asset_handlerreturns200+index.htmlfor any unmatched/ui/*path, which is why this failure produced HTTP 200s instead of surfacing in the network tab./ui/healthstill returns200 text/htmlafter this change — expected, unchanged, and tracked as the separable follow-up in #4980.main, not introduced herecargo clippy -p trusty-memoryfails onorigin/main:This PR changes zero
.rsfiles, that file is byte-identical toorigin/main, and it reproduces undercargo clippy -p trusty-commonalone with trusty-memory's feature set — nothing from this branch involved. It surfaces only via that feature unification; trusty-common's defaults pass, which is why-p trusty-searchand-p trusty-analyzeare clean. CI's--workspace --all-targets -- -D warningsunifies features, so the clippy job on main is likely red independently of this PR. Deliberately not fixed here — burying an unrelatedtrusty-commonedit in a UI fix would hide a main-is-red signal.🤖🤖🤖 Generated with trusty-mpm — https://github.com/bobmatnyc/trusty-tools