diff --git a/config/runtime-electron-baseline.txt b/config/runtime-electron-baseline.txt index f392f70ecc9..a308ec4b62c 100644 --- a/config/runtime-electron-baseline.txt +++ b/config/runtime-electron-baseline.txt @@ -1,6 +1,6 @@ -# Modules reachable from the Orca runtime that import `electron`. +# Modules reachable from the Orca runtime or structured-chat code that import `electron`. # Generated by config/scripts/check-runtime-electron-ratchet.mjs. -# This list is EMPTY and must stay that way: the runtime boots on plain Node -# (see `pnpm run build:orcad`). Any entry means the runtime got less portable; +# This list is EMPTY and must stay that way: both run in orcad, the headless runtime, +# on plain Node (see `pnpm run build:orcad`). Any entry means that code got less portable; # migrate the module behind a host port instead (src/main/host/). diff --git a/config/scripts/audit-localization-coverage.mjs b/config/scripts/audit-localization-coverage.mjs index c5f27a66422..c48770d9a88 100644 --- a/config/scripts/audit-localization-coverage.mjs +++ b/config/scripts/audit-localization-coverage.mjs @@ -5,11 +5,9 @@ import process from 'node:process' // TypeScript 7 is a native CLI; AST consumers still need the legacy JavaScript API. import ts from 'typescript-api' +import { isTestOnlySourcePath } from './test-only-source-path.mjs' const SOURCE_EXTENSIONS = new Set(['.ts', '.tsx', '.js', '.jsx', '.mts', '.cts']) -// Why: test-only modules live beside their spec as `*-test-harness.ts` / `*-test-rig.ts` / `*-fixtures.ts` here, not under `__tests__/`. -const TEST_SUPPORT_FILE_PATTERN = - /[.-](?:test-harness|test-rig|test-fixtures?|test-state|test-support|fixtures?)\.[cm]?[jt]sx?$/ const SKIP_PATH_PARTS = new Set(['.git', 'dist', 'node_modules', 'out', '__snapshots__', 'assets']) const LOCALIZATION_CALL_NAMES = new Set(['t', 'translate']) const USER_VISIBLE_JSX_ATTRIBUTES = new Set([ @@ -78,13 +76,7 @@ function normalizePath(root, filePath) { export function isSkippedFile(root, filePath) { const relative = normalizePath(root, filePath) - if ( - relative.endsWith('.d.ts') || - relative.includes('.test.') || - relative.includes('.spec.') || - relative.includes('/__tests__/') || - TEST_SUPPORT_FILE_PATTERN.test(relative) - ) { + if (relative.endsWith('.d.ts') || isTestOnlySourcePath(relative)) { return true } return relative.split('/').some((part) => SKIP_PATH_PARTS.has(part)) diff --git a/config/scripts/check-runtime-electron-ratchet.mjs b/config/scripts/check-runtime-electron-ratchet.mjs index 7c165bfc02b..19810c2f402 100644 --- a/config/scripts/check-runtime-electron-ratchet.mjs +++ b/config/scripts/check-runtime-electron-ratchet.mjs @@ -1,14 +1,11 @@ #!/usr/bin/env node /** - * Ratchet gate for Electron imports reachable from the Orca runtime. + * Ratchet gate for Electron imports reachable from the Orca runtime and structured chat. * - * The runtime is meant to become host-agnostic so it can also run on plain Node - * (see docs/design/node-only-runtime-backend.html). Nothing enforces that today: - * `orca-runtime.ts` reaches ~50 modules that import `electron`, and the number - * silently grows whenever someone adds an import several hops away, because no - * single reviewer sees the transitive edge. + * The runtime boots on plain Node, where Electron is unavailable. Keep desktop + * dependencies out of its graph, including structured chat not yet wired into it. * - * This bundles the runtime with esbuild, reads the metafile for every module that + * This bundles the runtime and the structured-chat lanes with esbuild, reads the metafile for every module that * imports `electron`, and compares that set to a checked-in baseline. A NEW module * fails the build; a removed one must be dropped from the baseline. The baseline * may only shrink, so the migration is measurable and cannot regress. @@ -19,10 +16,11 @@ * Usage: node config/scripts/check-runtime-electron-ratchet.mjs [--write] */ import { build } from 'esbuild' -import { readFileSync, writeFileSync } from 'node:fs' +import { existsSync, readdirSync, readFileSync, writeFileSync } from 'node:fs' import path from 'node:path' import { pathToFileURL } from 'node:url' import process from 'node:process' +import { isTestOnlyDirectoryName, isTestOnlySourcePath } from './test-only-source-path.mjs' // Why absolute, not cwd-relative: `pnpm lint` runs from the repo root but CI steps and // editors do not always, and a cwd-relative miss surfaced as an unhandled ENOENT stack @@ -41,6 +39,56 @@ const ENTRY_POINTS = [ path.join(ROOT, 'src', 'main', 'orcad', 'main.ts') ] +// Code that must run in orcad whether or not a runtime entry reaches it yet. Whole directories, +// so a new file is covered by default; src/main/runtime still holds desktop-only code (browser +// commands, desktop relay), so only its structured-chat files are entries there. +export const STRUCTURED_CHAT_LANES = [ + { directory: ['src', 'main', 'native-chat'] }, + { directory: ['src', 'main', 'claude'] }, + { directory: ['src', 'main', 'codex'] }, + { directory: ['src', 'shared'] }, + { directory: ['src', 'main', 'runtime'], basename: /^(?:structured-|agent-session-)/ }, + // Allowed absent until they land; every other lane throws if missing, so a rename can't empty it. + { directory: ['src', 'main', 'acp'], mayBeAbsent: true }, + { directory: ['src', 'main', 'provider-process'], mayBeAbsent: true } +] + +export function collectStructuredChatEntryPoints(root = ROOT) { + return STRUCTURED_CHAT_LANES.flatMap((lane) => { + const directory = path.join(root, ...lane.directory) + if (!existsSync(directory)) { + if (lane.mayBeAbsent) { + return [] + } + throw new Error( + `[runtime-electron-ratchet] ${lane.directory.join('/')} is missing. If it moved, update STRUCTURED_CHAT_LANES; otherwise the gate would silently check nothing there.` + ) + } + return collectLaneFiles(directory, lane.basename) + }).sort() +} + +function collectLaneFiles(directory, basename) { + return readdirSync(directory, { withFileTypes: true }).flatMap((entry) => { + const file = path.join(directory, entry.name) + if (entry.isDirectory()) { + return isTestOnlyDirectoryName(entry.name) ? [] : collectLaneFiles(file, basename) + } + return entry.isFile() && + /\.[cm]?[jt]sx?$/.test(entry.name) && + !entry.name.endsWith('.d.ts') && + !isTestOnlySourcePath(entry.name) && + (!basename || basename.test(entry.name)) + ? [file] + : [] + }) +} + +/** What the CLI and CI check: the runtime graph plus every structured-chat lane file. */ +export function defaultEntryPoints(root = ROOT) { + return [...ENTRY_POINTS, ...collectStructuredChatEntryPoints(root)] +} + // Native addons and electron cannot be bundled; externalising them is what the // relay build already does (config/scripts/build-relay.mjs). const EXTERNAL = [ @@ -66,7 +114,8 @@ const externalNativeAddons = { } } -export async function collectElectronImporters(entryPoints = ENTRY_POINTS) { +// Why `plugins`: lets a test add an Electron import to a real file in memory, never on disk. +export async function collectElectronImporters(entryPoints, { plugins = [] } = {}) { const result = await build({ entryPoints, bundle: true, @@ -81,7 +130,7 @@ export async function collectElectronImporters(entryPoints = ENTRY_POINTS) { metafile: true, absWorkingDir: ROOT, logLevel: 'silent', - plugins: [externalNativeAddons] + plugins: [externalNativeAddons, ...plugins] }) const importers = new Set() for (const [file, info] of Object.entries(result.metafile.inputs)) { @@ -114,24 +163,25 @@ export function diffAgainstBaseline(current, baseline) { function renderBaseline(files) { return [ - '# Modules reachable from the Orca runtime that import `electron`.', + '# Modules reachable from the Orca runtime or structured-chat code that import `electron`.', '# Generated by config/scripts/check-runtime-electron-ratchet.mjs.', - '# This list is EMPTY and must stay that way: the runtime boots on plain Node', - '# (see `pnpm run build:orcad`). Any entry means the runtime got less portable;', + '# This list is EMPTY and must stay that way: both run in orcad, the headless runtime,', + '# on plain Node (see `pnpm run build:orcad`). Any entry means that code got less portable;', '# migrate the module behind a host port instead (src/main/host/).', '', ...files ].join('\n') } -async function main() { - const write = process.argv.includes('--write') - const current = await collectElectronImporters() +// Exported so tests run the CLI path itself; `plugins` is the same in-memory hook as above. +export async function main(argv = process.argv, { plugins = [] } = {}) { + const write = argv.includes('--write') + const current = await collectElectronImporters(defaultEntryPoints(), { plugins }) if (write) { writeFileSync(BASELINE_PATH, `${renderBaseline(current)}\n`) console.log(`[runtime-electron-ratchet] wrote ${current.length} entries to ${BASELINE_PATH}`) - return + return 0 } const baseline = readBaseline(readFileSync(BASELINE_PATH, 'utf8')) @@ -139,15 +189,15 @@ async function main() { if (added.length > 0) { console.error( - `[runtime-electron-ratchet] ${added.length} new module(s) reachable from the Orca runtime now import electron: + `[runtime-electron-ratchet] ${added.length} new module(s) reachable from the Orca runtime or structured-chat code now import electron: ${added.map((file) => ` + ${file}`).join('\n')} -The runtime must stay bootable on plain Node. Put the Electron facility behind a port in -src/main/host/ and depend on the port, or move the code out of the runtime's import graph. -See docs/design/node-only-runtime-backend.html.` +The runtime and structured chat must run in orcad, the headless runtime, where Electron is +unavailable. That holds for structured-chat code the runtime doesn't load yet, so renaming or +moving the file is not a fix. Put the Electron facility behind a port in src/main/host/ and +depend on the port, or drop the import that pulls Electron in.` ) - process.exitCode = 1 - return + return 1 } if (removed.length > 0) { @@ -158,11 +208,11 @@ ${removed.map((file) => ` - ${file}`).join('\n')} node config/scripts/check-runtime-electron-ratchet.mjs --write` ) - process.exitCode = 1 - return + return 1 } console.log(`[runtime-electron-ratchet] ok — ${current.length} entries, unchanged.`) + return 0 } // Why pathToFileURL and not a `file://` template: on Windows process.argv[1] is a @@ -170,5 +220,5 @@ ${removed.map((file) => ` - ${file}`).join('\n')} // template never matches and the gate would exit 0 without checking anything — a // lint gate that fails open. Same idiom as check-max-lines-ratchet.mjs:225. if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) { - await main() + process.exitCode = await main() } diff --git a/config/scripts/check-runtime-electron-ratchet.test.mjs b/config/scripts/check-runtime-electron-ratchet.test.mjs index e993ca32fc5..1f4f37c01a9 100644 --- a/config/scripts/check-runtime-electron-ratchet.test.mjs +++ b/config/scripts/check-runtime-electron-ratchet.test.mjs @@ -1,11 +1,181 @@ -import { describe, expect, it } from 'vitest' -import { readFileSync } from 'node:fs' +import { afterEach, describe, expect, it, vi } from 'vitest' +import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import path from 'node:path' +import process from 'node:process' import { collectElectronImporters, + collectStructuredChatEntryPoints, + defaultEntryPoints, diffAgainstBaseline, - readBaseline + main, + readBaseline, + STRUCTURED_CHAT_LANES } from './check-runtime-electron-ratchet.mjs' +describe('structured chat coverage', () => { + const roots = [] + afterEach(() => { + for (const root of roots.splice(0)) { + rmSync(root, { recursive: true, force: true }) + } + }) + + function fixture(files) { + const root = mkdtempSync(path.join(tmpdir(), 'orca-electron-ratchet-')) + roots.push(root) + for (const [file, source] of Object.entries(files)) { + const absolute = path.join(root, file) + mkdirSync(path.dirname(absolute), { recursive: true }) + writeFileSync(absolute, source) + } + return root + } + + // Every lane that must exist; acp/ and provider-process/ may be absent until they land. + const requiredLanes = { + 'src/main/native-chat/reader.ts': 'export {}', + 'src/main/claude/claude-session.ts': 'export {}', + 'src/main/codex/codex-session.ts': 'export {}', + 'src/main/runtime/structured-agent-session-host.ts': 'export {}', + 'src/shared/agent-session-record.ts': 'export {}' + } + + it('covers whole lane directories, structured runtime files at any depth, and no test code', () => { + const sources = [ + ...Object.keys(requiredLanes), + 'src/main/native-chat/nested/reader.ts', + 'src/main/native-chat/worker.mjs', + 'src/main/claude/other.ts', + 'src/main/codex/codex-provider-timeline-identity.ts', + 'src/shared/nested/agent-session-account-home.ts', + 'src/shared/relay-runtime-self-test-report.ts', + 'src/main/runtime/agent-session-record.ts', + 'src/main/runtime/structured-agent-runtime-registrations.ts', + 'src/main/runtime/rpc/methods/structured-agent-session-agents.ts', + 'src/main/acp/adapter.ts', + 'src/main/provider-process/worker.ts' + ] + const excluded = [ + 'src/main/native-chat/reader.test.ts', + 'src/main/native-chat/reader.spec.ts', + 'src/main/native-chat/reader-test-support.ts', + 'src/main/native-chat/reader.test-support.ts', + 'src/main/native-chat/structured-agent-session-rest-test-rig.ts', + 'src/main/native-chat/structured-agent-session-host-test-data.ts', + 'src/main/native-chat/reader.test-fixture.ts', + 'src/main/native-chat/reader-fixtures.ts', + 'src/main/codex/codex-turn-lifecycle-fake.ts', + 'src/main/codex/codex-session-backfill-fs-mocks.ts', + 'src/main/native-chat/__fixtures__/reader.ts', + 'src/main/native-chat/test-support/reader.ts', + 'src/main/runtime/orca-runtime-tests/structured-agent-session-host.ts', + 'src/shared/types.d.ts', + 'src/main/runtime/other.ts', + 'src/main/runtime/rpc/methods/browser.ts' + ] + const root = fixture( + Object.fromEntries([...sources, ...excluded].map((file) => [file, 'export {}'])) + ) + expect(collectStructuredChatEntryPoints(root)).toEqual( + sources.map((file) => path.join(root, ...file.split('/'))).sort() + ) + }) + + it.each(Object.keys(requiredLanes).map((file) => path.posix.dirname(file)))( + 'fails loudly when %s goes missing, so a rename cannot empty it', + (lane) => { + const without = Object.fromEntries( + Object.entries(requiredLanes).filter(([file]) => !file.startsWith(`${lane}/`)) + ) + expect(() => collectStructuredChatEntryPoints(fixture(without))).toThrow(`${lane} is missing`) + expect(collectStructuredChatEntryPoints(fixture(requiredLanes))).toHaveLength(5) + } + ) + + it('finds Electron through a package imported by each unwired future lane', async () => { + const root = fixture({ + ...requiredLanes, + 'src/main/acp/adapter.ts': "import 'acp-desktop-package'", + 'src/main/provider-process/worker.ts': "import 'provider-desktop-package'", + 'node_modules/acp-desktop-package/package.json': '{"main":"index.js"}', + 'node_modules/acp-desktop-package/index.js': "require('electron')", + 'node_modules/provider-desktop-package/package.json': '{"main":"index.js"}', + 'node_modules/provider-desktop-package/index.js': "require('electron')" + }) + const current = await collectElectronImporters(collectStructuredChatEntryPoints(root)) + expect(current.map((file) => file.split('/node_modules/').pop())).toEqual([ + 'acp-desktop-package/index.js', + 'provider-desktop-package/index.js' + ]) + }) +}) + +describe('the default entry points', () => { + const lanes = ['native-chat', 'claude', 'codex', 'runtime'].map((lane) => `src/main/${lane}/`) + + it('are the runtime entries plus a file from every lane that exists', () => { + const entries = defaultEntryPoints().map((file) => + path.relative(process.cwd(), file).split(path.sep).join('/') + ) + expect(entries.slice(0, 3)).toEqual([ + 'src/main/runtime/orca-runtime.ts', + 'src/main/runtime/runtime-rpc.ts', + 'src/main/orcad/main.ts' + ]) + for (const lane of [...lanes, 'src/shared/']) { + expect(entries.some((file) => file.startsWith(lane))).toBe(true) + } + }) + + // Retires the temporary flag: the PR that adds acp/ or provider-process/ must make it required. + it('lets only directories that have not landed yet be absent', () => { + for (const lane of STRUCTURED_CHAT_LANES.filter((candidate) => candidate.mayBeAbsent)) { + expect( + existsSync(path.join(process.cwd(), ...lane.directory)), + lane.directory.join('/') + ).toBe(false) + } + }) +}) + +// Why `main`: it is what `pnpm lint` and CI run, so these fail if its entry list drops the lanes. +describe('the command-line check', () => { + afterEach(() => { + vi.restoreAllMocks() + }) + + // Why a file the runtime doesn't load: only the structured-chat lanes can catch it. + it('fails on Electron in structured-chat code the runtime graph does not reach', async () => { + const target = path.join( + process.cwd(), + 'src', + 'main', + 'native-chat', + 'transcript-read-cache.ts' + ) + const addElectron = { + name: 'add-electron-import', + setup(pluginBuild) { + pluginBuild.onLoad({ filter: /transcript-read-cache\.ts$/ }, (args) => + args.path === target + ? { contents: `import 'electron'\n${readFileSync(target, 'utf8')}`, loader: 'ts' } + : undefined + ) + } + } + const error = vi.spyOn(console, 'error').mockImplementation(() => {}) + expect(await main([], { plugins: [addElectron] })).toBe(1) + expect(error.mock.calls.join('\n')).toContain('+ src/main/native-chat/transcript-read-cache.ts') + }, 120_000) + + // Why real: the value of this gate is the transitive edges, which a fixture cannot model. + it('passes on the tree as it is, matching the checked-in baseline', async () => { + vi.spyOn(console, 'log').mockImplementation(() => {}) + expect(await main([])).toBe(0) + }, 120_000) +}) + describe('readBaseline', () => { it('drops comments and blank lines and sorts, so baseline formatting cannot cause a false diff', () => { expect(readBaseline('# header\n\n b/second.ts \na/first.ts\n')).toEqual([ @@ -36,15 +206,6 @@ describe('diffAgainstBaseline', () => { }) describe('the checked-in baseline', () => { - // Why real: the value of this gate is the transitive edges, which a fixture cannot model. - // If this is slow enough to hurt, it is still cheaper than shipping a runtime that - // cannot boot on Node. - it('matches what the runtime actually reaches today', async () => { - const current = await collectElectronImporters() - const baseline = readBaseline(readFileSync('config/runtime-electron-baseline.txt', 'utf8')) - expect(diffAgainstBaseline(current, baseline)).toEqual({ added: [], removed: [] }) - }, 120_000) - // Why an exact-empty assertion now: the reachable set reached zero, so "may only // shrink" has no room left and any entry at all is a regression. This is strictly // stronger than the old under-src/ check, which only stopped a node_modules path from diff --git a/config/scripts/test-only-source-path.mjs b/config/scripts/test-only-source-path.mjs new file mode 100644 index 00000000000..c0fd74bc130 --- /dev/null +++ b/config/scripts/test-only-source-path.mjs @@ -0,0 +1,23 @@ +/** + * Whether a repo source path is test-only by this repo's naming conventions: specs, test helpers + * and doubles that sit beside their spec (`*-test-harness.ts`, `*-fixtures.ts`, `*-fake.ts`), and + * test directories (`__tests__/`, `orca-runtime-tests/`). Shared by the Electron-import check and + * the localization audit, so a helper is test-only to both or to neither. + */ + +// Why `(? { + it('matches specs, helpers and doubles beside a spec, and test directories', () => { + for (const file of [ + 'src/main/a/reader.test.ts', + 'src/main/a/reader.spec.tsx', + 'src/main/a/reader.test-support.ts', + 'src/main/a/structured-agent-session-rest-test-rig.ts', + 'src/main/a/structured-agent-session-host-test-data.ts', + 'src/main/a/ipc-events-test-fixtures.ts', + 'src/main/a/routing-fixture.ts', + 'src/main/a/codex-turn-lifecycle-fake.ts', + 'src/main/a/github-ipc-module-mocks.ts', + 'src/main/a/settled-pty-write-stub.ts', + 'src/main/a/subscription-registry-test-double.ts', + 'src/main/a/__tests__/reader.ts', + 'src/main/a/__fixtures__/reader.ts', + 'src/main/a/test-support/reader.ts', + 'src/main/runtime/orca-runtime-tests/setup.ts', + 'src/main/runtime/orca-runtime-test-mocks/store.ts' + ]) { + expect(isTestOnlySourcePath(file), file).toBe(true) + } + }) + + it('keeps shipped modules whose names merely mention testing or fixtures', () => { + for (const file of [ + 'src/shared/relay-runtime-self-test-report.ts', + 'src/main/ssh/ssh-relay-runtime-self-test.ts', + 'src/main/ipc/local-network-connection-test.ts', + 'src/main/updater/latest-release.ts', + 'src/renderer/src/components/browser-pane/fixture-picker.tsx', + 'src/renderer/src/components/settings/fixtures-panel.tsx', + 'src/main/contest/entry.ts' + ]) { + expect(isTestOnlySourcePath(file), file).toBe(false) + } + }) +})