Skip to content

feat(electron): Electron-compatible app shell — run existing Electron apps natively#5547

Draft
proggeramlug wants to merge 2 commits into
mainfrom
feat/electron-compat
Draft

feat(electron): Electron-compatible app shell — run existing Electron apps natively#5547
proggeramlug wants to merge 2 commits into
mainfrom
feat/electron-compat

Conversation

@proggeramlug

@proggeramlug proggeramlug commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

What

A drop-in Electron-compat toolkit for Perry: existing Electron app code (app, BrowserWindow, ipcMain, contextBridge, Menu, dialog, …) runs unmodified. App logic compiles to a native binary; the view runs in the OS-native webview (WKWebView). Internally it's the Tauri model (system webview, single native process, no bundled Chromium); the public surface is Electron's — so it's a drop-in, with no Rust to write.

Electron's all-TypeScript DX, Tauri's ~5MB footprint.

macOS first. Windows (WebView2) / Linux (WebKitGTK) are the same pattern, not yet ported.

How it works

Electron splits across a main process (Node) and renderers (Chromium); this mirrors that:

Electron surface Perry implementation
app, BrowserWindow, ipcMain, Menu, dialog, webContents native TS — packages/electron/src/index.ts on perry/ui + node:*
ipcRenderer, contextBridge, require('electron') in preload JS injected into each webview — bridge/preload-runtime.js

IPC: a WKScriptMessageHandler carries renderer→main; evaluateJavaScript carries main→renderer; ipcRenderer.invokeipcMain.handle correlate by id.

Native changes (perry-ui-macos / perry-runtime / perry-codegen / perry-dispatch)

  • webview.rs — bidirectional IPC bridge (WKScriptMessageHandler + document-start user-script injection + set_on_message); file:// loads via loadFileURL:allowingReadAccessToURL: + allowFileAccessFromFileURLs so a file:// page can load its own subresources.
  • app.rsapp_run_loop (body-less loop resolving whenReady), app_quit, full-window Auto-Layout pinning, window activation.
  • ui_loop.rs + codegen entry.rs hookjs_ui_loop_take_over runs the UI loop at the top level of generated main (not from a microtask). Required: entering [NSApp run] from a microtask leaves windows non-composited on macOS.
  • FFI exports, perry-dispatch table rows, types/perry/ui/index.d.ts decls.

TS toolkit (packages/electron)

  • app, BrowserWindow, ipcMain, webContents, Menu, dialog, shell, contextBridge/ipcRenderer.
  • Renderer bridge runtime: require('electron'), a CommonJS loader for local renderer requires, console forwarding to the main process.
  • examples/system-explorer — full IPC demo (invoke/handle, send/on, main→renderer push, fs/os, persisted userData).

Verified end-to-end (macOS)

  • czonios/pomodoro-electron (vanilla jQuery) — unmodified, window renders.
  • electron-vite-react (React + Vite + Tailwind, contextBridge) — unmodified, window renders, IPC works. (npm install + vite build for the renderer; built ESM main + CJS preload compiled by Perry; electron-updater swapped for a small shim.)

Known limits (honest)

  • macOS only so far.
  • dialog/Menu are v1 stubs (template accepted; native NSOpenPanel/menubar TODO).
  • JSON-only IPC (no transferables/Buffer/MessagePort yet).
  • Single process — no contextIsolation security boundary; a webview hang takes the app down.
  • Renderer-side Node usage is unsupported by design — old nodeIntegration + remote apps that call fs/npm directly in the renderer hit the system-webview boundary (same as Tauri).
  • webContents did-finish-load doesn't fire yet (webview onLoaded delegate gap for file://).
  • npm long-tail: main-process pure-JS deps need small TS shims.

See packages/electron/{README,DESIGN}.md for details.

Notes for review

  • No version bump / CHANGELOG entry (left for maintainer per the contributor note in CLAUDE.md).
  • Native changes are macOS-gated (perry-ui-macos); no behavior change for non-UI programs (the codegen js_ui_loop_take_over call is a no-op when no UI loop is registered).

Summary by CodeRabbit

Release Notes

New Features

  • Electron compatibility layer to run Electron apps unmodified on Perry’s native webview model
  • Added WebView IPC bridge APIs (message handler + document-start user script injection)
  • New app lifecycle/loop integration for Electron-compat top-level UI event loops

Documentation

  • Added Electron compatibility design guide and expanded README

Examples

  • New “System Explorer” Electron example app

Bug Fixes

  • Improved warm-cache optimized-lib linking reliability

Chores

  • Enhanced perry init TS path alias sync and perry run directory entry resolution

… apps natively

Adds `packages/electron`, a drop-in Electron-compat toolkit, plus the native
support it needs (macOS first). Existing Electron app code (`app`,
`BrowserWindow`, `ipcMain`, `contextBridge`, …) runs unmodified: app logic
compiles native, the view runs in the OS-native webview (WKWebView) — Tauri-model
internals, Electron API surface, no bundled Chromium, no Rust to write.

Native (perry-ui-macos / perry-runtime / perry-codegen / perry-dispatch):
- webview.rs: bidirectional IPC bridge — WKScriptMessageHandler (renderer→main),
  document-start user-script injection (add_user_script), set_on_message; file://
  loads via loadFileURL:allowingReadAccessToURL: + allowFileAccessFromFileURLs so a
  file:// page can load its own subresources.
- app.rs: app_run_loop (body-less event loop resolving whenReady), app_quit,
  full-window Auto-Layout pinning in window_set_body, window_show activates +
  orderFrontRegardless.
- ui_loop.rs + codegen entry.rs hook: js_ui_loop_take_over runs the UI loop at the
  TOP LEVEL of generated main (not from a microtask) — required or windows created
  from app.whenReady().then(...) never composite on macOS.
- FFI exports, perry-dispatch table rows, and types/perry/ui/index.d.ts decls.

TS toolkit (packages/electron, a nativeModule package):
- app, BrowserWindow, ipcMain, webContents, Menu, dialog, shell, contextBridge/
  ipcRenderer implemented on perry/ui + node:*.
- Renderer-side bridge runtime (bridge/preload-runtime.js): require('electron'),
  a CommonJS loader for local renderer requires, ipcRenderer/contextBridge,
  console forwarding to the main process.
- examples/system-explorer: a full IPC demo (invoke/handle, send/on, main→renderer
  push, fs/os, persisted userData).

Verified end-to-end on macOS: czonios/pomodoro-electron (vanilla) and
electron-vite-react (React + Vite + Tailwind, contextBridge) run unmodified with
rendering windows and working IPC.

Known limits: macOS only so far (Windows/Linux bridge is the same pattern);
dialog/Menu are v1 stubs; JSON-only IPC; renderer-side Node usage (old
nodeIntegration + `remote` apps) is unsupported by design (system webview has no
Node — same boundary as Tauri).
@coderabbitai

