From fe1fbefee7bb24406b68c581f92cb20d43cbe84e Mon Sep 17 00:00:00 2001 From: Logan Date: Thu, 20 Aug 2026 17:17:23 -0400 Subject: [PATCH 1/2] fix(player): give the seek bar's interval one owner MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `handleInput()` called `stopProgress()` and mutated no reactive state, so Lit scheduled no update, `updated()` never ran, and the tail of `updated()` that restarts the interval never executed. Only a `change` event or the next backend report could bring it back — so an `input` that never commits froze the interpolation: a drag cancelled outside the element, a pointer taken by a scroll, or a touch on the track treated as a scrub, all ordinary gestures on a phone. While playing the 1 Hz report papered over it within a second; with reports not arriving it was permanent. The drag is `@state` now and `updated()` decides whether the interval runs, so there is one place that knows. `handleChange` no longer starts it directly for the same reason. A flag set on `input` can strand, which would turn a stall of up to a second into a permanent one — the failure this removes. `change` is the ordinary end; `pointerup`/`pointercancel`/`touchend`/`touchcancel` on the document are the ends that are not, attached with the drag and dropped with it, because the pointer is routinely released outside the element it started in. The other half is that a report arriving mid-drag used to overwrite `seekValue` and pull the thumb out from under the finger once a second. It is skipped while dragging, and its seq is deliberately left unrecorded so the first report after the drag still counts as fresh. Three tests, all exercised against the fault: two fail on the old component, and the third fails if the drag flag is left set — which is the failure mode the fix introduces and the listeners exist to prevent. Verified on the device too (Chrome 113): mid-drag the bar holds its value and ignores reports, and on release it adopts the backend's real position and resumes ticking. Closes #164 --- .../audio-player/seekbar/seek-bar.ts | 82 ++++++++++++-- frontend/test/components/transport.test.ts | 105 ++++++++++++++++++ 2 files changed, 178 insertions(+), 9 deletions(-) diff --git a/frontend/src/components/audio-player/seekbar/seek-bar.ts b/frontend/src/components/audio-player/seekbar/seek-bar.ts index 5ef3ea6..85cacf4 100644 --- a/frontend/src/components/audio-player/seekbar/seek-bar.ts +++ b/frontend/src/components/audio-player/seekbar/seek-bar.ts @@ -22,6 +22,24 @@ export class SeekBar extends LitElement { @state() private seekValue: number = 0; + /** + * Whether the user is dragging the thumb right now. + * + * It is `@state` rather than a plain field because `updated()` owns + * the interval and only reactive state brings `updated()` round. A + * bare `stopProgress()` in the input handler mutated nothing, so + * nothing re-rendered, so the tail of `updated()` that restarts the + * interval never ran — and the only things that could restart it + * were a `change` event or the next backend report. Any `input` + * without a committed `change` therefore froze the interpolation: + * a drag cancelled outside the element, a pointer taken by a scroll, + * or a touch on the track treated as a scrub, which on a phone are + * ordinary gestures. While playing, the 1 Hz report papered over it + * within a second; with reports not arriving it was permanent. + */ + @state() + private dragging: boolean = false; + /** Whether the right-hand clock shows time remaining or total. */ @state() private showRemaining: boolean = true; @@ -133,6 +151,7 @@ export class SeekBar extends LitElement { override disconnectedCallback() { super.disconnectedCallback(); this.stopProgress(); + this.endDrag(); } override updated() { @@ -154,18 +173,33 @@ export class SeekBar extends LitElement { // A report for a track that is no longer loaded is stale by // definition: the change id is the only thing that distinguishes // it, since the same file can play twice in a row. + // + // A report arriving mid-drag is deliberately *not* applied: the + // thumb belongs to the finger on it, and adopting a report once a + // second pulls it back out from under them. The seq is left + // unrecorded too, so the first report after the drag still counts + // as fresh. const position = this.player.position; const forThisTrack = position !== null && position.trackChangeId === currentChangeId; - if (position && forThisTrack && position.seq !== this.previousPositionSeq) { + if ( + position && + forThisTrack && + !this.dragging && + position.seq !== this.previousPositionSeq + ) { this.previousPositionSeq = position.seq; this.seekValue = position.positionSeconds; this.stopProgress(); } - // Start/stop progress interval based on playback state - if (this.isPlaying && this.hasTrack) { + // One owner for the interval, and this is it. Every other place + // that wants it started or stopped says so by changing state that + // brings us back here, so the timer cannot be left running by a + // path that forgot to stop it or stopped by a path that forgot to + // start it again. + if (this.isPlaying && this.hasTrack && !this.dragging) { this.startProgress(); } else { this.stopProgress(); @@ -210,18 +244,48 @@ export class SeekBar extends LitElement { private handleChange(e: Event) { const newSeekVal = (e.target as WaSlider).value; + this.endDrag(); this.setSeekValue(newSeekVal); this.player.seek(newSeekVal); + } - if (this.isPlaying) { - this.startProgress(); + /** + * The user is moving the thumb. + * + * This only records that fact; `updated()` decides what it means for + * the interval. `seekValue` follows the slider so the clocks track + * the thumb during the drag rather than jumping when it is released. + */ + private handleInput(e: Event) { + this.setSeekValue((e.target as WaSlider).value); + + if (this.dragging) { + return; } + + this.dragging = true; + + // A drag that never commits must not strand the flag, or this fix + // turns a stall of up to one second into a permanent one -- which + // is the failure it exists to remove. `change` is the ordinary + // end; these are the ones that are not, and they are on the + // document because the pointer is routinely released outside the + // element it started in. A drag's listeners belong to the drag, + // so they go on with it and come off with it. + document.addEventListener('pointerup', this.endDrag); + document.addEventListener('pointercancel', this.endDrag); + document.addEventListener('touchend', this.endDrag); + document.addEventListener('touchcancel', this.endDrag); } - // Stops progress while user is dragging the thumb - private handleInput() { - this.stopProgress(); - } + private endDrag = () => { + document.removeEventListener('pointerup', this.endDrag); + document.removeEventListener('pointercancel', this.endDrag); + document.removeEventListener('touchend', this.endDrag); + document.removeEventListener('touchcancel', this.endDrag); + + this.dragging = false; + }; private setSeekValue(val: number) { if (val < 0) val = 0; diff --git a/frontend/test/components/transport.test.ts b/frontend/test/components/transport.test.ts index f503c02..7863a05 100644 --- a/frontend/test/components/transport.test.ts +++ b/frontend/test/components/transport.test.ts @@ -397,6 +397,111 @@ describe('', () => { expect(lastArgs('player.Player.Seek')).toEqual([42]); }); + // #164. `handleInput` used to call `stopProgress()` and mutate no + // reactive state, so Lit scheduled no update, `updated()` never ran, + // and the tail of `updated()` that restarts the interval never + // executed. Only a `change` or the next backend report could bring + // it back -- so an `input` that never commits froze the clock, which + // on a touch device is an ordinary cancelled gesture. With no + // reports arriving, that is permanent. + it('keeps ticking after a drag that never commits', async () => { + vi.useFakeTimers(); + + const el = await fixture('seek-bar'); + + emit(Events.TrackChanged, TRACK); + emit(Events.PlaybackStateChanged, { state: 'playing' }); + await vi.advanceTimersByTimeAsync(2000); + await el.updateComplete; + + // A touch lands on the track and is then cancelled: `input`, and + // no `change` ever follows. + const slider = shadow(el, 'wa-slider'); + + if (slider) slider.value = 20; + + slider?.dispatchEvent(new Event('input')); + await el.updateComplete; + + document.dispatchEvent(new Event('pointerup')); + await vi.advanceTimersByTimeAsync(0); + await el.updateComplete; + + await vi.advanceTimersByTimeAsync(3000); + await el.updateComplete; + + expect(text(el, '[data-testid="elapsed-time"]')).toBe('00:23'); + }); + + // The other half of the same fix: while the thumb is held, a report + // arriving once a second used to overwrite `seekValue` and pull it + // back out from under the finger. + it('leaves the thumb where the finger is while a drag is live', async () => { + vi.useFakeTimers(); + + const el = await fixture('seek-bar'); + + emit(Events.TrackChanged, { ...TRACK, trackChangeId: 20 }); + emit(Events.PlaybackStateChanged, { state: 'playing' }); + await vi.advanceTimersByTimeAsync(0); + await el.updateComplete; + + const slider = shadow(el, 'wa-slider'); + + if (slider) slider.value = 60; + + slider?.dispatchEvent(new Event('input')); + await el.updateComplete; + + emit(Events.PlaybackPositionChanged, { + positionSeconds: 4, + trackLength: 90, + trackChangeId: 20, + seq: 7, + playing: true, + }); + await vi.advanceTimersByTimeAsync(0); + await el.updateComplete; + + expect(text(el, '[data-testid="elapsed-time"]')).toBe('01:00'); + }); + + // And the drag must not hold the interval hostage once it ends: the + // report that was skipped mid-drag is not recorded as seen, so the + // next one is still fresh and is applied. + it('takes the backend back as the authority once the drag commits', async () => { + vi.useFakeTimers(); + + const el = await fixture('seek-bar'); + + emit(Events.TrackChanged, { ...TRACK, trackChangeId: 21 }); + emit(Events.PlaybackStateChanged, { state: 'playing' }); + await vi.advanceTimersByTimeAsync(0); + await el.updateComplete; + + const slider = shadow(el, 'wa-slider'); + + if (slider) slider.value = 60; + + slider?.dispatchEvent(new Event('input')); + await el.updateComplete; + + slider?.dispatchEvent(new Event('change')); + await el.updateComplete; + + emit(Events.PlaybackPositionChanged, { + positionSeconds: 61, + trackLength: 90, + trackChangeId: 21, + seq: 9, + playing: true, + }); + await vi.advanceTimersByTimeAsync(0); + await el.updateComplete; + + expect(text(el, '[data-testid="elapsed-time"]')).toBe('01:01'); + }); + it('bounds the slider by the track length', async () => { const el = await fixture('seek-bar'); From 67eeb75e7b88074d0270724302d3929a807308da Mon Sep 17 00:00:00 2001 From: Logan Date: Thu, 20 Aug 2026 17:17:33 -0400 Subject: [PATCH 2/2] docs(android): the device can be driven, not just looked at MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The runtime call does not go over HTTP on Android — the WebView cannot deliver a fetch() POST body to shouldInterceptRequest, so v3 routes runtime calls through the addJavascriptInterface bridge. Two things follow that cost an hour each before the v3 source was read: `.playwright/init-events.js` does not transfer to the device (its outbound half hooks fetch, and a POST to /wails/runtime answers "missing object value" — which reads like a wrong payload and is the interceptor getting no body at all), and hooking fetch from an eval is too late on any platform because the bundle captured its reference at module scope. The recipe that does work goes in, along with how to get audio onto the phone (scoped storage silently swallows a push into /sdcard/Android/data//files, and the fixtures are 2 seconds long, which is useless for watching a seek bar) and the permission dialog a reinstall raises, which looks exactly like the app failing to start. NOTES.md takes the #53 measurements: that its frontend is byte-identical to the v0.3.1 the phone carries, that the symptom does not reproduce on main in four scenarios, and that reverting only backend/player/ to v0.3.1 reproduces #125 instead — with the shim that makes that a ten-minute experiment rather than a full checkout. --- .../references/android-tier.md | 69 +++++++++++++++++++ .planning/NOTES.md | 60 ++++++++++++++++ 2 files changed, 129 insertions(+) diff --git a/.pi/skills/yellowjacket-dev/references/android-tier.md b/.pi/skills/yellowjacket-dev/references/android-tier.md index 62ff776..4b4afd3 100644 --- a/.pi/skills/yellowjacket-dev/references/android-tier.md +++ b/.pi/skills/yellowjacket-dev/references/android-tier.md @@ -515,6 +515,75 @@ Four things about it, each of which costs an hour if met cold: script. Plug in over USB for anything longer than a couple of probes. - **The socket name carries the pid**, which changes on every launch, so it is resolved rather than remembered. +- **A reinstall resets the runtime permissions**, and the grant dialog + is a separate activity that takes focus — so the app is up, `am start` + reports "delivered to currently running top-most instance", and + `pidof` is empty because it never got to the foreground. + `dumpsys window | grep mCurrentFocus` naming + `GrantPermissionsActivity` is the tell. `adb shell pm grant + app.yellowjacket.dev android.permission.READ_MEDIA_AUDIO` (and + `POST_NOTIFICATIONS`) ahead of the launch skips it. + +### Calling a binding on the device + +**The runtime call does not go over HTTP on Android**, and this is worth +knowing before an hour is spent on it. The WebView cannot deliver a +`fetch()` POST body to `shouldInterceptRequest`, so v3 routes runtime +calls through the `addJavascriptInterface` bridge instead: the +@wailsio/runtime installs a `customTransport` that calls +`window.wails.invokeAsync(id, payload)` and receives the answer on +`window._wailsAndroidCallback`. Two consequences: + +- **`.playwright/init-events.js` does not transfer to the device.** Its + outbound half hooks `fetch`, which sees nothing here, and its + `call()` posts to `/wails/runtime`, which answers + `Invalid runtime call: missing object value` — the interceptor got the + URL with no body. Its *inbound* half is still right, because + `dispatchWailsEvent` is the entry point in every mode. +- **Hooking `fetch` from an eval is too late anyway**, on any platform: + the bundle captured its reference at module scope, so a wrapper + installed afterwards records nothing. That is why the harness is an + `initScript` and not a step in a spec. + +What works is to borrow the bridge, chaining the runtime's own callback +so its pending calls still resolve: + +```js +const pending = new Map(); +const prev = window._wailsAndroidCallback; +window._wailsAndroidCallback = (id, response, error) => { + if (!pending.has(id)) return prev && prev(id, response, error); + const p = pending.get(id); pending.delete(id); + const env = JSON.parse(response || "{}"); + return env.ok ? p.resolve(env.data ?? env.text) : p.reject(new Error(env.error)); +}; +window.__yj = { call(name, args) { + return new Promise((resolve, reject) => { + const id = "yj" + Math.random().toString(36).slice(2); + pending.set(id, { resolve, reject }); + window.wails.invokeAsync(id, JSON.stringify({ + object: 0, method: 0, windowName: "", + args: { "call-id": id, methodName: "yellowjacket/backend/" + name, args: args || [] }, + clientId: window._wails.clientId, + })); + }); +} }; +``` + +That turns the device into a tier that can be *driven* rather than only +looked at — `__yj.call("player.Player.LoadFile", [path])` and +`__yj.call("library.Library.AddLibrary", ["/sdcard/Music/..."])` are how +#53 was measured. Names are the Go ones (`GetTracks`, not +`GetAllTracks`); an unknown one comes back as a plain +`unknown bound method name`, so a wrong guess is loud. + +**Getting audio onto the phone**: `adb push` into +`/sdcard/Android/data//files/` looks like it works and then the +files are not there — scoped storage. `/sdcard/Music/...` plus +`pm grant … READ_MEDIA_AUDIO` does work, and `AddLibrary` takes the +plain path. The generated fixtures are **~2 seconds** each, which is +fine for a scan and useless for watching a seek bar, so synthesise a +long one: `ffmpeg -f lavfi -i sine=frequency=440:duration=240`. **And the reason to bother: the phone is an engine, not a screen.** The first device here renders in **Chrome 113** at 424x439 CSS px. Every diff --git a/.planning/NOTES.md b/.planning/NOTES.md index 7c507e1..4fb55b8 100644 --- a/.planning/NOTES.md +++ b/.planning/NOTES.md @@ -4242,3 +4242,63 @@ took another ~10 s to come up. Harmless here (the emulator was up before anything used it) and a straightforward race otherwise: `start` should wait for a device that is an emulator, not for whatever `pick_device` returns. Filed as #162. + +## The Android runtime transport is not HTTP (measured 2026-08-20) + +Found while trying to drive the phone for #53. `wails3` routes runtime +calls through `addJavascriptInterface` on Android, not through +`/wails/runtime` — the WebView cannot deliver a `fetch()` POST body to +`shouldInterceptRequest`, which the v3 source says in as many words +(`application_android.go`, "The Android transport"). The runtime +installs a `customTransport` over `window.wails.invokeAsync(id, +payload)` and takes the answer on `window._wailsAndroidCallback`. + +Two things follow, and both cost time before the source was read: + +- **`.playwright/init-events.js` does not transfer to the device.** Its + outbound half hooks `fetch`; a POST to `/wails/runtime` answers + `Invalid runtime call: missing object value`, which reads like a + wrong payload shape and is actually the interceptor receiving a URL + with no body at all. The payload shape was right the whole time. Its + *inbound* half is still correct, because `dispatchWailsEvent` is the + entry point in every mode. +- **Hooking `fetch` from an `eval` is too late on any platform.** The + bundle captured its reference at module scope, so a wrapper installed + afterwards records nothing — which is exactly why the harness is an + `initScript`. Measured: zero calls captured while the app was + demonstrably making them. + +The working recipe is in `android-tier.md`; it chains the runtime's own +callback rather than replacing it, so its pending calls still resolve. +This is what makes the device a tier that can be *driven*. + +## #53's frontend is byte-identical to the build it was reported against (2026-08-20) + +`git diff v0.3.1 HEAD -- frontend/src/components/audio-player/seekbar/ +frontend/src/store/player-store.ts` is **empty**; the whole diff in that +area is `backend/player/`. The phone carries the released `v0.3.1`, so +whatever #53 saw, the component was not what changed — and v0.4.0 is +where the player audit (#122–#127) landed. + +Measured on that phone, current `main`, with a synthesised 4-minute +track: the Now Playing seek bar tracks correctly when mounted +mid-playback (`seekValue` 28 of 240), when the view is opened before +playback starts, after a tap on the track, and across an activity +recreation (same pid, bar resumes at 30 → 35). The issue's stated +symptom did not reproduce in any of them. + +Reverting **only** `backend/player/` to v0.3.1 — the frontend and +everything else at HEAD — does reproduce a real position defect on the +same device: six seconds into a 20-second file with no database row, +played after a 240-second one, the bar read **01:27 of 240**. That is +#125's stale `trackLengthMs` ("cleared only by UnloadTrack, so a file +with no row inherited the previous track's duration"), and it is fixed +at HEAD. Note the *shape* of it: the fraction is roughly right and the +absolute numbers are wrong, so it presents as a clock that lies rather +than as a handle that will not move. + +The one-line experiment is worth remembering: v0.3.1's `backend/player` +compiles against HEAD with a single shim +(`SetPlaybackFinishedHandler` gained a `srcErr error` parameter), which +makes "did the backend fix cause this" a ten-minute question instead of +a full checkout.