Commit Graph
1179 Commits
Author SHA1 Message Date
Tobias GesellchenandClaude Sonnet 5 4478863d7c test(service): update router route snapshot for the new zone candidates endpoint
TestPrintRoutes compares against a checked-in route list; the new
GET .../zone/candidates route (added for Zone.js's candidate source
fix) needs to be reflected there too.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-30 22:27:56 +02:00
Tobias GesellchenandClaude Sonnet 5 862ddbb87d fix(player): follow Library's selected device to its pair master
If the device selected in the Library tab disappeared from the
devices map (e.g. it just became a hidden stereo-pair member per
device_projection.go), the sync effect fell back to entries[0][0] --
whichever key happens to sort first in the map -- silently redirecting
the user's Library browsing session to an unrelated speaker.

Now checks first whether the vanished device reappears as a member of
some other device's stereoPair (the pair's master, which now
represents the same physical speaker for control purposes) and
follows it there. Only falls back to an arbitrary device when the
selection is gone for a genuinely unrelated reason (removed, discovery
gap), matching the prior behavior for that case.

No JS unit-test framework exists in this repo for component-level
logic (consistent with the rest of the client-side code), so this is
verified by manual trace rather than an automated regression test.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-30 22:27:56 +02:00
Tobias GesellchenandClaude Sonnet 5 78c6c1bfca fix(player): make Zone.js source zone candidates independently of Group projection
Zone.js derived its "add speaker" candidate list from the `devices`
prop, which is app.js's projected/collapsed device list. Once the
stereo-pair projection (device_projection.go) started hiding a pair's
non-master member from that list, it silently became impossible to
add that physical device to an unrelated multiroom zone, even though
the backend's HandleZoneAdd/HandleZoneRemove already operate on the
raw device registry directly and never cared about pairing at all.
Zone.js's own file wasn't touched by that change; its effective input
just changed underneath it.

Added GET /api/control/devices/{id}/zone/candidates, deliberately
bypassing deviceViewSnapshot's projection and deliberately not
excluding {id} itself -- which candidates to exclude is a caller
concern (Zone.js already does this via the existing zoneIps set,
which includes the zone master's own IP even when standalone, per
models.ZoneInfo.IsStandalone). Zone and Group are separate, unrelated
groupings and should stay that way.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-30 22:27:56 +02:00
Tobias GesellchenandClaude Sonnet 5 522c6b8cb6 fix(models): make group-equality order-insensitive everywhere
sameGroupClaim (device_projection.go, used to validate a member's
claim agrees with the master's) compared roles via a device-ID-keyed
map, making it order-insensitive. webtypes.replaceGroup's change
detection used reflect.DeepEqual on the whole *Group, which is
order-sensitive for Roles.Roles. Both the polled /getGroup response
and the pushed groupUpdated event populate Roles.Roles directly from
XML unmarshaling in wire order, so nothing guarantees a pair's roles
list in the same order across two reads -- DeepEqual could then report
a spurious "changed" for a pair that didn't actually change.

Extracted the order-insensitive comparison into models.SameGroup as
the single shared implementation (also handles the nil/nil case
correctly, unlike the old sameGroupClaim, which mattered for
replaceGroup's existing "no prior group" path). Both call sites now
use it; the duplicate sameGroupClaim is gone.

Added TestApplyGroupEventIgnoresRoleOrder, verified to fail against
the prior DeepEqual-based logic and pass with this fix.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-30 22:27:56 +02:00
Tobias GesellchenandClaude Sonnet 5 9629d3e057 fix(player): don't mark a device connected on GetGroup success alone
statusUpdated (which drives IsConnected) ORed in
"stereoCapable && groupErr == nil" alongside the five substantive
status fetches. Since GetGroup is gated to stereo-capable models and
trivially succeeds even when a device is struggling (an empty
<group/> is a near-guaranteed reply, per Client.GetGroup's doc
comment), a round where NowPlaying/Volume/Presets/Sources/Bass all
fail but GetGroup alone succeeds would still report the device
connected -- masking a real status-refresh failure specifically on
ST10 hardware.

Removed the extra OR term entirely: IsConnected now depends only on
the five substantive fetches, matching the comment's own stated
intent ("mirrors prior behaviour"). GetGroup's own success/failure
still drives whether Group gets refreshed (unchanged, see
ApplyPolledGroup below), just no longer feeds the connectivity signal.

Added TestUpdateDeviceStatusNotConnectedWhenOnlyGroupSucceeds,
verified to fail against the prior logic and pass with this fix.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-30 22:27:56 +02:00
Tobias GesellchenandClaude Sonnet 5 b180796ed4 docs(client): capture exact ST20 /getGroup failure mode and supportedURLs caveat
Refines the previous commit's doc fix with more precise, hardware-
verified detail: the ST20 doesn't just silently drop the connection --
its own firmware ("AllegroWebserver") eventually returns an explicit
"AllegroWebserver timeout: /getGroup" plain-text error after an
internal delay of several+ seconds, well past what client.get()'s
timeout will tolerate.

Also documents a dead-end a future contributor might otherwise try:
the ST20's own /supportedURLs response lists /getGroup (and the other
group endpoints) despite not actually servicing it, confirmed against
the same real hardware. A supportedURLs-based capability probe would
not have caught this either -- the model-name check has to stay.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-30 22:27:56 +02:00
Tobias GesellchenandClaude Sonnet 5 68d8719be0 docs(client): correct GetGroup's non-ST10 behavior claim
The exported GetGroup doc comment claimed non-ST10 devices reply to
/getGroup "harmlessly" with an empty group. Verified against real
hardware this is wrong: a SoundTouch 20 does not reply at all -- the
request hangs until the client's own timeout (10-30s depending on how
the Client was constructed) instead of returning quickly. Confirmed
by direct request against a real ST20 (curl, 8s timeout, zero bytes
back) and cross-checked against two actively-paired real ST10 units,
which both replied in ~30-40ms with full group data.

