test: check structured-chat code for Electron imports even before the runtime loads it (#24988)

* test: keep structured chat free of Electron imports

* test: widen runtime Electron ratchet to structured chat

* test: check whole structured-chat directories for Electron imports

Cover src/main/{native-chat,claude,codex}, src/shared and every structured-*
or agent-session-* file under src/main/runtime, so new files are checked by
default. A lane that exists today now fails loudly if it goes missing; only
the not-yet-landed acp/ and provider-process/ may be absent.

Move the test-only file rule into one classifier shared with the
localization audit, so -test-<thing> helpers and test doubles no longer
enter the production gate.

* test: run the Electron-import CLI path in tests and retire may-be-absent lanes

Export main() so the real-tree tests exercise the entry list CI uses,
check every required lane for the missing-directory error, and fail once
acp/ or provider-process/ exists while still allowed to be absent.
This commit is contained in:
Brennan Benson
2026-10-05 14:48:33 -07:00
committed by GitHub
parent e817b0e237
commit 508419f11e
6 changed files with 318 additions and 51 deletions
+3 -3
View File
@@ -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/).
+2 -10
View File
@@ -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))
@@ -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()
}
@@ -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
+23
View File
@@ -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 `(?<!self)`: `*-self-test-*` modules are a shipped runtime probe, not tests.
const TEST_ONLY_BASENAME =
/\.(?:test|spec)\.|(?<!self)[.-]test-|[.-](?:fixtures?|mocks?|fakes?|stubs?|doubles?)\.[cm]?[jt]sx?$/
const TEST_ONLY_DIRECTORY = /^(?:__)?(?:tests?|fixtures?|mocks)(?:__)?$|(?:^|[.-])tests?(?:[.-]|$)/
/** @param {string} relativePath Repo-relative path with `/` separators. */
export function isTestOnlySourcePath(relativePath) {
const directories = relativePath.split('/')
const basename = directories.pop() ?? ''
return TEST_ONLY_BASENAME.test(basename) || directories.some(isTestOnlyDirectoryName)
}
/** Lets a directory walk prune a whole test tree instead of testing every file in it. */
export function isTestOnlyDirectoryName(name) {
return TEST_ONLY_DIRECTORY.test(name)
}
@@ -0,0 +1,41 @@
import { describe, expect, it } from 'vitest'
import { isTestOnlySourcePath } from './test-only-source-path.mjs'
describe('isTestOnlySourcePath', () => {
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)
}
})
})