From 394af47af100d7fd834baa0ef4462e6055e2c80d Mon Sep 17 00:00:00 2001 From: Thales <> Date: Tue, 22 Sep 2026 20:51:50 +0100 Subject: [PATCH] Offer an update only for the release GitHub calls latest GitHub's publish dialog has three states, not two: set as the latest release, set as a pre-release, or neither. The updater could only tell two of them apart, and treated the third as latest. It read the list endpoint and took the first release that was neither a draft nor a pre-release. A release published with nothing ticked comes back as `draft: false, prerelease: false`, indistinguishable from a promoted one by any field the list carries, so it was offered to every install. That is not a fault anyone hits by following the release runbook, which publishes as a pre-release and promotes afterwards. It is a fault that turns one slip in the publish dialog into a build shipped to everybody, and the runbook itself describes promotion as the thing the updater watches, which was never true: what it watched was the absence of the pre-release flag. So it asks /releases/latest, which answers the question directly. That endpoint returns the release marked latest and nothing else, excludes drafts and pre-releases by definition, and 404s while every release is a pre-release, which is the correct answer to "is there an update" in exactly the window between publishing and promoting. The draft and pre-release checks stay as belt and braces rather than as the decision, so a change in that endpoint's behaviour leaves the app quiet instead of inheriting it. Six tests, where there were none. Two of them fail without this change, including the one that needs both endpoints stubbed to express the bug at all: the unpromoted release is absent from /releases/latest and present in the list. --- static/js/catalog.js | 37 +++++---- tests/e2e/helpers.mjs | 20 +++-- tests/e2e/update-check.spec.mjs | 136 ++++++++++++++++++++++++++++++++ 3 files changed, 168 insertions(+), 25 deletions(-) create mode 100644 tests/e2e/update-check.spec.mjs diff --git a/static/js/catalog.js b/static/js/catalog.js index 3b38bf61..c8017f22 100644 --- a/static/js/catalog.js +++ b/static/js/catalog.js @@ -2300,8 +2300,21 @@ const RELEASES_URL = "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/stemdeckapp/stemdeck/releases"; // update exists. StemDeck has historically published even its alphas as normal // releases, which is why that has not bitten yet -- this makes the check // correct either way rather than dependent on remembering not to tick a box. +// The one release GitHub itself calls latest. +// +// Not the list endpoint. A release can be published in three states, and the +// list only distinguishes two of them: "set as the latest release", "set as a +// pre-release", and neither. That third state comes back as +// `draft: false, prerelease: false`, indistinguishable from a promoted release +// by any field the list carries, so filtering it was offering every user a +// build that was deliberately not promoted (#666). +// +// This endpoint answers the question directly: it returns the release marked +// latest and nothing else, and excludes drafts and pre-releases by definition. +// It also 404s while every release is a pre-release, which is the correct +// answer to "is there an update" at that moment. const RELEASES_API = - "https://api.github.com/repos/stemdeckapp/stemdeck/releases?per_page=10"; + "https://api.github.com/repos/stemdeckapp/stemdeck/releases/latest"; const DISMISSED_UPDATE_KEY = "stemdeck.dismissed_update"; // The full GitHub release object from the last successful update check, used to @@ -2738,22 +2751,12 @@ async function checkForUpdate() { // The check itself succeeded, regardless of what it finds below — clear // any stale "update check failed" card (#401). dismissFailuresByKind("update"); - // Newest first, as GitHub returns them. Drafts are invisible to an - // unauthenticated request anyway, but filter them so a maintainer running a - // dev build is not offered a release that has no assets yet. - // - // Pre-releases are skipped as well: a release is published as a pre-release - // first, verified, and only then promoted ("Set as the latest release"), so - // nobody is offered a build that has not been through that. The list - // endpoint is used rather than /releases/latest because it keeps the choice - // here, in code, rather than in GitHub's endpoint semantics. It assumes a - // stable release inside the last 10 -- true unless ten consecutive - // pre-releases go out without one being promoted. - const releases = await res.json(); - const data = Array.isArray(releases) - ? releases.find((r) => !r.draft && !r.prerelease) - : null; - if (!data) return; + // One release, already chosen by the endpoint. The two checks are belt and + // braces rather than the decision: this endpoint is documented to exclude + // drafts and pre-releases, and if that ever stops being true the app should + // stay quiet rather than inherit the change. + const data = await res.json(); + if (!data || data.draft || data.prerelease || !data.tag_name) return; const latest = normalizeVersion(data.tag_name); // Compare canonically so a PEP440 current version (0.7.0a9) matches the // release tag form (0.7.0-alpha.9) and we don't nag an already-current app. diff --git a/tests/e2e/helpers.mjs b/tests/e2e/helpers.mjs index 8dc5d66f..e8ade154 100644 --- a/tests/e2e/helpers.mjs +++ b/tests/e2e/helpers.mjs @@ -200,18 +200,22 @@ export async function stubUpdateCheck(page, { available = false } = {}) { demucs_device: "cpu", }), })); + // A single object, not an array: the app asks for /releases/latest, which + // is GitHub's own answer to "which release is the latest one". A release + // published as a pre-release, or as neither latest nor pre-release, is not + // returned by that endpoint at all, so it cannot be offered (#666). await page.route("https://api.github.com/**", (route) => route.fulfill({ status: 200, contentType: "application/json", - // An ARRAY: the app polls /releases (the list), not /releases/latest. - // The unpromoted pre-release in front of the stable one is deliberate: - // it must be skipped, because a release is only offered once it has - // been promoted to the latest release. - body: JSON.stringify([ - { tag_name: "v9.9.10", draft: false, prerelease: true, body: "unpromoted", html_url: "https://example.invalid", assets: [] }, - { tag_name: "v9.9.9", draft: false, prerelease: false, body: "notes", html_url: "https://example.invalid", assets: [] }, - ]), + body: JSON.stringify({ + tag_name: "v9.9.9", + draft: false, + prerelease: false, + body: "notes", + html_url: "https://example.invalid", + assets: [], + }), })); return; } diff --git a/tests/e2e/update-check.spec.mjs b/tests/e2e/update-check.spec.mjs new file mode 100644 index 00000000..6b840cc9 --- /dev/null +++ b/tests/e2e/update-check.spec.mjs @@ -0,0 +1,136 @@ +// A build is offered to users only when GitHub itself calls it the latest +// release. +// +// GitHub's publish dialog has three states, not two: set as the latest +// release, set as a pre-release, or neither. The updater used to read the list +// endpoint and take the first release that was neither a draft nor a +// pre-release, which cannot tell the third state from the first: a release +// published with nothing ticked comes back as `draft: false, prerelease: +// false` and was pushed to every install (#666). +// +// So these are about which releases stay silent, which is the half that has no +// visible symptom until it is too late. +import { test, expect } from "@playwright/test"; +import { seedLibrary, stubExportEndpoints } from "./helpers.mjs"; + +// The app skips the check on dev builds, so it has to look like a release. +const stubVersion = (page, version) => + page.route("**/api/health**", (route) => + route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify({ + name: "StemDeck", + status: "ok", + version, + ffmpeg_configured: true, + demucs_model: "htdemucs_6s", + demucs_device: "cpu", + }), + }), + ); + +const stubGithub = (page, { status = 200, body = null }) => + page.route("https://api.github.com/**", (route) => + route.fulfill({ + status, + contentType: "application/json", + body: JSON.stringify(body ?? { message: "Not Found" }), + }), + ); + +const release = (over = {}) => ({ + tag_name: "v9.9.9", + draft: false, + prerelease: false, + body: "notes", + html_url: "https://example.invalid", + assets: [], + ...over, +}); + +async function open(page) { + await seedLibrary(page); + await stubExportEndpoints(page); + await page.goto("/", { waitUntil: "domcontentloaded" }); +} + +const cardShown = (page) => + page.evaluate( + () => !document.getElementById("notifReleaseCard")?.classList.contains("hidden"), + ); + +test.describe("update check", () => { + test("a release GitHub calls latest is offered", async ({ page }) => { + await stubVersion(page, "0.5.0"); + await stubGithub(page, { body: release() }); + await open(page); + + await expect.poll(() => cardShown(page)).toBe(true); + }); + + test("nothing is offered while every release is a pre-release", async ({ page }) => { + // /releases/latest 404s when there is no promoted release, which is exactly + // the state a project is in between publishing and promoting. Before this, + // the list endpoint would hand back the newest stable release regardless. + await stubVersion(page, "0.5.0"); + await stubGithub(page, { status: 404 }); + await open(page); + + await page.waitForTimeout(1200); + expect(await cardShown(page)).toBe(false); + }); + + test("a pre-release is not offered even if the endpoint returns one", async ({ page }) => { + // Belt and braces: the endpoint is documented not to return these, and the + // app should stay quiet rather than inherit a change in that behaviour. + await stubVersion(page, "0.5.0"); + await stubGithub(page, { body: release({ prerelease: true }) }); + await open(page); + + await page.waitForTimeout(1200); + expect(await cardShown(page)).toBe(false); + }); + + test("a draft is not offered either", async ({ page }) => { + await stubVersion(page, "0.5.0"); + await stubGithub(page, { body: release({ draft: true }) }); + await open(page); + + await page.waitForTimeout(1200); + expect(await cardShown(page)).toBe(false); + }); + + test("the current version is not offered to itself", async ({ page }) => { + await stubVersion(page, "9.9.9"); + await stubGithub(page, { body: release() }); + await open(page); + + await page.waitForTimeout(1200); + expect(await cardShown(page)).toBe(false); + }); + + // The bug itself, which needs both endpoints stubbed to express. + // + // A release published with neither box ticked is absent from + // /releases/latest and present in the list, indistinguishable there from a + // promoted one. Reading the list offered it; asking for the latest release + // does not see it at all. + test("a release published as neither latest nor pre-release is not offered", async ({ page }) => { + await stubVersion(page, "0.5.0"); + await page.route("https://api.github.com/**", (route) => { + const url = route.request().url(); + if (url.includes("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/releases/latest")) { + return route.fulfill({ status: 404, contentType: "application/json", + body: JSON.stringify({ message: "Not Found" }) }); + } + // The list still carries it, and says nothing about it being unpromoted. + return route.fulfill({ status: 200, contentType: "application/json", + body: JSON.stringify([release({ tag_name: "v9.9.11", body: "published as none" })]) }); + }); + await open(page); + + await page.waitForTimeout(1200); + expect(await cardShown(page)).toBe(false); + }); +});