Fix/updater reliability - #79
Merged
Merged
Conversation
added 16 commits
August 2, 2026 23:01
Reported symptom: a Docker instance running for some time sits on WAITING FOR SERVER and never updates. Every step of the update discarded its output. The route answered "Update started" before checking anything could work, and the pull handler was `if (code !== 0) return` — so a failed pull meant the helper never ran, nothing changed, and nothing was written down. The client polled every three seconds forever with no deadline, so a stack that could not update looked exactly like one still working. The likeliest cause for a long-running container is the compose file's self-mount at /tmp/docker-compose.yml, which the update reads. Containers started before that line existed do not have it, so the pull fails, and everything above turns that into an indefinite wait. Which is to say the instances least able to update in place are precisely the ones that have been running longest. POST /api/update now preflights the mount, the docker socket and the compose project labels, and returns 409 naming what is missing and what to do. Both steps append to backend/data/update.log on the data volume, so it survives the container being replaced. GET /api/update/status reports phase and error. The modal renders those and gives up after six minutes. Two further faults found while in there. The client waited for the reported version to change, but a build without APP_VERSION reports 'dev' before and after, so a successful update hung too — /api/version now carries a boot id and the client waits on the restart. And hasUpdate was `latest !== current`, so a published tag trailing the running one counted as an update, which is why a 1.8.0 instance was offered 1.7.4. The logic moved out of the route into backend/updater.js; none of it was reachable from a test where it was, which is much of why three separate faults sat in it unnoticed. 20 backend tests and 3 modal tests. No automation added — the trigger stays a button.
A container from before the self-checking backend answers POST /api/update
with "Update started" and then does nothing — the failure this branch
exists to stop being silent. The hardening only helps once that build is
running, which the stuck instance by definition is not.
So the client asks first. GET /api/update/status exists only in the new
build; if it is missing, the container predates the fix, and the modal
says so and shows the command to run on the host instead of posting an
update that will be swallowed. That turns a six-minute wait into an
immediate, actionable answer, and it works from a frontend newer than its
backend — which is exactly the shape of a partly-failed update.
The response shape is checked, not just the status code: a setup serving
index.html for unknown paths answers 200 with a page and would otherwise
read as modern. There is a test for that specifically.
Also staged the waiting message — at 45 seconds it says a pull genuinely
takes a few minutes, so a working update does not look stalled — and the
host command is now shown on the timeout path too, as selectable text.
Two existing tests stubbed every response as {}, which the probe correctly
reads as a stale server; their stubs now describe a modern one.
DEV=false in backend/.env. Stable releases are all anyone sees unless they
deliberately ask for otherwise, which is the right default for unreleased
code.
Dev builds are tagged X.Y.Z-dev with an optional counter, so 1.9.0-dev and
1.9.0-dev.7 both parse and publishing counters later is a change to the
build workflow rather than to this code — which removes the one decision
that had to be made up front.
Ordering follows the three rules that matter: a dev build of a newer
release is offered to someone on an older release; the release supersedes
its own dev builds when it lands, so a dev user is carried onto it; and a
release user is never dragged back onto a dev build of the same version.
Two settings rather than one, because they do different jobs. DEV decides
what is offered, IMAGE_TAG=dev decides what is pulled — docker-compose.yml
now reads ${IMAGE_TAG:-latest} rather than hardcoding latest. Setting only
DEV would offer a dev version and then install the stable one, since the
compose file is what decides the image. Both are documented together in
.env.example with that trap spelled out.
This also fixes a latent fault that would have bitten on the first dev tag
published, and would have hurt stable users rather than dev ones: the tag
filter was unanchored, so 1.9.0-dev passed it, parsed to NaN, and made the
sort comparator return NaN. With the ordering undefined a prerelease could
surface as the newest tag, which the version check would then correctly
refuse — leaving stable users told there was no update when there was.
Two gaps from this branch. The backend test listing named every other
suite but not the new one, and docker-compose.yml sat in the root list
with no note — worth one now that its image tags read ${IMAGE_TAG:-latest}
and the release channel is a setting rather than a file edit.
The module had 35 tests and the seam around it had none, which is the wrong way round for this branch: every fault fixed here lived in that seam. A correct updater.js is no use if the route ignores it. Route level, in admin.test.js: POST /update answers 409 with a reason instead of a false success, /update/status needs no auth because the restart it reports drops the caller's session, /version carries a boot id, and the update is refused without a usable token. Config level, in docker_config.test.js: image tags read IMAGE_TAG rather than a bare :latest — hardcoding it is what would make DEV cosmetic, since compose decides what is actually pulled — neither service is left pinned so a channel cannot half-switch, the compose self-mount the updater reads is still present, and .env.example ships DEV=false. Checked these fail against main's versions of docker-compose.yml and .env.example rather than assuming: four of them go red. One assertion named the wrong layer and was corrected — a temporary admin is turned away at 401 by the middleware, so the route's own isTemporary guard is the second line, reachable only by an elevated temporary.
DEV=true with IMAGE_TAG=latest is a state a user can reach in one edit, and
it does not fail cleanly. The check offers 1.9.0-dev, compose pulls the
stable image because compose is what decides that, and if latest has not
moved then `up -d` is a no-op, nothing restarts, and the modal waits out
its timeout. The next check sees the same dev version as newer and offers
it again — a nag loop that can never resolve, the same shape as the
1.8.0 → 1.7.4 one this branch already fixed.
Capability now wins over intent: dev versions are ignored until DEV and
IMAGE_TAG agree. Offering something that cannot be installed is worse than
offering nothing, because it never settles.
Reported in three places, because the obvious one is unreliable. At
startup in the server log, which is where a config error belongs. In the
check-update response. And in the modal — which needed App.tsx to show the
modal for a warning even with no update available, since a suppressed dev
channel is usually the reason there is no update to show.
Also corrects the .env.example note, which was wrong: compose interpolates
${IMAGE_TAG} from the .env beside docker-compose.yml, not from env_file,
so setting it only in backend/.env silently falls back to latest. That is
a second route into the same contradiction and the warning names it.
The edit was in the previous commit's command but ran from the wrong directory and was silently skipped, so the code landed without it.
IMAGE_TAG=dev with DEV=false was unguarded, and it is the more dangerous of the two directions. DEV=false filters dev tags out of the check, so a stable release is offered — but compose pulls :dev, so whatever that currently points at is installed under the release's name. The version changes and the boot id changes, so the poll reports success. The operator is told they are on 1.9.0 stable and is in fact running 1.10.0-dev.1, with nothing anywhere saying otherwise. The two directions need different remedies, which is why one check was not enough. With DEV=true against a stable tag, a stable offer still installs correctly, so only dev offers are suppressed. Here every offer would be a lie, so updates are suspended entirely until the two agree — and the message names both ways out, since either is a legitimate intention. The three failing combinations are now enumerated in one place rather than inferred, which is what let the reverse case go unnoticed while the forward one was being fixed.
Suspending updates on a contradictory channel left the panel reporting "You're up to date (1.9.0-dev.7)" beside a red warning saying updates were suspended. Of the two, the reassuring half is the one people read — and it was untrue: nothing had been compared, because nothing could be offered. It now says updates are suspended and the channel needs attention. A wart introduced by the guard two commits ago, found by asking what the previous answer actually looks like on screen rather than in the response body.
DEV and IMAGE_TAG said the same thing, and two settings saying one thing produced three contradictory states — each of which I then wrote a guard for. Offering a dev version and installing stable. Offering a release and installing dev under its name, with the update reporting success. A nag loop that could never settle because the thing offered was never the thing installed. None of them is expressible now. IMAGE_TAG is what compose interpolates to decide which images are pulled, so it is already the authority on what a deployment is; the check reads that rather than a separate declaration of intent. DEV is gone, and with it channelState, channelMismatch, shouldOfferDev, shouldOfferUpdates, warnOnChannelMismatch, the boot warning, the warning plumbed through App.tsx into the modal, and the "updates suspended" message — about a hundred lines whose only job was to defend a contradiction that should not have existed. This is also the conventional shape: a single variable naming the tag, as Immich, Paperless, Nextcloud and the LinuxServer images all do. The alternative I was part-way through — the app injecting IMAGE_TAG into the compose child process so DEV could stay authoritative — would have made `docker compose up -d` by hand and the in-app update pull different images, which is the same class of bug in a new place. Tests follow the code: the mismatch suite is deleted, the channel suite is rewritten around the one setting, and a guard asserts .env.example does not reintroduce a second switch. A pinned version tag now counts as stable, which is new and correct — pinning 1.8.1 is not a dev channel.
The env-var section stopped at 1.2.3, so a release adding a setting had nowhere to say so. IMAGE_TAG is optional and existing installs need to change nothing — absent resolves to latest, which is what they already run — but that is worth stating plainly rather than leaving to be inferred. Also records something the guide never said: the in-app update pulls images, not repository files. docker-compose.yml is where the tag became a variable, so the channel is unavailable until a git pull, even on an otherwise fully updated instance. Nothing breaks meanwhile; the capability is simply absent, which is a confusing thing to hit undocumented. And a line on the updater now reporting failures, with the log path, since the section previously promised it "will pull, restart and reload" with no acknowledgement that it might not.
check-update had no test anywhere, and it is the route that carried three of this branch's four bugs: an unanchored tag filter, a comparator returning NaN, and a "different" test that counted a downgrade as an update. Each is tested in updater.js — but the module was correct in isolation the whole time, and it was the route's use of it that shipped broken. Same gap I closed for /update and left open here. Seven tests against a stubbed registry: a newer release is offered, the 1.8.0 → 1.7.4 downgrade is not, dev builds are ignored on the stable channel and offered on the dev one, a dev deployment is carried onto the release when it lands, and a registry with no version tags offers nothing. The one that matters most asserts a prerelease on the registry does not hide a stable release — the fault that would have appeared on the first dev tag published and would have hurt stable users, not dev ones. Verified by restoring the old filter, sort and comparison and re-running: three go red. Without that check they would only have proved the current code agrees with itself.
The dev channel had nothing to find. This publishes it, on manual dispatch or a push to a dev branch, with the tests run first. Two tags per build, and both are needed. `dev` is the moving pointer that IMAGE_TAG=dev pulls. `X.Y.Z-dev.N` is immutable and is the only form the update check can see, since `dev` is not a version and is filtered out of the tag listing — publishing only the moving tag would mean a dev deployment never being offered anything. The run number supplies N, so each build is a distinct version rather than the same one repeatedly. APP_VERSION is baked in as the same string. Without it the container reports 'dev', which parses as nothing, and the check can neither offer it an update nor confirm one landed. It never writes `latest`, which is the one thing that must not happen: a dev build there would reach every stable deployment. And it refuses to run while package.json still holds an already-released version. X.Y.Z-dev sorts below X.Y.Z, so dev builds of a released version are older than what is already out and would be offered to nobody — a confusing silence to debug, and cheap to catch here instead. Verified the comparison against 1.9.0/1.8.1/1.8.0. Five config tests guard the invariants that are easy to lose in a later edit: no latest, both tags, APP_VERSION baked, a distinct version per build, tests before publish.
dev is where unreleased work integrates and where development images are published from, so a PR into it should get the same test feedback as one into main. Without this a fault would surface at publish time rather than at review time. Only the pull_request trigger. A push to dev already runs both suites inside Dev Build to Docker Hub before it publishes, so adding push here would run the same tests twice on every dev commit for no extra signal.
Asked whether anything was missing, and this was: there are two places to start an update, and only one of them was fixed. Sidebar.applyUpdate ignored the response from /api/update entirely, so a 409 naming the problem was discarded and it still said "waiting for server". It waited on the version changing, which never happens on a build without APP_VERSION. And it polled every three seconds with no deadline. That is the originally reported bug, intact, in the path UPGRADE.md tells people to click — while the modal beside it had been carefully fixed. Both now call one shared client. Having two implementations is the only reason one could be hardened and the other left alone, so the fix is to have one rather than to fix the second copy. Also: the "read more" link on both surfaces pointed at README.md#updating, an anchor that does not exist anywhere in the README — so the link offered to someone whose update just failed dropped them at the top of a 570-line file. Both now point at UPGRADE.md, which is the actual guide. 12 tests for the shared client, covering what each caller relied on and what neither of them checked: an index.html fallback answering 200, a refusal with no body, and nothing being POSTed to a server that cannot act on it.
Missed in the previous commit — the anchor I matched on did not exist, and the script reported it rather than failing, which I read past.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Test plan
Pre-merge checklist
Code quality:
Version & Release:
frontend/package.jsonversiondocker-compose.ymlAPP_VERSIONCHANGELOG.mdwith release notesBefore merging to main:
Related issues