Skip to content

ci: pin Node to the patch in .tool-versions, and stop pinning the URL parser's verdict on xn-- - #171

Merged
ArloL merged 2 commits into
mainfrom
claude/configurable-containers-ci-fix-4qyqge
Sep 2, 2026
Merged

ArloL merged 2 commits into
mainfrom
claude/configurable-containers-ci-fix-4qyqge

Conversation

@ArloL

@ArloL ArloL commented Sep 2, 2026 •

Copy link
Copy Markdown
Owner

What failed

CI run 33662723404 on main: test (latest-esr) went red in npm run test:coverage while test (latest) passed on the same commit.

FAIL test/matcher/matcher.test.ts > hostMatcher rejects non-hostnames, naming the value it rejected
AssertionError: expected [Function] to throw an error
 ❯ test/matcher/matcher.test.ts:83:39
    expect(() => hostMatcher("xn--")).toThrow('not a bare hostname: "xn--"');

Why

The two legs got different Node builds from the runner's tool cache (node-version: 24, check-latest: false):

Job Node ada new URL("http://xn--/")
test (latest) 24.19.0 3.4.4 throws
test (latest-esr) 24.20.0 4.0.0 hostname xn--

Node 24.20.0 updated ada to 4.0.0, which passes an undecodable xn-- label through as a plain host. The assertion was pinning the URL parser's answer, not the matcher's: canonicalHost hands hostname validity to new URL deliberately and owns no IDNA rule of its own.

Changes

Pin Node to the patch. .tool-versions carries nodejs 24.20.0; all ten setup-node steps across the five workflows read it through node-version-file instead of spelling node-version: 24, so every job in a run builds on one version and a bump is one line. Renovate's asdf manager (on under config:recommended) keeps it current. asdf and mise pick the same file up locally.

Pin the pin. The AMO reviewer-notes case that derived "Node 24" from the workflows now takes the major from .tool-versions and asserts every setup-node step reads that file and none carries a node-version: of its own. Revert-verified: one workflow put back to node-version: 24 goes red. The notes name only the major, so a patch bump does not touch them.

Drop the parser-dependent assertion. The xn-- line goes, with a comment saying why only inputs every parser refuses belong in that block. The catch branch it documented stays covered by "" and "[". No source change: adding our own punycode check would make the matcher stricter than the browser whose URL it mirrors. The pin alone would not have been enough here, since 24.20.0 is the version being pinned.

README's build line now points at .tool-versions rather than "verified on 24".

Verified

  • test/matcher/ green on Node 22.22.2, 24.19.0 and 24.20.0 (24.20.0 reproduced the failure before the change)
  • npm run typecheck, npm run lint, npm run test:coverage (72 files, 1013 tests) green
  • stryker run --mutate src/matcher/matcher.ts: 189/189 killed, score 100

🤖 Generated with Claude Code

https://claude.ai/code/session_01DhHZArj9LxcKUemN1EMVKt

@ArloL
ArloL deployed to sonarcloud September 2, 2026 19:00 — with GitHub Actions Active
@ArloL ArloL changed the title test(matcher): stop pinning the URL parser's verdict on an xn-- label ci: pin Node to the patch in .tool-versions, and stop pinning the URL parser's verdict on xn-- Sep 2, 2026
`hostMatcher("xn--")` was asserted to throw as "undecodable punycode". That
was the URL parser's answer, not the matcher's: `canonicalHost` hands
hostname validity to `new URL` on purpose and owns no IDNA rule of its own.
ada 4.0.0, which Node 24.20.0 ships, passes the label through as a plain
host, so with `node-version: 24` the two legs of one CI run — one runner
image cached 24.19.0, the other 24.20.0 — disagreed on the same commit.

The catch branch the line was documenting stays covered by `""` and `"["`,
which every parser refuses. Verified green on Node 22.22.2, 24.19.0 and
24.20.0.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DhHZArj9LxcKUemN1EMVKt
`node-version: 24` in ten setup-node steps asked for whatever 24.x the
runner image had cached, and the two legs of one CI run built on 24.19.0
and 24.20.0 and disagreed on the same commit. The version now lives in
`.tool-versions` (`nodejs 24.20.0`), which setup-node reads through
`node-version-file` and asdf/mise read locally; Renovate's asdf manager
keeps it current.

The AMO reviewer-notes case now takes the major from that file and pins
every workflow's setup-node step to reading it — a step that spells its
own version is the drift this replaces. Revert-verified: one workflow put
back to `node-version: 24` goes red. The notes name only the major, so a
patch bump does not touch them.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DhHZArj9LxcKUemN1EMVKt
@ArloL
ArloL force-pushed the claude/configurable-containers-ci-fix-4qyqge branch from 9d7e579 to 099f291 Compare September 2, 2026 19:08
@ArloL
ArloL deployed to sonarcloud September 2, 2026 19:08 — with GitHub Actions Active
@sonarqubecloud

sonarqubecloud Bot commented Sep 2, 2026

Copy link
Copy Markdown

@ArloL
ArloL marked this pull request as ready for review September 2, 2026 19:10
@ArloL
ArloL enabled auto-merge (rebase) September 2, 2026 19:10
@ArloL
ArloL merged commit 47f7cc9 into main Sep 2, 2026
20 checks passed
@ArloL
ArloL deleted the claude/configurable-containers-ci-fix-4qyqge branch September 2, 2026 19:13

This branch was successfully deployed

1 active deployment
sonarcloud — 099f2914 Deployed Sep 2, 2026 by ArloL via analyze #94
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants