Skip to content

fix(sync, sw, update): a change that is saved nowhere, a build swept off the device, and an update that is never offered - #250

Closed
kurktchiev wants to merge 5 commits into
DuarteSantos8:mainfrom
kurktchiev:gh/sync-and-shell
Closed

kurktchiev wants to merge 5 commits into
DuarteSantos8:mainfrom
kurktchiev:gh/sync-and-shell

Conversation

@kurktchiev

@kurktchiev kurktchiev commented Sep 19, 2026 •

Copy link
Copy Markdown
Contributor

Five fixes, one commit each, all with tests. Four of them are about a change or a build going missing
without saying anything; the other is a two-character parser bug in the update check.

Sections 2 and 5 are the same file and belong together. The rest touch separate files and rebase
independently, so I am happy to split any of them out.

1. A tab left open pushes its stale state over the other tab's work

What breaks. Two tabs of one browser share the saved copy (gym_state_v1) and the revision marker
(gym_sync), but each holds its own S in memory. The tab that was left open never learned that the
other one had saved. Its next change went out as its whole stale state, quoting the marker the other
tab had just written, so the server saw a current baseRev, took the write without a 409, and the
workout logged in the other tab was gone from both copies. The conditional write cannot help here. The
stale tab is quoting the right number, it just does not have the data behind it.

Reproduce. Open the app in two tabs, signed in. In tab B, log and finish a workout. In tab A
(untouched since before that), change any setting. Tab B's workout is gone from tab A, from the
server, and from tab B on its next pull.

What changed. A storage listener on KEY. A change this tab still owes is merged in with the
same mergeStates union a 409 already uses; otherwise it takes the newer copy. The in-progress
workout stays with the tab running it (next.active = S.active). Only memory is touched. The saved
copy is already what the event carried, and writing it back would land as a storage event in the other
tab, which would answer in kind.

An event whose gym_owner no longer matches this tab's user is left alone, because another profile
signing in elsewhere writes its wiped copy through this same key just before it records itself as
owner; that case belongs to the existing gym_owner listener, which drops this profile rather than
merging it into the new owner's copy. The push the merge arms waits for ready behind the same gate
persist uses, since before boot has pulled, the copy in hand may be older than the server's and the
marker stale with it.

The owed expression checkRev already had is now a named owes() helper, since both places ask the
same question.

2. A service worker that never got the shell still swept the build off the device

What breaks. precache() ended with if (!res.ok) return, and install wrapped it in
.catch(() => {}). So a new worker whose precache failed still resolved its install, called
skipWaiting(), activated, and activate deleted every cache that was not CACHE. A precache fails
when the server restarts mid-deploy, when a phone has a weak signal, or when an auth proxy in front
(Authelia, Cloudflare Access; docs/SELF_HOSTING.md) answers with its login page once the session
there expires. The previous build's shell is what a Home Screen app reopened without a network comes
back from, so after that the app opened on the browser's error page until the next load with a
network.

Reproduce. With the app installed and working offline, deploy while the client is installing the
new worker (or block index.html at the proxy). Then go offline and reopen. Error page.

What changed. precache() throws instead of returning quietly, so the install fails, which is the
only thing that keeps the build already on the device in place. A response that is redirected is
treated the same as a bad status. Whatever came back is not the shell, and caching a login page as
index.html is worse than not installing. activate sweeps the older caches only once this build's
shell is really in its own cache, so an activate that happens anyway leaves the working build alone.

3. A release tag that says which build it is is never offered as an update

What breaks. compareSemver splits on ".". A version carrying semver build metadata,
"1.3.8+2026-09-18.2", splits to ["1","3","8+2026-09-18","2"], so the patch is NaN, which
(pa[i] || 0) reads as 0. A release tagged that way therefore compares as 1.3.0. Against a
running 1.3.7, hasUpdate is false and the update is never offered. The same blind spot from the
other side has the check offer an update to the release already installed, when it is the running
build that carries the metadata.

Reproduce. On main as it stands, the added test fails. hasUpdate is false for a release one
patch ahead whose tag carries build metadata: × judges a tag that carries build metadata on its numbers alone … expected false to be true.

What changed. The metadata is dropped from both operands before the split. Semver says it plays no
part in precedence, so "1.3.7+anything" and "1.3.7" are the same version here. Two characters;
latestVersion is still echoed exactly as the tag came.

