webcore: replace FileSink's ref guard with bun_ptr::ScopedRef

[robobun]

Problem

• No user-facing bug; cleanup in one file. FileSink.rs has a private ref guard, FileSinkRef, that is a one-type copy of bun_ptr::ScopedRef, which FileSink already qualifies for.
• assign_to_stream casts its AnyPromise to a raw *mut JSPromise and reads it in two unsafe blocks, under a comment saying AnyPromise lacks status()/result(). It has both; the cast and comment are stale.

Fix

• Delete FileSinkRef; its 9 sites use ScopedRef in the same mode as before (5 new, which takes a ref; 4 adopt, which takes over one).
• assign_to_stream calls promise.status() and promise.result(vm), as the other assign_to_stream callers already do.
• Property to check: no site changed mode. Both guards are one pointer wide, bump the same Cell<u32> and drop through the same CellRefCounted::deref; the promise accessors reach the same JSC calls the cast did.
• Verification: refactor only, no new test. cargo check/clippy clean, debug build passes, existing filesink and spawn stdin tests pass (92). Removes 4 unsafe blocks and 2 unsafe fns, adds none.

Background

• FileSink is the native sink behind file and pipe writers. It is intrusively refcounted: the count lives in the struct (bun_ptr::CellRefCounted derive) and it is freed at zero. JS holds raw pointers into it, so it is not an Rc.
• Its callbacks (on_write, run_pending, promise reactions) re-enter JS, which can drop the last outside ref mid-function, so each holds a local ref until it returns.
• bun_ptr::ScopedRef<T> is the shared guard for that: new(p) takes a ref, unsafe adopt(p) takes over a ref taken earlier (when a task or promise reaction was queued) without bumping the count, and drop releases one ref.
• bun_jsc::AnyPromise wraps a JSPromise or JSInternalPromise (an alias of JSPromise in bun_jsc) and exposes status()/result(vm) for both.

Original description

What

