build: WebKit → preview-pr-182-d8d42dd3#29252
Conversation
…inux TLC decommit)
|
Updated 4:45 AM PT - Apr 13th, 2026
❌ @Jarred-Sumner, your commit b1c46c3 has 3 failures in
🧪 To try this PR locally: bunx bun-pr 29252That installs a local version of the PR into your bun-29252 --bun |
|
Found 9 issues this PR may fix:
🤖 Generated with Claude Code |
WalkthroughUpdated the Changes
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/build/deps/webkit.ts`:
- Line 6: The prebuilt extraction directory keying uses
cfg.webkitVersion.slice(0, 16) which truncates structured preview strings like
WEBKIT_VERSION ("preview-pr-182-98dc8d6e") and causes collisions; update the
logic in prebuiltDestDir to use a stable, unique identifier instead (for example
use the full cfg.webkitVersion or include the full hash/suffix rather than
slice(0,16)) so each preview revision gets its own directory—locate references
to WEBKIT_VERSION and the slice operation (cfg.webkitVersion.slice(0, 16)) and
replace with the non-truncated value or a deterministic unique substring (e.g.,
full version or last N characters containing the hash).
- Line 6: The WEBKIT_VERSION constant currently set to "preview-pr-182-98dc8d6e"
is not a valid ref in oven-sh/WebKit and will cause the git checkout in
scripts/sync-webkit-source.ts (the git checkout ${expectedCommit} call) to fail;
update the WEBKIT_VERSION constant to a valid branch, tag, or commit hash that
exists in oven-sh/WebKit or create that ref upstream before syncing so that
scripts/sync-webkit-source.ts can successfully checkout ${expectedCommit}.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: d51b435e-04d0-4b6f-b642-19eeae3bdd92
📒 Files selected for processing (1)
scripts/build/deps/webkit.ts
scripts/build/deps/webkit.ts
Outdated
| * From https://github.com/oven-sh/WebKit releases. | ||
| */ | ||
| export const WEBKIT_VERSION = "42f80a684c5df57121a97e20825a3bcab7a0741b"; | ||
| export const WEBKIT_VERSION = "preview-pr-182-98dc8d6e"; |
There was a problem hiding this comment.
Preview version now collides in prebuilt cache directory naming.
Line 6 introduces a structured version string, but prebuiltDestDir() still keys on cfg.webkitVersion.slice(0, 16). For preview-pr-182-98dc8d6e, that truncates to preview-pr-182-, so future preview revisions can reuse the same extraction directory.
Proposed fix
function prebuiltDestDir(cfg: Config): string {
- const version16 = cfg.webkitVersion.slice(0, 16);
- return resolve(cfg.cacheDir, `webkit-${version16}${prebuiltSuffix(cfg)}`);
+ return resolve(cfg.cacheDir, `webkit-${cfg.webkitVersion}${prebuiltSuffix(cfg)}`);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/build/deps/webkit.ts` at line 6, The prebuilt extraction directory
keying uses cfg.webkitVersion.slice(0, 16) which truncates structured preview
strings like WEBKIT_VERSION ("preview-pr-182-98dc8d6e") and causes collisions;
update the logic in prebuiltDestDir to use a stable, unique identifier instead
(for example use the full cfg.webkitVersion or include the full hash/suffix
rather than slice(0,16)) so each preview revision gets its own directory—locate
references to WEBKIT_VERSION and the slice operation (cfg.webkitVersion.slice(0,
16)) and replace with the non-truncated value or a deterministic unique
substring (e.g., full version or last N characters containing the hash).
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
REPO_URL="https://github.com/oven-sh/WebKit.git"
REF="preview-pr-182-98dc8d6e"
echo "Checking exact tag/branch refs..."
git ls-remote --refs "$REPO_URL" "refs/tags/$REF" "refs/heads/$REF"
echo "Checking generic ref resolution..."
git ls-remote "$REPO_URL" "$REF"Repository: oven-sh/bun
Length of output: 123
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Checking scripts/sync-webkit-source.ts ==="
if [ -f "scripts/sync-webkit-source.ts" ]; then
cat -n scripts/sync-webkit-source.ts | head -50
else
echo "File not found"
fi
echo -e "\n=== Searching for WEBKIT_VERSION references ==="
rg "WEBKIT_VERSION" --type ts --max-count 10 -B 2 -A 2
echo -e "\n=== Checking for preview-pr-182 in codebase ==="
rg "preview-pr-182" --type ts --max-count 5Repository: oven-sh/bun
Length of output: 2743
The WebKit version string preview-pr-182-98dc8d6e does not exist in oven-sh/WebKit and will break the source sync.
Verification confirms this is not a valid Git ref (tag, branch, or commit). The git checkout ${expectedCommit} call in scripts/sync-webkit-source.ts:24 will fail. Either use a valid commit hash, tag, or branch from oven-sh/WebKit, or confirm the ref needs to be created first.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/build/deps/webkit.ts` at line 6, The WEBKIT_VERSION constant
currently set to "preview-pr-182-98dc8d6e" is not a valid ref in oven-sh/WebKit
and will cause the git checkout in scripts/sync-webkit-source.ts (the git
checkout ${expectedCommit} call) to fail; update the WEBKIT_VERSION constant to
a valid branch, tag, or commit hash that exists in oven-sh/WebKit or create that
ref upstream before syncing so that scripts/sync-webkit-source.ts can
successfully checkout ${expectedCommit}.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — straightforward version bump.
Extended reasoning...
Overview
Single-line change updating WEBKIT_VERSION from a 40-character commit SHA to a preview build identifier (preview-pr-182-98dc8d6e). Touches only scripts/build/deps/webkit.ts.
Security risks
None. This is a build configuration change pointing at a specific prebuilt artifact. No auth, crypto, or permissions code is involved.
Level of scrutiny
Low. Version bumps to build deps follow a mechanical, well-established pattern in this repo. The prebuiltUrl() and prebuiltSuffix() logic correctly handles the new version format (the autobuild- prefix is handled at line 73).
Other factors
The inline comment about prebuiltDestDir().slice(0,16) is a valid nit: with the new 24-char version string the slice loses most hash uniqueness, causing potential cache-dir collisions when alternating between preview builds. However, correctness is preserved by the identity field using the full version string (re-download is triggered on mismatch). The impact is limited to developer workflow (cache thrashing), not production builds, and is pre-existing behavior not introduced by this PR. Inline comment already covers it.
scripts/build/deps/webkit.ts
Outdated
| * From https://github.com/oven-sh/WebKit releases. | ||
| */ | ||
| export const WEBKIT_VERSION = "42f80a684c5df57121a97e20825a3bcab7a0741b"; | ||
| export const WEBKIT_VERSION = "preview-pr-182-98dc8d6e"; |
There was a problem hiding this comment.
🟡 prebuiltDestDir() slices webkitVersion to 16 chars, but 'preview-pr-182-98dc8d6e'.slice(0,16) = 'preview-pr-182-9', retaining only 1 character of the 8-char hash. Two preview builds of PR 182 whose short hashes both start with '9' would map to the same cache directory, causing them to overwrite each other and forcing repeated re-downloads when alternating between them.
Extended reasoning...
What the bug is
prebuiltDestDir() computes the cache directory name using cfg.webkitVersion.slice(0, 16). For the old 40-character commit SHA format (e.g., 42f80a684c5df57121a97e20825a3bcab7a0741b), slicing to 16 chars gave a well-distributed, practically unique prefix. The new preview version string 'preview-pr-182-98dc8d6e' (24 chars) changes the calculus: the first 16 chars are 'preview-pr-182-9', which preserves only the leading '9' of the 8-char short hash.
The specific code path
function prebuiltDestDir(cfg: Config): string {
const version16 = cfg.webkitVersion.slice(0, 16); // 'preview-pr-182-9' for WEBKIT_VERSION
return resolve(cfg.cacheDir, `webkit-${version16}${prebuiltSuffix(cfg)}`);
}With WEBKIT_VERSION = 'preview-pr-182-98dc8d6e', this produces a cache path like ~/.bun/webkit-preview-pr-182-9, where all 7 characters of real uniqueness ('8dc8d6e') have been discarded.
Why existing code doesn't fully prevent it
The identity field (line 179) correctly uses the full version string: identity: ${cfg.webkitVersion}${prebuiltSuffix(cfg)}``. This means that if a developer switches between two preview builds whose first 16 chars are identical, the identity check will detect the mismatch and trigger a re-download — correctness is maintained. However, the re-download overwrites the same directory, so both builds cannot coexist in cache simultaneously.
Impact
A developer alternating between, say, 'preview-pr-182-98dc8d6e' and 'preview-pr-182-9abc1234' would see cache thrashing: every switch triggers a full re-download of the ~50–200 MB WebKit tarball into the same directory. Production builds using full 40-char commit SHAs are entirely unaffected. The collision probability is 1/16 per pair of preview builds of the same PR (sharing the first hash character), which is non-negligible during active preview testing.
How to fix it
The simplest fix is to use the full version string for the cache directory:
function prebuiltDestDir(cfg: Config): string {
return resolve(cfg.cacheDir, `webkit-${cfg.webkitVersion}${prebuiltSuffix(cfg)}`);
}Alternatively, if short paths are a concern, use a hash of the full version string, or at least ensure the slice captures the entire unique suffix.
Step-by-step proof
- WEBKIT_VERSION = 'preview-pr-182-98dc8d6e' → slice(0,16) = 'preview-pr-182-9'
- A hypothetical next commit for PR 182 produces WEBKIT_VERSION = 'preview-pr-182-9abc1234' → slice(0,16) = 'preview-pr-182-9' (same)
- Both map to destDir =
<cacheDir>/webkit-preview-pr-182-9 - Developer downloads build 1 → directory populated with '98dc8d6e' artifacts
- Developer switches to build 2 → identity mismatch detected → re-download overwrites same directory
- Developer switches back to build 1 → identity mismatch again → another full re-download
Jarred-Sumner
changed the title
There was a problem hiding this comment.
♻️ Duplicate comments (1)
scripts/build/deps/webkit.ts (1)
6-6:⚠️ Potential issue | 🟠 Major
WEBKIT_VERSIONpreview format still collides with truncated cache dir key.Line 6 switches to a structured preview version, but extraction dir naming still truncates to 16 chars (
prebuiltDestDir), which can cause different preview revisions to reuse the same cache directory.Proposed fix
function prebuiltDestDir(cfg: Config): string { - const version16 = cfg.webkitVersion.slice(0, 16); - return resolve(cfg.cacheDir, `webkit-${version16}${prebuiltSuffix(cfg)}`); + return resolve(cfg.cacheDir, `webkit-${cfg.webkitVersion}${prebuiltSuffix(cfg)}`); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/build/deps/webkit.ts` at line 6, The preview-format WEBKIT_VERSION can collide when prebuiltDestDir truncates to 16 chars; update the directory naming logic that builds prebuiltDestDir to derive a stable, unique key from the entire WEBKIT_VERSION (or its commit/hash portion) instead of simply truncating the string—e.g., compute a short cryptographic hash (SHA256/MD5) of WEBKIT_VERSION and use a longer prefix (e.g., 32 hex chars) or include the full revision identifier in the name; locate the code that constructs prebuiltDestDir and replace the simple substring/truncation with this hashing or full-version inclusion to prevent cache collisions for preview revisions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@scripts/build/deps/webkit.ts`:
- Line 6: The preview-format WEBKIT_VERSION can collide when prebuiltDestDir
truncates to 16 chars; update the directory naming logic that builds
prebuiltDestDir to derive a stable, unique key from the entire WEBKIT_VERSION
(or its commit/hash portion) instead of simply truncating the string—e.g.,
compute a short cryptographic hash (SHA256/MD5) of WEBKIT_VERSION and use a
longer prefix (e.g., 32 hex chars) or include the full revision identifier in
the name; locate the code that constructs prebuiltDestDir and replace the simple
substring/truncation with this hashing or full-version inclusion to prevent
cache collisions for preview revisions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4e468172-7424-4583-9130-82939bcf84b0
📒 Files selected for processing (1)
scripts/build/deps/webkit.ts
| * From https://github.com/oven-sh/WebKit releases. | ||
| */ | ||
| export const WEBKIT_VERSION = "42f80a684c5df57121a97e20825a3bcab7a0741b"; | ||
| export const WEBKIT_VERSION = "preview-pr-182-d8d42dd3"; |
There was a problem hiding this comment.
🔴 Setting WEBKIT_VERSION to a preview label ('preview-pr-182-d8d42dd3') breaks scripts/sync-webkit-source.ts for developers using local WebKit mode: the script runs 'git checkout preview-pr-182-d8d42dd3' in the oven-sh/WebKit clone, but this string is a GitHub release asset label—not a commit SHA, branch, or tag—so git always fails with 'pathspec did not match any file(s) known to git'. To fix, add a guard in sync-webkit-source.ts to skip checkout when WEBKIT_VERSION is not a 40-char hex SHA, or keep a real commit SHA alongside the preview label for local-mode use.
Extended reasoning...
What the bug is
scripts/sync-webkit-source.ts is responsible for updating a local oven-sh/WebKit clone to the expected commit. It imports WEBKIT_VERSION and uses it directly as the git ref to check out (line 24: git checkout ${expectedCommit}). With the old value—a 40-character hex SHA like '42f80a684c5df57121a97e20825a3bcab7a0741b'—this worked correctly. The new value 'preview-pr-182-d8d42dd3' is a GitHub release asset label, not any kind of valid git ref in the oven-sh/WebKit repository.
The specific code path
sync-webkit-source.ts line 14 reads the currently checked-out commit via git rev-parse HEAD (always a 40-char hex SHA). Line 15 imports WEBKIT_VERSION = 'preview-pr-182-d8d42dd3'. Line 17 compares them: checkedOutCommit == expectedCommit will always evaluate to false because no git HEAD can ever equal a non-SHA preview label. As a result the script unconditionally proceeds to lines 21–24: git checkout main, git pull, git checkout preview-pr-182-d8d42dd3. The final checkout fails immediately.
Why existing code doesn't prevent it
There is no guard or format check for whether WEBKIT_VERSION is a valid git ref before attempting the checkout. The equality check on line 17 is designed to skip work when already on the right commit, not to validate the version format. Since a preview string can never match a HEAD SHA, the short-circuit path is permanently bypassed for any preview-format version.
Impact
Any contributor who runs sync-webkit-source.ts while this preview version is set receives: error: pathspec 'preview-pr-182-d8d42dd3' did not match any file(s) known to git. Local WebKit builds (cfg.webkit='local') become entirely broken. Prebuilt mode is unaffected because prebuiltUrl() correctly prepends 'autobuild-' to construct the GitHub release download URL.
Step-by-step proof
- Developer clones oven-sh/WebKit to vendor/WebKit/ and runs sync-webkit-source.ts.
- Script reads HEAD SHA (e.g., '42f80a684c5df57121a97e20825a3bcab7a0741b').
- Script reads expectedCommit = 'preview-pr-182-d8d42dd3'.
- Comparison: '42f80a684c5df57121a97e20825a3bcab7a0741b' == 'preview-pr-182-d8d42dd3' → false.
- Script runs:
git checkout main && git pull && git checkout preview-pr-182-d8d42dd3. - Last command fails: 'preview-pr-182-d8d42dd3' does not exist as a tag, branch, or commit SHA in oven-sh/WebKit.
- Exit with 'pathspec did not match any file(s) known to git'.
How to fix
Add a guard at the top of sync-webkit-source.ts: if WEBKIT_VERSION does not match /^[0-9a-f]{40}$/, print a message explaining that local mode requires a full commit SHA and exit cleanly (or skip the sync). Alternatively, the webkit.ts file could export a separate LOCAL_WEBKIT_COMMIT constant pointing to the underlying real SHA while WEBKIT_VERSION continues to hold the preview label for prebuilt downloads.
2 participants
Points the WebKit prebuilt at oven-sh/WebKit#182's preview build (
autobuild-preview-pr-182-98dc8d6e).That PR fixes Linux RSS not recovering after load drops:
pas_bitfit_directoryrace stranded pages so the scavenger never returned thempas_thread_suspendercallback so Linux can route throughWTF::Thread::suspendBaseline artifacts are present in this preview release.