# Web, frontend and TUI audit notes — 2026-10-03

Read-only component audit for the full repository report. No tracked product/test files were edited. Scratch Go probes use `go test -overlay` to add virtual test files; Go temp directories isolate HOME/USERPROFILE/APPDATA/LOCALAPPDATA/XDG_CONFIG_HOME. These notes deliberately distinguish defects from cleanup or unverified hardening candidates.

## Confirmed defects

### UI-01 — P2: Applying an unrelated Config edit overwrites custom values absent from fixed dropdowns

**Locations:** `internal/webassets/templates/config.html:58` (video output), `:69` (profile), `:81` (scale); `internal/webassets/static/config-page.js:33` / `:79` (collect and submit every field); `pkg/web/api_config.go:173` (writes every nonempty submitted field).

**Trigger:** An existing mpv.conf has `profile=cinema` with a valid `[cinema]` profile, `scale=ewa_lanczos`, and/or a video-output setting outside the fixed option list. Open Config and change only Screenshot Directory, then Apply.

**Behavior and impact:** The template never emits an option for the existing value. Browser `<select>` initialization chooses the first available option. Apply collects all controls, so the unrelated edit changes the active profile to `high-quality`, scale to `ewa_lanczossharp`, and video output to `gpu-next`. The profile block itself survives; its activation is lost. Users importing/editing their own configurations can lose their effective settings through an ordinary unrelated save. This is distinct from the previously fixed profile-aware parser: the new issue is at the template/form boundary.

**Verification:**

1. Actual Go handler/template overlay probe wrote a disposable config, parsed the real current settings, and rendered the shipped `config-content` template to `/tmp/mpv-manager-ui-audit-custom-config.html`. It reported existing effective values `profile="cinema" scale="ewa_lanczos" vo="libmpv"`.
2. T3 `preview_evaluate` parsed that exact rendered HTML into a detached Chromium HTML document. Real DOM values were `{profile:"high-quality",videoOutputDriver:"gpu-next",videoScaleFilter:"ewa_lanczossharp",explicitSelectedProfiles:[]}`.
3. The actual handler accepted the resulting form values with HTTP 200 and persisted `profile=high-quality`, `scale=ewa_lanczossharp`, `vo=gpu-next`, and the edited screenshot directory. `[cinema]\nbrightness=3` remained intact.

**Recommendation:** Preserve an existing unsupported value with a selected “Current/custom” option and submit only changed fields. Keep validation strict for explicitly requested replacements; preserving an unchanged custom value must not require treating it as a new allowlisted choice.

**Regression:** Render a config with a named custom profile and supported mpv values outside each dropdown; use a real browser to edit just screenshot directory and verify all untouched values remain byte/effectively unchanged. Also cover a profile list such as `profile=cinema,extra` if the parser permits it.

### UI-02 — P2: Clearing a Config text field returns success while retaining the old setting

**Locations:** `pkg/web/api_config.go:203`; `internal/webassets/static/config-page.js:37`, `:94`; `internal/webassets/templates/config.html:144` explicitly promises “Leave empty for current directory.”

**Trigger:** With `screenshot-directory=/tmp/existing-shots` configured, clear Screenshot Directory and Apply. The same omission affects text settings such as screenshot-template and screenshot-avif-opts.

**Behavior and impact:** ConfigApplyRequest uses plain strings, so absent and explicit-empty values are indistinguishable. `configUpdatesForApply` skips every empty string. The handler returns HTTP 200 `{"status":"success"}`; the frontend advances its saved baseline to the blank form and displays “All changes saved.” mpv.conf retains the old nonempty setting. The screenshot-directory default advertised in the page cannot be restored by clearing the input.

**Verification:** `/tmp/mpv-manager-ui-audit-probe.go`, `TestOctoberAuditEmptySettingProbe`, ran through the actual handler using Go overlay. POST body `{"screenshot_directory":"","screenshot_template":"","screenshot_avif_opts":""}` returned HTTP 200 success. On disk, all three original nonempty lines remained exactly unchanged.

**Recommendation:** Represent omission separately from explicit clearing (pointer/optional fields or a changed-fields request). Implement the correct option-reset/empty-value semantics atomically and report saved effective values to the frontend.

**Regression:** Start with nonempty screenshot directory, clear the real browser input, Apply, and assert both the persisted value/default and saved UI state after reload. Backend test should distinguish omitted from explicitly empty fields and preserve comments/profile sections.

### UI-03 — P2: A terminal SSE event during the details GET leaves the output dialog stuck Running

**Locations:** `internal/webassets/static/jobs.js:700` / `:709`; `internal/webassets/static/jobs-modal.js:53`, `:66`, `:92`.

**Trigger:** Open View Details while a job is finishing. Its GET `/api/jobs/{id}` captures a running snapshot. Before that response is delivered/processed, SSE delivers complete/error/cancelled. The details response then arrives.

