Compare commits

..
Author SHA1 Message Date
yonlu 8d46c4abb7 fix(scripts): refuse to start dev-headless on a port somebody else holds
CI / check (push) Skipped
CI / e2e (push) Skipped
dev-headless.sh checked the PID in *this* worktree's .dev/app.pid and
nothing else, so an app orphaned by a deleted worktree went on listening
with nothing left to stop it — `make dev-stop` only kills the pid it
wrote. The new app then started, failed to bind, exited, and every
subsequent curl and playwright-cli call went to the other process: the
harness reported facts about an app nobody asked for.

That fails a long way from its cause. It presented as "no such table:
libraries" against a *freshly created* YJ_HOME, which reads exactly like
applySchema or staleshape.go having gone wrong, with a zero-byte app.log
beside it saying nothing.

The startup wait cannot catch this, because its health check is satisfied
by any app on the port — which is precisely the failure — so the check is
before the launch and refuses rather than warns. It names the holder's
pid, cmdline and /proc/<pid>/cwd, which is what identifies the checkout
and says "(deleted)" for the case this exists for. It does not suggest
`make dev-stop`: the PID-file check has already passed, so by
construction dev-stop does not know about this process and would report
success while changing nothing. --port already covers the legitimate
second-app case.

The second, cheaper guard the report asks for goes in after the wait:
"the port answered" is not "the app we started answered", so a dead
APP_PID at that point is now an error with the log tail rather than a
success message about somebody else's process.