This matters beyond prose accuracy: the newer stereoPairCapable gate
in websocket.go's UpdateDeviceStatus is load-bearing, not an
optimization. A future contributor trusting the old (wrong, and more
prominent/exported) doc could reasonably "simplify" by removing that
gate, reintroducing a 10-30s hang on every poll cycle for every
SoundTouch 20/30 on the network. Rewrote the doc to state the real
behavior and point at the gate that depends on it; the websocket.go
comment now defers to this doc instead of independently (and
incorrectly worded) restating it.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-30 22:27:56 +02:00
Lukáš Lipinský 342cd47e6a fix(player): snapshot device timestamps safely 2026-08-30 22:27:56 +02:00
Lukáš Lipinský 3b04a6eb57 fix(player): skip stereo polling on unsupported models 2026-08-30 22:27:56 +02:00
Lukáš Lipinský ce66b103b4 fix(player): keep stereo updates coherent and compatible 2026-08-30 22:27:56 +02:00
Lukáš Lipinský 2f2d886e24 docs(player): mark stereo pair projection implemented 2026-08-30 22:27:56 +02:00
Lukáš Lipinský b01ab1e1bb fix(player): project stereo pairs as logical devices 2026-08-30 22:27:56 +02:00
Tobias GesellchenandClaude Sonnet 5 79eb5dd038 docs(player): restore the async=false rationale and its regression test
PR #664 rewrote the comment explaining why the dynamically-inserted
es-module-shims script sets async = false, replacing the actual
execution-order reasoning (it must run before the deferred
type="module" script below, without blocking the parser for browsers
that never reach this branch) with a vaguer, inaccurate description
("inspects module graphs that fail static linking") that doesn't
explain the async choice at all. It also dropped the regression test
asserting .async = false is present, so a future "cleanup" removing
that line would go uncaught -- and the misleading comment no longer
warns against doing so.

The actual .async = false code was untouched by #664; this only
restores the documentation and its test coverage.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-30 21:05:02 +02:00
Lukáš Lipinský d3893dff76 chore(player): tighten shim dependency handling 2026-08-30 21:05:02 +02:00
Tobias GesellchenandClaude Sonnet 5 14437fd568 fix(example-dlna-server): sanitize logged request values
CodeQL alert 313 (go/log-injection). The access-log middleware logged
r.URL.Path and SOAP-body-derived objectID/browseFlag verbatim, without
stripping newlines -- an attacker-controlled request could inject fake
log lines or control characters. Add the same sanitizeLog helper this
repo already uses in ~18 other packages for exactly this class of
finding.

