feat(ext): perry-ext-undici — undici served by the native fetch stack - #7032
Conversation
|
Warning Review limit reached
Next review available in: 6 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: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (9)
📝 WalkthroughWalkthroughThis PR adds native ChangesNative undici integration
Runtime and compilation fixes
Supply-chain soak and security tooling
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related issues
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 8
🧹 Nitpick comments (3)
.github/zizmor.yml (1)
32-38: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winScope the cache-poisoning ratchet to the known workflows.
disable: truealso exempts workflows added later, unlike theexcessive-permissionscarve-out right below which names its debt explicitly. Anignore:list of the current release workflows keeps the grandfathering auditable and makes new surface fail the gate as intended.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/zizmor.yml around lines 32 - 38, Update the cache-poisoning configuration in zizmor.yml to replace the global disable: true exemption with an ignore list containing only the currently affected release workflows. Preserve the existing grandfathered workflows, document the scoped debt similarly to excessive-permissions, and ensure newly added workflows remain subject to the lint check..github/workflows/zizmor.yml (1)
6-7: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winInclude the tool-pin sources in the PR path filter.
The audit runs a rack-installed binary, so a PR touching
external-tools.jsonorscripts/soak/external-tools.mts(e.g. bumping the zizmor pin) skips this workflow entirely and only breaks after merge.♻️ Proposed change
pull_request: - paths: ['.github/**'] + paths: + - '.github/**' + - 'external-tools.json' + - 'scripts/soak/external-tools.mts'🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/zizmor.yml around lines 6 - 7, Update the pull_request paths filter in the workflow trigger to include external-tools.json and scripts/soak/external-tools.mts alongside .github/**, ensuring changes to the tool-pin sources also run the zizmor audit.scripts/soak/soak.mts (1)
83-130: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winnpm-side excludes have no staleness/prune path.
The yaml side gets both a warning (
staleExcludes) and pruning (fixWorkspaceYaml), so its ledger converges; a version-pinnedmin-release-age-exclude[]=entry whoseremovablehas passed is never warned about and never pruned, so cleared npm bypasses accumulate indefinitely. Worth extending the same pair of helpers over.npmrclines.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/soak/soak.mts` around lines 83 - 130, The npmrc validation in checkNpmrcExcludes only checks annotation presence and date consistency, so expired version-pinned excludes are neither reported nor removable. Extend the npm-side soak flow with the same stale-exclude warning and pruning behavior used by staleExcludes and fixWorkspaceYaml, operating on .npmrc lines while preserving bare-name exclusions and valid active pins.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/security-audit.yml:
- Around line 128-137: Update the “Scan each skill” workflow to run skillspector
through the locked uv project defined by external-tools.json, using uv sync
--locked and the configured exclude-newer constraint instead of resolving
dependencies via uv tool run. Derive the skillspector revision from
external-tools.json, or add a parity check that fails when the workflow revision
differs, so the job cannot use a stale SHA.
In `@crates/perry-ext-fetch/src/lib.rs`:
- Around line 260-267: Update both js_fetch_set_global_proxy implementations in
crates/perry-ext-fetch/src/lib.rs:260-267 and
crates/perry-stdlib/src/fetch/mod.rs:154-161 to check uri_ptr.is_null() before
decoding and clear GLOBAL_PROXY_CLIENT only for null pointers; return 0.0 for
decoded empty URIs, and revise the related documentation from “null/empty” to
null-only.
In `@crates/perry-runtime/src/event_target.rs`:
- Around line 329-360: Update js_dom_exception_subclass_init to root this_value
before any allocation-capable string conversion or field update, and avoid
retaining the raw exception pointer across those operations. Reload the rooted
object and any needed property-name handles after optional_string_from_value and
each set_event_field call so GC cannot invalidate them.
In `@crates/perry/src/commands/compile/optimized_libs/driver.rs`:
- Around line 328-339: Add an `undici` case to the `PERRY_DISABLE_WELL_KNOWN`
validation in the optimized-libraries driver, alongside the existing fastify
handling. When the environment flag is enabled and `module_normalized ==
"undici"`, emit the same clear unsupported-configuration error and stop before
codegen proceeds.
In `@external-tools.json`:
- Around line 48-57: Update the npm entry in external-tools.json, especially its
notes, to identify npm/cli 11.10.0 as the minimum version supporting
min-release-age. Do not label min-release-age as requiring npm 12; reserve the
npm 11.17 requirement for min-release-age-exclude if it is documented.
In `@scripts/soak/external-tools.mts`:
- Around line 341-375: Update linkHandle and its interaction with writeShims so
installing rack-pinned managers such as npm or pnpm cannot silently replace an
existing firewall shim at BIN_DIR/<cmd>. Detect an existing shim body before
creating the direct handle, then preserve or recreate the shim (or emit the
required warning) while maintaining idempotent behavior regardless of whether
--install or --shims runs first.
In `@scripts/soak/soak.mts`:
- Around line 494-516: Update fixCargoConfig and fixWorkspaceYaml to handle
missing configuration keys by appending valid entries, matching fixNpmrc’s
behavior; preserve existing-value rewrites. Ensure the check messages for
missing keys no longer advertise --fix unless both fixers can successfully
repair absent keys.
- Around line 48-60: Update checkCargoConfig so the global-min-publish-age regex
only matches a value within the [registry] table, mirroring the existing
[unstable] section-scoping behavior. Ensure an unsectioned or differently
sectioned setting is reported as missing while preserving the expected age
validation and finding output.
---
Nitpick comments:
In @.github/workflows/zizmor.yml:
- Around line 6-7: Update the pull_request paths filter in the workflow trigger
to include external-tools.json and scripts/soak/external-tools.mts alongside
.github/**, ensuring changes to the tool-pin sources also run the zizmor audit.
In @.github/zizmor.yml:
- Around line 32-38: Update the cache-poisoning configuration in zizmor.yml to
replace the global disable: true exemption with an ignore list containing only
the currently affected release workflows. Preserve the existing grandfathered
workflows, document the scoped debt similarly to excessive-permissions, and
ensure newly added workflows remain subject to the lint check.
In `@scripts/soak/soak.mts`:
- Around line 83-130: The npmrc validation in checkNpmrcExcludes only checks
annotation presence and date consistency, so expired version-pinned excludes are
neither reported nor removable. Extend the npm-side soak flow with the same
stale-exclude warning and pruning behavior used by staleExcludes and
fixWorkspaceYaml, operating on .npmrc lines while preserving bare-name
exclusions and valid active pins.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 42f0f3ab-9406-4dc5-bace-4824ca3d0a04
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.locktools/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (57)
.cargo/config.toml.claude/skills/soak/SKILL.md.github/dependabot.yml.github/workflows/security-audit.yml.github/workflows/soak-autofix.yml.github/workflows/zizmor.yml.github/zizmor.yml.npmrcCargo.tomlchangelog.d/6912-security-tooling-soak.mdchangelog.d/7021-real-npm-cli-compile-gaps.mdchangelog.d/7032-ext-undici.mdcrates/perry-api-manifest/src/entries.rscrates/perry-api-manifest/src/entries/part_4.rscrates/perry-codegen/src/expr/this_super_call.rscrates/perry-codegen/src/lower_call/native_table/mod.rscrates/perry-codegen/src/lower_call/native_table/undici.rscrates/perry-codegen/src/lower_call/new_helpers.rscrates/perry-codegen/src/runtime_decls/strings_part2.rscrates/perry-ext-fetch/src/lib.rscrates/perry-ext-undici/Cargo.tomlcrates/perry-ext-undici/src/lib.rscrates/perry-ext-zlib/Cargo.tomlcrates/perry-ext-zlib/src/stream.rscrates/perry-hir/src/lower/expr_new.rscrates/perry-runtime/src/event_target.rscrates/perry-runtime/src/object/global_this.rscrates/perry-runtime/src/object/global_this/ctor_thunks.rscrates/perry-runtime/src/object/global_this/populate.rscrates/perry-stdlib/src/fetch/abort_bridge.rscrates/perry-stdlib/src/fetch/mod.rscrates/perry/src/commands/compile/optimized_libs.rscrates/perry/src/commands/compile/optimized_libs/driver.rscrates/perry/src/commands/compile/optimized_libs/freshness.rscrates/perry/src/commands/compile/optimized_libs/tests.rscrates/perry/src/commands/compile/resolve.rscrates/perry/src/commands/compile/strip_dedup.rscrates/perry/src/commands/compile/well_known.rscrates/perry/src/commands/sandbox_profile.rscrates/perry/src/commands/stdlib_features.rscrates/perry/well_known_bindings.tomldocs/api/perry.d.tsdocs/src/api/reference.mdexternal-tools.jsonpackage.jsonscripts/soak/constants.mtsscripts/soak/external-tools.mtsscripts/soak/external-tools.test.mtsscripts/soak/paths.mtsscripts/soak/soak.mtsscripts/soak/soak.test.mtsscripts/soak/update-deps.mtsscripts/soak/update-deps.test.mtstools/package.jsontools/pnpm-workspace.yamltools/taze.config.mtsworkspace-architecture.json
| - name: Scan each skill | ||
| run: | | ||
| set -euo pipefail | ||
| for skill in .claude/skills/*/; do | ||
| echo "::group::skillspector ${skill}" | ||
| uv tool run --python 3.12 \ | ||
| --from git+https://github.com/NVIDIA/skillspector@2eb84478 \ | ||
| skillspector scan "${skill}" --no-llm | ||
| echo "::endgroup::" | ||
| done |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Unlocked uv tool run resolution defeats the soak for this job, and the SHA is a second, unchecked copy of the pin.
external-tools.json documents skillspector as installed through a locked uv project (uv sync --locked plus exclude-newer) precisely so its dependency tree is reproducible and soaked; uv tool run --from git+...@2eb84478 resolves transitive Python deps fresh on every run, so freshly published packages execute in CI — the exact exposure SOAK_DAYS exists to prevent. Add --exclude-newer/a locked project here, and read the revision from external-tools.json (or add a parity assertion) so a manifest bump can't leave this job scanning with a stale SHA.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/security-audit.yml around lines 128 - 137, Update the
“Scan each skill” workflow to run skillspector through the locked uv project
defined by external-tools.json, using uv sync --locked and the configured
exclude-newer constraint instead of resolving dependencies via uv tool run.
Derive the skillspector revision from external-tools.json, or add a parity check
that fails when the workflow revision differs, so the job cannot use a stale
SHA.
| let uri = read_str(uri_ptr).unwrap_or_default(); | ||
| if uri.is_empty() { | ||
| if let Ok(mut guard) = GLOBAL_PROXY_CLIENT.write() { | ||
| *guard = None; | ||
| return 1.0; | ||
| } | ||
| return 0.0; | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate files =="
git ls-files | rg '(^|/)perry-(ext-fetch|stdlib).*Fetch|fetch/mod\.rs$|lib\.rs$' || true
echo "== outline relevant dirs =="
for f in crates/perry-ext-fetch/src/lib.rs crates/perry-stdlib/src/fetch/mod.rs; do
if [ -f "$f" ]; then
echo "--- $f ($(wc -l < "$f") lines) ---"
ast-grep outline "$f" --view condensed || true
fi
done
echo "== relevant source snippets =="
sed -n '230,285p' crates/perry-ext-fetch/src/lib.rs
printf '\n---\n'
sed -n '120,180p' crates/perry-stdlib/src/fetch/mod.rs
echo "== search related symbols =="
rg -n "js_fetch_set_global_proxy|GLOBAL_PROXY_CLIENT|ProxyAgent|read_str|string_from_header|unwrap_or_default|clear" crates/perry-ext-fetch crates/perry-stdlib crates -g '*.{rs,md,js,ts}'Repository: PerryTS/perry
Length of output: 50370
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== exact proxy function snippets =="
sed -n '245,275p' crates/perry-ext-fetch/src/lib.rs
printf '\n---\n'
sed -n '140,170p' crates/perry-stdlib/src/fetch/mod.rs
echo "== proxy helper definitions/imports =="
rg -n "unsafe fn (read_str|string_from_header)|fn (read_str|string_from_header)|read_string|JsString::from_raw|StringHeader|GLOBAL_PROXY_CLIENT|new ProxyAgent|Agent / ProxyAgent" \
crates/perry-ext-fetch/src/lib.rs crates/perry-stdlib/src/fetch/mod.rs crates/perry-codegen/src/lower_call/native_table/undici.rs -C 4
echo "== focused docs/tests references =="
rg -n "ProxyAgent|AGENT |new ProxyAgent|set_global_proxy|GLOBAL_PROXY_CLIENT|clear proxy|null |empty uri|uri" crates/perry-codegen crates -g '*.rs' -g '*.md' | head -n 120
echo "== read-only code-flow summary with python =="
python3 - <<'PY'
from pathlib import Path
for path in ["crates/perry-ext-fetch/src/lib.rs", "crates/perry-stdlib/src/fetch/mod.rs"]:
s = Path(path).read_text()
needle = "js_fetch_set_global_proxy"
idx = s.find(needle)
print(f"\n--- {path} index={idx} ---")
if idx == -1:
print("missing")
continue
idx2 = s.find("->", idx)
idx3 = s.find("{", idx2) if idx2 != -1 else -1
idx4 = s.find("}", idx3)
func = s[idx: idx4+1] if idx != -1 and idx4 != -1 else s[idx:]
print(f"first 220 chars:\n{func[:220]!r}")
print("contains before is_empty:", "uri_ptr.is_null()" in func, "read_str(uri_ptr)" in func or "string_from_header(uri_ptr)" in func)
print("line counts in function:", func.count("\n")+1)
PYRepository: PerryTS/perry
Length of output: 50369
Check for a null proxy URI before decoding. Both js_fetch_set_global_proxy copies decode the URI pointer first and then use is_empty() to decide whether to clear GLOBAL_PROXY_CLIENT, so a non-null empty proxy URI is treated the same as the documented null “clear proxy” signal. Distinguish uri_ptr.is_null() from a decoded empty string, and return 0.0 for the latter while changing the doc wording from “null/empty” if that signal is meant to be null-only.
📍 Affects 2 files
crates/perry-ext-fetch/src/lib.rs#L260-L267(this comment)crates/perry-stdlib/src/fetch/mod.rs#L154-L161
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/perry-ext-fetch/src/lib.rs` around lines 260 - 267, Update both
js_fetch_set_global_proxy implementations in
crates/perry-ext-fetch/src/lib.rs:260-267 and
crates/perry-stdlib/src/fetch/mod.rs:154-161 to check uri_ptr.is_null() before
decoding and clear GLOBAL_PROXY_CLIENT only for null pointers; return 0.0 for
decoded empty URIs, and revise the related documentation from “null/empty” to
null-only.
| /// `class X extends DOMException` — `super(message, name)` initializer | ||
| /// (undici's `WebSocketError`, and its module-init `class Test extends | ||
| /// DOMException` capability probe). The subclass instance is a registry-class | ||
| /// object, not the ErrorHeader `new DOMException(...)` allocates, so stamp the | ||
| /// DOMException surface onto `this`: `message`, `name` (default `"Error"`, | ||
| /// matching `js_dom_exception_new`), and the legacy numeric `code` for that | ||
| /// name. | ||
| #[no_mangle] | ||
| pub extern "C" fn js_dom_exception_subclass_init(this_value: f64, message: f64, name: f64) -> f64 { | ||
| let Some(exception) = value_as_ptr::<ObjectHeader>(this_value) else { | ||
| return undefined_value(); | ||
| }; | ||
| let message_ptr = optional_string_from_value(message, b""); | ||
| let name_ptr = optional_string_from_value(name, b"Error"); | ||
| let name_string = unsafe { | ||
| let len = (*name_ptr).byte_len as usize; | ||
| let data = (name_ptr as *const u8).add(std::mem::size_of::<StringHeader>()); | ||
| String::from_utf8_lossy(std::slice::from_raw_parts(data, len)).into_owned() | ||
| }; | ||
| set_event_field( | ||
| exception, | ||
| b"message", | ||
| crate::value::js_nanbox_string(message_ptr as i64), | ||
| ); | ||
| set_event_field( | ||
| exception, | ||
| b"name", | ||
| crate::value::js_nanbox_string(name_ptr as i64), | ||
| ); | ||
| set_event_field(exception, b"code", dom_exception_code(&name_string)); | ||
| undefined_value() | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether ordinary allocation paths can evacuate/move existing objects,
# and whether other *_subclass_init helpers already root `this` across
# allocating calls (to see if this is an established, accepted pattern).
rg -n "fn arena_alloc_gc|fn alloc_block|evacuat|RuntimeHandleScope" crates/perry-runtime/src -g '*.rs' -A3 -B3 | head -200
rg -n "fn js_string_from_bytes" crates/perry-runtime/src -g '*.rs' -A20
rg -n "_subclass_init" crates/perry-runtime/src/event_target.rs -B2 -A25Repository: PerryTS/perry
Length of output: 30646
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== event_target exact lines =="
sed -n '300,360p' crates/perry-runtime/src/event_target.rs
echo "== event target earlier allocation pattern =="
sed -n '230,300p' crates/perry-runtime/src/event_target.rs
echo "== string_from_value / optional_string_from_value =="
rg -n "fn optional_string_from_value|fn string_from_value" crates/perry-runtime/src -g '*.rs' -A25 -B5
echo "== value_as_ptr and RuntimeHandleScope API =="
rg -n "fn value_as_ptr|struct RuntimeHandleScope|pub struct RuntimeHandleScope|fn root_nanbox_f64|fn root_raw_mut_ptr|fn root_nanbox_f64_slice|refreshed_nanbox_f64_slice" crates/perry-runtime/src -g '*.rs' -A12 -B8
echo "== gc files outline =="
fd . crates/perry-runtime/src -g 'gc.rs|gc*.rs' -x sh -c 'echo "--- $1"; ast-grep outline "$1" --view compact || true' sh {}Repository: PerryTS/perry
Length of output: 50369
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== event_target value helpers =="
sed -n '30,70p' crates/perry-runtime/src/event_target.rs
echo "== string_from_value in event_target =="
sed -n '80,96p' crates/perry-runtime/src/event_target.rs
echo "== string coercion path =="
rg -n "fn js_string_coerce|js_string_coerce" crates/perry-runtime/src -g '*.rs' -A20 -B5
echo "== set_event_field definitions =="
rg -n "fn set_event_field|unsafe fn set_event_field|pub fn set_event_field" crates/perry-runtime/src -g '*.rs' -A40 -B10
echo "== gc allocator allocation APIs =="
rg -n "string_storage_alloc|fn gc_alloc|fn allocate|RuntimeHandleScope|collect|move|evacuat|sweep|mark" crates/perry-runtime/src/gc crates/perry-runtime/src -g '*.rs' | head -300Repository: PerryTS/perry
Length of output: 50370
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== event_target helper lines =="
sed -n '30,96p' crates/perry-runtime/src/event_target.rs
echo "== js_string_coerce direct definitions =="
rg -n "pub extern \"C\" fn js_string_coerce|pub fn js_string_coerce|fn js_string_coerce" crates/perry-runtime/src -g '*.rs' --glob '!**/string/*' --glob '!**/util_debuglog.rs' -A30 -B10 | head -220
echo "== set_event_field definitions in event_target =="
rg -n "fn set_event_field|unsafe fn set_event_field|pub unsafe fn set_event_field" crates/perry-runtime/src/event_target.rs -A45 -B15
echo "== focused gc allocator/root patterns =="
rg -n "string_storage_alloc|memory_order|Ordering::|RuntimeHandleScope::new|RootVisitor|gc_alloc|alloc_block|Collect|evacuate|evacuat|move.*object" crates/perry-runtime/src/gc crates/perry-runtime/src -g '*.rs' | head -260Repository: PerryTS/perry
Length of output: 40526
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== set_builtin_property_attrs with gc/alloc context =="
rg -n "pub fn set_builtin_property_attrs|fn set_builtin_property_attrs|fn builtin_property_put" crates/perry-runtime/src -g '*.rs' -A80 -B20 | head -220
echo "== js_string_coerce remainder =="
sed -n '625,676p' crates/perry-runtime/src/builtins/numbers.rs
echo "== RuntimeHandleScope additional methods =="
rg -n "pub fn root_string_ptr|get_raw_const_ptr|get_raw_mut_ptr|get_nanbox_f64" crates/perry-runtime/src/gc/roots/runtime_handles.rs -A20 -B8
echo "== object allocation APIs that can collect =="
rg -n "fn js_object_set_field_by_name|fn js_object_put|fn js_object_put_by_name|ObjectHeader allocation|object_alloc_gc|alloc_block|maybe_collect|gc_collect" crates/perry-runtime/src/object crates/perry-runtime/src/value -g '*.rs' | head -220Repository: PerryTS/perry
Length of output: 18069
Root the this object across string creations and field setting.
exception is dereferenced from a local NaN-boxed f64, then used after optional_string_from_value() may materialize strings and after each set_event_field() can allocate/collect. Root this_value up front and reload it/its property-name handles after each allocation-calling step, or perform the string conversions before rooting the object pointer.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/perry-runtime/src/event_target.rs` around lines 329 - 360, Update
js_dom_exception_subclass_init to root this_value before any allocation-capable
string conversion or field update, and avoid retaining the raw exception pointer
across those operations. Reload the rooted object and any needed property-name
handles after optional_string_from_value and each set_event_field call so GC
cannot invalidate them.
Source: Learnings
| // `undici` (#466): perry-ext-undici is thin glue over the | ||
| // native Web Fetch stack. Its `setGlobalDispatcher` writes | ||
| // the proxy config through `js_fetch_set_global_proxy`, | ||
| // defined in perry-stdlib's `web-fetch` module (and mirrored | ||
| // by perry-ext-fetch). A ProxyAgent-only program never calls | ||
| // `fetch()` in a way `uses_fetch` detects, so re-assert | ||
| // `web-fetch` here or the wrapper's extern reference dangles | ||
| // at link time. (`web-fetch` implies `async-runtime`, which | ||
| // the wrapper's JsPromise surface needs anyway.) | ||
| if module_normalized == "undici" { | ||
| features.insert("web-fetch"); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate driver.rs and relevant references =="
fd -a 'driver\.rs$' . | sed 's#^\./##'
rg -n "imports_fastify|imports_undici|use_well_known|perry-ext-undici|js_undici|fastify|undici|web-fetch" -S --glob '!target/**' .Repository: PerryTS/perry
Length of output: 50370
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== target file size =="
wc -l crates/perry/src/commands/compile/optimized_libs/driver.rs
echo "== relevant ranges in driver.rs =="
sed -n '1,110p' crates/perry/src/commands/compile/optimized_libs/driver.rs
echo "---"
sed -n '300,420p' crates/perry/src/commands/compile/optimized_libs/driver.rs
echo "---"
sed -n '540,610p' crates/perry/src/commands/compile/optimized_libs/driver.rs
echo "---"
sed -n '840,890p' crates/perry/src/commands/compile/optimized_libs/driver.rs
echo "== targeted occurrences (fastify/undici/use_well_known only) =="
rg -n "imports_fastify|imports_undici|use_well_known|fastify|undici|well_known" crates/perry/src/commands/compile/optimized_libs/driver.rsRepository: PerryTS/perry
Length of output: 22971
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== well_known_iteration_set and well_known lookup/bundled logic =="
rg -n "well_known_iteration_set|lookup_well_known|bundled_staticlib_path_for_target|well_known_bindings" crates/perry/src/commands/compile -S
echo "== well_known module files =="
fd -a 'well_known.*|bindings.*|toml$' crates/perry/src/commands/compile | sed 's#^\./##'
for f in $(fd 'well_known.*|well_known_bindings|bindings' crates/perry/src/commands/compile | sed 's#^\./##'); do
echo "--- $f"
wc -l "$f"
rg -n "undici|fastify|node:" "$f" -S || true
done
echo "== parse well_known bindings for undici entries =="
python3 - <<'PY'
from pathlib import Path
p = Path("crates/perry/src/commands/compile/well_known_bindings.toml")
if p.exists():
print(p.read_text(errors="replace"))
else:
print("missing", p)
PY
echo "== stdlib feature/source references for undici =="
rg -n "undici|js_undici|web-fetch|fetch" crates/perry-stdlib -S --glob '*.rs' -g '!README.md' -g '!docs/**' | head -n 200Repository: PerryTS/perry
Length of output: 27741
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate well_known_bindings.toml =="
git ls-files | grep -F 'well_known_bindings.toml' || true
find . -name 'well_known_bindings.toml' -print || true
echo "== all undici/js_undici/native undici references outside lock files =="
rg -n "undici|js_undici|perry-ext-undici|perry_ext_undici" -S --glob '!target/**' --glob '!**/Cargo.lock' --glob '!CHANGELOG.md' .
echo "== stdlib module list around fetch/fastify =="
sed -n '130,210p' crates/perry-stdlib/src/lib.rs
echo "---"
rg -n "export .*(fastify|fetch|client|server)|pub mod (fastify|http|fetch)|pub fn js_(fastify|fetch|undici)" crates/perry-stdlib/src -SRepository: PerryTS/perry
Length of output: 23388
Add a PERRY_DISABLE_WELL_KNOWN guard for undici.
undici is wired through perry-ext-undici with no matching perry-stdlib feature/module to provide js_undici_*, just like fastify. If PERRY_DISABLE_WELL_KNOWN=1 is set while a program imports undici, the well-known routing path that links perry_ext_undici.a is skipped and codegen still emits native undici symbols, leading to an undefined-symbol link failure instead of a clear fastify-style error.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/perry/src/commands/compile/optimized_libs/driver.rs` around lines 328
- 339, Add an `undici` case to the `PERRY_DISABLE_WELL_KNOWN` validation in the
optimized-libraries driver, alongside the existing fastify handling. When the
environment flag is enabled and `module_normalized == "undici"`, emit the same
clear unsupported-configuration error and stop before codegen proceeds.
| "npm": { | ||
| "notes": [ | ||
| "npm 12 (min-release-age support). ONE platform-agnostic registry tarball, single integrity", | ||
| "Installed from the pinned tarball only — never npm install -g npm, no self-update path" | ||
| ], | ||
| "description": "npm — pinned, SRI-verified registry tarball; installed without self-update", | ||
| "repository": "npm:npm", | ||
| "version": "12.0.1", | ||
| "integrity": "sha512-L5T9i/YAQWQWqTS/xZxJkei/9zcu99hCeE4qi41IyBVV7mRQad3qc2JfuOktwmH+qwGI/V2rbCL+/UYxb1+RQA==" | ||
| }, |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Which npm CLI version introduced the min-release-age and min-release-age-exclude config options?
💡 Result:
The npm CLI configuration options min-release-age and min-release-age-exclude were introduced in different versions of the npm CLI, both within the v11 release series [1]. 1. min-release-age: This configuration was introduced in npm CLI v11.10.0 [2][3]. It allows users to specify a relative time (in minutes) to filter out package versions published too recently, acting as a security policy to prevent the installation of newly published, potentially malicious packages [4][2]. 2. min-release-age-exclude: This configuration was introduced in npm CLI v11.17.0 [1]. It allows users to exempt specific packages (by exact name or glob pattern) from the restrictions imposed by the min-release-age policy [5][6].
Citations:
- 1: https://docs.npmjs.com/cli/v11/using-npm/changelog/
- 2: feat: add min-release-age npm/cli#8965
- 3: npm/cli@66d6e11
- 4: feat: add minimum package age policy to prevent supply chain attacks npm/cli#8825
- 5: feat: add min-release-age-exclude config npm/cli#9534
- 6: feat: add min-release-age-exclude with glob pattern support npm/cli#9175
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== external-tools.json npm entry =="
sed -n '44,58p' external-tools.json || true
echo
echo "== soak constants mentioning npm version =="
fd -a 'constants\.mts$' . | sed 's#^\./##' | while read -r f; do
if rg -q 'npm|version' "$f"; then
echo "--- $f ---"
sed -n '1,40p' "$f"
fi
done
echo
echo "== min-release-age references =="
rg -n "min-release-age|npm >= 11\.17|npm 12" -S .Repository: PerryTS/perry
Length of output: 6798
Record npm’s minimum min-release-age support accurately.
min-release-age is supported starting npm npm/cli@v11.10.0, while the >= 11.17 requirement applies to min-release-age-exclude; avoid labeling the npm min-release-age floor as npm 12 in external-tools.json.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@external-tools.json` around lines 48 - 57, Update the npm entry in
external-tools.json, especially its notes, to identify npm/cli 11.10.0 as the
minimum version supporting min-release-age. Do not label min-release-age as
requiring npm 12; reserve the npm 11.17 requirement for min-release-age-exclude
if it is documented.
| export function linkHandle(target: string, name: string): void { | ||
| mkdirSync(BIN_DIR, { recursive: true }) | ||
| const handle = path.join(BIN_DIR, name) | ||
| // force also removes a DANGLING handle (existsSync would report false | ||
| // for one and a bare symlink would then throw EEXIST). | ||
| rmSync(handle, { force: true }) | ||
| if (process.platform === 'win32') { | ||
| // A symlinked/copied handle breaks Windows SEA binaries: pnpm.exe | ||
| // resolves its dist/ siblings from the handle's OWN directory, not the | ||
| // rack. Forward to the absolute rack target instead — a .cmd for | ||
| // cmd/pwsh and an extensionless bash shim for Git Bash. Non-.exe | ||
| // targets are node entry scripts (registry-tarball tools). The handle | ||
| // BASE must not keep a caller-supplied .exe suffix: pwsh resolves | ||
| // `pnpm` to `pnpm.exe` first, and a bash text file wearing that name | ||
| // is "not a valid application for this OS platform". | ||
| const base = path.join(BIN_DIR, name.replace(/\.exe$/, '')) | ||
| const viaExe = target.endsWith('.exe') | ||
| rmSync(base, { force: true }) | ||
| rmSync(`${base}.cmd`, { force: true }) | ||
| rmSync(`${base}.exe`, { force: true }) | ||
| writeFileSync( | ||
| `${base}.cmd`, | ||
| viaExe ? `@echo off\r\n"${target}" %*\r\n` : `@echo off\r\nnode "${target}" %*\r\n`, | ||
| ) | ||
| writeFileSync( | ||
| base, | ||
| viaExe | ||
| ? `#!/usr/bin/env bash\nexec "${target}" "$@"\n` | ||
| : `#!/usr/bin/env bash\nexec node "${target}" "$@"\n`, | ||
| ) | ||
| chmodSync(base, 0o755) | ||
| return | ||
| } | ||
| symlinkSync(target, handle) | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
An install after --shims silently removes the firewall shim.
linkHandle and writeShims own the same BIN_DIR/<cmd> path for the rack-pinned managers (npm, pnpm). Within one main() call the order is safe (--install-all then --shims), but a later --install pnpm overwrites the shim with a direct rack symlink, so installs stop routing through sfw with no diagnostic — the opposite of the fail-open-but-loud contract the shim body implements. Detecting a shim body in linkHandle and either re-writing it or warning would keep the two idempotent in any order.
Also applies to: 580-629
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/soak/external-tools.mts` around lines 341 - 375, Update linkHandle
and its interaction with writeShims so installing rack-pinned managers such as
npm or pnpm cannot silently replace an existing firewall shim at BIN_DIR/<cmd>.
Detect an existing shim body before creating the direct handle, then preserve or
recreate the shim (or emit the required warning) while maintaining idempotent
behavior regardless of whether --install or --shims runs first.
| export function fixCargoConfig(body: string): string { | ||
| return body.replace( | ||
| /^(global-min-publish-age\s*=\s*)"[^"]*"/m, | ||
| `$1"${SOAK_DAYS} days"`, | ||
| ) | ||
| } | ||
|
|
||
| export function fixNpmrc(body: string): string { | ||
| // [ \t] not \s: `\s` matches newlines, so `\s*$` under /m swallowed the | ||
| // blank lines that follow the key (silent reformatting of the file). | ||
| if (/^min-release-age=\d+[ \t]*$/m.test(body)) { | ||
| return body.replace(/^min-release-age=\d+[ \t]*$/m, `min-release-age=${SOAK_DAYS}`) | ||
| } | ||
| return `${body.trimEnd()}\nmin-release-age=${SOAK_DAYS}\n` | ||
| } | ||
|
|
||
| export function fixWorkspaceYaml(body: string): string { | ||
| // [ \t] not \s on the trailing match: `\s*$` under /m consumes the | ||
| // newlines after the value, deleting following blank lines. | ||
| let out = body.replace( | ||
| /^(minimumReleaseAge:[ \t]*)\d+[ \t]*$/m, | ||
| `$1${SOAK_MINUTES}`, | ||
| ) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fixers only patch existing values, but the check text promises --fix for missing keys too.
fixCargoConfig and fixWorkspaceYaml are pure value-drift rewrites (unlike fixNpmrc, which appends). When the key is absent, the finding at Line 58 / Line 157 still advertises "(or run --fix)", so the soak-autofix workflow no-ops, re-raises, and opens the same failing run every day with guidance that cannot work. Either insert the key like fixNpmrc does, or drop the --fix hint from the (missing) path (as checkDependabotCooldown already does for a missing block).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/soak/soak.mts` around lines 494 - 516, Update fixCargoConfig and
fixWorkspaceYaml to handle missing configuration keys by appending valid
entries, matching fixNpmrc’s behavior; preserve existing-value rewrites. Ensure
the check messages for missing keys no longer advertise --fix unless both fixers
can successfully repair absent keys.
…tack (PerryTS#466) Bare `import { ProxyAgent, setGlobalDispatcher, fetch, Agent, getGlobalDispatcher } from 'undici'` now resolves through the well-known bindings table to a thin native wrapper instead of AOT-compiling undici's ~50k lines of JS. - crates/perry-ext-undici: ProxyAgent(uri | {uri, token?}) / Agent dispatcher handles; setGlobalDispatcher pushes the proxy config into the shared fetch client via the new js_fetch_set_global_proxy symbol; getGlobalDispatcher returns the installed handle (lazily creating the implicit global Agent); request() rejects with a clear not-implemented error pointing at fetch. - perry-stdlib fetch + perry-ext-fetch (mirror): global proxy override state + proxied reqwest client (CONNECT tunneling for https, absolute-form for http, token sent as Proxy-Authorization); every fetch path now routes through fetch_client(). - perry-hir: bare-ident `new ProxyAgent(...)` / `new Agent(...)` imported from undici lowers to receiver-less NativeMethodCall (mirrors the http/https Agent arm), so locals tag as (undici, ProxyAgent|Agent) instances for close()/destroy() dispatch. - perry-codegen: UNDICI_ROWS dispatch family (constructors, set/getGlobalDispatcher, request, close/destroy). `fetch` from undici reuses the name-keyed Expr::FetchWithOptions lowering. - perry CLI: well_known_bindings.toml row, shared-tokio set (+ codegen-units pins), web-fetch re-assert in the flip loop, sandbox-profile network unlock, PERRY_NATIVE_EXTENSION_PACKAGES guard, manifest NATIVE_MODULES + API entries, regenerated API docs. Targets undici's stable dispatcher API (unchanged across 6.x -> 7.x). E2E-verified against a local mock proxy: proxied fetch carries the ProxyAgent token as Proxy-Authorization, installing a plain Agent restores direct connections.
bc14d47 to
c5dd43f
Compare
2 participants
Serves
import { ProxyAgent, setGlobalDispatcher, fetch, Agent, getGlobalDispatcher } from 'undici'from perry's native fetch/HTTP stack instead of AOT-compiling undici's ~50k lines of JS. The wrapper is thin glue — no reqwest/tokio deps of its own.Why this matters
undici is Node's own HTTP stack (the engine behind global
fetch), so it lands in a huge share of dependency graphs — and its module-init paths were the source of two compiler bugs fixed in #7021 (theclass extends DOMExceptionprobe and zstd content-decoding). Serving it natively removes that surface from every compiled binary that pulls it in. Socket Firewall uses exactlyProxyAgent+setGlobalDispatcher(src/lib/proxy/fetch-proxy.ts).API coverage
new ProxyAgent(uri | {uri, token?})UND_ERR_INVALID_ARG)setGlobalDispatcher(agent)fetchnew Agent(opts?)getGlobalDispatcher()agent.close()/.destroy()request()How the proxy reaches the client
setGlobalDispatcher(proxyAgent)→js_undici_set_global_dispatcher→ newjs_fetch_set_global_proxy(uri, token)symbol added to both perry-stdlib'sfetch/mod.rsand perry-ext-fetch (mirrors; the linked archive wins). It builds a proxiedreqwest::Client(Proxy::all+ auth) stored behind anRwLock; every fetch path selectsfetch_client(). Blast radius is nil for non-undici programs: with no dispatcher set,fetch_client()returns the existingHTTP_CLIENTunchanged (one RwLock read → Arc clone per fetch). reqwest gives real CONNECT tunneling for https targets. Because Node's global fetch is undici, this correctly makes perry's global fetch honor the dispatcher.Wiring
New
crates/perry-ext-undici(staticlib+rlib, perry-ffi only); codegen row familynative_table/undici.rs+ a HIRnew ProxyAgent/Agentarm;[bindings.undici]inwell_known_bindings.tomlwith an upstream provenance pin (lock-step per #7031);undiciadded tobinding_needs_shared_tokioso the driver rebuilds it in the shared-tokio invocation;module_to_features("undici") = []with the flip loop re-assertingweb-fetch(mapping it tohttp-clientwould strip the very fetch impl the glue calls); NATIVE_MODULES + manifest entries + sandbox network unlock + regenerated d.ts.Validation
cargo test -p perry --bin perry: 758 passed, 0 failed; manifest/unimplemented-API/stub gates green; clippy + fmt clean.via: none→setGlobalDispatcher(new ProxyAgent({uri, token}))→via: 1, proxyAuth: Basic …→new Agent()restores direct → string-form proxies w/o auth →request()rejects clearly. PASS.Known gaps: namespace form
new undici.ProxyAgent()andimport { fetch as f }unsupported (latter matches node-fetch today). Should rebase on #7031 at merge (sharedwell_known_bindings.toml/ NATIVE_MODULES). Pre-existingperry-ext-fetchtest-link failure on the base commit is unrelated (verified via git stash).Summary by CodeRabbit
New Features
undicisupport, includingfetch,Agent,ProxyAgent, global dispatchers, and proxy authentication.DOMExceptionsubclassing andRegExpcall behavior.Security
Documentation
undiciinterfaces.