🐛 fix(desktop): prevent duplicate IPC handler registration from dynamic imports#11827
Conversation
…ic imports Fix an issue where dynamic imports in file-loaders package would cause the debug package to be bundled into index.js, leading to side-effect pollution and duplicate electron-log IPC handler registration. - Add manualChunks config to isolate debug package into separate chunk - Add @napi-rs/canvas to native modules for proper externalization
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Reviewer's guide (collapsed on small PRs)Reviewer's GuideConfigures Rollup output in the desktop Electron Vite config to isolate the File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
TestGru AssignmentSummary
Tip You can |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## next #11827 +/- ##
========================================
Coverage 73.64% 73.64%
========================================
Files 1213 1213
Lines 97619 97619
Branches 13091 12653 -438
========================================
Hits 71896 71896
Misses 25632 25632
Partials 91 91
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
|
❤️ Great PR @Innei ❤️ The growth of project is inseparable from user feedback and contribution, thanks for your contribution! If you are interesting with the lobehub developer community, please join our discord and then dm @arvinxx or @canisminor1990. They will invite you to our private developer channel. We are talking about the lobe-chat development or sharing ai newsletter around the world. |
## [Version 2.0.0-next.379](v2.0.0-next.378...v2.0.0-next.379) <sup>Released on **2026-01-25**</sup> #### ✨ Features - **utils**: Added `trimBasedOnBatchProbe` for truncating without compromising structured data. #### 🐛 Bug Fixes - **desktop**: Prevent duplicate IPC handler registration from dynamic imports. - **misc**: Fix update memory tools, resolve server version check issue for desktop app, slove the descktop use offical endpoint mcp not use stdio. <br/> <details> <summary><kbd>Improvements and Fixes</kbd></summary> #### What's improved * **utils**: Added `trimBasedOnBatchProbe` for truncating without compromising structured data, closes [#11836](#11836) ([6dac3d1](6dac3d1)) #### What's fixed * **desktop**: Prevent duplicate IPC handler registration from dynamic imports, closes [#11827](#11827) ([c3fd2dc](c3fd2dc)) * **misc**: Fix update memory tools, closes [#11831](#11831) ([cfc03dd](cfc03dd)) * **misc**: Resolve server version check issue for desktop app, closes [#11834](#11834) ([0bd2a59](0bd2a59)) * **misc**: Slove the descktop use offical endpoint mcp not use stdio, closes [#11813](#11813) ([370bf16](370bf16)) </details> <div align="right"> [](#readme-top) </div>
|
🎉 This PR is included in version 2.0.0-next.379 🎉 The release is available on: Your semantic-release bot 📦🚀 |
…eged should be called before app is ready`
Two-part fix for a regression where reading any text/JSON/source file via the
local-system `readFile` tool surfaced an Electron protocol error in the response
content. The error fired *after* `stat()` succeeded (so missing-file ENOENT was
unaffected), making it look like the file couldn't be parsed.
## Root cause
Stack trace (instrumented `read.ts` to capture it):
```
Error: protocol.registerSchemesAsPrivileged should be called before app is ready
at new App (apps/desktop/dist/main/index.js:105339:21)
at Module.<anonymous> (apps/desktop/dist/main/index.js:105615:11)
at Module._compile (...)
```
`Module._compile` on `dist/main/index.js` means the main bundle is being freshly
evaluated as a CJS module — re-running its top-level `var app = new App(); …;
app.bootstrap();` after the real Electron-launched App was already ready.
Triggering chain: agent calls `readFile` → main runs `loadFile(path)` from
`@lobechat/file-loaders` → `getFileLoader('txt')` → `await import('./text')`.
The lazy text-loader chunk back-references the main bundle for the shared util
`detectUtf16NoBom`:
```js
// dist/main/text-Cbmlmtca.js
const require_index = require("./index.js"); // ← re-evaluates main
…
const variant = require_index.detectUtf16NoBom(buffer);
```
Electron's main entry is not in Node's CJS module cache (it's bootstrapped
separately), so this `require("./index.js")` triggers a fresh compile of the
main bundle — re-running `new App()` and `protocol.registerSchemesAsPrivileged`
*after* `app.whenReady()`, which is illegal per Electron's API contract.
Introduced by #14602 (`fix(local-system): guard readFile against binary blobs
and oversized output`): adding `isBinaryContent.ts` made `detectUtf16NoBom`
shared between the main bundle (via `sniffBinaryFile`) and the lazy text chunk,
so rolldown placed it in main and rewrote the text chunk's call as a
`require_index.detectUtf16NoBom`.
Identical class of bug previously fixed for the `debug` package in #11827.
## Fix
1. **`packages/file-loaders/src/loaders/index.ts`** — TextLoader was lazy-imported
for no real benefit. It's a 10KB module whose only deps are `node:fs/promises`
and a tiny utf-16 detect util — nothing like the multi-MB parsers (pdfjs-dist,
xlsx, mammoth) that the lazy pattern was designed for. Make it a static
import; `getFileLoader('txt')` returns it synchronously. Result: the text
chunk disappears entirely, removing this back-reference at the source.
2. **`apps/desktop/electron.vite.config.ts`** — defensive `manualChunks` rules
so any future shared symbol doesn't recreate the same trap:
- `vendor-file-loaders-utils` for the three small text/binary detection
utils (`detectUtf16` / `isBinaryContent` / `isTextReadableFile`).
Explicitly enumerated to avoid catching `parser-utils.ts`, which pulls
in xmldom/yauzl/concat-stream (≈900KB) and belongs in the docx/pptx
chunks instead.
- `vendor-jszip` for JSZip — same root cause for `.docx` reads: the docx
chunk had `require_index.require_lib()` (JSZip) back-referencing main.
Both ends now share the vendor chunk; no main re-eval.
Follows the project precedent set by #11827 for `debug`.
## Verification (live Electron via CDP)
Bundle inventory before/after:
| Chunk | Before | After |
| --- | --- | --- |
| `text-*.js` | 9.7KB (back-refs main) | (gone, inlined into main) |
| `vendor-file-loaders-utils-*.js` | n/a | 18KB |
| `vendor-jszip-*.js` | n/a | 899KB |
| `docx-*.js` back-refs | `require_index.require_lib` | none |
End-to-end via `tool.invokeBuiltinTool('lobe-local-system', 'readFile', …)`:
| File | Before | After |
| --- | --- | --- |
| `.md` / `.json` / `.ts` | `Error accessing or processing file: protocol.registerSchemesAsPrivileged should be called before app is ready` | real file content |
`grep -o 'require_index\\.[a-zA-Z_]*' dist/main/*-*.js | sort -u` → empty.
All 61 file-loaders tests pass; all 64 builtin-tool-local-system tests pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…o /client (#14888) * 🐛 fix(local-system): forward all grepContent params + move executor to /client The local-system executor was reducing the agent's full grepContent params ({pattern, glob, output_mode, -i/-n/-A/-B/-C, multiline, head_limit, type, scope, ...}) down to {directory, pattern} before handing them to the runtime. `directory` isn't recognized by the IPC layer (which expects path/scope), so cwd silently fell back to process.cwd() (= apps/desktop/ in dev), and with glob/-i/output_mode all stripped grep matched anything containing the pattern across the whole tree — explaining LOBE-8666's dist/main/index.js + tsconfig.tsbuildinfo leaks. Also audited the rest of the executor layer: - listFiles: forward `limit` (was silently dropped → manifest default of 100 always won). - getCommandOutput: forward `filter` (was silently dropped → no regex filter ever applied to streamed output). - runCommand: mirror `run_in_background` → `background` so ComputerRuntime.RunCommandState.isBackground reflects reality (the IPC handler reads run_in_background directly, so the command itself ran in background — only the state field was wrong). Structure: moved src/executor/ → src/client/executor/ to match the other builtin-tool packages (task / lobe-agent / knowledge-base) and consolidate renderer-only code under /client. Dropped the `./executor` package subpath; consumers now import from `…/client`. Defensive: also added a resolveSearchPath helper in apps/desktop's contentSearch module that reads params.scope as a fallback for params.path, so any non-executor caller (direct IPC, future Gateway path) that passes `scope` still gets routed correctly instead of falling through to process.cwd(). Regression coverage: - grepContent full forwarding (LOBE-8666 case + all optional flags) - listFiles.limit forwarding - getCommandOutput.filter forwarding - runCommand.run_in_background → background mirror - resolveSearchPath fallback semantics (3 cases in base.test.ts) Verified end-to-end via Electron CDP — tool.invokeBuiltinTool with the LOBE-8666 params returns 9 clean .ts matches (no dist/, no .tsbuildinfo); listFiles {limit:3} returns 3 files (totalCount 10); runCommand {run_in_background:true} reports state.isBackground=true. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * 🐛 fix(desktop): readFile fails with `protocol.registerSchemesAsPrivileged should be called before app is ready` Two-part fix for a regression where reading any text/JSON/source file via the local-system `readFile` tool surfaced an Electron protocol error in the response content. The error fired *after* `stat()` succeeded (so missing-file ENOENT was unaffected), making it look like the file couldn't be parsed. ## Root cause Stack trace (instrumented `read.ts` to capture it): ``` Error: protocol.registerSchemesAsPrivileged should be called before app is ready at new App (apps/desktop/dist/main/index.js:105339:21) at Module.<anonymous> (apps/desktop/dist/main/index.js:105615:11) at Module._compile (...) ``` `Module._compile` on `dist/main/index.js` means the main bundle is being freshly evaluated as a CJS module — re-running its top-level `var app = new App(); …; app.bootstrap();` after the real Electron-launched App was already ready. Triggering chain: agent calls `readFile` → main runs `loadFile(path)` from `@lobechat/file-loaders` → `getFileLoader('txt')` → `await import('./text')`. The lazy text-loader chunk back-references the main bundle for the shared util `detectUtf16NoBom`: ```js // dist/main/text-Cbmlmtca.js const require_index = require("./index.js"); // ← re-evaluates main … const variant = require_index.detectUtf16NoBom(buffer); ``` Electron's main entry is not in Node's CJS module cache (it's bootstrapped separately), so this `require("./index.js")` triggers a fresh compile of the main bundle — re-running `new App()` and `protocol.registerSchemesAsPrivileged` *after* `app.whenReady()`, which is illegal per Electron's API contract. Introduced by #14602 (`fix(local-system): guard readFile against binary blobs and oversized output`): adding `isBinaryContent.ts` made `detectUtf16NoBom` shared between the main bundle (via `sniffBinaryFile`) and the lazy text chunk, so rolldown placed it in main and rewrote the text chunk's call as a `require_index.detectUtf16NoBom`. Identical class of bug previously fixed for the `debug` package in #11827. ## Fix 1. **`packages/file-loaders/src/loaders/index.ts`** — TextLoader was lazy-imported for no real benefit. It's a 10KB module whose only deps are `node:fs/promises` and a tiny utf-16 detect util — nothing like the multi-MB parsers (pdfjs-dist, xlsx, mammoth) that the lazy pattern was designed for. Make it a static import; `getFileLoader('txt')` returns it synchronously. Result: the text chunk disappears entirely, removing this back-reference at the source. 2. **`apps/desktop/electron.vite.config.ts`** — defensive `manualChunks` rules so any future shared symbol doesn't recreate the same trap: - `vendor-file-loaders-utils` for the three small text/binary detection utils (`detectUtf16` / `isBinaryContent` / `isTextReadableFile`). Explicitly enumerated to avoid catching `parser-utils.ts`, which pulls in xmldom/yauzl/concat-stream (≈900KB) and belongs in the docx/pptx chunks instead. - `vendor-jszip` for JSZip — same root cause for `.docx` reads: the docx chunk had `require_index.require_lib()` (JSZip) back-referencing main. Both ends now share the vendor chunk; no main re-eval. Follows the project precedent set by #11827 for `debug`. ## Verification (live Electron via CDP) Bundle inventory before/after: | Chunk | Before | After | | --- | --- | --- | | `text-*.js` | 9.7KB (back-refs main) | (gone, inlined into main) | | `vendor-file-loaders-utils-*.js` | n/a | 18KB | | `vendor-jszip-*.js` | n/a | 899KB | | `docx-*.js` back-refs | `require_index.require_lib` | none | End-to-end via `tool.invokeBuiltinTool('lobe-local-system', 'readFile', …)`: | File | Before | After | | --- | --- | --- | | `.md` / `.json` / `.ts` | `Error accessing or processing file: protocol.registerSchemesAsPrivileged should be called before app is ready` | real file content | `grep -o 'require_index\\.[a-zA-Z_]*' dist/main/*-*.js | sort -u` → empty. All 61 file-loaders tests pass; all 64 builtin-tool-local-system tests pass.
Summary
__ELECTRON_LOG__IPC handler registration caused by Rollup bundlingdebugpackage intoindex.js@lobechat/file-loadersloaded, they wouldrequire("./index.js")to get shareddebugdependency, re-executing side-effect code including electron-log's IPC handler registrationmanualChunksconfig to isolatedebugpackage into separate chunk (vendor-debug.js)@napi-rs/canvasto native modules list for proper externalizationRoot Cause
Rollup's default code-splitting strategy merges shared dependencies into the entry chunk (
index.js). When dynamic import chunks need these shared deps, theyrequirethe entry chunk, causing side-effects (like IPC handler registration) to execute again in CJS format.Test plan
Summary by Sourcery
Adjust desktop Electron bundling to avoid duplicate IPC handler side effects and ensure native dependencies are properly externalized.
Build:
Deployment: