ci: the web suite runs only on Node 24, so Node-26-only breakage is invisible to the gate

[svengerber]

Split out of #2058 / #2073 rather than grown into that PR's scope.

The blind spot

Four web tests were red on a clean main for every contributor on Node 26 and
green in CI, for a week, because CI pins one Node version:

.github/workflows/ci.yml:159        node-version: 24
.github/workflows/e2e.yml:87        node-version: 24
.github/workflows/desktop.yml:45    node-version: 24

#2073 fixes that instance — Node 26 ships an inert localStorage global that
vitest's happy-dom environment declines to replace, so happy-dom tests read
undefined. It does not fix the reason nobody found out from CI.

Nothing pins the local version either: no .nvmrc, no engines in
web/package.json. So contributors drift onto whatever Node they have, and
AGENTS.md tells every one of them to trust make ci locally before pushing. A
suite that is red for reasons unrelated to your change is a suite people learn
to ignore — which is the part of #2058 worth more than the four tests.

Why this shape of bug will recur

The failure was not a broken API. Node 26 added globals (Storage,
localStorage, sessionStorage), and adding one was enough: vitest's
populateGlobal skips any window key the platform already defines unless the
key is on its hardcoded allow-list, and localStorage is not. Every future Node
that lands a web global can shadow happy-dom the same way, silently — and
silently is the operative word: profile.svelte.ts and palette.svelte.ts guard
on typeof localStorage === 'undefined', so the no-storage branch passes on
default data rather than erroring. Only a test asserting storage round-trips
goes red.

What to decide

Two candidate remedies, and they are not exclusive:

1. Add the current Node to the web job's matrix (node-version: [24, 26]).
   Catches this class at the gate. Costs one more web-suite run per PR — the Go,
   e2e and vulncheck jobs need not be duplicated, and this repo already cancels
   superseded runs (Revert "ci: report e2e on the PRs where it does not run" #963). The open question is whether to gate the merge on the
   newer entry or let it be advisory, since a Node-26 regression in a
   dependency would then block unrelated PRs.
2. Pin the local Node so the two gates cannot disagree — .nvmrc plus an
   engines field. Cheap, but advisory: no tooling here reads .nvmrc
   automatically, and pinning to 24 tells contributors on 26 that their working
   setup is unsupported when, after fix(web): restore happy-dom's localStorage over Node 26's inert global #2073, it is not.

My read is that (1) is the durable one and (2) alone would only have relabelled
this bug rather than surfaced it. Worth a contributor's call on whether the
second matrix entry blocks or informs, which is why this is an issue and not a PR.

[svengerber]

Taking the informational half only, on ci/node-matrix: the web suite gets a second run on Node 26 as an advisory job, and the open question in "What to decide" — whether that entry blocks a merge or merely informs — stays here for a contributor to settle. So the PR says Refs #2074, not Closes; the issue stays open on that decision, and I will put both options and their costs in this thread.

Not doing (2): #2073 declined .nvmrc/engines for a reason I agree with — after that fix, 26 works, so pinning to 24 would tell contributors on 26 their setup is unsupported when it is not.

One finding up front, because it changes the shape of option (1): web is one of the four required status checks on the main ruleset (server, web, vulncheck, docs). A node-version: [24, 26] matrix on the existing web job renames its context to web (24), so the required web would never be reported again and every merge in the repo would wedge on a check nothing satisfies — with bypass_actors: [], only a repo admin could unstick it. The second Node therefore has to be a separately named job, whichever way the blocking question goes.

[svengerber]

The advisory half is #2103. What is left on this issue is one decision: does the Node-26 entry block a merge, or only inform? #2103 ships it advisory because that is the option that cannot break anything while the question is open — but advisory is a real choice with a real cost, not a neutral default, so here are both sides.

The mechanics are settled either way, and worth stating first: the second Node has to be a separately named job, not a matrix entry on web. web is a required context on the main ruleset (server, web, vulncheck, docs; bypass_actors: []), a matrix renames it to web (24), and the required web would then never be reported by anything — every PR in the repo unmergeable on a check that no longer exists, clearable only by an admin editing the ruleset. So the decision is purely about the job web-node-next: leave it as it is, or add its context to the required list.

Option A — leave it advisory (what #2103 does)

• Nothing upstream can stop the repo. Node 26 is Current and moves fast: 26.8.2 landed yesterday. A breaking change in Node, or in a dependency reacting to one, cannot block eight agents merging unrelated work.
• Costs: an advisory check is a check someone has to look at. The failure mode is precisely the one this issue was filed about — a signal nobody reads. continue-on-error keeps the run green, so a Node-26 break shows only in the run's job list, not on the PR's headline status.
• Mitigation if we stay here: nothing automatic. It relies on the same attention that missed fix(web): custom.svelte.test.ts fails locally on Node 26 while CI is green on Node 24 #2058 for a week, which is the honest argument against A.

Option B — make web-node-next required

• The signal cannot be ignored, and this class of bug is genuinely worth that: fix(web): custom.svelte.test.ts fails locally on Node 26 while CI is green on Node 24 #2058 was not a broken API but an added global, and the loud four tests were the better half — the storage-backed stores guard on typeof localStorage === 'undefined', so the rest of the suite took the no-storage branch and passed on default data. Only a test asserting a round-trip goes red at all.
• Costs: every merge in the repo now depends on a Node line that is not our LTS. An upstream regression becomes a repo-wide stop, and the unblock is a ruleset edit by an admin — the same manual step as the wedge above, just chosen deliberately.
• Also: adding a context to the required list is enforced immediately, so it must be added after ci: run the web suite on Node 26 as an advisory check #2103 merges, or open PRs stall waiting for a job their branch does not define.

Two facts that bear on the timing

1. The suite passes on Node 26.8.2 today (170 files / 1526 tests, identical to Node 22), and was seen to fail correctly with fix(web): restore happy-dom's localStorage over Node 26's inert global #2073's setup block disabled — 2 files red on 26, green on 22. So B would not be turning on a broken gate.
2. Node 26 becomes the Active LTS next month (even majors go LTS the October after their April release). At that point it stops being "a newer Node we advise on" and becomes the version contributors get by default — and the argument for A, that 26 is not the line we support, expires with it. The natural shape may be: A now, and when 26 goes LTS it graduates into the web job (already required) while a new advisory job takes the next Current. ci: run the web suite on Node 26 as an advisory check #2103's job carries that instruction in a comment.

My read, for what it is worth: A until 26 goes LTS, then the graduation in (2) — which gets B's enforcement without ever making a non-LTS Node able to stop the repo. But that is a risk-appetite call and I have not made it; leaving this open and unassigned for a contributor.

[svengerber]

#2103 is merged. The advisory job web-node-next is live on main and green on Node 26.8.2 in CI (node: v26.8.2, 170 files / 1529 tests).

Unassigning myself: what remains here is the blocking-versus-informing decision in the comment above, not implementation work. Leaving the issue open and free for whoever makes that call.

[janlauber]

Checked the board rather than the title: half of this shipped. web-node-next has been running the web suite on Node 26 since #2103 — advisory (continue-on-error), deliberately not named web so it stays out of the branch ruleset's required contexts, and with the promotion note in the workflow for when 26 becomes Active LTS.

What is still open here is the part the issue itself calls a contributor's call, and I am leaving it: (1) whether that check graduates to blocking — a Node-26 regression in a dependency would then hold up every unrelated PR, which is risk appetite, not something the repo can answer; and (2) whether to pin local Node with .nvmrc + engines, where the issue's own objection stands — pinning to 24 tells a contributor on 26 that their working setup is unsupported, which after #2073 it is not.

Not standing down on anything I could implement: the implementable half is done, and the rest is two decisions. Proceed, do not stand down on account of my comment.

[svengerber]

Decided: advisory now, graduate when Node 26 goes Active LTS.

Maintainer's call, 2026-09-17. web-node-next stays continue-on-error and stays out of the ruleset's required contexts. When 26 becomes Active LTS next month it graduates into the web job — which is already required — and a new advisory job takes the next Current line. That gets option B's enforcement without a non-LTS Node ever being able to stop every unrelated PR in the repo, and it is the shape #2103's workflow comment already carries as an instruction.

The second half of the issue stands as declined for the reason #2073 gave: no .nvmrc/engines pin, because after #2073 a contributor on 26 has a working setup and pinning to 24 would tell them it is unsupported.

Both halves now have an answer and the implementable half shipped in #2103, so this closes.

Co-Authored-By: Claude Opus 5 (1M context) noreply@anthropic.com