Skip to content

Remove dead code from bun-uws, the native BufferList class, built-in JS, the bake dev server assets, the shell, and the SQL crates - #40066

Merged
Jarred-Sumner merged 2 commits into
mainfrom
claude/farm/3be5706a/dead-code-uws-bufferlist-bake-sql
Aug 23, 2026
Merged

Jarred-Sumner merged 2 commits into
mainfrom
claude/farm/3be5706a/dead-code-uws-bufferlist-bake-sql

Conversation

@robobun

@robobun robobun commented Aug 22, 2026 •

Copy link
Copy Markdown
Collaborator

Scheduled dead-code sweep. Net -3275 lines (78 files, +112 / -3387; the source side is +32 / -3387, the rest is the test pin). No behavior change: every item below has zero references on main in src/, packages/, scripts/, test/ and the regenerated build/debug/codegen/, and the build passes without it.

About 2,350 of these lines carry over the removals from #37659, which was closed only for merge conflicts. Each of those was re-verified against current main (nothing gained a caller), and the diff applies to current main with no conflicts. The rest comes from fresh scans of the shell, SQL, bake and built-in JS areas, which no open dead-code PR covers.

bun-uws (packages/bun-uws/src, -1168)

Bun reaches uWS only through the C shim in src/uws_sys/ and eight bindings files, so those includers are the complete set of callers. Checked against every one of them:

  • Whole headers nothing includes: ClientApp.h, HttpError.h (superseded by HttpErrors.h), Multipart.h and MessageParser.h (its only includer), ProxyParser.h (only reachable under UWS_WITH_PROXY, which no build defines; grep UWS_ build/debug/compile_commands.json is empty), plus every #ifdef UWS_WITH_PROXY block and the reserved / proxyParser parameters that only carried the proxy parser into getHeaders().
  • WebSocketBehavior::subscription and WebSocketContextData::subscriptionHandler: the only place a WebSocketBehavior is built (uws_ws() in the shim) never sets it, so every dispatch site in WebSocket::end/subscribe/unsubscribe and WebSocketContext::onClose was unreachable. TopicTree::unsubscribe drops the newCount only those sites read.
  • HttpRequest: the per-route std::map of parameter offsets (setParameterOffsets, getParameter(name); every caller uses the index overload), getQuery() with no key, isShortRead, notFieldNameWord / hasMore / hasBetween.
  • HttpContext::HTTP_IDLE_TIMEOUT_S, HttpFlags::isAuthorized (the live flag is the per-socket one in AsyncSocketData), HttpResponse::getHttpResponseDataS, HttpResponse::endWithoutBody, HttpResponse::overrideWriteOffset (the shim calls setWriteOffset).
  • App::publish(unsigned char), two App::listen overloads, the UWS_NO_ZLIB block; two H3App::listen overloads; Http3Request::isAncient / getCaseSensitiveMethod; nine Http3Response members whose uws_h3_res_* shims are stubs; the write-only Http3ResponseData::totalSize; Loop::integrate, Loop::setSilent and LoopData::noMark; the UWS_NO_ZLIB / UWS_MOCK_ZLIB mock streams and UWS_USE_LIBDEFLATE guards in PerMessageDeflate.h (the macro is defined unconditionally three lines above its first use).
  • HttpContext::layoutAssert() and WebSocketContext::layoutAssert() were never called, and as static members of class templates their static_asserts never ran. The HttpContext one guarded nothing. The WebSocketContext one guards the layout WebSocket::getContextData() depends on (group first, data right after), so it becomes a RELEASE_ASSERT in create(): offsetof cannot express it because the struct is not standard-layout, and the layout can differ per ABI, so the check runs once per context in every build.
  • misc/ (upstream README assets, demo cert.pem / key.pem), and the .gitattributes, .dockerignore, .cursorignore entries for a fuzzing/ directory that no longer exists.

Native JSBufferList (src/jsc/bindings, -641)

JSBufferList.cpp / .h, the lazy class structure on GlobalObject, its three accessors, and the DOMIsoSubspaces / DOMClientIsoSubspaces slots. The JS streams implementation replaced it; the only remaining mention of "BufferList" in the tree is a comment in internal/streams/readable.ts. The sources are globbed, so no build-script change is needed.

