From 61aedcf86e655cdab839e18cf8d9ac329fa69aca Mon Sep 17 00:00:00 2001 From: Bingbing Date: Tue, 3 Nov 2026 11:44:17 +0800 Subject: [PATCH] refactor: break main-process circular imports (git-service/plugin-window/plugin discovery) ## Background The main process carried three circular import groups that `scripts/circular-references/known-violations.json` had to record: - `main/git-service.ts` and `sync/git/repo-file-watcher.ts` imported each other because the `WorkspaceFileIssue` type lived in `git-service`, while the watcher that produces those issues imported the type back. - `main/plugin-window.ts` reached `main/window-utils.ts` for `getMainWindow`, both directly and through `main/prompt-bridge.ts`. - `main/templating-worker-database.ts` and `plugins/index.ts` imported each other because `plugins/index` lazily imported the sandbox-backed user-plugin export discovery from the main-only templating worker module. ## Changes - Move `WorkspaceFileIssue` to `sync/git/repo-file-watcher.ts`, which owns the file issues, and update the renderer type imports. - Add `main/window-registry.ts` to own `browserWindows` and `getMainWindow`, so `plugin-window`, `prompt-bridge` and `templating-db-auth` read the main window without importing `window-utils`. `window-utils` still writes the registry and drives the plugin window directly. - Add `common/templating/user-plugin-export-discovery.ts` as the registration slot for the sandbox-backed discovery. `main/templating-worker-database` registers its implementation at module load, and `plugins/index` calls the registered function in the main process while keeping the protocol path in renderers. Calling it without registration throws an explicit error. - Remove the five resolved cycles from `scripts/circular-references/known-violations.json`. - Register the discovery stub through the new hook in the plugin-load-order QuickJS test, and assert the main-process registration contract in the templating-worker-database test. ## Testing - `npm run check-cycle-references`: insomnia 13 -> 8 cycles, 0 new. - `npm run type-check -w insomnia` and eslint on the changed files: clean. - `npx vitest run` for the plugins suite, the templating-worker-database suites, plugin-window IPC authorization, and the window security/guard tests: 134 passed, 1 skipped. - `npx vitest run` for the repo-file-watcher and import suites: 75 passed. - `npm run build:electron-entrypoints`: succeeds. ## References Follows #10496 in the circular-reference cleanup chain. --- .../user-plugin-export-discovery.ts | 27 ++++++++++ .../plugin-window-ipc-authorization.test.ts | 2 +- .../templating-worker-database.test.ts | 8 +++ packages/insomnia/src/main/git-service.ts | 10 +--- .../src/main/ipc/templating-db-auth.ts | 2 +- packages/insomnia/src/main/plugin-window.ts | 2 +- packages/insomnia/src/main/prompt-bridge.ts | 2 +- .../src/main/templating-worker-database.ts | 7 +++ packages/insomnia/src/main/window-registry.ts | 11 ++++ packages/insomnia/src/main/window-utils.ts | 5 +- ...ad-order-quickjs-module-resolution.test.ts | 51 +++++++++++-------- packages/insomnia/src/plugins/index.ts | 8 +-- .../src/sync/git/repo-file-watcher.ts | 15 +++++- .../insomnia/src/ui/components/tabs/tab.tsx | 2 +- .../src/ui/hooks/use-git-file-issues.ts | 3 +- .../circular-references/known-violations.json | 5 -- 16 files changed, 108 insertions(+), 52 deletions(-) create mode 100644 packages/insomnia/src/common/templating/user-plugin-export-discovery.ts create mode 100644 packages/insomnia/src/main/window-registry.ts diff --git a/packages/insomnia/src/common/templating/user-plugin-export-discovery.ts b/packages/insomnia/src/common/templating/user-plugin-export-discovery.ts new file mode 100644 index 0000000000..200925f695 --- /dev/null +++ b/packages/insomnia/src/common/templating/user-plugin-export-discovery.ts @@ -0,0 +1,27 @@ +import type { PluginExportManifest } from '~/templating/sandbox/marshal'; + +// Shared registry for the sandbox-backed user-plugin export discovery. `plugins/index` runs in both +// the main process and the renderer (plugin window) and must not statically import +// `main/templating-worker-database` (the main-side implementation) — that module in turn loads +// `plugins/index` for getPlugins/getTemplateTags, which would be a circular dependency. Keeping the +// registration slot in this neutral module lets both sides depend on it without importing each other. +type UserPluginExportDiscovery = (body: { + directory: string; + name: string; + permissions?: { modules?: string[]; capabilities?: string[] }; +}) => Promise; + +let discoverUserPluginExportsInMain: UserPluginExportDiscovery | null = null; + +export function registerUserPluginExportDiscovery(discover: UserPluginExportDiscovery): void { + discoverUserPluginExportsInMain = discover; +} + +export function getRegisteredUserPluginExportDiscovery(): UserPluginExportDiscovery { + if (!discoverUserPluginExportsInMain) { + throw new Error( + 'User plugin export discovery is not registered; sandboxed plugin discovery requires the Electron main process (main/templating-worker-database)', + ); + } + return discoverUserPluginExportsInMain; +} diff --git a/packages/insomnia/src/main/__tests__/plugin-window-ipc-authorization.test.ts b/packages/insomnia/src/main/__tests__/plugin-window-ipc-authorization.test.ts index 4d09d4ab29..408151b05e 100644 --- a/packages/insomnia/src/main/__tests__/plugin-window-ipc-authorization.test.ts +++ b/packages/insomnia/src/main/__tests__/plugin-window-ipc-authorization.test.ts @@ -40,7 +40,7 @@ vi.mock('electron', () => ({ }, })); -vi.mock('../window-utils', () => ({ +vi.mock('../window-registry', () => ({ getMainWindow: vi.fn(() => fakeMainWindow), })); diff --git a/packages/insomnia/src/main/__tests__/templating-worker-database.test.ts b/packages/insomnia/src/main/__tests__/templating-worker-database.test.ts index 12c21e3f5f..95feecd813 100644 --- a/packages/insomnia/src/main/__tests__/templating-worker-database.test.ts +++ b/packages/insomnia/src/main/__tests__/templating-worker-database.test.ts @@ -46,10 +46,12 @@ vi.mock('../prompt-bridge', () => ({ requestPromptFromRenderer: vi.fn() })); vi.mock('../secure-read-file', () => ({ secureReadFile: vi.fn() })); import { parsePluginPermissions } from '~/common/plugins/permissions'; +import { getRegisteredUserPluginExportDiscovery } from '~/common/templating/user-plugin-export-discovery'; import { requestPromptFromRenderer } from '../prompt-bridge'; import { _testOnlyResetMigrationWarnings, + discoverUserPluginExportsForLoader, getPluginEntrySource, maybeWarnMissingManifest, readPluginModuleMap, @@ -280,3 +282,9 @@ describe('resolveDbByKey — app.prompt', () => { }); }); }); + +describe('user plugin export discovery registration', () => { + it('registers the sandbox-backed discovery with the plugin loader', () => { + expect(getRegisteredUserPluginExportDiscovery()).toBe(discoverUserPluginExportsForLoader); + }); +}); diff --git a/packages/insomnia/src/main/git-service.ts b/packages/insomnia/src/main/git-service.ts index c838acffa0..5537bbacb8 100644 --- a/packages/insomnia/src/main/git-service.ts +++ b/packages/insomnia/src/main/git-service.ts @@ -41,7 +41,7 @@ import { type ProviderEmail, type ProviderRepository, } from '~/sync/git/providers'; -import type { FileIssue, FileIssueKind } from '~/sync/git/repo-file-watcher'; +import type { FileIssue, WorkspaceFileIssue } from '~/sync/git/repo-file-watcher'; import { getAppVersion, INSOMNIA_GITLAB_API_URL } from '../common/constants'; import { database } from '../common/database'; @@ -184,14 +184,6 @@ export function vcsEventProperties(type: 'git', action: VCSAction, error?: strin return { type, action, error }; } -export interface WorkspaceFileIssue { - workspaceId: string; - gitRepositoryId: string; - relPath: string; - kind: FileIssueKind; - message: string; -} - interface GetProjectGitFileIssuesOptions { projectId: string; workspaceId?: string; diff --git a/packages/insomnia/src/main/ipc/templating-db-auth.ts b/packages/insomnia/src/main/ipc/templating-db-auth.ts index 88ce595d71..6c3d497492 100644 --- a/packages/insomnia/src/main/ipc/templating-db-auth.ts +++ b/packages/insomnia/src/main/ipc/templating-db-auth.ts @@ -2,7 +2,7 @@ import type { IpcMainInvokeEvent } from 'electron'; import { getPluginWindow } from '../plugin-window'; import { getOrCreateTemplatingDbAuthToken } from '../templating-worker-database-auth'; -import { getMainWindow } from '../window-utils'; +import { getMainWindow } from '../window-registry'; import { ipcMainHandle } from './electron'; // Only hands the templating db auth token to the main app window or the hidden plugin window. diff --git a/packages/insomnia/src/main/plugin-window.ts b/packages/insomnia/src/main/plugin-window.ts index 545c39ae70..eba822f76a 100644 --- a/packages/insomnia/src/main/plugin-window.ts +++ b/packages/insomnia/src/main/plugin-window.ts @@ -4,7 +4,7 @@ import path from 'node:path'; import { app, BrowserWindow, ipcMain, type IpcMainEvent, type IpcMainInvokeEvent } from 'electron'; import { requestPromptFromRenderer } from './prompt-bridge'; -import { getMainWindow } from './window-utils'; +import { getMainWindow } from './window-registry'; let pluginWindow: BrowserWindow | null = null; let windowReady = false; diff --git a/packages/insomnia/src/main/prompt-bridge.ts b/packages/insomnia/src/main/prompt-bridge.ts index 3107f59fe9..fba1166e64 100644 --- a/packages/insomnia/src/main/prompt-bridge.ts +++ b/packages/insomnia/src/main/prompt-bridge.ts @@ -4,7 +4,7 @@ import { ipcMain } from 'electron'; import type { AppPromptOptions } from '~/common/templating/types'; -import { getMainWindow } from './window-utils'; +import { getMainWindow } from './window-registry'; export interface PromptRequestOptions extends AppPromptOptions { title: string; diff --git a/packages/insomnia/src/main/templating-worker-database.ts b/packages/insomnia/src/main/templating-worker-database.ts index cdfb78ebad..1931d04865 100644 --- a/packages/insomnia/src/main/templating-worker-database.ts +++ b/packages/insomnia/src/main/templating-worker-database.ts @@ -20,6 +20,7 @@ import type { PluginTemplateTagContext, PluginToMainAPIPaths, } from '~/common/templating/types'; +import { registerUserPluginExportDiscovery } from '~/common/templating/user-plugin-export-discovery'; import { getPluginCommonContext, getPlugins, getTemplateTags } from '~/plugins'; import { HOOK_REQUEST_FIELDS, @@ -482,6 +483,12 @@ export const discoverUserPluginExportsForLoader = async (body: { }); }; +// Register the sandbox-backed user-plugin discovery with the plugin loader (plugins/index.ts) via +// the shared discovery registry, which exists because plugins/index.ts can't import this +// main-only module without creating a circular dependency. Loading this module in main always +// precedes plugin discovery, since getPlugins/getTemplateTags live here. +registerUserPluginExportDiscovery(discoverUserPluginExportsForLoader); + const pickHookRequestFields = (req: Record): Record => { const out: Record = {}; for (const key of HOOK_REQUEST_FIELDS) { diff --git a/packages/insomnia/src/main/window-registry.ts b/packages/insomnia/src/main/window-registry.ts new file mode 100644 index 0000000000..c8a6ea04d5 --- /dev/null +++ b/packages/insomnia/src/main/window-registry.ts @@ -0,0 +1,11 @@ +import { type BrowserWindow } from 'electron'; + +// Holds the application's live BrowserWindow handles so modules that only need to read the main +// window (`plugin-window`, `prompt-bridge`, `templating-db-auth`) don't have to import +// `window-utils` — which, combined with `window-utils` driving the plugin window from this module, +// would otherwise form a circular import. +export const browserWindows = new Map<'Insomnia' | 'HiddenBrowserWindow', BrowserWindow>(); + +export function getMainWindow(): BrowserWindow | null { + return browserWindows.get('Insomnia') ?? null; +} diff --git a/packages/insomnia/src/main/window-utils.ts b/packages/insomnia/src/main/window-utils.ts index f72d3d4158..225970959e 100644 --- a/packages/insomnia/src/main/window-utils.ts +++ b/packages/insomnia/src/main/window-utils.ts @@ -30,16 +30,13 @@ import { ipcMainOn } from './ipc/electron'; import { getLogDirectory } from './log'; import { createPluginWindow, destroyPluginWindow, getPluginWindow } from './plugin-window'; import { isTrustedAppOrigin } from './trusted-origin'; +import { browserWindows } from './window-registry'; import { MAIN_WINDOW_SECURITY } from './window-security'; const DEFAULT_WIDTH = 1280; const DEFAULT_HEIGHT = 720; const MINIMUM_WIDTH = 500; const MINIMUM_HEIGHT = 400; -const browserWindows = new Map<'Insomnia' | 'HiddenBrowserWindow', ElectronBrowserWindow>(); -export function getMainWindow(): ElectronBrowserWindow | null { - return browserWindows.get('Insomnia') ?? null; -} let hiddenWindowIsBusy = false; // Every open window has its own copy of the menu, so keep them all in sync with the Debug Mode setting. diff --git a/packages/insomnia/src/plugins/__tests__/plugin-load-order-quickjs-module-resolution.test.ts b/packages/insomnia/src/plugins/__tests__/plugin-load-order-quickjs-module-resolution.test.ts index 696193f789..1f773e3e63 100644 --- a/packages/insomnia/src/plugins/__tests__/plugin-load-order-quickjs-module-resolution.test.ts +++ b/packages/insomnia/src/plugins/__tests__/plugin-load-order-quickjs-module-resolution.test.ts @@ -22,32 +22,39 @@ import os from 'node:os'; import path from 'node:path'; import { services } from 'insomnia-data'; -import { afterEach, describe, expect, it, vi } from 'vitest'; +import { afterEach, describe, expect, it } from 'vitest'; -vi.mock('~/main/templating-worker-database', () => ({ - discoverUserPluginExportsForLoader: async () => { - // Stands in for the real function's eventual `await import('../templating/sandbox/ - // plugin-tag-sandbox')` → `getQuickJSModule()` call chain, without needing to also mock - // `buildSandboxBridge`/`readPluginModuleMap`/`electron.app.getVersion()` for no added - // evidentiary value — the property under test is what `getQuickJSModule()` itself resolves to, - // not the rest of real discovery's plumbing. - const { getQuickJSModule } = await import('~/templating/sandbox/quickjs-runtime'); - (globalThis as any).__sandboxedPluginResolvedQuickJSModule = await getQuickJSModule(); - return { - templateTags: [], - requestHooks: 0, - responseHooks: 0, - requestActions: [], - requestGroupActions: [], - workspaceActions: [], - documentActions: [], - themes: [], - }; - }, -})); +// plugins/index.ts no longer dynamically imports `~/main/templating-worker-database`; the main +// process now hands it the sandbox-backed discovery via the shared `registerUserPluginExportDiscovery` +// registry. Mock that module would never register anything, so register a stub discovery through the +// real registration hook instead — standing in for the real function's eventual +// `await import('../templating/sandbox/plugin-tag-sandbox')` → `getQuickJSModule()` call chain, +// without needing to also mock `buildSandboxBridge`/`readPluginModuleMap`/`electron.app.getVersion()` +// for no added evidentiary value — the property under test is what `getQuickJSModule()` itself +// resolves to, not the rest of real discovery's plumbing. +import { registerUserPluginExportDiscovery } from '~/common/templating/user-plugin-export-discovery'; import { _testOnlySetPlugins, getPlugins } from '../index'; +registerUserPluginExportDiscovery(async () => { + // Deliberately dynamic: this test's premise is that the process's first-ever + // `quickjs-emscripten` resolution happens *during* the sandboxed sibling's discovery call, so a + // static import would defeat what's being proven. The assignment below avoids a cast; the test + // assertions read the flag back off `globalThis`. + const { getQuickJSModule } = await import('~/templating/sandbox/quickjs-runtime'); + Object.assign(globalThis, { __sandboxedPluginResolvedQuickJSModule: await getQuickJSModule() }); + return { + templateTags: [], + requestHooks: 0, + responseHooks: 0, + requestActions: [], + requestGroupActions: [], + workspaceActions: [], + documentActions: [], + themes: [], + }; +}); + const originalProcessType = process.type; const writePluginFolder = (baseDir: string, folderName: string, pluginName: string, indexJs: string) => { diff --git a/packages/insomnia/src/plugins/index.ts b/packages/insomnia/src/plugins/index.ts index 61f357fe99..477bca0df0 100644 --- a/packages/insomnia/src/plugins/index.ts +++ b/packages/insomnia/src/plugins/index.ts @@ -21,6 +21,7 @@ import type { } from '~/common/plugins/types'; import { fetchFromTemplateWorkerDatabase } from '~/common/templating/liquid-extension-worker'; import type { PluginTemplateTag, RenderPurpose } from '~/common/templating/types'; +import { getRegisteredUserPluginExportDiscovery } from '~/common/templating/user-plugin-export-discovery'; import type { ActionDescriptor, PluginExportManifest } from '~/templating/sandbox/marshal'; import { getAppBundlePlugins, isDevelopment } from '../common/constants'; @@ -63,7 +64,9 @@ export async function init() { // main process (via getTemplateTags in templating-worker-database) and the plugin window, so the // discovery has to reach the sandbox from either. In main we call the sandbox directly — the // `insomnia-templating-worker-database://` protocol is a renderer<->main channel and main's own -// `fetch` can't resolve it. In a renderer (the plugin window) we go over that protocol. +// `fetch` can't resolve it. In a renderer (the plugin window) we go over that protocol. In main the +// implementation is provided by `main/templating-worker-database`, registered through the shared +// `user-plugin-export-discovery` module — this file can't import that main module without a cycle. async function discoverUserPluginExports( directory: string, name: string, @@ -73,8 +76,7 @@ async function discoverUserPluginExports( if (__IS_RENDERER__) { return (await fetchFromTemplateWorkerDatabase('plugin.discoverUserPluginExports', body)) as PluginExportManifest; } - const { discoverUserPluginExportsForLoader } = await import('~/main/templating-worker-database'); - return discoverUserPluginExportsForLoader(body); + return getRegisteredUserPluginExportDiscovery()(body); } function buildUserPluginModuleFromManifest(pluginName: string, manifest: PluginExportManifest): Plugin['module'] { diff --git a/packages/insomnia/src/sync/git/repo-file-watcher.ts b/packages/insomnia/src/sync/git/repo-file-watcher.ts index 26ea515e79..599c258f41 100644 --- a/packages/insomnia/src/sync/git/repo-file-watcher.ts +++ b/packages/insomnia/src/sync/git/repo-file-watcher.ts @@ -44,8 +44,6 @@ import path from 'node:path'; import type { BaseModel, Workspace, WorkspaceMeta } from 'insomnia-data'; import { models, services } from 'insomnia-data'; -import type { WorkspaceFileIssue } from '~/main/git-service'; - import { database as db } from '../../common/database'; import { InsomniaFileTypeValues } from '../../common/import-v5-parser'; import { getInsomniaV5DataExport, tryImportV5Data } from '../../common/insomnia-v5'; @@ -70,6 +68,19 @@ export interface FileIssue { message: string; } +/** + * A file issue scoped to its workspace and git repository, as reported over IPC to the + * renderer. Lives here (not in `main/git-service`) to avoid a circular dependency: + * git-service loads this module for the watcher registry. + */ +export interface WorkspaceFileIssue { + workspaceId: string; + gitRepositoryId: string; + relPath: string; + kind: FileIssueKind; + message: string; +} + export interface FileProblemsChangedPayload { repoId: string; problems: FileIssue[]; diff --git a/packages/insomnia/src/ui/components/tabs/tab.tsx b/packages/insomnia/src/ui/components/tabs/tab.tsx index b5d667752a..908a259f9b 100644 --- a/packages/insomnia/src/ui/components/tabs/tab.tsx +++ b/packages/insomnia/src/ui/components/tabs/tab.tsx @@ -5,7 +5,7 @@ import React, { useCallback } from 'react'; import { Button, GridListItem } from 'react-aria-components'; import { scopeToBgColorMap, scopeToIconMap, scopeToTextColorMap } from '~/common/get-workspace-label'; -import type { WorkspaceFileIssue } from '~/main/git-service'; +import type { WorkspaceFileIssue } from '~/sync/git/repo-file-watcher'; import { scrollElementIntoView } from '~/ui/utils'; import { type BaseTab, type TabType, useInsomniaTabContext } from '../../context/app/insomnia-tab-context'; diff --git a/packages/insomnia/src/ui/hooks/use-git-file-issues.ts b/packages/insomnia/src/ui/hooks/use-git-file-issues.ts index edeef77496..cede3924a2 100644 --- a/packages/insomnia/src/ui/hooks/use-git-file-issues.ts +++ b/packages/insomnia/src/ui/hooks/use-git-file-issues.ts @@ -10,8 +10,7 @@ import React, { } from 'react'; import { invariant } from '~/common/utils/invariant'; -import type { WorkspaceFileIssue } from '~/main/git-service'; -import type { FileProblemsChangedPayload } from '~/sync/git/repo-file-watcher'; +import type { FileProblemsChangedPayload, WorkspaceFileIssue } from '~/sync/git/repo-file-watcher'; const mapIssuesByWorkspaceId = (issues: WorkspaceFileIssue[]) => { return Object.fromEntries(issues.map(issue => [issue.workspaceId, issue])) as Record; diff --git a/scripts/circular-references/known-violations.json b/scripts/circular-references/known-violations.json index 98b2d71838..8c000cdbff 100644 --- a/scripts/circular-references/known-violations.json +++ b/scripts/circular-references/known-violations.json @@ -2,11 +2,6 @@ "insomnia": [ "packages/insomnia/src/common/import.ts -> packages/insomnia/src/common/insomnia-v5.ts", "packages/insomnia/src/common/render.ts -> packages/insomnia/src/network/network.ts", - "packages/insomnia/src/main/git-service.ts -> packages/insomnia/src/sync/git/repo-file-watcher.ts", - "packages/insomnia/src/main/plugin-window.ts -> packages/insomnia/src/main/prompt-bridge.ts -> packages/insomnia/src/main/window-utils.ts", - "packages/insomnia/src/main/plugin-window.ts -> packages/insomnia/src/main/window-utils.ts", - "packages/insomnia/src/main/templating-worker-database.ts -> packages/insomnia/src/plugins/index.ts", - "packages/insomnia/src/main/templating-worker-database.ts -> packages/insomnia/src/templating/index.ts -> packages/insomnia/src/plugins/index.ts", "packages/insomnia/src/sync/git/providers/github.ts -> packages/insomnia/src/sync/git/utils.ts -> packages/insomnia/src/sync/git/providers/index.ts", "packages/insomnia/src/sync/git/providers/gitlab.ts -> packages/insomnia/src/sync/git/utils.ts -> packages/insomnia/src/sync/git/providers/index.ts", "packages/insomnia/src/ui/components/modals/export-requests-modal.tsx -> packages/insomnia/src/ui/components/settings/import-export.tsx",