Conversation
Move @elizaOS package pins to the available alpha dist-tag and record the Bun lockfile, including the Rollup WASM override used by local Vite builds. Replace the desktop preflight chain with a tolerant optional-check wrapper, skip absent postinstall bridge patch scripts, and restore the top-level production build wrapper. Stub server-only elizaOS runtime imports in the app Vite build so renderer bundles do not pull native API-side modules.
ApprovabilityVerdict: Needs human review Unable to check for correctness in ba530fe. Diff is too large for automated approval analysis. A human reviewer should evaluate this PR. You can customize Macroscope's approvability policy. Learn more. |
|
Credit balance is too low |
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| Compatibility | 5 medium 18 high |
| UnusedCode | 14 medium |
| ErrorProne | 10 high |
| CodeStyle | 1 minor |
| Complexity | 1 medium |
| Performance | 1 medium |
🟢 Metrics 56 complexity · 0 duplication
Metric Results Complexity 56 Duplication 0
AI Reviewer: first review requested successfully. AI can make mistakes. Always validate suggestions.
TIP This summary will be updated as you push new changes.
There was a problem hiding this comment.
Pull request overview
Restores and hardens Milady’s install/build entrypoints by repinning elizaOS packages to the available alpha dist-tag, reintroducing a top-level production build wrapper, making desktop preflight checks tolerant of missing scripts, and expanding Vite stubbing so renderer bundles don’t pull server-only elizaOS runtime modules.
Changes:
- Replaced the chained
dev:desktop:preflightscript with a new wrapper that skips missing optional checks. - Added a top-level
scripts/run-production-build.mjswrapper delegating into@elizaos/app-core. - Updated
@elizaos/*dependency pins toalpha, added a Rollup WASM override, and extended Vite’s native-module stubbing for server-only imports.
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| scripts/run-production-build.mjs | Restores a production build entrypoint delegating to app-core. |
| scripts/milady-postinstall-repo-setup.mjs | Skips missing Milady bridge patch scripts instead of hard-failing postinstall. |
| scripts/dev-desktop-preflight.mjs | New tolerant preflight runner for optional desktop checks. |
| package.json | Switches desktop preflight to wrapper, repins elizaOS deps to alpha, and adds Rollup WASM override. |
| apps/homepage/package.json | Repins homepage elizaOS deps to alpha. |
| apps/app/vite.config.ts | Expands renderer stubbing for server-only elizaOS/agent/plugin imports and adds missing export shims. |
| apps/app/package.json | Repins app elizaOS deps to alpha. |
There was a problem hiding this comment.
Pull Request Overview
This PR successfully transitions dependencies to alpha tags and introduces scripts intended to improve the developer experience on Windows and production build reliability. However, several critical areas require attention before merging:
- The new scripts
scripts/dev-desktop-preflight.mjsandscripts/run-production-build.mjsshow an increase in logic complexity but lack any unit or integration tests to verify their resilience. - The Vite configuration's
nativeModuleStubPluginuses global, context-unaware regex patterns to detect exports. This approach is brittle and could lead to broken builds if exports are formatted across multiple lines or if identifiers appear within comments. - There are minor consistency issues with Windows path normalization in the Vite plugin logic.
About this PR
- The mechanism for avoiding duplicate export injections in the Vite transform phase is context-unaware. Reliance on simple regex for code transformation can fail on multi-line exports or code containing documentation comments that mirror export names.
- The new logic in 'dev-desktop-preflight.mjs' and 'run-production-build.mjs' lacks unit or integration tests. Given these scripts are meant to handle environment-specific edge cases, automated verification is recommended to prevent regressions during future updates.
Test suggestions
- Verify that dev-desktop-preflight.mjs correctly identifies and skips non-existent scripts without throwing errors.
- Verify that run-production-build.mjs correctly sets MILADY_REPO_ROOT and delegates to the app-core build script.
- Test Vite nativeModuleStubPlugin's path normalization to ensure it correctly stubs paths containing Windows backslashes.
- Confirm that the Vite transform regex 'directExport' correctly detects existing 'export const/function' declarations to avoid duplicate stubs.
Prompt proposal for missing tests
Consider implementing these tests if applicable:
1. Verify that dev-desktop-preflight.mjs correctly identifies and skips non-existent scripts without throwing errors.
2. Verify that run-production-build.mjs correctly sets MILADY_REPO_ROOT and delegates to the app-core build script.
3. Test Vite nativeModuleStubPlugin's path normalization to ensure it correctly stubs paths containing Windows backslashes.
4. Confirm that the Vite transform regex 'directExport' correctly detects existing 'export const/function' declarations to avoid duplicate stubs.
TIP Improve review quality by adding custom instructions
TIP How was this review? Give us feedback
|
Credit balance is too low |
Include the new desktop preflight and production build wrappers in the published files allowlist. Normalize the agent subpath check before matching and rely on the @elizaos/plugin-* catch-all instead of duplicate explicit plugin entries.
|
Credit balance is too low |
Point the root build script at scripts/run-production-build.mjs so package-mode CI uses the Milady wrapper instead of invoking the app-core script directly with a stale Node executable path.
|
Credit balance is too low |
Re-add the root script aliases consumed by the Electrobun release workflows, including test:regression-matrix:release-contract and the release contract/e2e package checks. Keep heavy-only e2e paths explicitly excluded from the default e2e command and included in test:e2e:heavy so validate-regression-matrix passes.
|
Credit balance is too low |
Split the generated logger shim payload out of ensureLoggerShimPackage, remove nested template-string content from the shim source, and scope the dynamic write helper to the known shim directory.
|
Credit balance is too low |
Replace the generated @elizaos/logger string payload with a tracked shim package under scripts/shims. The alignment script now links that package directly, avoiding the oversized helper, generated template strings, and dynamic file writes Codacy reported.
|
Credit balance is too low |
Keep the @elizaos/logger compatibility shim as vendored code and point CI alignment at vendor/elizaos-logger. This avoids Codacy treating the shim declaration files as normal scripts while preserving the local package link needed for eliza refs that import @elizaos/logger.
|
Credit balance is too low |
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| Complexity | 1 medium |
🟢 Metrics 31 complexity · 0 duplication
Metric Results Complexity 31 Duplication 0
AI Reviewer: first review requested successfully. AI can make mistakes. Always validate suggestions.
TIP This summary will be updated as you push new changes.
Resolve a standard Node runtime before spawning build children and fall back to Bun when only the Codex-bundled macOS Node is visible. This prevents Rolldown native bindings from loading under the signed Codex Node while keeping fresh Bun-only developer installs buildable.
|
Credit balance is too low |
Move @elizaOS package pins to the available alpha dist-tag and record the Bun lockfile, including the Rollup WASM override used by local Vite builds.
Replace the desktop preflight chain with a tolerant optional-check wrapper, skip absent postinstall bridge patch scripts, and restore the top-level production build wrapper.
Stub server-only elizaOS runtime imports in the app Vite build so renderer bundles do not pull native API-side modules.
Note
Restore Milady install and build entrypoints with Node 22+ validation and plugin stubbing
MILADY_REPO_ROOTand CLI args to the downstream build script@elizaos/plugin-*bare import and any path under/packages/agent/src/as virtual modules during browser bundling, replacing the previous explicit plugin list; also adds stubs forregisterJsRuntimeFactoryandreadWorkspaceFolderConfigpackage.jsonand apppackage.jsonfiles switch@elizaos/*dependencies frombeta/pinned versions to thealphatag@elizaos/*deps toalphameans any breaking alpha release will affect installs immediatelyMacroscope summarized ba530fe.