Alert 312 (go/reflected-xss, same file/area) was investigated and left
open deliberately: objectID is only ever used as a lookup key in
pkg/dlna/dlnatest, never echoed into the response, and every actual
output field goes through xmlEsc/xmlAttr (encoding/xml.EscapeText)
before being written -- looks like a CodeQL false positive rather than
a real gap, but not dismissing it yet per discussion.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-29 23:19:32 +02:00
Tobias GesellchenandClaude Sonnet 5 966214c5a0 fix(player): use hasOwnProperty for the device-status guard
CodeQL alert 318 (js/remote-property-injection). setDevices guarded
the status_update write with a plain "!prev[msg.deviceId]" truthy
check; a deviceId of "__proto__" or "constructor" resolves through
the prototype chain to a truthy value, so it would pass the guard
despite not being a real known device, letting the spread write a
bogus own-property (not actual prototype pollution -- computed keys
in object literals use [[DefineOwnProperty]], not the legacy __proto__
setter -- but still corrupts the rendered device list). Use
Object.prototype.hasOwnProperty.call for a real own-property check;
avoided Object.hasOwn (ES2022, Safari 15.4+) given #649's recent
Safari-15.0 compatibility work.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-29 23:19:32 +02:00
Tobias GesellchenandClaude Sonnet 5 ed09141175 fix(player): remove leftover debug console.log
CodeQL alert 319 (js/log-injection). This logged the WebSocket
discovery_status payload verbatim to the browser console under a
"[DEBUG_LOG]" tag -- development-only cruft left in, not something
that serves any product purpose.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-29 23:19:32 +02:00
Tobias GesellchenandClaude Sonnet 5 3dbf4bcd7e fix(admin-ui): remove unused session variable
CodeQL alert 320 (js/unused-local-variable). The interactions table
never had a session column; i.session/i.Session was extracted but
never referenced anywhere in the row template.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-29 23:19:32 +02:00
Tobias GesellchenandClaude Sonnet 5 cff94d5d96 fix(bmx): correct misleading docs, restore TuneIn token request validation
HandleTuneInToken's docstring described the minted token as "fresh",
but datastore.GenerateSerialSecret("tunein") is a pure function of a
hardcoded literal -- it returns the identical value for every device
and every call, not a per-session secret. Correct the framing and
document why the constant value is safe today (Authorization gate
disabled for all TuneIn handlers, nothing validates uniqueness), so a
future change relying on per-device uniqueness doesn't get misled.

Also restore request-body validation dropped when the handler stopped
using the body's values: a genuine bootstrap call is still well-formed
JSON (confirmed against a captured real request), just with an empty
refresh_token, so decoding-but-discarding the body still rejects only
truly malformed requests with 400, without reintroducing the original
echo bug.

Also fixes 4 pre-existing wsl_v5 lint findings in the reordered
children-check in bmx/tunein.go (whitespace only, no behavior change).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-29 17:46:52 +02:00
YongXin 22d77c23f3 add TestTuneInSectionsAshx_UntypedContainerSurfacesStations 2026-08-29 17:46:52 +02:00
Will c1342360e0 Update test 2026-08-29 17:46:52 +02:00
Will 03c42d1909 Fix TuneIn browse can't be played 2026-08-29 17:46:52 +02:00
Will 255023702e update 2026-08-29 17:46:52 +02:00
Will 1c7f32e736 Fix the bug of the child not showing 2026-08-29 17:46:52 +02:00
Tobias GesellchenandClaude Sonnet 5 734b921ac1 ci: exclude vendored static JS libs from CodeQL analysis
es-module-shims.js (vendored verbatim from npm) tripped 3 CodeQL
findings (js/incomplete-sanitization, js/bad-code-sanitization x2) --
real escaping-order bugs in the library's own source, verified by hand,
but not reachable in how this project uses it (no dynamic import()
built from untrusted input, no CSP nonce ever set). Reported upstream
separately.