src/runtime/webcore/FileSink.rs defined a private struct FileSinkRef(*mut FileSink) with unsafe fn new_ref (bumps the count), unsafe fn adopt (does not) and a Drop that calls FileSink::deref. That is a one-type copy of bun_ptr::ScopedRef, and FileSink already derives bun_ptr::CellRefCounted, which emits the AnyRefCounted bridge ScopedRef requires. The local type is deleted and its 9 call sites use the shared guard: 5 new_ref sites become ScopedRef::new, 4 adopt sites become ScopedRef::adopt (the FlushPendingTask::release_unrun site becomes drop(ScopedRef::<FileSink>::adopt(sink)), the same shape as FileResponseStream's Taskable::release_unrun).

// before
let _guard = FileSinkRef::new_ref(this);
let _guard = unsafe { FileSinkRef::adopt(this) };

// after
let _guard = ScopedRef::new(this);
let _guard = unsafe { ScopedRef::adopt(this) };

FileSink::assign_to_stream also matched on the AnyPromise to recover a raw *mut JSPromise and dereferenced it in two unsafe blocks, under a comment saying AnyPromise had no status()/result(). It has both, so the function now calls promise.status() and promise.result(vm) like the sibling assign_to_stream callers in Blob.rs, s3/client.rs, FetchTasklet.rs and html_rewriter.rs, and the cast block and stale comment are gone.

Net effect in FileSink.rs: removes 4 unsafe blocks and 2 unsafe fns, no new unsafe. One file, +12/-61 lines.

Why

The ref/deref bracket around FileSink's re-entrant callbacks (on_write, run_pending, on_auto_flush, on_attached_process_exit, the promise reactions and the flush task) is now expressed with the same ScopedRef type used by the other intrusively refcounted types in the runtime, so a reader sees one guard with one documented new/adopt contract instead of a file-local variant. It is zero-cost: ScopedRef<FileSink> is a NonNull<FileSink>, the same size as the *mut FileSink it replaces; the derive's rc_ref is the same Cell<u32> increment new_ref performed, and ScopedRef's Drop calls the same CellRefCounted::deref that FileSink::deref forwards to, all #[inline]. AnyPromise::status()/result() reach the same JSC__JSPromise__status/__result calls the removed cast did (JSInternalPromise is an alias of JSPromise in bun_jsc); the only difference is the opaque handle's non-null assertion, which as_any_promise has already established for both variants, and it is the shape the four sibling sites already use.

Part of a series of small type-system hardening changes; each PR stands alone.

Verification

cargo check and cargo clippy are clean for the touched crates. Debug build succeeds. bun bd test test/js/bun/util/filesink.test.ts test/js/bun/spawn/spawn-stdin-readable-stream.test.ts test/js/bun/spawn/spawn-streaming-stdin.test.ts: 92 pass, 0 fail (92 tests across 3 files).

[coderabbitai]

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 9 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 1882cb48-6507-48ea-b679-2a75234ab6d1

📥 Commits

Reviewing files that changed from the base of the PR and between e7abdf7 and 214c065.

📒 Files selected for processing (1)

• src/runtime/webcore/FileSink.rs

---

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

[robobun]

^{Updated 6:27 PM PT - Aug 11th, 2026}

✅ @robobun, your commit 214c065ef27d664b1245ff3d8d3e3daf75b753ae passed in Build #92693! 🎉

---

🧪   To try this PR locally:

bunx bun-pr 37695

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

bun-37695 --bun

[claude]

LGTM — mechanical swap of a file-local ref guard for the shared bun_ptr::ScopedRef, plus removal of a stale AnyPromise workaround.

What was reviewed:

• Verified ScopedRef::new/adopt/Drop in src/ptr/ref_count.rs are semantically identical to the deleted FileSinkRef (same Cell<u32> bump via rc_ref, same CellRefCounted::deref on drop — the exact path the inherent FileSink::deref forwards to).
• Confirmed the #[derive(CellRefCounted)] on FileSink emits the AnyRefCounted bridge with DestructorCtx = (), satisfying ScopedRef's bound.
• Confirmed AnyPromise::status()/result() dispatch to the same JSC__JSPromise__status/__result FFI calls the removed raw-pointer cast reached; the release_unrun shape matches FileResponseStream's.

Extended reasoning...

Overview

Single-file refactor in src/runtime/webcore/FileSink.rs (+12/-61). Two independent simplifications:

1. Deletes the private FileSinkRef RAII guard (a file-local reimplementation of bun_ptr::ScopedRef) and rewrites its 9 call sites to use ScopedRef directly — 5 new_ref → ScopedRef::new, 4 adopt → ScopedRef::adopt.
2. In assign_to_stream, replaces a manual match that recovered *mut JSPromise from AnyPromise (plus two unsafe derefs) with direct calls to AnyPromise::status()/result(), removing a stale comment claiming those methods didn't exist.

Security risks

None. No new inputs, no new I/O, no auth/crypto/permissions surface. The change reduces unsafe surface (drops 4 unsafe blocks and 2 unsafe fns) without adding any.

Level of scrutiny

This is refcount-lifetime code on a re-entrant callback path, so I traced the full equivalence rather than trusting the shape:

• ScopedRef::new(ptr) → T::rc_ref(ptr) → (per the CellRefCounted derive in src/bun_core_macros/lib.rs) increments the Cell<u32> field. The old FileSinkRef::new_ref called (*this).ref_(), the inherent forwarder to the same trait method.
• ScopedRef::adopt(ptr) wraps without bumping — identical to old FileSinkRef::adopt.
• ScopedRef::Drop → T::rc_deref → rc_deref_with_context → <Self as CellRefCounted>::deref(this). The old FileSinkRef::Drop called FileSink::deref(self.0), which is the derive-emitted inherent forwarder to that same CellRefCounted::deref. The destroy hook (Self::deinit) is wired via #[ref_count(destroy = Self::deinit)] on the derive, unchanged.
• ScopedRef stores NonNull<T> (via new_unchecked) vs. the old *mut FileSink; every call site passes the canonical heap pointer or ptr::from_mut(self), so non-null holds.
• AnyPromise::status()/result() (checked in src/jsc/AnyPromise.rs) macro-dispatch to JSPromise::status()/result(), which call JSC__JSPromise__status/__result — the same FFI the removed cast reached. The Internal-variant cast is now handled inside the safe wrapper.

Every substitution is provably behavior-preserving; there is no judgment call on ordering, ref balance, or error paths.

Other factors

• The drop(ScopedRef::<FileSink>::adopt(sink)) shape in release_unrun matches the existing sibling in FileResponseStream::release_unrun (src/runtime/server/FileResponseStream.rs:667), and the AnyPromise usage matches sibling assign_to_stream callers cited in the description.
• PR reports filesink.test.ts, spawn-stdin-readable-stream.test.ts, and spawn-streaming-stdin.test.ts all pass (92 tests) on the debug build.
• No prior reviewer comments to address; no CODEOWNERS gating this path.
• Aligns with the repo's "grep for the in-tree helper before hand-writing anything" and "delete dead code in the same PR" review rules.

[robobun]

Closing. #40478 removed bun_ptr::ScopedRef and folded the FileSinkRef guard into RefPtr. On main, FileSink.rs uses RefPtr::init_ref and RefPtr::from_raw at the sites this PR converted. The ScopedRef migration no longer applies.

The second change, promise.status() and promise.result(vm) in assign_to_stream instead of the raw JSPromise cast, is already part of #40213.