Skip to content

ci: pin ripgrep and the type packages, enforce the lockfile, and run the Node tests on 22 and 24 - #589

Open
ronleizrowice-ant wants to merge 8 commits into
mainfrom
ci/pin-toolchain
Open

ronleizrowice-ant wants to merge 8 commits into
mainfrom
ci/pin-toolchain

Conversation

@ronleizrowice-ant

@ronleizrowice-ant ronleizrowice-ant commented Sep 21, 2026 •

Copy link
Copy Markdown
Collaborator

Human: @ronleizrowice-ant

Requested by Ron · project thread

Pins what CI runs on (ripgrep, type packages), enforces the lockfile, and runs the tests that use Node on 24 as well as 22. ripgrep's output and exit status, which the library reads, differ by release.

+104 −53 in 7 files, half of it the lockfile.

What changes

  • Linux legs take ripgrep from upstream's archives by sha256: 14.1.1 for every step, and 15.2.0 for the second full run, the one that already repeats npm test on bubblewrap 0.12.0.
  • New script npm run test:node: the whichSync fallback and the five suites that start node themselves. It runs on Node 24 at the end of every Linux and macOS leg. Everything before it has run with Node 22 on PATH.
  • npm install becomes npm ci on the non-Windows legs.
  • @types/node ^22, @types/bun ^1.4.2, and the two type fixes they ask for. head in http-proxy.ts is typed Buffer. src/node-tls.d.ts gives 'node:tls' the three names the Bun type package uses there and @types/node 22 declares in 'tls' alone; without them tls.connect() takes and returns any.

One fix to shipped code

openFdStreams(), under Bun: a chunk too large to buffer makes FileSink.write() itself reject. main leaves that promise floating and reports the write to the stream as a success. The new typings made lint point at it. The three ways a write fails (write rejects, flush throws, flush rejects) now go through one async function, and a test writes 1 byte and 1 MiB to /dev/full. The second fails on main.

No longer in it (main merged in on 2026-10-20)

  • engines is main's >=22.12.0.
  • release.yml is main's, which was rewritten there and already runs the package on the engines floor. Nothing of this branch's changes to it is carried.
  • The actions are pinned by commit on main already.

Who sees a difference

  • A host that runs srt proxy under Bun: a failed large write to the decider's fd is now an error on the stream. Otherwise contributors and CI only.

Known limits

  • The second full run varies two tools at once, so a failure in it alone has to be told apart by hand.
  • skipLibCheck also skips src/node-tls.d.ts. If it stops working, seven suites fail the type check. Once upstream fixes the declaration it is a silent leftover.
  • test:node names its suites in package.json, and bun test passes over a name that matches no file.
  • Node lines above 24, which engines admits, are not run. No Windows leg runs Node 24.
  • macOS keeps Homebrew's unpinned ripgrep; the docker leg keeps the distribution's ripgrep.

Merge order

#592 is stacked on this.

Review

  • One automated review pass over the whole diff against main, at f88e744 (2026-10-20): seven findings for this PR, all blind spots or clean-ups, answered in cf6cfdd.
  • One automated simplification pass (four reviewers: reuse, simplification, efficiency, altitude) at cf6cfdd, applied in 9c54fab. It replaced changes in twenty files by the nine-line declaration, and a hand-kept list of thirteen suites by the second full run. The PR was +220 −76 in 16 files before.
  • No human has reviewed it yet.

Testing

  • Linux x86-64 with bubblewrap 0.12.0 and ripgrep 15.2.0, the pair of the second full run: the whole suite fails the same 24 names as main on the same machine, all for want of an outside network, and passes the two new cases. Both type checks, eslint and prettier are clean.
  • A probe file: connect({ servernme }) from 'node:tls' is refused, and the result does not assign to a number.
  • CI: see the checks on the head commit.

main merged in on 2026-11-06

main at 27fce0d is merged in at 5b979ee, without a conflict. What this pull request changes is the same as before, +104 −53 in 7 files. The lockfile as merged agrees with package.json (npm ci --dry-run), with the new optional dependency in it, and the new sources pass the type check under @types/node 22.

main merged in again on 2026-11-09

main at 951e641 is merged in at acac083, without a conflict. What this pull request changes is the same as before. The ripgrep archives are fetched after the step that drops CA roots from the runner, whose list main has made longer: see the Linux legs on the head commit.

Claude session #01FydxqU


Generated by Claude Code

… hand

@types/node goes from ^18 to ^22 and @types/bun from ^1.3.2 to ^1.4.2,
which is what CI runs. Two things surfaced. The CONNECT handler's `head`
is reassigned with bytes read off the socket, so it is annotated as a
plain Buffer. And the Bun type package redeclares `connect` for
'node:tls' in a way that, against @types/node 22, takes and returns an
untyped value, so the four call sites pin their options with
`satisfies ConnectionOptions`, and a guard in parent-proxy.test.ts fails
the typecheck if ConnectionOptions ever stops being a real type.

Adds `npm run test:node`, which runs the two checks that start the
library under node rather than Bun. The emitted JavaScript changes only
by comment lines.
Linux legs install ripgrep 14.1.1 from the upstream release by sha256
instead of taking whatever apt serves, and run the suites that exercise
the scan a second time against 15.2.0: the two differ in exit status
for an unreadable directory at --max-depth. The main legs move to
Node 22 and run `npm run test:node` on Node 20, 22 and 24; publish
moves from Node 18, below the package's floor, to 22. `npm install`
becomes `npm ci`. Actions are pinned by commit with the version in a
trailing comment.
Node 20 is past its end of life, and nothing here is built or tested on
it first: the legs build on 22. engines becomes >=22, the steps that
ran the node-side checks on Node 20 go, and the package verification in
release.yml covers 22 and 24. An install on Node 20 with engine-strict
set is now refused, where it used to go through.
@ronleizrowice-ant ronleizrowice-ant changed the title ci: pin ripgrep, Node, type packages and actions to what the package ships on ci: pin ripgrep, Node, type packages and actions, and raise engines to Node 22 Sep 21, 2026
@ronleizrowice-ant

Copy link
Copy Markdown
Collaborator Author

/approve b7fc80a

@ronleizrowice-ant
ronleizrowice-ant enabled auto-merge (squash) October 17, 2026 16:16
release.yml is main's: it was rewritten there (pack, verify, publish what
was packed), it already runs the package on the engines floor, and nothing
of this branch's changes to it is carried. `engines` is main's >=22.12.0.

integration-tests.yml keeps both sets of steps: bubblewrap 0.4.1 from main,
then ripgrep 15.2.0 and the Node 22 and 24 steps from here. The Docker legs
keep main's bound-in Node 22.

Joins that no conflict marker shows:

- With @types/node 22 and @types/bun 1.4.2 together, connect() of node:tls
  is any, so seventeen callback parameters in six test files that are new
  on main were implicitly any. They are annotated.
- FileSink.write() may return a promise under those typings, which
  openFdStreams() left floating. It is chained before flush(), so that its
  failure reaches the stream's callback.
- The list of suites rerun on ripgrep 15.2.0 says it is every suite that
  names ripgrep or the scan, less those that do not depend on it. Six such
  suites are new on main: linux-file-above-deny and literal-path-entries
  depend on the scan and join the list, the other four are named as left out.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FydxqUESqxwSCVFnhk3gfv
Co-authored-by: ronleizrowice <ronleizrowice@anthropic.com>
@ronleizrowice-ant ronleizrowice-ant changed the title ci: pin ripgrep, Node, type packages and actions, and raise engines to Node 22 ci: pin ripgrep and the type packages, enforce the lockfile, and run the Node tests on 22 and 24 Oct 20, 2026
@ronleizrowice-ant

Copy link
Copy Markdown
Collaborator Author

/approve f88e744

claude and others added 2 commits October 20, 2026 11:04
- A failed write to an inherited fd has a test. Under Bun a chunk too large
  to buffer makes write() itself reject, which main leaves floating and
  reports to the stream as a success. The three ways a write fails now go
  through one async function.
- tls.connect() is typed once, in test/helpers/tls-connect.ts, in place of
  an annotation at each callback. All eighteen suites that dial TLS use it,
  so their options and sockets are checked again. The guard on
  ConnectionOptions moves there.
- The two ripgrep installs share .github/scripts/install-ripgrep.sh.
- `bun test` passes over an argument that matches no file, so the step that
  reruns thirteen suites on ripgrep 15.2.0 first checks that each exists.
- test:node also runs the four other suites that start `node` themselves
  (real-path, default-write-paths, rules-through-links-on-every-runtime,
  tls-terminate-tunnel-ends), which so far only saw Node 22.
- The Node 22 step goes back to what it is on main: `npm test` has already
  run the rest with Node 22 on PATH. The comment no longer promises every
  release line.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FydxqUESqxwSCVFnhk3gfv
Co-authored-by: ronleizrowice <ronleizrowice@anthropic.com>
- tls.connect() is untyped because the Bun type package redeclares it in
  'node:tls' with three names that @types/node 22 declares in 'tls' alone.
  src/node-tls.d.ts gives the module those names. The helper, the import
  lines in eighteen suites and the `satisfies` in parent-proxy.ts all go: a
  misspelt option is refused and the result is a TLSSocket wherever it is
  imported, in new code too. Should the file stop working, seven suites
  fail the type check.
- ripgrep 15.2.0 goes on PATH for the second full run, the one on bubblewrap
  0.12.0, in place of a list of thirteen suites kept by hand. Every suite
  now sees it, a new one included, and the step, its list and the check
  that each name exists go. That run varies two tools at once.
- One step installs both releases, so the script is a function in it.
- Comments are cut to what the next reader needs.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FydxqUESqxwSCVFnhk3gfv
Co-authored-by: ronleizrowice <ronleizrowice@anthropic.com>
@ronleizrowice-ant
ronleizrowice-ant enabled auto-merge (squash) October 20, 2026 12:10
claude and others added 2 commits November 6, 2026 01:52
Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FydxqUESqxwSCVFnhk3gfv
Co-authored-by: ronleizrowice <ronleizrowice@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FydxqUESqxwSCVFnhk3gfv
Co-authored-by: ronleizrowice <ronleizrowice@anthropic.com>

This branch has not been deployed

No deployments
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