test(ui): clear localStorage between component tests

A test file does not get its own origin. `@vitest/browser-playwright`
opens one BrowserContext per session and runs several files in it, one
after another, so everything a component persists survives from file to
file. `track-list` restores its sort in `connectedCallback` and
`aria-tail.test.ts` activates the Title column header, so any file that
mounts a track list later in that tab opens sorted by title — where
`track-11` precedes `track-3`, which is #138's failure exactly.

Nothing about it is specific to that pair: a probe that throws when a
test starts with a non-empty `localStorage` failed 24 test-starts in one
full run (11 with the two sort keys, 8 with `cover-grid-size`, 5 with
`track-list-column-widths`) and cascaded into 248 failures. Which files
share a tab, and in what order, changes run to run, which is the whole
of why this reads as a 1-in-3 flake and passes in isolation.

The clear belongs in `setup.ts` rather than in the specs that write,
because the spec that reads is never the one that knows — and it is
safe for the same reason the leak exists: files within a session are
sequential, so it cannot wipe storage a concurrent file is using.

The spec now also states the order it asserts rather than inheriting a
default, and checks the row it is about to double-click carries the path
it expects, so a stray sort fails by naming itself instead of as an
off-by-eight file path.

Closes #138

.pi/prompts/next-issue.md

@@ -133,71 +133,10 @@ status and `GET /api/v1/repos/yonlu/yellowjacket/actions/jobs/<id>/logs` for
 the log — and fix it. Two consecutive failed CI runs on the same cause: stop,
 comment what you know on the PR, and leave it for a human.
 
-**Do not merge, and do not review your own work.** Comment on the issue
-linking the PR, leave `Status/In Progress` on, and end the run. Whether
-this merges is decided by a reviewer that did not write it — see below.
-A CI run is not a review: it proves the tests you wrote pass, which is
-exactly the thing an author is worst placed to judge.
+**Do not merge.** Comment on the issue linking the PR, leave
+`Status/In Progress` on, and end the run.
 
 ## Finally
 
 Report in three lines: which issue you took, what state it is in
 (PR open / CI green / stopped and why), and any issues you filed.
-
----
-
-# The merge gate
-
-This half is **not** run by the author. It is run against a PR by
-someone who has not seen the branch before, and it decides whether the
-work lands on its own or waits for a human.
-
-A push to `main` publishes nothing here — `release.yml` is
-`workflow_dispatch` only and all four publishers key on `v*` tags — so
-the cost of a wrong merge is a bad commit on `main` and the time to
-revert it. That is the whole reason this gate can exist. If that ever
-changes, this section is void.
-
-**Merge only when every one of these is true.** Any single no means
-leave it open.
-
-1. An independent review of the diff returns **MERGE** or **MERGE WITH
-   NITS**. `CHANGES NEEDED`, or a review that could not reach a verdict,
-   means a human looks.
-2. `CI / check (pull_request)` **and** `CI / e2e (pull_request)` are
-   `success` on the PR's current head. Ignore the `(push)` contexts —
-   they are `skipped` by design and Gitea folds `skipped` into a
-   combined state of `pending` that never clears.
-3. The PR is mergeable with no conflicts, rebased onto current `main`.
-4. The diff touches **none** of: `.gitea/workflows/`, `.releaserc.yml`,
-   `packaging/`, `build/`, `scripts/gitea-release.sh`,
-   `backend/database/sql/schemas/`, `backend/database/staleshape*.go`,
-   `go.mod`, `go.sum`. These either publish to somewhere a mistake
-   cannot be taken back from, or can destroy a database that a user
-   cannot rebuild.
-5. The issue is `Kind/Bug`, `Kind/Testing`, `Kind/Documentation` or
-   `Kind/Enhancement`. **A `Kind/Feature` is a design decision and is
-   never auto-merged**, however green it is.
-6. The diff is under ~600 changed lines across under ~15 files. Past
-   that, "a reviewer read it" stops being a claim anyone should take on
-   trust.
-7. The PR does not claim to have verified something no tier here can
-   see. A change whose evidence would have to come from a physical
-   device is reported, not merged.
-
-**When it merges**, use `{"Do":"merge"}` on
-`POST /api/v1/repos/yonlu/yellowjacket/pulls/<n>/merge`. Then **check
-the issue actually closed** — a squash or an edited merge message drops
-the `Closes` footer — and close it by hand with a comment naming the
-commit if it did not. `unclaim.yml` strips `Status/In Progress` on
-close; if the label is still there a minute later, strip it yourself.
-
-**When it does not merge**, say so on the PR in one paragraph: which
-condition failed and what would satisfy it. Leave the PR open, leave
-the label on, and file the review's substantive findings as issues so
-they are searchable rather than buried in a PR comment.
-
-**A nit is not a blocker, and it is not free either.** A `MERGE WITH
-NITS` merges, and each nit worth keeping becomes an issue. Do not fix
-nits on the branch: that is a second author pass with no second review,
-which is the thing this gate exists to prevent.

CLAUDE.md

@@ -262,6 +262,26 @@ real store code. A binding carries an **ID**, not a name
 `yellowjacket/backend/home.Service.GetShelves`), so the fake derives
 that map from the generated tree rather than writing it down.
 
+**A test file does not get its own origin, so `setup.ts` clears
+`localStorage` between tests.** `@vitest/browser-playwright` opens one
+BrowserContext per session and runs several files in it one after
+another, so everything a component persists — the track list's sort and
+column widths, the cover size, `now-playing`'s scroll mode — is still
+there when the next file mounts the same component. Which files share a
+tab, and in what order, changes run to run, so the symptom is a spec
+that fails about one test in three and passes every time it is run on
+its own: #138 cost three scheduled runs, one of them a PR whose diff
+held no frontend code at all. Measured on the build before the fix, a
+single full run started **24** tests with storage already set. Two
+things follow. The clear is safe precisely because the leak is
+sequential — files in a session do not overlap, so it cannot wipe
+storage a concurrently-running file is in the middle of using — and it
+belongs in `setup.ts` rather than in the specs that write, because the
+spec that *reads* is never the one that knows. And a spec whose
+assertion depends on an order still **states that order** rather than
+inheriting a default, or the next change to a default is the same
+mystery again.
+
 **`frontend/bindings/` is generated by `wails3`, not `go generate`**, so
 the pre-commit codegen check does not cover it. `make bindings-check`
 (~3.5 s warm, ~20 s on a cold build cache, also a pre-commit hook)

frontend/test/components/play-in-context.test.ts

@@ -161,6 +161,15 @@ describe('double-clicking a row in the track list', () => {
     localStorage.removeItem('track-list-column-widths');
 
     el = await fixture<LitElement>('track-list', { externalTracks: LIST });
+
+    // Say which order is being asserted rather than inheriting one.
+    // `restoreSortPreferences()` runs in `connectedCallback`, so the
+    // list opens in whatever sort was last persisted -- and
+    // `track-11` sorts before `track-3` by title, which is the shape
+    // of #138. The row below is the fixture's third track only while
+    // nothing is sorting the list.
+    (el as unknown as { sortField: string | null }).sortField = null;
+
     el.style.display = 'block';
     el.style.height = '600px';
     await flush();
@@ -172,6 +181,10 @@ describe('double-clicking a row in the track list', () => {
     const rows = shadowAll(el, '.track-row');
     const row = rows.find((r) => r.getAttribute('data-index') === '3');
 
+    // Stated first, so a list that is not in the order this asserts
+    // fails by saying so rather than as an off-by-eight file path.
+    expect(row?.getAttribute('data-file-path')).toBe('/music/track-3.mp3');
+
     dblclick(row!);
     await flush();
 

frontend/test/setup.ts

@@ -72,6 +72,23 @@ document.body.style.margin = '0';
 
 beforeEach(() => {
   resetHarness();
+
+  // A test file does not get its own origin. `@vitest/browser-playwright`
+  // opens one BrowserContext per session and runs several files in it,
+  // one after another, so everything a component persists — the track
+  // list's sort and column widths, the cover size, `now-playing`'s
+  // scroll mode — is still there when the next file mounts the same
+  // component. That is invisible until it is intermittent, because
+  // which files share a tab and in what order changes run to run: it
+  // cost #138 three scheduled runs, one of them a PR with no frontend
+  // code in it at all.
+  //
+  // Clearing here rather than in the specs that write is deliberate —
+  // the spec that *reads* is never the one that knows. It is safe for
+  // the same reason the leak exists: files in a session are
+  // sequential, so this cannot wipe storage a concurrent file is in
+  // the middle of using.
+  localStorage.clear();
 });
 
 afterEach(() => {