The `UNRESOLVED_IMPORT` warnings that `quiet-config-warnings.mjs` suppressed no longer occur, so the filter (and the build-time `onwarn` suppressions for the same three deps) are removed. Root cause: Vite's config bundler (Rolldown) statically walks the node-only dev backend (`core/mcp/remote/node/server.ts`) when it loads `vite.config.ts`, reaching `chokidar`, `atomically`, and `@napi-rs/keyring`. `core/` has no node_modules of its own, so at config-bundle time those deps resolve from `core/`'s parent chain — the repo-root `node_modules`. Before #1452 the deps lived only under `clients/web/node_modules` (a sibling, off the resolution chain), so Rolldown emitted `UNRESOLVED_IMPORT`. #1452 declared them in the repo-root `package.json` (for the CLI/TUI/launcher, which consume core/'s node code), installing them at the repo root and making them resolvable. Verified by hiding the root copy: the warning returns; restore it and the dev/build startup is clean. A dynamic import of the backend does not help — Rolldown follows dynamic imports during config bundling too. The fix is the deps being resolvable, so the suppression is just removed and the invariant is documented in `vite.config.ts`. - Remove `clients/web/server/quiet-config-warnings.mjs` + `.d.mts` and its test - Restore the `dev` script to plain `vite` (drop the `--import` hook) - Remove the now-dead `build.rollupOptions.onwarn` UNRESOLVED suppressions (never fire: the Hono plugin is `apply: 'serve'`, out of the build graph) - Document why config-bundle resolution succeeds in `vite.config.ts` - Update AGENTS.md project tree Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Cliff Hall
GitHub
parent
dc31c28102
commit
a1f40a4222
6 changed files with 14 additions and 178 deletions
|
|
@ -16,9 +16,7 @@ inspector/
|
|||
│ │ │ # web-server-config.ts (env parsing + initial-config payload + banner),
|
||||
│ │ │ # sandbox-controller.ts (MCP Apps sandbox HTTP server),
|
||||
│ │ │ # inject-auth-token.ts (embeds the API token into served index.html),
|
||||
│ │ │ # vite-base-config.ts (shared optimizeDeps exclusions),
|
||||
│ │ │ # quiet-config-warnings.mjs (--import hook for `npm run dev`: drops benign
|
||||
│ │ │ # node-only UNRESOLVED_IMPORT warnings Rolldown prints at config load)
|
||||
│ │ │ # vite-base-config.ts (shared optimizeDeps exclusions)
|
||||
│ │ └── static/ # sandbox_proxy.html (served by sandbox-controller for MCP Apps tab)
|
||||
│ ├── cli/ # CLI client (tsup bundle, @inspector/core alias)
|
||||
│ ├── tui/ # TUI client (Ink + React, tsup bundle)
|
||||
|
|
|
|||
|
|
@ -11,7 +11,7 @@
|
|||
".": "./build/index.js"
|
||||
},
|
||||
"scripts": {
|
||||
"dev": "node --import ./server/quiet-config-warnings.mjs ./node_modules/vite/bin/vite.js",
|
||||
"dev": "vite",
|
||||
"build": "tsc -b && vite build && npm run build:runner",
|
||||
"build:runner": "tsup --config tsup.runner.config.ts",
|
||||
"lint": "eslint .",
|
||||
|
|
|
|||
|
|
@ -1,8 +0,0 @@
|
|||
/** Type declarations for the dev-only `quiet-config-warnings.mjs` hook so its
|
||||
* pure helpers can be imported (and unit-tested) from TypeScript. */
|
||||
|
||||
/** (package-quote, repo-relative-source-path) pairs of benign warnings to drop. */
|
||||
export declare const BENIGN_PAIRS: ReadonlyArray<readonly [string, string]>;
|
||||
|
||||
/** True when `text` is a benign node-only UNRESOLVED_IMPORT warning. */
|
||||
export declare function isBenignWarning(text: string): boolean;
|
||||
|
|
@ -1,66 +0,0 @@
|
|||
/**
|
||||
* Drops the benign "UNRESOLVED_IMPORT" warnings Vite's config bundler (Rolldown)
|
||||
* prints at `vite dev` startup for node-only backend deps that the Hono plugin
|
||||
* pulls in (`chokidar`, `atomically`, `@napi-rs/keyring`).
|
||||
*
|
||||
* Why a stream filter and not a Vite hook: the dev backend
|
||||
* (`core/mcp/remote/node/server.ts`) is statically imported by the Hono plugin,
|
||||
* so loading vite.config.ts bundles it and Rolldown walks its node-only deps.
|
||||
* Those deps are correctly externalized (the browser never loads them; Node
|
||||
* resolves them at runtime), but Rolldown still warns. The warnings are printed
|
||||
* directly to the process streams during config loading — before any Vite
|
||||
* logger exists — so `customLogger`, `logLevel`, and `rollupOptions.onwarn` all
|
||||
* miss them. Each warning (header + code frame) is emitted as a single write,
|
||||
* so we drop any write whose text carries the unresolved-import signature for
|
||||
* one of these known-benign (package, source-file) pairs. Installed via
|
||||
* `node --import` ahead of the Vite CLI; only affects the dev script.
|
||||
*
|
||||
* This matcher is tied to Rolldown's current warning format (the `'pkg'` quote
|
||||
* style and the repo-relative source path it prints). If these warnings ever
|
||||
* reappear, check whether Rolldown changed its message format. The failure mode
|
||||
* is safe: a format change just makes the filter a no-op (the warnings return),
|
||||
* it never drops anything else.
|
||||
*/
|
||||
|
||||
// (package, source-file) pairs. The file is a repo-relative path fragment — not
|
||||
// just a basename — so an unrelated future warning that happens to mention the
|
||||
// same dep and a same-named file isn't silently dropped.
|
||||
export const BENIGN_PAIRS = [
|
||||
["'chokidar'", "core/mcp/remote/node/server.ts"],
|
||||
["'atomically'", "core/storage/store-io.ts"],
|
||||
["'@napi-rs/keyring'", "core/auth/node/secret-store.ts"],
|
||||
];
|
||||
|
||||
export function isBenignWarning(text) {
|
||||
if (!text.includes("UNRESOLVED_IMPORT")) return false;
|
||||
return BENIGN_PAIRS.some(
|
||||
([dep, file]) => text.includes(dep) && text.includes(file),
|
||||
);
|
||||
}
|
||||
|
||||
function patch(stream) {
|
||||
const original = stream.write.bind(stream);
|
||||
stream.write = (chunk, encoding, callback) => {
|
||||
const text =
|
||||
typeof chunk === "string"
|
||||
? chunk
|
||||
: Buffer.isBuffer(chunk)
|
||||
? chunk.toString("utf8")
|
||||
: "";
|
||||
if (text && isBenignWarning(text)) {
|
||||
// Pretend the write succeeded so callers/promises don't stall.
|
||||
if (typeof encoding === "function") encoding();
|
||||
else if (typeof callback === "function") callback();
|
||||
return true;
|
||||
}
|
||||
return original(chunk, encoding, callback);
|
||||
};
|
||||
}
|
||||
|
||||
// Guard so importing this module (e.g. from a unit test for isBenignWarning)
|
||||
// doesn't monkey-patch the test runner's streams. Under `npm run dev` VITEST is
|
||||
// unset, so the patch installs as intended.
|
||||
if (!process.env.VITEST) {
|
||||
patch(process.stdout);
|
||||
patch(process.stderr);
|
||||
}
|
||||
|
|
@ -1,54 +0,0 @@
|
|||
import { describe, it, expect } from "vitest";
|
||||
import {
|
||||
isBenignWarning,
|
||||
BENIGN_PAIRS,
|
||||
} from "../../../server/quiet-config-warnings.mjs";
|
||||
|
||||
// A representative Rolldown config-load warning block (header + code frame), as
|
||||
// captured from `vite dev` startup.
|
||||
function warning(dep: string, file: string): string {
|
||||
return (
|
||||
`${file} (8:36) [UNRESOLVED_IMPORT] Warning: Could not resolve ${dep} in ${file}\n` +
|
||||
` ╭─[ ${file}:8:37 ]\n` +
|
||||
` 8 │ import { readFile } from ${dep};\n` +
|
||||
` ╰─ Module not found, treating it as an external dependency\n`
|
||||
);
|
||||
}
|
||||
|
||||
describe("quiet-config-warnings / isBenignWarning", () => {
|
||||
it("matches each benign node-only dep warning", () => {
|
||||
for (const [dep, file] of BENIGN_PAIRS) {
|
||||
expect(isBenignWarning(warning(dep, file))).toBe(true);
|
||||
}
|
||||
});
|
||||
|
||||
it("ignores non-UNRESOLVED output", () => {
|
||||
expect(isBenignWarning("VITE v8.0.0 ready in 603 ms")).toBe(false);
|
||||
expect(
|
||||
isBenignWarning("Sandbox (MCP Apps): http://localhost:61146/sandbox"),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("does not drop an UNRESOLVED_IMPORT for a different (real) dependency", () => {
|
||||
expect(
|
||||
isBenignWarning(
|
||||
"src/App.tsx (1:0) [UNRESOLVED_IMPORT] Warning: Could not resolve 'some-real-pkg' in src/App.tsx",
|
||||
),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("requires the matching source path, not just the package name", () => {
|
||||
// Same dep, but in an unrelated file → not silenced (guards over-matching).
|
||||
expect(
|
||||
isBenignWarning(
|
||||
"src/elsewhere.ts (1:0) [UNRESOLVED_IMPORT] Warning: Could not resolve 'chokidar' in src/elsewhere.ts",
|
||||
),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("uses repo-relative path fragments (not bare basenames)", () => {
|
||||
for (const [, file] of BENIGN_PAIRS) {
|
||||
expect(file).toContain("/");
|
||||
}
|
||||
});
|
||||
});
|
||||
|
|
@ -32,58 +32,24 @@ export default defineConfig({
|
|||
// `honoMiddlewarePlugin` is gated by `apply: 'serve'` so it only attaches
|
||||
// during `vite dev` / `vite preview` — vitest projects share this config
|
||||
// but never invoke `configureServer`, so the plugin stays inert there.
|
||||
//
|
||||
// The plugin statically imports the node-only dev backend
|
||||
// (`core/mcp/remote/node/server.ts`), so Vite's config bundler (Rolldown)
|
||||
// walks that chain when it loads this file and reaches node-only deps
|
||||
// (`chokidar`, `atomically`, `@napi-rs/keyring`). Those resolve cleanly at
|
||||
// config-bundle time because they're declared in the repo-root
|
||||
// `package.json` and installed into the repo-root `node_modules`, which sits
|
||||
// on `core/`'s module-resolution chain (core/ has no node_modules of its
|
||||
// own). Keep them in the root manifest: drop one and Rolldown can no longer
|
||||
// resolve it from core/, reviving the benign `UNRESOLVED_IMPORT` warnings
|
||||
// that #1491 eliminated at the source (by removing the old stream filter and
|
||||
// build-time onwarn suppressions rather than re-hiding the symptom).
|
||||
plugins: [react(), honoMiddlewarePlugin(buildWebServerConfigFromEnv())],
|
||||
// Shared optimizeDeps exclusions so node-only packages
|
||||
// (`@modelcontextprotocol/sdk/client/stdio.js`, `cross-spawn`, `which`)
|
||||
// consumed by the dev backend aren't scanned for browser pre-bundling.
|
||||
// Browser code reaches the node-side stack via the Hono plugin only.
|
||||
...getViteBaseConfig(),
|
||||
build: {
|
||||
rollupOptions: {
|
||||
// Loading vite.config.ts pulls `core/mcp/remote/node/server.ts` (via the
|
||||
// Hono plugin) into Rollup's module cache. That chain reaches
|
||||
// `core/storage/store-io.ts`, which imports node-only `atomically`. The
|
||||
// module never lands in the browser bundle (it's unreachable from
|
||||
// `index.html`), but Rollup's scanner still warns about the unresolved
|
||||
// import. Silence it here so the build log stays clean.
|
||||
onwarn(warning, defaultHandler) {
|
||||
if (
|
||||
warning.code === 'UNRESOLVED_IMPORT' &&
|
||||
typeof warning.message === 'string' &&
|
||||
warning.message.includes("'atomically'") &&
|
||||
warning.message.includes('store-io.ts')
|
||||
) {
|
||||
return;
|
||||
}
|
||||
// Same story for `chokidar`: the mcp.json file watcher in
|
||||
// `core/mcp/remote/node/server.ts` is reachable only through the dev
|
||||
// backend, never from the browser bundle. Rollup still scans the
|
||||
// import statement and warns when it can't resolve it.
|
||||
if (
|
||||
warning.code === 'UNRESOLVED_IMPORT' &&
|
||||
typeof warning.message === 'string' &&
|
||||
warning.message.includes("'chokidar'") &&
|
||||
warning.message.includes('server.ts')
|
||||
) {
|
||||
return;
|
||||
}
|
||||
// `@napi-rs/keyring` is the native-binding keychain backend
|
||||
// consumed by `core/auth/node/secret-store.ts` from the Hono
|
||||
// `/api/servers` handlers. Same reasoning as the entries above:
|
||||
// unreachable from the browser bundle, so the unresolved-import
|
||||
// warning is noise.
|
||||
if (
|
||||
warning.code === 'UNRESOLVED_IMPORT' &&
|
||||
typeof warning.message === 'string' &&
|
||||
warning.message.includes("'@napi-rs/keyring'") &&
|
||||
warning.message.includes('secret-store.ts')
|
||||
) {
|
||||
return;
|
||||
}
|
||||
defaultHandler(warning);
|
||||
},
|
||||
},
|
||||
},
|
||||
resolve: {
|
||||
// NOTE: the unit vitest project (below) overrides this — see comment there.
|
||||
//
|
||||
|
|
|
|||
Loading…
Reference in a new issue