coderabbitai Bot commented Jun 22, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Adds a full Electron-compatibility layer to Perry and improves the Perry CLI tooling. A native UI event-loop takeover mechanism is introduced in perry-runtime and wired into codegen's generated main. macOS app lifecycle functions (appRunLoop, appRequestLoop, appQuit) and a WebView IPC bridge via WKUserContentController are added. A new packages/electron TypeScript package emulates Electron's ipcMain, BrowserWindow, app, Menu, dialog, and shell. A renderer-injected preload runtime polyfills ipcRenderer and contextBridge. A complete system-explorer example demonstrates the stack. Separately, perry init now syncs tsconfig.json with package aliases, perry run accepts directory inputs, the warm-cache linker is fixed, and a new @perryts/react type package is introduced.

Changes

Electron Compat Layer

Layer / File(s) Summary
Runtime UI-loop takeover foundation
crates/perry-runtime/src/lib.rs, crates/perry-runtime/src/ui_loop.rs, crates/perry-codegen/src/runtime_decls/strings_part2.rs, crates/perry-codegen/src/codegen/entry.rs
Introduces a process-global AtomicPtr-based UI-loop registration mechanism, exports perry_runtime_register_ui_loop and js_ui_loop_take_over, declares the FFI function in codegen, and inserts the takeover call into the generated entry-module startup sequence before the async event loop.
macOS app lifecycle: appRunLoop, appRequestLoop, appQuit, window fixes
types/perry/ui/index.d.ts, crates/perry-dispatch/src/ui_table.rs, crates/perry-ui-macos/src/app.rs, crates/perry-ui-macos/src/lib_ffi/core_widgets.rs
Implements app_run_loop (initializes NSApplication, resolves on_ready, enters NSApplication.run()), request_app_loop (registers UI-loop hook), and app_quit. Updates window_set_body to pin content via Auto Layout and window_show to re-activate and call orderFrontRegardless. Wires FFI exports, dispatch table entries, and type declarations.
macOS WebView IPC bridge: onMessage, addUserScript, file:// loading
types/perry/ui/index.d.ts, crates/perry-dispatch/src/ui_table.rs, crates/perry-ui-macos/src/widgets/webview.rs, crates/perry-ui-macos/src/lib_ffi/interactivity.rs
Adds on_message field to WebViewState, implements userContentController:didReceiveScriptMessage: to dispatch to TS closures, rewires create(...) for delegate-first ordering, registers the "perry" script message handler on WKUserContentController, enables file:// access preferences, adds file:// URL loading via loadFileURL:allowingReadAccessToURL:, and exposes set_on_message / add_user_script.
Perry Electron TS package: ipcMain, BrowserWindow, app, stubs, and docs
packages/electron/package.json, packages/electron/src/index.ts, packages/electron/.gitignore, packages/electron/DESIGN.md, packages/electron/README.md
Implements the full Electron-compat main-process API surface: IpcMain with invoke dispatch, WebContents shim, BrowserWindow backed by Perry Window/WebView with preload injection ordering, App with whenReady/appRequestLoop loop scheduling, Menu/MenuItem, dialog stubs, and shell open via spawn. Includes architecture spec, usage docs, IPC flow details, and .gitignore.
Renderer preload bridge: ipcRenderer, contextBridge, require polyfill
packages/electron/src/preload-runtime.ts, packages/electron/bridge/preload-runtime.js
Injects a renderer bridge that routes messages to native via platform-specific postMessage, defines window.__perryResolve and window.__perryDeliver for native→renderer delivery, polyfills ipcRenderer and contextBridge, forwards console/errors to native, and provides a synchronous CommonJS-like require for electron and local files via XHR.
System Explorer example app: main/preload/renderer
packages/electron/examples/system-explorer/*
Adds a complete example demonstrating IPC handlers for system info (via os APIs), filesystem listing/reading, notes persistence, and a live 1Hz clock ticker. Includes preload bridge, dark-themed HTML UI with notes and file browser, and renderer event-driven loading and interaction logic.

Perry CLI Improvements, Build Fixes, and Supporting Packages

Layer / File(s) Summary
Version bump to v0.5.1204 and release notes
Cargo.toml, CLAUDE.md, CHANGELOG.md
Updates workspace version from 0.5.1199 to 0.5.1204. Adds comprehensive CHANGELOG entry documenting warm-cache linker fix, CLI improvements, React types package, Electron docs, and root package.json metadata.
Build: warm-cache library linker fix
crates/perry/src/commands/compile/optimized_libs.rs
Fixes build_optimized_libs to extend (not overwrite) CPU-only well_known_libs when handling warm-cache auto-optimize archives, ensuring both CPU-only and tokio-using well-known wrapper archives are available in the final link line.
CLI init: tsconfig.json path alias synchronization
crates/perry/src/commands/init.rs
Extends perry init to keep tsconfig.json in sync with perry.packageAliases. Adds template baseUrl and {paths} placeholder, helper functions to build IDE compilerOptions.paths entries, and JSON-based merge logic with fallback printing for manual insertion.
CLI run: directory-aware entry point resolution
crates/perry/src/commands/run/entry.rs
Refactors resolve_entry_file to treat directory inputs as project roots, resolving an entry within that directory (preferring perry.toml entry, then src/main.ts, then main.ts). Introduces private helpers for directory-scoped entry resolution replacing the previous parameterless read_perry_toml_entry().
New @perryts/react types package
packages/perry-react/*
Introduces @perryts/react as a type layer for Perry's React support on native widgets. Provides PerryRootOptions interface for native-window options, augments react-dom/client's createRoot for null container plus native options, bundles React type dependencies, and includes TypeScript config, README with usage examples, and MIT license.
Root package.json: install and publish metadata
package.json
Adds package metadata to the root including name ("perry"), version ("0.0.0"), private: true, and description to support git-repo npm installation and proper package identification.

Sequence Diagram(s)

sequenceDiagram
  rect rgba(70, 130, 180, 0.5)
    Note over GeneratedMain,AsyncEventLoop: Startup
    participant GeneratedMain
    participant js_ui_loop_take_over
    participant app_run_loop as app_run_loop (macOS)
    participant AsyncEventLoop
  end

  GeneratedMain->>js_ui_loop_take_over: call after microtask/timer flush
  js_ui_loop_take_over->>app_run_loop: invoke registered fn ptr
  app_run_loop->>app_run_loop: init NSApplication, delegate, menu, timers
  app_run_loop->>app_run_loop: resolve on_ready TS closure
  app_run_loop->>AsyncEventLoop: NSApplication.run() (blocks)
Loading
sequenceDiagram
  rect rgba(60, 179, 113, 0.5)
    Note over RendererPage,TSHandler: IPC invoke flow
  end
  participant RendererPage
  participant preload_runtime as preload-runtime (renderer)
  participant WKScriptMessageHandler
  participant ipcMain
  participant TSHandler

  RendererPage->>preload_runtime: ipcRenderer.invoke(channel, args)
  preload_runtime->>WKScriptMessageHandler: postMessage({kind:"invoke", id, channel, args})
  WKScriptMessageHandler->>ipcMain: dispatch message
  ipcMain->>TSHandler: registered handle(channel, event, args)
  TSHandler-->>ipcMain: result
  ipcMain->>WKScriptMessageHandler: evaluateJavaScript(__perryResolve(id, true, json))
  WKScriptMessageHandler-->>preload_runtime: window.__perryResolve(id, true, json)
  preload_runtime-->>RendererPage: Promise resolves with result
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

Poem

🐇 Hop! A new tunnel digs through the web,
Renderer talks to native—perry the thread.
js_ui_loop_take_over leaps to the fore,
WKUserContentController opens the door.
ipcMain hops through JSON, swift and fleet—
Electron compat is now complete! 🎉
And tsconfig paths hop in sync today! ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and concisely captures the main feature: introducing an Electron-compatible app shell for Perry that allows running existing Electron apps natively.
Description check ✅ Passed The PR description is comprehensive and well-structured, covering What/How/Native changes/TS toolkit/Verification/Limits. It deviates from the template structure (which uses Summary/Changes/Related issue/Test plan sections) but provides detailed, organized information that exceeds typical template requirements.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/electron-compat
⚔️ Resolve merge conflicts
  • Resolve merge conflict in branch feat/electron-compat

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🧹 Nitpick comments (1)
packages/electron/bridge/preload-runtime.js (1)

170-193: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

Module cache checked after file load; wastes network round-trip for already-cached modules.

loadSync(url) is called before checking moduleCache[url]. If the module is already cached, the XHR is unnecessary.

Move the cache check before the load:

♻️ Proposed fix
   function makeRequire(baseUrl) {
     return function (name) {
       if (name === "electron") return electron;
       if (name.charAt(0) === "." || name.charAt(0) === "/") {
         var url = resolveUrl(name, baseUrl);
+        if (moduleCache[url]) return moduleCache[url].exports;
         var src = loadSync(url);
-        if (src === null && url.slice(-3) !== ".js") { url = url + ".js"; src = loadSync(url); }
+        if (src === null && url.slice(-3) !== ".js") {
+          url = url + ".js";
+          if (moduleCache[url]) return moduleCache[url].exports;
+          src = loadSync(url);
+        }
         if (src === null) throw new Error("[perry-electron] require: cannot load '" + name + "'");
-        if (moduleCache[url]) return moduleCache[url].exports;
         var mod = { exports: {} };
🤖 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 `@packages/electron/bridge/preload-runtime.js` around lines 170 - 193, In the
makeRequire function, the moduleCache check is performed after calling
loadSync(url), which wastes a network round-trip for already-cached modules.
Move the cache lookup check (if (moduleCache[url]) return
moduleCache[url].exports;) to occur before the loadSync(url) call, so that
cached modules are returned immediately without making unnecessary network
requests.
🤖 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 `@crates/perry-ui-macos/src/widgets/webview.rs`:
- Around line 619-621: The `allowUniversalAccessFromFileURLs` setting is
unconditionally enabled and should instead be made opt-in behind an explicit
security configuration flag. Additionally, locate the `forMainFrameOnly` setting
(referenced around line 951) and change its default from false to true to
restrict script bridge injection to the main frame only. Both settings should be
guarded by explicit insecure configuration options that require deliberate
opt-in rather than being enabled by default. Ensure the
`did_receive_script_message` message handler includes origin and frame context
validation before forwarding IPC calls to the renderer-main bridge.
- Around line 597-603: Several Objective-C objects created with `new`, `alloc`,
or `init` methods are stored as raw pointers instead of using the `Retained`
wrapper type, causing reference count leaks when ownership transfers to receiver
methods. Convert the declarations for `ucc` at line 599, `cfg` at line 580, `wv`
at lines 624-625, and `script` in the loop at lines 944-952 to use
`Retained<AnyObject>` type annotations instead of raw pointers. Update each
`msg_send!` invocation that creates these objects to directly assign to the
`Retained`-typed variable, which will properly track and balance the +1
reference count from Cocoa's ownership conventions.

In `@packages/electron/examples/system-explorer/main.ts`:
- Around line 95-106: The notes:load and notes:save IPC handlers lack schema
validation for the string[] contract between main and renderer processes. In the
notes:load handler, after parsing the JSON from notesPath(), add validation to
ensure the result is actually an array of strings before returning it; if
validation fails, return an empty array instead. In the notes:save handler,
validate that the incoming notes parameter is a string[] before calling
writeFileSync; reject the operation or return an error response if validation
fails. This prevents malformed notes.json from breaking the renderer's UI logic.

In `@packages/electron/examples/system-explorer/renderer/renderer.js`:
- Around line 23-25: The code at lines 23-25 and 35-56 uses innerHTML with
dynamically concatenated HTML strings containing untrusted values (file names),
which creates an XSS vulnerability. Replace the innerHTML assignment with safe
DOM node construction by creating elements using document.createElement for the
container div and span elements, setting the untrusted values (r[0] and r[1])
using textContent instead of concatenating them into HTML strings, and then
appending the constructed nodes to the parent element. This ensures that
untrusted data is treated as text content rather than executable markup.

In `@packages/electron/src/index.ts`:
- Around line 293-298: The loadFile method constructs a file:// URL by directly
concatenating the absolute path without proper URL encoding, which will break
for paths containing special characters like spaces, hash marks, or question
marks. Fix this by encoding the absolute path before constructing the URL. In
the webviewLoadUrl call, replace the string concatenation of "file://" + abs
with either the URL constructor approach (new URL("file://" + abs).href) or by
wrapping the abs variable with encodeURI to ensure special characters are
properly percent-encoded in the resulting URL.

---

Nitpick comments:
In `@packages/electron/bridge/preload-runtime.js`:
- Around line 170-193: In the makeRequire function, the moduleCache check is
performed after calling loadSync(url), which wastes a network round-trip for
already-cached modules. Move the cache lookup check (if (moduleCache[url])
return moduleCache[url].exports;) to occur before the loadSync(url) call, so
that cached modules are returned immediately without making unnecessary network
requests.
🪄 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: 616057d5-eb3d-47b7-a23d-ac01c5b98913

📥 Commits

Reviewing files that changed from the base of the PR and between f392dba and 7ea3e2a.

📒 Files selected for processing (22)
  • crates/perry-codegen/src/codegen/entry.rs
  • crates/perry-codegen/src/runtime_decls/strings_part2.rs
  • crates/perry-dispatch/src/ui_table.rs
  • crates/perry-runtime/src/lib.rs
  • crates/perry-runtime/src/ui_loop.rs
  • crates/perry-ui-macos/src/app.rs
  • crates/perry-ui-macos/src/lib_ffi/core_widgets.rs
  • crates/perry-ui-macos/src/lib_ffi/interactivity.rs
  • crates/perry-ui-macos/src/widgets/webview.rs
  • packages/electron/.gitignore
  • packages/electron/DESIGN.md
  • packages/electron/README.md
  • packages/electron/bridge/preload-runtime.js
  • packages/electron/examples/system-explorer/main.ts
  • packages/electron/examples/system-explorer/package.json
  • packages/electron/examples/system-explorer/preload.js
  • packages/electron/examples/system-explorer/renderer/index.html
  • packages/electron/examples/system-explorer/renderer/renderer.js
  • packages/electron/package.json
  • packages/electron/src/index.ts
  • packages/electron/src/preload-runtime.ts
  • types/perry/ui/index.d.ts

Comment on lines +597 to +603
let ucc_cls = AnyClass::get(c"WKUserContentController")
.expect("WKUserContentController not found — link WebKit.framework");
let ucc: *mut AnyObject = msg_send![ucc_cls, new];
let handler_name = NSString::from_str("perry");
let _: () = msg_send![ucc, addScriptMessageHandler: &*delegate, name: &*handler_name];
let _: () = msg_send![cfg, setUserContentController: ucc];

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Inspect +1 object creation patterns in changed webview paths
rg -n -C2 'msg_send!\[.*\b(new|alloc|initWithFrame|initWithSource)\b' crates/perry-ui-macos/src/widgets/webview.rs

Repository: PerryTS/perry

Length of output: 1772


🏁 Script executed:

#!/bin/bash
# Get broader context around the identified lines
echo "=== Lines 575-610 (around line 599 ucc) ==="
sed -n '575,610p' crates/perry-ui-macos/src/widgets/webview.rs | cat -n

echo "=== Lines 620-650 (around lines 624-625 wv) ==="
sed -n '620,650p' crates/perry-ui-macos/src/widgets/webview.rs | cat -n

echo "=== Lines 940-960 (around line 944 script) ==="
sed -n '940,960p' crates/perry-ui-macos/src/widgets/webview.rs | cat -n

Repository: PerryTS/perry

Length of output: 5411


🏁 Script executed:

#!/bin/bash
# Check the function context and object usage patterns
echo "=== Check line 580 cfg creation and usage ==="
sed -n '575,630p' crates/perry-ui-macos/src/widgets/webview.rs | cat -n | head -60

echo "=== Verify line numbers in create_webview function ==="
rg -n 'fn (create_webview|new_webview)' crates/perry-ui-macos/src/widgets/webview.rs

echo "=== Check if Retained is used elsewhere for similar patterns ==="
rg -n 'Retained<.*>' crates/perry-ui-macos/src/widgets/webview.rs | head -20

Repository: PerryTS/perry

Length of output: 4336


🏁 Script executed:

#!/bin/bash
# Check how cfg and ucc are managed after creation
echo "=== Lines 650-655 showing wv conversion to Retained ==="
sed -n '650,655p' crates/perry-ui-macos/src/widgets/webview.rs | cat -n

# Check the full add_user_script function context
echo "=== Lines 920-960 showing script creation in loop ==="
sed -n '920,960p' crates/perry-ui-macos/src/widgets/webview.rs | cat -n

# Check if cfg and ucc are passed to methods that transfer ownership
echo "=== Check setUserContentController method call ==="
sed -n '595,610p' crates/perry-ui-macos/src/widgets/webview.rs | cat -n

Repository: PerryTS/perry

Length of output: 3752


🏁 Script executed:

#!/bin/bash
# Check if cfg is retained by WKWebView
echo "=== Check Cocoa documentation patterns in comments ==="
sed -n '570,610p' crates/perry-ui-macos/src/widgets/webview.rs | grep -A5 -B5 'retain'

# Check the actual function definition to see return type
echo "=== Check the create_webview function signature ==="
rg -B5 'let cfg: \*mut AnyObject = msg_send!' crates/perry-ui-macos/src/widgets/webview.rs | head -20

# Compare with line 149 pattern
echo "=== Verify line 149 uses Retained consistently ==="
sed -n '145,165p' crates/perry-ui-macos/src/widgets/webview.rs | cat -n

Repository: PerryTS/perry

Length of output: 3214


🏁 Script executed:

#!/bin/bash
# Check if msg_send returns Retained or raw pointer for these methods
echo "=== Search for other setUserContentController patterns ==="
rg -i 'setUserContentController' crates/perry-ui-macos/src/widgets/webview.rs

echo "=== Check if cfg/ucc are truly stored or just passed through ==="
sed -n '597,605p' crates/perry-ui-macos/src/widgets/webview.rs | cat -n

echo "=== Check the whole cfg lifecycle ==="
rg -n 'cfg:' crates/perry-ui-macos/src/widgets/webview.rs | head -20

echo "=== Check what happens to script after addUserScript ==="
sed -n '940,960p' crates/perry-ui-macos/src/widgets/webview.rs | cat -n

Repository: PerryTS/perry

Length of output: 2315


🏁 Script executed:

#!/bin/bash
# Check the function return type and how cfg is ultimately used
echo "=== Check create_webview full context around cfg ==="
sed -n '560,630p' crates/perry-ui-macos/src/widgets/webview.rs | cat -n | grep -A2 -B2 'initWithFrame'

# Verify that cfg is passed to WKWebView initialization
echo "=== Check initWithFrame signature expectation ==="
rg -A3 'initWithFrame:.*configuration:' crates/perry-ui-macos/src/widgets/webview.rs

# Check if WKWebViewConfiguration retains objects passed to it
echo "=== Understand ownership transfer semantics in comments ==="
sed -n '577,608p' crates/perry-ui-macos/src/widgets/webview.rs | cat -n

Repository: PerryTS/perry

Length of output: 3003


Use Retained ownership for Objective-C objects created with new/alloc/init.

Line 599 (ucc), line 580 (cfg), lines 624–625 (wv), and lines 944–952 (script) create +1 Cocoa objects via raw pointers without balanced ownership tracking. When cfg and ucc are passed to methods like setUserContentController: and initWithFrame:configuration:, ownership transfers to the receiver (which retains them), leaving the raw pointer pointing to objects with leaked +1 refcounts. Line 944's script in the add_user_script loop leaks on every call, causing cumulative memory pressure.

Convert these to Retained on creation:

  • let cfg: Retained<AnyObject> = msg_send![cfg_cls, new];
  • let ucc: Retained<AnyObject> = msg_send![ucc_cls, new];
  • let script: Retained<AnyObject> = msg_send![script, initWithSource: ...];

Per coding guidelines, "use msg_send! which returns Retained directly" for all +1 methods.

🤖 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-ui-macos/src/widgets/webview.rs` around lines 597 - 603, Several
Objective-C objects created with `new`, `alloc`, or `init` methods are stored as
raw pointers instead of using the `Retained` wrapper type, causing reference
count leaks when ownership transfers to receiver methods. Convert the
declarations for `ucc` at line 599, `cfg` at line 580, `wv` at lines 624-625,
and `script` in the loop at lines 944-952 to use `Retained<AnyObject>` type
annotations instead of raw pointers. Update each `msg_send!` invocation that
creates these objects to directly assign to the `Retained`-typed variable, which
will properly track and balance the +1 reference count from Cocoa's ownership
conventions.

Source: Coding guidelines

Comment on lines +619 to +621
let k2 = NSString::from_str("allowUniversalAccessFromFileURLs");
let _: () = msg_send![cfg, setValue: yes_num, forKey: &*k2];

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify broad bridge exposure and file-origin relaxations
rg -n -C2 'allowUniversalAccessFromFileURLs|forMainFrameOnly:\s*false|addScriptMessageHandler:\s*&\*delegate' crates/perry-ui-macos/src/widgets/webview.rs
rg -n -C2 'messageHandlers\.perry\.postMessage|webviewSetOnMessage|webviewAddUserScript' packages/electron/src/preload-runtime.ts packages/electron/src/index.ts

Repository: PerryTS/perry

Length of output: 4119


🏁 Script executed:

#!/bin/bash
# Get broader context around the sensitive lines
echo "=== Context around line 619-620 (allowUniversalAccessFromFileURLs) ==="
sed -n '605,630p' crates/perry-ui-macos/src/widgets/webview.rs

echo ""
echo "=== Context around line 946-951 (forMainFrameOnly: false) ==="
sed -n '935,960p' crates/perry-ui-macos/src/widgets/webview.rs

echo ""
echo "=== Check for any validation in the message handler (around line 601) ==="
sed -n '590,620p' crates/perry-ui-macos/src/widgets/webview.rs

Repository: PerryTS/perry

Length of output: 4928


🏁 Script executed:

#!/bin/bash
# Find the did_receive_script_message implementation
rg -n 'did_receive_script_message|WKScriptMessageHandler' crates/perry-ui-macos/src/widgets/webview.rs | head -20

echo ""
echo "=== Look for the handler implementation ==="
rg -n -A15 'fn did_receive_script_message' crates/perry-ui-macos/src/widgets/webview.rs

Repository: PerryTS/perry

Length of output: 1087


🏁 Script executed:

#!/bin/bash
# Get full implementation of did_receive_script_message
sed -n '461,530p' crates/perry-ui-macos/src/widgets/webview.rs

Repository: PerryTS/perry

Length of output: 2836


Restrict script bridge injection to main frame by default and require explicit opt-in for file-origin access.

The forMainFrameOnly: false setting (line 951) injects the perry IPC bridge into all frames including iframes, and allowUniversalAccessFromFileURLs (line 620) allows any file-origin to access cross-origin resources. The message handler (did_receive_script_message, lines 461–497) does not validate the origin or frame context before forwarding IPC calls, allowing untrusted iframe or file-origin content to reach the renderer→main bridge without restriction.

While both settings appear designed for Electron-style loadFile() compatibility, they should be opt-in and guarded behind explicit insecure flags rather than unconditionally enabled. Default to forMainFrameOnly: true and protect universal file-access behind a documented security caveat.

🤖 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-ui-macos/src/widgets/webview.rs` around lines 619 - 621, The
`allowUniversalAccessFromFileURLs` setting is unconditionally enabled and should
instead be made opt-in behind an explicit security configuration flag.
Additionally, locate the `forMainFrameOnly` setting (referenced around line 951)
and change its default from false to true to restrict script bridge injection to
the main frame only. Both settings should be guarded by explicit insecure
configuration options that require deliberate opt-in rather than being enabled
by default. Ensure the `did_receive_script_message` message handler includes
origin and frame context validation before forwarding IPC calls to the
renderer-main bridge.

Comment on lines +95 to +106
ipcMain.handle("notes:load", async () => {
try {
const text = fs.readFileSync(notesPath(), "utf8");
return JSON.parse(text);
} catch (e) {
return [];
}
});

ipcMain.handle("notes:save", async (_event, notes: string[]) => {
fs.writeFileSync(notesPath(), JSON.stringify(notes, null, 2), "utf8");
return { ok: true, count: notes.length };

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Validate notes schema at IPC boundary before returning/saving.

notes:load returns arbitrary parsed JSON, but the renderer assumes an array and immediately calls .map(...). A malformed notes.json can break the notes UI path. Validate both load and save payloads as string[] to keep the main↔renderer contract stable.

Proposed fix
 ipcMain.handle("notes:load", async () => {
   try {
     const text = fs.readFileSync(notesPath(), "utf8");
-    return JSON.parse(text);
+    const parsed = JSON.parse(text);
+    if (!Array.isArray(parsed)) return [];
+    return parsed.filter((n): n is string => typeof n === "string");
   } catch (e) {
     return [];
   }
 });
 
 ipcMain.handle("notes:save", async (_event, notes: string[]) => {
-  fs.writeFileSync(notesPath(), JSON.stringify(notes, null, 2), "utf8");
-  return { ok: true, count: notes.length };
+  const safeNotes = Array.isArray(notes) ? notes.filter((n): n is string => typeof n === "string") : [];
+  fs.writeFileSync(notesPath(), JSON.stringify(safeNotes, null, 2), "utf8");
+  return { ok: true, count: safeNotes.length };
 });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
ipcMain.handle("notes:load", async () => {
try {
const text = fs.readFileSync(notesPath(), "utf8");
return JSON.parse(text);
} catch (e) {
return [];
}
});
ipcMain.handle("notes:save", async (_event, notes: string[]) => {
fs.writeFileSync(notesPath(), JSON.stringify(notes, null, 2), "utf8");
return { ok: true, count: notes.length };
ipcMain.handle("notes:load", async () => {
try {
const text = fs.readFileSync(notesPath(), "utf8");
const parsed = JSON.parse(text);
if (!Array.isArray(parsed)) return [];
return parsed.filter((n): n is string => typeof n === "string");
} catch (e) {
return [];
}
});
ipcMain.handle("notes:save", async (_event, notes: string[]) => {
const safeNotes = Array.isArray(notes) ? notes.filter((n): n is string => typeof n === "string") : [];
fs.writeFileSync(notesPath(), JSON.stringify(safeNotes, null, 2), "utf8");
return { ok: true, count: safeNotes.length };
});
🤖 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 `@packages/electron/examples/system-explorer/main.ts` around lines 95 - 106,
The notes:load and notes:save IPC handlers lack schema validation for the
string[] contract between main and renderer processes. In the notes:load
handler, after parsing the JSON from notesPath(), add validation to ensure the
result is actually an array of strings before returning it; if validation fails,
return an empty array instead. In the notes:save handler, validate that the
incoming notes parameter is a string[] before calling writeFileSync; reject the
operation or return an error response if validation fails. This prevents
malformed notes.json from breaking the renderer's UI logic.

Comment on lines +23 to +25
document.getElementById("sysinfo").innerHTML = rows
.map((r) => '<div class="kv"><span>' + r[0] + "</span><span>" + r[1] + "</span></div>")
.join("");

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Replace dynamic innerHTML rendering with DOM node construction to prevent XSS.

The current string-based HTML rendering injects untrusted values (notably file names) directly into markup/attributes. A crafted filename can execute script in the renderer and then invoke privileged preload APIs. Build nodes with textContent/dataset instead of concatenated HTML strings.

Proposed fix
-  document.getElementById("sysinfo").innerHTML = rows
-    .map((r) => '<div class="kv"><span>' + r[0] + "</span><span>" + r[1] + "</span></div>")
-    .join("");
+  const sysEl = document.getElementById("sysinfo");
+  sysEl.textContent = "";
+  rows.forEach((r) => {
+    const row = document.createElement("div");
+    row.className = "kv";
+    const k = document.createElement("span");
+    const v = document.createElement("span");
+    k.textContent = String(r[0]);
+    v.textContent = String(r[1]);
+    row.appendChild(k);
+    row.appendChild(v);
+    sysEl.appendChild(row);
+  });
@@
-  const html = res.entries
-    .map(function (e) {
-      const ic = e.isDir ? "📁" : "📄";
-      const sz = e.isDir ? "" : '<span class="sz">' + fmtBytes(e.sizeBytes) + "</span>";
-      return (
-        '<div class="file" data-name="' +
-        e.name +
-        '" data-dir="' +
-        (e.isDir ? "1" : "0") +
-        '"><span class="ic">' +
-        ic +
-        '</span><span>' +
-        e.name +
-        "</span>" +
-        sz +
-        "</div>"
-      );
-    })
-    .join("");
   const filesEl = document.getElementById("files");
-  filesEl.innerHTML = html;
-  filesEl.querySelectorAll(".file").forEach(function (el) {
-    el.addEventListener("click", async function () {
-      if (el.getAttribute("data-dir") === "1") return;
-      const name = el.getAttribute("data-name");
+  filesEl.textContent = "";
+  const frag = document.createDocumentFragment();
+  res.entries.forEach(function (e) {
+    const el = document.createElement("div");
+    el.className = "file";
+    el.dataset.name = e.name;
+    el.dataset.dir = e.isDir ? "1" : "0";
+
+    const ic = document.createElement("span");
+    ic.className = "ic";
+    ic.textContent = e.isDir ? "📁" : "📄";
+    const name = document.createElement("span");
+    name.textContent = e.name;
+    el.appendChild(ic);
+    el.appendChild(name);
+    if (!e.isDir) {
+      const sz = document.createElement("span");
+      sz.className = "sz";
+      sz.textContent = fmtBytes(e.sizeBytes);
+      el.appendChild(sz);
+    }
+
+    el.addEventListener("click", async function () {
+      if (el.dataset.dir === "1") return;
+      const name = el.dataset.name || "";
       const fileRes = await api.readFile(lastDir + "/" + name);
       const pre = document.getElementById("preview");
       pre.textContent = fileRes.ok ? fileRes.text : "⚠️ " + fileRes.error;
     });
+    frag.appendChild(el);
   });
+  filesEl.appendChild(frag);

Also applies to: 35-56

🤖 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 `@packages/electron/examples/system-explorer/renderer/renderer.js` around lines
23 - 25, The code at lines 23-25 and 35-56 uses innerHTML with dynamically
concatenated HTML strings containing untrusted values (file names), which
creates an XSS vulnerability. Replace the innerHTML assignment with safe DOM
node construction by creating elements using document.createElement for the
container div and span elements, setting the untrusted values (r[0] and r[1])
using textContent instead of concatenating them into HTML strings, and then
appending the constructed nodes to the parent element. This ensures that
untrusted data is treated as text content rather than executable markup.

Source: Linters/SAST tools

Comment on lines +293 to +298
loadFile(filePath: string, _options?: any): Promise<void> {
// Resolve relative to cwd; Electron resolves relative to the app dir.
const abs = path.isAbsolute(filePath) ? filePath : path.join(process.cwd(), filePath);
webviewLoadUrl(this.webContents._wv, "file://" + abs);
return Promise.resolve();
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

file:// URL may break on paths with special characters.

filePath is concatenated directly into the URL without encoding. Paths containing spaces, #, ?, or other URL-significant characters will produce malformed URLs or truncated loads.

Consider using encodeURI or constructing via URL:

const fileUrl = new URL("file://" + abs).href;

Or at minimum:

webviewLoadUrl(this.webContents._wv, "file://" + encodeURI(abs));
🤖 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 `@packages/electron/src/index.ts` around lines 293 - 298, The loadFile method
constructs a file:// URL by directly concatenating the absolute path without
proper URL encoding, which will break for paths containing special characters
like spaces, hash marks, or question marks. Fix this by encoding the absolute
path before constructing the URL. In the webviewLoadUrl call, replace the string
concatenation of "file://" + abs with either the URL constructor approach (new
URL("file://" + abs).href) or by wrapping the abs variable with encodeURI to
ensure special characters are properly percent-encoded in the resulting URL.

…bs (electron/EventEmitter link); + electron-compat contributor DX fixes

Eight items from contributor feedback on the feat/electron-compat branch.

#7 (the real bug): perry/ui + events apps failed to link from source with
`Undefined symbols: _js_event_emitter_*`. build_optimized_libs splits
well-known bindings into CPU-only (events -> perry-ext-events.a, resolved
immediately into well_known_libs) and tokio-using (deferred). The warm-cache
early-return rebuilt the list from the tokio-using set ALONE, dropping every
CPU-only wrapper from the link line while the stdlib still stripped
bundled-events -> a stdlib calling js_event_emitter_* with no provider. Only
bit on a warm auto-optimize cache (cold cache took the fresh path and linked),
the intermittent "can't compile from source" report. Fix: cached path now
extends well_known_libs with the tokio-resolved set, mirroring the fresh path.
Reproduced + verified with the system-explorer example (links + runs).

#8: perry run . now resolves an entry inside a directory arg (perry.toml entry,
src/main.ts, main.ts) instead of erroring 'Is a directory'.

#3: perry init mirrors perry.packageAliases into tsconfig compilerOptions.paths
(create + best-effort merge into an existing tsconfig).

#4/#5: new packages/perry-react (@perryts/react) - bundles @types/react* and
augments react-dom/client with createRoot(null, PerryRootOptions). Validated
with tsc under moduleResolution bundler + node.

#1: root package.json gains name/version/private (npm arborist crash).
#2: packages/electron README documents the install workaround + scoped alias.
#6: verified webviewAddUserScript is wired on this branch (release-lag only).

Version 0.5.1204; changelog updated.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
crates/perry/src/commands/run/entry.rs (1)

112-115: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

entry key detection is too broad and can misread other TOML keys

Line 112 uses starts_with("entry"), so keys like entrypoint are treated as entry. That can return the wrong path and produce misleading “no entry point” failures.

Proposed fix
-        if trimmed.starts_with("entry") {
-            if let Some(eq_pos) = trimmed.find('=') {
-                let value = trimmed[eq_pos + 1..].trim().trim_matches('"');
-                return Some(PathBuf::from(value));
-            }
+        if let Some((key, raw_value)) = trimmed.split_once('=') {
+            if key.trim() == "entry" {
+                let value = raw_value.trim().trim_matches('"');
+                return Some(PathBuf::from(value));
+            }
         }
🤖 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/run/entry.rs` around lines 112 - 115, The issue is
that the condition using `starts_with("entry")` is too broad and matches TOML
keys like `entrypoint` in addition to the intended `entry` key. Fix this by
making the check more specific to only match the exact `entry` key. Instead of
checking if the trimmed line starts with the string "entry", verify that the key
portion (the part before the equals sign) is exactly "entry" by checking for
"entry=" or by splitting on the equals sign and comparing the trimmed left side
to "entry".
🤖 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 `@crates/perry/src/commands/init.rs`:
- Around line 156-167: The code preserves the existing `baseUrl` value in
`compilerOptions` at line 156 using `or_insert_with`, but the generated `paths`
targets at lines 250-252 are hardcoded as `./node_modules/...` without
accounting for the preserved `baseUrl`. Since the TypeScript resolver resolves
`paths` entries relative to `baseUrl` when it exists, this causes incorrect path
resolution (e.g., with `baseUrl: "src"`, paths become `src/node_modules/...`).
Fix this by ensuring the `baseUrl` is reset to `"."` instead of preserving the
existing value, or alternatively adjust the generated path targets to be correct
relative to whatever `baseUrl` value exists. The simplest fix is to change line
156 to unconditionally set `baseUrl` to `"."` instead of using `or_insert_with`.

In `@package.json`:
- Line 3: The hardcoded version value "0.0.0" in package.json conflicts with the
release workflow validation logic in .github/workflows/release-hono-server.yml,
which extracts the version from the git tag and compares it against the
package.json version field. Update the version field in package.json to reflect
the actual current release version or the version strategy used by your release
process, ensuring that the tag-derived version will match the value stored in
package.json during the publish-time validation check performed by the release
workflow.

In `@packages/perry-react/package.json`:
- Around line 1-55: The package.json file for `@perryts/react` is missing the
required perry.compilePackages configuration object. Add a new top-level perry
property to the package.json that contains a compilePackages array. This array
should list the package names that require native compilation, which in this
case should include the peer dependencies that this package augments (react and
react-dom).

---

Outside diff comments:
In `@crates/perry/src/commands/run/entry.rs`:
- Around line 112-115: The issue is that the condition using
`starts_with("entry")` is too broad and matches TOML keys like `entrypoint` in
addition to the intended `entry` key. Fix this by making the check more specific
to only match the exact `entry` key. Instead of checking if the trimmed line
starts with the string "entry", verify that the key portion (the part before the
equals sign) is exactly "entry" by checking for "entry=" or by splitting on the
equals sign and comparing the trimmed left side to "entry".
🪄 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: 8f6c7726-a603-410d-abe4-4904d961519c

📥 Commits

Reviewing files that changed from the base of the PR and between 7ea3e2a and f602810.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (14)
  • CHANGELOG.md
  • CLAUDE.md
  • Cargo.toml
  • crates/perry/src/commands/compile/optimized_libs.rs
  • crates/perry/src/commands/init.rs
  • crates/perry/src/commands/run/entry.rs
  • package.json
  • packages/electron/README.md
  • packages/electron/package.json
  • packages/perry-react/LICENSE
  • packages/perry-react/README.md
  • packages/perry-react/index.d.ts
  • packages/perry-react/package.json
  • packages/perry-react/tsconfig.json
✅ Files skipped from review due to trivial changes (7)
  • packages/perry-react/README.md
  • packages/perry-react/tsconfig.json
  • Cargo.toml
  • packages/perry-react/LICENSE
  • CHANGELOG.md
  • CLAUDE.md
  • packages/electron/README.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/electron/package.json

Comment on lines +156 to +167
co.entry("baseUrl")
.or_insert_with(|| serde_json::json!("."));
let existing_paths = co
.entry("paths")
.or_insert_with(|| serde_json::json!({}));
let existing_paths = existing_paths
.as_object_mut()
.ok_or_else(|| render_paths_block(paths))?;
let mut changed = false;
for (k, v) in paths {
if existing_paths.get(k) != Some(v) {
existing_paths.insert(k.clone(), v.clone());

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

baseUrl preservation makes generated alias paths incorrect for existing tsconfig files

Line 156 keeps a pre-existing compilerOptions.baseUrl, but Lines 250-252 generate paths targets as ./node_modules/.... Per the resolver behavior in crates/perry/src/commands/compile/resolve/tsconfig_paths.rs (Line 240-310), those targets are resolved relative to baseUrl when it exists, so baseUrl: "src" rewrites these to src/node_modules/... and breaks alias resolution in the IDE.

Proposed fix
-    co.entry("baseUrl")
-        .or_insert_with(|| serde_json::json!("."));
+    let mut changed = false;
+    if co.get("baseUrl").and_then(|v| v.as_str()) != Some(".") {
+        co.insert("baseUrl".to_string(), serde_json::json!("."));
+        changed = true;
+    }
@@
-    let mut changed = false;
     for (k, v) in paths {
         if existing_paths.get(k) != Some(v) {
             existing_paths.insert(k.clone(), v.clone());
             changed = true;
         }

Also applies to: 250-252

🤖 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/init.rs` around lines 156 - 167, The code preserves
the existing `baseUrl` value in `compilerOptions` at line 156 using
`or_insert_with`, but the generated `paths` targets at lines 250-252 are
hardcoded as `./node_modules/...` without accounting for the preserved
`baseUrl`. Since the TypeScript resolver resolves `paths` entries relative to
`baseUrl` when it exists, this causes incorrect path resolution (e.g., with
`baseUrl: "src"`, paths become `src/node_modules/...`). Fix this by ensuring the
`baseUrl` is reset to `"."` instead of preserving the existing value, or
alternatively adjust the generated path targets to be correct relative to
whatever `baseUrl` value exists. The simplest fix is to change line 156 to
unconditionally set `baseUrl` to `"."` instead of using `or_insert_with`.

Comment thread package.json
@@ -1,4 +1,8 @@
{
"name": "perry",
"version": "0.0.0",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

"version": "0.0.0" breaks the existing release contract.

Line 3 sets a static version, but .github/workflows/release-hono-server.yml compares tag-derived version against require('./package.json').version; tagged releases will fail unless the tag is hono-server-v0.0.0. Please align this value (or the workflow lookup target) so publish-time validation stays consistent.

🤖 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 `@package.json` at line 3, The hardcoded version value "0.0.0" in package.json
conflicts with the release workflow validation logic in
.github/workflows/release-hono-server.yml, which extracts the version from the
git tag and compares it against the package.json version field. Update the
version field in package.json to reflect the actual current release version or
the version strategy used by your release process, ensuring that the tag-derived
version will match the value stored in package.json during the publish-time
validation check performed by the release workflow.

Comment on lines +1 to +55
{
"name": "@perryts/react",
"version": "0.1.0",
"description": "TypeScript types for Perry's React support — bundles @types/react / @types/react-dom and augments react-dom/client's createRoot with Perry's native-window overload (createRoot(null, { title, width, height })).",
"types": "./index.d.ts",
"exports": {
".": {
"types": "./index.d.ts"
},
"./package.json": "./package.json"
},
"files": [
"index.d.ts",
"README.md",
"LICENSE"
],
"scripts": {
"typecheck": "tsc --noEmit -p tsconfig.json"
},
"keywords": [
"perry",
"react",
"react-dom",
"createRoot",
"native",
"types",
"typescript"
],
"author": "Perry Contributors",
"license": "MIT",
"repository": {
"type": "git",
"url": "git+https://github.com/PerryTS/perry.git",
"directory": "packages/perry-react"
},
"homepage": "https://github.com/PerryTS/perry/tree/main/packages/perry-react#readme",
"bugs": {
"url": "https://github.com/PerryTS/perry/issues"
},
"peerDependencies": {
"react": ">=18",
"react-dom": ">=18"
},
"peerDependenciesMeta": {
"react": { "optional": true },
"react-dom": { "optional": true }
},
"dependencies": {
"@types/react": "^18.3.0 || ^19.0.0",
"@types/react-dom": "^18.3.0 || ^19.0.0"
},
"publishConfig": {
"access": "public"
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Add required perry.compilePackages configuration.

This package.json is missing the required perry.compilePackages array for native compilation package configuration.

Suggested patch
 {
   "name": "`@perryts/react`",
   "version": "0.1.0",
@@
   "publishConfig": {
     "access": "public"
+  },
+  "perry": {
+    "compilePackages": []
   }
 }

As per coding guidelines, files matching **/package.json must define perry.compilePackages as an array of package names.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
{
"name": "@perryts/react",
"version": "0.1.0",
"description": "TypeScript types for Perry's React support — bundles @types/react / @types/react-dom and augments react-dom/client's createRoot with Perry's native-window overload (createRoot(null, { title, width, height })).",
"types": "./index.d.ts",
"exports": {
".": {
"types": "./index.d.ts"
},
"./package.json": "./package.json"
},
"files": [
"index.d.ts",
"README.md",
"LICENSE"
],
"scripts": {
"typecheck": "tsc --noEmit -p tsconfig.json"
},
"keywords": [
"perry",
"react",
"react-dom",
"createRoot",
"native",
"types",
"typescript"
],
"author": "Perry Contributors",
"license": "MIT",
"repository": {
"type": "git",
"url": "git+https://github.com/PerryTS/perry.git",
"directory": "packages/perry-react"
},
"homepage": "https://github.com/PerryTS/perry/tree/main/packages/perry-react#readme",
"bugs": {
"url": "https://github.com/PerryTS/perry/issues"
},
"peerDependencies": {
"react": ">=18",
"react-dom": ">=18"
},
"peerDependenciesMeta": {
"react": { "optional": true },
"react-dom": { "optional": true }
},
"dependencies": {
"@types/react": "^18.3.0 || ^19.0.0",
"@types/react-dom": "^18.3.0 || ^19.0.0"
},
"publishConfig": {
"access": "public"
}
}
{
"name": "`@perryts/react`",
"version": "0.1.0",
"description": "TypeScript types for Perry's React support — bundles `@types/react` / `@types/react-dom` and augments react-dom/client's createRoot with Perry's native-window overload (createRoot(null, { title, width, height })).",
"types": "./index.d.ts",
"exports": {
".": {
"types": "./index.d.ts"
},
"./package.json": "./package.json"
},
"files": [
"index.d.ts",
"README.md",
"LICENSE"
],
"scripts": {
"typecheck": "tsc --noEmit -p tsconfig.json"
},
"keywords": [
"perry",
"react",
"react-dom",
"createRoot",
"native",
"types",
"typescript"
],
"author": "Perry Contributors",
"license": "MIT",
"repository": {
"type": "git",
"url": "git+https://github.com/PerryTS/perry.git",
"directory": "packages/perry-react"
},
"homepage": "https://github.com/PerryTS/perry/tree/main/packages/perry-react#readme",
"bugs": {
"url": "https://github.com/PerryTS/perry/issues"
},
"peerDependencies": {
"react": ">=18",
"react-dom": ">=18"
},
"peerDependenciesMeta": {
"react": { "optional": true },
"react-dom": { "optional": true }
},
"dependencies": {
"`@types/react`": "^18.3.0 || ^19.0.0",
"`@types/react-dom`": "^18.3.0 || ^19.0.0"
},
"publishConfig": {
"access": "public"
},
"perry": {
"compilePackages": []
}
}
🤖 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 `@packages/perry-react/package.json` around lines 1 - 55, The package.json file
for `@perryts/react` is missing the required perry.compilePackages configuration
object. Add a new top-level perry property to the package.json that contains a
compilePackages array. This array should list the package names that require
native compilation, which in this case should include the peer dependencies that
this package augments (react and react-dom).

Source: Coding guidelines

@proggeramlug proggeramlug marked this pull request as draft June 25, 2026 18:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant