Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 5 additions & 14 deletions app/frontend/js/api/library.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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);
Expand Down
35 changes: 33 additions & 2 deletions app/frontend/js/api/shared.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -18,14 +30,29 @@ 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')
* @param {object} options - Fetch options
* @returns {Promise<any>} Response data
*/
export async function request(endpoint, options = {}) {
const url = `${API_BASE}${endpoint}`;
const url = `${apiBase}${endpoint}`;

const config = {
headers: {
Expand All @@ -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);

Expand Down
29 changes: 29 additions & 0 deletions app/frontend/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
@@ -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: []
Expand All @@ -21,9 +21,47 @@ This is the final subtask of the Zig sidecar POC: prove the frontend can call th

## Acceptance Criteria
<!-- AC:BEGIN -->
- [ ] #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)
<!-- AC:END -->

## Implementation Notes

<!-- SECTION:NOTES:BEGIN -->
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<SidecarEndpoint> { 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 <token>` 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:<port>/api/library?limit=2&sort_by=artist&sort_order=asc` with `Authorization: Bearer <token>` → **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.
<!-- SECTION:NOTES:END -->
2 changes: 2 additions & 0 deletions crates/mt-tauri/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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,
Expand Down
28 changes: 27 additions & 1 deletion crates/mt-tauri/src/sidecar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -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")]
Expand Down Expand Up @@ -197,6 +208,21 @@ async fn probe_health(runtime_dir: &Path) -> Result<Endpoint, ProbeError> {
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<SidecarEndpoint> {
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.
Expand Down
Loading