Add version dispatch to playground engine#467
Conversation
Picking a version in the playground dropdown now actually runs against that release. The prebuild script vendors nightly plus the top three precedence picks under vendor/<tag>/ (preserving each archive's original binary names), and the API dispatches per request against the resulting manifest. A per-binary --help probe captures which long-option flags each engine accepts, so older versions silently drop modern sandbox flags (--max-memory, --max-instructions, --stack-size, --allowed-host) instead of erroring on first unknown-option. User-toggled compat flags (--compat-var, --compat-function) are passed unconditionally so the engine's own error surfaces when the version doesn't support them. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughReplaces single-nightly vendoring with a Bun/TypeScript prebuild that vendors multiple engine releases and produces a vendor manifest; runtime and playground now resolve engine versions and supported flags from that manifest instead of probing GitHub or local fixed paths. Changes
Sequence Diagram(s)sequenceDiagram
actor BuildProcess as Build Process
participant FetchBinaries as fetch-binaries.ts
participant GitHub as GitHub API
participant Manifest as vendor/manifest.json
BuildProcess->>FetchBinaries: Run prebuild (bun scripts/fetch-binaries.ts)
FetchBinaries->>GitHub: Query releases/tags & download release archives
GitHub-->>FetchBinaries: Release archives + metadata
FetchBinaries->>FetchBinaries: Extract platform binaries, probe --help flags
FetchBinaries->>Manifest: Write/update vendor/manifest.json (versions + features)
Manifest-->>FetchBinaries: manifest written
sequenceDiagram
actor User as User
participant Page as playground/page.tsx
participant ManifestServer as getVendorManifest()
participant Component as Playground
participant API as goccia-api.ts
participant VendorManifest as vendor/manifest.json
User->>Page: Load playground
Page->>ManifestServer: getVendorManifest()
ManifestServer-->>Page: VendorManifest (versions + defaultVersion)
Page->>Component: Render with versions, defaultVersion
User->>Component: Select/version + submit
Component->>API: POST /goccia (includes version)
API->>VendorManifest: findVersion(requestedVersion)
VendorManifest-->>API: VendorEntry (path, features)
API->>API: isFlagSupported(...) per flag -> build argv
API-->>Component: Execution result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Review rate limit: 3/5 reviews remaining, refill in 23 minutes and 24 seconds. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
website/src/lib/goccia-api.ts (1)
750-751: Variable shadowing:resolveddeclared twice in nested scopes.The outer
resolved(line 728) holds the binary resolution result, while the innerresolved(line 751) tracks promise callback state. This works but can cause confusion during maintenance.Consider renaming the inner variable for clarity
return await new Promise<Response>((resolve) => { - let resolved = false; + let finished = false; let cleaned = false; // ... const finish = (res: Response) => { - if (resolved) return; - resolved = true; + if (finished) return; + finished = true;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@website/src/lib/goccia-api.ts` around lines 750 - 751, The outer variable named resolved (the binary resolution flag) is being shadowed by another resolved declared inside the new Promise executor; rename the inner promise-state flag (for example to promiseResolved or callbackResolved) used in the Promise returned by the function so it no longer shadows the outer resolved, and update all references inside the Promise (the executor closure and any places that set/check that flag) to use the new name.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@website/src/lib/goccia-api.ts`:
- Around line 750-751: The outer variable named resolved (the binary resolution
flag) is being shadowed by another resolved declared inside the new Promise
executor; rename the inner promise-state flag (for example to promiseResolved or
callbackResolved) used in the Promise returned by the function so it no longer
shadows the outer resolved, and update all references inside the Promise (the
executor closure and any places that set/check that flag) to use the new name.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 730917d9-1a22-4fd8-a063-0c24db121ac1
📒 Files selected for processing (12)
website/package.jsonwebsite/scripts/fetch-binaries.tswebsite/scripts/fetch-nightly-binary.mjswebsite/src/__tests__/goccia-tool-schema.test.tswebsite/src/__tests__/vendor-manifest.test.tswebsite/src/app/playground/page.tsxwebsite/src/components/playground.tsxwebsite/src/lib/github.tswebsite/src/lib/goccia-api.tswebsite/src/lib/goccia-tool-schema.tswebsite/src/lib/vendor-manifest-server.tswebsite/src/lib/vendor-manifest.ts
💤 Files with no reviewable changes (1)
- website/scripts/fetch-nightly-binary.mjs
Benchmark Results407 benchmarks Interpreted: 🟢 355 improved · 🔴 7 regressed · 45 unchanged · avg +9.7% arraybuffer.js — Interp: 🟢 13, 1 unch. · avg +11.8% · Bytecode: 🔴 9, 5 unch. · avg -5.2%
arrays.js — Interp: 🟢 18, 1 unch. · avg +11.2% · Bytecode: 🔴 16, 3 unch. · avg -5.5%
async-await.js — Interp: 🟢 5, 1 unch. · avg +8.5% · Bytecode: 🔴 5, 1 unch. · avg -6.2%
async-generators.js — Interp: 🟢 2 · avg +11.3% · Bytecode: 🔴 1, 1 unch. · avg -8.1%
base64.js — Interp: 🟢 10 · avg +11.3% · Bytecode: 🔴 9, 1 unch. · avg -5.5%
classes.js — Interp: 🟢 29, 2 unch. · avg +8.3% · Bytecode: 🟢 8, 🔴 6, 17 unch. · avg +1.9%
closures.js — Interp: 🟢 11 · avg +10.4% · Bytecode: 🔴 7, 4 unch. · avg -3.2%
collections.js — Interp: 🟢 12 · avg +9.7% · Bytecode: 🔴 12 · avg -13.0%
csv.js — Interp: 🟢 12, 1 unch. · avg +11.4% · Bytecode: 🔴 2, 11 unch. · avg -2.2%
destructuring.js — Interp: 🟢 22 · avg +9.9% · Bytecode: 🔴 11, 11 unch. · avg -3.8%
fibonacci.js — Interp: 🟢 8 · avg +11.0% · Bytecode: 🔴 7, 1 unch. · avg -8.2%
float16array.js — Interp: 🟢 26, 🔴 4, 2 unch. · avg +5.3% · Bytecode: 🟢 1, 🔴 24, 7 unch. · avg -9.1%
for-of.js — Interp: 🟢 6, 1 unch. · avg +4.6% · Bytecode: 🔴 6, 1 unch. · avg -10.6%
generators.js — Interp: 🟢 3, 1 unch. · avg +8.7% · Bytecode: 🔴 4 · avg -12.9%
iterators.js — Interp: 🟢 40, 2 unch. · avg +9.7% · Bytecode: 🟢 17, 🔴 9, 16 unch. · avg +1.5%
json.js — Interp: 🟢 20 · avg +17.0% · Bytecode: 🔴 12, 8 unch. · avg -4.2%
jsx.jsx — Interp: 🟢 13, 8 unch. · avg +3.5% · Bytecode: 🟢 2, 🔴 3, 16 unch. · avg -0.4%
modules.js — Interp: 🟢 6, 3 unch. · avg +8.3% · Bytecode: 🔴 9 · avg -7.8%
numbers.js — Interp: 🟢 11 · avg +12.4% · Bytecode: 🔴 8, 3 unch. · avg -5.3%
objects.js — Interp: 🟢 7 · avg +11.5% · Bytecode: 🟢 4, 3 unch. · avg +3.1%
promises.js — Interp: 🟢 6, 6 unch. · avg +4.2% · Bytecode: 🔴 2, 10 unch. · avg -1.2%
regexp.js — Interp: 🟢 11 · avg +10.1% · Bytecode: 🟢 1, 🔴 2, 8 unch. · avg -1.8%
strings.js — Interp: 🟢 19 · avg +13.3% · Bytecode: 🔴 15, 4 unch. · avg -6.0%
tsv.js — Interp: 🟢 4, 5 unch. · avg +4.7% · Bytecode: 🟢 5, 4 unch. · avg +4.4%
typed-arrays.js — Interp: 🟢 19, 🔴 1, 2 unch. · avg +21.5% · Bytecode: 🟢 11, 🔴 7, 4 unch. · avg +5.3%
uint8array-encoding.js — Interp: 🟢 11, 🔴 1, 6 unch. · avg +11.8% · Bytecode: 🟢 10, 🔴 2, 6 unch. · avg +25.0%
weak-collections.js — Interp: 🟢 11, 🔴 1, 3 unch. · avg -2.5% · Bytecode: 🟢 2, 🔴 12, 1 unch. · avg -12.1%
Measured on ubuntu-latest x64. Benchmark ranges compare cached main-branch min/max ops/sec with the PR run; overlapping ranges are treated as unchanged noise. Percentage deltas are secondary context. |
Suite Timing
Measured on ubuntu-latest x64. |
Two follow-ups on the version-dispatch PR: - The build script now sets `manifest.defaultVersion` to the first stable pick (`versions[0]` after newest-first sorting) instead of `nightly`, so the playground dropdown opens on the latest release and unspecified API requests run against it. Fallback to `nightly` only when no stable was vendored. - Renamed the inner `let resolved = false` flag in `runHandler`'s Promise executor to `responseSent` so it stops shadowing the outer `const resolved = resolveBinaryPath(...)`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
nightly. The selection rides along to/api/executeand/api/testas aversionfield; the API resolves it againstvendor/manifest.jsonand short-circuits with a 400UNKNOWN_VERSIONfor tags it didn't vendor.prebuildscript (scripts/fetch-binaries.ts, replacingfetch-nightly-binary.mjs) downloadsnightlyplus the top threepickPrecedenceVersionspicks intovendor/<tag>/. Pre-0.7.0 archives preserve their original binary names (ScriptLoader/TestRunner); 0.7.0+ keep theGoccia*prefix. SHA-256 digests cache cross-build re-runs.--help/ no-args fallback probe records each binary's supported long-option flags in the manifest. The API filters sandbox flags (--max-memory,--max-instructions,--stack-size,--allowed-host) against the probed set so older engines silently drop unknown options and still execute. User-toggled compat flags (--compat-var,--compat-function) pass through unconditionally — the engine's own "Unknown option" error surfaces, so opting in against a version that doesn't support it is visible.0.7.0,0.6.1,0.5.1,nightly(~34 MB total undervendor/, comfortably below Vercel's function-size ceiling). The set re-balances automatically as future stable releases land.Test plan
bun test— 122 pass, 6 new forisFlagSupportedbun run lint— cleanbun run build—vendor/manifest.jsonwritten with all 4 entries + per-binary feature listscurl /api/execute -d '{"code":"console.log(1+1)","version":"0.7.0"}'→{"ok":true,"output":"2\n"}curl /api/execute -d '{"code":"console.log(1+1)","version":"0.6.1"}'→ executes (was failing on--max-memorybefore)curl /api/execute -d '{"code":"var x=1","version":"0.6.1","compatVar":true}'→ engine surfacesUnknown option "--compat-var"inrawStdout(deliberate)curl /api/execute -d '{"version":"v999.0.0",...}'→ 400UNKNOWN_VERSIONcurl /api/test -d '{"version":"0.6.1",...}'→ legacy test runner executes,{passed:1,totalTests:1}v0.7.0 · latest,v0.6.1,v0.5.1,nightly🤖 Generated with Claude Code