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); + }); +});