Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 59 additions & 0 deletions .claude/skills/verify/SKILL.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
---
name: verify
description: Build Bun and drive the changed code at its real surface (CLI, socket, FFI) to observe it running.
---

# Verifying a change to Bun

**Build:** `bun bd` (no timeout — it can take many minutes). Exit 0 is setup, not evidence.

**Drive:** `bun bd run <script.js>` builds *and* runs, forwarding args to the debug binary.
Put driver scripts under `~/code/tmp/**` — Santa blocks unsigned executables elsewhere.

## Two ways to invoke the debug build

| Need | Use |
|---|---|
| run a script, stay in the repo | `bun bd run /path/to/drive.js` |
| any command, from another cwd | `/Users/jarred/code/bun/build/debug/bun-debug <cmd>` |
Comment on lines +11 to +18

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.

🟡 This skill file hardcodes contributor-specific absolute paths (/Users/jarred/code/bun/build/debug/bun-debug on line 18, ~/code/bun-3 on line 58) and a macOS-only Santa MDM constraint (line 11). Since .claude/skills/ is checked in and loaded by every contributor's Claude Code session, agents on other machines will follow these instructions verbatim and hit ENOENT — consider using repo-relative ./build/debug/bun-debug or marking these as environment-specific placeholders.

Extended reasoning...

What the issue is