4. A save this device refuses swallows the change

What breaks. persist wrote to localStorage before it told the store. A refused write, from a
device genuinely full or a private window with almost no quota, threw out of persist, so
set({ S }) never ran. Finish did nothing at all, and said nothing either. The set you just logged
was gone from the screen as well as from the disk.

Reproduce. Fill the origin's quota (or use a private window with little of it), then finish a
workout. Nothing happens, and there is no error.

What changed. The write is wrapped. The copy is kept in memory whatever the write does, and marked
gym_dirty so a signed-in device still gets the change to the server, the one place it can still be
safe. It is said out loud once per refusal streak, in the same shape as the existing 413 "upload too
large" toast and reset as soon as a write succeeds, because a change that is not on the device is the
one thing the screen cannot show on its own.

One new user-facing string, in all fourteen locale packs. pt-BR takes it as an override, so the
inherited pt-PT fingerprint is untouched and only its override count moves (644 → 645).

5. An install that lost one chunk still swept the build that worked

What breaks. Section 2 made a failed index.html fail the install. Every sub-resource the shell
references was still cached with c.add(u).catch(() => {}). So an install that fetched index.html
and then lost one script to a flaky connection, the phone that walked out of wifi mid-update, reported
success, activated, and swept the previous build's cache, the only thing a Home Screen app reopened
without a network has to come back from. The next offline open was the shell with no code behind it. A
blank page until the device was online again, with no way in from the app itself.

Reproduce. Install the app, then deploy and let the worker fetch index.html but fail one
assets/index-*.js. Go offline and reopen. Blank.

What changed. The scripts and the stylesheets are the app, so one of them failing to cache now
fails the install. Nothing is lost when an install fails: the worker is thrown away and the build
already on the device, cache and all, stays exactly where it was and keeps serving. Images, icons and
the webmanifest stay best-effort, because a missing icon is not a broken app.

Each one is fetched by hand rather than with cache.add, for the same reason index.html is. add
takes a redirect for an answer, and an auth proxy in front answers every request with its login page
once the session there expires. A 200 of HTML stored under the main bundle's URL is worse than nothing
cached at all, because the install would report success and then sweep the build that worked.

The shell itself is written to the cache last, after its own code. Activate's guard is "an
index.html in THIS build's cache"; with index.html written first, a cache holding nothing but the
shell satisfied that guard, so an activate that ran anyway would have swept the working build on the
strength of a file that cannot boot on its own.

Tests

  • useStore.sync.test.jsx, "another tab of the same browser saves": takes the newer copy while
    keeping its own in-progress workout and then pushes both sides' work at the right baseRev; merges
    into a change it still owes and pushes the two together; waits for boot before pushing that merge;
    and leaves the copy alone when the owner changed or nothing newer was saved.
  • sw.test.js (new): frontend/public/sw.js against a stand-in worker environment, with fake
    caches and an awaited waitUntil, so a rejection really is the install failing. A precache that
    cannot reach the server, a redirect to a login page, and a 502 all fail the install and leave the
    previous build's cache in place; an install that got the shell takes over and drops it.
  • update.test.js: one case, where a tag carrying build metadata is judged on its numbers alone, in
    all four directions (ahead, equal, behind, a major ahead), with latestVersion echoed as it came.
  • sw.test.js again, for section 5: a 404 on a chunk fails the install, names the file it could not
    get, leaves this build's cache without a shell and the previous build's cache untouched; a redirect
    to a login page fails it the same way; a 404 on an icon installs, activates and sweeps as usual.
  • useStore.restore.test.jsx, "a save this device refuses": the change survives in memory, is marked
    owed, and the toast is said once per streak and again after a write succeeds in between.

Checklist

  • npx vitest run passes in frontend/: 1481 passing across 106 files (main is 1468 across
    105; +13 tests, +1 file)
  • npm test in api/ unchanged: 181 passing, same as main
  • npx vite build succeeds (no new warnings; the useUI.js dynamic-import notice is already
    there on main)
  • User-facing string is in every locale pack. node scripts/check-locales.mjs: 14 locales, 1292
    keys each, in sync (one string added)
  • node scripts/pt-br-inheritance-fingerprint.mjs fingerprint unchanged: the new key is a pt-BR
    override, so nothing inherited moved
  • No new runtime dependency
  • CHANGELOG.md is left alone