Closes #119
2026-08-19 14:08:05 -04:00
logan bb7dde1963 Merge pull request 'A CI-only change is ci:, not fix(ci):' (#112) from docs/ci-commit-type into main
CI / check (push) Successful in 2m26s
CI / e2e (push) Successful in 6m31s
2026-08-19 16:23:20 +00:00
yonlu 446380e3a9 docs: a CI-only change is ci:, not fix(ci):
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / e2e (pull_request) Successful in 6m41s
CI / check (pull_request) Successful in 2m28s
The commit-analyzer reads the type and ignores the scope, so `fix` is a
patch whatever sits in the brackets. Two commits touching nothing but
.gitea/workflows/unclaim.yml were written `fix(ci):` and cut v0.2.1 and
v0.2.2 -- real releases, published to Arch, Homebrew and the APK
registry, containing no user-facing change.

CLAUDE.md already warned that a mistyped feat ships a minor version.
That was not enough, because this was not a mistyped type: `fix` was
chosen deliberately, in the belief that the (ci) scope qualified it.

The version bump is the small half, which is why this gets a paragraph
rather than a clause. A merge to main starts two workflows; if
release.yml then pushes a tag, that tag push starts four more --
arch-package, homebrew-formula, android-apk and desktop-assets -- on a
runner with capacity 1, where the APK build alone is tens of minutes
and publishes a signed artifact to a public registry. So a mistyped
type is six workflow runs, not an odd-looking changelog.

`make release-dry` answers this before the merge instead of after, and
is cheaper than any one of those runs.

The two releases are staying: they are already published, and a version
that vanishes is worse for whoever pulled it than one that turns out to
be empty.

Closes #111
2026-08-19 16:03:38 +00:00
logan e07f248cc8 Merge pull request 'Wait for the scroll range the assertion needs' (#134) from fix/133-album-dropdown-scroll-race into main
CI / e2e (push) Successful in 6m39s
CI / check (push) Successful in 2m33s
2026-08-19 16:03:17 +00:00
logan 90ac6e0825 test(e2e): wait for the scroll range the assertion needs
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 2m33s
CI / e2e (pull_request) Successful in 6m22s
The guard polled for `scrollHeight > clientHeight + 40` and the next
line asserted the container could be scrolled to 80, so any range in
41-79 satisfied the precondition and could not satisfy the assertion.
The grid passes through exactly that while it settles, because it
recomputes its columns after a viewport change rather than during it,
so the test read a clamped scrollTop and reported 10 against 80.

It failed CI on a pull request that changes one paragraph of CLAUDE.md
and nothing else, while WebKit passed in the same run. Reproduced
locally: 0 failures in 6 runs before #132, 2 in 9 after, 0 in 10 with
this change.

#132 is what made it reachable rather than what broke it. The queue
panel's mode is measured rather than media-queried, so a viewport
change at this width costs one more layout pass, and cover-grid settles
after it instead of before. The settled range is 330 and stable, the
main panel is 700px, and the panel is correctly display:none while
closed — there is no user-visible defect, only a wider window for a
race the spec already had.

A threshold below the value its caller depends on is not a guard, so
the target is one constant that both the guard and the assertion read.

Closes #133
2026-08-19 11:50:46 -04:00
logan 4e3c953acf Merge pull request 'Decide the supported sizes, and stop the queue taking the page's width' (#132) from feat/24-supported-sizes-queue-model into main
CI / check (push) Successful in 2m30s
CI / e2e (push) Successful in 6m20s
2026-08-19 15:22:39 +00:00
logan ede183d026 test(shell): check 900x600, which is narrower than the minimum
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 2m34s
CI / e2e (pull_request) Successful in 6m42s
The sidebar collapses to icons *below* 900, so the main panel is 843px
at 899 and 700px at 900: the narrowest content area any desktop width
produces is at the top of the Compact band, not at the enforced floor.
A viewport list that stopped at "the minimum" was missing its own worst
case.

MinWidth's comment loses both reasons it used to give, because neither
mechanism can happen any more — the subtitle is display:none from 899
down, and the sidebar host is overflow-y:auto (at 600x460 its
scrollHeight is 434 against a 332px client, and Settings is reachable
after scrolling). The value does not change: 800x600 is where desktop
chrome stops being comfortable, not where the app breaks, and below
600 the phone layout takes over. A floor defended by two expired
mechanisms is a number nobody can argue with, which is worse than
either answer.

Closes #24
2026-08-19 10:54:52 -04:00
logan 481c9dca65 docs: record the size bands and what the queue model cost to find
CLAUDE.md gains the three bands as a promise (Phone <600, Compact
600-899, Desktop >=900, and "no action is ever unreachable at any
supported size"), the computed queue rule and why it cannot be a media
query, and the correction that 900 — not the 800x600 minimum — is the
worst desktop width.

NOTES.md gets the measurements, including two things worth more than
the fix. My first probe for the sidebar's scroller searched
shadowRoot.querySelectorAll('*') and reported "no scroller, items are
unreachable", which reads exactly like a live Settings-unreachable bug;
the scroller is the host, and a host is not inside its own shadow root.
And the plan's first draft claimed the overlay "removes the desktop
half of #69", which the screenshot disproved: open and closed are now
identical at 900x600, so the queue's contribution is gone, but the
header's own overflow remains and is still a live defect.

Refs #24
2026-08-19 10:54:52 -04:00
logan 4025106234 fix(queue): overlay the content instead of taking its width
The panel is flex-shrink: 0 in the flow of .content-area, so an open
queue was paid for by the main panel rather than covering it. Measured
on Playlists: 379px of content left at 900x600 with all three of the
page header's actions clipped, 69px at 390px, and 0px at 320px — where
the content was not degraded but gone.

It goes to an overlay with a scrim when the content cannot spare the
width, and the rule is computed rather than breakpointed:
`available - panelWidth < 480`, where available is .content-area's
width and so already accounts for the sidebar's collapse at 900. A
media query cannot express this, which is the reason for the property:
the panel is drag-resizable between 200 and 500px and persisted, so a
viewport breakpoint silently assumes the default 320 and is wrong by up
to 180px for a user who widened it — in the direction that hurts, since
a wider queue is exactly when the content can least afford it.

480 is a judgement and the comment says so: there is no cliff to derive
it from (the track list rescales continuously, 213px to 124px columns
with no row overflow), so it is anchored to keep the default 1100px
window inline while putting every measured-broken case on the overlay
side.

The overlay is a presentation and not a fork — #55 asks for one
component with two mount points — so the roving tab stop, Alt+Arrow
reorder, drag reorder and selection semantics are untouched. Escape
closes it and returns focus, attached only while the overlay is up: it
is a dismissal rather than a shortcut, which is why it is not a
panel-scoped binding. The scrim covers the content area only, not the
sidebar or the transport, because the queue is not modal.

Refs #24
2026-08-19 10:54:52 -04:00
logan a3134f997f docs(planning): decide the supported sizes and the queue panel's model
#24 asks for a design pass, and #73 hangs the rest of Phase 2 off the
answer, so the decision is written down before any CSS moves.

Measured against the running app, and five things are not in the issue:
the Playlists header clips at 800x600 with the queue *closed* — the
minimum window is the only size this app promises; 900x600 is worse
than 800x600, because the sidebar expands at 900, so the worst desktop
case is not the minimum and every test that stops at the minimum misses
it; at 320px with the queue open the main panel is 0px wide, because
the panel is in the flow rather than over it; only Playlists overflows,
so #69 is one view's action set and not a systemic header failure; and
both reasons in MinWidth's comment describe mechanisms that no longer
exist.

The queue's mode cannot be a media query: its width is drag-resizable
between 200 and 500px and persisted, so a fixed breakpoint assumes the
default 320 and is wrong by 180px in the direction that hurts. It is
computed from the measured widths instead.

#69 stays its own PR on a finding rather than an estimate: page-header
cannot collapse actions that arrive as arbitrary light-DOM markup
through a slot, so the fix needs an actions API across all three hosts.

A very small window becomes the phone layout, which already exists and
is already tested, rather than the mini-player: #12 is a second
always-on-top window, and making it a mode of the main window would
discard navigation state on a resize and put the process-level MPRIS
question on a path a drag can trigger.
2026-08-19 10:54:52 -04:00
logan 3607fe445e Merge pull request 'Fix the player states that report one track's progress against another' (#129) from fix/player-playing-state into main
CI / check (push) Successful in 2m30s
CI / e2e (push) Successful in 6m14s
2026-08-19 14:53:46 +00:00
yonlu 61d549a9d5 ci: run the pre-push hooks sequentially
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / e2e (pull_request) Successful in 6m23s
CI / check (pull_request) Successful in 2m30s
`go test -race ./...` saturates every core for ~47s, and the UI tier
it was sharing them with is a real Chromium with wall-clock timeouts.
So the browser lost, at random: setup took 106s inside the hook
against 63s standalone, and a different suite failed on each run --
three failing to fetch setup.ts from Vitest's own dev server once, a
15s "did not mount itself" the next time -- against a suite that
passes 898/898 five times running on its own.

That reads as "your branch broke the frontend" when nothing is wrong,
which is the most expensive kind of false negative: the next person
bisects a change that was never at fault. It cost two pushes here
before the summary line gave it away.

Sequential costs about 15s.

Closes #128
2026-08-19 09:16:43 -04:00
yonlu 2b84bc53e9 fix(player): stop reporting one track's state against another
Five faults found while auditing the play/pause and position path for
a desktop report of the pause icon showing over a seek bar that was
not moving. They are one commit because they are one file's worth of
tangled state, and two of them do not compile apart.

The finished callback did not know which chain it came from. It is
dispatched as a goroutine from the beep callback and then queues for
p.mu, so a user pressing Next in the last second of a track had it
wake up holding the lock for a player that had loaded something else
-- and rewind it, stop it, and hand a stale finish to the queue's
auto-advance. updateStreamers now stamps a chainID and the callback
carries the one it was registered with. (#123)

It also emitted PlaybackFinished and PlaybackStateChanged(stopped)
*after* releasing p.mu, alone in this file, so a Play() taking the
lock in that gap emitted `playing` first and the stale `stopped`
landed last -- the button showing play over a track that was audibly
running. Both emits are back under the lock. (#123)

A source that failed mid-track was reported to the queue as a natural
end, so a broken file auto-advanced in silence and was counted as
played. The handler takes the reason now: the player cannot name the
track, because the metadata is the queue's, so the queue emits
PlaybackFailed and skips recording the play. (#123)

p.format was assigned once, in the constructor, to the *speaker's*
rate, and never again -- so it claimed 44.1 kHz for every file. The
replay-after-finish path resamples from it, meaning a finished track
played a second time was resampled from a rate the decoder never
produced: audibly wrong speed and pitch, and the length and position
fallbacks wrong with it. The fixtures are 22050 Hz, which is what lets
a test see this at all. (#124)

p.trackLengthMs was written only when the database had a row and
cleared only by UnloadTrack, so a file with no row inherited the
previous track's duration -- and every position report is scaled by
it, so the bar reported one track's progress on another's scale.
(#125)

Queue.OnPlaybackFinished indexed q.tracks[currentIndex] having checked
only that the queue was non-empty. currentIndex is -1 whenever the
queue has been exhausted, and onQueueExhausted deliberately leaves the
finished track loaded -- so playing it from there and letting it end
panicked, on a goroutine with no caller to recover it. (#126)

The position readers guarded the decoder with the speaker lock, which
the read-ahead goroutine has no reason to hold and never takes -- so
Position() raced readAhead's Stream() on every position emit, once a
second for the whole of playback. srcMu is the lock that excludes that
goroutine, and taking it naively deadlocks, because seekLocked already
holds it and then emits the landing position from inside that region.
seekSourceLocked is that region extracted, so the lock is released
before anything is emitted. Found by the race detector, via the test
added here for the chain guard: the existing suite never loads a file
outside the integration guard, so make test was green over it. (#127)

OnPlaybackFinished picks up //wails:ignore along with its error
parameter: v3's generator segfaults on a bound method taking an error,
and this was never IPC. That removes a binding the frontend could have
called to force an auto-advance.

Closes #123
Closes #124
Closes #125
Closes #126
Closes #127
2026-08-19 09:06:43 -04:00
yonlu 282dab43eb fix(player): end the stream when the audio source stops producing
BufferedStreamer.Stream treated an empty ring buffer as a momentary
underrun and answered with silence and ok. That is right while the
read-ahead is still going to deliver something, and two of its three
exit paths left it never going to: a Close, and a source returning
(0, true) in a loop. Neither set done, so the ring drained and every
call after it was silence claiming to be audio, for the life of the
process.

Nothing above this type could tell that from healthy playback. The
beep.Seq chain never ended, so the player stayed in Playing with the
button showing pause; the decoder's position never moved, so the 1 Hz
report pinned the seek bar at a constant -- and since every report
resets the bar's interpolation, the report actively suppressed the one
thing that would still have moved it. A frozen bar over a track that
was not playing, with no watchdog anywhere to notice.

Every exit now marks the stream done, and the silence fill is bounded
by a duration *and* a run of calls. It needs both. Wall clock is the
real measure, because the speaker paces itself and a stall is a
question about time -- but a caller draining in a tight loop makes
hundreds of calls in microseconds and would outrun a duration alone.
A call count alone is the opposite failure, and not a hypothetical
one: the first attempt used one and spent the whole budget before the
read-ahead goroutine had been scheduled once, ending a perfectly good
stream at sample zero and breaking TestBufferedStreamer_BasicStream.

Err is plumbed out at the same time, because a drained source and a
failed one both arrive as (0, false) and are not the same event.
Reading it is a separate change; without it there is nothing to read.

Closes #122
2026-08-19 09:05:55 -04:00
21 changed files with 2002 additions and 84 deletions
+84
View File
@@ -3580,3 +3580,87 @@ per-card flag can answer at all.
The general point: **two columns that agree today are not one column.**
Which of them a new surface reads should be decided by which one has
something that can un-set it.
## The queue panel was a column that could not afford to be one (measured 2026-08-19)
Plan 018, issue #24. Measured against the running app (`make
dev-headless SEED=default`, Chromium) on Playlists, sweeping the
viewport with the queue open and closed. Main panel width, and how much
of the page header survived:
| viewport | sidebar | main (queue open) | actions clipped |
|---|---|---|---|
| 1280×800 | 200 | 759 | — |
| 1000×700 | 200 | 479 | 2 of 3 |
| **900×600** | 200 | **379** | all three |
| 800×600 | 56 | 423 | all three |
| 390×780 | — | **69** | all three |
| 320×600 | — | **0** | all three |
| 800×600 | 56 | 744 *(closed)* | New Smart Playlist, 158/162px |
Five things came out of it that the issue did not say.
- **The header clips at the enforced minimum with the queue closed.**
800×600 is the only size this app promises, and "New Smart Playlist"
loses 4px of its 162 there. The queue makes it dramatic; it is not
the cause.
- **900×600 is worse than 800×600.** `AUTO_COLLAPSE_VIEWPORT` collapses
the sidebar *below* 900, so the main panel is 843px at 899 and 700px
at 900. **The worst desktop case is the top of the Compact band, not
the enforced floor** — so every viewport list that stopped at "the
minimum" was missing its own worst case. `layout-overflow.spec.ts`
carries 900 now.
- **At 320px the main panel was 0px.** The panel is `flex-shrink: 0` in
the flow of `.content-area`, so an open queue is paid for by the
content rather than covering it. Not degraded — gone. That is the
measurement #55 wanted and did not have.
- **Only Playlists overflows.** All ten primary views swept at 900×600
and 390×780; every other header reports `scrollWidth ==
clientWidth`, and Albums at 390 renders title, count and sort legibly
(checked on a screenshot, not just the number). So #69 is one view's
action set — three text buttons totalling 390px — and not a systemic
header failure.
- **Both reasons in `MinWidth`'s comment had expired.** The subtitle is
`display: none` from 899 down, and the sidebar host is
`overflow-y: auto` (at 600×460, `scrollHeight` 434 against a 332px
client, Settings reachable after scrolling). The floor is right; its
stated defence was two mechanisms that can no longer happen, which is
worse than either answer because nobody can argue with it.
**A correction worth keeping, because it nearly went in the plan.** My
first probe for the sidebar's scroller searched
`shadowRoot.querySelectorAll('*')` and reported "no scroller — items
are unreachable", which reads exactly like a live Settings-unreachable
bug. The scroller is the **host**, and a host is not inside its own
shadow root. CLAUDE.md was right and the probe was wrong.
**And one claim in the plan's first draft was too strong**: that the
overlay "removes the desktop half of #69". After phase 2, at 900×600,
open and closed are now *identical* (main 700, one action clipped)
where open used to be main 379 with all three clipped. The queue's
contribution is gone; the header's own overflow remains and is still a
live defect at a supported size.
### The mode cannot be a media query
The panel is drag-resizable 200500px and persisted, so a viewport
breakpoint assumes the default 320 and is wrong by up to 180px for a
user who widened it — in the direction that hurts, since a wider queue
is exactly when the content can least afford it. It is computed from
`.content-area`'s width instead (which already accounts for the
sidebar's collapse), and the component test that matters widens the
panel at a *fixed* parent width and asserts the flip.
The floor (480) is a judgement, and the measurement is why: there is no
cliff. The track list rescales its columns continuously — 213px down to
124px between main widths of 900 and 544, `rowOverflow=0` at every step
— and the album grid steps 3 columns to 2 somewhere between 564 and 644
without breaking. So 480 is anchored at both ends instead: it keeps the
default 1100px window inline, and puts every measured-broken case on
the overlay side.
The scrim is perceptible but subtle on a dark ramp, which is worth
knowing before someone "fixes" it as broken: sampled from screenshots at
900×600, the main panel's background goes 33,37,41 → 18,20,23 and a
row's text 242 → 133. It covers the content area only — not the sidebar
or the transport — because the queue is not modal.
@@ -0,0 +1,294 @@
# 018 — Supported sizes, and what the queue panel is
**Issue:** #24 (`Area/Shell-Nav`, `Priority/High`, `Reviewed/Confirmed`)
**Unblocks:** #55 (queue as a screen) — a real Gitea dependency
**Relates:** #69 (page-header overflow), #12 (mini-player), #51 (small-screen umbrella)
**Status:** in flight
#73 puts this first in Phase 2 and hangs the rest of the phase off it,
so the decision has to be written down and arguable before any CSS
moves. This document is the decision. Everything below the matrix is
either a measurement or an argument for one of the four choices #24
asks for.
---
## What is actually wrong, measured
Against the running app (`make dev-headless SEED=default`, Chromium),
Playlists, sweeping the viewport with the queue open and closed. The
number that matters is how much of the page header survives.
| viewport | sidebar | queue | main panel | header needs | actions clipped |
|---|---|---|---|---|---|
| 1280×800 | 200 | open 321 | 759 | 759 | — |
| 1000×700 | 200 | open 321 | 479 | 747 | New Playlist, New Smart Playlist |
| **900×600** | 200 | open 321 | **379** | 747 | **all three** |
| 800×600 | 56 | open 321 | 423 | 747 | all three |
| 700×600 | 56 | open 321 | 323 | 747 | all three |
| 390×780 | — | open 321 | **69** | 747 | all three |
| 320×600 | — | open 321 | **0** | 747 | all three |
| 900×600 | 200 | closed | 700 | 747 | New Smart Playlist |
| **800×600** | 56 | closed | 744 | 747 | **New Smart Playlist (158/162px)** |
| 320×600 | — | closed | 320 | 747 | all three |
Five things in that table are not in the issue.
**The header clips at the supported minimum with the queue closed.**
At 800×600 — the size `backend/config/window.go` enforces and the only
size this app *promises* — "New Smart Playlist" loses 4px of its 162.
#24 reads as a queue-panel bug; the queue makes it dramatic, but the
header overflows on its own at the minimum window.
**900×600 is worse than 800×600, because the sidebar expands at 900.**
`AUTO_COLLAPSE_VIEWPORT` collapses the sidebar to icons *below* 900, so
at 899px the main panel is 843px and at 900px it is 700px. The worst
desktop case is therefore not the minimum window; it is the pixel
immediately above the collapse. Anything that tests "the minimum" and
stops has not tested the worst case, which is what
`layout-overflow.spec.ts` does today.
**At phone widths the queue is not a drawer, it is an amputation.**
`queue-panel`'s host is `flex-shrink: 0; width: 0`, going to
`width: var(--queue-width, 320px)` under `[open]` — it is *in the flow*
of `.content-area`, so it takes its width from the main panel rather
than covering it. At 390px that leaves 69px of the page; at 320px it
leaves **0px**, and the app is not degraded but gone. This is the
measurement #55 needs and did not have.
**Only Playlists overflows.** Sweeping all ten primary views at 900×600
and at 390×780, every other header reports `scrollWidth ==
clientWidth`, and Albums at 390px renders title, count and sort
legibly (checked on a screenshot, not just the number). #69 is
therefore one view's action set — three text buttons totalling 390px —
and not a systemic header failure, though the *rule* still belongs in
`page-header`.
**Both reasons in `MinWidth`'s comment are stale.** It says the floor is
800×600 because "below ~780 the header's subtitle wraps" and "below
~600 tall the eleven sidebar items no longer fit". The subtitle is
`display: none` below 900 (index.css), and the sidebar host is
`overflow-y: auto` — at 600×460 its `scrollHeight` is 434 against a
332px client, and Settings is reachable after scrolling. Neither
mechanism can happen any more. That does not mean the floor should
move; it means its stated reason no longer supports it, which is worse
than either answer.
*(Care needed: my first probe for the sidebar scroller searched
`shadowRoot.querySelectorAll('*')` and reported "items are
unreachable", because the scroller is the **host** and a host is not in
its own shadow root. The claim in CLAUDE.md is correct.)*
---
## Decision 1 — the supported size matrix
Three bands. Two of them already exist and are already argued; what is
new is that they are written down as a *promise*, and that the queue is
part of it.
| band | width | navigation | queue | promise |
|---|---|---|---|---|
| **Phone** | < 600 | `bottom-nav` + drawer | overlay, full width | reflows; nothing needs sideways scrolling; fits 320px |
| **Compact** | 600 899 | icon sidebar | overlay + scrim | nothing is clipped or unreachable at any width in the band |
| **Desktop** | ≥ 900 | labelled sidebar | inline where it fits (see decision 2), else overlay | as Compact |
And one promise across all three: **no action is ever unreachable.**
That is the sentence #69 asks for and it is the one the matrix exists
to make checkable.
**400% zoom** keeps the meaning it already has: WCAG 1.4.10 names 320px
as the reflow target, the phone band covers it, and
`layout-overflow.spec.ts` already asserts a 320px viewport needs no
sideways scrolling. What changes is that the *queue* must be part of
that assertion — it is not today, and with the queue open at 320px the
main panel is 0px wide, which no current test can see.
**The window minimum stays 800×600**, and its comment gets the real
reason. The old mechanisms are gone, but the floor is still where the
Compact band's chrome stops being comfortable, and lowering it would
mean promising the desktop layout at sizes where only the phone layout
works. The interesting consequence is decision 4.
---
## Decision 2 — the queue is an overlay when it cannot afford to be a column
**The rule.** The queue panel renders inline — in the flow, as today —
only while
```
viewport sidebar queueWidth ≥ 480
```
and as an overlay with a scrim otherwise.
**Why it cannot be a media query**, which is the load-bearing half:
the queue's width is *user state*. It is drag-resizable between 200 and
500px and persisted (`--queue-width`, `MIN_WIDTH`/`MAX_WIDTH` in
`queue-panel.ts`). A breakpoint at a fixed viewport width silently
assumes the default 320, and is wrong by 180px for a user who has
dragged the panel wide — in the direction that hurts, since a wider
queue is exactly when the content can least afford it. So the mode is
computed from the measured widths and published as an attribute, the
way `data-active-view` already is, and the CSS keys off that.
**Why 480, honestly.** There is no cliff to derive it from. The track
list rescales its columns continuously — at main widths from 900 down
to 544 its `--grid-cols` shrink from 213px to 124px with
`rowOverflow=0` throughout — and the album grid steps 3 columns to 2
somewhere between 564 and 644 without breaking. So this is a judgement,
anchored on two things: it keeps the *default* window (1100 wide, main
= 580) inline, because the inline queue is a desktop affordance people
choose and turning it into an overlay for the common case would be a
regression in feel; and it puts every case measured as broken —
900×600 at main=379, and every phone width — on the overlay side.
1024×768 lands at main=504 and stays inline.
**The scrim is the other half of the issue's complaint** ("make the
queue obviously an overlay *over* the content so it reads as something
to close"). An overlay queue gets a scrim, closes on scrim click and on
Escape, and returns focus to `#queue-button`.
**What must not change**: #55's Direction is explicit — one component,
two mount points, do not fork it. The overlay is a *presentation* of
the same `queue-panel`, so the roving tab stop, Alt+Arrow reorder, drag
reorder, selection semantics and the `virtualizer.requestUpdate()` on
selection and current-track change all come along untouched. This
decision deliberately stops short of #55's detail-view mount, but it is
the shape that makes it possible, and it unblocks it.
---
## Decision 3 — #69 is its own PR, and here is the finding that decides it
`page-header` **cannot collapse its own actions**, and that is not an
effort estimate but a fact about the API. Actions arrive through
`<slot name="actions">` as arbitrary light-DOM markup — Playlists slots
a `<div class="header-actions">` of three `<button>`s with click
handlers, drag handlers and a conditional class. A component cannot
move another component's light-DOM children into a dropdown and keep
their behaviour; there is nothing generic to render as a menu item.
So the overflow rule needs an *actions API* — hosts declaring
`{icon, label, handler, priority}` data that `page-header` can render
either as buttons or as menu items — which is a change to all three
hosts that slot actions, not a rule added in one place. That is a
different piece of work from this one, it is independently verifiable,
and the desktop half of #69's symptom is removed by decision 2 anyway
(the queue stops eating the header's width).
It therefore stays #69, gets the finding above recorded on it, and
follows immediately after this. What *this* plan owes it is the
promise in the matrix — no action unreachable at any supported size —
and the measurement that the only offender today is Playlists.
**And the promise is not kept yet, which is the honest version of a
claim this document made in its first draft.** "Decision 2 removes the
desktop half of #69's symptom" was too strong. Measured after phase 2,
at 900×600 on Playlists:
| | before | after |
|---|---|---|
| queue open | main 379px, **all three** actions clipped | main 700px, **one** clipped |
| queue closed | main 700px, one clipped | unchanged |
So the queue's *contribution* is gone — open and closed are now
identical, which is the whole of what this decision owed — and the
residual "New Smart Playlist: 114/162px" is the header overflowing on
its own, at a size the queue never touched. #69 is still a live defect
at a supported size, and the matrix's promise is what will close it.
---
## Decision 4 — a very small window becomes the phone layout, not the mini-player
#24 asks whether a very small window should switch to the mini-player
(#12) "or simply refuse to go there". Both options in the question are
worse than the one the codebase already has.
**#12 is a second window, not a mode.** Its findings say so: v3
supports multiple windows, `AlwaysOnTop` is a window *option*, and the
frontend would need an entry branch mounting only the mini-player root
for a second window loading the same bundle. Turning the main window
into a mini-player at some width conflates the two: it would throw away
the user's navigation state on a resize, and it puts the MPRIS question
(#12's own open question — media controls are process-level and must
not be per-window) on a code path that a drag can trigger by accident.
**And "refuses" is unnecessary, because the reflow already exists.**
The phone band is real, tested, and reached by width alone — a desktop
window narrowed below 600px already gets `bottom-nav` and the phone
shell. That is a better answer than refusing: it is strictly more
usable than a hard minimum, it costs nothing new, and it is the same
code Android runs, so it stays exercised.
So: the main window reflows and never becomes a mini-player; #12 stays
a separate always-on-top window and is not blocked by, or coupled to,
this decision. The window minimum stays 800×600 for the reason in
decision 1 — but the phone band is what happens below it, not a
refusal, which is why the minimum is a comfort floor rather than a
correctness one.
---
## Phases
1. **This document**, linked from #24, with the matrix reported on the
issue and #55 told whether it is unblocked. *(no code)***done**
2. **The queue's overlay mode** — computed mode attribute, scrim,
Escape and scrim-click close, focus return. The inline path is
unchanged above the threshold. — **done**
3. **The window minimum's comment** — replace both stale reasons with
the measured ones. No value change. — **done**
4. **Verification**, below. Including the specs that must change
because they assert the old behaviour. — **done**
#69 follows as its own branch; #55 became unblocked at phase 2.
## What landed, measured
Main panel width with the queue open, before and after:
| viewport | before | after | mode |
|---|---|---|---|
| 1280×800 | 759 | 759 | inline |
| 1100×720 (default window) | 579 | 579 | inline |
| 1024×768 | 503 | 503 | inline |
| 900×600 | **379** | **700** | overlay |
| 800×600 | 423 | 744 | overlay |
| 390×780 | **69** | **390** | overlay |
| 320×600 | **0** | **320** | overlay |
The scrim is perceptible but subtle on a dark ramp, which is worth
knowing before someone "fixes" it: sampled from the screenshots at
900×600, the main panel's background goes 33,37,41 → 18,20,23 and a
row's text 242 → 133. It covers the **content area only** — not the
sidebar or the transport — on purpose: the queue is not modal, and
leaving the navigation live means the scrim reads as "this is over the
content" (which is what #24 asked for) without pretending the rest of
the app is unavailable.
## Verification, and what each tier cannot see
- `make ui-test` — the queue panel's mode logic is component-tier
work and belongs there. It **cannot** see the shell: the threshold is
computed from the sidebar and viewport, which do not exist in that
tier.
- `make e2e``layout-overflow.spec.ts` gains the queue-open case at
every band (it has none today, which is why main=0px at 320px has
never failed anything) and **gains 900×600**, since the minimum is
not the worst case. `queue-toggle-state.spec.ts` and
`phone-shell.spec.ts` both touch the panel and must be re-read before
editing.
- **Screenshots at every band, read by a human.** This is not optional
here: `layout-overflow.spec.ts` asserts the *shell* needs no sideways
scrolling and passes on a build whose album header clips its own
buttons (measured this session at 390px; filed on #66). Clipping
*inside* a component is invisible to it, and clipping is this issue.
- `make ui-visual` **cannot help at all** — the component tier renders
the token fallbacks, because the theme only reaches `:root` in the
real app.
- Accessible names via `page.getByRole(...)`, never a shadow-root
query. A drawer with a scrim is exactly the shape that grows a
nameless control, and this repo has shipped one three times.
+79
View File
@@ -1328,6 +1328,68 @@ is 32px each. Which four is plan 016's committed subset, and everything
else — Settings included, because a phone still needs it — is behind
"More".
**There are three supported size bands, and the queue is part of the
promise.** Plan 018 (#24) wrote them down: **Phone** below 600 (bottom
nav, reflows, fits 320px exactly), **Compact** 600899 (icon sidebar),
**Desktop** from 900 (labelled sidebar) — plus one sentence across all
three, *no action is ever unreachable at any supported size*. The bands
themselves already existed; what was new is that they are a promise and
that the queue panel is inside it.
**900 is the worst desktop width, not the 800×600 minimum.** The
sidebar collapses to icons *below* 900, so the main panel is 843px at
899 and 700px at 900 — the narrowest content area any desktop width
produces is at the top of the Compact band, not at the enforced floor.
Every viewport list that stopped at "the minimum" was therefore missing
its own worst case, which is why `layout-overflow.spec.ts` carries 900
now. And **both reasons in `MinWidth`'s comment had expired** — the
subtitle is `display: none` from 899 down and the sidebar host scrolls
(`overflow-y: auto`; at 600×460 its `scrollHeight` is 434 against a
332px client) — so 800×600 is a *comfort* floor for desktop chrome and
not a correctness one. Below it the phone layout takes over, which is
also why a very small window reflows rather than becoming a
mini-player: **#12 is a second always-on-top window, not a mode of this
one**, and making it a mode would discard navigation state on a resize
and put the process-level MPRIS question on a path a drag can trigger.
**The queue panel is a column only while the content can spare the
width, and that cannot be a media query.** In flow the host is
`flex-shrink: 0`, so an open queue is paid for by the main panel: it
left 379px at 900×600 (with all three of the Playlists header's actions
clipped), 69px at 390, and **0px** at 320 — the content was not
degraded but gone. It goes to an overlay with a scrim when
`available - panelWidth < 480`, where `available` is
`.content-area`'s width and therefore already accounts for the
sidebar's collapse.
Four things about it are load-bearing. **The mode is computed, not
breakpointed**, because the panel's width is user state — drag-resizable
200500px and persisted — so a viewport breakpoint silently assumes the
default 320 and is wrong by up to 180px in the direction that hurts;
widening the panel at a fixed window size must flip it, and
`queue-overlay-mode.test.ts` is written around exactly that. **480 is a
judgement and says so**: there is no cliff to derive it from (the track
list rescales continuously, 213px to 124px columns with no row
overflow), so it is anchored to keep the default 1100px window inline
and put every measured-broken case on the overlay side. **The scrim
covers the content area only** — not the sidebar or the transport —
because the queue is not modal, and it is subtle on a dark ramp by
arithmetic rather than by accident (33,37,41 → 18,20,23). And **the
overlay is a presentation, not a fork**: #55 asks for one component
with two mount points, so the roving tab stop, Alt+Arrow reorder, drag
reorder, selection semantics and `virtualizer.requestUpdate()` all come
along untouched. Escape closes it and returns focus, and is attached
only while the overlay is up — it is a dismissal, not a shortcut, which
is why it is not a panel-scoped binding.
What this does **not** fix is `page-header` overflowing on its own:
at 900×600 "New Smart Playlist" is still clipped to 114 of 162px with
the queue *closed*. That is #69, and it cannot be fixed in
`page-header` alone — actions arrive through `<slot name="actions">` as
arbitrary light-DOM markup with their own handlers, so collapsing them
into a "More actions" menu needs an actions *API* (data, not markup)
across all three hosts that slot them.
**The phone section of `index.css` is last on purpose.** A media query
adds no specificity, so a `@media (max-width: 599px)` block placed
above the plain rules it overrides loses to them — which is how phase 1
@@ -2307,6 +2369,23 @@ Pre-commit hooks verify generated code is fresh — always run `make generate` a
mistyped `feat` ships a minor version. `make release-dry` answers "what
would this merge release" without pushing.
**The analyzer reads the type and ignores the scope, so a CI-only change
is `ci:` and never `fix(ci):`.** The scope is decoration; `fix` is a
patch whatever is in the brackets. Two commits touching nothing but
`.gitea/workflows/unclaim.yml` were written `fix(ci):` and cut `v0.2.1`
and `v0.2.2` — real releases, published to Arch, Homebrew and the APK
registry, containing no user-facing change. They were left in place
rather than deleted, because a version that vanishes is worse for
whoever pulled it than one that turns out to be empty.
**The blast radius is bigger than the version number**, which is what
makes this worth a paragraph. A merge to `main` starts two workflows;
if `release.yml` then pushes a tag, that tag push starts **four more**
(`arch-package`, `homebrew-formula`, `android-apk`, `desktop-assets`) —
on a runner with capacity 1, where the APK build alone is tens of
minutes. `make release-dry` before merging is how you find out, and it
is cheaper than every one of those.
**`@semantic-release/github` is not in that config and must not be.**
Gitea's API is `/api/v1` and is not GitHub's surface, so
`@semantic-release/exec` calls `scripts/gitea-release.sh` instead — one
+25 -7
View File
@@ -13,13 +13,31 @@ const (
// enforces this at runtime; it is also the floor below which a
// reported size is treated as bogus and not persisted.
//
// 800x600 is where the shell was measured to still work, rather
// than a round number: below ~780 the header's subtitle wraps and
// pushes the title out of the 4em top bar, and below ~600 tall the
// eleven sidebar items no longer fit at once. The previous
// 512x384 was aspirational — at 700x480 the sidebar overflowed
// behind the player bar with no scroll and Settings and Jobs could
// not be reached at all.
// **Both reasons this comment used to give have expired**, and the
// value is right for a third one. It said the floor was 800x600
// because "below ~780 the header's subtitle wraps and pushes the
// title out of the 4em top bar" and "below ~600 tall the eleven
// sidebar items no longer fit at once". Neither mechanism can
// happen now: the subtitle is display:none from 899px down
// (index.css), and the sidebar host is overflow-y:auto — measured
// at 600x460, its scrollHeight is 434 against a 332px client and
// Settings is reachable after scrolling. A floor defended by two
// mechanisms that no longer exist is a number nobody can argue
// with, which is worse than either answer.
//
// It stays 800x600 because that is where the *desktop* chrome
// stops being comfortable — the Compact band of plan 018's size
// matrix (#24) — and not because the app breaks below it. It does
// not: under 600px wide the phone layout takes over (bottom-nav,
// no sidebar) and the shell fits 320px exactly, which is what
// makes this a comfort floor rather than a correctness one, and
// why a very small window reflows instead of becoming a
// mini-player (#12 is a second always-on-top window, not a mode of
// this one).
//
// The previous 512x384 was aspirational — at 700x480 the sidebar
// overflowed behind the player bar with no scroll and Settings and
// Jobs could not be reached at all.
MinWidth = 800
// MinHeight is the smallest allowed window height in pixels.
MinHeight = 600
+188
View File
@@ -0,0 +1,188 @@
package player
import (
"errors"
"testing"
"time"
"github.com/gopxl/beep/v2"
)
// errTestDecode stands in for a decoder blowing up mid-track.
var errTestDecode = errors.New("decode blew up")
// stalledStreamer never produces a sample and never reports
// end-of-stream: (0, true), forever. A damaged file that decodes to
// nothing looks like this, and so does any source whose producer has
// quietly stopped.
type stalledStreamer struct{}
func (stalledStreamer) Stream(_ [][2]float64) (int, bool) { return 0, true }
func (stalledStreamer) Err() error { return nil }
// failingStreamer produces n good samples and then fails, which is
// what a decode error mid-track looks like: the same (0, false) a
// finished track returns, distinguishable only by Err.
type failingStreamer struct {
remaining int
err error
}
func (f *failingStreamer) Stream(samples [][2]float64) (int, bool) {
if f.remaining <= 0 {
return 0, false
}
n := min(len(samples), f.remaining)
for i := range n {
samples[i] = [2]float64{1, 1}
}
f.remaining -= n
return n, true
}
func (f *failingStreamer) Err() error { return f.err }
// drainUntilEnd calls Stream until it reports end-of-stream, or gives
// up. It returns whether the stream ended.
//
// The give-up bound is wall clock rather than a call count: the stall
// budget is a duration, so a tight loop has to actually wait it out.
func drainUntilEnd(bs *BufferedStreamer, within time.Duration) bool {
buf := make([][2]float64, 512)
deadline := time.Now().Add(within)
for time.Now().Before(deadline) {
if _, ok := bs.Stream(buf); !ok {
return true
}
time.Sleep(time.Millisecond)
}
return false
}
// A source that stops producing without ever ending is the fault this
// whole file exists for: Stream used to answer with silence and ok
// forever, so the chain never ended, the player stayed in Playing
// with the button showing pause, and the decoder's position never
// moved -- a frozen seek bar over a track that was not playing.
func TestAStalledSourceEndsTheStream(t *testing.T) {
bs := NewBufferedStreamer(stalledStreamer{}, 2048)
defer bs.Close()
if !drainUntilEnd(bs, maxStarvedDuration+2*time.Second) {
t.Fatal(
"a stalled source never ended the stream: the player " +
"would sit in Playing with a frozen position",
)
}
if !errors.Is(bs.Err(), errSourceStalled) {
t.Fatalf(
"expected the stall to be reported, got %v", bs.Err(),
)
}
}
// Close is the other exit that used to leave `done` false, with the
// same consequence: the ring drains and every call after it is
// silence that claims to be audio.
func TestClosingEndsTheStream(t *testing.T) {
bs := NewBufferedStreamer(finiteStreamer(1<<20), 2048)
// Let the read-ahead fill something, so this exercises the drain
// after Close rather than a buffer that was empty anyway.
time.Sleep(20 * time.Millisecond)
bs.Close()
if !drainUntilEnd(bs, 2*time.Second) {
t.Fatal("a closed streamer never reported end-of-stream")
}
}
// A source that fails is not a source that finished, and only Err
// tells them apart. Before this, the player reported a mid-track
// decode failure to the queue as a natural end, so the queue
// auto-advanced in silence and counted the broken track as played.
func TestAFailedSourceReportsItsError(t *testing.T) {
src := &failingStreamer{remaining: 4096, err: errTestDecode}
bs := NewBufferedStreamer(src, 2048)
defer bs.Close()
if !drainUntilEnd(bs, 2*time.Second) {
t.Fatal("a failing source never reported end-of-stream")
}
if !errors.Is(bs.Err(), errTestDecode) {
t.Fatalf(
"expected the source's error to survive, got %v",
bs.Err(),
)
}
}
// The ordinary case has to keep working: a source that ends cleanly
// ends with no error, or every finished track would be reported as a
// failure and skipped.
func TestADrainedSourceReportsNoError(t *testing.T) {
bs := NewBufferedStreamer(finiteStreamer(4096), 2048)
defer bs.Close()
if !drainUntilEnd(bs, 2*time.Second) {
t.Fatal("a finite source never reported end-of-stream")
}
if bs.Err() != nil {
t.Fatalf(
"a track that finished normally reported %v", bs.Err(),
)
}
}
// A slow source is exactly what the read-ahead exists to absorb, so
// underruns must not be charged cumulatively -- otherwise a file on a
// slow disk ends itself partway through.
func TestUnderrunsDoNotAccumulateAcrossASlowSource(t *testing.T) {
const total = 8192
src := &slowStreamer{
inner: finiteStreamer(total),
delay: 2 * time.Millisecond,
}
bs := NewBufferedStreamer(src, 1024)
defer bs.Close()
buf := make([][2]float64, 256)
got := 0
for {
n, ok := bs.Stream(buf)
if !ok {
break
}
for i := range n {
if buf[i][0] != 0 {
got++
}
}
}
if got != total {
t.Fatalf(
"a slow but healthy source was cut short: got %d of %d "+
"samples",
got, total,
)
}
}
// beep.Streamer is what the player wraps; keep the type honest.
var _ beep.Streamer = (*BufferedStreamer)(nil)
+113 -9
View File
@@ -1,6 +1,7 @@
package player
import (
"errors"
"sync"
"time"
@@ -34,8 +35,45 @@ type BufferedStreamer struct {
done bool
err error
closed chan struct{}
// starved counts consecutive Stream calls served with silence
// because the ring was empty, and starvedSince is when that run
// began. An underrun is legitimate for a moment -- that is what
// the read-ahead exists to absorb -- but it is not legitimate
// forever, and "forever" is indistinguishable from healthy
// playback everywhere above this type: the chain never ends, so
// the player stays in Playing with the button showing pause, and
// the decoder's position never moves, so the 1 Hz report pins the
// seek bar and suppresses its interpolation.
starved int
starvedSince time.Time
}
// The silence fill is bounded by both a duration and a run of calls,
// and it needs both.
//
// Duration alone is the real measure -- the speaker paces itself, so
// wall clock is what says whether the source has actually stopped --
// but a caller draining in a tight loop (a test, a decode-to-buffer)
// makes hundreds of calls in microseconds and would trip nothing.
// A call count alone is the opposite failure: the same tight loop
// spends the whole budget before the read-ahead goroutine has been
// scheduled once, and ends a perfectly good stream at sample zero.
//
// The duration is longer than the 2 s read-ahead it is there to
// outlast, and the count is short enough that the speaker (~200 ms a
// call) reaches it well inside that.
const (
maxStarvedDuration = 3 * time.Second
minStarvedCalls = 8
)
// errSourceStalled is returned by Err when the source stopped
// producing samples without ever reporting end-of-stream.
var errSourceStalled = errors.New(
"audio source stopped producing samples",
)
// NewBufferedStreamer creates a BufferedStreamer that pre-fills
// bufferSize samples from source via a background goroutine.
// A typical bufferSize is 2× the sample rate (~2 seconds of audio).
@@ -54,8 +92,24 @@ func NewBufferedStreamer(
return bs
}
// finish marks the stream ended, recording err as the reason when
// there is one. Every exit from readAhead goes through it: an exit
// that leaves done false strands Stream in its underrun branch,
// where it returns silence and ok forever.
func (bs *BufferedStreamer) finish(err error) {
bs.mu.Lock()
defer bs.mu.Unlock()
bs.done = true
if err != nil && bs.err == nil {
bs.err = err
}
}
// readAhead continuously reads from the source into the ring buffer
// until the source is drained, an error occurs, or Close is called.
// It always marks the stream done on the way out.
func (bs *BufferedStreamer) readAhead() {
// Temporary buffer for reading from source outside the lock.
// 512 samples per chunk keeps the critical section short.
@@ -63,6 +117,13 @@ func (bs *BufferedStreamer) readAhead() {
tmp := make([][2]float64, chunkSize)
// Every exit marks the stream done. An exit that does not is what
// stranded Stream in its underrun branch, returning silence and ok
// for the rest of the process's life.
var exitErr error
defer func() { bs.finish(exitErr) }()
for {
// Check if closed.
select {
@@ -72,6 +133,15 @@ func (bs *BufferedStreamer) readAhead() {
}
bs.mu.Lock()
// Stream gave up waiting for us. Nothing downstream is
// listening any more, so filling the ring is work for nobody.
if bs.done {
bs.mu.Unlock()
return
}
space := len(bs.ring) - bs.count
if space == 0 {
@@ -115,14 +185,12 @@ func (bs *BufferedStreamer) readAhead() {
}
if !ok {
bs.mu.Lock()
bs.done = true
if srcErr := bs.source.Err(); srcErr != nil {
bs.err = srcErr
}
bs.mu.Unlock()
// A drained source and a failed one both land here and are
// not the same event: one is a track that ended, the other
// is a track that broke. Err is what tells them apart, and
// it is why the player must ask before treating this as a
// natural finish.
exitErr = bs.source.Err()
return
}
@@ -154,7 +222,27 @@ func (bs *BufferedStreamer) Stream(
}
if bs.count == 0 {
// Buffer temporarily empty — fill with silence.
// The read-ahead has not caught up. Silence buys it time --
// but only for a bounded stretch, because "forever" is
// reported upward as healthy playback and there is no watchdog
// above this to notice otherwise.
bs.starved++
if bs.starvedSince.IsZero() {
bs.starvedSince = time.Now()
}
if bs.starved >= minStarvedCalls &&
time.Since(bs.starvedSince) > maxStarvedDuration {
bs.done = true
if bs.err == nil {
bs.err = errSourceStalled
}
return 0, false
}
for i := range samples {
samples[i] = [2]float64{}
}
@@ -162,6 +250,9 @@ func (bs *BufferedStreamer) Stream(
return len(samples), true
}
// Samples arrived, so whatever the stall was, it is over.
bs.resetStarvationLocked()
// Copy available samples from ring buffer.
n := len(samples)
if n > bs.count {
@@ -197,6 +288,19 @@ func (bs *BufferedStreamer) Flush() {
bs.readPos = 0
bs.writPos = 0
bs.count = 0
// A seek empties the ring on purpose, and the refill that follows
// is exactly the stall the budget exists to tolerate. Charging it
// against a budget the previous underrun already spent would end
// the track on a seek near the end of a slow file.
bs.resetStarvationLocked()
}
// resetStarvationLocked forgets an underrun run. Must be called with
// bs.mu held.
func (bs *BufferedStreamer) resetStarvationLocked() {
bs.starved = 0
bs.starvedSince = time.Time{}
}
// LockSource blocks the read-ahead goroutine from touching the
+213
View File
@@ -0,0 +1,213 @@
package player
import (
"log/slog"
"testing"
"time"
"github.com/wailsapp/wails/v3/pkg/application"
"yellowjacket/backend/events"
"yellowjacket/internal/testfixtures"
)
// fixtureSampleRate is what cmd/gentestdata writes (audio.go). It is
// deliberately not the speaker rate, which is what lets these tests
// tell the decoder's format from the player's default.
const fixtureSampleRate = 22050
// newTestPlayer is a player with a context and no database, so the
// track-metadata lookup cannot succeed.
func newTestPlayer(t *testing.T) *Player {
t.Helper()
p := NewPlayer(slog.Default(), nil)
rec := events.NewRecorder()
_ = p.ServiceStartup(
events.WithSink(t.Context(), rec),
application.ServiceOptions{},
)
return p
}
// loadFileLocked needs no speaker: it decodes, builds the chain and
// registers it paused. speaker.Play on an uninitialised device is
// what the integration guard elsewhere is about, so these assert on
// the state the load computed rather than on playback.
// p.format used to be assigned once, in the constructor, to the
// *speaker's* rate -- so it claimed 44.1 kHz for every file ever
// loaded. Play()'s replay-after-finish path resamples from it, so a
// finished track played again was resampled from a rate the decoder
// never produced: audibly the wrong speed and pitch, and wrong
// length and position arithmetic with it.
//
// The fixtures are 22050 Hz, which is exactly the point -- any of
// them disagrees with the speaker rate.
func TestLoadRecordsTheDecodersOwnFormat(t *testing.T) {
m := testfixtures.Load(t)
path := m.Case(t, testfixtures.CaseCoverDedup)[0]
p := newTestPlayer(t)
if got := p.format.SampleRate; got != speakerSampleRate {
t.Fatalf(
"precondition: a fresh player should hold the speaker "+
"rate, got %d",
got,
)
}
if err := p.LoadFile(path); err != nil {
t.Fatalf("LoadFile(%s): %v", path, err)
}
if p.format.SampleRate == speakerSampleRate {
t.Fatalf(
"p.format still holds the speaker rate (%d) after "+
"loading a %d Hz file: the replay path would "+
"resample from the wrong rate",
speakerSampleRate, fixtureSampleRate,
)
}
if got := int(p.format.SampleRate); got != fixtureSampleRate {
t.Errorf(
"expected the decoder's rate %d, got %d",
fixtureSampleRate, got,
)
}
}
// trackLengthMs is written only when the database has a row for the
// file and cleared only by UnloadTrack, so a track with no row used
// to inherit whatever the last track's duration was -- and every
// position report is scaled by it, so the whole seek bar was then
// reporting one track's progress on another track's scale.
//
// There is no database here, so the lookup cannot succeed: exactly
// the case that used to inherit.
func TestLoadDoesNotInheritThePreviousTracksDuration(t *testing.T) {
m := testfixtures.Load(t)
path := m.Case(t, testfixtures.CaseCoverDedup)[0]
p := newTestPlayer(t)
// Stand in for a previous track whose duration was resolved.
p.trackLengthMs = 9_999_000
if err := p.LoadFile(path); err != nil {
t.Fatalf("LoadFile(%s): %v", path, err)
}
if p.trackLengthMs == 9_999_000 {
t.Fatal(
"the previous track's duration survived the load: every " +
"position report for this track would be scaled by it",
)
}
}
// A new chain supersedes the old one's pending finished callback.
// Without this, a callback that queued for p.mu behind a LoadFile
// woke up and rewound, stopped and auto-advanced the *new* track.
func TestANewChainSupersedesTheOldFinishedCallback(t *testing.T) {
m := testfixtures.Load(t)
paths := m.Case(t, testfixtures.CaseCoverDedup)
if len(paths) < 2 {
t.Skip("need two fixture tracks")
}
p := newTestPlayer(t)
if err := p.LoadFile(paths[0]); err != nil {
t.Fatalf("LoadFile(%s): %v", paths[0], err)
}
stale := p.chainID
if err := p.LoadFile(paths[1]); err != nil {
t.Fatalf("LoadFile(%s): %v", paths[1], err)
}
if p.chainID == stale {
t.Fatal("loading a second file did not supersede the chain")
}
called := false
p.SetPlaybackFinishedHandler(func(error) { called = true })
// The first track's callback, arriving late.
p.onPlaybackFinished(stale, nil)
if called {
t.Error(
"a superseded chain's callback drove auto-advance: the " +
"track that is loaded now would be skipped",
)
}
if p.state == Stopped {
t.Error(
"a superseded chain's callback stopped the current track",
)
}
}
// The decoder is read by the read-ahead goroutine and by every
// position emit, and those used to be guarded by different mutexes:
// the read by srcMu, the position by the speaker lock, which
// read-ahead never takes. Under -race this failed on the emit that
// LoadFile itself makes.
//
// It needs the read-ahead goroutine to actually be running, so it
// keeps asking for the position for long enough to overlap it.
func TestPositionReadsDoNotRaceTheReadAhead(t *testing.T) {
m := testfixtures.Load(t)
path := m.Case(t, testfixtures.CaseFLACAlbum)[0]
p := newTestPlayer(t)
if err := p.LoadFile(path); err != nil {
t.Fatalf("LoadFile(%s): %v", path, err)
}
for range 200 {
if _, err := p.CurrentPositionSeconds(); err != nil {
t.Fatalf("CurrentPositionSeconds: %v", err)
}
}
}
// Seeking emits the landing position, and that emit reads the
// decoder -- so the source lock the seek holds must be released
// before it. A reentrant take here is a deadlock, not a failure,
// which is why this test exists rather than a comment.
func TestSeekEmitsWithoutDeadlocking(t *testing.T) {
m := testfixtures.Load(t)
path := m.Case(t, testfixtures.CaseFLACAlbum)[0]
p := newTestPlayer(t)
if err := p.LoadFile(path); err != nil {
t.Fatalf("LoadFile(%s): %v", path, err)
}
done := make(chan struct{})
go func() {
defer close(done)
_ = p.Seek(1)
}()
select {
case <-done:
case <-time.After(10 * time.Second):
t.Fatal("Seek deadlocked: the position emit re-took the source lock")
}
}
+171 -34
View File
@@ -52,9 +52,17 @@ type Player struct {
control *beep.Ctrl
volume *effects.Volume
speakerStreamer beep.Streamer
playbackFinishedHandler func()
playbackFinishedHandler func(error)
trackChangeID uint64
mediaControls mediacontrols.Handler
// chainID identifies the streamer chain currently registered with
// the speaker. updateStreamers bumps it, and the finished
// callback carries the value it was registered with, so a callback
// that queued for p.mu behind a LoadFile can tell that the player
// has moved on and return rather than rewinding somebody else's
// track.
chainID uint64
mediaControls mediacontrols.Handler
// duckAmount is the attenuation currently applied on top of the
// user's volume, in the same base-2 exponent effects.Volume uses.
@@ -180,11 +188,18 @@ func (p *Player) InitSpeaker() error {
}
// SetPlaybackFinishedHandler sets a callback invoked when a track
// finishes naturally. This allows the queue to drive auto-advance
// stops streaming. This allows the queue to drive auto-advance
// without circular imports.
//
// The error says *why* the track stopped: nil for a track that
// reached its end, non-nil for one that broke partway through. Both
// arrive here because both look identical to the speaker, and only
// the queue holds the metadata a PlaybackFailed needs -- but they are
// not the same event, and reporting a decode failure as a natural
// finish is how a broken file used to auto-advance in silence.
//
//wails:ignore // internal wiring, not part of the app's IPC surface.
func (p *Player) SetPlaybackFinishedHandler(handler func()) {
func (p *Player) SetPlaybackFinishedHandler(handler func(error)) {
p.mu.Lock()
defer p.mu.Unlock()
@@ -424,6 +439,19 @@ func (p *Player) updateStreamers(
newBaseStreamer beep.StreamSeeker,
sr beep.SampleRate,
) error {
// A new chain supersedes the old one, so any finished callback the
// old one still owes is stale from here on.
p.chainID++
// The previous read-ahead goroutine reads the same decoder this
// one is about to, under its own srcMu -- two goroutines, two
// mutexes, one decoder that is not safe for concurrent use. The
// replay-after-finish path rebuilds from p.seeker without going
// through LoadFile, which is where that pair could meet.
if p.buffered != nil {
p.buffered.Close()
}
// set base streamer
p.baseStreamer = newBaseStreamer
p.seeker = newBaseStreamer
@@ -474,23 +502,57 @@ func (p *Player) startPaused() {
p.control.Paused = true
speaker.Unlock()
// Captured, not read at callback time: by then p.chainID names
// whatever is loaded *now*, which is the thing the guard exists to
// distinguish this chain from.
chainID := p.chainID
buffered := p.buffered
// The beep.Callback runs with the speaker mutex held, so we
// dispatch to a goroutine that can safely acquire p.mu.
speaker.Play(beep.Seq(
p.speakerStreamer,
beep.Callback(func() {
go p.onPlaybackFinished()
// Asked here rather than under p.mu: this is the chain that
// just ended, and by the time the goroutine holds the lock
// p.buffered may be a different one.
var err error
if buffered != nil {
err = buffered.Err()
}
go p.onPlaybackFinished(chainID, err)
}),
))
p.state = Paused
}
// onPlaybackFinished handles the natural end of a track. It is
// called on a new goroutine from the beep callback (which holds
// the speaker lock) so that it can safely acquire p.mu.
func (p *Player) onPlaybackFinished() {
// onPlaybackFinished handles a track that stopped streaming, whether
// it ended or broke. It is called on a new goroutine from the beep
// callback (which holds the speaker lock) so that it can safely
// acquire p.mu.
//
// chainID names the streamer chain the callback fired for and srcErr
// says why it stopped.
func (p *Player) onPlaybackFinished(chainID uint64, srcErr error) {
p.mu.Lock()
// The player has moved on while this callback queued for the lock
// -- a user pressing Next during the last second of a track is
// enough. Everything below is about the *current* track: rewinding
// the decoder, saying playback stopped, asking the queue to
// advance. Doing any of it now would do it to the wrong track.
if chainID != p.chainID {
p.mu.Unlock()
p.logger.Debug(
"Ignoring finished callback for a superseded chain",
"chain", chainID, "current", p.chainID,
)
return
}
p.state = Stopped
handler := p.playbackFinishedHandler
mc := p.mediaControls
@@ -501,10 +563,11 @@ func (p *Player) onPlaybackFinished() {
// the Stopped state anyway, so this only moves the decoder.
p.rewindLocked()
p.emitPositionLocked()
p.mu.Unlock()
// Emit Wails events outside the lock — these are non-blocking
// calls that don't need player state.
// Emitted under p.mu, like every other transition in this file.
// Outside it, a Play() taking the lock in the gap emits `playing`
// first and this stale `stopped` lands last -- leaving the button
// showing play over a track that is audibly running.
p.emitPlaybackFinished()
events.Emit(
@@ -513,6 +576,8 @@ func (p *Player) onPlaybackFinished() {
map[string]string{"state": string(Stopped)},
)
p.mu.Unlock()
// Notify media controls outside the lock. The track just
// ended so position is 0.
if mc != nil {
@@ -521,12 +586,19 @@ func (p *Player) onPlaybackFinished() {
)
}
p.logger.Info("Playback finished naturally")
if srcErr != nil {
p.logger.Error(
"Playback stopped: the audio source failed",
"err", srcErr,
)
} else {
p.logger.Info("Playback finished naturally")
}
// Notify queue for auto-advance. Called without p.mu held
// because it re-enters the player via LoadFile/Play.
if handler != nil {
handler()
handler(srcErr)
}
}
@@ -587,6 +659,18 @@ func (p *Player) loadFileLocked(filePath string) error {
p.currentFile = f
// The decoder's own format, kept for the paths that rebuild the
// chain later: Play()'s replay branch resamples from it, so a
// stale rate there plays a finished track back at the wrong speed.
p.format = format
// The previous track's duration must not outlive it. This is set
// again by emitTrackChanged below, but only when the database has
// a row for the file -- and every position this player reports is
// scaled by it, so inheriting means every report is wrong by the
// ratio between two unrelated tracks.
p.trackLengthMs = 0
if err := p.updateStreamers(
streamer, format.SampleRate,
); err != nil {
@@ -906,6 +990,8 @@ func (p *Player) CurrentPosition() (int, error) {
return 0, errNoAudioFileLoaded
}
defer p.lockSourceLocked()()
speaker.Lock()
pos := math.Round(
100.0 * float64(p.seeker.Position()) /
@@ -924,6 +1010,28 @@ func (p *Player) Seek(targetSeconds int) error {
return p.seekLocked(targetSeconds)
}
// lockSourceLocked blocks the read-ahead goroutine from touching the
// decoder and returns the function that releases it, so a caller can
// `defer p.lockSourceLocked()()`.
//
// Reading the decoder's position is a read *of the decoder*, and the
// speaker lock does not exclude the read-ahead goroutine -- it never
// takes it. That was a genuine data race on every position emit,
// once a second for the whole of playback.
//
// srcMu is not reentrant, so nothing that already holds it may call
// this; seekSourceLocked exists to keep that region free of emits.
// Must be called with p.mu held.
func (p *Player) lockSourceLocked() func() {
if p.buffered == nil {
return func() {}
}
p.buffered.LockSource()
return p.buffered.UnlockSource
}
// rewindLocked returns the decoder to the start of the track without
// touching playback state. Must be called with p.mu held.
func (p *Player) rewindLocked() {
@@ -959,6 +1067,46 @@ func (p *Player) seekLocked(targetSeconds int) error {
return fmt.Errorf("cannot get track length: %w", err)
}
// The source lock is released before anything below is emitted:
// emitPositionLocked reads the decoder's position and takes the
// same lock, which is not reentrant.
seekErr := p.seekSourceLocked(targetSeconds, lengthSecs)
if seekErr != nil {
p.logger.Warn(
"Seek failed, playback will start from "+
"the beginning",
"target-seconds", targetSeconds,
"err", seekErr,
)
// The optimistic move the UI already made has to be taken
// back, and only the backend knows it did not happen.
events.Emit(p.ctx, events.SeekFailed)
p.emitPositionLocked()
return fmt.Errorf("failed to seek: %w", seekErr)
}
if p.mediaControls != nil {
p.mediaControls.NotifySeek(targetSeconds)
}
// Report the landing position immediately rather than leaving the
// UI to guess until the next tick — this is the half of H-3 that
// desynced the seek bar by 30 s over four keyboard seeks.
p.emitPositionLocked()
return nil
}
// seekSourceLocked moves the decoder and flushes the stale read-ahead
// behind it. It owns the source lock for exactly that long and
// emits nothing, so its caller is free to read the position
// afterwards. Must be called with p.mu held.
func (p *Player) seekSourceLocked(
targetSeconds int,
lengthSecs int,
) error {
// Block the read-ahead goroutine from reading the source while
// we seek it. The decoder (e.g. FLAC's bufseekio.ReadSeeker) is
// not safe for concurrent Read+Seek, and read-ahead runs on its
@@ -1014,19 +1162,11 @@ func (p *Player) seekLocked(targetSeconds int) error {
if seekErr != nil {
speaker.Unlock()
p.logger.Warn(
"Seek failed, playback will start from "+
"the beginning",
"target-seconds", targetSeconds,
"samples", samples,
"err", seekErr,
p.logger.Debug(
"seek rejected by the decoder",
"samples", samples, "err", seekErr,
)
// The optimistic move the UI already made has to be taken
// back, and only the backend knows it did not happen.
events.Emit(p.ctx, events.SeekFailed)
p.emitPositionLocked()
return fmt.Errorf("failed to seek: %w", seekErr)
}
@@ -1039,15 +1179,6 @@ func (p *Player) seekLocked(targetSeconds int) error {
p.buffered.Flush()
}
if p.mediaControls != nil {
p.mediaControls.NotifySeek(targetSeconds)
}
// Report the landing position immediately rather than leaving the
// UI to guess until the next tick — this is the half of H-3 that
// desynced the seek bar by 30 s over four keyboard seeks.
p.emitPositionLocked()
return nil
}
@@ -1140,6 +1271,10 @@ func (p *Player) seekerLengthSecsLocked() (int, error) {
return 0, errNoAudioFileLoaded
}
// Len is fixed for the life of the decoder, so unlike Position it
// races with nothing and needs no source lock -- which it must not
// take anyway: displayPositionSecsLocked calls this while holding
// it, and srcMu is not reentrant.
speaker.Lock()
length := p.seeker.Len() / int(p.format.SampleRate)
speaker.Unlock()
@@ -1156,6 +1291,8 @@ func (p *Player) displayPositionSecsLocked() int {
return 0
}
defer p.lockSourceLocked()()
speaker.Lock()
pos := p.seeker.Position()
total := p.seeker.Len()
+3 -3
View File
@@ -83,7 +83,7 @@ func TestFallback_TriggersOnNaturalFinish(t *testing.T) {
q.SetFallbackSource(fake)
q.SetQueue(seedPaths, 0, false, Source{Type: "album", ID: 1, Label: "Seed Album"})
q.OnPlaybackFinished()
q.OnPlaybackFinished(nil)
waitUntil(t, func() bool { return fake.callCount() == 1 }, "fallback to be resolved")
waitUntil(t, func() bool {
@@ -159,7 +159,7 @@ func TestFallback_EmptyResultLeavesQueueExhausted(t *testing.T) {
q.SetFallbackSource(fake)
q.SetQueue(seedPaths, 0, false, Source{})
q.OnPlaybackFinished()
q.OnPlaybackFinished(nil)
waitUntil(t, func() bool { return fake.callCount() == 1 }, "fallback to be resolved")
@@ -193,7 +193,7 @@ func TestFallback_StaleResolutionDiscarded(t *testing.T) {
q.SetFallbackSource(fake)
q.SetQueue(seedPaths, 0, false, Source{})
q.OnPlaybackFinished() // starts resolving, blocked on gate
q.OnPlaybackFinished(nil) // starts resolving, blocked on gate
time.Sleep(20 * time.Millisecond) // let the goroutine reach the gate
+78
View File
@@ -0,0 +1,78 @@
package queue
import (
"errors"
"testing"
"yellowjacket/backend/events"
)
// errTestDecode stands in for a decoder blowing up mid-track.
var errTestDecode = errors.New("decode blew up")
// currentIndex == -1 against a non-empty queue is a state this
// package produces on purpose: onQueueExhausted(false) sets it and
// deliberately leaves the finished track loaded in the player, so it
// stays on the now-playing bar. Pressing play from there and letting
// it finish re-enters OnPlaybackFinished with exactly that pair --
// which used to index q.tracks[-1] and panic, on a goroutine
// dispatched from the audio callback with no caller to recover it.
func TestFinishedWithNoCurrentTrackDoesNotPanic(t *testing.T) {
t.Parallel()
tests := []struct {
name string
index int
}{
{"exhausted queue leaves -1", -1},
{"index past the end", 3},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
q, _, _ := setupRecordedQueue(t)
q.tracks = []Track{
{FilePath: "/a.mp3"},
{FilePath: "/b.mp3"},
}
q.currentIndex = tt.index
// The assertion is that this returns at all.
q.OnPlaybackFinished(nil)
if q.currentIndex != tt.index {
t.Errorf(
"an out-of-range index was acted on: %d became %d",
tt.index, q.currentIndex,
)
}
})
}
}
// A track that broke mid-playback is not a track that was listened
// to. The player cannot say so itself -- the metadata is here -- so
// it hands the reason over and this is where it becomes a
// PlaybackFailed rather than a silent auto-advance.
func TestAFailedTrackIsReportedAndNotCountedAsAPlay(t *testing.T) {
t.Parallel()
q, _, rec := setupRecordedQueue(t)
q.tracks = []Track{
{FilePath: "/a.mp3", Title: "A", AudioFileID: 1},
{FilePath: "/b.mp3", Title: "B", AudioFileID: 2},
}
q.currentIndex = 0
q.OnPlaybackFinished(errTestDecode)
if _, ok := rec.Last(events.PlaybackFailed); !ok {
t.Errorf(
"a track that failed mid-playback told the user nothing; "+
"got %v",
rec.Names(),
)
}
}
+36 -9
View File
@@ -1,18 +1,45 @@
package queue
// OnPlaybackFinished is called when a track finishes playing naturally.
// This drives the auto-advance behavior and records the play.
func (q *Queue) OnPlaybackFinished() {
// OnPlaybackFinished is called when a track stops streaming. This
// drives the auto-advance behavior and records the play.
//
// srcErr says why the track stopped: nil for one that reached its
// end, non-nil for one that broke partway through. The player cannot
// tell the user which, because the metadata lives here -- so a failure
// is reported as PlaybackFailed and *not* recorded as a play, while
// the advance happens either way. Before this, a file that failed
// mid-track advanced in silence and was counted as listened to.
//
//wails:ignore // internal wiring, not part of the app's IPC surface.
func (q *Queue) OnPlaybackFinished(srcErr error) {
q.mu.Lock()
if len(q.tracks) == 0 {
// currentIndex is -1 whenever the queue has been exhausted, and
// onQueueExhausted deliberately leaves the finished track loaded
// in the player -- so a natural finish can re-enter here against a
// queue that is not empty and an index that is not valid. Every
// other path in this package bounds-checks before indexing; this
// one panicked, on a goroutine with no caller to recover it.
if q.currentIndex < 0 || q.currentIndex >= len(q.tracks) {
q.mu.Unlock()
return
}
// Capture the track that just finished before advancing.
finishedID := q.tracks[q.currentIndex].AudioFileID
finished := q.tracks[q.currentIndex]
finishedID := finished.AudioFileID
if srcErr != nil {
q.emitPlaybackFailed(finished, srcErr)
}
// A track that broke was not listened to.
recordFinished := func() {
if srcErr == nil {
q.recordPlay(finishedID)
}
}
// Repeat One: replay the current track.
if q.repeatMode == RepeatOne {
@@ -21,7 +48,7 @@ func (q *Queue) OnPlaybackFinished() {
}
q.mu.Unlock()
q.recordPlay(finishedID)
recordFinished()
return
}
@@ -31,7 +58,7 @@ func (q *Queue) OnPlaybackFinished() {
// Queue exhausted — this is the extension point for a future fallback playlist.
q.onQueueExhausted(false)
q.mu.Unlock()
q.recordPlay(finishedID)
recordFinished()
return
}
@@ -44,12 +71,12 @@ func (q *Queue) OnPlaybackFinished() {
if !q.playCurrentOrSkip(true, q.nextIndex) {
q.onQueueExhausted(false)
q.mu.Unlock()
q.recordPlay(finishedID)
recordFinished()
return
}
q.emitIndexChanged()
q.mu.Unlock()
q.recordPlay(finishedID)
recordFinished()
}
+2 -2
View File
@@ -107,7 +107,7 @@ func TestPlaybackFailed_AutoAdvanceSkipsPastIt(t *testing.T) {
// The first track finished: auto-advance lands on the missing
// file and must step over it rather than stopping dead.
q.OnPlaybackFinished()
q.OnPlaybackFinished(nil)
if got := q.GetState().CurrentIndex; got != 2 {
t.Errorf("currentIndex after skipping: got %d, want 2", got)
@@ -183,7 +183,7 @@ func TestQueueExhausted_KeepsTheFinishedTrackLoaded(t *testing.T) {
q.SetQueue(paths, 0, false, Source{})
q.Play()
q.OnPlaybackFinished()
q.OnPlaybackFinished(nil)
if q.GetState().CurrentIndex != -1 {
t.Errorf(
+27 -11
View File
@@ -1,6 +1,12 @@
import { test, expect } from '../support/fixtures.js';
import type { Page } from '@playwright/test';
/**
* How far the scroll test scrolls. One constant, because the guard and
* the assertion have to agree about it — they did not, which is #133.
*/
const SCROLL_TARGET = 80;
/**
* Plan 007 phase 5: expanding an album shows its tracks.
*
@@ -104,20 +110,27 @@ test.describe('the album dropdown', () => {
await app.setViewportSize({ width: 900, height: 600 });
try {
await expect.poll(() => scrollRange(app)).toMatchObject({
scrollable: true,
overflowY: 'auto',
});
// Wait for the range the assertion below actually needs, not for
// "scrollable at all" (#133). The guard used to be
// `scrollHeight > clientHeight + 40` while the next line asks to
// reach 80, so any range in 41-79 satisfied it and could not
// satisfy the assertion — and the grid passes through exactly
// that while it settles, because it recomputes its columns after
// the resize rather than during it. The settled range here is
// 330, so this waits rather than weakening anything.
await expect
.poll(() => scrollRange(app))
.toMatchObject({ room: true, overflowY: 'auto' });
await app.evaluate(() => {
await app.evaluate((target) => {
const sc = document
.querySelector('cover-grid')
?.shadowRoot?.querySelector('.grid-scroll-container');
if (sc) sc.scrollTop = 80;
});
if (sc) sc.scrollTop = target;
}, SCROLL_TARGET);
expect(await scrollTop(app)).toBe(80);
expect(await scrollTop(app)).toBe(SCROLL_TARGET);
// And the dropdown it opens is on screen, wherever the manager
// decides that leaves the scroll. It is *not* "the position is
@@ -250,16 +263,19 @@ async function closeDropdown(app: Page): Promise<void> {
/** Whether the grid can scroll at all, which decides if a probe can move. */
async function scrollRange(app: Page) {
return app.evaluate(() => {
return app.evaluate((target) => {
const sc = document
.querySelector('cover-grid')
?.shadowRoot?.querySelector('.grid-scroll-container');
return {
scrollable: !!sc && sc.scrollHeight > sc.clientHeight + 40,
// `room` is the precondition of the assertion that follows it:
// enough range to actually reach the target. A threshold below
// what the caller depends on is not a guard.
room: !!sc && sc.scrollHeight - sc.clientHeight >= target,
overflowY: sc ? getComputedStyle(sc).overflowY : '',
};
});
}, SCROLL_TARGET);
}
async function scrollTop(app: Page): Promise<number> {
+6
View File
@@ -26,6 +26,12 @@ const MIN_VIEWPORT = { width: 800, height: 600 };
const VIEWPORTS = [
{ name: '1440×900', width: 1440, height: 900 },
{ name: '1024×768', width: 1024, height: 768 },
// Not the minimum, and that is the point (#24). The sidebar collapses
// to icons *below* 900, so the main panel is 843px at 899 and 700px
// at 900 — the narrowest content area any desktop width produces is
// here, not at the enforced floor. A list that stopped at the minimum
// was missing its own worst case.
{ name: '900×600 (the widest sidebar, so the narrowest content)', width: 900, height: 600 },
{ name: `the minimum (${MIN_VIEWPORT.width}×${MIN_VIEWPORT.height})`, ...MIN_VIEWPORT },
];
+187
View File
@@ -0,0 +1,187 @@
import { test, expect } from '../support/fixtures.js';
/**
* #24 the queue panel does not take the page's width away from it.
*
* The panel is `flex-shrink: 0` in the flow of `.content-area`, so an
* open queue used to be paid for by the main panel. Measured on
* Playlists before the fix:
*
* | viewport | main panel |
* |---|---|
* | 900×600 | 379px all three header actions clipped |
* | 390×780 | 69px |
* | 320×600 | **0px** |
*
* **900×600 is the worst desktop case, not the 800×600 minimum**, and
* that is the trap this file exists to keep closed: the sidebar
* collapses to icons *below* 900, so the main panel is 843px at 899 and
* 700px at 900. A spec that checks "the minimum" and stops has not
* checked the worst case which is what every viewport list in this
* suite did before this.
*
* These assert the *content's* width rather than the panel's mode
* wherever they can, because the mode is the mechanism and the width is
* the complaint.
*/
/** The bands from plan 018's size matrix, plus the pixel above the collapse. */
const BANDS = [
{ name: 'a wide desktop (1280×800)', width: 1280, height: 800, inline: true },
{ name: 'the default window (1100×720)', width: 1100, height: 720, inline: true },
{ name: 'a laptop (1024×768)', width: 1024, height: 768, inline: true },
{ name: 'the worst desktop width (900×600)', width: 900, height: 600, inline: false },
{ name: 'the enforced minimum (800×600)', width: 800, height: 600, inline: false },
{ name: 'a phone (390×780)', width: 390, height: 780, inline: false },
{ name: '400% zoom (320×600)', width: 320, height: 600, inline: false },
];
/**
* How much room the content has, and whether the shell needs scrolling
* to reach any of itself.
*/
const shellGeometry = (page: import('@playwright/test').Page) =>
page.evaluate(() => {
const main = document.querySelector('#main-content')!.getBoundingClientRect();
const panel = document.querySelector('#queue-panel')!;
return {
mainWidth: Math.round(main.width),
overlay: panel.hasAttribute('overlay'),
open: panel.hasAttribute('open'),
bodyScrollWidth: document.body.scrollWidth,
bodyClientWidth: document.body.clientWidth,
};
});
async function openQueue(page: import('@playwright/test').Page) {
const toggle = page.locator('#queue-button');
if ((await toggle.getAttribute('aria-expanded')) !== 'true') {
await toggle.click();
}
await expect(toggle).toHaveAttribute('aria-expanded', 'true');
}
test.describe('an open queue leaves the content its width', () => {
for (const band of BANDS) {
test(`at ${band.name}`, async ({ app }) => {
await app.setViewportSize({ width: band.width, height: band.height });
await openQueue(app);
// The mode is settled by a ResizeObserver, so poll rather than
// read once: a single read races the resize and reports the
// previous viewport's answer.
await expect
.poll(async () => (await shellGeometry(app)).overlay)
.toBe(!band.inline);
const geo = await shellGeometry(app);
// The floor is the point of the whole issue. Inline, the queue is
// affordable and the content keeps the rest; as an overlay the
// content keeps *everything*, which is what makes 0px at 320
// impossible rather than merely unlikely.
expect(geo.mainWidth).toBeGreaterThanOrEqual(320);
if (!band.inline) {
expect(geo.mainWidth).toBeGreaterThanOrEqual(
Math.min(band.width, 320),
);
}
// And opening the queue must not make the shell overflow.
expect(geo.bodyScrollWidth).toBeLessThanOrEqual(geo.bodyClientWidth);
});
}
});
test.describe('an overlaid queue says it is over the content', () => {
test.beforeEach(async ({ app }) => {
await app.setViewportSize({ width: 900, height: 600 });
});
test('draws a scrim and closes when it is clicked', async ({ app }) => {
await openQueue(app);
const panel = app.locator('#queue-panel');
await expect(panel).toHaveAttribute('overlay', '');
// The scrim is `aria-hidden` on purpose — it is a dismissal target,
// and the named routes out are the close button and Escape — so it
// is located structurally rather than by role.
await panel.evaluate((el) =>
el.shadowRoot!.querySelector<HTMLElement>('.scrim')!.click(),
);
await expect(app.locator('#queue-button')).toHaveAttribute(
'aria-expanded',
'false',
);
});
/**
* `getByRole`, not a shadow-root query: this repo has shipped a
* nameless control three times, and a drawer with a scrim is exactly
* the shape that grows a fourth.
*/
test('offers a named close button', async ({ app }) => {
await openQueue(app);
const close = app.getByRole('button', { name: 'Close queue' });
await expect(close).toBeVisible();
await close.click();
await expect(app.locator('#queue-button')).toHaveAttribute(
'aria-expanded',
'false',
);
});
test('closes on Escape and gives focus back to the toggle', async ({
app,
}) => {
const toggle = app.locator('#queue-button');
await toggle.focus();
await toggle.click();
await expect(toggle).toHaveAttribute('aria-expanded', 'true');
await app.keyboard.press('Escape');
await expect(toggle).toHaveAttribute('aria-expanded', 'false');
await expect(toggle).toBeFocused();
});
});
/**
* The inline panel is the mode that already worked, and the one every
* other queue spec is written against. It keeps its resize handle and
* gains none of the overlay's chrome.
*/
test.describe('a wide window keeps the queue beside the content', () => {
test('no scrim, no close button, and the content is narrower', async ({
app,
}) => {
await app.setViewportSize({ width: 1280, height: 800 });
const widthWithoutQueue = (await shellGeometry(app)).mainWidth;
await openQueue(app);
await expect(app.locator('#queue-panel')).not.toHaveAttribute(
'overlay',
'',
);
const geo = await shellGeometry(app);
expect(geo.mainWidth).toBeLessThan(widthWithoutQueue);
await expect(
app.getByRole('button', { name: 'Close queue' }),
).toHaveCount(0);
});
});
@@ -113,14 +113,6 @@ export function Next(): $CancellablePromise<void> {
return $Call.ByID(1968784044);
}
/**
* OnPlaybackFinished is called when a track finishes playing naturally.
* This drives the auto-advance behavior and records the play.
*/
export function OnPlaybackFinished(): $CancellablePromise<void> {
return $Call.ByID(2184869763);
}
/**
* Play handles a play request by either resuming the current track or
* starting playback from the beginning of the queue. When a track is
+8
View File
@@ -231,6 +231,14 @@ body div.sidebar {
display: flex;
overflow: hidden;
contain: layout style;
/* The containing block for the queue panel's overlay mode (plan
018, #24), which spans this box rather than taking width from
the main panel beside it. `contain: layout` already establishes
one; this says so on purpose, so that removing the containment
for a paint reason does not silently reparent the overlay to the
viewport. */
position: relative;
}
.main-panel {
@@ -72,6 +72,24 @@ const MIN_WIDTH = 200;
const MAX_WIDTH = 500;
const DEFAULT_WIDTH = 320;
/**
* The narrowest main panel the queue is allowed to leave behind before
* it stops being a column and becomes an overlay (plan 018, issue #24).
*
* There is no cliff to derive this from, and pretending otherwise would
* be the more dishonest answer: the track list rescales its columns
* continuously (213px down to 124px between main widths of 900 and 544,
* with no row overflow at any of them) and the album grid steps 3
* columns to 2 without breaking. So this is a judgement, anchored at
* both ends it keeps the *default* 1100px window inline, because an
* inline queue is a desktop affordance people choose and demoting the
* common case to an overlay would be a regression in feel; and it puts
* every case measured as broken on the overlay side, which is 900x600
* (main = 379px, where all three of the Playlists header's actions are
* clipped) and every phone width (main = 69px at 390, 0px at 320).
*/
const MAIN_PANEL_FLOOR = 480;
@customElement('queue-panel')
export class QueuePanel
extends LitElement
@@ -85,6 +103,22 @@ export class QueuePanel
@property({ type: Boolean, reflect: true })
open = false;
/**
* Whether the panel is covering the content instead of sitting
* beside it. **Computed, never set by a caller** it is reflected
* so the stylesheet and a spec can both read it.
*
* It is deliberately *not* a media query, which is the whole reason
* this is a property and not a `@media` block. The panel's width is
* user state: drag-resizable between MIN_WIDTH and MAX_WIDTH and
* persisted. A breakpoint at a fixed viewport width silently
* assumes the default 320, so it is wrong by up to 180px for a user
* who has widened the panel in the direction that hurts, since a
* wider queue is exactly when the content can least afford it.
*/
@property({ type: Boolean, reflect: true })
overlay = false;
@state()
private isDragging = false;
@@ -190,6 +224,19 @@ export class QueuePanel
private panelWidth = DEFAULT_WIDTH;
private scrollbarDragging = false;
/** Watches `.content-area`, which is the viewport minus the sidebar. */
private spaceObserver?: ResizeObserver;
/**
* What had focus when the overlay opened, so Escape and the scrim
* can give it back. Focus is only taken back if the panel had it
* the same rule `MenuKeyboard` follows, for the same reason: the
* queue can also be closed by the button in the bottom bar, and
* yanking focus away from wherever the user actually is would be
* worse than leaving it.
*/
private overlayOpener: HTMLElement | null = null;
// _itemSize is an internal property applied via Object.assign in BaseLayout's
// config setter. Setting it to match the actual fixed .track-item height (49px)
// prevents lit-virtualizer's scroll error correction from fighting the native
@@ -265,6 +312,86 @@ export class QueuePanel
border-left: 1px solid var(--yj-border-subtle, #333);
}
/* ---------------------------------------------------------
Overlay mode (plan 018, #24).
In flow the panel takes its width *from the main panel*,
which is the reported bug: at 900x600 that left 379px and
clipped every action in the Playlists header, and at 320px
it left 0px the content was not degraded but gone.
Here the host spans the whole content area instead and
stops being a layout participant, so the main panel keeps
its full width and the queue sits over it. The host itself
is transparent and click-through; the scrim and the panel
are what take pointer events. The containment drops paint,
which would otherwise clip the panel's own shadow.
--------------------------------------------------------- */
:host([overlay]) {
position: absolute;
inset: 0;
width: auto;
background-color: transparent;
overflow: visible;
pointer-events: none;
contain: layout style;
z-index: 20;
}
/* Closed, an overlay is not there at all. In flow the panel is
width: 0, which is its own way of saying this; absolutely
positioned there is no width to collapse. */
:host([overlay]:not([open])) {
display: none;
}
:host([overlay][open]) {
border-left: none;
}
:host([overlay]) .panel-content {
position: absolute;
top: 0;
right: 0;
bottom: 0;
width: var(--queue-width, ${unsafeCSS(DEFAULT_WIDTH)}px);
max-width: 100%;
box-sizing: border-box;
background-color: var(--yj-bg-surface, #212529);
border-left: 1px solid var(--yj-border-subtle, #333);
box-shadow: -8px 0 24px rgb(0 0 0 / 45%);
pointer-events: auto;
}
/* Dragging the edge of something that is already covering the
content answers a question nobody asked, and it is a
mouse-only affordance either way. */
:host([overlay]) .resize-handle {
display: none;
}
.scrim {
position: absolute;
inset: 0;
background-color: rgb(0 0 0 / 45%);
pointer-events: auto;
border: none;
padding: 0;
margin: 0;
cursor: pointer;
}
/* The phone gets the whole width: below 600 there is no
"beside" left to be, and this is the shape #55 turns into a
real screen. A media query inside a shadow root is answered
by the viewport, so the component states this itself rather
than the shell reaching in. */
@media (max-width: 599px) {
:host([overlay]) .panel-content {
width: 100%;
}
}
.resize-handle {
position: absolute;
top: 0;
@@ -656,6 +783,20 @@ export class QueuePanel
'--queue-width',
`${this.panelWidth}px`,
);
// The mode is a measurement, so it is observed rather than
// computed once: the parent is `.content-area`, whose width is
// the viewport minus the sidebar — including the sidebar's own
// collapse at 900px, which is what makes 900 the *worst*
// desktop width rather than the minimum.
this.updateOverlayMode();
if (this.parentElement) {
this.spaceObserver = new ResizeObserver(() =>
this.updateOverlayMode(),
);
this.spaceObserver.observe(this.parentElement);
}
document.addEventListener(
'mousemove',
this.handleMouseMove,
@@ -690,6 +831,9 @@ export class QueuePanel
super.disconnectedCallback();
this.creditsUnsub?.();
this.creditsUnsub = undefined;
this.spaceObserver?.disconnect();
this.spaceObserver = undefined;
document.removeEventListener('keydown', this.onOverlayKeydown);
document.removeEventListener(
'mousemove',
this.handleMouseMove,
@@ -736,7 +880,79 @@ export class QueuePanel
this.delegationAttached = false;
}
/**
* Decide whether the queue can afford to be a column.
*
* The parent is `.content-area`, so its width is the viewport minus
* the sidebar and the sum already accounts for the sidebar's own
* collapse. It is stable across the panel's own open/closed state
* in both modes in flow the panel is a child of that box, and as
* an overlay it is out of flow so this cannot oscillate.
*/
private updateOverlayMode = () => {
const available = this.parentElement?.clientWidth ?? 0;
// Before layout there is nothing to measure, and answering 0 by
// flipping to overlay would show the scrim for a frame.
if (available === 0) return;
this.overlay = available - this.panelWidth < MAIN_PANEL_FLOOR;
};
/**
* Escape closes a scrimmed overlay, which is the one keyboard rule
* every dialog in this app already follows.
*
* It is a document listener rather than a panel-scoped binding
* (`services/shortcut-scope.ts`) because it is not a *shortcut*: it
* is the dismissal of something covering the page, and it has to
* work while focus is still behind the scrim. It is attached only
* while the overlay is actually up and removed on close, so it is
* scoped to a state rather than being a permanent global. Nothing
* else binds Escape the shortcut service only uses it to blur a
* text input.
*/
private onOverlayKeydown = (e: KeyboardEvent) => {
if (e.key !== 'Escape' || !this.open || !this.overlay) return;
e.stopPropagation();
this.closeFromOverlay();
};
private closeFromOverlay = () => {
const hadFocus = this.contains(
document.activeElement as Node | null,
);
this.open = false;
if (hadFocus) {
const back =
this.overlayOpener ??
document.getElementById('queue-button');
back?.focus();
}
this.overlayOpener = null;
};
override updated() {
// The overlay owns Escape only while it is up.
if (this.open && this.overlay) {
document.addEventListener('keydown', this.onOverlayKeydown);
this.overlayOpener ??=
document.activeElement instanceof HTMLElement &&
document.activeElement !== document.body
? document.activeElement
: null;
} else {
document.removeEventListener('keydown', this.onOverlayKeydown);
if (!this.open) this.overlayOpener = null;
}
// Closed, the panel is `width: 0` — which hides it from the eye
// and from nobody else: its Clear and Add buttons still took tab
// stops at x=1440 and were still read out (H-5). `inert` is the
@@ -1631,6 +1847,11 @@ export class QueuePanel
'--queue-width',
`${clampedWidth}px`,
);
// Widening the panel is one of the two ways the content can run
// out of room, and it is the way a viewport-width media query
// cannot see at all.
this.updateOverlayMode();
};
private handleMouseUp = () => {
@@ -1714,6 +1935,15 @@ export class QueuePanel
const tracks = this.queue.tracks;
return html`
${this.overlay
? html`<div
class="scrim"
part="scrim"
data-testid="queue-scrim"
aria-hidden="true"
@click=${this.closeFromOverlay}
></div>`
: nothing}
<div class="panel-content">
<div
class="resize-handle ${this.isDragging
@@ -1763,6 +1993,16 @@ export class QueuePanel
name=${ICON_PLAYLIST}
></wa-icon>
</button>
${this.overlay
? html`<button
class="header-action-button"
data-testid="queue-close"
aria-label="Close queue"
@click=${this.closeFromOverlay}
>
<wa-icon name="xmark"></wa-icon>
</button>`
: nothing}
</div>
</div>
@@ -0,0 +1,175 @@
/**
* #24 the queue stops being a column when it cannot afford to be one.
*
* In flow the panel is `flex-shrink: 0`, so it takes its width *from
* the main panel* rather than covering it. Measured against the running
* app on the Playlists page, that left 379px of content at 900×600
* with all three of the page header's actions clipped 69px at 390px
* wide, and **0px** at 320px, where the content was not degraded but
* gone.
*
* The rule is `available - panelWidth >= MAIN_PANEL_FLOOR`, and the
* reason it is a computed property rather than a `@media` block is the
* third test here: the panel's width is user state, drag-resizable
* between 200 and 500px and persisted, so a breakpoint on the viewport
* alone is wrong by up to 180px for a user who has widened it in the
* direction that hurts, since a wider queue is exactly when the content
* can least afford it.
*
* The parent is `.content-area`, i.e. the viewport minus the sidebar,
* which is why these mount into a sized wrapper rather than into
* `document.body`: the width that decides this is the *parent's*, and
* `fixture()` would hand the panel the whole test window.
*/
import { describe, expect, it, afterEach } from 'vitest';
import '@components/queue-panel/queue-panel';
import type { QueuePanel } from '@components/queue-panel/queue-panel';
import { shadow } from '@test/support/render';
const wrappers: HTMLElement[] = [];
afterEach(() => {
for (const w of wrappers.splice(0)) w.remove();
});
/**
* Mount a panel inside a parent of a stated width.
*
* The wrapper is `position: relative` and `display: flex` because that
* is what `.content-area` is; the mode is measured from
* `parentElement.clientWidth`, so a wrapper that collapses to its
* content would measure the panel rather than the space around it.
*/
async function panelIn(parentWidth: number): Promise<QueuePanel> {
const wrapper = document.createElement('div');
wrapper.style.cssText = `position: relative; display: flex; width: ${parentWidth}px;`;
document.body.append(wrapper);
wrappers.push(wrapper);
const el = document.createElement('queue-panel') as QueuePanel;
el.open = true;
wrapper.append(el);
await el.updateComplete;
await settle(el);
return el;
}
/**
* A ResizeObserver delivers on a frame, not a microtask, so the mode
* lands a frame after the width that decides it.
*/
async function settle(el: QueuePanel): Promise<void> {
for (let frame = 0; frame < 4; frame += 1) {
await new Promise((r) => {
requestAnimationFrame(() => r(null));
});
await el.updateComplete;
}
}
/** Drag the resize handle by `dx`, the way a user widens the panel. */
async function dragHandleBy(el: QueuePanel, dx: number): Promise<void> {
const handle = shadow<HTMLElement>(el, '.resize-handle');
const startX = el.getBoundingClientRect().left;
if (!handle) throw new Error('no resize handle to drag');
handle.dispatchEvent(
new MouseEvent('mousedown', { clientX: startX, bubbles: true }),
);
document.dispatchEvent(
new MouseEvent('mousemove', { clientX: startX - dx, bubbles: true }),
);
document.dispatchEvent(new MouseEvent('mouseup', { bubbles: true }));
await settle(el);
}
describe('the queue panel decides whether it can be a column', () => {
it('stays inline while the content can spare the width', async () => {
const el = await panelIn(1080);
expect(el.overlay).toBe(false);
expect(el.hasAttribute('overlay')).toBe(false);
});
it('becomes an overlay when it cannot', async () => {
const el = await panelIn(700);
expect(el.overlay).toBe(true);
expect(el.hasAttribute('overlay')).toBe(true);
});
/**
* The test the media query could not have passed. The parent does not
* move; only the user's own panel width does.
*/
it('flips to overlay when the user widens the panel, at a fixed width', async () => {
const el = await panelIn(880);
expect(el.overlay).toBe(false);
await dragHandleBy(el, 180);
expect(el.overlay).toBe(true);
});
it('gives an overlay a scrim and a named way out, and an inline panel neither', async () => {
const overlaid = await panelIn(700);
expect(shadow(overlaid, '.scrim')).toBeTruthy();
const close = shadow(overlaid, '[data-testid="queue-close"]');
expect(close?.getAttribute('aria-label')).toBe('Close queue');
const inline = await panelIn(1080);
expect(inline.shadowRoot?.querySelector('.scrim')).toBeNull();
expect(
inline.shadowRoot?.querySelector('[data-testid="queue-close"]'),
).toBeNull();
});
it('closes on the scrim, on the close button and on Escape', async () => {
for (const close of [
(el: QueuePanel) => shadow<HTMLElement>(el, '.scrim')?.click(),
(el: QueuePanel) =>
shadow<HTMLElement>(el, '[data-testid="queue-close"]')?.click(),
() =>
document.dispatchEvent(
new KeyboardEvent('keydown', { key: 'Escape', bubbles: true }),
),
]) {
const el = await panelIn(700);
expect(el.open).toBe(true);
close(el);
await el.updateComplete;
expect(el.open).toBe(false);
}
});
/**
* Escape belongs to the overlay, not to the queue. An inline panel is
* beside the content rather than over it, so there is nothing to
* dismiss and the key has to reach whatever else wants it.
*/
it('leaves Escape alone while inline', async () => {
const el = await panelIn(1080);
document.dispatchEvent(
new KeyboardEvent('keydown', { key: 'Escape', bubbles: true }),
);
await el.updateComplete;
expect(el.open).toBe(true);
});
});
+10 -1
View File
@@ -63,8 +63,17 @@ pre-commit:
root: "frontend/"
run: node scripts/check-css-literals.mjs
# Deliberately sequential, unlike pre-commit. `go test -race`
# saturates every core for the better part of a minute and the UI tier
# is a real browser with wall-clock timeouts, so run together the
# browser loses: setup took 106s inside the hook against 63s
# standalone, and a different suite failed each time -- three suites
# failing to fetch setup.ts from Vitest's own dev server on one run, a
# 15s "did not mount itself" on the next, against a suite that passes
# 898/898 on its own. A gate that fails at random is not a gate. The
# ~15s saved is not worth it.
pre-push:
parallel: true
parallel: false
commands:
go-test:
glob: "*.go"
+63
View File
@@ -97,6 +97,55 @@ if [ -f "$PID_FILE" ] && kill -0 "$(cat "$PID_FILE")" 2>/dev/null; then
fi
rm -f "$PID_FILE"
# ── Refuse to inherit somebody else's port ───────────────────────────
# The PID check above only knows about *this* worktree: `make dev-stop`
# kills the pid in this .dev/app.pid and nothing else. Several worktrees
# of this repo share the default port, so an app orphaned by a deleted
# worktree goes on listening with nothing left to stop it.
#
# Without this check the new app starts, fails to bind, exits — and every
# curl and playwright-cli call afterwards goes to the *other* process, so
# the harness reports facts about an app nobody asked for. That is not a
# quiet wrongness either: it presented as
# "no such table: libraries" against a freshly created YJ_HOME, which
# reads exactly like applySchema or staleshape.go having gone wrong and
# is a frightening place to start looking.
#
# The startup wait below cannot catch it, because the health check is
# satisfied by *any* app on the port — which is precisely the failure.
# So it is refused here, before anything is launched, rather than warned
# about. --port already exists for the legitimate second-app case.
port_holder() {
command -v ss >/dev/null || return 0
ss -lptn "sport = :$PORT" 2>/dev/null | grep -oP 'pid=\K[0-9]+' | head -n 1
}
if curl -sf -o /dev/null --max-time 2 "http://localhost:$PORT/" ||
[ -n "$(port_holder)" ]; then
holder="$(port_holder)"
echo "dev-headless: :$PORT is already in use; refusing to start" >&2
if [ -n "$holder" ]; then
# /proc/<pid>/cwd names the checkout it belongs to, and says
# "(deleted)" for the orphaned-worktree case that is the whole
# reason this is worth a check.
cwd="$(readlink "/proc/$holder/cwd" 2>/dev/null || echo unknown)"
cmd="$(tr '\0' ' ' <"/proc/$holder/cmdline" 2>/dev/null || echo unknown)"
echo " pid $holder ($cmd)" >&2
echo " cwd $cwd" >&2
# The PID-file check above has already passed, so whatever this
# is, `make dev-stop` does not know about it — saying otherwise
# sends you to a command that will report success and change
# nothing. Never `pkill -f` here either: the pattern would
# match this script's own command line.
echo " 'make dev-stop' will not touch it (it is not in" >&2
echo " ${PID_FILE#"$REPO_ROOT"/}): kill $holder, or pass --port." >&2
else
echo " The holder could not be identified (no ss, or it belongs" >&2
echo " to another user). Try: ss -lptn 'sport = :$PORT'" >&2
fi
exit 1
fi
# ── Choose the YJ_HOME ───────────────────────────────────────────────
# A seed is a YJ_HOME that a previous run of the app produced, tarred
# up (see scripts/seed-sandbox.sh). Restoring it means starting *in*
@@ -200,6 +249,20 @@ until curl -sf -o /dev/null "http://localhost:$PORT/"; do
sleep 0.25
done
# The loop above exits on the first answer from the port, and "something
# answered" is not "the app we started answered". The pre-launch guard
# makes that unlikely rather than impossible — a race, or a listener
# started in between — and the check is one signal, so it is worth making
# here too. An empty log beside a dead pid is the "it exited immediately
# and nothing said so" case that the original report spent its time on.
if ! kill -0 "$APP_PID" 2>/dev/null; then
echo "dev-headless: :$PORT answered, but the app we started (pid" >&2
echo " $APP_PID) is gone — something else holds the port." >&2
tail -n 30 "$LOG_FILE" >&2
rm -f "$PID_FILE"
exit 1
fi
cat <<EOF
dev-headless: up
url http://localhost:$PORT