**Behavior and impact:** No modal callbacks exist until the GET returns and `open` registers them. The terminal event has already settled the JobManager, but `open` blindly installs the stale GET state. There is no subsequent poll/reconciliation in the modal. It remains Running with a Cancel button and can hide an actual failure indefinitely. Closing/reopening fetches fresh state. This is separate from the fixed start-response/onJobComplete registration race.

**Verification:** `/tmp/mpv-manager-ui-audit-probe.cjs` executes the shipped `jobs.js` JobManager and shipped `jobs-modal.js` in a Node VM. It holds the details response, delivers an actual `handleJobStatus` terminal event, then releases a running response. Result: `{"modalStatus":"running","modalRunning":true,"knownStatus":"error","modalError":""}`. No modified production logic is used, but DOM/EventSource are test adapters: this is executable JavaScript integration evidence, **not** a real-browser/framework/SSE gate.

**Recommendation:** Own the selected job's observation before requesting details and merge buffered/newer events after the GET, or retain an authoritative terminal cache/generation and reconcile before opening. Add request ownership/generation so an older details response cannot replace a subsequently selected job's dialog.

**Regression:** Real browser with real handlers: hold a details GET after the server snapshots it, finish/fail the job through SSE, release the GET, and assert terminal state/error/output are shown. Include reordered requests for two different jobs.

### UI-04 — P3: Adoption refresh depends on a 1.5-second timer and excludes terminal completion

**Locations:** `internal/webassets/static/ui-select.js:457`; `internal/webassets/static/jobs.js:462`; `pkg/web/api_adopt.go:337` creates type `adopt`; `pkg/web/server.go:189` protects refresh-installed with resourceManagerConfig, which adoption leases.

**Trigger:** Import an externally detected player while selected UI download/setup takes more than 1.5 seconds (normal network timing).

**Behavior and impact:** The one-shot timer attempts refresh while adoption still leases manager configuration and receives 409. Actual terminal completion then passes through a whitelist that excludes `adopt`, so nothing retries the refresh. The card can remain Detected/Import to Manager although the app was successfully marked managed; Uninstall/Change UI controls do not appear until manual refresh/scan.

**Verification:** Executing shipped JobManager code in the same VM, `refreshAppsListIfNeeded({type:'adopt',status:'complete'})` caused zero htmx refresh calls; the install control caused one. Source confirms the timer runs during the held lease for a slow operation. No native installation/adoption was run.

**Recommendation:** Include adoption in terminal-triggered refresh; use the job's terminal callback to schedule refresh rather than predicting completion time. Keep retries for transient resource conflicts.

**Regression:** Hold an adoption worker beyond 1.5 seconds, verify refresh conflicts while active, complete it, and assert the actual browser card transitions from Detected to Managed without manual navigation.

### Optional minor UI defect — P3: An empty Tasks page stops observing newly created jobs

`internal/webassets/static/tasks-page.js:25` polls only when `this.active.length > 0`; the component does not subscribe to JobManager created/complete/status events. If the page loads empty, or the last job finishes, a new job started in another browser tab will update the SSE sidebar badge but never populate this page. Recommendation: observe global job events or keep a low-frequency idle poll. Source-confirmed call path; no dedicated executable reproduction beyond source inspection, so this is lower confidence/evidence strength than UI-01 through UI-04.

## Security and lifecycle review results

- Read `AGENTS.md`, README/architecture and September audit/reconciliation/review/remediation before reviewing current implementations.
- Loopback bind validation, literal localhost/loopback Host validation, same-origin Origin check, HttpOnly SameSiteStrict per-start cookie, OPTIONS rejection, bounded API bodies, and keyring rate limiting are present. No reachable remote auth/CSRF/DNS-rebinding bypass was established by this component audit.
- CSP rejects inline scripts/event attributes and htmx script execution is disabled. Rendered logs and untrusted dynamic UI text use Go template escaping, `escapeHtml`, or textContent/x-text along reviewed paths. `formatKeys` escapes all dynamic key tokens before template.HTML conversion. No concrete DOM XSS finding was established.
- Current Web shutdown excludes admission, closes SSE clients, cancels staging jobs, waits for committed workers and includes synchronous leased mutations in its WaitGroup. Output relays drain before archiving. This supports the previous lifecycle fixes; no repetition of September's already fixed HTTP-only shutdown finding.
- Current TUI `Model.Run`, operationRead ownership, commit guard, worker panic conversion, output drain and terminal history are implemented. Both signal-driven and framework-error exits have maintained real-framework regression coverage. Previous R2-01 is not repeated.
- TUI installer output/log/history paths sanitize terminal controls and bound retention. A scratch probe established that raw input.conf inline-comment OSC bytes reach `hotkeysBindingsListView` (`pkg/tui/hotkey_bindings.go:24`, `:62`, `:340`), but this is **not a confirmed clipboard-injection vulnerability**: current Bubble Tea v2 parses styled strings through Ultraviolet; its parser recognizes SGR/OSC8 links and ignores arbitrary OSC52 in that path. Retain a display-sanitization hardening candidate for user-controlled binding/path/record list text (styles/conceal/hyperlinks), not a claim of clipboard execution or RCE. The scratch Go probe is `/tmp/mpv-manager-tui-audit-probe.go`; it proves pre-render view retention only.
- TUI `runApplyPresetCmd` is still a conventional tea.Cmd outside installer operation ownership. Preset writes use lower-level atomic file operations; abrupt exit/signal behavior for this noninstaller mutation was not separately exercised. This is a lifecycle coverage candidate, not a confirmed corruption finding.