Built-in JS (src/js, -508)

  • internal/util/inspect.d.ts (ambient types for node-inspect-extracted, which nothing imports) and src/js/.gitignore (ignores src/js/out, which codegen has not written since the build moved under build/).
  • private.d.ts: the BunFS / BunFSWatcher types (Bun.fs() no longer exists), Bun.TOML, Bun.tty.
  • QuicStream.prototype[kSendHeaders] and its symbol, the unused get [kVerifyPeer] (the setter stays; session[kVerifyPeer] = … writes it), colors.ts yellow / gray / clear / reset, the runtime SSLMode export of internal/sql/shared.ts (only imported as a type), the REPL_MODE_* re-exports of internal/repl/utils.js (repl.js takes them from internal/repl/mode), Dequeue#clear(), the write-only listeningId in net.ts, and the kGetNativeReadableProto symbol in internal/shared.ts (its last reader went in 032713c).

Bake (src/runtime/bake, -830)

Shell (src/runtime/shell, src/shell_parser, -51)

  • Base::end_scope(), an empty function, and its 12 call sites in states/*.rs. Script::deinit_from_interpreter only called it and goes too.
  • ShellErr::InvalidArguments: never constructed (only matched), with its four match arms.
  • bun_shell_parser dependencies scopeguard, const_format, enum-map, enumset, typed-arena, bun_collections: no use or path reference in the crate.

SQL (src/sql, src/sql_jsc, -22) and hawk.toml (-104)

  • bun_sql dependencies const_format, enum-map, enumset, libc; bun_sql_jsc dependencies const_format, enum-map, enumset, libc, bun_uws_sys, and a commented-out bun_runtime edge that would be a dependency cycle (bun_runtime depends on bun_sql_jsc).
  • pub use re-exports nothing imports: ArrayBuffer, JSCell, host_fn in sql_jsc/jsc.rs (the proc-macro is used by its absolute path), mysql::{MySQLConnection, MySQLQuery, MySQLStatement} (every consumer imports the leaf module), postgres::SASL.
  • hawk.toml: 13 overrides for bun_sql::mysql::status_flags::StatusFlag::* variants that Remove dead code from shell, bake dev server, sql, install, cli, css, and websocket client #37229 removed; StatusFlag has one variant left.

Rust (-18)

zig__ModuleInfo__destroy and zig_log in src/bundler/analyze_transpiled_module.rs: extern "C" exports that no C++ declares or calls (BunAnalyzeTranspiledModule.cpp frees through zig__ModuleInfoDeserialized__deinit), with no hit in vendor/WebKit either.

Verification

  • rg for every symbol across src/, packages/, scripts/, test/, docs/ and build/debug/codegen/; .classes.ts files, src/codegen/, string-named lookups ($getByIdDirectPrivate(this, "…"), $zig / $cpp) and token-paste macros (name##PublicName) were checked by hand. Two candidates failed those checks during the run and stay: BunBuiltinNames mockedFunction (reached through BUN_COMMON_STRINGS_EACH_NAME) and writer (reached through $getByIdDirectPrivate(this, "writer") in ConsoleObject.ts).
  • bun bd builds. cargo check --workspace passes on all 12 target triples. cargo fmt --check, prettier, oxlint and clang-format are clean. tsc -p src/js reports the same errors before and after (only line numbers move).
  • test/js/bun/websocket/websocket-server.test.ts gains a pin for the pub/sub paths this PR edits: with two sockets on shared topics, unsubscribe() of a topic the socket never joined (or that does not exist) reports false and changes nothing, leaving the last topic frees the subscriber and a later subscribe() counts again, and close() drops the socket from every topic without touching the other socket. Because this PR only deletes unreachable code, the pin passes before and after by design.
  • bun bd test: bun/websocket/websocket-server.test.ts, web/websocket/websocket-permessage-deflate.test.ts, bun/http/bun-serve-routes.test.ts, bun/http/serve-http3.test.ts, node/http/node-http.test.ts, node/quic/quic-stream.test.ts, node/net/node-net-server.test.ts, bun/util/inspect.test.js, six bun/shell/*.test.ts files, sql/sql-mysql*.test.ts, sql/sql-mariadb-json.test.ts, sql/postgres-binary-numeric.test.ts, sql/sql-connect-error-reporting.test.ts, node/stream/node-stream.test.js, node/console/console-table-iterators.test.ts, web/console/console-log.test.ts, internal/fifo.test.ts, bake/dev/esm.test.ts, bake/dev/hot.test.ts, bun/repl/repl.test.ts (mode tests), node/test/parallel/test-repl-definecommand.js. Details on the local failures that also fail on the released bun are under Notes.

Looked at and left alone

  • src/runtime/api/bun/h2/: the outbound half (Connection::send_header_block / send_data / send_push_promise, hpack::Coder::encode, SendWindow::available) is only exercised by its unit tests. The module doc describes it as the engine that will replace h2_frame_parser.rs, so it is in progress, not dead.
  • The bake visualizer protocol residue: MessageId::Visualizer, the IncrementalVisualizer topic, and the empty emit_*_visualizer_* methods and their timer. All reachable; removing them is a protocol change that also touches the generated generated.ts ids.
  • ColumnDefinition41::{fixed_length_fields_length, decimals} are decoded and never read, but they mirror the MySQL wire struct; left in place.
  • bun_sql::mysql::StatusFlags's Display impl prints nothing, but a debug log in MySQLConnection.rs still formats it.
Notes

How the candidates were found: the dead-code PRs that were closed for merge conflicts were re-applied against current main and every removal re-verified (#37659 applied with no conflicts; its assertion_error.ts and util.ts hunks had already landed via #39924 and #39628 and are not in this PR). Read-only scans of src/runtime/shell + src/shell_parser, src/sql + src/sql_jsc + src/runtime/node, src/runtime/bake + src/runtime/api + src/runtime/image, and src/js + src/jsc/modules produced the fresh items. src/runtime/node, src/runtime/api (outside h2/), src/runtime/image and src/jsc/modules came back clean.

Local test failures, all reproduced with USE_SYSTEM_BUN=1 (the released bun) in the same container, so none is caused by this diff:

  • websocket-server.test.ts: eight it.concurrent cases that overlap the 300,000-message (benchmark) test time out under the debug ASAN build in this container (the whole file takes 64 s here against a 6.4 s CI budget); each passes when run with -t.
  • node-http.test.ts "request via http proxy": ECONNREFUSED from localhost resolution in this container.
  • bunshell.test.ts -c, -f character device, and the seven "stdin redirect from a zero-length buffer" cases: this container's /dev/null is a regular file, not a character device.
  • sql-mysql-binary-null-indexed.test.ts: Access denied for user 'root'@'localhost' against the local MariaDB.
  • fifo.test.ts "pushing and shifting a lot of items": a 10 s perf budget exceeded under ASAN; the only fifo.ts change is the removal of the unused clear() method.

Files shared with open PRs, different hunks: src/jsc/bindings/ZigGlobalObject.cpp / .h (#39581 removes other accessors; its HTMLRewriterSinkPrototype() hunk sits one blank line above the JSBufferList accessors removed here), Cargo.lock (#39618 edits other package blocks).


no test proof · iteration 4 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/bun/websocket/websocket-server.test.ts

@coderabbitai

coderabbitai Bot commented Aug 22, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 361e9ca9-7304-4e44-8669-4f7455fecc6d

📥 Commits

Reviewing files that changed from the base of the PR and between 420d049 and 30a80fd.

⛔ Files ignored due to path filters (7)
  • Cargo.lock is excluded by !**/*.lock
  • packages/bun-uws/misc/Single-threaded echo performance over varying message sizes, Linux.png is excluded by !**/*.png
  • packages/bun-uws/misc/bigshot_lineup.png is excluded by !**/*.png
  • packages/bun-uws/misc/cert.pem is excluded by !**/*.pem
  • packages/bun-uws/misc/key.pem is excluded by !**/*.pem
  • packages/bun-uws/misc/logo.svg is excluded by !**/*.svg
  • packages/bun-uws/misc/websocket_lineup.png is excluded by !**/*.png