The new .claude/skills/verify/SKILL.md contains three environment-specific references:

  • Line 11: Put driver scripts under ~/code/tmp/** — Santa blocks unsigned executables elsewhere. — Santa is a macOS-only binary-authorization MDM tool. Contributors on Linux (or macOS without corporate MDM) have no such restriction, and ~/code/tmp/ may not exist.
  • Line 18: | any command, from another cwd | /Users/jarred/code/bun/build/debug/bun-debug <cmd> | — an absolute path rooted at one specific contributor's home directory.
  • Line 58: ~/code/bun-3 tracks main and usually has a built build/debug/bun-debug — references a second personal checkout that won't exist on other machines.

Why it matters here

Unlike a personal note or a scratch script, .claude/skills/verify/SKILL.md is checked into the repository. Per Claude Code's skill mechanism, every contributor's agent will load this file and follow its instructions verbatim when asked to verify a change. The PR description explicitly says "Also adds a verify skill capturing the build-and-drive recipe", so the intent is for this to be shared tooling.

Concrete failure

A contributor on Linux with the repo cloned at /home/alice/bun invokes the verify skill. The agent, following line 18's instruction for "any command, from another cwd", runs:

/Users/jarred/code/bun/build/debug/bun-debug pm pack

This fails with No such file or directory. Similarly, an agent trying to "compare against a baseline binary" per line 58 will cd ~/code/bun-3 and fail. The Santa reference on line 11 will lead a Linux agent to needlessly restrict where it writes driver scripts.

Why nothing prevents it

There is no indirection or placeholder marker — the paths are presented as literal instructions in a table and prose. The skill's own header says "Build Bun and drive the changed code", positioning it as general-purpose. Nothing in the file signals "adjust these paths for your machine."

Suggested fix

Replace the absolute path with the repo-relative form the rest of the repo already uses (CLAUDE.md documents ./build/debug/bun-debug), and either drop the Santa/~/code/bun-3 references or annotate them as environment-specific. For line 18 specifically, something like $PWD/build/debug/bun-debug (run from the repo root) or <repo>/build/debug/bun-debug would be portable.

This is documentation rather than runtime code — nothing crashes at build or test time — so it's a nit, but checked-in agent instructions should work for everyone who clones the repo.


`bun bd` is a **package.json script** — it only resolves with the repo root as cwd.
A probe that `cd`s into a temp dir must call the binary by absolute path.
The binary refuses `bun-debug test <file>` on purpose ("use `bun bd test`"); every other
subcommand (`pm pack`, `install`, `build`, `run`) works directly. Directory args to
`bun-debug test` also trip a filter guard — pass explicit file paths.

## Surfaces, by what you touched

| Changed | Drive it with |
|---|---|
| `src/uws_sys/**`, `src/runtime/server/**` | `Bun.serve({port:0})` + real `fetch()`; `routes:` for static routes |
| WebSocket / `Response::upgrade` | `Bun.serve` + `new WebSocket(...)`, echo a `Uint8Array` |
| TLS / `SSL_CTX` | `Bun.serve({tls:{cert,key}})` + `fetch(https, {tls:{rejectUnauthorized:false}})`. Make a cert with `openssl req -x509 -newkey rsa:2048 -nodes -subj /CN=localhost -addext subjectAltName=DNS:localhost` |
| `ConnectingSocket` (connect-failure path) | `Bun.connect()` to a port you opened then closed → `connectError` fires |
| `src/runtime/bake/**` (dev server) | run `bun-debug index.html --port 0`, read the URL off stdout, then open `ws://host:port/_bun/hmr` |
| `libdeflate`, `zstd`, `node:zlib` | `Bun.gzipSync`/`gunzipSync`, `Bun.zstdCompressSync`, `zlib.brotliCompress`. Feed garbage in too — it must throw, not crash |
| `libarchive` | write side = `bun-debug pm pack`; read side = `bun-debug install ./x.tgz --no-save`, then check the extracted file exists |
| `src/jsc/CachedBytecode.rs` | `bun-debug build x.js --bytecode --target=bun --outdir=out` then run `out/x.js` |
| `src/tcc_sys/**` | `import { cc } from "bun:ffi"` and call a compiled C symbol |
| Yarr `RegularExpression` | `.npmrc` with `public-hoist-pattern[]=*x*`, then `bun-debug install --dry-run` |
| `TextCodec` | `TextDecoder`, including `{stream:true}` across a split multi-byte codepoint |
| `JSUint8Array` | `crypto.getRandomValues(new Uint8Array(n))` (DOMJIT fast path); `ws.send(bytes)` |
| `SourceProvider` | `new Error().stack` must contain `file:line` |
| `Strong` / `Weak` | `WeakRef` + `Bun.gc(true)`; churn thousands of promises |

## Gotchas that cost real time

- **A debug assert you add is only real if it's in the binary**: `strings build/debug/bun-debug | rg '<your panic message>'`.
- **`cargo check` is not an oracle.** It never monomorphizes, so it never evaluates
`const { assert!(...) }` inside a generic fn (`bun_opaque::opaque_deref*`). Finish with
`cargo build -p bun_bin` or `bun bd`.
- **Generated code is built by ninja, not cargo.** `build/debug/codegen/*.rs` goes stale under
a bare `cargo check`. Regenerate a single file with e.g.
`bun src/codegen/generate-host-exports.ts build/debug/codegen`, or just run `bun bd`.
- **Multi-file test runs share one process.** RSS/GC assertions (`gcUntilCountAtMost`,
"does not leak memory") and tests that mutate process globals (`buffer.kMaxLength`) fail
when run alongside other files even with `--isolate`. Re-run the file alone before believing it.
- **Compare against a baseline binary, not intuition.** `~/code/bun-3` tracks `main` and usually
has a built `build/debug/bun-debug`. Run the same file with it to tell a regression from a
pre-existing flake.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

35 changes: 20 additions & 15 deletions src/boringssl/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ type SslCustomVerifyCb =

unsafe extern "C" {
fn SSL_CTX_set_custom_verify(
ctx: *mut boring::SSL_CTX,
ctx: *mut boring::sys::SSL_CTX,
mode: c_int,
callback: SslCustomVerifyCb,
);
Expand All @@ -114,7 +114,7 @@ unsafe extern "C" fn noop_custom_verify(

/// `Send + Sync` newtype around the process-lifetime client `SSL_CTX*` so it
/// can sit inside a `OnceLock` (raw pointers opt out of `Send`/`Sync`).
struct CtxStore(ptr::NonNull<boring::SSL_CTX>);
struct CtxStore(ptr::NonNull<boring::sys::SSL_CTX>);
// SAFETY: `SSL_CTX` is internally thread-safe per BoringSSL docs (its refcount
// and method tables are guarded by `CRYPTO_MUTEX`); we only ever bump the
// refcount and hand it to `SSL_new`, both of which BoringSSL documents as
Expand All @@ -138,11 +138,12 @@ std::thread_local! {
///
/// # Safety
/// `ctx` must be a live `SSL_CTX*`.
pub unsafe fn ssl_ctx_setup(ctx: *mut boring::SSL_CTX) {
pub unsafe fn ssl_ctx_setup(ctx: *mut boring::sys::SSL_CTX) {
let ctx = boring::sys::SSL_CTX::opaque_ref(ctx);
AUTO_CRYPTO_BUFFER_POOL.with(|pool| {
// SAFETY: caller guarantees `ctx` is a live `SSL_CTX*`; the pool pointer
// is either freshly returned by `CRYPTO_BUFFER_POOL_new` or a previously
// stored thread-local pool, and `SSL_DEFAULT_CIPHER_LIST` is a valid C string.
// SAFETY: the pool pointer is either freshly returned by
// `CRYPTO_BUFFER_POOL_new` or a previously stored thread-local pool, and
// `SSL_DEFAULT_CIPHER_LIST` is a valid C string.
unsafe {
if pool.get().is_null() {
pool.set(CRYPTO_BUFFER_POOL_new());
Expand All @@ -159,23 +160,25 @@ pub fn init_client() -> *mut boring::SSL {
// Bump the refcount on every call after the first; the first call's
// `SSL_CTX_new` already returns refcount = 1.
if let Some(stored) = CTX_STORE.get() {
let _ = boring::SSL_CTX_up_ref(stored.0.as_ptr());
let _ = boring::SSL_CTX_up_ref(boring::sys::SSL_CTX::opaque_ref(stored.0.as_ptr()));
}
let ctx = CTX_STORE
.get_or_init(|| {
// Three steps:
// 1. SSL_CTX_new(TLS_with_buffers_method())
// 2. setCustomVerify(noop_custom_verify) → SSL_CTX_set_custom_verify(ctx, 0, cb)
// 3. setup() → CRYPTO_BUFFER_POOL_new + set0_buffer_pool + set_cipher_list("ALL")
let ctx = boring::SSL_CTX_new(boring::TLS_with_buffers_method());
SSL_CTX_set_custom_verify(ctx, 0, Some(noop_custom_verify));
ssl_ctx_setup(ctx);
CtxStore(ptr::NonNull::new(ctx).expect("SSL_CTX_new"))
let ctx =
boring::SSL_CTX::new(boring::TLS_with_buffers_method()).expect("SSL_CTX_new");
SSL_CTX_set_custom_verify(ctx.as_ptr(), 0, Some(noop_custom_verify));
ssl_ctx_setup(ctx.as_ptr());
// Process-lifetime: this +1 is never given back.
CtxStore(ctx.leak())
})
.0
.as_ptr();

let ssl = boring::SSL_new(ctx);
let ssl = boring::SSL_new(boring::sys::SSL_CTX::opaque_ref(ctx));
boring::SSL_set_connect_state(ssl);

ssl
Expand Down Expand Up @@ -323,7 +326,7 @@ fn match_dns_name(pattern: &[u8], hostname: &[u8]) -> bool {
strings::eql_case_insensitive_ascii(pattern, hostname, true)
}

pub fn check_x509_server_identity(x509: &mut boring::X509, hostname: &[u8]) -> bool {
pub fn check_x509_server_identity(x509: &mut boring::sys::X509, hostname: &[u8]) -> bool {
let host_is_ip = strings::is_ip_address(hostname);
// Node.js: CN is consulted only when the certificate carries no
// DNS / IP / URI subjectAltName entries. Track whether any were seen.
Expand All @@ -333,7 +336,7 @@ pub fn check_x509_server_identity(x509: &mut boring::X509, hostname: &[u8]) -> b
// SAFETY: x509 is a valid &mut so non-null/aligned; all boring:: fns are
// null-safe where documented.
unsafe {
let x509: *mut boring::X509 = x509;
let x509: *mut boring::sys::X509 = x509;
let index = boring::X509_get_ext_by_NID(x509, boring::NID_subject_alt_name, -1);
if index >= 0 {
// we can check hostname
Expand All @@ -353,7 +356,9 @@ pub fn check_x509_server_identity(x509: &mut boring::X509, hostname: &[u8]) -> b
None
};

if let Some(names) = boring::GeneralNames::from_raw(boring::X509V3_EXT_d2i(ext)) {
if let Some(names) =
boring::struct_stack_st_GENERAL_NAME::from_raw(boring::X509V3_EXT_d2i(ext))
{
for name in names.iter() {
match name.name_type {
boring::GEN_URI => {
Expand Down
Loading
Loading