refactor: move database and services to a dedicated utility process - #10259
ZxBing0066 wants to merge 11 commits into
Conversation
✅ Circular References ReportGenerated at: 2026-07-31T07:30:36.506Z Summary
Click to view all circular references in PR (9)Click to view all circular references in base branch (9)Analysis✅ No Change: This PR does not introduce or remove any circular references. This report was generated automatically by comparing against the |
There was a problem hiding this comment.
Pull request overview
Refactors Insomnia’s database + insomnia-data service execution out of the main/renderer IPC handlers into a dedicated Electron utilityProcess, using a MessagePort-based RPC layer and a shared initDataBridge() initializer across entrypoints.
Changes:
- Added a new
data-processsubsystem (utility process entry, port-based RPC client/server, restart manager, and shared bridge initializer). - Updated main/renderer/hidden-window/plugin-window entrypoints + preloads to request/attach a data-process port and initialize
insomnia-dataviainitDataBridge(...). - Removed the legacy IPC-based database/services bridge proxies and updated build/test/debug configuration accordingly.
Reviewed changes
Copilot reviewed 28 out of 29 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/insomnia/types/global.d.ts | Replaces legacy window.database/services globals with window.invokeDataPort typing. |
| packages/insomnia/src/ui/services-proxy.ts | Removes renderer-side services proxy builder (superseded by data-port bridge). |
| packages/insomnia/src/ui/renderer-services-proxy.ts | Removes IPC-based renderer services proxy wiring. |
| packages/insomnia/src/ui/database.client.ts | Removes IPC-based renderer database client implementation. |
| packages/insomnia/src/main/ipc/main.ts | Removes services.invoke IPC handler (services now come from data-process RPC). |
| packages/insomnia/src/main/ipc/electron.ts | Adds data-process port request/restart channels; removes services.invoke channel. |
| packages/insomnia/src/main/ipc/database.ts | Removes legacy DatabaseBridgeAPI type (no longer exposed on window). |
| packages/insomnia/src/main/database.plugin-window.ts | Removes plugin-window database IPC proxy (replaced by data-process bridge). |
| packages/insomnia/src/main/database.main.ts | Removes main-process NeDB owner/IPC bridge (ownership moved to utility process). |
| packages/insomnia/src/konnect/tests/sync.test.ts | Updates tests to use in-memory NeDB directly (no mainDatabase dependency). |
| packages/insomnia/src/entry.preload.ts | Exposes invokeDataPort via preload using attachDataPortRpc. |
| packages/insomnia/src/entry.plugin-window.ts | Initializes insomnia-data via initDataBridge(window.invokeDataPort) for plugin window. |
| packages/insomnia/src/entry.plugin-window-preload.ts | Attaches data-port RPC in plugin window preload and assigns window.invokeDataPort. |
| packages/insomnia/src/entry.main.ts | Spawns utility process, issues ports to windows, initializes insomnia-data via main RPC. |
| packages/insomnia/src/entry.hidden-window.ts | Replaces services-global init with initDataBridge(window.invokeDataPort) gating script runs. |
| packages/insomnia/src/entry.hidden-window-preload.ts | Attaches and exposes invokeDataPort in hidden-window preload. |
| packages/insomnia/src/entry.data.ts | New utility process entry: initializes NeDB + servicesNodeImpl and starts the RPC server. |
| packages/insomnia/src/entry.client.tsx | Renderer now initializes insomnia-data via initDataBridge(window.invokeDataPort). |
| packages/insomnia/src/data-process/server.ts | Implements data-process-side request dispatch for database + services namespaces. |
| packages/insomnia/src/data-process/serialization.ts | Adds structured error serialization for cross-port error transport. |
| packages/insomnia/src/data-process/README.md | Documents architecture/topology and how to add windows to the data-process bridge. |
| packages/insomnia/src/data-process/port-rpc.ts | Adds transport-agnostic RPC client with correlation + restart invalidation. |
| packages/insomnia/src/data-process/init-data-bridge.ts | Centralizes wiring of initDatabase/initServices from a single invoke function. |
| packages/insomnia/src/data-process/data-process-manager.ts | Spawns/restarts utility process, maintains main RPC, and issues ports to windows. |
| packages/insomnia/src/data-process/data-port-preload.ts | Preload helper to request/attach a transferred MessagePort and expose InvokeFn. |
| packages/insomnia/setup-vitest.ts | Updates vitest setup to use in-memory NeDB without mainDatabase. |
| packages/insomnia/esbuild.entrypoints.ts | Adds esbuild entry/watch pipeline for entry.data.ts. |
| packages/insomnia-data/setup-vitest.ts | Removes unnecessary await on sync initServices. |
| .vscode/launch.json | Adds debugger attach config for the new data-process inspector port. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Replace IPC-based database/services bridge (database.main, database.client, services-proxy) with a MessagePort RPC layer backed by an Electron utilityProcess (entry.data.ts). - Add PortRpc transport, data-process server, and crash-restart manager - Unify all entry points to use initDataBridge(invokeDataPort)
baa50b9 to
7da5fa2
Compare
jackkav
left a comment
There was a problem hiding this comment.
Left some concerns below — request changes for now.
Issues / risks
1. serializeValue/deserializeValue can silently corrupt non-plain-object types other than Buffer/Date/Array. (serialization.ts) Anything else that is typeof === 'object' — a bare Uint8Array/ArrayBuffer (not wrapped as Buffer), Map, Set, RegExp, URL — falls into the generic Object.entries branch and gets flattened into a plain object (e.g. a Map becomes {}, a raw Uint8Array becomes {0: 1, 1: 2, ...}). If any IDatabase/Services method ever passes one of these across the boundary, the data is silently mangled rather than erroring. Can we add an explicit unsupported-type guard/warning, or confirm (and test) that no service method uses these types?
2. No retry when a window's data-process port request loses the race with process startup/restart. issuePort() (data-process-manager.ts) silently no-ops (just logs) if child isn't ready yet, and attachDataPortRpc's ipcRenderer.invoke('data-process.request-port') has no retry (just a .catch that logs). A window opened while the child is mid-crash-restart (or, in the macOS activate edge case, before the initial spawnDataProcess resolves) can end up with a permanently unattached bridge and no recovery path.
3. Restart exhaustion has no user-facing signal. After MAX_RESTARTS (10/60s), mainRpc.invalidate(...) is called but this is console.error-only — the app is now fully non-functional (no DB/services) with nothing telling the user to restart. Might be acceptable for now but worth a follow-up.
4. Widened privilege for the hidden window. Previously entry.hidden-window-preload.ts only exposed window._dataServices (services only — no database access at all). Now it exposes window.invokeDataPort, and entry.hidden-window.ts's initDataBridge unconditionally wires up both initDatabase and initServices. The hidden window's trusted host code (run-script.ts) now has full raw DB CRUD (docCreate, remove, unsafeRemove, batchModifyDocs, ...) reachable from its top-level context where before it had none. The sandboxed user-script layer itself is unaffected (still QuickJS-isolated via context.*), but this is a least-privilege regression for the host renderer process if it's ever the thing that's compromised. Can we confirm this access is actually needed, or scope it down?
5. Missing test coverage for the highest-risk new logic. data-process-manager.ts (spawn/crash-detection/backoff/port-reissue) and server.ts (namespace/method dispatch + error paths) have zero tests, despite being the most stateful and failure-prone pieces introduced here. port-rpc.test.ts/serialization.test.ts cover the easier, more mechanical pieces.
6. Scope creep — two unrelated fixes bundled in:
key-value-editor.tsx— an uncommitted-blank-row ref fix.in-sandbox-bootstrap.ts+ its regression test — a QuickJS use-after-free fix for fire-and-forget bridge calls.
Neither relates to INS-3052's stated scope (moving DB/services to a utility process). If they weren't caused by this refactor, could we split them into their own PRs? Would make this diff much easier to review and bisect if something regresses.
Minor
entry.data.ts'skeepAliveinterval is only cleared in thecatchbranch, never on success — intentional (keeps the utility process alive), but a one-line comment saying so would save the next reader a double-take.castMessageEventindata-process-manager.tsis aRecord<string, unknown>cast with no runtime validation ofmsg.type/msg.message/msg.uri/msg.changesshapes — low risk since it's Electron-internal, but any typo in apostMessagecall on either side would fail silently rather than at compile time.
I'd like #1 and #2 addressed or explicitly ruled out, and #4 double-checked, before merging — and would appreciate splitting out #6 if possible.
INS-3052
Replace IPC-based database/services bridge (database.main, database.client,
services-proxy) with a MessagePort RPC layer backed by an Electron utilityProcess (entry.data.ts).