## Dead/duplicated-code and performance opportunities

1. **Runtime bundle includes frontend tests.** `internal/webassets/assets.go:8` embeds the whole `static` tree; all 22 `static/tests/*.test.js` files are therefore shipped and served by the runtime static file server. `wc -c` totals **101,187 bytes** of test sources. Move tests outside embedded runtime assets or embed only the intended runtime inventory. This is package/binary hygiene, not an XSS/security finding.
2. **Unused legacy frontend method.** `internal/webassets/static/jobs.js:73` defines `seedActiveJobs`, but a repository-wide reference search finds no call. SSE initial snapshot now owns seeding/reconciliation. Remove the obsolete second seeding implementation after verifying no external embedding contract needs it. Confirmed production-unreachable method, not a blanket claim that exported Go APIs are dead.
3. **Duplicate method policy tables.** `pkg/web/api_adopt.go:71` and `pkg/tui/ui_select.go:69` repeat the same six UI-capable install IDs; `pkg/tui/hwaccel_config.go:512` has another related MPV table; frontend ui-select has a partial special list. Centralize the shared capability classification in the existing constants/installer boundary. This is a maintainability candidate; the current matching six-item tables alone are not a defect.
4. **Repeated Settings reads.** `settings.js:17` starts `loadShortcutPreference`, `loadConfigFilePath`, and `loadInstallPath` independently; each calls `/api/settings`. One payload can populate all three and avoid three HTTP/API/config-path reads on every page load. Snapshot coherence improves too. No latency benchmark was run.
5. **Job list snapshots copy output headers they omit from JSON.** `pkg/web/api_jobs.go:48` calls GetActiveJobs/GetRecentJobs; `pkg/web/jobs.go:419`, `:810`, `:824` copy each output slice although `jobToAPIResponse` omits output. Up to 10 recent × 1,999 lines means roughly 320 KB of string-header allocations per full retained list snapshot on 64-bit systems, plus active jobs and an extra tail-marker copy for trimmed jobs. Strings themselves are shared; **this does not copy 10 MiB of output bytes**. Add summary snapshots for list/SSE metadata while keeping full copied output for details. Benchmark before prioritizing.
6. **Language-priority component lacks destroy.** `languages-priority.js:95` adds instances to a module-level Set and `:100` installs an anonymous window listener without a destroy handler. Ordinary navigation replaces the whole document; no current in-place wrapper replacement was established. Add proper teardown as lifecycle hygiene, not an unproven accumulating navigation leak.
7. **TUI background update checks remain context-free observation commands.** `pkg/tui/models_messages.go:221` invokes `version.CheckForUpdate` in a tea.Cmd rather than the owning process context. Existing bounded fetches limit lifetime and no live mutation occurs; cancellation coverage remains an observation-work optimization candidate. Earlier CLI cancellation finding R2-08 was fixed and is not being reasserted against that path.

## Validation evidence and limits

- Ran actual Go overlay tests `TestOctoberAuditEmptySettingProbe` and `TestOctoberAuditCustomSelectionsProbe`: both passed their assertions that the observed defects still exist.
- Ran Node VM probe for details/terminal race and adoption refresh: exact outputs recorded above.
- Ran pre-render TUI raw-comment probe: found raw OSC in model view; dependency review prevents overstating terminal execution.
- Used T3 `preview_status`, `preview_snapshot`, and corrected `preview_navigate` with environment-port target. Direct 127.0.0.1 and environment-relative 10.0.20.50 navigation both failed `net::ERR_CONNECTION_REFUSED` because the fixture's loopback listener is not reachable from the browser host. Did not change binding/security policy or switch to an alternate browser automation system. Detached Chromium DOM evaluation was successful, and is accurately scoped above.
- Root reports the existing **21 Chromium browser tests pass**. That full existing gate is separate from these focused reproductions; it does not cover the newly identified cases.
- No native Windows/macOS destructive QA, privilege/password operations, actual network adoption/install, or release claims were performed in this component review. Root owns broad normal/race/lint/frontend/build/security checks and Atlas.
- Scratch artifacts available for copying into dated evidence: `/tmp/mpv-manager-ui-audit-probe.go`, `/tmp/mpv-manager-ui-audit-overlay.json`, `/tmp/mpv-manager-ui-audit-custom-config.html`, `/tmp/mpv-manager-ui-audit-probe.cjs`, `/tmp/mpv-manager-tui-audit-probe.go`, `/tmp/mpv-manager-tui-audit-overlay.json`. Rename Go probes to `.go.txt` in documentation so they do not become Go packages.