Six small, independent fixes claimed off the tracker, each on its own
branch and preserved here as a merge — the shape #83 used, so authorship
survives and the batch lands as one release rather than six.
Commits to issues
Commit
Issue
What
fix(scripts): let issue.sh claim work on a write:issue-only token
#130 no longer reproduces on this machine, because the token has
since been re-issued with read:user — the issue's option 1. The fix is
still option 2, which the issue argues for: the script should not demand
a scope it needs for one lookup, and claim now works on the least
privilege the job actually needs.
#131's check is a snapshot, not a path list. The issue offered both;
a snapshot keeps working when a fourth generator is added, and a path
list is a second place to remember it. Two things it has to get right
are in the commit message — the comparison is a symmetric difference
(a hand-edited generated file the generator puts back is stale too, and
one direction reports it as current), and the snapshot is content rather
than names.
#118 is latent. Nothing in the tree writes in_library = 1 with a
NULL local id today; the exposure is an older database and the next
writer that does. The test seeds it with raw SQL deliberately — upsertBatch writes a zero id as literal 0, which satisfies IS NOT NULL, so a fixture built through the upsert cannot reproduce
this at all.
Verification
Run on this branch after rebasing onto current main (it was
originally cut from a main four merges stale, including #132's shell
and queue-model changes):
make lint — 0 issues, all three build configurations
make test — pass, all three configurations
make bindings-check, make skill-check — current
tsc --noEmit — clean
make ui-test — 910 passed
make e2e — 127 passed (chromium; WebKit is CI-only)
Each fix was also reverted and re-run to confirm its test fails
without it: #118 fails on all three entity types against the old gate, #68 and #61 fail against the old markup.
#68 and #61 were additionally verified against the real running app,
because no test tier can see a hover: media query — CDP's Emulation.setEmulatedMedia does not reach ui-test's iframe, and
e2e's projects are all desktop, so its phone specs run with hover: hover. Under a devices['Pixel 5'] context the play button
computes display: none against flex at 1440px, and the mini player
renders 0 .explore-links against 2, with the title and source text
intact. That limitation is written into .planning/NOTES.md rather than
left in a commit message.
Filed, not fixed
#137 — the sweep for #68 found three other hover-only
affordances (queue remove, cover-art edit, shortcut reset) where the
correct fix is the opposite: always visible on touch, since hiding
them removes the only route to the action. Sequenced after #60.
#138 — play-in-context.test.ts flakes ~1 in 3 full make ui-test
runs on pristine main. It failed a pre-push hook on the codegen-check fails on any unstaged change, and blames the code generators (#131)
branch, which contains no frontend changes at all, which is how it was
identified as pre-existing. Unrelated to this PR, but it can fail CI
here.
Both touch home-view.ts and .planning/NOTES.md. The code merges
cleanly — #136 changes render(), this changes static styles. Only
the NOTES.md append conflicts, and it is a trivial both-appended-at-EOF
resolution whichever lands second.
Six small, independent fixes claimed off the tracker, each on its own
branch and preserved here as a merge — the shape #83 used, so authorship
survives and the batch lands as one release rather than six.
## Commits to issues
| Commit | Issue | What |
|---|---|---|
| `fix(scripts): let issue.sh claim work on a write:issue-only token` | #130 | `me()` prefers `GITEA_USER`, falling back to `GET /user` |
| `fix(scripts): report only what generation changed, not the worktree` | #131 | New `scripts/codegen-check.sh`; the hook calls it |
| `fix(scripts): refuse to start dev-headless on a port somebody else holds` | #119 | Pre-launch port guard naming the holder |
| `fix(explore): clear in_library on a row that has no local id` | #118 | Prune gated on the flag *or* the id |
| `fix(home): gate the card play button on the device having hover` | #68 | `(hover: hover) and (pointer: fine)` |
| `feat(now-playing): plain text instead of links in the phone mini player` | #61 | Links become text below 600px |
| `docs(notes): record that no test tier can see a hover media query` | — | Measured fact from #68 |
## Notes on three of them
**#130 no longer reproduces on this machine**, because the token has
since been re-issued with `read:user` — the issue's option 1. The fix is
still option 2, which the issue argues for: the script should not demand
a scope it needs for one lookup, and `claim` now works on the least
privilege the job actually needs.
**#131's check is a snapshot, not a path list.** The issue offered both;
a snapshot keeps working when a fourth generator is added, and a path
list is a second place to remember it. Two things it has to get right
are in the commit message — the comparison is a *symmetric* difference
(a hand-edited generated file the generator puts back is stale too, and
one direction reports it as current), and the snapshot is content rather
than names.
**#118 is latent.** Nothing in the tree writes `in_library = 1` with a
NULL local id today; the exposure is an older database and the next
writer that does. The test seeds it with raw SQL deliberately —
`upsertBatch` writes a zero id as literal `0`, which satisfies
`IS NOT NULL`, so a fixture built through the upsert cannot reproduce
this at all.
## Verification
Run on this branch **after rebasing onto current `main`** (it was
originally cut from a `main` four merges stale, including #132's shell
and queue-model changes):
- `make lint` — 0 issues, all three build configurations
- `make test` — pass, all three configurations
- `make bindings-check`, `make skill-check` — current
- `tsc --noEmit` — clean
- `make ui-test` — 910 passed
- `make e2e` — 127 passed (chromium; WebKit is CI-only)
Each fix was also **reverted and re-run** to confirm its test fails
without it: #118 fails on all three entity types against the old gate,
#68 and #61 fail against the old markup.
**#68 and #61 were additionally verified against the real running app**,
because no test tier can see a `hover:` media query — CDP's
`Emulation.setEmulatedMedia` does not reach `ui-test`'s iframe, and
e2e's projects are all desktop, so its phone specs run with
`hover: hover`. Under a `devices['Pixel 5']` context the play button
computes `display: none` against `flex` at 1440px, and the mini player
renders 0 `.explore-link`s against 2, with the title and source text
intact. That limitation is written into `.planning/NOTES.md` rather than
left in a commit message.
## Filed, not fixed
- **#137** — the sweep for #68 found three *other* hover-only
affordances (queue remove, cover-art edit, shortcut reset) where the
correct fix is the opposite: always visible on touch, since hiding
them removes the only route to the action. Sequenced after #60.
- **#138** — `play-in-context.test.ts` flakes ~1 in 3 full `make ui-test`
runs **on pristine `main`**. It failed a `pre-push` hook on the #131
branch, which contains no frontend changes at all, which is how it was
identified as pre-existing. Unrelated to this PR, but it can fail CI
here.
## Interaction with #136
Both touch `home-view.ts` and `.planning/NOTES.md`. The code merges
cleanly — #136 changes `render()`, this changes `static styles`. Only
the NOTES.md append conflicts, and it is a trivial both-appended-at-EOF
resolution whichever lands second.
Closes #130
Closes #131
Closes #119
Closes #118
Closes #68
Closes #61
`claim` is the one step the workflow requires before the first edit, and
it failed outright on a token scoped to the work it does: `me()` calls
`GET /user` purely to name the assignee, and that endpoint needs
read:user. So the documented process was blocked by its own tooling, and
the fallback was to do the assignment, the label and the comment by hand
— which is the half-made claim `claim` exists to prevent.
GITEA_USER short-circuits the lookup, so least privilege is enough. The
lookup stays as the fallback because it is right when the scope is there
and needs no setup. Failure is now actionable and says both remedies,
and it still happens before any of the three halves are mutated.
Closes#130
The codegen-check hook was `go generate` followed by a bare
`git diff --name-only`, which is the whole unstaged worktree rather than
the generators' output. So a commit whose staged changes were fine failed
whenever anything unrelated sat unstaged — notes, a plan document, the
next commit's files — reporting "Generated code is out of date" and then
a diffstat of files no generator has ever written. `make generate` fixed
nothing, because nothing was stale, so the message sent you looking for a
codegen problem that did not exist. Splitting one piece of work into
several commits is exactly the shape that triggers it.
The tree is snapshotted either side of `go generate` and only what moved
across it is reported. That is deliberately a snapshot rather than the
list of generated paths the issue offers as the other option: a fourth
generator is one //go:generate line away, and a path list is a second
place to remember it.
Two things it has to get right. The comparison is a *symmetric*
difference, because generation can push a file into the unstaged set or
pull it out of one — a hand-edited generated file that the generator puts
back is stale generated code just as much as a source change that
outdates it, and comparing one direction reports it as current. And the
snapshot is content, not names, or a generated file that was already
dirty and is then rewritten further keeps its name on both sides and
slips through.
Closes#131
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
`in_library = 1 AND local_*_id IS NULL` was a fixed point.
upsertBatch's conflict clause is `MAX(in_library, excluded.in_library)`,
so it can only ever raise the flag, and pruneStaleLocalCrossReferences —
which its own comment calls the only place a removal from the library is
reflected back into the index — was gated on the id being present. So
nothing in the app could clear such a row, ever: a permanent claim of
ownership with no local row to check it against.
The gate is now the flag *or* the id, for all three entity types. A NULL
id fails the existence test on its own, so this needs no second clause to
say what "not owned" means.
Nothing in the tree writes that shape today — collectLibraryEntities sets
both together — which is why this is worth closing rather than leaving:
the exposure is a database written by a version whose local-id columns
were populated differently, and the next writer that sets the flag
without an id, which nothing structurally prevents and which this shape
made permanent rather than merely wrong until the next scan.
The test seeds the row with raw SQL on purpose. upsertBatch writes a zero
LocalArtistID as literal 0, and 0 satisfies `IS NOT NULL`, so the old
gate already caught that shape — a fixture built through the upsert
cannot reproduce this at all. NULL is what the artifact importer and any
older writer leave behind, the columns being nullable with no default.
Reverted against the old gate, it fails on all three types.
Closes#118
The play button on a home shelf's cover cards is revealed by :hover, and
a touch long-press synthesises a hover state in the WebView — so on a
phone it flashed into view during the 500ms hold that
utils/long-press.ts is measuring for a context menu. A control appearing
because the user was reaching for a different one.
It is gated on `(hover: hover) and (pointer: fine)` rather than on width,
so it is absent on any touch device and present on a desktop with a small
window. A phone user taps the album and plays from the detail view, so
nothing replaces it.
The default outside the query is display:none, not opacity:0. An
opacity-0 button still takes taps and is still in the accessibility tree,
so leaving the reveal as the only guarded part would keep the hit area
for a control the phone can never show.
The test asserts the parsed stylesheet rather than rendering as a phone,
and says so: CDP's Emulation.setEmulatedMedia does not reach this tier's
iframe, so matchMedia still answers `hover: hover` after it is set. The
regression worth catching is someone hoisting the rule back out of the
query as a tidy-up — a change no desktop assertion can see.
Closes#68
The bottom bar's title, artist and "Playing from X" all navigate. In a
bar sized for a bar they are a few characters of text, which is not a
touch target — and explore-link holds its navigation for one
double-click interval and drops it if a second click arrives, a gesture
that exists so double-clicking a row can play it and that means nothing
on touch.
Below the shell's phone breakpoint the three render as plain text. The
words are unchanged: the source line still says where the queue came
from, because dropping the link is the change and dropping the
information would be a different and worse one. The cover art already
carries the phone-only button that opens the full-screen Now Playing
view, which is where the links live.
This is in JS rather than in the stylesheet because what changes is the
content, not its appearance — no CSS rule takes a click handler off an
element. matchMedia is read in connectedCallback for the reason the
reduce-motion query beside it already is, so a test can answer it first.
Two smaller things. PHONE_QUERY moves out of track-list.ts into
utils/breakpoints.ts: it was a private const when one component needed
it, and a second reader is where a copy starts drifting from index.css.
And `phone` joins geometryKey(), because crossing the breakpoint swaps a
link for a bare string and the marquee travels a distance read from
measuring it — the words being identical either side is not the same as
the box measuring the same.
Closes#61
Both browser tiers are blind to `(hover: hover)` gating, in different
ways and without failing: CDP media emulation does not reach ui-test's
iframe, and e2e's phone specs reach phone width with setViewportSize,
which changes no media feature but width. Written down with what does
work — a device-descriptor context — because the next person to gate an
affordance this way will otherwise re-derive it, and the tempting
conclusion from a green suite is that the gate is covered.
logan
merged commit cceeb40b16 into main2026-08-19 19:08:46 +00:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Six small, independent fixes claimed off the tracker, each on its own
branch and preserved here as a merge — the shape #83 used, so authorship
survives and the batch lands as one release rather than six.
Commits to issues
fix(scripts): let issue.sh claim work on a write:issue-only tokenme()prefersGITEA_USER, falling back toGET /userfix(scripts): report only what generation changed, not the worktreescripts/codegen-check.sh; the hook calls itfix(scripts): refuse to start dev-headless on a port somebody else holdsfix(explore): clear in_library on a row that has no local idfix(home): gate the card play button on the device having hover(hover: hover) and (pointer: fine)feat(now-playing): plain text instead of links in the phone mini playerdocs(notes): record that no test tier can see a hover media queryNotes on three of them
#130 no longer reproduces on this machine, because the token has
since been re-issued with
read:user— the issue's option 1. The fix isstill option 2, which the issue argues for: the script should not demand
a scope it needs for one lookup, and
claimnow works on the leastprivilege the job actually needs.
#131's check is a snapshot, not a path list. The issue offered both;
a snapshot keeps working when a fourth generator is added, and a path
list is a second place to remember it. Two things it has to get right
are in the commit message — the comparison is a symmetric difference
(a hand-edited generated file the generator puts back is stale too, and
one direction reports it as current), and the snapshot is content rather
than names.
#118 is latent. Nothing in the tree writes
in_library = 1with aNULL local id today; the exposure is an older database and the next
writer that does. The test seeds it with raw SQL deliberately —
upsertBatchwrites a zero id as literal0, which satisfiesIS NOT NULL, so a fixture built through the upsert cannot reproducethis at all.
Verification
Run on this branch after rebasing onto current
main(it wasoriginally cut from a
mainfour merges stale, including #132's shelland queue-model changes):
make lint— 0 issues, all three build configurationsmake test— pass, all three configurationsmake bindings-check,make skill-check— currenttsc --noEmit— cleanmake ui-test— 910 passedmake e2e— 127 passed (chromium; WebKit is CI-only)Each fix was also reverted and re-run to confirm its test fails
without it: #118 fails on all three entity types against the old gate,
#68 and #61 fail against the old markup.
#68 and #61 were additionally verified against the real running app,
because no test tier can see a
hover:media query — CDP'sEmulation.setEmulatedMediadoes not reachui-test's iframe, ande2e's projects are all desktop, so its phone specs run with
hover: hover. Under adevices['Pixel 5']context the play buttoncomputes
display: noneagainstflexat 1440px, and the mini playerrenders 0
.explore-links against 2, with the title and source textintact. That limitation is written into
.planning/NOTES.mdrather thanleft in a commit message.
Filed, not fixed
affordances (queue remove, cover-art edit, shortcut reset) where the
correct fix is the opposite: always visible on touch, since hiding
them removes the only route to the action. Sequenced after #60.
play-in-context.test.tsflakes ~1 in 3 fullmake ui-testruns on pristine
main. It failed apre-pushhook on the codegen-check fails on any unstaged change, and blames the code generators (#131)branch, which contains no frontend changes at all, which is how it was
identified as pre-existing. Unrelated to this PR, but it can fail CI
here.
Interaction with #136
Both touch
home-view.tsand.planning/NOTES.md. The code mergescleanly — #136 changes
render(), this changesstatic styles. Onlythe NOTES.md append conflicts, and it is a trivial both-appended-at-EOF
resolution whichever lands second.
Closes #130
Closes #131
Closes #119
Closes #118
Closes #68
Closes #61