node:inspector: type the CDP adapter's JSC side against the protocol snapshot and typecheck it in CI - #39471
node:inspector: type the CDP adapter's JSC side against the protocol snapshot and typecheck it in CI#39471robobun wants to merge 4 commits into
Conversation
…ocol snapshot
cdp.ts read every JSC event and response as Record<string, any>, which is
how `params.module` kept being read after WebKit renamed it to `scriptType`:
the field silently became undefined in Debugger.scriptParsed.
Import the JSC protocol types from bun-inspector-protocol (type-only, so the
builtin bundler erases it) and use them for everything that crosses to the
backend: #sendToBackend is generic over the command name so request params
and onResult callbacks are typed per command, #translateBackendEvent takes a
discriminated union keyed on the event name, #translateResult narrows each
reshaped response via TranslatedResponses, and the console/stack trace
helpers and the scope/console lookup tables take the snapshot's types.
No runtime change; the only non-type edit drops the `|| {}` fallback on
Console.messageAdded's required `message` parameter.
|
Status: follow-up to #39051 requested by @alii. cdp.ts's backend side is typed against |
|
Updated 4:36 PM PT - Aug 17th, 2026
✅ @robobun, your commit 936d384c8fa36c7686f6a3e8e64dc1bcd7d869b8 passed in 🧪 To try this PR locally: bunx bun-pr 39471That installs a local version of the PR into your bun-39471 --bun |
bun-inspector-protocol.test.ts already checks that the snapshot matches what this build of bun sends. Add the other half: typecheck cdp.ts against the snapshot (under node, since the debug build takes tens of seconds just to load typescript.js), and check that renaming an event parameter, a response field and a request parameter in the snapshot is reported at cdp.ts's uses of them, so the typecheck cannot silently stop covering the JSC side.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
robobun
changed the title
|
Since the first revision: the added comments in cdp.ts were removed or cut to single lines (fc03f27, 936d384), and |
2 participants
Follow-up to #39051.
Problem
src/js/internal/inspector/cdp.tsreads every JSC event and response asAnyObject(Record<string, any>):#translateBackendEvent(method, params: AnyObject),#translateResult,#translateConsoleMessage,#translateStackTrace, theonResultcallbacks, and the request objects it builds.params.modulekept being read after WebKit renamed it toscriptType(node:inspector: derive scriptParsed isModule and scriptLanguage from JSC's scriptType #39051): the read type-checked and the field silently becameundefinedinDebugger.scriptParsed.packages/bun-inspector-protocol/src/protocol/jsc/index.d.tsis generated from the pinned WebKit (bun-inspector-protocol: regenerate the JSC protocol snapshot from the pinned WebKit #39110) and already describes every shape this file touches, but nothing insrc/jsreferenced it, and nothing in CI typecheckssrc/js(bun run typecheckcovers neither the root solution file's zero sources nor, viatest/tsconfig.json, this file;tsc -p src/jshas unrelated pre-existing errors).Fix
import type { JSC } from ".../bun-inspector-protocol/src/protocol/jsc/index.d.ts"in cdp.ts. Type-only, so the builtin bundler erases it; the bundledinternal/inspector/cdp.jsis unchanged apart from the one edit noted below.#sendToBackend<M extends keyof JSC.RequestMap>(method: M, params?: JSC.RequestMap[M], ..., onResult?: (result: JSC.ResponseMap[M], error?) => void): command names, the request objects built for JSC (excess property checks catch a renamed parameter) and the chainedonResultcallbacks are typed per command. The two places that forward client params verbatim cast them toJSC.RequestMap[typeof method].#translateBackendEvent({ method, params }: BackendEvent):BackendEventisJSC.EventMapturned into a union discriminated onmethod, so eachcasenarrowsparamsto that event's type (JSC.Debugger.ScriptParsedEvent,PausedEvent,BreakpointResolvedEvent,JSC.Console.MessageAddedEvent). TheBreakpointpause reason readsdataasJSC.Debugger.BreakpointPauseReason, asbun-debug-adapter-protocoldoes.#translateResult(method, response):TranslatedResponsesrecords which JSC response answers each reshaped CDP command (not always the namesake:getPossibleBreakpointsis served bygetBreakpointLocations,evaluatewithawaitPromisebyRuntime.awaitPromise); each case narrows withresponse as TranslatedResponses[typeof method].#translateConsoleMessage/#translateStackTracetakeJSC.Console.ConsoleMessage/JSC.Console.StackTrace;SCOPE_TYPE_MAP,CONSOLE_TYPE_MAPandCONSOLE_LEVEL_MAPare keyed by the snapshot's enums, so a renamed scope type, console type or level fails too.handleBackendMessageparses into a smallBackendMessageenvelope type (error.codeis a number, perBackendDispatcher::sendPendingErrors).#dispatchClientCommandparams, everything passed to#replyToClient/#emitToClient) stayAnyObject, as requested.?? 0,?.) are left in place even where the snapshot marks the field required; the one that does not type-check,params.message || {}onConsole.messageAdded(wheremessageis required), is dropped.test/cli/inspect/bun-inspector-protocol.test.tsalready checks that the snapshot matches what this build of bun sends. It now also typechecks cdp.ts against the snapshot (cdp-protocol-types-fixture.mts, the options oftsc -p src/jsrestricted to this file), so regenerating the snapshot after a WebKit bump reports each field cdp.ts still reads or sends under the old name. It then renames an event parameter (scriptType), a response field (wasThrown) and a request parameter (doNotPauseOnExceptionsAndMuteConsole) in a copy of the snapshot and checks that each is reported at cdp.ts; against the currentAnyObjectversion of cdp.ts nothing is reported, so this half fails without the cdp.ts change. The fixture runs under node (present on every CI lane, and already used this way by other tests) because the debug build of bun takes about 20 seconds just to load typescript.js; the test takes ~0.6s.bun bd test test/cli/inspect/bun-inspector-protocol.test.ts: passes; fails as described above with main's cdp.ts swapped in.bun bd test test/js/node/inspector/inspector.test.ts(24 pass) andinspector-profiler.test.ts(45 pass).tsc --noEmit -p src/js: no diagnostics ininspector/before or after; the project's pre-existing error count elsewhere is unchanged. Renaming five further snapshot fields by hand produced 0 errors in cdp.ts on main and 7 on this branch (output below).Sequencing
AnyObject), inspector: isolate Debugger/Runtime state between concurrent CDP clients #35752, inspector: collapse JSC async wrapper frames in CDP Debugger.paused #36457. Opened as a draft so it is not merged under them; the intent is to rebase it on whatever lands first and type the code those PRs add. test: run the node:inspector child-process tests concurrently and assert their full output #39031 is test-only and does not conflict.Background
bun-inspector-protocol/src/protocol/jsc/index.d.tsis a generated TypeScript description of the JSC protocol: one type per event, request and response (JSC.Debugger.ScriptParsedEvent,JSC.Runtime.EvaluateRequest, ...) plusEventMap/RequestMap/ResponseMapindexing them by protocol method name.scripts/generate-protocol.tsregenerates it from the WebKit versionbun bdlinks against, which is what makes a type error here equivalent to "WebKit changed this field".methodhere); TypeScript narrows the whole object, includingparams, inside aswitchon that property.typeof methodinside acaseis the narrowed literal type, soTranslatedResponses[typeof method]is the response type for exactly the commands listed in that case.src/js/builtins.d.tsdeclares the$-prefixed intrinsics builtins use (map.$get, ...). The fixture loads it but drops its references to the build's codegen output (which is what typesrequire()per module and does not exist in a test checkout) and declaresrequire()loosely instead; only the JSC side is under test.Simulated snapshot renames (manual, in addition to the three the test performs)
Applied to
jsc/index.d.ts:scriptType->module, pause reason"Breakpoint"->"BreakpointHit",doNotPauseOnExceptionsAndMuteConsole->muteConsole,StackTrace.parentStackTrace->parent,GetPropertiesResponse.internalProperties->internals.On main:
tsc --noEmit -p src/js 2>&1 | grep inspector/prints nothing.On this branch:
[review] gate passed · iteration 1 · 3 files touched
fails on main (without fix)
passes on PR (with fix)
diff hotspot
gate history · 1 passed · 1 rejected · iteration 1
evidence per changed file