diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index bc05e1d2a7e..19d420e3ff2 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -219,6 +219,17 @@ jobs: if [ "$PREFLIGHT_PHASE_SELECTED" != true ] || [ "$PREFLIGHT_PRIOR_SUCCESS" != true ]; then exit 0; fi bun --bun run check:process-host-imports + - name: Check main-process sync launches + if: '!cancelled()' + id: main-sync-spawn-ratchet + 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 + bun --bun run check:main-sync-spawn-ratchet + # Keep each check in its own log while sharing this runner. - name: Lint if: '!cancelled()' @@ -472,6 +483,8 @@ jobs: - wait: process-host-imports + - wait: main-sync-spawn-ratchet + - wait: unit-plan - uses: actions/upload-artifact@v7 diff --git a/config/main-sync-spawn-baseline.txt b/config/main-sync-spawn-baseline.txt new file mode 100644 index 00000000000..14a347ff772 --- /dev/null +++ b/config/main-sync-spawn-baseline.txt @@ -0,0 +1,86 @@ +# Synchronous child-process launches in production src/main and src/shared, per file: uses of +# execFileSync/spawnSync/execSync, runProcessSync and the repo sync wrappers listed in +# config/scripts/check-main-sync-spawn-ratchet.mjs (secure writers run icacls on Windows). +# Some entries run in the daemon or orcad process, or are test harnesses outside the test-path +# conventions. This is a RATCHET: counts may only go DOWN. +# Prune after removing launches: pnpm check:main-sync-spawn-ratchet --prune +1 src/main/agent-hooks/installer-utils.ts +1 src/main/artifacts/artifact-create-intent-store.ts +5 src/main/artifacts/artifact-share-record-store.ts +2 src/main/browser/browser-cookie-key.ts +1 src/main/browser/browser-route-egress-electron-launch.ts +1 src/main/browser/browser-route-partition-binding-store.ts +1 src/main/browser/browser-route-persisted-worker-electron-process.ts +1 src/main/claude-accounts/runtime-auth-service.ts +3 src/main/codex-accounts/fs-utils.ts +2 src/main/codex-accounts/legacy-wsl-runtime-auth-drain-recovery-script-harness.ts +2 src/main/codex-accounts/legacy-wsl-runtime-auth-drain-script-harness.ts +1 src/main/codex-accounts/runtime-home-service-home-routing.ts +1 src/main/codex-accounts/runtime-home-service-wsl.ts +2 src/main/codex-accounts/service.ts +1 src/main/computer/macos-computer-use-permissions.ts +2 src/main/credentials/encrypted-api-key-file-store.ts +2 src/main/daemon/daemon-cgroup-scope.ts +1 src/main/daemon/daemon-process-identity-query.ts +1 src/main/daemon/daemon-process-start-time.ts +1 src/main/daemon/daemon-user-manager-lifetime.ts +1 src/main/daemon/pty-subprocess/shell-launch-plan.ts +1 src/main/emulator/serve-sim-runtime-materializer.ts +1 src/main/git/command-runner/git-exec-file.ts +1 src/main/git/repo-detection-command.ts +1 src/main/git/runner.ts +1 src/main/ipc/worktree-logic.ts +2 src/main/macos-press-and-hold-default.ts +2 src/main/managed-data-accounts/credential-capture.ts +1 src/main/managed-data-accounts/profile-removal.ts +1 src/main/managed-data-accounts/service.ts +2 src/main/minimax/minimax-cookie-store.ts +1 src/main/network/macos-tailscale-dns-diagnostic.ts +1 src/main/opencode/legacy-plugin-acl-retry.ts +2 src/main/orca-profiles/profile-artifact-cloud-cleanup.ts +1 src/main/orca-profiles/profile-cloud-session-mutation.ts +2 src/main/orca-profiles/profile-cloud-session-store.ts +2 src/main/orcad/node-pty-precondition.ts +1 src/main/orcad/orcad-completed-stop-receipt.ts +1 src/main/orcad/orcad-data-root-privacy.ts +1 src/main/orcad/orcad-idle-stop-record.ts +1 src/main/orcad/orcad-managed-stop-completion.ts +1 src/main/orcad/orcad-managed-stop-decision.ts +2 src/main/persistence/profile-state/profile-state-access-identity.ts +1 src/main/persistence/profile-state/profile-state-recovery-copy.ts +1 src/main/plugins/plugin-private-marketplace-ssh-shim.cjs +2 src/main/plugins/plugin-secrets-store.ts +2 src/main/plugins/plugin-storage-store.ts +1 src/main/providers/working-directory-validation.ts +1 src/main/pty/posix-pty-foreground-group.ts +1 src/main/pty/posix-pty-process-groups.ts +1 src/main/pwsh.ts +3 src/main/runtime/device-registry.ts +1 src/main/runtime/e2ee-keypair.ts +1 src/main/runtime/mobile-notification-dismissal-store.ts +1 src/main/runtime/push/push-unregister-outbox.ts +1 src/main/runtime/relay/relay-region-preference.ts +1 src/main/runtime/relay/relay-revoke-outbox.ts +1 src/main/runtime/runtime-metadata.ts +1 src/main/runtime/runtime-skill-install-authority.ts +1 src/main/runtime/tls-certificate.ts +1 src/main/skills/skill-discovery-target.ts +1 src/main/ssh/orcad-held-fence-tokens.ts +1 src/main/ssh/orcad-migration-cutover-journal.ts +2 src/main/startup/cli-launch-redirect.ts +1 src/main/startup/linux-secret-service-probe.ts +2 src/main/win32-utils.ts +1 src/main/worktree-runner-script.ts +2 src/main/wsl-availability.ts +5 src/main/wsl.ts +3 src/main/zcode/zcode-plan-api-key-store.ts +2 src/main/zsh-startup-hook-pty-harness.ts +1 src/shared/bounded-secure-json-file.ts +1 src/shared/ephemeral-vm-runtime-feature-store.ts +2 src/shared/ephemeral-vm-runtime-store.ts +1 src/shared/fish-binary-requirement.ts +1 src/shared/openssh-default-shell.ts +1 src/shared/runtime-environment-sidecar.ts +1 src/shared/runtime-environment-store-file.ts +3 src/shared/secure-file.ts +3 src/shared/secure-path-windows-acl.ts diff --git a/config/scripts/check-main-sync-spawn-ratchet.mjs b/config/scripts/check-main-sync-spawn-ratchet.mjs new file mode 100644 index 00000000000..b5e2be3a49f --- /dev/null +++ b/config/scripts/check-main-sync-spawn-ratchet.mjs @@ -0,0 +1,436 @@ +import { globSync, readFileSync, writeFileSync } from 'node:fs' +import path from 'node:path' +import { pathToFileURL } from 'node:url' +import ts from 'typescript-api' +import { + diffCounts, + formatBaseline, + parseBaseline, + parseBaselineNotes +} from './check-focus-read-ratchet.mjs' +import { literalString, propertyName, unwrap } from './static-module-specifiers.mjs' +import { isTestOnlySourcePath } from './test-only-source-path.mjs' + +// Ratchet for synchronous child-process launches in Electron main and shared code: each one +// freezes every window until the child exits, and on Windows antivirus makes every launch slow. +// Per file it counts uses of the launch APIs and of the repo's synchronous wrappers around them. + +const ROOT = path.resolve(import.meta.dirname, '..', '..') +const BASELINE_PATH = 'config/main-sync-spawn-baseline.txt' +const SCAN_DIRECTORIES = ['src/main', 'src/shared'] +const PROCESS_ADVICE = 'use runProcess or spawnProcess from @orca/process-host' +const SECURE_ADVICE = `no async secure writer exists yet; keep the synchronous one and raise this file's count in ${BASELINE_PATH} (or add a row), never a plain write (it loses the Windows ACL hardening)` +const SECURE_WRITERS = [ + 'writeSecureFile', + 'writeSecureJsonFile', + 'writeDurableSecureJsonFile', + 'writeSecureJsonFileWithinLimit' +] +// Gated modules (package names, or repo-relative paths without extension, re-exporters included) +// and the advice for each gated function they export. +const GATED_FAMILIES = [ + { + modules: ['child_process', 'node:child_process'], + advice: { execFileSync: PROCESS_ADVICE, spawnSync: PROCESS_ADVICE, execSync: PROCESS_ADVICE } + }, + { modules: ['@orca/process-host'], advice: { runProcessSync: PROCESS_ADVICE } }, + { + modules: ['src/shared/secure-file', 'src/shared/bounded-secure-json-file'], + advice: Object.fromEntries( + [...SECURE_WRITERS, 'hardenSecurePath'].map((name) => [name, SECURE_ADVICE]) + ) + }, + { + modules: ['src/shared/secure-path-windows-acl'], + advice: { restrictWindowsPathSync: SECURE_ADVICE } + }, + { + modules: ['src/main/git/command-runner/git-exec-file', 'src/main/git/runner'], + advice: { gitExecFileSync: 'use gitExecFileAsync' } + }, + { + modules: ['src/main/wsl'], + advice: { + getWslHome: 'use getWslHomeAsync', + listWslDistros: 'use listWslDistrosAsync', + wslUncDirectoryExists: 'use wslUncDirectoryExistsAsync' + } + }, + { + modules: ['src/main/wsl-availability', 'src/main/wsl'], + advice: { isWslAvailable: 'use isWslAvailableAsync' } + }, + { modules: ['src/main/pwsh'], advice: { isPwshAvailable: 'use isPwshAvailableAsync' } }, + { modules: ['src/main/win32-utils'], advice: { grantDirAcl: 'use grantDirAclAsync' } } +] +const ADVICE = new Map(GATED_FAMILIES.flatMap((family) => Object.entries(family.advice))) +// Only spread-free calls with no options or a literal with absent/false `sync` are exempt. +const SYNC_FLAG_SYMBOLS = new Set(['hardenSecurePath']) +const FALLBACK_OPERATORS = new Set([ts.SyntaxKind.QuestionQuestionToken, ts.SyntaxKind.BarBarToken]) +const BASELINE_HEADER = [ + '# Synchronous child-process launches in production src/main and src/shared, per file: uses of', + '# execFileSync/spawnSync/execSync, runProcessSync and the repo sync wrappers listed in', + '# config/scripts/check-main-sync-spawn-ratchet.mjs (secure writers run icacls on Windows).', + '# Some entries run in the daemon or orcad process, or are test harnesses outside the test-path', + '# conventions. This is a RATCHET: counts may only go DOWN.', + '# Prune after removing launches: pnpm check:main-sync-spawn-ratchet --prune' +] + +function normalized(file) { + return file.replaceAll('\\', '/') +} + +function moduleKey(file) { + return normalized(file).replace(/\.[cm]?[jt]sx?$/, '') +} + +function symbolsOf(key) { + const names = GATED_FAMILIES.filter((family) => family.modules.includes(key)).flatMap((family) => + Object.keys(family.advice) + ) + return names.length > 0 ? new Set(names) : null +} + +/** Gated functions a module specifier exports, or null when the module is not gated. */ +export function gatedSymbolsFor(file, specifier) { + const normalizedSpecifier = normalized(specifier) + if (!normalizedSpecifier.startsWith('.')) { + return symbolsOf(normalizedSpecifier) + } + return symbolsOf( + moduleKey( + path.posix.normalize( + path.posix.join(path.posix.dirname(normalized(file)), normalizedSpecifier) + ) + ) + ) +} + +function isAsyncOnlyCall(callee) { + const call = callee.parent + if ( + !ts.isCallExpression(call) || + call.expression !== callee || + call.arguments.some(ts.isSpreadElement) + ) { + return false + } + const options = unwrap(call.arguments[1]) + return ( + !options || + (ts.isObjectLiteralExpression(options) && + options.properties.every((property) => { + if (ts.isSpreadAssignment(property)) { + return false + } + const name = ts.isIdentifier(property.name) + ? property.name.text + : literalString(property.name) + return ( + name !== null && + (name !== 'sync' || + (ts.isPropertyAssignment(property) && + unwrap(property.initializer).kind === ts.SyntaxKind.FalseKeyword)) + ) + })) + ) +} + +function isValueReference(node) { + const parent = node.parent + if ( + ts.isImportSpecifier(parent) || + ts.isImportClause(parent) || + ts.isNamespaceImport(parent) || + ts.isImportEqualsDeclaration(parent) + ) { + return false + } + if (ts.isExportSpecifier(parent)) { + return !parent.parent.parent.moduleSpecifier && node === (parent.propertyName ?? parent.name) + } + if (ts.isBindingElement(parent)) { + return node === parent.initializer + } + return ( + ts.isShorthandPropertyAssignment(parent) || + (parent.name !== node && parent.propertyName !== node && parent.label !== node) + ) +} + +/** + * Every value reference to a gated function counts (call, hand-off, alias initializer, each alias + * use), plus each re-export and a gated module's own references; imports alone count nothing. + * File-wide, not scope-aware; computed specifiers and member names are not resolved. + * Counted by reference only: not traced through non-const holders or the file's own wrappers. + */ +export function collectSyncLaunchUses(file, contents) { + const source = ts.createSourceFile(file, contents, ts.ScriptTarget.Latest, true) + if (source.parseDiagnostics.length > 0) { + throw new Error(`Cannot parse ${file}; the sync-spawn ratchet must not skip it.`) + } + const used = new Set() + let count = 0 + const namespaces = new Map() + // Local name -> gated function it refers to. + const functions = new Map([...(symbolsOf(moduleKey(file)) ?? [])].map((name) => [name, name])) + const aliases = [] + const members = [] + const identifiers = [] + const destructures = [] + + function moduleSymbols(node) { + node = unwrap(node) + if (!node) { + return null + } + if (ts.isIdentifier(node)) { + return namespaces.get(node.text) ?? null + } + const loads = + ts.isCallExpression(node) && + (node.expression.kind === ts.SyntaxKind.ImportKeyword || + (ts.isIdentifier(node.expression) && node.expression.text === 'require')) + const specifier = loads ? literalString(node.arguments[0]) : null + return specifier === null ? null : gatedSymbolsFor(file, specifier) + } + function gatedFunction(node) { + node = unwrap(node) + if (!node) { + return null + } + if (ts.isIdentifier(node)) { + return functions.get(node.text) ?? null + } + if (ts.isPropertyAccessExpression(node) || ts.isElementAccessExpression(node)) { + const name = propertyName(node) + return moduleSymbols(node.expression)?.has(name) ? name : null + } + if (ts.isConditionalExpression(node)) { + return gatedFunction(node.whenTrue) ?? gatedFunction(node.whenFalse) + } + if (ts.isBinaryExpression(node) && FALLBACK_OPERATORS.has(node.operatorToken.kind)) { + return gatedFunction(node.left) ?? gatedFunction(node.right) + } + return null + } + function recordUse(name, node) { + if (SYNC_FLAG_SYMBOLS.has(name) && isAsyncOnlyCall(node)) { + return + } + used.add(name) + count += 1 + } + function visitImport(node) { + const symbols = gatedSymbolsFor(file, node.moduleSpecifier.text) + const clause = node.importClause + if (!symbols || !clause || clause.isTypeOnly) { + return + } + if (clause.name) { + namespaces.set(clause.name.text, symbols) + } + const named = clause.namedBindings + if (named && ts.isNamespaceImport(named)) { + namespaces.set(named.name.text, symbols) + } + for (const element of named && ts.isNamedImports(named) ? named.elements : []) { + const imported = (element.propertyName ?? element.name).text + if (!element.isTypeOnly && symbols.has(imported)) { + functions.set(element.name.text, imported) + } + } + } + function visitExport(node) { + const symbols = + node.moduleSpecifier && ts.isStringLiteral(node.moduleSpecifier) && !node.isTypeOnly + ? gatedSymbolsFor(file, node.moduleSpecifier.text) + : null + if (!symbols) { + return + } + const names = + !node.exportClause || ts.isNamespaceExport(node.exportClause) + ? [...symbols] + : node.exportClause.elements + .filter((element) => !element.isTypeOnly) + .map((element) => (element.propertyName ?? element.name).text) + for (const name of names.filter((name) => symbols.has(name))) { + used.add(name) + count += 1 + } + } + function visit(node) { + if (ts.isTypeNode(node) && !ts.isExpressionWithTypeArguments(node)) { + return + } + if (ts.isImportDeclaration(node) && ts.isStringLiteral(node.moduleSpecifier)) { + visitImport(node) + } else if ( + ts.isImportEqualsDeclaration(node) && + !node.isTypeOnly && + ts.isExternalModuleReference(node.moduleReference) && + ts.isStringLiteral(node.moduleReference.expression) + ) { + const symbols = gatedSymbolsFor(file, node.moduleReference.expression.text) + if (symbols) { + namespaces.set(node.name.text, symbols) + } + } else if (ts.isExportDeclaration(node)) { + visitExport(node) + } else if (ts.isPropertyAccessExpression(node) || ts.isElementAccessExpression(node)) { + members.push(node) + } else if (ts.isIdentifier(node) && isValueReference(node)) { + identifiers.push(node) + } else if (ts.isVariableDeclaration(node) && node.initializer) { + if (ts.isIdentifier(node.name)) { + aliases.push({ name: node.name.text, initializer: node.initializer }) + } else if (ts.isObjectBindingPattern(node.name)) { + destructures.push(node) + } + } + ts.forEachChild(node, visit) + } + visit(source) + + // Aliases can chain (`const cp = require('child_process'); const run = cp.execFileSync`). + for (let changed = true; changed;) { + changed = false + for (const { name, initializer } of aliases) { + const symbols = moduleSymbols(initializer) + const gated = symbols ? null : gatedFunction(initializer) + if (symbols && !namespaces.has(name)) { + namespaces.set(name, symbols) + changed = true + } else if (gated && !functions.has(name)) { + functions.set(name, gated) + changed = true + } + } + for (const node of destructures) { + const symbols = moduleSymbols(node.initializer) + for (const element of symbols ? node.name.elements : []) { + const key = element.propertyName ?? element.name + const name = ts.isIdentifier(key) ? key.text : literalString(key) + if ( + symbols.has(name) && + ts.isIdentifier(element.name) && + !functions.has(element.name.text) + ) { + functions.set(element.name.text, name) + changed = true + } + } + } + } + for (const node of members) { + const name = propertyName(node) + if (moduleSymbols(node.expression)?.has(name)) { + recordUse(name, node) + } + } + for (const node of identifiers) { + const name = functions.get(node.text) + if (name) { + recordUse(name, node) + } + } + return { count, symbols: [...used].sort() } +} + +export function isRatchetedSourcePath(file) { + return ( + SCAN_DIRECTORIES.some((directory) => file.startsWith(`${directory}/`)) && + !file.endsWith('.d.ts') && + !isTestOnlySourcePath(file) + ) +} + +/** Per-file use counts and the gated functions each file uses. */ +export function countSyncSpawnUses(sources) { + const counts = new Map() + const symbols = new Map() + for (const [file, contents] of sources) { + if (!isRatchetedSourcePath(file)) { + continue + } + const uses = collectSyncLaunchUses(file, contents) + if (uses.count > 0) { + counts.set(file, uses.count) + symbols.set(file, uses.symbols) + } + } + return { counts, symbols } +} + +export function adviceFor(symbols) { + const groups = new Map() + for (const symbol of symbols) { + groups.set(ADVICE.get(symbol), [...(groups.get(ADVICE.get(symbol)) ?? []), symbol]) + } + return [...groups].map(([advice, names]) => `${names.join(', ')}: ${advice}.`).join(' ') +} + +export function collectSyncSpawnSources(root = ROOT) { + return new Map( + globSync( + SCAN_DIRECTORIES.map((directory) => `${directory}/**/*.{ts,tsx,mts,cts,js,mjs,cjs}`), + { cwd: root, exclude: ['**/node_modules/**'] } + ) + .map(normalized) + .filter(isRatchetedSourcePath) + .sort() + .map((file) => [file, readFileSync(path.join(root, file), 'utf8')]) + ) +} + +function total(counts) { + return [...counts.values()].reduce((sum, count) => sum + count, 0) +} + +export function main(root = ROOT) { + const { counts, symbols } = countSyncSpawnUses(collectSyncSpawnSources(root)) + const { grown, shrunk } = diffCounts( + counts, + parseBaseline(readFileSync(path.join(root, BASELINE_PATH), 'utf8')) + ) + for (const { file, now, allowed } of grown) { + console.error( + `::error file=${file}::${now} synchronous launch use(s), baseline allows ${allowed}. They block the main thread. ${adviceFor(symbols.get(file))}` + ) + } + for (const { file, now, allowed } of shrunk) { + console.error( + `::error file=${file}::${now} synchronous launch use(s), baseline still allows ${allowed}. Run: pnpm check:main-sync-spawn-ratchet --prune` + ) + } + if (grown.length + shrunk.length > 0) { + return 1 + } + console.log( + `Main sync-spawn ratchet OK — ${total(counts)} use(s) in ${counts.size} legacy file(s).` + ) + return 0 +} + +export function prune(root = ROOT) { + const baselineFile = path.join(root, BASELINE_PATH) + const text = readFileSync(baselineFile, 'utf8') + const { counts } = countSyncSpawnUses(collectSyncSpawnSources(root)) + // Lowers entries only; growth still has to be fixed in the code. + const pruned = new Map( + [...parseBaseline(text)].map(([file, allowed]) => [ + file, + Math.min(allowed, counts.get(file) ?? 0) + ]) + ) + writeFileSync(baselineFile, formatBaseline(pruned, BASELINE_HEADER, parseBaselineNotes(text))) + console.log(`Pruned ${BASELINE_PATH} to ${total(pruned)} use(s).`) +} + +if (process.argv[1] && import.meta.url === pathToFileURL(path.resolve(process.argv[1])).href) { + if (process.argv[2] === '--prune') { + prune() + } + process.exitCode = main() +} diff --git a/config/scripts/check-main-sync-spawn-ratchet.test.mjs b/config/scripts/check-main-sync-spawn-ratchet.test.mjs new file mode 100644 index 00000000000..b622cc99d7b --- /dev/null +++ b/config/scripts/check-main-sync-spawn-ratchet.test.mjs @@ -0,0 +1,310 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' +import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import path from 'node:path' +import { + adviceFor, + collectSyncLaunchUses, + countSyncSpawnUses, + gatedSymbolsFor, + main, + prune +} from './check-main-sync-spawn-ratchet.mjs' + +const roots = [] +const BASELINE = 'config/main-sync-spawn-baseline.txt' + +afterEach(() => { + vi.restoreAllMocks() + for (const root of roots.splice(0)) { + rmSync(root, { recursive: true, force: true }) + } +}) + +function uses(source, file = 'src/main/consumer.ts') { + return collectSyncLaunchUses(file, source) +} + +function tree(files) { + const root = mkdtempSync(path.join(tmpdir(), 'orca-main-sync-spawn-')) + roots.push(root) + for (const [file, source] of Object.entries(files)) { + mkdirSync(path.dirname(path.join(root, file)), { recursive: true }) + writeFileSync(path.join(root, file), source) + } + return root +} + +function captureErrors() { + vi.spyOn(console, 'log').mockImplementation(() => {}) + return vi.spyOn(console, 'error').mockImplementation(() => {}) +} + +describe('main sync-spawn ratchet', () => { + it.each([ + ["import { execFileSync } from 'node:child_process'\nexecFileSync('git')", 1, ['execFileSync']], + [ + "import { execFileSync as run } from 'child_process'\nrun('git')\nrun('gh')", + 2, + ['execFileSync'] + ], + ["import * as cp from 'node:child_process'\ncp.spawnSync('git')", 1, ['spawnSync']], + ["import cp from 'child_process'\ncp['execSync']('git')", 1, ['execSync']], + ["import cp = require('child_process')\ncp.execFileSync('git')", 1, ['execFileSync']], + ["require('node:child_process').execSync('git')", 1, ['execSync']], + [ + "const cp = require('child_process')\nconst alias = cp\nalias.spawnSync('git')", + 1, + ['spawnSync'] + ], + ["const { spawnSync: run } = require('node:child_process')\nrun('git')", 1, ['spawnSync']], + ["const { execSync } = await import('node:child_process')\nexecSync('git')", 1, ['execSync']], + ["(await import('child_process')).execFileSync('git')", 1, ['execFileSync']], + ["export { spawnSync } from 'node:child_process'", 1, ['spawnSync']], + ["import { execSync } from 'child_process'\nexport { execSync }", 1, ['execSync']], + [ + "import { execSync } from 'child_process'\nqueue.push(execSync, { execSync })", + 2, + ['execSync'] + ], + [ + "import { runProcessSync } from '@orca/process-host'\nconst run = options.run ?? runProcessSync\nrun(a)\nrun(b)", + 3, + ['runProcessSync'] + ], + [ + "import { writeDurableSecureJsonFile, writeSecureJsonFile } from '../shared/secure-file.ts'\nconst write = durable ? writeDurableSecureJsonFile : writeSecureJsonFile\nwrite(p, v)", + 3, + ['writeDurableSecureJsonFile', 'writeSecureJsonFile'] + ], + [ + "import * as store from '..\\\\shared\\\\bounded-secure-json-file'\nstore.writeSecureJsonFileWithinLimit(p, v, 1)", + 1, + ['writeSecureJsonFileWithinLimit'] + ], + [ + "const { execSync } = require('child_process')\nconst run = execSync\nrun('a')", + 2, + ['execSync'] + ], + [ + "function f() { const run = cp.execSync; run('a') }\nconst cp = require('child_process')", + 2, + ['execSync'] + ], + [ + "import { execFileSync } from 'child_process'\nclass A extends mixin(execFileSync) {}", + 1, + ['execFileSync'] + ], + [ + "import { runProcessSync } from '@orca/process-host'\nconst run = runProcessSync", + 1, + ['runProcessSync'] + ] + ])('counts %s', (source, count, symbols) => { + expect(uses(source)).toEqual({ count, symbols }) + }) + + it.each([ + "import { execFileSync } from 'node:child_process'", + "import { spawn, execFile } from 'node:child_process'\nspawn('git')", + "import type { execFileSync } from 'node:child_process'\nexecFileSync('x')", + "import { type spawnSync } from 'child_process'\nspawnSync('x')", + "import * as cp from 'child_process'\ntype Run = typeof cp.execFileSync", + "import { runProcessSync } from '@orca/process-host'\ntype O = { run?: typeof runProcessSync }", + "import { runProcess } from '@orca/process-host'\nrunProcess(spec)", + "import { runProcessSync } from '@orca/process-host/process-spec'\nrunProcessSync(spec)", + "import { writeSecureFile } from './secure-file'\nwriteSecureFile(p, v)", + "import { execFileSync } from 'child_process'\nconst o = { execFileSync: 1 }\no.execFileSync", + 'const other = { spawnSync() {} }\nother.spawnSync()', + "// execFileSync('git')\nconst script = \"require('child_process').execSync('ls')\"", + "import * as cp from 'child_process'\ncp.spawn('git')", + "import { gitExecFileAsync } from './git/runner'\ngitExecFileAsync(['status'])" + ])('ignores %s', (source) => { + expect(uses(source)).toEqual({ count: 0, symbols: [] }) + }) + + it.each([ + [ + "import { gitExecFileSync } from './git/runner'\ngitExecFileSync(['status'])", + 'gitExecFileSync' + ], + [ + "import { gitExecFileSync as git } from './git/command-runner/git-exec-file.js'\ngit([])", + 'gitExecFileSync' + ], + [ + "import { restrictWindowsPathSync } from '../shared/secure-path-windows-acl'\nrestrictWindowsPathSync(p, true)", + 'restrictWindowsPathSync' + ], + ["import { grantDirAcl } from './win32-utils'\ngrantDirAcl(dir)", 'grantDirAcl'], + ["import { getWslHome } from './wsl'\ngetWslHome(distro)", 'getWslHome'], + ["import { listWslDistros } from './wsl'\nlistWslDistros()", 'listWslDistros'], + [ + "import { wslUncDirectoryExists } from './wsl'\nwslUncDirectoryExists(p)", + 'wslUncDirectoryExists' + ], + ["import { isWslAvailable } from './wsl'\nisWslAvailable()", 'isWslAvailable'], + ["import { isWslAvailable } from './wsl-availability'\nisWslAvailable()", 'isWslAvailable'], + ["import { isPwshAvailable } from './pwsh'\nisPwshAvailable()", 'isPwshAvailable'] + ])('gates the repo sync wrapper in %s', (source, symbol) => { + expect(uses(source)).toEqual({ count: 1, symbols: [symbol] }) + }) + + it('counts hardenSecurePath unless the call is visibly async', () => { + const imported = "import { hardenSecurePath } from '../shared/secure-file'\n" + const count = (call) => uses(`${imported}${call}`).count + expect(count('hardenSecurePath(p, { isDirectory: true, sync: true })')).toBe(1) + expect(count("hardenSecurePath(p, { isDirectory: true, 'sync': true })")).toBe(1) + expect(count('hardenSecurePath(p, options)')).toBe(1) + expect(count('hardenSecurePath(p, { ...base, isDirectory: true })')).toBe(1) + expect(count('hardenSecurePath(p, { isDirectory: true, sync })')).toBe(1) + expect(count('hardenSecurePath(p, { sync: true } as const)')).toBe(1) + expect(count('tasks.map(hardenSecurePath)')).toBe(1) + expect(count('hardenSecurePath(p, { isDirectory: true })')).toBe(0) + expect(count('hardenSecurePath(p, { isDirectory: true, sync: false })')).toBe(0) + expect(count('hardenSecurePath(p)')).toBe(0) + expect( + uses("import * as sf from '../shared/secure-file'\nsf.hardenSecurePath(p, { sync: true })") + ).toEqual({ count: 1, symbols: ['hardenSecurePath'] }) + }) + + it.each([ + 'const args = [p, { isDirectory: false, platform: "win32", sync: true }] as const\nhardenSecurePath(...args)', + 'const options = [{ isDirectory: false, platform: "win32", sync: true }] as const\nhardenSecurePath(p, ...options)', + 'hardenSecurePath(...paths, { isDirectory: true, sync: false })' + ])('counts hardenSecurePath with spread call arguments: %s', (source) => { + expect(uses(`import { hardenSecurePath } from '../shared/secure-file'\n${source}`)).toEqual({ + count: 1, + symbols: ['hardenSecurePath'] + }) + }) + + it('counts re-exports in the re-exporting module and internal calls in the defining one', () => { + expect( + uses( + "export { gitExecFileAsync, gitExecFileSync } from './command-runner/git-exec-file'", + 'src/main/git/runner.ts' + ) + ).toEqual({ count: 1, symbols: ['gitExecFileSync'] }) + expect( + uses( + 'export function listWslDistros() { return [] }\nexport function first() { return listWslDistros()[0] }', + 'src/main/wsl.ts' + ) + ).toEqual({ count: 1, symbols: ['listWslDistros'] }) + }) + + it('resolves writer imports by the module they point at, not by name', () => { + expect( + uses( + "import { writeSecureFile } from './secure-file'\nwriteSecureFile(p, v)", + 'src/shared/store.ts' + ) + ).toEqual({ + count: 1, + symbols: ['writeSecureFile'] + }) + }) + + it('resolves backslash paths in both the importing file and the specifier', () => { + expect([...(gatedSymbolsFor('src\\main\\foo\\x.ts', '..\\wsl') ?? [])]).toContain('getWslHome') + }) + + it('fails closed on source it cannot parse', () => { + expect(() => uses('import {')).toThrow('Cannot parse src/main/consumer.ts') + }) + + it('counts only production main and shared source', () => { + const sync = "import { execSync } from 'node:child_process'\nexecSync('a')\nexecSync('b')" + const { counts } = countSyncSpawnUses( + new Map( + Object.entries({ + 'src/main/legacy.ts': sync, + 'src/shared/new.ts': + "import { runProcessSync } from '@orca/process-host'\nrunProcessSync(s)", + 'src/main/clean.ts': "import { spawn } from 'node:child_process'\nspawn('a')", + 'src/main/consumer.test.ts': sync, + 'src/main/fixtures/script.ts': sync, + 'src/main/types.d.ts': sync, + 'src/cli/handlers/core.ts': sync, + 'src/relay/host.ts': sync + }) + ) + ) + expect([...counts]).toEqual([ + ['src/main/legacy.ts', 2], + ['src/shared/new.ts', 1] + ]) + }) + + it('gives each gated family its own advice', () => { + expect(adviceFor(['execFileSync', 'runProcessSync'])).toBe( + 'execFileSync, runProcessSync: use runProcess or spawnProcess from @orca/process-host.' + ) + expect(adviceFor(['writeSecureFile'])).toContain( + "no async secure writer exists yet; keep the synchronous one and raise this file's count in config/main-sync-spawn-baseline.txt" + ) + expect(adviceFor(['restrictWindowsPathSync'])).toContain('never a plain write') + expect(adviceFor(['gitExecFileSync', 'grantDirAcl'])).toBe( + 'gitExecFileSync: use gitExecFileAsync. grantDirAcl: use grantDirAclAsync.' + ) + }) + + it('fails a new file and a rising count, and passes at the baseline count', () => { + const one = "import { spawnSync } from 'node:child_process'\nspawnSync('a')" + const root = tree({ + 'src/main/launch.ts': one, + 'src/main/launch.test.ts': `${one}\nspawnSync('b')`, + 'src/renderer/src/view.ts': one, + [BASELINE]: '# debt\n' + }) + const error = captureErrors() + expect(main(root)).toBe(1) + expect(error.mock.calls.map(([line]) => line)).toEqual([ + '::error file=src/main/launch.ts::1 synchronous launch use(s), baseline allows 0. They block the main thread. spawnSync: use runProcess or spawnProcess from @orca/process-host.' + ]) + writeFileSync(path.join(root, BASELINE), '1 src/main/launch.ts\n') + expect(main(root)).toBe(0) + writeFileSync(path.join(root, 'src/main/launch.ts'), `${one}\nspawnSync('b')`) + error.mockClear() + expect(main(root)).toBe(1) + expect(error.mock.calls[0][0]).toContain('2 synchronous launch use(s), baseline allows 1.') + }) + + it('fails a lowered or stale count until it is pruned, and prune never raises', () => { + const write = "import { writeSecureFile } from '../shared/secure-file'\nwriteSecureFile(a, b)\n" + const root = tree({ + 'src/main/store.ts': write, + 'src/main/clean.ts': 'export const done = true\n', + 'src/main/new.ts': write, + 'src/main/grown.ts': `${write}writeSecureFile(c, d)\n`, + [BASELINE]: + '# old header\n3 src/main/store.ts # tracked in #26754\n1 src/main/clean.ts\n1 src/main/grown.ts\n' + }) + const error = captureErrors() + expect(main(root)).toBe(1) + const lines = error.mock.calls.map(([line]) => line) + expect(lines[0]).toContain('src/main/grown.ts::2 synchronous launch use(s), baseline allows 1.') + expect(lines[1]).toContain('src/main/new.ts::1 synchronous launch use(s), baseline allows 0.') + expect(lines[1]).toContain('writeSecureFile: no async secure writer exists yet') + expect(lines.slice(2)).toEqual([ + '::error file=src/main/clean.ts::0 synchronous launch use(s), baseline still allows 1. Run: pnpm check:main-sync-spawn-ratchet --prune', + '::error file=src/main/store.ts::1 synchronous launch use(s), baseline still allows 3. Run: pnpm check:main-sync-spawn-ratchet --prune' + ]) + prune(root) + const pruned = readFileSync(path.join(root, BASELINE), 'utf8') + expect(pruned).toContain('counts may only go DOWN') + expect(pruned.split('\n').filter((line) => line && !line.startsWith('#'))).toEqual([ + '1 src/main/grown.ts', + '1 src/main/store.ts # tracked in #26754' + ]) + error.mockClear() + expect(main(root)).toBe(1) + expect(error.mock.calls.map(([line]) => line.split('::')[1])).toEqual([ + 'error file=src/main/grown.ts', + 'error file=src/main/new.ts' + ]) + }) +}) diff --git a/config/scripts/pr-preflight-gates.test.mjs b/config/scripts/pr-preflight-gates.test.mjs index cbf0581bc01..367ed18294f 100644 --- a/config/scripts/pr-preflight-gates.test.mjs +++ b/config/scripts/pr-preflight-gates.test.mjs @@ -244,6 +244,7 @@ it('pins every foreground and background step to its selected phase', () => { .map((step) => [step.id, step.env.PREFLIGHT_PHASE_SELECTED]) ).toEqual([ ['process-host-imports', `\${{ ${staticPhase} }}`], + ['main-sync-spawn-ratchet', `\${{ ${staticPhase} }}`], ['root-lint', `\${{ ${staticPhase} }}`], ['native-code-quality', `\${{ ${staticPhase} }}`], ['changed-code-quality', `\${{ ${staticPhase} }}`], diff --git a/config/scripts/static-module-specifiers.mjs b/config/scripts/static-module-specifiers.mjs index 01d284bc853..c79b7900a29 100644 --- a/config/scripts/static-module-specifiers.mjs +++ b/config/scripts/static-module-specifiers.mjs @@ -3,7 +3,7 @@ import ts from 'typescript-api' const MODULE_BUILTINS = new Set(['module', 'node:module']) const PROCESS_BUILTINS = new Set(['process', 'node:process']) -function unwrap(node) { +export function unwrap(node) { while ( node && (ts.isParenthesizedExpression(node) || @@ -22,14 +22,14 @@ function isNamed(node, name) { return ts.isIdentifier(node) && node.text === name } -function literalString(node) { +export function literalString(node) { node = unwrap(node) return node && (ts.isStringLiteral(node) || ts.isNoSubstitutionTemplateLiteral(node)) ? node.text : null } -function propertyName(node) { +export function propertyName(node) { if (ts.isPropertyAccessExpression(node)) { return node.name.text } diff --git a/package.json b/package.json index 4e3e2a62d0d..c742c4a8433 100644 --- a/package.json +++ b/package.json @@ -14,7 +14,7 @@ "audit:perf": "oxlint --config config/oxlint-performance-audit.json --format json src", "test:perf:contracts": "vitest run --config config/vitest.performance.config.ts", "format": "oxfmt --write .", - "lint": "oxlint && pnpm run audit:anti-slop && pnpm run audit:code-quality:native && pnpm run audit:code-quality:type-aware && pnpm run check:reliability-gates && pnpm run check:dead-classes && pnpm run check:max-lines-ratchet && pnpm run check:ts-nocheck-ratchet && pnpm run check:runtime-electron-ratchet && pnpm run check:focus-read-ratchet && pnpm run check:layout-writer-ratchets && pnpm run check:readme-local-links && pnpm run check:node-runtime-pin && pnpm run verify:rpc-params-catalog && pnpm run verify:acp-protocol && pnpm run verify:bundled-skill-guides && pnpm run verify:skill-bundle-manifest && pnpm run verify:localization-catalogs && pnpm run verify:localization-extraction && pnpm run verify:localization-coverage && pnpm run check:process-host-imports && pnpm run check:compat-boundary-imports", + "lint": "oxlint && pnpm run audit:anti-slop && pnpm run audit:code-quality:native && pnpm run audit:code-quality:type-aware && pnpm run check:reliability-gates && pnpm run check:dead-classes && pnpm run check:max-lines-ratchet && pnpm run check:ts-nocheck-ratchet && pnpm run check:runtime-electron-ratchet && pnpm run check:focus-read-ratchet && pnpm run check:layout-writer-ratchets && pnpm run check:readme-local-links && pnpm run check:node-runtime-pin && pnpm run verify:rpc-params-catalog && pnpm run verify:acp-protocol && pnpm run verify:bundled-skill-guides && pnpm run verify:skill-bundle-manifest && pnpm run verify:localization-catalogs && pnpm run verify:localization-extraction && pnpm run verify:localization-coverage && pnpm run check:process-host-imports && pnpm run check:main-sync-spawn-ratchet && pnpm run check:compat-boundary-imports", "audit:code-quality": "pnpm run audit:code-quality:native && pnpm run audit:code-quality:type-aware && pnpm run audit:react-doctor", "audit:code-quality:native": "oxlint --config config/oxlint-code-quality-native-plugins.json src config tests mobile --deny-warnings", "audit:code-quality:type-aware": "oxlint --type-aware --config config/oxlint-code-quality-type-aware.json src config tests --deny-warnings", @@ -178,6 +178,7 @@ "sync:anti-slop-plugin": "node config/scripts/sync-anti-slop-plugin.mjs", "build:packages": "pnpm --filter @orca/process-host build", "check:process-host-imports": "node config/scripts/check-process-host-imports.mjs", + "check:main-sync-spawn-ratchet": "node config/scripts/check-main-sync-spawn-ratchet.mjs", "check:compat-boundary-imports": "node config/scripts/check-compat-boundary-imports.mjs" }, "dependencies": { diff --git a/src/main/diagnostics/main-thread-churn-probe.ts b/src/main/diagnostics/main-thread-churn-probe.ts index af6877e408e..7b68fd384c1 100644 --- a/src/main/diagnostics/main-thread-churn-probe.ts +++ b/src/main/diagnostics/main-thread-churn-probe.ts @@ -76,8 +76,8 @@ export function classifySubprocessCommand(command: string, args: readonly string export type SubprocessSpawnStats = { count: number - // Cumulative and worst synchronous cost of initiating the spawn (the - // uv_spawn → posix_spawn call runs on the main thread before returning). + // Cumulative and worst main-thread block: the spawn initiation (uv_spawn → + // posix_spawn) for async launches, the whole run for runProcessSync. blockMsTotal: number blockMsMax: number } @@ -86,7 +86,7 @@ const spawnStatsByCommand = new Map() /** * Record one subprocess spawn from the main process. `blockMs` is how long - * the synchronous spawn/execFile initiation call held the main thread. + * the spawn call held the main thread (until exit for synchronous launches). * No-op unless ORCA_MAIN_THREAD_DIAGNOSTICS=1. */ export function recordSubprocessSpawn( diff --git a/src/packages/process-host/src/run-process.ts b/src/packages/process-host/src/run-process.ts index 7cf50c12c0d..a5d5c2d2e37 100644 --- a/src/packages/process-host/src/run-process.ts +++ b/src/packages/process-host/src/run-process.ts @@ -3,7 +3,7 @@ import { spawnSync as nodeSpawnSync, type ChildProcess } from 'node:child_process' -import { resolveSpawn } from './spawn-resolution' +import { resolveSpawn, type ResolvedSpawn } from './spawn-resolution' import { forceTerminateProcessTree, signalProcessTree } from './process-tree-termination' import { hasSpawnObserver, notifySpawnObserver } from './spawn-observer' @@ -339,6 +339,19 @@ function terminate(child: ChildProcess, signal?: NodeJS.Signals): void { */ export function runProcessSync(spec: ProcessSpec): ProcessResult { const resolved = resolveSpawn(spec, process.platform) + // Diagnostics only: spawnSync holds the calling thread until the child exits, + // so the whole call is the block, reported even when it throws. + const spawnStartedAt = hasSpawnObserver() ? performance.now() : null + try { + return runResolvedProcessSync(spec, resolved) + } finally { + if (spawnStartedAt !== null) { + notifySpawnObserver(resolved.file, resolved.args, performance.now() - spawnStartedAt) + } + } +} + +function runResolvedProcessSync(spec: ProcessSpec, resolved: ResolvedSpawn): ProcessResult { const result = nodeSpawnSync(resolved.file, [...resolved.args], { ...resolved.options, input: spec.input, diff --git a/src/packages/process-host/src/spawn-observer.test.ts b/src/packages/process-host/src/spawn-observer.test.ts index 6700fb1b351..bd21ffa4f53 100644 --- a/src/packages/process-host/src/spawn-observer.test.ts +++ b/src/packages/process-host/src/spawn-observer.test.ts @@ -1,6 +1,8 @@ import { afterEach, describe, expect, it } from 'vitest' +import { tmpdir } from 'node:os' +import path from 'node:path' import { setSpawnObserver } from './spawn-observer' -import { runProcess, spawnProcess } from './run-process' +import { runProcess, runProcessSync, spawnProcess } from './run-process' afterEach(() => { setSpawnObserver(null) @@ -27,6 +29,33 @@ describe('spawn observer', () => { expect(calls).toBe(1) }) + it('reports the whole blocking run of runProcessSync', () => { + const seen: { command: string; blockMs: number }[] = [] + setSpawnObserver((command, _args, blockMs) => seen.push({ command, blockMs })) + const result = runProcessSync({ + program: process.execPath, + args: ['-e', 'setTimeout(() => process.exit(3), 50)'] + }) + expect(result.code).toBe(3) + expect(seen).toHaveLength(1) + expect(seen[0].command).toBe(process.execPath) + expect(seen[0].blockMs).toBeGreaterThanOrEqual(40) + }) + + it('reports a runProcessSync launch that throws or times out', () => { + const seen: string[] = [] + setSpawnObserver((command) => seen.push(command)) + const missing = path.join(tmpdir(), 'orca-spawn-observer-missing-program') + expect(() => runProcessSync({ program: missing, args: [] })).toThrow() + const timedOut = runProcessSync({ + program: process.execPath, + args: ['-e', 'setTimeout(() => {}, 10_000)'], + timeoutMs: 50 + }) + expect(timedOut.timedOut).toBe(true) + expect(seen).toEqual([missing, process.execPath]) + }) + it('stays silent with no observer registered', async () => { let calls = 0 setSpawnObserver(null) diff --git a/tests/e2e/main-thread-git-cost.spec.ts b/tests/e2e/main-thread-git-cost.spec.ts index 53784cfe779..b7e977229e4 100644 --- a/tests/e2e/main-thread-git-cost.spec.ts +++ b/tests/e2e/main-thread-git-cost.spec.ts @@ -7,7 +7,7 @@ * paint and git. This one runs git churn alone against a hidden window and * takes a V8 CPU profile OF THE MAIN PROCESS for each phase, which is the only * way to split spawn initiation from stdout drain from porcelain parsing — the - * churn probe reports spawn initiation only. + * churn probe reports spawn initiation only (the whole run for synchronous launches). * * Run: ORCA_GIT_COST_BENCH=1 ORCA_BACKGROUND_LAUNCH=1 \ * npx playwright test tests/e2e/main-thread-git-cost.spec.ts \ diff --git a/tests/tools/benchmarks/main-thread-jank-bench.mjs b/tests/tools/benchmarks/main-thread-jank-bench.mjs index a64eecaf985..12284e176fb 100644 --- a/tests/tools/benchmarks/main-thread-jank-bench.mjs +++ b/tests/tools/benchmarks/main-thread-jank-bench.mjs @@ -8,8 +8,9 @@ * `[main-thread] {json}` stderr line every 5s containing: * - worst event-loop stall in the window (maxGapMs) and stall counts * - subprocess spawns since the last report, keyed by command - * ("git status", "gh api", …) with the synchronous spawn-initiation cost - * that held the main thread (blockMsTotal/blockMsMax) + * ("git status", "gh api", …) with how long each launch held the main + * thread (blockMsTotal/blockMsMax): spawn initiation for async launches, + * the whole run for synchronous ones * On macOS the bench also tails the unified log for the exact * "Performance Diagnostics" warnings from the issue and records which process * emitted them, so spawn churn and OS warnings can be correlated by timestamp.