perf(cli): skip feature formatters during help and error startup (#18923)

* perf(cli): load error reporting without feature formatters

* test(cli): follow extracted error reporter in import guard

* chore(cli): track cli-error.ts in deferral equivalence baseline

The equivalence script restores TOUCHED files from the baseline rev to
rebuild the pre-deferral CLI. reportCliError/formatCliError moved from
format.ts into cli-error.ts, so the baseline arm must also drop
cli-error.ts (absent at older revs) or the old tree would still compile
against the new module.
This commit is contained in:
Neil
2026-09-05 20:03:19 -07:00
committed by GitHub
parent 388e9fb776
commit fb7b75d55d
6 changed files with 289 additions and 155 deletions
@@ -0,0 +1,121 @@
import assert from 'node:assert/strict'
import { createRequire } from 'node:module'
import { existsSync, realpathSync } from 'node:fs'
import { delimiter, join, resolve } from 'node:path'
// Emit each revision with tsc -p config/tsconfig.cli.json --outDir <dir> --composite false --incremental false.
// Run: node config/scripts/benchmark-cli-error-imports.mjs <before-dir> <after-dir>
const [beforeDir, afterDir] = process.argv.slice(2)
assert.ok(beforeDir && afterDir, 'Pass distinct before and after TypeScript output directories.')
assert.notEqual(
realpathSync(beforeDir),
realpathSync(afterDir),
'Do not compare a build to itself.'
)
const entries = {
before: join(resolve(beforeDir), 'cli', 'index.js'),
after: join(resolve(afterDir), 'cli', 'index.js')
}
for (const entry of Object.values(entries)) {
assert.ok(existsSync(entry), `Missing emitted CLI: ${entry}`)
}
const { runProcessSync } = createRequire(import.meta.url)(
join(resolve(afterDir), 'shared', 'child-process', 'run-process.js')
)
const child = String.raw`
const { performance } = require('node:perf_hooks')
const { writeSync } = require('node:fs')
const { createHash } = require('node:crypto')
const { basename } = require('node:path')
let stdout = '', stderr = ''
process.stdout.write = (text) => { stdout += text; return true }
process.stderr.write = (text) => { stderr += text; return true }
const started = performance.now()
const cli = require(process.argv[1])
const importMs = performance.now() - started
cli.main(JSON.parse(process.argv[2])).then(() => {
const totalMs = performance.now() - started
const modules = Object.keys(require.cache)
writeSync(1, JSON.stringify({
importMs, totalMs, modules: modules.length,
featureFormatters: modules.filter((file) => ['browser', 'terminal', 'project', 'automation', 'workspace', 'computer'].some((name) => basename(file) === name + '-format.js')),
stdout: createHash('sha256').update(stdout).digest('hex'),
stderr: createHash('sha256').update(stderr).digest('hex'),
exitCode: process.exitCode || 0
}))
process.exitCode = 0
}).catch((error) => { writeSync(2, String(error)); process.exitCode = 1 })
`
const cases = [
['--help'],
['help', 'terminal', 'read'],
['does-not-exist'],
['computer', 'click', '--does-not-exist'],
['does-not-exist', '--json']
]
const median = (values) => [...values].sort((a, b) => a - b)[Math.floor(values.length / 2)]
const summarize = (samples) => ({
importMs: median(samples.map((sample) => sample.importMs)),
totalMs: median(samples.map((sample) => sample.totalMs)),
modules: samples[0].modules
})
const rows = []
for (const args of cases) {
const samples = { before: [], after: [] }
let expected
for (let run = 0; run < 22; run++) {
for (const variant of run % 2 ? ['after', 'before'] : ['before', 'after']) {
const result = runProcessSync({
program: process.execPath,
args: ['-e', child, entries[variant], JSON.stringify(args)],
timeoutMs: 30_000,
env: {
...process.env,
NODE_PATH: [resolve('node_modules'), process.env.NODE_PATH]
.filter(Boolean)
.join(delimiter)
}
})
assert.equal(result.timedOut, false, 'CLI child timed out.')
assert.equal(result.code, 0, result.stderr)
const sample = JSON.parse(result.stdout)
const output = { stdout: sample.stdout, stderr: sample.stderr, exitCode: sample.exitCode }
expected ??= output
assert.deepEqual(output, expected, `${variant} output changed for ${args.join(' ')}`)
if (variant === 'after') {
assert.deepEqual(
sample.featureFormatters,
[],
'Help and syntax errors must skip feature formatters.'
)
}
if (run >= 2) {
samples[variant].push(sample)
}
}
}
assert.ok(samples.after[0].modules < samples.before[0].modules, 'Expected fewer loaded modules.')
rows.push({
args,
before: summarize(samples.before),
after: summarize(samples.after),
output: expected,
samples
})
}
console.log(
JSON.stringify(
{
node: process.version,
platform: process.platform,
measurement:
'Fresh-process import + main; excludes process creation; warmed filesystem; 2 warmups and 20 samples per variant, alternating order.',
entries,
rows
},
null,
2
)
)
@@ -2,7 +2,7 @@
// Equivalence check for deferring the RuntimeClient module graph in the CLI.
//
// Builds the CLI twice with the REAL tsc emit — once from the working tree and
// once with the seven touched files restored from git HEAD~ (the pre-deferral
// once with the touched files restored from git HEAD~ (the pre-deferral
// implementation) — then compares stdout, stderr and exit code BYTE FOR BYTE
// across a matrix of invocations.
//
@@ -13,7 +13,7 @@
//
// Usage: node config/scripts/cli-runtime-client-deferral-equivalence.mjs [--baseline <rev>]
import { execFileSync, spawnSync } from 'node:child_process'
import { mkdirSync, mkdtempSync, rmSync, writeFileSync, readFileSync } from 'node:fs'
import { existsSync, mkdirSync, mkdtempSync, rmSync, writeFileSync, readFileSync } from 'node:fs'
import { join, resolve } from 'node:path'
import { fileURLToPath } from 'node:url'
@@ -21,8 +21,11 @@ const REPO = fileURLToPath(new URL('../..', import.meta.url))
// The files this change touches. Restoring exactly these from the baseline rev
// reconstructs the old implementation without disturbing anything else.
// Files absent at the baseline (e.g. cli-error.ts, split out of format.ts
// later) are removed for the baseline build and put back afterwards.
const TOUCHED = [
'src/cli/args.ts',
'src/cli/cli-error.ts',
'src/cli/dispatch.ts',
'src/cli/flags.ts',
'src/cli/format.ts',
@@ -72,12 +75,16 @@ function buildTree(label, baselineRev) {
if (baselineRev) {
for (const file of TOUCHED) {
const path = join(REPO, file)
restored.push([path, readFileSync(path)])
const old = execFileSync('git', ['show', `${baselineRev}:${file}`], {
restored.push([path, existsSync(path) ? readFileSync(path) : null])
const old = spawnSync('git', ['show', `${baselineRev}:${file}`], {
cwd: REPO,
maxBuffer: 64 * 1024 * 1024
})
writeFileSync(path, old)
if (old.status === 0) {
writeFileSync(path, old.stdout)
} else {
rmSync(path, { force: true })
}
}
}
execFileSync(
@@ -97,7 +104,11 @@ function buildTree(label, baselineRev) {
)
} finally {
for (const [path, contents] of restored) {
writeFileSync(path, contents)
if (contents === null) {
rmSync(path, { force: true })
} else {
writeFileSync(path, contents)
}
}
}
return join(outDir, 'cli/index.js')
+144
View File
@@ -0,0 +1,144 @@
import { computerUseErrorRecoveryData } from '../shared/computer-use-error-recovery'
import {
matchAutomationOwnerConflict,
stripAutomationOwnerConflictCode
} from '../shared/automation-owner-conflict'
import { automationOwnerConflictRecovery } from './automation-owner-conflict-recovery'
import type { RuntimeRpcFailure } from './runtime-client'
import { RuntimeClientError, RuntimeRpcFailureError } from './runtime/types'
type CliErrorContext = {
commandPath?: readonly string[]
}
export function formatCliError(error: unknown, context: CliErrorContext = {}): string {
const message = error instanceof Error ? error.message : String(error)
if (error instanceof RuntimeClientError && error.code === 'runtime_unavailable') {
if (hasOrchestrationRequestId(error.data)) {
return message
}
return `${message}\nOrca is not running. Run 'orca open' first.`
}
// Why: error-specific recovery must win over the generic computer fallback.
// Classified from the whole error, not just `.code`: a hop that flattens the class leaves only the token.
const conflict = automationOwnerConflictRecovery(matchAutomationOwnerConflict(error))
if (conflict) {
return formatMessageWithNextSteps(stripAutomationOwnerConflictCode(message), conflict.nextSteps)
}
if (error instanceof RuntimeClientError) {
const nextSteps = nextStepsFromData(error.data)
if (nextSteps.length > 0) {
return formatMessageWithNextSteps(message, nextSteps)
}
if (error.code === 'invalid_argument' && context.commandPath?.[0] === 'computer') {
return formatMessageWithNextSteps(
message,
computerUseErrorRecoveryData('invalid_argument')?.nextSteps ?? []
)
}
}
if (
error instanceof RuntimeRpcFailureError &&
error.response.error.code === 'runtime_unavailable'
) {
return `${message}\nOrca is not running. Run 'orca open' first.`
}
if (error instanceof RuntimeRpcFailureError) {
return formatMessageWithNextSteps(message, nextStepsFromData(error.response.error.data))
}
return message
}
function hasOrchestrationRequestId(data: unknown): boolean {
return (
data !== null &&
typeof data === 'object' &&
typeof (data as { orchestrationRequestId?: unknown }).orchestrationRequestId === 'string'
)
}
export function reportCliError(error: unknown, json: boolean, context: CliErrorContext = {}): void {
if (json) {
if (error instanceof RuntimeRpcFailureError) {
console.log(JSON.stringify(withAutomationOwnerConflictRecovery(error.response), null, 2))
} else {
const response: RuntimeRpcFailure = {
id: 'local',
ok: false,
error: {
code:
matchAutomationOwnerConflict(error) ??
(error instanceof RuntimeClientError ? error.code : 'runtime_error'),
message: stripAutomationOwnerConflictCode(
error instanceof Error ? error.message : String(error)
),
data: localCliErrorData(error, context)
},
_meta: {
runtimeId: null
}
}
console.log(JSON.stringify(response, null, 2))
}
} else {
console.error(formatCliError(error, context))
}
}
/** Machine-readable half of the same recovery the human message carries. */
function withAutomationOwnerConflictRecovery(response: RuntimeRpcFailure): RuntimeRpcFailure {
const code = matchAutomationOwnerConflict(response)
const conflict = automationOwnerConflictRecovery(code)
if (!conflict || !code) {
return response
}
return {
...response,
error: {
...response.error,
// Restores the classification a flattening hop dropped, so --json consumers read the conflict, not the transport.
code,
message: stripAutomationOwnerConflictCode(response.error.message),
data: response.error.data ?? conflict
}
}
}
function formatMessageWithNextSteps(message: string, nextSteps: readonly string[]): string {
if (nextSteps.length === 0) {
return message
}
return `${message}\n${nextSteps.map((step) => `Next step: ${step}`).join('\n')}`
}
function nextStepsFromData(data: unknown): string[] {
if (
data &&
typeof data === 'object' &&
Array.isArray((data as { nextSteps?: unknown }).nextSteps)
) {
return (data as { nextSteps: unknown[] }).nextSteps.filter(
(step): step is string => typeof step === 'string'
)
}
return []
}
function localCliErrorData(error: unknown, context: CliErrorContext): unknown {
// Why: error-specific recovery must win over the generic computer fallback.
if (error instanceof RuntimeClientError && error.data !== undefined) {
return error.data
}
const conflict = automationOwnerConflictRecovery(matchAutomationOwnerConflict(error))
if (conflict) {
return conflict
}
if (
error instanceof RuntimeClientError &&
error.code === 'invalid_argument' &&
context.commandPath?.[0] === 'computer'
) {
return computerUseErrorRecoveryData('invalid_argument')
}
return undefined
}
+3 -144
View File
@@ -1,13 +1,8 @@
import type { CliStatusResult } from '../shared/runtime-types'
import { computerUseErrorRecoveryData } from '../shared/computer-use-error-recovery'
import {
matchAutomationOwnerConflict,
stripAutomationOwnerConflictCode
} from '../shared/automation-owner-conflict'
import { automationOwnerConflictRecovery } from './automation-owner-conflict-recovery'
import { prepareComputerCliJsonResult } from './computer-format'
import type { RuntimeRpcFailure, RuntimeRpcSuccess } from './runtime-client'
import { RuntimeClientError, RuntimeRpcFailureError } from './runtime/types'
import type { RuntimeRpcSuccess } from './runtime-client'
export { formatCliError, reportCliError } from './cli-error'
export {
formatBrowserProfileList,
@@ -67,10 +62,6 @@ export {
formatWorktreeShow
} from './workspace-format'
type CliErrorContext = {
commandPath?: readonly string[]
}
export function printResult<TResult>(
response: RuntimeRpcSuccess<TResult>,
json: boolean,
@@ -83,138 +74,6 @@ export function printResult<TResult>(
console.log(formatter(response.result))
}
export function formatCliError(error: unknown, context: CliErrorContext = {}): string {
const message = error instanceof Error ? error.message : String(error)
if (error instanceof RuntimeClientError && error.code === 'runtime_unavailable') {
if (hasOrchestrationRequestId(error.data)) {
return message
}
return `${message}\nOrca is not running. Run 'orca open' first.`
}
// Why: error-specific recovery must win over the generic computer fallback.
// Classified from the whole error, not just `.code`: a hop that flattens the class leaves only the token.
const conflict = automationOwnerConflictRecovery(matchAutomationOwnerConflict(error))
if (conflict) {
return formatMessageWithNextSteps(stripAutomationOwnerConflictCode(message), conflict.nextSteps)
}
if (error instanceof RuntimeClientError) {
const nextSteps = nextStepsFromData(error.data)
if (nextSteps.length > 0) {
return formatMessageWithNextSteps(message, nextSteps)
}
if (error.code === 'invalid_argument' && context.commandPath?.[0] === 'computer') {
return formatMessageWithNextSteps(
message,
computerUseErrorRecoveryData('invalid_argument')?.nextSteps ?? []
)
}
}
if (
error instanceof RuntimeRpcFailureError &&
error.response.error.code === 'runtime_unavailable'
) {
return `${message}\nOrca is not running. Run 'orca open' first.`
}
if (error instanceof RuntimeRpcFailureError) {
return formatMessageWithNextSteps(message, nextStepsFromData(error.response.error.data))
}
return message
}
function hasOrchestrationRequestId(data: unknown): boolean {
return (
data !== null &&
typeof data === 'object' &&
typeof (data as { orchestrationRequestId?: unknown }).orchestrationRequestId === 'string'
)
}
export function reportCliError(error: unknown, json: boolean, context: CliErrorContext = {}): void {
if (json) {
if (error instanceof RuntimeRpcFailureError) {
console.log(JSON.stringify(withAutomationOwnerConflictRecovery(error.response), null, 2))
} else {
const response: RuntimeRpcFailure = {
id: 'local',
ok: false,
error: {
code:
matchAutomationOwnerConflict(error) ??
(error instanceof RuntimeClientError ? error.code : 'runtime_error'),
message: stripAutomationOwnerConflictCode(
error instanceof Error ? error.message : String(error)
),
data: localCliErrorData(error, context)
},
_meta: {
runtimeId: null
}
}
console.log(JSON.stringify(response, null, 2))
}
} else {
console.error(formatCliError(error, context))
}
}
/** Machine-readable half of the same recovery the human message carries. */
function withAutomationOwnerConflictRecovery(response: RuntimeRpcFailure): RuntimeRpcFailure {
const code = matchAutomationOwnerConflict(response)
const conflict = automationOwnerConflictRecovery(code)
if (!conflict || !code) {
return response
}
return {
...response,
error: {
...response.error,
// Restores the classification a flattening hop dropped, so --json consumers read the conflict, not the transport.
code,
message: stripAutomationOwnerConflictCode(response.error.message),
data: response.error.data ?? conflict
}
}
}
function formatMessageWithNextSteps(message: string, nextSteps: readonly string[]): string {
if (nextSteps.length === 0) {
return message
}
return `${message}\n${nextSteps.map((step) => `Next step: ${step}`).join('\n')}`
}
function nextStepsFromData(data: unknown): string[] {
if (
data &&
typeof data === 'object' &&
Array.isArray((data as { nextSteps?: unknown }).nextSteps)
) {
return (data as { nextSteps: unknown[] }).nextSteps.filter(
(step): step is string => typeof step === 'string'
)
}
return []
}
function localCliErrorData(error: unknown, context: CliErrorContext): unknown {
// Why: error-specific recovery must win over the generic computer fallback.
if (error instanceof RuntimeClientError && error.data !== undefined) {
return error.data
}
const conflict = automationOwnerConflictRecovery(matchAutomationOwnerConflict(error))
if (conflict) {
return conflict
}
if (
error instanceof RuntimeClientError &&
error.code === 'invalid_argument' &&
context.commandPath?.[0] === 'computer'
) {
return computerUseErrorRecoveryData('invalid_argument')
}
return undefined
}
export type HostListEntry = {
kind: 'local' | 'ssh' | 'environment'
name: string
+1 -1
View File
@@ -15,7 +15,7 @@ import {
resolveHostFlagEnvironmentId
} from './execution-host-flag'
import { listSshTargets } from './host-selector-alternatives'
import { reportCliError } from './format'
import { reportCliError } from './cli-error'
import { printHelp } from './help'
import type { RuntimeClient } from './runtime-client'
import { COMMAND_SPECS } from './specs'
+3 -4
View File
@@ -84,14 +84,12 @@ describe('RuntimeClient module-graph deferral', () => {
process.exitCode = 0
})
// Why: the whole point of the change. These six modules load on EVERY
// invocation, so a value-import of the barrel from any of them drags the
// RuntimeClient graph (zod, ws, tweetnacl) back onto the --help path.
// These eager modules must not pull the RuntimeClient dependency graph into help.
it.each([
'args.ts',
'flags.ts',
'dispatch.ts',
'format.ts',
'cli-error.ts',
'selectors.ts',
'execution-host-flag.ts'
])('%s imports error classes from ./runtime/types, not the barrel', (file) => {
@@ -110,6 +108,7 @@ describe('RuntimeClient module-graph deferral', () => {
expect(source).toContain("import type { RuntimeClient } from './runtime-client'")
expect(source).not.toMatch(/^import \{[^}]*RuntimeClient[^}]*\} from '\.\/runtime-client'/m)
expect(source).toContain("await import('./runtime-client.js')")
expect(source).toContain("import { reportCliError } from './cli-error'")
})
it('constructs no client for --help', async () => {