📒 Files selected for processing (71)
  • .cursorignore
  • .dockerignore
  • .gitattributes
  • hawk.toml
  • packages/bun-uws/misc/READMORE.md
  • packages/bun-uws/src/App.h
  • packages/bun-uws/src/ClientApp.h
  • packages/bun-uws/src/Http3App.h
  • packages/bun-uws/src/Http3Request.h
  • packages/bun-uws/src/Http3Response.h
  • packages/bun-uws/src/Http3ResponseData.h
  • packages/bun-uws/src/HttpContext.h
  • packages/bun-uws/src/HttpContextData.h
  • packages/bun-uws/src/HttpError.h
  • packages/bun-uws/src/HttpParser.h
  • packages/bun-uws/src/HttpResponse.h
  • packages/bun-uws/src/HttpResponseData.h
  • packages/bun-uws/src/Loop.h
  • packages/bun-uws/src/LoopData.h
  • packages/bun-uws/src/MessageParser.h
  • packages/bun-uws/src/Multipart.h
  • packages/bun-uws/src/PerMessageDeflate.h
  • packages/bun-uws/src/ProxyParser.h
  • packages/bun-uws/src/TopicTree.h
  • packages/bun-uws/src/WebSocket.h
  • packages/bun-uws/src/WebSocketContext.h
  • packages/bun-uws/src/WebSocketContextData.h
  • src/bundler/analyze_transpiled_module.rs
  • src/js/.gitignore
  • src/js/internal/fifo.ts
  • src/js/internal/quic/quic.ts
  • src/js/internal/quic/symbols.ts
  • src/js/internal/repl/utils.js
  • src/js/internal/shared.ts
  • src/js/internal/sql/shared.ts
  • src/js/internal/util/colors.ts
  • src/js/internal/util/inspect.d.ts
  • src/js/node/net.ts
  • src/js/private.d.ts
  • src/jsc/bindings/JSBufferList.cpp
  • src/jsc/bindings/JSBufferList.h
  • src/jsc/bindings/ZigGlobalObject.cpp
  • src/jsc/bindings/ZigGlobalObject.h
  • src/jsc/bindings/webcore/DOMClientIsoSubspaces.h
  • src/jsc/bindings/webcore/DOMIsoSubspaces.h
  • src/runtime/bake/client/websocket.ts
  • src/runtime/bake/dev_server/serialized_failure.rs
  • src/runtime/bake/incremental_visualizer.html
  • src/runtime/bake/memory_visualizer.html
  • src/runtime/shell/Builtin.rs
  • src/runtime/shell/interpreter.rs
  • src/runtime/shell/shell_body.rs
  • src/runtime/shell/states/Assigns.rs
  • src/runtime/shell/states/Async.rs
  • src/runtime/shell/states/Base.rs
  • src/runtime/shell/states/Binary.rs
  • src/runtime/shell/states/Cmd.rs
  • src/runtime/shell/states/CondExpr.rs
  • src/runtime/shell/states/Expansion.rs
  • src/runtime/shell/states/If.rs
  • src/runtime/shell/states/Pipeline.rs
  • src/runtime/shell/states/Script.rs
  • src/runtime/shell/states/Stmt.rs
  • src/runtime/shell/states/Subshell.rs
  • src/shell_parser/Cargo.toml
  • src/sql/Cargo.toml
  • src/sql_jsc/Cargo.toml
  • src/sql_jsc/jsc.rs
  • src/sql_jsc/mysql.rs
  • src/sql_jsc/postgres.rs
  • test/js/bun/websocket/websocket-server.test.ts
