blob: make StoreRef::data_mut unsafe, drop Sync - #30812
Open
robobun wants to merge 4 commits into
Open
Claude / Claude Code Review
completed
Aug 15, 2026 in 17m 15s
Code review found 6 potential issues
Found 4 candidates, confirmed 6. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 0 |
| 🟡 Nit | 6 |
| 🟣 Pre-existing | 0 |
| Severity | File:Line | Issue |
|---|---|---|
| 🟡 Nit | src/runtime/webcore/Blob.rs:2146-2151 |
get_last_modified: second re-borrow uses silent INIT_TIMESTAMP fallback instead of unreachable!() |
| 🟡 Nit | src/jsc/webcore_types.rs:1158-1160 |
Send SAFETY CAVEAT cites non-existent S3::init_with_referenced_credentials |
| 🟡 Nit | test/internal/storeref-not-sync.test.ts:1-24 |
storeref-not-sync.test.ts belongs in test/internal/source-lints/ |
Annotations
Check warning on line 2151 in src/runtime/webcore/Blob.rs
claude / Claude Code Review
get_last_modified: second re-borrow uses silent INIT_TIMESTAMP fallback instead of unreachable!()
🟡 The post-`resolve_file_stat` re-borrow uses `_ => jsc::INIT_TIMESTAMP` for the non-`File` fallback arm, while the identical snapshot 12 lines above uses `_ => unreachable!("checked via matches! above")`, and the four sibling re-borrow sites this same PR adds in `resolve_size`/`resolved_size` all use `_ => unreachable!("tag matched File")`. The arm is equally unreachable here (`resolve_file_stat` does `data_mut().as_file_mut()` — panics on non-File — and never reassigns `store.data`). Suggest `
Check warning on line 1160 in src/jsc/webcore_types.rs
claude / Claude Code Review
Send SAFETY CAVEAT cites non-existent S3::init_with_referenced_credentials
The `Data::S3` CAVEAT in the `unsafe impl Send for StoreRef` SAFETY block cites `S3::init_with_referenced_credentials`, but no function by that name exists anywhere in `src/` — the identifier appears to have been copied verbatim from earlier review prose without verifying it names a real function. The substantive claim (the `Rc<S3Credentials>` is non-atomic and typically shared with the JS-thread `S3Client`) is correct; only the parenthetical citation is wrong. Suggest dropping "via `S3::init_wi
Check warning on line 24 in test/internal/storeref-not-sync.test.ts
claude / Claude Code Review
storeref-not-sync.test.ts belongs in test/internal/source-lints/
This test is a pure source-text lint (reads `webcore_types.rs` via `Bun.file().text()` and asserts on regexes) — it does not import `bun:internal-for-testing`, spawn `bunExe()`, or call `Bun.build`/`Bun.Transpiler`, so per `test/internal/source-lints/README.md` it belongs in `test/internal/source-lints/`. Its own header already cites `source-lints/dead-code-escapes.test.ts` as its model sibling. Suggest moving it to `test/internal/source-lints/storeref-not-sync.test.ts` (and adding one more `"..
Loading