Skip to content
This repository was archived by the owner on Aug 6, 2026. It is now read-only.

fix(updates): supersede pending updates instead of stepping one per restart - #3724

Closed
thiagosalvatore wants to merge 4 commits into
mainfrom
posthog-code/updates-supersede-to-latest
Closed

fix(updates): supersede pending updates instead of stepping one per restart#3724
thiagosalvatore wants to merge 4 commits into
mainfrom
posthog-code/updates-supersede-to-latest

Conversation

@thiagosalvatore

Copy link
Copy Markdown

Problem

When several PostHog Code releases had shipped, the app updated one version at a time — you'd download an update, restart, immediately get prompted for another, restart again, and so on. Reported from Slack; users were restarting 3+ times to reach the latest build.

Root cause is entirely client-side (the release feed always advertises the single newest build). The update state machine latched onto the first update it found: once a release was available or downloaded, it stopped re-checking the feed — and on download it cleared the hourly check interval entirely — until the staged update was installed and the app relaunched. So a session could only ever advance one version, and any newer release that shipped in the meantime wasn't noticed until after the next restart.

Changes

Keep checking in the background while an update is pending and supersede it in place when a strictly-newer release appears, so the app converges to the latest version within one session:

  • Periodic checks now do a silent background re-check while an update is available or ready — no banner flicker, no status emitted, and the pending update is left untouched unless something newer comes back.
  • A strictly-newer available/downloaded version replaces the pending one (re-downloading seamlessly when auto-download is on).
  • The periodic interval is no longer cleared once an update is staged.
  • Install handoffs, and duplicate/older events, are still never disturbed.
  • Adds a small semver-ish comparator (versionCompare.ts) so supersede decisions are based on real version ordering, tolerating a leading v, build metadata, and prereleases.

