Conversation
… `latest` dist-tag DefinitelyTyped published the @types/node backport lines (25.9.6, 24.13.4, 22.20.2) right after 26.5.1 on 2026-09-09, and the npm `latest` dist-tag followed the last publish: it now points at 22.20.2. The fixture resolved `@types/node@latest`, so every run type-checked bun-types against @types/node 22, whose modules are declared as "tls" with a "node:tls" re-export (the reverse of 25+). The bun-types augmentations of "node:tls" and "node:fs/promises" then miss the primary declaration and tsc reports seven errors that no bun-types change can fix. The test now asks the registry for the version list, picks the highest release, and pins the fixture copy to it through `resolutions`. A new case asserts that the installed @types/node is that version, and the diagnostics assertion names the version it checked against.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Essentials Run ID: 📒 Files selected for processing (2)
💤 Files with no reviewable changes (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 1 remains after this review. WalkthroughThe bun-types integration test now selects the newest stable Changes
Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to The test selects and verifies the newest stable 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
|
Status Reproduced on main with the system bun: The test now resolves the highest published One thing for a maintainer to weigh, outside this PR: while the tag points at the 22.x line, a fresh |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — test-only CI fix that swaps the fixture's static @ types/node@ latest resolution for a dynamically computed highest-release pin.
What was reviewed:
newestRelease:versions --json→ filter prereleases →Bun.semver.ordersort →.at(-1)picks the highest; throws if empty.beforeAll: writesresolutionsinto the copied fixture (not the checked-in one) beforebun add, so the deleted staticresolutionsblock is still enforced at runtime.- The extra registry call matches this file's existing pattern (it already
bun adds from the live registry); no new hermeticity regression. No CODEOWNERS match for the changed paths.
Extended reasoning...
Overview
This PR touches only the bun-types integration test (test/integration/bun-types/bun-types.test.ts) and its fixture package.json. It replaces the fixture's static "resolutions": {"@ types/node": "latest"} with a dynamic resolution computed in beforeAll: bun pm view @ types/node versions --json, filter out prereleases, sort with Bun.semver.order, take the last entry, and inject that exact version as a resolutions pin into the copied fixture before installing the packed bun-types tarball. It also threads the resolved version into the typeTest failure message and adds a test asserting the installed node_modules/@ types/node/package.json version matches the pin. The motivation — DefinitelyTyped publishing backport lines after the current line and leaving the latest dist-tag on an older major — is documented in a JSDoc with a concrete dated example.
Security risks
None. This is test infrastructure only; no production code, no auth/crypto/permissions paths, no user-facing API surface. The only external interaction is a read-only registry metadata query, which is consistent with what this file already does (it already runs bun add against the live registry to install @ types/node and typescript).
Level of scrutiny
Low. The change is small, self-contained, and confined to a single integration test file plus its fixture manifest. The logic is straightforward and the invariant the deleted static resolutions block enforced (pin @ types/node for the fixture) is preserved — now more robustly, since it no longer trusts the latest dist-tag. The changed paths are not covered by CODEOWNERS (which lists *.d.ts and /packages/bun-types/, neither of which match here). The expect(value, message) two-arg form is already used elsewhere in the test suite.
Other factors
The one repo-guideline tension is the "never contact live registries" rule from REVIEW.md, but this file is already built around live-registry installs (bun add in beforeAll and createIsolatedFixture) — that is the whole point of this integration test, and the new bun pm view call adds no new class of external dependency. The JSDoc on newestRelease is load-bearing (explains a non-obvious "why" a future reader would otherwise spend time rediscovering), fitting the comment guidance. The PR description states 22/22 pass with system Bun vs. 11/22 on main, which is the expected shape for a fix to an upstream-caused CI break.
|
Updated 5:55 PM PT - Sep 15th, 2026
✅ @robobun, your commit b467fe2d42598e6e8e5f4e4af0e7869fc27d31fe passed in 🧪 To try this PR locally: bunx bun-pr 42230That installs a local version of the PR into your bun-42230 --bun |
|
shouldn't we instead pin the specific version bun matches, so 26.3.0? |
…types-node-26.5.1
…n Bun reports The fixture now pins @types/node 26.3.0 in package.json#resolutions, the release for NODEJS_VERSION (26.3.0), and no longer resolves a floating version. An upstream publish or a moved dist-tag can then no longer fail every PR that touches bun-types. A new case fails when the pin and NODEJS_VERSION disagree on major.minor, and checks that the installed @types/node is the pinned one. The bun-types workflow also runs when scripts/build/deps/nodejs-headers.ts changes, so the PR that bumps the Node.js version meets that case itself. The lib.dom case expects the 24154.ts diagnostic that @types/node before 26.5.0 produces (TS2322 on stream()), as it did before #41884.
robobun
changed the title
latest dist-taglatest dist-tag
|
@dylan-conway Yes, that is better. A fixed pin makes this check reproducible, and a floating version has now failed unrelated PRs two times in one week (26.5.0, then the moved
The cost of the pin: this test no longer reports a break that a newer @types/node causes for fresh installs. The PR that bumps the pin reports it.
|
There was a problem hiding this comment.
No new blocking issues. 1 optional suggestion (a nit or a note on pre-existing code) was found and not posted. Nothing in this review needs a push before merging.
One verified lower-impact observation (a convention, logging or cleanup point) was not posted.
|
actually this test is to confirm the latest types work with bun right? maybe we switch back |
This test exists to confirm that bun-types works with the newest @types/node (#22588), so a fixed pin does not fit it. Go back to the newest release on the registry: the test reads the version list with `bun pm view`, takes the highest release, and pins the fixture copy to it. It does not use the `latest` dist-tag, which points at the 22.x backport line since 2026-09-09. This removes the 26.3.0 pin, the case that tied the pin to NODEJS_VERSION, and the nodejs-headers.ts path filter of the workflow. The lib.dom case expects the Blob.textStream() diagnostic of @types/node 26.5 and later again.
…types-node-26.5.1
robobun
changed the title
latest dist-taglatest dist-tag
|
@dylan-conway Yes, that is its purpose. #22588 added the Switched back in d0b5b50. The diff against main is the same as in the first version of this PR:
One limit stays, by design: a new @types/node release that changes a diagnostic (as 26.5.0 did, #41884) still fails this test on the next PR that touches bun-types.
|
…mmonJS (#42590) ### What does this PR do? Finishes `Bun.ModuleGraph` (experimental): many instances of one app in one process, with one copy of its compiled code. ```ts const graph = new Bun.ModuleGraph({ globals: { process: tenantProcess }, onError }); const app = await graph.import("./app.ts"); await graph.run(() => app.handle(request)); graph.dispose(); ``` The whole API: `new Bun.ModuleGraph({ globals?, onError? })`, `graph.import(specifier)`, `graph.run(fn, ...args)`, `graph.dispose()` (also `Symbol.dispose`), and the static `Bun.ModuleGraph.current`. Documented in `docs/runtime/module-graph.mdx`; typed in `packages/bun-types/bun.d.ts`. - **Every graph has a context of its own for timers and I/O.** What its code opens is the graph's: timers, `Bun.serve` / `Bun.listen` servers, sockets, `fetch`, WebSockets, watchers, child processes (including `Bun.$`), workers, database connections, files opened through `Bun.file().writer()`, `bun:sqlite` and `node:sqlite`, and graphs its code makes. The context follows the code the way `AsyncLocalStorage` does: through `await`, timers, socket handlers and event listeners. - **`graph.dispose()` behaves like `worker.terminate()`.** Everything the graph opened is closed and what it had in flight is discarded: nothing of the graph is called from the event loop again and its pending promises stay pending. No `close` / `onExit` / `'error'` callbacks, no rejections. From then on `graph.import()`, `graph.run()` and the graph's `require()` fail with `ERR_INVALID_STATE`. - **What a disposed graph's leftover code starts does not start.** Microtasks and `process.nextTick` callbacks the graph had already queued still run once (the engine's promise machinery is not changed). What they ask for does not leave the process: a connection is not dialed, a UDP socket is not bound, nobody can connect to a server it listens with, and the promise stays pending instead of rejecting, so code that retries on failure stops there. The exception is a child process, which is started and then killed. - **`graph.run(fn, ...args)`** calls `fn` inside the graph's context. A function runs in its caller's context, whichever graph defined it, so code of a graph that the host calls directly runs as the host. `Bun.ModuleGraph.current` is the graph whose context the caller is in. - **The first module a graph `import()`s is its main module**: `import.meta.main` and `require.main === module` are true in it and in no other module of the graph. - **CommonJS is per graph**: a graph has its own `require.cache`, and `require()`, `createRequire()` and `new Module()` belong to the context they are called in. - **Errors belong to the context they happen in.** An uncaught exception goes to the `onError` of the graph in whose context it was *thrown*, whoever wrote the code and whatever it unwound through afterwards; that includes a synchronous throw that escapes `graph.run(fn)` when the caller does not catch it (`run()` still rethrows to its caller). An unhandled rejection goes to the graph in whose context the promise was rejected. An error no code threw (a socket of the graph's failing with no `error` handler) goes to the graph that opened the thing. `onError` runs in the context the graph was made in, so what it throws, rejects or starts is its maker's, and what escapes it goes on to the maker and does not come back to the same handler. A graph with no `onError` hands its errors to its maker's. An unhandled `graph.import()` promise is its caller's. - **Shared, and therefore not the graph's**: `globalThis`, `process` (unless replaced through `globals`), plugins, native addons, `http.globalAgent` / `https.globalAgent` (a request with no `agent` uses a connection that is not the graph's; code whose connections should go with its graph passes an `Agent` it made), listeners a graph puts on something the host owns, and file descriptors that script holds (one from `fs.openSync()`, or inside a `FileHandle` or an fs stream): those are the script's to close before `dispose()`. `node:quic` endpoints are not covered yet. It is not a security sandbox. How it is built: 1. **Contexts.** A Rust `ScriptExecutionContext` holds an intrusive list of `AbortHandle`s (membership in the context, plus optionally the `AbortSignal` script passed). It replaces the `ActiveHandle` registry: a server, socket, watcher, fetch, child, upload, cron job and so on embeds a handle and is told to stop when its context stops. The VM's own script has a root context; `bun test --isolate`, worker termination and VM teardown stop the root context's handles through the same list. A `ContextId` is the C++ `ScriptExecutionContextIdentifier`; Rust allocates none. Every C++ `ScriptExecutionContext` is made together with its Rust half. 2. **`JSModuleGraph`.** A module loader of its own whose module scope (an overlay over the global lexical environment) holds the host's `globals`, the loader and the graph. Graphs with the same `globals` names share one overlay `SymbolTable`, which is what JSC keys shared module executables on. 3. **The current context rides the async context.** Entering a graph pushes a frame shaped like an `AsyncLocalStorage` frame whose storage is the graph; native code finds the current graph by walking the chain to the first such frame. Until a graph has been constructed every lookup is one flag test. Native functions that start work take one `&JsThread` (the global and the calling script's context) instead of a bare global. 4. **Work that returns to the JS thread says whose script it continues.** A `Task` carries its context (the `Taskable` trait makes every task type say so) and the dispatcher releases a task whose context has stopped. Beyond that there are three places where native code reaches script and a disposed graph is turned away, instead of one per completion: wrapped callbacks and DOM listeners (`shouldDropCallbackOfStoppedModuleGraph`), settling a promise (`JSPromise::resolve` / `reject`), and `JSValue::call`. 5. **Native dispatchers that report errors themselves run inside their owner's context** for the whole dispatch: `Bun.serve`'s request paths and the `node:http` request path, `Bun.listen` / `Bun.connect` handlers (`Handlers::enter`), UDP callbacks, and the server's websocket entry points. That is what gives the owner an error nobody threw, and a thrown value a dispatcher has already unwrapped. 6. **Callbacks script stores on something long-lived** continue the graph whose script stored them: most are stored with the async context, as before; the `node:http` server socket's `ondata` / `ondrain` / `onclose` and `RedisClient`'s `onconnect` / `onclose` are stored with only the frame that entered the graph, and bare outside a graph, so that without a graph they run exactly as they did. Long-lived JS objects that open things later (`http.Agent`, `Bun.SQL`, `PerformanceObserver`) remember the frame of the graph they were made in and do that work in it. `Agent.createSocket` hands a socket that came through a proxy tunnel to the waiting request in the requester's frame when the tunnel's callbacks ran in another graph's context. 7. **CommonJS.** A `JSCommonJSModule` has one `m_moduleGraph`. Its wrapper is evaluated normally and re-created over the graph's overlay; `CommonJS.ts` reads the module's require map once and passes it down. 8. **Builtin modules are evaluated in the realm's context** whoever loads them first, so what `node:http` and friends set up at load is never a graph's. The HTTP `date` cache is keyed by the second instead of cleared by a timer, because a timer belongs to whoever set it. 9. **Leftover code is refused where work starts, not where it completes.** Native entry points that would put something on the network ask the calling script's context first and return a pending promise or an object that never connects (`fetch`, `Bun.connect` / `Bun.listen`, UDP, `WebSocket`, the Redis client's `connect()`, the functions that put an S3 request on the HTTP thread); a DNS answer for a stopped context settles nothing. Four places in Node's and Bun's own JavaScript that start or announce something from `process.nextTick()` check for a disposed graph: `'listening'` in `node:net` and `node:http`, a child's spawn in `node:child_process`, and the function both SQL drivers dial through. 10. **N-API.** Async work and threadsafe-function completions queued by a graph run in its context. After dispose they still run, since the addon owns memory only it can free, but the functions that run script return `napi_cannot_run_js`, and resolving a deferred frees it and settles nothing. A threadsafe function is its maker's. 11. **Errors.** `JSC::Exception` records the async context it was first thrown in (oven-sh/WebKit#689), and `Bun__ModuleGraph__handleUncaughtException` reads the graph from it; a bare value is attributed to the context that is current where it is reported. `process.nextTick`'s loop no longer catches in JS: what a tick throws reaches `JSNextTickQueue::drain` as it was thrown, with the tick's frame still current; `drain` reports it there, restores the async context and resumes the queue, so the order of ticks, handlers and microtasks is unchanged. The process's `uncaughtException` / `unhandledRejection` handlers run in the realm's context whichever graph failed. 12. **`Bun.cron()`.** A tick's promise reactions own the job through a `NativePromiseContext` cell, so a tick still waiting when its graph is disposed, or settled by the host afterwards, neither leaks nor touches a freed job. Behaviour that changes for programs that never construct a `Bun.ModuleGraph` (each compared with the last release): - `bun test --isolate` is stricter: more of what a file left running is stopped at the swap: `Bun.cron` jobs, S3 multipart uploads (their promises reject with `AbortError`), `Bun.FetchSession` idle sockets, N-API threadsafe functions' hold on the loop, and a `Bun.build()` still in flight (its promise stays pending). Handles armed by the outgoing file's close handlers are stopped at the same swap. A `crypto.subtle` promise left pending across the swap no longer settles in the next file. - An S3 multipart upload whose final commit fails now sends the abort request, as one that fails earlier already did. - A closed `RedisClient` that had subscriptions no longer keeps the process alive. - An `fs.promises.open()` whose result is never delivered (its realm was retired, its worker terminated) closes the descriptor instead of leaking it. A `Bun.write()` promise in flight at an `--isolate` swap, and an S3 stream upload whose pump promise is collected unsettled, are released. - `listener.stop()` followed by `listener.stop(true)` closes the accepted connections (the second call used to do nothing), and `listener.data` is cleared once a stopped listener's last connection closes. - `BroadcastChannel` drops its hold on the event loop when its context stops. - An uncaught error thrown from a `process.nextTick` callback is printed from the exception's throw site, as one from a timer already was; a thrown non-`Error` value is printed with its source frame. - An `enterWith()` made inside an `uncaughtException` / `unhandledRejection` handler ends with the handler. - Windows: a named-pipe client still connecting when its context stops is cancelled. - `"ModuleGraph" in Bun` is true. What async context an `uncaughtException` handler, a `node:http` socket listener or an `http.Agent` request callback sees is the same as in the last release; `test/js/node/async_hooks/AsyncLocalStorage.test.ts` pins it, with node's answers as the expectations. The WebKit pin is the one `main` already uses. ### How did you verify your code works? CI on every platform, and targeted runs locally on a debug+ASAN build; GC behaviour on release builds. - `test/js/bun/module-graph/`: `module-graph.test.ts` (the API, module semantics, CommonJS, error attribution and `onError`), `module-graph-isolation.test.ts` (what a graph owns, what dispose closes, what leftover code can and cannot do, per kind of I/O), `module-graph-io.test.ts` (the context following code; a test that walks some fifty event sources and checks who is given what their listener throws; an error nobody threw; two graphs on one connection; handlers of a disposed graph on a client of the host's; a request through the host's `Agent`, direct and through a CONNECT proxy), `module-graph-matrix.test.ts` (kinds of work × who started it × when it is disposed), `module-graph-workers.test.ts` (graphs × workers and every order of `terminate()` / `dispose()` / `process.exit()`), `module-graph-compile.test.ts` (single-file executables), `module-graph-gc.test.ts` (what keeps a graph alive and that nothing else does), `module-graph-callbacks.test.ts`. - New tests were checked to fail on the build without the change they cover. - Outside that directory: `test/js/node/async_hooks/AsyncLocalStorage.test.ts` (which store an `uncaughtException` handler reads, for a callback that throws directly and one that throws inside a store of its own; `node:http` server socket and client request events; an `Agent` has no new visible property), `test/js/node/process/process-nexttick.test.js` (a tick that throws does not stop the ticks after it, and the order), `test/cli/test/isolation.test.ts` (what a leaked listener's close handler opens is closed before the next file; the default DNS resolver answers in every file), `test/napi/napi.test.ts` (completions for a graph), `test/js/web/websocket/websocket.test.js` and `test/bake/dev/production.test.ts` (a `WebSocket` made inside a ShadowRealm / while a page is prerendered), `test/integration/bun-types/fixture/module-graph.ts`. - Two existing tests changed. `test/js/bun/test/bun_test.test.ts`: a string thrown from a tick is now printed with its source frame, like the string thrown from `describe()` a few lines above it in the same snapshot. `test/js/bun/http/serve-pending-promise-abort-leak.test.ts` "client abort of a streaming Response releases the body stream it held" gives each collection in its retry loop a fresh turn, as the h2 stream-release tests do since #40966: `WeakRef.prototype.deref()` keeps its target alive until the end of the job, and every collection after the first ran in the same job as a `deref()`, so its 20 passes were one. On this branch's x64-asan build that first pass missed: nothing references the last stream (no incoming edge, no root in a heap snapshot), but one stack word in an AddressSanitizer redzone of JSC's live `runInternalMicrotask` frame still holds a callee-saved register the release path itself pushed while the stream was in it; zeroing that word in a debugger frees the stream. Built from `main` the same push lands 288 bytes away, on a local's slot that is rewritten. - `cargo clippy`. Known: - The "TypeScript types" check fails for a reason unrelated to this change (#42230). - Three tests are `todo`. `module-graph-isolation.test.ts` "node:cluster: what a worker it forked tells the primary…": in about 2% of runs the fixture's probe of the worker's port right after `dispose()` connects instead of being refused, and the fixture then never exits; whether the primary's listener for the worker is closed inside `dispose()` or a turn later is not established. `module-graph-gc.test.ts` "a repeating timer keeps its graph alive; clearing it lets the graph go" is a todo on macOS x64 only, where it fails on every build: its heap snapshot shows the graph's cycle with nothing in the heap and no root reaching it, and what holds it is not established. "a dropped graph whose module keeps a FileHandle open is collected" is a todo on Linux aarch64 only, where it fails on every build and nowhere else (it passes on that build under emulation); what holds the graph there is not established. --------- Co-authored-by: Dylan Conway <dylan.conway567@gmail.com> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
This branch has not been deployed
3 participants
Problem
bun-typesworkflow fails on every PR that touchespackages/bun-types/**since 2026-09-09, 10 of 21 cases:fs.ts(6,10): error TS2305: Module '"fs/promises"' has no exported member 'exists'.and six more (example run). No bun commit caused it.latestdist-tag followed it (22.20.3 today, 26.6.1 is out). The fixture resolved@types/node@latest, so it checked bun-types against @types/node 22. That major declaresmodule "tls"with a"node:tls"re-export (25+ is the reverse), so the bun-types augmentations of"node:tls"and"node:fs/promises"miss.Fix
bun pm view @types/node versions --json, takes the highest release, and pins the fixture copy to it throughresolutions. A range does not help: bun resolves a range tolatestwhenever that tag satisfies it.@types/nodeis that version. The diagnostics assertion names it.bun test test/integration/bun-types/bun-types.test.ts(system bun), 22 pass against 26.6.1. main gives 11 pass, 10 fail.Background
bun-typesdepends on@types/node@*and augments its modules. An augmentation merges only into the module that holds the declarations, not into one that re-exports it.latestdist-tag when it satisfies the range (src/install/npm.rs:1918). Today*gives 22.20.3.@latesttag on npm does not point to the "latest" version microsoft/DefinitelyTyped-tools#443.Notes
@types/node@26.3.0, the release for the Node.js version Bun reports, with a case that tied the pin toNODEJS_VERSION. That made the check reproducible, but the test then no longer reported a break that a newer @types/node causes for fresh installs. After a second comment, d0b5b50 went back to the newest release. The diff against main is the same as in the first version.latestdist-tag.latestpoints at the 22.x line, a freshbun initorbun add -d @types/buninstalls@types/node@22.xthrough bun-types'*dependency (checked locally with bun 1.4.3). With the defaultskipLibCheck: truethe errors inside bun-types stay hidden, butimport { exists } from "fs/promises"andimport type { BunConnectionOptions } from "tls"stop type-checking, and the names thatoverrides.d.tscannot resolve degrade toany. Nothing in this PR changes that. It needs the upstream tag fixed. I found no upstream report as of 2026-09-10 18:00 UTC."@types/node": "*"inpackages/bun-types/package.json. A floor such as>=25would sidestep the tag for new installs, but a project that pins an older @types/node would then get a second, nested copy under bun-types and duplicate global declarations.fs.ts(6,10) TS2305,index.ts(42,28) TS2724 '"tls"' has no exported member named 'BunConnectionOptions',globals.d.ts(320,74) TS2694 Namespace '"node:util"' has no exported member 'TextEncoderEncodeIntoResult',overrides.d.ts(352,47) TS2552 ConnectionOptions,overrides.d.ts(390,67) TS2552 KeyObject,overrides.d.ts(394,88) TS2304 TLSSocket,test.ts(347,28) TS2554. The same fixture is clean under 26.5.1 and 26.6.1.time): 26.5.1 18:09:34Z, 25.9.6 18:09:44Z, 24.13.4 18:10:47Z, 22.20.2 18:10:54Z, all 2026-09-09. dist-tags on 2026-09-16:latest: 22.20.3,ts6.0: 26.6.1.tls.d.tsper line: 22.20.2"tls", 24.13.4"tls", 25.9.6"node:tls", 26.5.1"node:tls".TextEncoderEncodeIntoResultexists innode:utilfrom 25 on.process event methods with @types/node@24case is unaffected: it installs@types/node@24into its own directory and checks withskipLibCheck: true..github/workflows/bun-types.yml). The type-checking cases areskipIf(isDebug), so a debug build observes neither the failure nor the fix.[auto-merge] gate passed · iteration 3 · 2 files touched
passes on PR (with fix)
diff hotspot
gate history · 4 passed · 0 rejected · iteration 3
evidence per changed file