The javascript-typescript CodeQL matrix entry had no path exclusions at
all, unlike the existing Go config's paths-ignore for vendor/generated
code, so preact.module.js and htm.module.js were exposed to the same
risk even though neither had tripped a finding yet. Add a JS-specific
config excluding pkg/service/soundtouchweb/static/lib/** -- we don't
control or modify these files, so findings there aren't actionable
from this repo.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-29 17:09:36 +02:00
Tobias GesellchenandClaude Sonnet 5 de30aa8d7f ci(player): add a separate workflow for browser compatibility tests
Runs the opt-in "make test-browser" chromedp tests (added in the
previous commit) on their own, independent of ci.yml's main test job,
since they need a Chrome/Chromium binary in the runner. Mirrors
ci.yml's checkout/setup-go/cache steps and pinned action versions, and
verifies google-chrome is present with a clear error message before
running, rather than surfacing a cryptic chromedp allocator failure if
the runner image ever stops shipping it preinstalled.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-29 17:09:36 +02:00
Tobias GesellchenandClaude Sonnet 5 efcb96998f test(player): add browser-level compatibility tests via chromedp
Adds two opt-in tests (build tag "browsertest", run via `make
test-browser`) that drive a real headless Chrome instead of only
asserting on the raw HTML/JS source:

- TestPlayerRendersNatively confirms the shipped page still renders
  normally and never injects es-module-shims on a browser with native
  import map support.
- TestPlayerRendersUnderForcedShimMode forces es-module-shims into its
  own shimMode (importmap-shim/module-shim, per the library's docs),
  routing the real app.js and vendored dependencies through the
  library's actual polyfill resolution. This exercises the old-Safari
  code path directly in CI/local headless Chrome, without needing
  physical iPadOS 15 hardware.

Not wired into `test`/`check`/CI yet, since chromedp has not
previously been exercised as part of this repo's test suite (only in
the standalone doc-screenshot tool) and needs a Chrome/Chromium binary
available to the runner.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-29 17:09:36 +02:00
Tobias GesellchenandClaude Sonnet 5 966d9962a5 fix(player): feature-detect es-module-shims instead of always loading it
Loading es-module-shims unconditionally would charge every browser an
~80KB uncompressed download on every page view, including the vast
majority that already support import maps natively -- the static
asset server here applies no compression. Feature-detect
HTMLScriptElement.supports('importmap') instead, so only a browser
that actually lacks support ever fetches it.

Insert the script via the DOM with async = false rather than
document.write: document.write is deprecated and subject to browser
interventions that can silently drop externally-sourced scripts it
injects, where DOM insertion with async explicitly disabled gives the
same before-the-deferred-module-script execution guarantee without
that risk.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-29 17:09:36 +02:00
Tobias GesellchenandClaude Sonnet 5 6b2c3b5c7c fix(player): use es-module-shims instead of removing import maps
Restores the import map and bare-specifier imports across all
components, and instead polyfills import map support for Safari on
iPadOS 15 (which has ES modules but not import maps) via
es-module-shims, loaded unconditionally since it detects native
support and no-ops there.

The previous approach converted every component to relative imports
through a dependencies.js facade and permanently sed-patched the
vendored preact-hooks build, baking the compatibility workaround into
the whole codebase instead of keeping it encapsulated in index.html.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-29 17:09:36 +02:00
Lukáš Lipinský 6752f71e67 refactor(player): centralize static dependencies 2026-08-29 17:09:36 +02:00
Lukáš Lipinský c97e9958f6 fix(player): support browsers without import maps 2026-08-29 17:09:36 +02:00
Tobias GesellchenandClaude Sonnet 5 35724bcffc test(service): fix flaky serialization test by filtering non-/info requests
The handler counted every request regardless of path, so the background
status-update goroutine AddDeviceByHost spawns after a successful probe
could land before the assertion and be mistaken for a second concurrent
seed probe, occasionally failing with request count 2 instead of 1. Filter
by path like the existing TestDiscoverDevicesRetriesConfiguredHosts test.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-29 16:14:56 +02:00
Tobias GesellchenandClaude Sonnet 5 a21b4d71ae fix(service): address code-review findings on PR #652's startup retry
Fixes correctness issues found reviewing the bounded device-seed retry
loop before merging: a datastore read failure could make the readiness
check trivially pass; stale-host pruning only considered hosts inserted
in the current attempt and only ran inside the retry loop, not the
plain SeedExtraDevices path; the retry loop and a devices-changed-hook
seed could probe the same offline host concurrently; and a zero-change
startup window silently dropped the previously-unconditional device-list
broadcast. Also makes the retry interval/window configurable instead of
hardcoded, following the existing discovery-interval flag pattern.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-29 16:14:56 +02:00
Lukáš Lipinský 0d15bce96f fix(service): retry persisted player devices after startup 2026-08-29 16:14:56 +02:00
dependabot[bot] 719cc446e6 ci(deps): bump the codeql-action group with 3 updates
Bumps the codeql-action group with 3 updates: [github/codeql-action/init](https://github.com/github/codeql-action), [github/codeql-action/analyze](https://github.com/github/codeql-action) and [github/codeql-action/upload-sarif](https://github.com/github/codeql-action).


Updates `github/codeql-action/init` from 4.37.7 to 4.37.8
- [Release notes](https://github.com/github/codeql-action/releases)
- [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md)
- [Commits](https://github.com/github/codeql-action/compare/ff2f1c621b7f889edc0d3c761ac2e6a3f8cdb0dd...db488ddef3bf6cb639b32c2e9a7c0a7ea8271d28)

Updates `github/codeql-action/analyze` from 4.37.7 to 4.37.8
- [Release notes](https://github.com/github/codeql-action/releases)
- [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md)
- [Commits](https://github.com/github/codeql-action/compare/ff2f1c621b7f889edc0d3c761ac2e6a3f8cdb0dd...db488ddef3bf6cb639b32c2e9a7c0a7ea8271d28)

Updates `github/codeql-action/upload-sarif` from 4.37.7 to 4.37.8
- [Release notes](https://github.com/github/codeql-action/releases)
- [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md)
- [Commits](https://github.com/github/codeql-action/compare/ff2f1c621b7f889edc0d3c761ac2e6a3f8cdb0dd...db488ddef3bf6cb639b32c2e9a7c0a7ea8271d28)

---
updated-dependencies:
- dependency-name: github/codeql-action/init
  dependency-version: 4.37.8
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: codeql-action
- dependency-name: github/codeql-action/analyze
  dependency-version: 4.37.8
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: codeql-action
- dependency-name: github/codeql-action/upload-sarif
  dependency-version: 4.37.8
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: codeql-action
...

Signed-off-by: dependabot[bot] <support@github.com>
2026-08-25 20:47:35 +02:00
Tobias GesellchenandClaude Sonnet 5 b10e6dfe8f docs(troubleshooting): add entry for reboot preset wipe on shared-account setups
Closes the loop on #614: presets wiped after a reboot when a speaker
shares its Marge account with other devices. Root cause stays
unconfirmed (firmware-internal), but removing the other devices from
the account is a reporter-confirmed workaround.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-25 20:34:16 +02:00
Tobias GesellchenandClaude Sonnet 5 44830790b8 test(player): adopt httptest.NewTestServer (Go 1.27) in the new discovery test
NewTestServer registers its own t.Cleanup(Close) instead of needing a
manual defer, and fails the test on a handler panic instead of just
logging it. It defaults to an in-memory transport reachable only via
Server.Client(), which wouldn't work here since our production
client.NewClient dials a real address rather than using that client --
calling Start() instead of Client() opts back into a real loopback
listener, identical to the old NewServer, confirmed by reading the
actual go1.27.0 source (server.go's Start implementation).

This is a proactive adoption of a new stdlib idiom, not one of the
review findings from the previous commit; it doesn't change the
goroutine-drain fix from that commit, which is a separate concern
Close()'s "wait for outstanding requests" guarantee doesn't fully
cover (a goroutine that hasn't started its request yet at Close() time
isn't "outstanding").

Verified: 10x -count re-run under -race, full suite + lint clean.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-23 22:00:11 +02:00
Tobias GesellchenandClaude Sonnet 5 df62073ab0 fix(player): address code review findings on #644
Five findings from reviewing gesellix/Bose-SoundTouch#644
(Retry configured player devices during discovery):

1. Source-labeling used exact string equality
   (device.DiscoveryMethod == "Configuration"), which breaks once a
   configured host is also found via mDNS/UPnP in the same sweep:
   mergeDeviceData concatenates methods into e.g.
   "Configuration+mDNS/Bonjour", so the check silently failed and the
   device got labeled "discovered" instead of "manual". Extracted the
   decision into classifySource() and switched to a substring match.
   Added TestClassifySource, which fails against the old exact-equality
   logic on exactly the composite-string cases (verified) and would
   have caught this before merge -- the PR's own test disables
   mDNS/UPnP, so it never exercised this path.

2. Manually configured devices no longer registered immediately at
   startup -- they now wait for the full mDNS/UPnP sweep (up to the
   10s discovery timeout) to complete, since the PR removed the
   synchronous registration loop and relies entirely on
   PreferredDevices. Restored the immediate loop alongside (not
   instead of) folding manualHosts into PreferredDevices, so a
   currently-online configured device registers immediately as
   before, while an offline one still gets retried on every
   subsequent discovery pass -- the actual value this PR adds.

3. The new PreferredDevices-seeding loop didn't dedupe against hosts
   already loaded from PREFERRED_DEVICES, so setting both for the same
   host produced duplicate entries. Currently harmless (absorbed by
   AddDeviceByHost's fast path) but fragile. Added dedup by host.

4. NewDiscoveryService's doc comment didn't mention the new
   configuredHosts parameter or its retry-on-every-sweep behavior.
   Documented.

5. The new test's second DiscoverDevices call spawns a one-shot
   status-update goroutine and a 30s-ticker poll loop with no
   guaranteed drain before the deferred server.Close(), risking
   benign but real -race/CI flakiness. Added a bounded settle delay
   after RemoveDevice.

Verified: full build/vet/race test suite/lint clean; the new
TestClassifySource fails against the pre-fix logic and passes with
it; TestDiscoverDevicesRetriesConfiguredHosts re-run 20x under -race
with no flakiness.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-23 22:00:11 +02:00
Lukáš Lipinský 7015e04556 fix(player): retry configured devices during discovery 2026-08-23 22:00:11 +02:00
Tobias GesellchenandClaude Sonnet 5 d3c5ad2d8e style(install): align the environment variables table columns
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-23 20:50:27 +02:00
Tobias GesellchenandClaude Sonnet 5 ef8bfdc74b docs(install): add an environment variables reference table
The nine env vars install.sh reads were only ever mentioned inline,
scattered across the file, or not documented at all
(AFTERTOUCH_FORCE_NO_BACKUP, GH_REPO, BINARY_URL, INIT_SCRIPT_URL,
FALLBACK_VERSION, AFTERTOUCH_LAN_PORT). Collect them into one table.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-23 20:50:27 +02:00
Tobias GesellchenandClaude Sonnet 5 fdc08d2745 docs(install): replace stale "we're unsure how to update" note
The Space Limitation section still described update safety as an open
problem ("we are currently working on this"). Replace it with what the
installer now actually does: gzip-compressed backups, a preflight
disk-space check with an interactive confirm-or-abort before skipping
the backup, and a hard abort before downloading anything if there
isn't even room for the update itself.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-23 20:50:27 +02:00
Tobias GesellchenandClaude Sonnet 5 ec340847f2 fix(install): preflight disk-space check before replacing the live binary
The gzip fix in the previous commit only helps once a backup is being
made; it doesn't address the actual moment that broke on real hardware:
the cross-device mv/copy of the new binary into place ran out of space
mid-write, leaving a truncated, non-executable binary as the live one.
UBIFS is a log-structured flash filesystem, so space "freed" by
overwriting the old binary isn't guaranteed reusable in time for the new
one to land -- this happened on a device with 15.7MB available against
a ~14.8MB binary.

Add a preflight check before downloading anything: fetch the new
binary's real size via a HEAD request (adapts automatically as binaries
grow, instead of a threshold that goes stale every release) and compare
against available space plus a flat 5MB safety margin.

- Comfortably enough room for old + new + a compressed backup: proceed
  exactly as before, silently.
- Enough for old + new but not enough extra for a backup: warn
  interactively and require explicit confirmation before proceeding
  without one. Reads from /dev/tty since the script is normally piped
  via `curl | sh` (stdin is consumed by the script itself). Defaults to
  the safe choice (abort) on empty input, matching the [y/N] prompt.
  AFTERTOUCH_FORCE_NO_BACKUP=yes overrides for non-interactive/scripted
  use.
- Not enough room even for the replace itself: abort before starting
  the download, rather than attempting a doomed download/replace that
  could leave a truncated live binary.
- No TTY available and the operator didn't set the override: abort
  rather than silently guessing.
- HEAD request fails for any reason: skip the check with a warning
  rather than blocking the install on it.

Verified: all five decision branches (plenty of room, warn+decline,
warn+confirm, warn+forced-override, hard abort) produce the correct
result under both dash and a real BusyBox v1.38.0 container, including
the gzip/gunzip streaming backup and glob-based pruning from the
previous commit. The HEAD-request size lookup was separately verified
against a live GitHub release URL with real curl -- catching and fixing
a bug where naively taking the first "content-length:" header grabbed
the 302 redirect's (0), not the actual asset's, size. Not yet re-tested
end-to-end on real hardware.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-23 20:50:27 +02:00
Tobias GesellchenandClaude Sonnet 5 e8f1b53992 fix(install): gzip the rollback backup to reduce on-device disk pressure
Binaries are tens of MB and only growing (Go 1.27 alone added ~655KB
to soundtouch-service via its own new stdlib defaults, unrelated to
this project's code), while the on-device install target (/mnt/nv) is
only tens of MB total. A user already hit "no space left on device"
attempting an update on real hardware.

Stream the pre-update backup straight through gzip instead of cp-then-
gzip: at that point in the script the old binary is still live and the
newly-downloaded one is already sitting in the temp dir, so writing an
intermediate uncompressed backup copy would briefly need three full
binary-sized copies on disk at once. Streaming avoids ever creating
that intermediate copy. Falls back to a plain uncompressed backup if
gzip is unavailable or the stream fails partway, matching prior
behavior exactly.

Both GC loops (pre- and post-install) now also prune stale
*.backup.gz artefacts, and the README's documented rollback command
covers both the compressed and (fallback) uncompressed cases.

Verified locally (not yet on real hardware): streaming path produces
no uncompressed intermediate, the gzip-unavailable fallback still
produces a plain backup, and the documented gunzip+chmod rollback
restores a byte-identical, executable binary.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-23 20:50:27 +02:00
Tobias GesellchenandClaude Sonnet 5 424631b93a build(lint): point golangci-lint install at the v2 module path
go install github.com/golangci/golangci-lint/cmd/golangci-lint@latest
silently resolves to the latest v1.x release (v1.64.8) -- Go's semantic
import versioning treats v2+ as a completely separate module path
(.../v2/cmd/golangci-lint), so the unsuffixed path's @latest can never
see v2 releases. That mismatched v1 binary can't even load this
repo's v2-format .golangci.yml, and separately doesn't understand the
go1.27.0 toolchain declared in go.mod.

Fix the install hint in `make lint`'s not-found message to use the /v2
path, and refresh the now-current version noted in .golangci.yml's
header comment (installed locally as v2.13.1, built with go1.27.0).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
v0.129.0
2026-08-23 14:50:33 +02:00
Tobias GesellchenandClaude Sonnet 5 97c28b5516 test(router): update route-name snapshot for Go 1.27, drop redundant special case
Go 1.27 changed how runtime.FuncForPC reports the symbol for
HandleWeb()'s returned closure: it now correctly attributes it to its
defining function (handlers.(*Server).HandleWeb) instead of leaking the
inlining call site's enclosing function name (setupRouter) the way
older Go versions did. The registered route itself is unchanged -- this
is purely a difference in the introspected debug name.

The test's cleanup logic had a dedicated special case for stripping a
leading "setupRouter" prefix, added to work around exactly that
inlining artifact. Verified empirically (temporarily instrumented with
the raw runtime.FuncForPC output, then diffed the full 299-route table
with the special case removed) that the general prefix-stripping loop
already produces an identical result for the remaining legitimate
cases (closures actually defined inline in setupRouter, e.g.
/favicon.ico) -- so the dedicated case was already redundant before
this Go bump and can be dropped.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-23 14:50:33 +02:00
Tobias Gesellchen fb69ce29e0 Bump Golang to 1.27.0 2026-08-23 14:50:33 +02:00
Tobias GesellchenandClaude Sonnet 5 18e6c32220 fix(web): show a sources count in Sync results, render as a list
syncSources never reported how many sources it actually saved, so the
Admin UI's success message always said the meaningless "sources:
synced" regardless of outcome. syncSources now returns the count saved
(-1 if the fetch failed), threaded through SyncResult.SourcesCount.

Also replaces the single run-on results string (which visually mashed
presets/recents/sources together with no separator) with a real <ul>
list, one <li> per resource, matching the presets/recents diff lines.
Built via DOM APIs rather than innerHTML string concatenation, since
preset/recent names ultimately come from user-editable station names
on the speaker.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-23 14:19:40 +02:00
Tobias GesellchenandClaude Sonnet 5 7fa70d725a fix(web): surface Sync's destructive-confirm gate in the admin UI
startSync() used to POST once and, on any 2xx, render a hardcoded
"Presets: OK / Recents: OK / Sources: OK" regardless of what the
response actually said -- exactly why a silent partial data loss (see
the previous commit) would have looked like success to the user.

Now: on a 409 (destructive) response, build a specific confirm message
from the diff (e.g. "presets: 6 -> 5: Ici Roussillon") and gate via
window.confirm(), matching the existing QuickFix confirm UX; on
confirm, retry with ?confirmed=true. On success, render the actual
per-resource counts from the response body instead of a canned string.

Adds an HTTP-level regression test
(TestHandleInitialSync_DestructiveSyncReturns409ThenAppliesWhenConfirmed)
covering the same refuse-then-confirm flow through the real handler and
router, complementing the lower-level setup package test.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-23 14:19:40 +02:00
Tobias GesellchenandClaude Sonnet 5 2bac4fb208 fix(setup): require confirmation before Sync would shrink stored data
SyncDeviceData's syncPresets/syncRecents unconditionally overwrote the
datastore with whatever the speaker's live :8090 API returned at that
instant, with no check against what's already stored. If the speaker's
own local cache was stale or incomplete at that moment (e.g. right
after a burst of preset writes, or shortly after a reboot before the
speaker resyncs with Marge), Sync would silently persist that bad
snapshot over good data. A reporter's fresh #614 repro showed the
account's /full response dropping from 6 to 5 presets right after a
Sync click, consistent with this mechanism.

SyncDeviceData now diffs a fresh live fetch against what's stored
before writing anything; if applying would shrink either list, it
returns the diff (via the new SyncResourceDiff/SyncResult types)
without writing unless the caller passes confirmed=true.
HandleInitialSync surfaces this as a 409 with the diff JSON; every call
(confirmed or not) re-fetches live from the speaker, so a confirmed
retry re-checks reality rather than replaying a stale snapshot. Sources
sync is left unconditional, as before -- lower risk in practice and
out of scope for this fix.

fetchLivePresets/fetchLiveRecents are extracted pure-fetch helpers;
syncPresets/syncRecents keep their unconditional-apply behavior (used
directly by existing tests) since the button-driven path now goes
through the diff/confirm guard instead.

Adds TestSyncDeviceData_DestructiveSyncRequiresConfirmation covering
both the refusal and the confirmed-retry path.

Frontend wiring (script.js's startSync + real per-resource result
rendering, replacing the current hardcoded "OK" text) is a follow-up
commit on this branch.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-23 14:19:40 +02:00