Conversation
…ernal sentinel Bun.file(missingPath).lastModified returned 4503599627370495 (2^52-1, the INIT_TIMESTAMP sentinel used to mark 'not yet statted'). resolve_file_stat silently no-ops when stat fails, so the sentinel flowed straight to JS. A missing file thus compared newer than every real file. The getter now normalizes the sentinel to 0, matching .size which already reports 0 for a missing file. Plain in-memory Blob.lastModified is also 0 now instead of the sentinel. new File() remains unchanged (Date.now() per the File spec).
|
Warning Review limit reached
Next review available in: 9 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the 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 configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
Comment |
|
Updated 4:53 PM PT - Jul 7th, 2026
✅ @robobun, your commit fa12f28959d56ed136bbef3c3f603a78387b30d7 passed in 🧪 To try this PR locally: bunx bun-pr 33652That installs a local version of the PR into your bun-33652 --bun |
There was a problem hiding this comment.
LGTM — straightforward sentinel normalization mirroring the existing .size behavior.
Extended reasoning...
Overview
This PR fixes Bun.file().lastModified to return 0 instead of the internal lazy-stat sentinel 2**52 - 1 (jsc::INIT_TIMESTAMP) when the underlying file does not exist or stat fails. It touches one getter in src/runtime/webcore/Blob.rs (get_last_modified) and adds four tests to test/js/bun/util/bun-file.test.ts. The fallthrough for in-memory Blobs (non-File) is also changed from the sentinel to 0.
Security risks
None. This is a read-only getter returning a numeric timestamp; no auth, crypto, path handling, or untrusted-input parsing is involved. The change only substitutes one constant return value for another on a path that was already reachable.
Level of scrutiny
Low-to-medium. The diff is ~10 lines of native code plus tests. The fix is a direct analogue of the existing .size → 0 normalization for missing files (resolve_size), so the design decision is already established in the codebase. The S3 branch (which skips resolve_file_stat) now also returns 0 instead of the sentinel when unstatted, which is strictly less surprising than 4503599627370495. The is_jsdom_file path (new File(...)) is unchanged and still returns the stored self.last_modified, so the File API's Date.now() default is preserved — I checked existing structured-clone and fetch tests that assert file.lastModified > 0 and they remain valid.
Other factors
- Tests cover the missing-path case, the deleted-after-creation case, ordering vs. a real file, and the in-memory
Blobfallthrough. - PR description shows the tests fail under
USE_SYSTEM_BUN=1and pass underbun bd, satisfying the repo's test-validity requirement. - No CODEOWNERS entries cover these paths.
- The bug-hunting system found no issues.
|
This PR may be a duplicate of:
🤖 Generated with Claude Code |
|
Not a duplicate of #32425. That PR makes This PR normalizes the sentinel in the file-backed branch, which is the bug reported here. The only overlap is the final fallthrough line; whichever lands second is a trivial rebase. |
|
CI status: this diff is green on every lane that executed tests.
|
There was a problem hiding this comment.
LGTM — straightforward sentinel normalization matching how .size already handles missing files.
Extended reasoning...
Overview
Two-line fix in src/runtime/webcore/Blob.rs get_last_modified: after resolve_file_stat fails to populate last_modified (ENOENT etc.), return 0 instead of leaking the internal jsc::INIT_TIMESTAMP sentinel (2**52 - 1) to JS. The fallthrough for in-memory Blob is normalized the same way. Four new tests in test/js/bun/util/bun-file.test.ts cover missing path, comparison against a real file, post-deletion, and in-memory Blob.
Security risks
None. This is a read-only getter returning a numeric timestamp; no input parsing, no allocation, no I/O added. The change only maps one sentinel constant to 0 at the JS boundary.
Level of scrutiny
Low. The fix is a two-branch value substitution in a getter, directly mirroring the existing pattern where .size already normalizes to 0 for missing files via resolve_size. The is_jsdom_file() branch (which handles new File(...) per the File API spec) is untouched, so spec-mandated Date.now() defaults for File are preserved. The S3 branch also now sees 0 instead of the sentinel when unresolved, which is strictly an improvement.
Other factors
- Tests follow harness conventions (
tempDirwithusing, no hardcoded paths, assert exact values), and the PR description confirms they fail underUSE_SYSTEM_BUN=1and pass under the debug build. - CI is green on all platforms that ran; the only red checks are expired darwin agent slots, not test failures.
- No outstanding reviewer comments; the duplicate-PR bot flag was addressed by the author (the overlap with #32425 is one line and non-conflicting in intent).
|
The |
|
Closing as part of a cleanup of stale pull requests. This PR has had no new commits since 2026-07-07 and it conflicts with main. This is not a judgment on the fix itself. If the problem still reproduces on a current build, reopen this PR after a rebase or open a new one against main. |
Repro
Cause
get_last_modifiedreads the file store'slast_modifiedfield, which is initialized tojsc::INIT_TIMESTAMP((1u64 << 52) - 1). When the field is still the sentinel it callsresolve_file_stat, but that function silently does nothing on stat failure (ENOENT etc.), so the sentinel is returned to JS unchanged. The fallthrough for in-memoryBlobs also returned the raw sentinel..sizealready normalizes this case to0viaresolve_size;.lastModifiedwas the one getter that leaked the lazy marker.Fix
After the resolve attempt, if the field is still
INIT_TIMESTAMP, return0instead of the sentinel. Same for the in-memoryBlobfallthrough.new File([...], name)is unchanged and still returnsDate.now()when nolastModifiedoption is passed, per the File API.Verification