Why: skipping intermediate versions is safe in this app — SQLite (drizzle) and zustand-persist migrations both run from the current on-disk state to the expected state (keyed on the data's own shape/ledger, never on the previous app version), and differential downloads are already disabled — so collapsing the multi-restart chain into a single jump-to-latest carries no migration-style risk. The only user-visible difference is seeing accumulated release notes at once.

How did you test this?

  • pnpm --filter @posthog/core exec vitest run — full core suite, 2491 passed, including the reworked updates.test.ts and new versionCompare.test.ts.
  • Updated the existing tests that asserted the old latch-and-stop behavior to assert the new silent-re-check + supersede behavior, and added cases for superseding available/downloaded updates and ignoring stale/older events.
  • pnpm --filter @posthog/core typecheck and biome lint packages/core/src/updates — both clean.

Automatic notifications

  • Publish to changelog?
  • Alert Sales and Marketing teams?

Created with PostHog from a Slack thread

…estart

The update state machine latched onto the first update it found: once a
release was available or downloaded it stopped re-checking the feed (and
cleared the periodic interval entirely on download) until the app was
installed and relaunched. When several releases were behind, each restart
only advanced one version, so users had to restart repeatedly to reach the
latest build.

Keep the periodic check running while an update is pending and supersede it
in place when a strictly-newer release appears:
- periodic checks now perform a silent background re-check while an update is
  available or downloaded (no banner flicker, no status emitted)
- a strictly-newer available/downloaded version replaces the pending one
  (seamlessly re-downloading when auto-download is on)
- an install handoff and duplicate/older events are still never disturbed

Skipping intermediate versions is safe here: SQLite and zustand-persist
migrations both run from current on-disk state to expected state, never keyed
on the previous app version, and differential downloads are already disabled.

Adds a small semver-ish comparator (versionCompare.ts) with tests.

Generated-By: PostHog Code
Task-Id: d03c3d60-e63e-4075-89dc-2f0af6c01b01
@trunk-io

trunk-io Bot commented Jul 22, 2026

Copy link
Copy Markdown

Merging to main in this repository is managed by Trunk.

  • To merge this pull request, check the box to the left or comment /trunk merge below.

After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here

@github-actions

github-actions Bot commented Jul 22, 2026

Copy link
Copy Markdown

React Doctor found no issues in the changed files. 🎉

Reviewed by React Doctor for commit 01c012d.

@thiagosalvatore
thiagosalvatore marked this pull request as ready for review July 22, 2026 19:28
@greptile-apps

greptile-apps Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Comments Outside Diff (1)

  1. packages/core/src/updates/updates.ts, line 510-518 (link)

    P1 Stale Download Bypasses Version Guard

    After a staged update is superseded, the state changes to available or downloading. If a delayed duplicate update-downloaded event for the old artifact arrives then, this guard is skipped and the old version becomes ready again, replacing the newer pending update in the UI.

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: packages/core/src/updates/updates.ts
    Line: 510-518
    
    Comment:
    **Stale Download Bypasses Version Guard**
    
    After a staged update is superseded, the state changes to `available` or `downloading`. If a delayed duplicate `update-downloaded` event for the old artifact arrives then, this guard is skipped and the old version becomes ready again, replacing the newer pending update in the UI.
    
    How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
packages/core/src/updates/versionCompare.ts:52
**Numeric Prereleases Sort Lexically**

When `beta.9` is pending and the feed publishes `beta.10`, this string comparison treats `beta.10` as older. The supersession gates then ignore the newer prerelease, so the app remains on the stale build.

### Issue 2 of 2
packages/core/src/updates/updates.ts:510-518
**Stale Download Bypasses Version Guard**

After a staged update is superseded, the state changes to `available` or `downloading`. If a delayed duplicate `update-downloaded` event for the old artifact arrives then, this guard is skipped and the old version becomes ready again, replacing the newer pending update in the UI.

Reviews (1): Last reviewed commit: "fix(updates): supersede pending updates ..." | Re-trigger Greptile

Comment thread packages/core/src/updates/versionCompare.ts Outdated
…lease ordering

Round 1 of PR review found two issues:

- A failed superseding auto-download discarded an already-downloaded,
  ready-to-install update: superseding nulled downloadedVersion before the
  newer build existed, so a download error dropped to `error` with nothing
  installable. Keep the prior build in `supersededReadyVersion` and roll back
  to it in handleError instead of losing it.
- compareVersions compared prerelease identifiers lexically, so numeric
  identifiers ordered wrong (e.g. alpha.10 < alpha.9). Compare per semver §11:
  numeric identifiers numerically, numeric below alphanumeric, longer sets
  above shorter prefixes.

Also folds in a simplify pass: performCheck/performSilentCheck share a
runUpdaterCheck scaffold, and handleUpdateAvailable's ready/available guards
are unified into one isStrictlyNewer check.

Generated-By: PostHog Code
Task-Id: d03c3d60-e63e-4075-89dc-2f0af6c01b01
@thiagosalvatore thiagosalvatore added the Stamphog This will request an autostamp by stamphog on small changes label Jul 22, 2026 — with PostHog
@stamphog

stamphog Bot commented Jul 22, 2026

Copy link
Copy Markdown

Note

🤖 stamphog reviewed 2a68d7c669efd0f7de82ea4639253f657742690b — verdict: REFUSED

Greptile's P1 concern ("Stale Download Bypasses Version Guard") is still present in the diff: handleUpdateDownloaded only guards against stale/duplicate events when state === "ready", but after a supersede kicks off (state moves to "available"/"downloading" while a newer version downloads), a delayed update-downloaded event for the old artifact skips that guard entirely and overwrites the in-flight newer update with the stale one — reintroducing the exact stuck-on-old-version problem this PR is meant to fix. This is a substantive, unaddressed reviewer concern I independently confirmed in the code, not resolved by a later commit.

  • 👍 on the PR from greptile-apps[bot].
  • packages/core/src/updates/updates.ts handleUpdateDownloaded: the isStrictlyNewer guard only fires when state === "ready"; while a supersede is in progress (state "available"/"downloading"), a stale/duplicate update-downloaded event for the old version is accepted unconditionally, overwriting the pending newer download.
  • Greptile's P1 comment on this exact gap ("Stale Download Bypasses Version Guard", updates.ts ~L510-518) is unresolved on the current head.
Gate mechanics and policy version
Gate Result
prerequisites all clear
deny-list no deny categories matched
size 294L, 2F substantive, 541L/4F incl. docs/generated/snapshots — within ceiling
tier T1-agent / T1d-complex (541L, 4F, single-area, fix)
stamphog 2.0.0b3 .stamphog/policy.yml @ aebec27 · reviewed head 2a68d7c

@stamphog stamphog Bot removed the Stamphog This will request an autostamp by stamphog on small changes label Jul 22, 2026
stamphog and Greptile flagged that handleUpdateDownloaded only rejected
stale/older download events when state === "ready". While a supersede is in
flight (state "downloading"/"available", downloadedVersion momentarily null), a
delayed update-downloaded event for the old artifact bypassed the guard and
overwrote the pending newer update — stranding the app on the old version, the
exact bug this PR fixes.

Make the guard state-independent: reject any download no newer than the version
we already have staged or are superseding away from (downloadedVersion ??
supersededReadyVersion). Adds a test for the stale-event-mid-supersede case.

Generated-By: PostHog Code
Task-Id: d03c3d60-e63e-4075-89dc-2f0af6c01b01
@thiagosalvatore thiagosalvatore added the Stamphog This will request an autostamp by stamphog on small changes label Jul 22, 2026 — with PostHog

Copy link
Copy Markdown
Author

CI note: the failing unit-test check is an unrelated flake in packages/harness/src/runtime.test.ts (createHarnessRuntime), which times out against the 5s per-test limit under CI load (the file takes ~7–9s there) but passes locally in ~2s. It's in a different package with no dependency on this core-only diff (packages/core/src/updates/), so I haven't modified it — it's out of scope for this PR and looks like a harness-side timeout/flake, not a regression from these changes. All 25 other checks pass.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Contained fix to the desktop update-check state machine with thorough new tests; both P1 concerns Greptile raised (lexical prerelease sort, stale download bypassing the version guard) are visibly fixed in the current diff via comparePrerelease and the floorVersion guard, each with a dedicated regression test. Not risky territory (no data migration, API contract, billing, auth, or CI change), so independent assurance beyond my own diff reading isn't required.

  • 👍 on the PR from greptile-apps[bot].
Gate mechanics and policy version
Gate Result
prerequisites all clear
deny-list no deny categories matched
size 299L, 2F substantive, 578L/4F incl. docs/generated/snapshots — within ceiling
tier T1-agent / T1d-complex (578L, 4F, single-area, fix)
stamphog 2.0.0b3 .stamphog/policy.yml @ aebec27 · reviewed head 005ec76

Generated-By: PostHog Code
Task-Id: d03c3d60-e63e-4075-89dc-2f0af6c01b01
@pauldambra
pauldambra requested a review from a team July 22, 2026 21:19
@pauldambra

Copy link
Copy Markdown
Member

this was mostly me prompting in a slack thread that Thiago started

feels significant enough not to just go on stamphog tbh

@github-actions

Copy link
Copy Markdown

This PR has had no activity for 7 days and has been marked stale. We are moving to the monorepo and tightening PR staleness in preparation, so it will be closed in 7 days if no further activity occurs.

@github-actions github-actions Bot added the stale No recent changes to PR label Jul 30, 2026
@charlesvien charlesvien closed this Aug 3, 2026
@charlesvien

Copy link
Copy Markdown
Member

PostHog/posthog#76331 takes care of this in the monorepo.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

stale No recent changes to PR Stamphog This will request an autostamp by stamphog on small changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants