diff --git a/app/frontend/js/api/library.js b/app/frontend/js/api/library.js index 178c4a7e..1e2389ad 100644 --- a/app/frontend/js/api/library.js +++ b/app/frontend/js/api/library.js @@ -78,7 +78,10 @@ export const library = { }, /** - * Get all tracks in library (uses Tauri command) + * Get all tracks in the library over HTTP. + * + * This is the one endpoint the Zig sidecar serves (TASK-355.3/355.5), so it + * calls the sidecar directly rather than going through `library_get_all`. * @param {object} params - Query parameters * @param {string} [params.search] - Search query * @param {string} [params.sort] - Sort field @@ -87,19 +90,7 @@ export const library = { * @param {number} [params.offset] - Offset for pagination * @returns {Promise<{tracks: Array, total: number, limit: number, offset: number}>} */ - async getTracks(params = {}) { - const result = await tauriInvoke('library_get_all', { - search: params.search || null, - artist: params.artist || null, - album: params.album || null, - sortBy: params.sort || null, - sortOrder: params.order || null, - limit: params.limit || null, - offset: params.offset || null, - ignoreWords: params.ignoreWords || null, - }); - if (result !== null) return result; - // Fallback to HTTP + getTracks(params = {}) { const query = new URLSearchParams(); if (params.search) query.set('search', params.search); if (params.sort) query.set('sort_by', params.sort); diff --git a/app/frontend/js/api/shared.js b/app/frontend/js/api/shared.js index e826b7e8..32f08e95 100644 --- a/app/frontend/js/api/shared.js +++ b/app/frontend/js/api/shared.js @@ -5,7 +5,19 @@ * ApiError class, HTTP request helper, and Tauri invoke wrapper. */ -const API_BASE = 'http://127.0.0.1:8765/api'; +/** + * Base URL of the HTTP API, including the `/api` prefix, plus the bearer token + * every request carries. Both default to the values a locally running sidecar + * uses and are replaced at runtime by `setBackendEndpoint` with what the Rust + * side reports (see `sidecar_get_endpoint`). Mutable module state rather than a + * constant because the port is chosen by the OS, so it is not knowable at build + * time; the defaults keep `request()` working when there is no Tauri context at + * all (Vitest runs in Node) or before the injected value arrives. + */ +const DEFAULT_API_BASE = 'http://127.0.0.1:8765'; + +let apiBase = `${DEFAULT_API_BASE}/api`; +let apiToken = null; /** * Custom API error class @@ -18,6 +30,21 @@ export class ApiError extends Error { } } +/** + * Replace the base URL and bearer token that `request()` uses. + * + * @param {string} baseUrl - Sidecar origin (e.g., 'http://127.0.0.1:43210'); the + * `/api` prefix is appended here, so callers pass the origin only. A + * falsy value leaves the current base URL untouched. + * @param {string|null} [token] - Bearer token; omitted or falsy means no + * `Authorization` header is sent. + */ +export function setBackendEndpoint(baseUrl, token = null) { + if (!baseUrl) return; + apiBase = `${baseUrl.replace(/\/$/, '')}/api`; + apiToken = token || null; +} + /** * Make an API request with error handling * @param {string} endpoint - API endpoint (e.g., '/library/tracks') @@ -25,7 +52,7 @@ export class ApiError extends Error { * @returns {Promise} Response data */ export async function request(endpoint, options = {}) { - const url = `${API_BASE}${endpoint}`; + const url = `${apiBase}${endpoint}`; const config = { headers: { @@ -35,6 +62,10 @@ export async function request(endpoint, options = {}) { ...options, }; + if (apiToken) { + config.headers.Authorization = `Bearer ${apiToken}`; + } + try { const response = await fetch(url, config); diff --git a/app/frontend/main.js b/app/frontend/main.js index bb98a91c..bcac2a41 100644 --- a/app/frontend/main.js +++ b/app/frontend/main.js @@ -10,6 +10,7 @@ import { settings } from './js/services/settings.js'; import { handleFilesDrop, handleInternalTrackDrop } from './js/utils/tauri-drag-drop.js'; import { installGlobalErrorHandlers } from './js/utils/error-reporter.js'; import { initWebVitals } from './js/utils/web-vitals.js'; +import { setBackendEndpoint } from './js/api/shared.js'; import './styles.css'; // Install global error handlers early so unhandled errors reach the backend log @@ -82,6 +83,31 @@ function initGlobalKeyboardShortcuts() { initKeyboardShortcuts(); } +/** + * Point the HTTP API client at the sidecar port this process actually got. + * The sidecar binds an ephemeral port and writes it, with its bearer token, to + * the runtime file the Rust health probe reads, so the value has to come from + * the running app rather than a constant. Any failure — no Tauri context, an + * unspawned sidecar, or a per-test invoke stub that answers unknown commands + * with `undefined` — leaves the client on its default base URL. + */ +async function initBackendUrl() { + if (!window.__TAURI__) return; + + try { + const { invoke } = window.__TAURI__.core; + const endpoint = await invoke('sidecar_get_endpoint'); + if (!endpoint || !endpoint.baseUrl) { + console.warn('[main] No sidecar endpoint reported, using default API base URL'); + return; + } + setBackendEndpoint(endpoint.baseUrl, endpoint.token); + console.log('[main] Backend URL:', endpoint.baseUrl); + } catch (error) { + console.warn('[main] Failed to get backend URL, using default:', error); + } +} + async function initTitlebarDrag() { if (!window.__TAURI__) return; @@ -199,6 +225,9 @@ async function initApp() { const t = { start: performance.now() }; window._perfTimings = t; + // Resolve the sidecar endpoint before anything issues an HTTP request. + await initBackendUrl(); + // Initialize settings service first (loads settings from backend) if (window.__TAURI__) { try { diff --git a/backlog/tasks/task-355.6 - Frontend-flip-for-the-one-endpoint-and-the-POC-report.md b/backlog/tasks/task-355.6 - Frontend-flip-for-the-one-endpoint-and-the-POC-report.md index 49caf597..ed13f240 100644 --- a/backlog/tasks/task-355.6 - Frontend-flip-for-the-one-endpoint-and-the-POC-report.md +++ b/backlog/tasks/task-355.6 - Frontend-flip-for-the-one-endpoint-and-the-POC-report.md @@ -1,7 +1,7 @@ --- id: TASK-355.6 title: 'Frontend flip for the one endpoint, and the POC report' -status: To Do +status: In Progress assignee: [] created_date: '2026-09-11 00:39' labels: [] @@ -21,9 +21,47 @@ This is the final subtask of the Zig sidecar POC: prove the frontend can call th ## Acceptance Criteria -- [ ] #1 app/frontend/js/api/shared.js reads its base URL and bearer token from a value injected by the Rust side at runtime, not from a hardcoded constant -- [ ] #2 Exactly one frontend API domain module calls the sidecar endpoint directly with no tauriInvoke fallback -- [ ] #3 No file under app/frontend/js/stores/ is modified -- [ ] #4 Existing Vitest and Playwright suites both pass unmodified +- [x] #1 app/frontend/js/api/shared.js reads its base URL and bearer token from a value injected by the Rust side at runtime, not from a hardcoded constant +- [x] #2 Exactly one frontend API domain module calls the sidecar endpoint directly with no tauriInvoke fallback +- [x] #3 No file under app/frontend/js/stores/ is modified +- [x] #4 Existing Vitest and Playwright suites both pass unmodified - [ ] #5 A report is recorded on this task covering: cold and warm Zig build times, the CI wall-clock delta measured against the TASK-352 baseline, the Rust dependency-count delta, every unexpected obstacle encountered across all six subtasks, and an explicit go/no-go recommendation for proceeding to the larger migration (writes, queue, scanner, Last.fm, Plex, and the full frontend flip) + +## Implementation Notes + + +AC#1-#4 only. AC#5 (the POC report and go/no-go recommendation) is deliberately not written here. + +### What was built + +Revives the runtime-injection mechanism this repo had for the Python PEX sidecar (`get_backend_url` in `621f681`, consumed by `initBackendUrl()` in `b3cc99c`, deleted wholesale in `9feabb0`), adapted for the two things that pattern never had to carry: a bearer token, and a `shared.js` split out of the old monolithic `api.js`. + +**`crates/mt-tauri/src/sidecar.rs`** — new `sidecar_get_endpoint` Tauri command returning `Option { baseUrl, token }`, camelCase-serialized to match the JS field names. It reads through the existing `SidecarState::endpoint()` (TASK-355.5's accessor for the endpoint the *startup health probe* already resolved) — there is no second parse of `sidecar.json`, so the frontend can never see an endpoint the probe never confirmed live. `None` (probe failed / sidecar never spawned) serializes to `null`, which is the frontend's cue to keep its default. `http://127.0.0.1:{port}` matches the existing convention in `probe_health` and `shadow_diff`. Registered in `lib.rs`'s `generate_handler![...]` next to `app_get_info`. + +**`app/frontend/js/api/shared.js`** — `const API_BASE` becomes module-level `let apiBase` / `let apiToken` with the old hardcoded value (`http://127.0.0.1:8765/api`, no token) as the default, plus `setBackendEndpoint(baseUrl, token)`, which appends the `/api` prefix itself so callers pass an origin (same contract as the old `setApiBase(url + '/api')`, one step less error-prone). A falsy `baseUrl` is ignored rather than corrupting the default. `request()` attaches `Authorization: Bearer ` only when a token is set, so header-agnostic fetch mocks and the tokenless default path behave exactly as before. + +**`app/frontend/main.js`** — `initBackendUrl()`, awaited at the top of `initApp()` before `settings.init()`, stores, components and `Alpine.start()`, so nothing issues an HTTP call against the pre-injection base URL. Guarded by `if (window.__TAURI__)` and try/catch like the prior art, *plus* a falsy-result check: several specs (`plex.spec.js:154`, `library-settings.spec.js:50`) stub `window.__TAURI__.core.invoke` to resolve `undefined` for commands they don't recognize, so a thrown error is not the only failure mode worth surviving — a `null`/`undefined` reply logs and keeps the default instead of propagating `undefined.baseUrl`. + +**`app/frontend/js/api/library.js`** — `getTracks()` loses its `tauriInvoke('library_get_all', ...)` branch entirely and calls `request('/library?...')` directly; the query-string building is unchanged. It is the only domain module flipped, because it is the only endpoint with a Zig-side implementation (TASK-355.3/355.5). The other ten modules still `tauriInvoke` commands with no sidecar counterpart. With the branch gone the method no longer contains an `await`, so `async` was dropped — it returns the same promise `request()` returns, which is what `require-await` (enforced by `task lint` over `app/frontend/js/**/*.js`) wants. + +### Verification + +- **AC#1 (injection works end to end against the real sidecar).** Spawned the staged `mt-zig-core` binary against `tests/fixtures/mt_fixture.db`, read its `sidecar.json`, and drove it with exactly the URL and header shape `setBackendEndpoint` + `request()` build: `GET http://127.0.0.1:/api/library?limit=2&sort_by=artist&sort_order=asc` with `Authorization: Bearer ` → **200, `total=301`, 2 tracks, first artist `Beginbot`**; the same URL *without* the header → **401 `{"detail":"unauthorized"}`**. That is the proof the injected port and token are the ones actually used, since the port is ephemeral (44933/51823 across two runs) and could not have come from the hardcoded default. The command's own payload was verified this way too, field for field. What is *not* covered: a real Tauri webview, i.e. the last hop from `sidecar_get_endpoint` to `initBackendUrl()` — this sandbox has no display/Tauri runtime, so `@tauri`-tagged tests stay out of scope here, same as TASK-355.5. +- **AC#2 — `api.errors.test.js` "Concurrent Request Handling" passes**, calling `api.library.getTracks()` three ways against a mocked global `fetch` with no `window.__TAURI__` present; that test exercises the flipped code path unmodified. The flip is a dead-branch removal, not a behavior change, for the existing suites: `tauriInvoke` returns `null` whenever `window.__TAURI__` is absent (all Vitest, and every Playwright spec except the seven that hand-roll their own `invoke` mock), so `request()` was already the path taken in practice. +- **AC#3 — `git status app/frontend/js/stores/` is empty.** The whole diff is `js/api/shared.js`, `js/api/library.js`, `main.js`, and the two Rust files. +- **AC#4 — Vitest: 626 passed / 17 failed (34 files), failure set byte-identical to the pre-change baseline** (captured by stash-and-compare; `comm -3` of baseline vs. after failure lists is empty). Those 17 are pre-existing and in files this diff does not touch (`context-menu-favorites`, `go-to-album`, `go-to-artist`, `library.store` FOUC #2 — `isRemote is not a function` family, already recorded on TASK-355.4/355.5). +- **AC#4 — Playwright (chromium, non-`@tauri`): 504 passed / 5 failed / 2 skipped.** All 5 — `lastfm.spec.js:201`, `library-type-to-jump.spec.js:187`, 3× `plex.spec.js` cloud badge — are in the baseline failure set, and the lastfm one was additionally re-proved individually: it fails with the same `strict mode violation: locator('text=Awaiting Authorization')…resolved to 2 elements` on the stashed clean tree as with the diff applied. The suite ran unmodified; no spec was edited, skipped, or rewritten. + - Two environment caveats, stated rather than papered over. (1) **WebKit, the default `fast` engine, cannot launch on this host** — AlmaLinux 10 ships `libjpeg.so.62`/`libjxl.so.0.10` where Playwright's webkit build wants `libjpeg.so.8`/`libjxl.so.0.8` (same finding as TASK-355.5); the 511 webkit tests error out with "Host system is missing dependencies", so the real browser evidence here is chromium-only. (2) **The 8 `visual-regression` snapshot tests failed in the first baseline run and pass now** — they self-skip when `CI` is set, because `*-snapshots/` is gitignored and this worktree starts with no baselines; the baseline run generated them locally, so they are no longer a comparison point either way. Nothing about the change affects them (they assert DOM/screenshot state, and the default `API_BASE` is the same string it was before). +- **Rust: 892 passed / 0 failed** (`cargo test --workspace`, 2 ignored). Two `shadow_diff_parity_test` cases were failing on this machine before and after any edit of mine, for an environment reason worth recording since it is easy to mistake for a regression: they need a *staged* sidecar binary and a *populated* fixture, and resolve the former as `mt-zig-core{host}` where `host_triple()` returns `x86_64-unknown-linux-gnu` but `task zig:stage` writes `mt-zig-core-x86_64-unknown-linux-gnu` — a missing hyphen in the test's own candidate path, so only its second candidate, `zig-core/zig-out/bin/mt-zig-core` from `task zig:build`, can ever satisfy it. Building that binary and regenerating the fixture (`task zig:build`, then the `#[ignore]`d `generate_mt_fixture_test`, which is what `task zig:fixture` invokes — that task's own filter is also wrong: it drops the `_test` suffix and matches 0 tests, silently leaving 0 rows in `mt_fixture.db`) makes both pass. **Both mismatches are pre-existing and left untouched** — out of this task's scope, and neither is reachable from the ACs here. +- **Lint/format:** `task lint` clean (deno lint + clippy + zig fmt), `deno fmt --check` clean, `rustfmt --check` clean on both Rust files. `cargo clippy -p mt-tauri --all-targets` reports zero findings in `sidecar.rs`; its 7 lib warnings are the same pre-existing `plex.rs`/`removed.rs`/`lib.rs` ones recorded on TASK-355.5. + +### Human review (post-run) + +Reviewed the agent's commit (`7e38f6a`) directly rather than trusting the notes above at face value: + +- Independently re-ran Vitest (626 passed / 17 failed) and Playwright chromium (`E2E_MODE=full --project=chromium`: 504 passed / 5 failed / 2 skipped) — both match the reported counts and failure sets exactly. Confirmed all 5 Playwright failures reproduce identically against base `main` (`92feb45`) in a throwaway comparison worktree, independent of the agent's own stash-based claim. +- Found and reverted two out-of-scope stylistic hunks the agent's diff carried that the ACs never asked for: `main.js`'s `handleFileDrop`/`testDialog` `function` → arrow-function conversion, and a ternary condition-order flip in `library.js`'s `getSection` `page` calculation. Both were semantically identical to what they replaced; reverted in `7bb6a72` to keep the diff to what AC#1-#4 describe. (Note: `main.js` isn't in `deno.jsonc`'s lint `include` list, so neither version affected `task lint`.) +- Discarded a stray, uncommitted `deno fmt`-style reformatting (double-quotes, altered indent) found sitting dirty in the worktree ~46s after the agent's commit — pure whitespace/quote noise (confirmed via `git diff -w`), never staged or committed by the agent itself, not part of the reviewed diff. +- Status left as `In Progress`, not `Done`: AC#1-#4 are genuinely satisfied, but AC#5 (the POC report/recommendation) remains outstanding and is being done separately. + diff --git a/crates/mt-tauri/src/lib.rs b/crates/mt-tauri/src/lib.rs index 247adf57..ff259cbf 100644 --- a/crates/mt-tauri/src/lib.rs +++ b/crates/mt-tauri/src/lib.rs @@ -64,6 +64,7 @@ use scanner::commands::{ scan_paths_to_library, }; use serde::Serialize; +use sidecar::sidecar_get_endpoint; use std::time::Duration; use tauri::{Emitter, Manager, State}; use tauri_plugin_global_shortcut::{Code, GlobalShortcutExt, Modifiers, Shortcut}; @@ -473,6 +474,7 @@ pub fn run() { media_set_paused, media_set_stopped, app_get_info, + sidecar_get_endpoint, export_diagnostics, save_file, log_frontend_error, diff --git a/crates/mt-tauri/src/sidecar.rs b/crates/mt-tauri/src/sidecar.rs index 268fb074..862ed377 100644 --- a/crates/mt-tauri/src/sidecar.rs +++ b/crates/mt-tauri/src/sidecar.rs @@ -10,7 +10,7 @@ use std::sync::Arc; use std::time::Duration; use parking_lot::Mutex; -use serde::Deserialize; +use serde::{Deserialize, Serialize}; use tauri::{AppHandle, Manager, Runtime}; use tauri_plugin_shell::ShellExt; use tauri_plugin_shell::process::{CommandChild, CommandEvent}; @@ -27,6 +27,17 @@ pub(crate) struct Endpoint { pub(crate) token: String, } +/// What the frontend needs to talk to the sidecar directly: the loopback base +/// URL plus the bearer token every request must carry. Serialized under +/// camelCase keys, matching the JS-side field names (Tauri's default argument +/// and return conversion). +#[derive(Debug, Clone, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct SidecarEndpoint { + pub base_url: String, + pub token: String, +} + #[derive(Debug, thiserror::Error)] enum ProbeError { #[error("sidecar runtime file did not appear within the probe window")] @@ -197,6 +208,21 @@ async fn probe_health(runtime_dir: &Path) -> Result { Ok(endpoint) } +/// Hand the frontend the sidecar's base URL and bearer token, so it can call +/// the sidecar over HTTP instead of through a Tauri command. `None` until the +/// startup health probe has resolved an endpoint — or if the sidecar never +/// started — which is the frontend's signal to keep its default base URL. +#[tauri::command] +pub(crate) fn sidecar_get_endpoint( + state: tauri::State<'_, SidecarState>, +) -> Option { + let endpoint = state.endpoint()?; + Some(SidecarEndpoint { + base_url: format!("http://127.0.0.1:{}", endpoint.port), + token: endpoint.token, + }) +} + /// Kills the sidecar on app exit, called from `RunEvent::Exit`. Idempotent: /// the child is `take()`n, so a second call (or a prior `Terminated` event) /// is a no-op rather than a double-kill error.