🤖 Generated with Claude Code

kurktchiev and others added 5 commits September 21, 2026 15:33
Two tabs of one browser share the saved copy and the revision marker but
each hold their own state in memory. The tab left open never learned that
the other one had saved: its next change went out as its whole stale state
quoting the marker the other tab had just written, so the server took it
without a 409 and the workout logged in the other tab was gone from both.

It now listens for the save. A change this tab still owes is merged in
(the same union a 409 uses); otherwise it takes the newer copy, keeping the
in-progress workout, which belongs to the tab running it. Only memory is
touched: the saved copy is already what the event carried, and writing it
back would land in the other tab, which would answer in kind. An event
that came from another profile's sign-in is left to the owner listener,
which drops this profile rather than merge into the new owner's copy.

The push the merge arms waits for boot behind the same gate persist uses:
before boot has pulled, the copy in hand may be older than the server's
and the marker stale with it.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A new worker whose precache failed (the server restarting mid-deploy, a
weak signal, an auth proxy in front answering with its login page) still
activated and deleted the previous build's cache, so opening the app
without a network showed the browser's error page until the next online
load. The install now fails instead of returning quietly, and the sweep
waits until this build's shell is really in its own cache.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A version that says which build it came from carries it as semver build
metadata: "1.3.8+2026-09-18.2". Split on ".", that makes the patch NaN,
which read as 0, so a release tagged that way compared as 1.3.0, and
Settings → Check for updates found nothing against a running 1.3.7. Semver
keeps build metadata out of precedence, and so does the comparison now, on
the tag side as well as the running build's.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
persist wrote to localStorage before it told the store, so a refused
write (a device genuinely full, a private window with almost no quota)
took the change with it: Finish did nothing at all, and said nothing
either. The copy is now kept in memory whatever the write does, marked as
owed to the server so a signed-in device still gets it there, and the one
thing the screen cannot show on its own is said out loud once per streak.

New string in all fourteen locale packs; pt-BR takes it as an override, so
the inherited pt-PT fingerprint is untouched and only its count moves.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…that worked

Every sub-resource the shell references was cached with
`c.add(u).catch(() => {})`. So an install that fetched index.html and then lost
one script to a flaky connection, the phone that walked out of wifi mid-update,
reported success, activated, and swept the previous build's cache, which is the
only thing a home-screen app reopened without a network has to come back from.
The next offline open was the shell with no code behind it. A blank page until
the device was online again, with no way in from the app itself.

The scripts and the stylesheets are the app, so one of them failing to cache now
fails the install. Nothing is lost when an install fails: the worker is thrown
away and the build already on the device, cache and all, stays exactly where it
was and keeps serving. Images, icons and the webmanifest stay best-effort,
because a missing icon is not a broken app.

Each one is fetched by hand rather than with `cache.add`, for the same reason
index.html is. `add` takes a redirect for an answer, and an auth proxy in front
answers every request with its login page once the session there expires. A 200
of HTML stored under the main bundle's URL is worse than nothing cached at all,
because the install would report success and then sweep the build that worked.

The shell itself is written to the cache last, after its own code. Activate's
guard is "an index.html in THIS build's cache"; with index.html written first, a
cache holding nothing but the shell satisfied that guard, and an activate that
ran anyway would have swept the working build on the strength of a file that
could not boot on its own.

Three cases added: a 404 on the chunk fails the install, names the file it could
not get, leaves this build's cache without a shell and the previous build's
cache untouched; a redirect to a login page fails it the same way; a 404 on an
icon installs, activates and sweeps as usual.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@DuarteSantos8

Copy link
Copy Markdown
Owner

Thanks @kurktchiev. Sections 2–5 are in v1.3.9 as you wrote them: an install without the shell no longer sweeps the working build, a lost chunk fails the install, the update check parses +build versions, and persisting is quota-safe.

Section 1, the stale tab (#283), is fixed differently. Each copy now tracks its own base revision, and there's a revision check when the gym_sync marker moves, so the stale tab gets a conflict and merges instead of overwriting. That came with the larger sync rework in this release, which also covers the cross-device case @trampi saw. So I'm closing this one.

Released in v1.3.9: https://github.com/DuarteSantos8/openGym/releases/tag/v1.3.9

@kurktchiev
kurktchiev deleted the gh/sync-and-shell branch September 29, 2026 11:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants