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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
Converts the remaining GetX-then-SaveX call sites (UpdatePreset,
RemovePreset, AddRecent's recent + learned-source persistence, AddSource)
to the new datastore.Mutate{Presets,Recents,ConfiguredSources} helpers,
closing the lost-update race for good on the actual write path the
speaker hits on every preset/recent store.
Adds a regression test that fires 6 concurrent UpdatePreset calls (same
shape as #614's rapid-fire repro) and asserts none are lost. Verified it
reliably fails against the pre-fix code (consistently drops presets
across repeated runs) and passes reliably with the fix, including under
-race.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
GetX-then-SaveX call sites did an unguarded read-modify-write: two
concurrent callers could each read the same starting list, mutate
different entries, and the second writer's Save clobber the first's
update. This is exactly what dropped a preset during #614's rapid-fire
preset-programming repro (overlapping PUT .../preset/N requests).
Add MutatePresets/MutateRecents/MutateConfiguredSources, each holding a
single write lock across the whole read-mutate-write cycle, and switch
resolvePresetSource's auto-add-canonical-source path (the same race,
for sources) to use the new MutateConfiguredSources.
Part of the #614 follow-up; more call sites (UpdatePreset's own preset
write, recents, other sources writers) still need converting.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Bumps golang from 1.26.6-alpine to 1.27.0-alpine.
[](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)
Dependabot will resolve any conflicts with this PR as long as you don't
alter it yourself. You can also trigger a rebase manually by commenting
`@dependabot rebase`.
[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)
---
<details>
<summary>Dependabot commands and options</summary>
<br />
You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits
that have been made to it
- `@dependabot show <dependency name> ignore conditions` will show all
of the ignore conditions of the specified dependency
- `@dependabot ignore this major version` will close this PR and stop
Dependabot creating any more for this major version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop
Dependabot creating any more for this minor version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop
Dependabot creating any more for this dependency (unless you reopen the
PR or upgrade to it yourself)
</details>
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Speakers paired via non-AfterTouch tooling (e.g. the USB-stick SSH-enable
method) can report a margeAccountUUID that isn't Bose's own 7-digit
numeric format, such as "stick@local". Discovery persisted this value
unvalidated, and the datastore's identifier check rejected it outright,
so the device was silently never saved.
Widens datastore.IsSafeIdentifier to accept any identifier that's safe
as a path component, XML value, and telnet-command token (still
excluding whitespace, control characters, and HTML/XML/shell
metacharacters), and makes it the single account-ID validator,
replacing setup's separate, stricter 7-digit-only IsValidAccountID.
Also closes related gaps found while widening the validator:
- postSetMargeAccount now XML-escapes the account ID instead of raw
string interpolation.
- SaveAccountInfo/HandleMargeCreateAccount now validate the account ID
the same way SaveDeviceInfo already did.
- handlers_export.go URL-escapes account/device IDs before building
outbound diagnostic-fetch URLs.
- pkg/service/health gained the sanitizeLog helper every other package
already has, applied to log lines carrying speaker-reported values.
- The admin web UI (script.js) renders account/device IDs via DOM APIs
instead of innerHTML/inline event-handler string interpolation,
closing a stored-XSS path, and a duplicate escape helper was
consolidated into one.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
readPresetsLocked's os.IsNotExist branch and GetRecents' equivalent
branch silently returned an empty result with no log line at all,
unlike their sibling 0-byte/malformed-XML branches which already log.
When a reporter's speaker got served an empty preset list at reboot
despite an intact on-disk Presets.xml, there was no durable record of
it anywhere except a live capture at the exact moment.
Also log the per-device preset count going into every /full response
in CreateAccountDevice, distinguishing a disk read that came back
empty from one where source-mapping silently dropped presets
afterward.
Diagnostic only, no behavior change - the actual trigger for the
empty response is still open.
Release notes previously pointed at the flat, alphabetical Assets
list, forcing readers to hunt for their platform's soundtouch-service
or soundtouch-cli build. Generate direct per-platform links (with
inline checksum links, one row per OS/arch) from the deterministic
asset naming convention, and wire it into both release paths: the
auto-generated notes (create_release) and the hand-authored notes a
maintainer publishes via the GitHub web UI (update_release, which now
replaces the Downloads footer line in place). The footer-replace logic
always goes through the same strip-then-append path so re-running the
job for the same tag stays byte-for-byte idempotent.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
A speaker can be reachable, named, and already account-paired yet still
report SOUNDTOUCH_NOT_CONFIGURED, leaving the "install the Bose app"
prompt on screen (reported for ST30 Series II/III in #615). Only a full
pass through the WebSocket setup state machine clears it, but running
that unconditionally risks re-running the bracket on speakers that
don't need or support it.
Add Manager.PreflightInitPlan: checks /supportedURLs for
/setMargeAccount, then requires /soundTouchConfigurationStatus to read
exactly SOUNDTOUCH_NOT_CONFIGURED before ExecuteInitPlan runs.
Already-configured devices are a no-op; an unsupported route or an
unrecognised status value aborts instead of guessing.
The default Suggested Plan (both XML-over-SSH and Telnet) migrates the
speaker over plain HTTP and never touches CA/TLS at all, but the CA/TLS
precondition always showed a red not-installed marker and the HTTPS
Connection Test panel was always rendered, regardless of whether the
current Target URL actually needs HTTPS. Both read as mandatory steps
even when nothing needed doing.
CA/TLS and HTTPS only matter when the Target URL is https:// or the
Customize form's DNS-interception method is chosen (that one always
targets https://*.bose.com).
- caVerdict() now takes whether the Target URL is HTTPS: shows a
neutral marker with a "not needed" note for HTTP targets, keeps the
red marker with a sharper "required" note for HTTPS targets.
- The HTTPS Connection Test panel gets a small note under its heading
("Optional for your current plan (HTTP)" / "Required ... (HTTPS)"),
computed from the same check. Stays visible either way so someone can
still run it if they want.
Frontend-only — showSummary already had the Target URL in scope.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Follow-up on #621: the Reboot Speaker button (plus Revert to Defaults,
Enable SSH, Disable SSH) was reachable only after expanding the
collapsed "Customize this migration" section and scrolling past three
fieldsets and the XML/telnet diff panes. Meanwhile every other real
action elsewhere in the admin UI (Save Settings, Apply Suggested Plan,
Start Sync, ...) is visible by default.
- Move Revert to Defaults and Reboot Speaker into an always-visible
"Speaker controls" row directly under the Migration State card.
- Move Enable/Disable SSH into the Preconditions table, inline with the
SSH (remote_services) status row, sized like the existing "Trust CA
Now" button next to the CA/TLS row. script.js now only rewrites the
inner status span on re-render (matching the CA/TLS pattern) so the
buttons survive summary refreshes.
- Add shared .btn-primary/.btn-danger CSS classes so button color
consistently means the same thing everywhere (primary = confirm,
danger = destructive) instead of ad-hoc inline colors; applied to
Save Settings, Apply Suggested/Custom Plan, Enable/Disable SSH,
Revert to Defaults, and Trust CA Now. Removed decorative gray from
Reboot Speaker and the connection/DNS test buttons.
- Replace the "Cancel" button (which only hid the whole summary panel,
not any of the actions it sat beside) with a "✕ Hide" control next
to the "Migration Summary for <device>" heading, alongside a new
"↻ Reload" shortcut for refreshSummary().
- Remove the now-unneeded force-open-the-details hack in migrate()
since Reboot no longer lives inside any collapsed container.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Two doc notes from dunha's #621 follow-up: the factory-reset button
combo is confirmed identical on the SoundTouch 30 Series III, and
checking the reported version right after an on-device update can
still show stale info until the speaker (or an open Admin UI tab) is
rebooted, even though the new binary is already running.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- setup: resync all four boseurls (not just marge/swUpdate) over telnet
after an SSH-XML migration. `envswitch boseurls set` persists whatever
is currently in the runtime layer, so leaving stats/bmx untouched froze
their stale pre-migration values into the persistence layer permanently
-- surviving reboot and previously requiring a factory reset to clear.
- admin-ui: Migrate tab's Target Domain edits now propagate into the four
service URL fields (tracked via a dataset.autofilled flag so real manual
edits still aren't clobbered), closing the gap where changing Target
Domain to a new value left the four fields pointed at a stale default.
- install.sh: prune stale binary backups before the download too, not
only after a successful install, so a backup left by a previously
aborted (out-of-space) run gets cleaned up instead of compounding.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
validateURL() unconditionally rejected the hostnames "localhost" and
"127.0.0.1" for the four Migrate-tab plan URL fields, with no awareness
of deployment mode. Since the Suggested Plan's URLs are derived from the
page's own configured Target URL, a fresh on-device install (whose
server_url is now correctly http://localhost:8000, since #546) loaded
the Migrate tab with "Apply Suggested Plan" and "Pre-flight" disabled
by default, before the user touched anything -- directly contradicting
the on-device docs' "Migrate -> accept the suggested plan -> apply"
instructions.
Found while investigating why a #614 reporter used the non-standard
"localhost.localdomain" as a workaround, and why a #621 reporter got
stuck with "Migration Status: Migrated (URL mismatch)" trying to follow
the (correct) on-device localhost guidance.
Fix: a loopback URL is only flagged when it doesn't match the plan's
own Target URL origin. A field that's exactly what the service itself
is already configured to answer as (the on-device case) is accepted;
a stray "localhost" typed into one field while Target URL is a real LAN
address (the external-host mistake the check exists to catch) is still
flagged, since the origins differ.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