💤 Files with no reviewable changes (60)
  • src/jsc/bindings/webcore/DOMClientIsoSubspaces.h
  • src/js/.gitignore
  • packages/bun-uws/src/Loop.h
  • packages/bun-uws/src/HttpError.h
  • packages/bun-uws/src/HttpContextData.h
  • src/runtime/bake/client/websocket.ts
  • src/js/internal/util/inspect.d.ts
  • src/runtime/shell/states/CondExpr.rs
  • src/js/internal/fifo.ts
  • .gitattributes
  • src/jsc/bindings/JSBufferList.h
  • src/js/internal/shared.ts
  • src/sql_jsc/postgres.rs
  • src/runtime/shell/states/Expansion.rs
  • src/runtime/bake/incremental_visualizer.html
  • src/js/node/net.ts
  • src/jsc/bindings/ZigGlobalObject.cpp
  • src/runtime/shell/states/Pipeline.rs
  • src/runtime/bake/dev_server/serialized_failure.rs
  • .cursorignore
  • src/runtime/shell/interpreter.rs
  • src/sql/Cargo.toml
  • packages/bun-uws/src/ClientApp.h
  • src/runtime/shell/shell_body.rs
  • src/js/internal/quic/symbols.ts
  • src/jsc/bindings/webcore/DOMIsoSubspaces.h
  • packages/bun-uws/src/Multipart.h
  • src/jsc/bindings/ZigGlobalObject.h
  • src/runtime/shell/states/Binary.rs
  • packages/bun-uws/src/MessageParser.h
  • src/runtime/bake/memory_visualizer.html
  • src/runtime/shell/states/Stmt.rs
  • packages/bun-uws/src/LoopData.h
  • packages/bun-uws/src/ProxyParser.h
  • src/js/internal/sql/shared.ts
  • src/shell_parser/Cargo.toml
  • hawk.toml
  • src/jsc/bindings/JSBufferList.cpp
  • src/runtime/shell/states/Cmd.rs
  • src/bundler/analyze_transpiled_module.rs
  • src/runtime/shell/states/Base.rs
  • src/runtime/shell/states/Subshell.rs
  • packages/bun-uws/src/Http3ResponseData.h
  • packages/bun-uws/misc/READMORE.md
  • packages/bun-uws/src/WebSocketContextData.h
  • packages/bun-uws/src/Http3Request.h
  • src/runtime/shell/Builtin.rs
  • src/js/internal/util/colors.ts
  • src/runtime/shell/states/Async.rs
  • src/js/internal/quic/quic.ts
  • src/sql_jsc/Cargo.toml
  • src/sql_jsc/mysql.rs
  • packages/bun-uws/src/HttpResponseData.h
  • packages/bun-uws/src/HttpResponse.h
  • src/runtime/shell/states/Script.rs
  • packages/bun-uws/src/Http3Response.h
  • packages/bun-uws/src/Http3App.h
  • .dockerignore
  • packages/bun-uws/src/App.h
  • packages/bun-uws/src/PerMessageDeflate.h

