From 71733ccc042d05dfddd4988da762623eba5fb406 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Fri, 9 Oct 2026 22:12:37 -0700 Subject: [PATCH] Speed up process-host import enforcement in CI (#27124) Reduce import-scanner work by omitting unused AST parents and the duplicate traversal. Run the mandatory process-host import check alongside static work and explicitly join it before CI can succeed. Update both workflow phase/enforcement contracts and migrate the newer benchmark to the public subprocess package API. Keep the original package build path, manifests, runtime, test selection, and shard count. Independent native review is clean; validation and timing evidence are in the PR. --- .github/workflows/pr.yml | 17 +++- .../codex-usage-parser-memory-benchmark.mjs | 8 +- config/scripts/pr-preflight-gates.test.mjs | 1 + .../scripts/pr-workflow-parallelism.test.mjs | 11 ++- config/scripts/static-module-specifiers.mjs | 82 +++++++++++-------- .../scripts/static-module-specifiers.test.mjs | 24 ++++++ 6 files changed, 104 insertions(+), 39 deletions(-) diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index cae41e3e217..4e7f3bee107 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -203,6 +203,17 @@ jobs: with: bun-version-file: config/.bun-version + - name: Check process-host imports + if: '!cancelled()' + id: process-host-imports + background: true + env: + PREFLIGHT_PHASE_SELECTED: ${{ needs.code_paths.outputs.static_analysis == 'true' }} + PREFLIGHT_PRIOR_SUCCESS: ${{ job.status == 'success' }} + run: | + if [ "$PREFLIGHT_PHASE_SELECTED" != true ] || [ "$PREFLIGHT_PRIOR_SUCCESS" != true ]; then exit 0; fi + pnpm run check:process-host-imports + # Keep each check in its own log while sharing this runner. - name: Lint if: '!cancelled()' @@ -268,9 +279,7 @@ jobs: - name: Check reliability gate manifest if: needs.code_paths.outputs.static_analysis == 'true' - run: | - pnpm run check:reliability-gates - pnpm run check:process-host-imports + run: pnpm run check:reliability-gates - name: Enforce dead design-system classes if: needs.code_paths.outputs.static_analysis == 'true' @@ -443,6 +452,8 @@ jobs: - run: pnpm run typecheck if: needs.code_paths.outputs.typecheck == 'true' + - wait: process-host-imports + - wait: unit-plan - uses: actions/upload-artifact@v7 diff --git a/config/scripts/codex-usage-parser-memory-benchmark.mjs b/config/scripts/codex-usage-parser-memory-benchmark.mjs index ce49d505d94..bdcff23bd05 100644 --- a/config/scripts/codex-usage-parser-memory-benchmark.mjs +++ b/config/scripts/codex-usage-parser-memory-benchmark.mjs @@ -4,7 +4,7 @@ import { appendFileSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from import { availableParallelism, cpus, release, tmpdir, totalmem } from 'node:os' import { dirname, join, relative, resolve, sep } from 'node:path' import { performance } from 'node:perf_hooks' -import { pathToFileURL } from 'node:url' +import { fileURLToPath, pathToFileURL } from 'node:url' const root = resolve(import.meta.dirname, '../..') const digest = (value) => createHash('sha256').update(value).digest('hex') @@ -134,7 +134,8 @@ function writeFixture(filePath, eventCount) { async function runComparison() { const { build, version: esbuildVersion } = await import('esbuild') - const { runProcessSync, describeProcessFailure } = await import('./script-child-process.mjs') + const { runProcessSync } = await import('@orca/process-host') + const { describeProcessFailure } = await import('./process-failure-message.mjs') const { buildCounterbalancedSchedule } = await import('./counterbalanced-benchmark-schedule.mjs') const eventCount = positiveInteger(argument('--events', '100000'), 'events') const pairCount = positiveInteger(argument('--pairs', '2'), 'pairs') @@ -275,7 +276,8 @@ async function runComparison() { instrumentationSha256: Object.fromEntries( [ import.meta.filename, - join(import.meta.dirname, 'script-child-process.mjs'), + fileURLToPath(import.meta.resolve('@orca/process-host')), + join(import.meta.dirname, 'process-failure-message.mjs'), join(import.meta.dirname, 'counterbalanced-benchmark-schedule.mjs') ].map((path) => [relative(root, path).split(sep).join('/'), digest(readFileSync(path))]) ), diff --git a/config/scripts/pr-preflight-gates.test.mjs b/config/scripts/pr-preflight-gates.test.mjs index 56b7926a693..3ec69891b09 100644 --- a/config/scripts/pr-preflight-gates.test.mjs +++ b/config/scripts/pr-preflight-gates.test.mjs @@ -218,6 +218,7 @@ it('pins every foreground and background step to its selected phase', () => { .filter((step) => step.background) .map((step) => [step.id, step.env.PREFLIGHT_PHASE_SELECTED]) ).toEqual([ + ['process-host-imports', `\${{ ${staticPhase} }}`], ['root-lint', `\${{ ${staticPhase} }}`], ['native-code-quality', `\${{ ${staticPhase} }}`], ['changed-code-quality', `\${{ ${staticPhase} }}`], diff --git a/config/scripts/pr-workflow-parallelism.test.mjs b/config/scripts/pr-workflow-parallelism.test.mjs index 0aa80e4963f..8c302a4563d 100644 --- a/config/scripts/pr-workflow-parallelism.test.mjs +++ b/config/scripts/pr-workflow-parallelism.test.mjs @@ -11,7 +11,16 @@ describe('PR workflow parallelism', () => { const guard = workflow.jobs.preflight.steps.find((step) => step.run?.includes('pnpm run check:process-host-imports') ) - expect(guard?.if).toBe("needs.code_paths.outputs.static_analysis == 'true'") + expect(guard?.if).toBe('!cancelled()') + expect(guard?.env.PREFLIGHT_PHASE_SELECTED).toBe( + "${{ needs.code_paths.outputs.static_analysis == 'true' }}" + ) + expect(guard?.background).toBe(true) + const typecheckIndex = workflow.jobs.preflight.steps.findIndex( + (step) => step.run === 'pnpm run typecheck' + ) + const waitIndex = workflow.jobs.preflight.steps.findIndex((step) => step.wait === guard.id) + expect(waitIndex).toBeGreaterThan(typecheckIndex) expect(guard?.['continue-on-error']).toBeUndefined() }) diff --git a/config/scripts/static-module-specifiers.mjs b/config/scripts/static-module-specifiers.mjs index e2fe2f1abe3..01d284bc853 100644 --- a/config/scripts/static-module-specifiers.mjs +++ b/config/scripts/static-module-specifiers.mjs @@ -36,46 +36,66 @@ function propertyName(node) { return ts.isElementAccessExpression(node) ? literalString(node.argumentExpression) : null } -function isConstDeclaration(node) { - return ts.isVariableDeclarationList(node.parent) && (node.parent.flags & ts.NodeFlags.Const) !== 0 -} - function collectBindings(source) { const strings = new Map() const declarations = [] const destructures = [] const imports = [] - function visit(node) { - if (ts.isVariableDeclaration(node) && node.initializer) { - const initializer = unwrap(node.initializer) - if (ts.isIdentifier(node.name)) { - declarations.push({ name: node.name.text, initializer }) - const value = literalString(initializer) - if (isConstDeclaration(node) && value !== null) { - strings.set(node.name.text, [...(strings.get(node.name.text) ?? []), value]) + const modules = [] + function visit(node, isConst = false) { + switch (node.kind) { + case ts.SyntaxKind.VariableDeclarationList: + for (const declaration of node.declarations) { + visit(declaration, (node.flags & ts.NodeFlags.Const) !== 0) } - } else if (ts.isObjectBindingPattern(node.name)) { - for (const element of node.name.elements) { - const key = element.propertyName ?? element.name - const name = ts.isIdentifier(key) ? key.text : literalString(key) - if (ts.isIdentifier(element.name) && name !== null) { - destructures.push({ name: element.name.text, key: name, initializer }) + return + case ts.SyntaxKind.VariableDeclaration: { + if (!node.initializer) { + break + } + const initializer = unwrap(node.initializer) + if (ts.isIdentifier(node.name)) { + declarations.push({ name: node.name.text, initializer }) + const value = literalString(initializer) + if (isConst && value !== null) { + strings.set(node.name.text, [...(strings.get(node.name.text) ?? []), value]) + } + } else if (ts.isObjectBindingPattern(node.name)) { + for (const element of node.name.elements) { + const key = element.propertyName ?? element.name + const name = ts.isIdentifier(key) ? key.text : literalString(key) + if (ts.isIdentifier(element.name) && name !== null) { + destructures.push({ name: element.name.text, key: name, initializer }) + } } } + break } - } else if (ts.isImportDeclaration(node) && ts.isStringLiteral(node.moduleSpecifier)) { - imports.push({ module: node.moduleSpecifier.text, clause: node.importClause }) - } else if ( - ts.isImportEqualsDeclaration(node) && - ts.isExternalModuleReference(node.moduleReference) && - ts.isStringLiteral(node.moduleReference.expression) - ) { - imports.push({ module: node.moduleReference.expression.text, namespace: node.name }) + case ts.SyntaxKind.ImportDeclaration: + if (ts.isStringLiteral(node.moduleSpecifier)) { + imports.push({ module: node.moduleSpecifier.text, clause: node.importClause }) + } + modules.push(node) + break + case ts.SyntaxKind.ImportEqualsDeclaration: + if ( + ts.isExternalModuleReference(node.moduleReference) && + ts.isStringLiteral(node.moduleReference.expression) + ) { + imports.push({ module: node.moduleReference.expression.text, namespace: node.name }) + } + modules.push(node) + break + case ts.SyntaxKind.ExportDeclaration: + case ts.SyntaxKind.ImportType: + case ts.SyntaxKind.CallExpression: + modules.push(node) + break } ts.forEachChild(node, visit) } visit(source) - return { strings, declarations, destructures, imports } + return { strings, declarations, destructures, imports, modules } } /** @@ -88,11 +108,11 @@ function collectBindings(source) { * are not resolved. */ export function collectModuleSpecifiers(file, contents) { - const source = ts.createSourceFile(file, contents, ts.ScriptTarget.Latest, true) + const source = ts.createSourceFile(file, contents, ts.ScriptTarget.Latest, false) if (source.parseDiagnostics.length > 0) { throw new Error(`Cannot parse ${file}; the process-host import check must not skip it.`) } - const { strings, declarations, destructures, imports } = collectBindings(source) + const { strings, declarations, destructures, imports, modules } = collectBindings(source) const loaders = new Set(['require']) const createRequires = new Set() const moduleNamespaces = new Set() @@ -212,7 +232,7 @@ export function collectModuleSpecifiers(file, contents) { specifiers.add(specifier) } } - function visit(node) { + for (const node of modules) { if (ts.isImportDeclaration(node) || ts.isExportDeclaration(node)) { add(node.moduleSpecifier) } else if ( @@ -233,8 +253,6 @@ export function collectModuleSpecifiers(file, contents) { add(node.arguments[0]) } } - ts.forEachChild(node, visit) } - visit(source) return [...specifiers] } diff --git a/config/scripts/static-module-specifiers.test.mjs b/config/scripts/static-module-specifiers.test.mjs index 53a1508721d..1eeaac902d8 100644 --- a/config/scripts/static-module-specifiers.test.mjs +++ b/config/scripts/static-module-specifiers.test.mjs @@ -118,6 +118,30 @@ describe('static module specifiers', () => { ).toHaveLength(1) }) + it('resolves escaped loader names and literals after parsing', () => { + expect( + specifiers(String.raw` + const load = \u0072equire + const CHILD_PROCESS = '\u0063hild_process' + load(CHILD_PROCESS) + process['get\u0042uiltinModule']('node:\u0063hild_process') + `) + ).toEqual(['child_process', 'node:child_process']) + }) + + it('preserves const binding order across nested declaration lists', () => { + expect( + specifiers(` + const nested = (() => { const NAME = 'first'; return NAME })(), NAME = 'second' + require(NAME) + `) + ).toEqual(['first', 'second']) + }) + + it('rejects invalid syntax even without imports or loader names', () => { + expect(() => specifiers('const broken = (')).toThrow('Cannot parse file.ts') + }) + it('ignores unrelated objects, computed property values, and non-constant specifiers', () => { expect( specifiers(