diff --git a/apps/desktop/scripts/assert-root-install.mjs b/apps/desktop/scripts/assert-root-install.mjs index bb1daa2dbf..c3d388eabd 100644 --- a/apps/desktop/scripts/assert-root-install.mjs +++ b/apps/desktop/scripts/assert-root-install.mjs @@ -31,13 +31,25 @@ import { isMain } from "./utils.mjs" // others, which is how an incomplete install reached `vite build` and died on // an unresolved `katex/dist/katex.min.css` with no hint that the install — not // the source — was at fault (#86443). +// +// These four are the documented floor — always checked, even when the app's +// package.json cannot be read. The full class is wider: EVERY non-optional +// package the workspace manifest declares is something the build may import +// (`vite.config.ts` pulls `@rolldown/plugin-babel`, `@vitejs/plugin-react`, +// `@tailwindcss/vite`; `bundle-electron-main.mjs` pulls `esbuild`; the renderer +// imports the rest). A hand-maintained list drifts the moment a new import +// lands, so `checkRootInstall` unions the floor with the manifest's declared +// `dependencies` + `devDependencies` — a partial install is refused whichever +// package it happened to drop. `optionalDependencies` are excluded by design: +// npm legitimately skips them (platform-gated natives like `get-windows`). const BUILD_CRITICAL_PACKAGES = ["vite", "katex", "electron", "electron-builder"] export { BUILD_CRITICAL_PACKAGES } // Resolve the way Node's own lookup does — walk `node_modules` upward — rather // than through `require.resolve`. A package whose `exports` map does not expose // `./package.json` is not resolvable by path even when correctly installed, and -// that must not read as "missing". +// that must not read as "missing". Scoped names (`@scope/name`) are a nested +// directory under `node_modules`, which `join` handles. function packageIsInstalled(name, fromDir) { let dir = fromDir for (;;) { @@ -48,10 +60,27 @@ function packageIsInstalled(name, fromDir) { } } +// Every package the workspace manifest at `appDir` declares as required +// (`dependencies` + `devDependencies`; never `optionalDependencies`). An +// unreadable or malformed manifest yields [] — the floor still applies, and +// the build's own manifest read fails loudly on its own. +export function requiredPackages(appDir) { + try { + const manifest = JSON.parse(readFileSync(join(appDir, "package.json"), "utf8")) + return [ + ...Object.keys(manifest.dependencies ?? {}), + ...Object.keys(manifest.devDependencies ?? {}), + ] + } catch { + return [] + } +} + // Pure check — returns { ok: true } or { ok: false, error: "..." }. // Kept side-effect-free so it can be unit tested without spawning a process. export function checkRootInstall(appDir, rootDir) { - const missing = BUILD_CRITICAL_PACKAGES.filter(pkg => !packageIsInstalled(pkg, appDir)) + const wanted = [...new Set([...BUILD_CRITICAL_PACKAGES, ...requiredPackages(appDir)])] + const missing = wanted.filter(pkg => !packageIsInstalled(pkg, appDir)) if (missing.length > 0) { return { ok: false, diff --git a/apps/desktop/scripts/assert-root-install.test.mjs b/apps/desktop/scripts/assert-root-install.test.mjs index 4f1403b80c..0d7ea4af6f 100644 --- a/apps/desktop/scripts/assert-root-install.test.mjs +++ b/apps/desktop/scripts/assert-root-install.test.mjs @@ -4,15 +4,17 @@ import os from 'node:os' import path from 'node:path' import { test } from 'vitest' -import { BUILD_CRITICAL_PACKAGES as BUILD_CRITICAL, checkRootInstall } from '../scripts/assert-root-install.mjs' +import { BUILD_CRITICAL_PACKAGES as BUILD_CRITICAL, checkRootInstall, requiredPackages } from '../scripts/assert-root-install.mjs' // Build a throwaway repo shaped like this one: an app workspace whose // dependencies are hoisted to the repo root, which is what the guard walks. -function makeTree({ rootPackages = BUILD_CRITICAL, react = '19.2.7', reactDom = '19.2.7' } = {}) { +// `manifest` is merged into the app's package.json so tests can declare +// dependencies the guard is expected to read. +function makeTree({ rootPackages = BUILD_CRITICAL, react = '19.2.7', reactDom = '19.2.7', manifest = {} } = {}) { const tempRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'hermes-assert-root-')) const appDir = path.join(tempRoot, 'apps', 'desktop') fs.mkdirSync(appDir, { recursive: true }) - fs.writeFileSync(path.join(appDir, 'package.json'), JSON.stringify({ name: 'desktop' }), 'utf8') + fs.writeFileSync(path.join(appDir, 'package.json'), JSON.stringify({ name: 'desktop', ...manifest }), 'utf8') const writePackage = (name, version) => { const dir = path.join(tempRoot, 'node_modules', name) @@ -120,3 +122,76 @@ test('checkRootInstall accepts a package nested in the app workspace', () => { fs.rmSync(tempRoot, { recursive: true, force: true }) } }) + +// The class, not the four instances: the floor list is what a partial install +// has been *seen* to drop, but any declared non-optional package can be the one +// missing next (`vite.config.ts` imports `@rolldown/plugin-babel`, which the +// floor never named). The guard must read the manifest so the list cannot drift +// behind a new import. +test('checkRootInstall fails when a declared devDependency outside the floor is missing', () => { + const { tempRoot, appDir } = makeTree({ + manifest: { devDependencies: { '@rolldown/plugin-babel': '1.0.0', esbuild: '1.0.0' } }, + rootPackages: [...BUILD_CRITICAL, 'esbuild'] + }) + try { + const result = checkRootInstall(appDir, tempRoot) + assert.equal(result.ok, false) + assert.match(result.error, /@rolldown\/plugin-babel/) + assert.doesNotMatch(result.error, /esbuild/) + } finally { + fs.rmSync(tempRoot, { recursive: true, force: true }) + } +}) + +test('checkRootInstall fails when a declared runtime dependency is missing', () => { + const { tempRoot, appDir } = makeTree({ + manifest: { dependencies: { '@vscode/codicons': '1.0.0' } } + }) + try { + const result = checkRootInstall(appDir, tempRoot) + assert.equal(result.ok, false) + assert.match(result.error, /@vscode\/codicons/) + } finally { + fs.rmSync(tempRoot, { recursive: true, force: true }) + } +}) + +// npm skips optionalDependencies legitimately (platform-gated natives), so an +// absent optional package is not a partial install. +test('checkRootInstall ignores missing optionalDependencies', () => { + const { tempRoot, appDir } = makeTree({ + manifest: { optionalDependencies: { 'get-windows': '9.3.0' } } + }) + try { + assert.deepEqual(checkRootInstall(appDir, tempRoot), { ok: true }) + } finally { + fs.rmSync(tempRoot, { recursive: true, force: true }) + } +}) + +test('checkRootInstall passes when every declared package is installed', () => { + const { tempRoot, appDir } = makeTree({ + manifest: { dependencies: { '@scope/pkg': '1.0.0' }, devDependencies: { esbuild: '1.0.0' } }, + rootPackages: [...BUILD_CRITICAL, '@scope/pkg', 'esbuild'] + }) + try { + assert.deepEqual(checkRootInstall(appDir, tempRoot), { ok: true }) + } finally { + fs.rmSync(tempRoot, { recursive: true, force: true }) + } +}) + +// The floor is unconditional: a manifest the guard cannot parse must not turn +// the check off. +test('checkRootInstall keeps the floor when the manifest is unreadable', () => { + const { tempRoot, appDir } = makeTree({ rootPackages: ['vite'] }) + fs.writeFileSync(path.join(appDir, 'package.json'), '{not json', 'utf8') + try { + assert.deepEqual(requiredPackages(appDir), []) + const result = checkRootInstall(appDir, tempRoot) + assert.equal(result.ok, false) + assert.match(result.error, /katex/) + } finally { + fs.rmSync(tempRoot, { recursive: true, force: true }) + } +})