Included review availability: Your plan provides up to 5 included reviews per hour; 0 remain after this review.


Walkthrough

The pull request removes unused APIs, compatibility methods, parser and proxy support, bindings, exports, dependencies, shell cleanup paths, and development visualizers. It also simplifies WebSocket subscription handling and adds a concurrent subscription regression test.

Changes

bun-uws API and WebSocket cleanup

Layer / File(s) Summary
WebSocket subscription lifecycle
packages/bun-uws/src/App.h, packages/bun-uws/src/TopicTree.h, packages/bun-uws/src/WebSocket*.h, test/js/bun/websocket/*
Subscription callbacks and subscriber counts were removed. WebSocket cleanup retains topic unlinking. The test covers concurrent subscriptions and socket closure.
HTTP and HTTP/3 surface cleanup
packages/bun-uws/src/Http*.h, packages/bun-uws/src/Loop.h
Unsupported response methods, proxy parsing, parameter maps, unused parser arguments, and obsolete state were removed. HTTP/3 listen calls now require options.
Compression configuration
packages/bun-uws/src/App.h, packages/bun-uws/src/PerMessageDeflate.h
The no-zlib and mock-zlib branches were removed. Zlib and libdeflate paths are always compiled.

JavaScript and runtime cleanup

Layer / File(s) Summary
Bindings and runtime exports
src/jsc/bindings/*, src/js/internal/*, src/js/node/net.ts, src/js/private.d.ts, src/runtime/bake/*
JSBufferList integration, internal symbols, selected exports, declaration blocks, and development visualizers were removed or reduced.
Shell error and scope cleanup
src/runtime/shell/*
ShellErr::InvalidArguments and Base::end_scope usage in shell deinitializers were removed.

SQL façade and repository cleanup

Layer / File(s) Summary
SQL façade and dependencies
src/shell_parser/Cargo.toml, src/sql/Cargo.toml, src/sql_jsc/*
Unused dependencies, disabled links, and selected SQL and JSC re-exports were removed.
Repository cleanup
.dockerignore, packages/bun-uws/src/MessageParser.h, packages/bun-uws/src/Multipart.h, packages/bun-uws/src/ProxyParser.h
Unused ignore entries and obsolete parser headers were removed.

Possibly related PRs

  • oven-sh/bun#37659: Removes overlapping dead code across bun-uws, JSBufferList, and related built-in code.

Suggested reviewers: alii, cirospaciari, jarred-sumner

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly summarizes the primary dead-code removals across the affected subsystems.
Description check ✅ Passed The description explains the changes and provides detailed verification results, despite not using the template headings exactly.

Comment @coderabbitai help to get the list of available commands.

@robobun

robobun commented Aug 22, 2026 •

Copy link
Copy Markdown
Collaborator Author

Status: ready for review, waiting on a maintainer.

@robobun

robobun commented Aug 22, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 2:56 AM PT - Aug 22nd, 2026

✅ @robobun, your commit c1cac52b1106c1876926dc94faad6e090ad40662 passed in Build #103489! 🎉


🧪   To try this PR locally:

bunx bun-pr 40066

That installs a local version of the PR into your bun-40066 executable, so you can run:

bun-40066 --bun

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed this PR and didn't find any bugs. Because it's a large sweep (75 files, -3.3k lines) that touches the HTTP parser, WebSocket pub/sub topic tree, JSC GlobalObject structure table, the shell interpreter's root-script teardown path, and the Cargo dependency graph across three crates, a maintainer look would still be worthwhile.

What was reviewed:

  • Verified the H1/H3 overrideWriteOffset/setWriteOffset split matches what the uws_sys shims actually call, and that the removed Http3Response members have no shim callers.
  • Confirmed JSBufferList, zig__ModuleInfo__destroy/zig_log, and m_JSBufferListClassStructure have zero remaining references in src/.
  • Checked that interpreter.rs calling free_node(child) directly is behavior-equivalent to the removed Script::deinit_from_interpreter (which was a no-op before free_node), and that net.ts this.listeningId is distinct from the still-used server[kClusterListeningId].
Extended reasoning...

Overview

A scheduled dead-code sweep removing ~3,275 net lines across 75 files: unreferenced uWS headers and members (subscription handler, proxy parser, parameter-name map, unused listen/publish overloads, zlib mock branches), the native JSBufferList C++ class and its GlobalObject lazy-structure slot, orphaned bake visualizer HTML assets, the empty Base::end_scope() and its 12 call sites in the shell state machine, the unconstructed ShellErr::InvalidArguments variant, unused Cargo dependencies in three crates, unused pub use re-exports in sql_jsc, stale hawk.toml overrides, and assorted built-in JS exports (kSendHeaders, Dequeue#clear, colors fields, listeningId, private type declarations). One net-new behavior: the never-invoked WebSocketContext::layoutAssert() static becomes a RELEASE_ASSERT inside create(). A new websocket-server test pins the subscribe/unsubscribe/close × subscriberCount interaction that the edited TopicTree::unsubscribe and WebSocket::subscribe/unsubscribe/end paths sit on.

Security risks

None identified. The removed ProxyParser and UWS_WITH_PROXY blocks were compile-time dead (no build defines the macro). The deleted misc/cert.pem/key.pem are upstream demo certs that were never referenced. No validation, auth, or bounds check is loosened.

Level of scrutiny

High. The sweep is well-documented and each removal is individually mechanical, but the breadth is significant: it edits the HTTP request parser signature chain, the WebSocket pub/sub TopicTree::unsubscribe return shape and its three call sites, the JSC GlobalObject lazy-class-structure X-macro table, the shell interpreter's root-script completion path, and workspace-level Cargo dependencies. Any one of these is low risk; together they touch enough hot-path native code that a maintainer should confirm CI is green across all platforms before merge.

Other factors

I spot-checked the removals most likely to have hidden callers: the H1 HttpResponse::overrideWriteOffset vs. H3 Http3Response::setWriteOffset pair (each shim calls the surviving name on its side), JSBufferList (zero hits in src/jsc/bindings), the removed Http3Response members (no uws_h3_res_* shim references them), zig__ModuleInfo__destroy/zig_log (zero hits repo-wide), net.ts this.listeningId (distinct from the still-live kClusterListeningId symbol slot), and the Script::deinit_from_interpreter → direct free_node change (the removed function was a no-op before free_node, and the regular Script::deinit already skips shell-env teardown when the parent is the interpreter, so behavior is unchanged). The buildNgHeaderString/assertValidPseudoHeader imports left behind after removing [kSendHeaders] are still used by other QuicStream methods. The new pub/sub test looks correct (uses port: 0, awaits open/close events, no sleeps) and ServerWebSocket.subscriptions is a documented API. No prior review comments to address; the PR carries over a previously-approved-in-principle sweep (#37659) that was closed only for merge conflicts.

@robobun
robobun force-pushed the claude/farm/3be5706a/dead-code-uws-bufferlist-bake-sql branch from 30a80fd to 21213b4 Compare August 22, 2026 08:29

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed this PR and didn't find any bugs. It's a broad sweep across ~75 files in six subsystems (bun-uws HTTP/WS hot paths, ZigGlobalObject, shell, SQL, bake, built-in JS), and a couple of hunks aren't pure deletions — the TopicTree::unsubscribe return-type change and the new RELEASE_ASSERT in WebSocketContext::create() — so a maintainer look would still be worthwhile.

What was reviewed:

  • consumePostPadded/getHeaders signature change: only caller is HttpContext.h, updated in the diff; end is still used for bounds checks.
  • TopicTree::unsubscribe → pair<bool,bool> and the subscriptionHandler removals in WebSocket::{subscribe,unsubscribe,end} / WebSocketContext::onClose: ok && last still gates freeSubscriber; the new test pins subscriberCount across unsubscribe/close with two sockets.
  • Http3Response::setWriteOffset vs H1 overrideWriteOffset: each shim (uws_h3_res_override_write_offset, uws_res_override_write_offset) calls the surviving method.
  • JSBufferList: no remaining references in src/; net.ts listeningId is distinct from the symbol-keyed kClusterListeningId still in use.
Extended reasoning...

Overview

Scheduled dead-code sweep, net −3275 lines across 75 files. Carries over #37659 (closed only for merge conflicts) plus fresh scans of shell, SQL, bake and built-in JS. Almost every hunk is a deletion; the two additions are an 80-line pub/sub pin in websocket-server.test.ts and a RELEASE_ASSERT in WebSocketContext::create() replacing a never-instantiated static_assert.

Security risks

None identified. The removed ProxyParser was already unreachable (UWS_WITH_PROXY is never defined). The deleted misc/cert.pem / key.pem were upstream demo assets. No auth, crypto, or permission surfaces are touched.

Level of scrutiny

High, because of breadth: the diff reaches the HTTP parser (consumePostPadded/getHeaders lose the reserved parameter), WebSocket pub/sub (TopicTree::unsubscribe return type, subscriptionHandler dispatch sites in end/subscribe/unsubscribe/onClose), ZigGlobalObject lazy-class-structure table and IsoSubspace slots, three Cargo.tomls, and node:net/node:quic builtins. Each removal is individually mechanical, but a maintainer should confirm the two non-deletion changes are wanted: the runtime layout assertion (offsetof isn't usable on the non-standard-layout WebSocketContext, so this is a once-per-context pointer-arithmetic check in every build) and the TopicTree::unsubscribe tuple→pair narrowing.

Other factors

The PR description enumerates every removed symbol with its no-caller proof, lists the token-paste / $getByIdDirectPrivate string-lookup checks, and names two candidates that failed those checks and stay. I spot-checked JSBufferList (no remaining refs), the H1/H3 write-offset shims (each calls the surviving method), net.ts listeningId (distinct from the still-used kClusterListeningId), and the quic.ts kSendHeaders removal (buildNgHeaderString etc. remain used by other methods). CI is building on the current rebase; the description's local-failure notes all reproduce on the released bun. No prior review from me on this PR.

…JS, the bake dev server assets, the shell, and the SQL crates

bun-uws: delete the header templates nothing includes (ClientApp.h,
HttpError.h, Multipart.h, MessageParser.h, ProxyParser.h, misc/), the
UWS_WITH_PROXY / UWS_NO_ZLIB / UWS_MOCK_ZLIB / UWS_USE_LIBDEFLATE guards
that no build defines, the never-set WebSocketBehavior::subscription
handler and every dispatch site for it, the per-route parameter-offset
map in HttpRequest, and the App / H3App / Http3Response / Loop members
the C shim never calls. The never-instantiated layoutAssert() helpers go
too; the WebSocketContext layout that WebSocket::getContextData() relies
on is now checked once per context with a RELEASE_ASSERT in create().

JSBufferList: remove the native class, its lazy structure on the global
object, and its iso subspace slots. The JS streams implementation
replaced it.

Built-in JS: remove the node-inspect-extracted ambient types, the BunFS
/ Bun.TOML / Bun.tty private declarations, and unreferenced exports and
symbols in fifo, quic, repl/utils, sql/shared, util/colors, net and
internal/shared.

Bake: delete the incremental and memory visualizer HTML pages whose
routes and embed sites were removed in #36184, an unreferenced client
helper, and a commented-out Zig function.

Shell: remove the empty Base::end_scope() and its call sites, and the
ShellErr::InvalidArguments variant that nothing constructs.

SQL: remove re-exports nothing imports and the unused dependencies of
bun_shell_parser, bun_sql and bun_sql_jsc; drop the hawk.toml overrides
for StatusFlag variants that no longer exist.

Also remove the two extern "C" exports in analyze_transpiled_module.rs
that no C++ declares, and the ignore-file entries for the bun-uws
fuzzing directory that is gone.
The pub/sub paths in WebSocket.h and TopicTree.h lost their
never-installed subscription handler branches. This test pins what
those paths do: unsubscribe() of a topic the socket never joined or
that does not exist reports false and changes nothing, leaving the
last topic frees the subscriber and a later subscribe() counts again,
and close() drops the socket from every topic without touching the
other socket's subscriptions.
@robobun
robobun force-pushed the claude/farm/3be5706a/dead-code-uws-bufferlist-bake-sql branch from 21213b4 to c1cac52 Compare August 22, 2026 09:24

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed this PR and didn't find any bugs. Because it spans 75 files across several critical subsystems (the uWS HTTP parser and WebSocket pub/sub path, JSC iso-subspace slots, the shell interpreter, Cargo dependency graph) and includes a handful of non-pure-deletion changes — the new RELEASE_ASSERT layout check in WebSocketContext::create(), the TopicTree::unsubscribe return-type change, and the reserved/proxyParser parameter drop threaded through HttpParser — a maintainer sign-off is still warranted.

What was reviewed:

  • Verified the H1 shim calls setWriteOffset (kept) and the H3 shim calls overrideWriteOffset (kept), so both write-offset removals are the unused twin.
  • Confirmed SSLMode is only imported as type outside shared.ts, and no JSBufferList references remain in src/jsc/ (the one BufferList grep hit is SourceBufferList).
  • Checked that the removed public: in HttpResponse.h was redundant (the only access specifier in the struct is public: at line 50), and that Script::deinit_from_interpreter only called the empty end_scope().
Extended reasoning...

Overview

Scheduled dead-code sweep: 75 files, net −3275 lines. It removes unreferenced uWS headers (ClientApp.h, HttpError.h, Multipart.h, MessageParser.h, ProxyParser.h, misc/), the never-set subscriptionHandler and its dispatch sites, per-route std::map parameter offsets, several unused HttpRequest/HttpResponse/Http3* members, the UWS_WITH_PROXY/UWS_NO_ZLIB/UWS_MOCK_ZLIB blocks, the native JSBufferList class with its ZigGlobalObject lazy-structure and iso-subspace slots, assorted unused built-in JS exports (SSLMode runtime export, Dequeue#clear, colors, quic kSendHeaders, kGetNativeReadableProto, REPL re-exports, net.ts listeningId), two orphaned bake visualizer HTML pages plus getMainWebSocket, the empty shell Base::end_scope() and its 12 callers, the never-constructed ShellErr::InvalidArguments variant, unused Cargo dependencies in bun_shell_parser/bun_sql/bun_sql_jsc, stale hawk.toml overrides, and two uncalled extern "C" exports in analyze_transpiled_module.rs. A pub/sub pin test is added to websocket-server.test.ts.

Security risks

None identified. The ProxyParser removal drops code that parsed the PROXY v2 protocol, but it was compiled out (UWS_WITH_PROXY is never defined by any build), so no live trust boundary changes. The removed misc/cert.pem/key.pem were upstream demo assets, not shipped.

Level of scrutiny

High. Although almost every hunk is a pure deletion of provably-unreachable code (and the description documents the reference search per item), the PR touches hot request-handling paths — HttpParser::consumePostPadded/fenceAndConsumePostPadded/getHeaders signatures, WebSocket::subscribe/unsubscribe/end, TopicTree::unsubscribe's return shape, WebSocketContext::onClose — and adds one new runtime check (RELEASE_ASSERT on WebSocketContext's field layout in create()). I spot-checked the correctness of each non-deletion change and the shim/caller pairing for the removed overloads (write-offset, listen), but the breadth across independent subsystems and the JSC iso-subspace edit are the kind of change a maintainer should confirm.

Other factors

The multi-agent bug-hunt returned zero findings. bun bd builds and cargo check --workspace passes on all 12 targets per the description; the previous two CI runs' only failures were unrelated main breaks that have since been fixed. The added test pins the pub/sub paths that lost their unreachable subscriptionHandler branches. The robobun status comment already flags "waiting on a maintainer".

@Jarred-Sumner
Jarred-Sumner merged commit a25c009 into main Aug 23, 2026
11 checks passed
@Jarred-Sumner
Jarred-Sumner deleted the claude/farm/3be5706a/dead-code-uws-bufferlist-bake-sql branch August 23, 2026 06:39
robobun added a commit that referenced this pull request Aug 23, 2026
One conflict, in net.ts's Server constructor: main's dead-code pass (#40066)
removed its unused `listeningId` initializer; this branch's `_listeningId`
(node's generation counter, read by close(), listen() and listenInCluster)
stays.
robobun added a commit that referenced this pull request Sep 8, 2026
…rs use, and treat run({ files: null }) as unset like node [allow size]

internal/util/colors lost yellow, gray and reset in #40066 because nothing
on main read them. node:test/reporters does, so every spec line and every
dot carried a literal "undefined" once main was merged in. The codes are
node's (33, 90, 0).

validateRunOptions accepts files: null as not given (files != null), but
discoverRunFiles and runFiles compared against undefined, so null reached
.map/.length and errored the stream. Both sites now use != null and fall
through to default discovery, as node does.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants