Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 20 additions & 17 deletions static/js/catalog.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down
20 changes: 12 additions & 8 deletions tests/e2e/helpers.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
136 changes: 136 additions & 0 deletions tests/e2e/update-check.spec.mjs
Original file line number Diff line number Diff line change
@@ -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("/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);
});
});
Loading