-
Notifications
You must be signed in to change notification settings - Fork 5k
Add ForeignRef<T> and make FetchHeaders the owned handle #33820
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
Jarred-Sumner
wants to merge
2
commits into
main
Choose a base branch
from
claude/foreign-owned-fetch-headers
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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>` | | ||
|
|
||
| `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. | ||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Hardcoded personal path in shared skill doc.
/Users/jarred/code/bun/build/debug/bun-debugonly resolves on one contributor's machine. Any other user/agent following this table row for "any command, from another cwd" will get a bogus path.✏️ Proposed fix
📝 Committable suggestion
🧰 Tools
🪛 SkillSpector (2.3.7)
[error] 39: [PE3] Credential Access: Code accesses credential files (SSH keys, AWS credentials, etc.). This could indicate credential theft attempts.
Remediation: Remove references to credential paths. Use environment variables or secrets managers. For docs, use placeholder paths (e.g., /path/to/config). Never load .env or token files in production code paths.
(Privilege Escalation (PE3))
🤖 Prompt for AI Agents