From 81eeb40ada495826b9dbdef106bde2692ebc2252 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Mon, 27 Jul 2026 01:01:25 -0700 Subject: [PATCH] perf(cli): load only the handler group a command dispatches into (#10883) Co-authored-by: Orca --- src/cli/browser-handler-groups.ts | 119 ++++++++++++++ src/cli/dispatch.ts | 90 ++++------- src/cli/handler-group-manifest.test.ts | 121 ++++++++++++++ src/cli/handler-group-manifest.ts | 214 +++++++++++++++++++++++++ 4 files changed, 482 insertions(+), 62 deletions(-) create mode 100644 src/cli/browser-handler-groups.ts create mode 100644 src/cli/handler-group-manifest.test.ts create mode 100644 src/cli/handler-group-manifest.ts diff --git a/src/cli/browser-handler-groups.ts b/src/cli/browser-handler-groups.ts new file mode 100644 index 000000000..0a7de2fa5 --- /dev/null +++ b/src/cli/browser-handler-groups.ts @@ -0,0 +1,119 @@ +import type { HandlerGroup } from './handler-group-manifest' + +// Why split out: the browser command surface is a third of the CLI's groups and +// changes as a unit, so it keeps handler-group-manifest.ts readable at a glance. +export const BROWSER_HANDLER_GROUPS: readonly HandlerGroup[] = [ + { + name: 'browser-nav', + keys: [ + 'snapshot', + 'screenshot', + 'goto', + 'back', + 'reload', + 'forward', + 'eval', + 'scroll', + 'wait', + 'pdf', + 'full-screenshot' + ], + load: async () => (await import('./handlers/browser-nav.js')).BROWSER_NAV_HANDLERS + }, + { + name: 'browser-interact', + keys: [ + 'click', + 'dblclick', + 'fill', + 'type', + 'select', + 'check', + 'uncheck', + 'focus', + 'clear', + 'select-all', + 'keypress', + 'hover', + 'drag', + 'upload', + 'scrollintoview', + 'get', + 'is', + 'inserttext', + 'mouse move', + 'mouse down', + 'mouse up', + 'mouse wheel', + 'find', + 'download', + 'highlight' + ], + load: async () => (await import('./handlers/browser-interact.js')).BROWSER_INTERACT_HANDLERS + }, + { + name: 'browser-tab', + keys: ['tab list', 'tab show', 'tab current', 'tab switch', 'tab create', 'tab close', 'exec'], + load: async () => (await import('./handlers/browser-tab.js')).BROWSER_TAB_HANDLERS + }, + { + name: 'browser-profile', + keys: [ + 'tab profile list', + 'tab profile create', + 'tab profile delete', + 'tab profile set', + 'tab profile show', + 'tab profile use-default', + 'tab profile clone' + ], + load: async () => (await import('./handlers/browser-profile.js')).BROWSER_PROFILE_HANDLERS + }, + { + name: 'browser-cookie', + keys: ['cookie get', 'cookie set', 'cookie delete'], + load: async () => (await import('./handlers/browser-cookie.js')).BROWSER_COOKIE_HANDLERS + }, + { + name: 'browser-capture', + keys: [ + 'intercept enable', + 'intercept disable', + 'intercept list', + 'capture start', + 'capture stop', + 'console', + 'network' + ], + load: async () => (await import('./handlers/browser-capture.js')).BROWSER_CAPTURE_HANDLERS + }, + { + name: 'browser-env', + keys: [ + 'viewport', + 'geolocation', + 'set device', + 'set offline', + 'set headers', + 'set credentials', + 'set media', + 'clipboard read', + 'clipboard write', + 'dialog accept', + 'dialog dismiss' + ], + load: async () => (await import('./handlers/browser-env.js')).BROWSER_ENV_HANDLERS + }, + { + name: 'browser-storage', + keys: [ + 'storage local get', + 'storage local set', + 'storage local clear', + 'storage session get', + 'storage session set', + 'storage session clear' + ], + load: async () => (await import('./handlers/browser-storage.js')).BROWSER_STORAGE_HANDLERS + } +] diff --git a/src/cli/dispatch.ts b/src/cli/dispatch.ts index b59962614..f4e30166d 100644 --- a/src/cli/dispatch.ts +++ b/src/cli/dispatch.ts @@ -1,30 +1,6 @@ import type { RuntimeClient } from './runtime-client' import { RuntimeClientError } from './runtime-client' -import { CORE_HANDLERS } from './handlers/core' -import { AUTOMATION_HANDLERS } from './handlers/automations' -import { PROJECT_HANDLERS } from './handlers/project' -import { REPO_HANDLERS } from './handlers/repo' -import { WORKTREE_HANDLERS } from './handlers/worktree' -import { FILE_HANDLERS } from './handlers/file' -import { TERMINAL_HANDLERS } from './handlers/terminal' -import { BROWSER_NAV_HANDLERS } from './handlers/browser-nav' -import { BROWSER_INTERACT_HANDLERS } from './handlers/browser-interact' -import { BROWSER_TAB_HANDLERS } from './handlers/browser-tab' -import { BROWSER_PROFILE_HANDLERS } from './handlers/browser-profile' -import { BROWSER_COOKIE_HANDLERS } from './handlers/browser-cookie' -import { BROWSER_CAPTURE_HANDLERS } from './handlers/browser-capture' -import { BROWSER_ENV_HANDLERS } from './handlers/browser-env' -import { BROWSER_STORAGE_HANDLERS } from './handlers/browser-storage' -import { ORCHESTRATION_HANDLERS } from './handlers/orchestration' -import { COMPUTER_HANDLERS } from './handlers/computer' -import { ENVIRONMENT_HANDLERS } from './handlers/environment' -import { AGENT_HOOK_HANDLERS } from './handlers/agent-hooks' -import { DIAGNOSTICS_HANDLERS } from './handlers/diagnostics' -import { INTROSPECTION_HANDLERS } from './handlers/introspection' -import { EMULATOR_HANDLERS } from './handlers/emulator' -import { LINEAR_HANDLERS } from './handlers/linear' -import { VM_HANDLERS } from './handlers/vm' -import { SKILL_HANDLERS } from './handlers/skills' +import { HANDLER_GROUPS, type HandlerGroup } from './handler-group-manifest' export type HandlerContext = { flags: Map @@ -36,56 +12,46 @@ export type HandlerContext = { export type CommandHandler = (ctx: HandlerContext) => Promise -function buildHandlers(): Map { - const table = new Map() - const groups = [ - CORE_HANDLERS, - AUTOMATION_HANDLERS, - PROJECT_HANDLERS, - REPO_HANDLERS, - WORKTREE_HANDLERS, - FILE_HANDLERS, - TERMINAL_HANDLERS, - BROWSER_NAV_HANDLERS, - BROWSER_INTERACT_HANDLERS, - BROWSER_TAB_HANDLERS, - BROWSER_PROFILE_HANDLERS, - BROWSER_COOKIE_HANDLERS, - BROWSER_CAPTURE_HANDLERS, - BROWSER_ENV_HANDLERS, - BROWSER_STORAGE_HANDLERS, - ORCHESTRATION_HANDLERS, - EMULATOR_HANDLERS, - COMPUTER_HANDLERS, - AGENT_HOOK_HANDLERS, - DIAGNOSTICS_HANDLERS, - INTROSPECTION_HANDLERS, - ENVIRONMENT_HANDLERS, - LINEAR_HANDLERS, - VM_HANDLERS, - SKILL_HANDLERS - ] +// Why: routing only needs key→group, so every CLI invocation can skip the +// transitive module graph of the 24 groups it does not dispatch into. +function buildRoutes(groups: readonly HandlerGroup[]): Map { + const table = new Map() for (const group of groups) { - for (const [key, handler] of Object.entries(group)) { - if (table.has(key)) { - throw new Error(`Duplicate CLI handler registration for "${key}"`) + for (const key of group.keys) { + const owner = table.get(key) + if (owner) { + throw new Error( + `Duplicate CLI handler registration for "${key}" (${owner.name} and ${group.name})` + ) } - table.set(key, handler) + table.set(key, group) } } return table } -const HANDLERS = buildHandlers() +const ROUTES = buildRoutes(HANDLER_GROUPS) // Why: exposes only the canonical command keys (not the handler internals) so the // registry-parity guard can check specs↔handlers without rebuilding the table. -export const HANDLER_COMMAND_KEYS: ReadonlySet = new Set(HANDLERS.keys()) +export const HANDLER_COMMAND_KEYS: ReadonlySet = new Set(ROUTES.keys()) export async function dispatch(commandPath: string[], ctx: HandlerContext): Promise { - const handler = HANDLERS.get(commandPath.join(' ')) + const key = commandPath.join(' ') + const group = ROUTES.get(key) + if (!group) { + throw new RuntimeClientError('invalid_argument', `Unknown command: ${key}`) + } + const handler = (await group.load())[key] + // Why: the manifest key list is verified against the real exports in CI, so a + // miss here means the group changed without the manifest — fail loudly. if (!handler) { - throw new RuntimeClientError('invalid_argument', `Unknown command: ${commandPath.join(' ')}`) + throw new RuntimeClientError( + 'invalid_argument', + `CLI handler group "${group.name}" does not export "${key}"` + ) } await handler(ctx) } + +export { buildRoutes as buildHandlerRoutes } diff --git a/src/cli/handler-group-manifest.test.ts b/src/cli/handler-group-manifest.test.ts new file mode 100644 index 000000000..d17c3446d --- /dev/null +++ b/src/cli/handler-group-manifest.test.ts @@ -0,0 +1,121 @@ +import { readdirSync } from 'node:fs' +import { join } from 'node:path' +import { describe, expect, it } from 'vitest' + +import { buildHandlerRoutes, dispatch, type HandlerContext } from './dispatch' +import { HANDLER_GROUPS, type HandlerGroup } from './handler-group-manifest' + +// Why: dispatch trusts the manifest's eager key lists to route without loading a +// group. These tests are the only thing standing between that trust and a +// silently unreachable command, so they load every group for real. + +describe('handler group manifest', () => { + it('lists a loadable group for every entry', async () => { + for (const group of HANDLER_GROUPS) { + const loaded = await group.load() + expect(loaded, `${group.name} resolved to a non-record`).toBeTypeOf('object') + } + }) + + it('matches each group export key-for-key', async () => { + const drift: string[] = [] + for (const group of HANDLER_GROUPS) { + const actual = Object.keys(await group.load()).sort() + const declared = [...group.keys].sort() + if (JSON.stringify(actual) !== JSON.stringify(declared)) { + drift.push( + `${group.name}: manifest ${JSON.stringify(declared)} !== export ${JSON.stringify(actual)}` + ) + } + } + expect(drift).toEqual([]) + }) + + it('exposes every declared key as a callable handler', async () => { + const notCallable: string[] = [] + for (const group of HANDLER_GROUPS) { + const loaded = await group.load() + for (const key of group.keys) { + if (typeof loaded[key] !== 'function') { + notCallable.push(`${group.name}/${key}`) + } + } + } + expect(notCallable).toEqual([]) + }) + + it('reaches every group through dispatch routing', () => { + const routes = buildHandlerRoutes(HANDLER_GROUPS) + const reached = new Set([...routes.values()].map((group) => group.name)) + const unreachable = HANDLER_GROUPS.filter((group) => !reached.has(group.name)).map( + (group) => group.name + ) + expect(unreachable).toEqual([]) + }) + + // Why: dropping a group from the manifest silently unregisters its commands — + // scan the directory so a new or forgotten handler file fails here, not in prod. + it('registers every handler module that exports a handler group', async () => { + // Why: __dirname works under both Vitest and the CommonJS tsc emit that + // build:cli type-checks this file against; import.meta.dirname does not. + const dir = join(__dirname, 'handlers') + const modules = readdirSync(dir).filter( + (file) => file.endsWith('.ts') && !file.endsWith('.test.ts') + ) + const registered = new Set(HANDLER_GROUPS.map((group) => group.name)) + const missing: string[] = [] + for (const file of modules) { + const name = file.slice(0, -'.ts'.length) + const exports: Record = await import(join(dir, file)) + const exportsGroup = Object.keys(exports).some((key) => key.endsWith('_HANDLERS')) + if (exportsGroup && !registered.has(name)) { + missing.push(name) + } + } + expect(missing).toEqual([]) + }) +}) + +describe('duplicate command keys', () => { + const group = (name: string, keys: string[]): HandlerGroup => ({ + name, + keys, + load: async () => ({}) + }) + + it('rejects the same key claimed by two groups', () => { + expect(() => + buildHandlerRoutes([group('alpha', ['ship it']), group('beta', ['ship it'])]) + ).toThrow('Duplicate CLI handler registration for "ship it" (alpha and beta)') + }) + + it('rejects a key duplicated inside one group list', () => { + expect(() => buildHandlerRoutes([group('alpha', ['ship it', 'ship it'])])).toThrow( + 'Duplicate CLI handler registration for "ship it"' + ) + }) + + it('accepts distinct keys across groups', () => { + const routes = buildHandlerRoutes([group('alpha', ['a']), group('beta', ['b'])]) + expect([...routes.keys()]).toEqual(['a', 'b']) + }) + + it('holds for the live manifest', () => { + expect(() => buildHandlerRoutes(HANDLER_GROUPS)).not.toThrow() + }) +}) + +describe('dispatch errors', () => { + const ctx = { + flags: new Map(), + cwd: '/tmp', + json: false + } as unknown as HandlerContext + + it('reports an unknown command with the joined path', async () => { + await expect(dispatch(['not', 'a', 'command'], ctx)).rejects.toMatchObject({ + code: 'invalid_argument', + message: 'Unknown command: not a command' + }) + }) +}) diff --git a/src/cli/handler-group-manifest.ts b/src/cli/handler-group-manifest.ts new file mode 100644 index 000000000..4d54c3aba --- /dev/null +++ b/src/cli/handler-group-manifest.ts @@ -0,0 +1,214 @@ +import type { CommandHandler } from './dispatch' +import { BROWSER_HANDLER_GROUPS } from './browser-handler-groups' + +export type HandlerGroup = { + name: string + // Why: eager string keys let dispatch build (and duplicate-check) the whole + // command table without loading any group's transitive module graph. + keys: readonly string[] + load: () => Promise> +} + +// Why: `keys` mirrors each group's exported record and is verified against the +// real exports by handler-group-manifest.test.ts, so drift fails CI, not dispatch. +export const HANDLER_GROUPS: readonly HandlerGroup[] = [ + { + name: 'core', + keys: ['claude-teams', 'open', 'serve', 'status'], + load: async () => (await import('./handlers/core.js')).CORE_HANDLERS + }, + { + name: 'automations', + keys: [ + 'automations list', + 'automations show', + 'automations create', + 'automations edit', + 'automations remove', + 'automations run', + 'automations runs' + ], + load: async () => (await import('./handlers/automations.js')).AUTOMATION_HANDLERS + }, + { + name: 'project', + keys: [ + 'project list', + 'project setups', + 'project setup-existing-folder', + 'project setup-clone', + 'project setup-create', + 'project setup-update', + 'project setup-delete' + ], + load: async () => (await import('./handlers/project.js')).PROJECT_HANDLERS + }, + { + name: 'repo', + keys: ['repo list', 'repo add', 'repo show', 'repo set-base-ref', 'repo search-refs'], + load: async () => (await import('./handlers/repo.js')).REPO_HANDLERS + }, + { + name: 'worktree', + keys: [ + 'worktree ps', + 'worktree list', + 'worktree show', + 'worktree current', + 'worktree create', + 'worktree set', + 'worktree rm' + ], + load: async () => (await import('./handlers/worktree.js')).WORKTREE_HANDLERS + }, + { + name: 'file', + keys: ['file open', 'file diff', 'file open-changed'], + load: async () => (await import('./handlers/file.js')).FILE_HANDLERS + }, + { + name: 'terminal', + keys: [ + 'terminal list', + 'terminal show', + 'terminal read', + 'terminal send', + 'terminal wait', + 'terminal stop', + 'terminal rename', + 'terminal create', + 'terminal switch', + 'terminal close', + 'terminal split' + ], + load: async () => (await import('./handlers/terminal.js')).TERMINAL_HANDLERS + }, + ...BROWSER_HANDLER_GROUPS, + { + name: 'orchestration', + keys: [ + 'orchestration send', + 'orchestration check', + 'orchestration reply', + 'orchestration inbox', + 'orchestration task-create', + 'orchestration task-list', + 'orchestration task-update', + 'orchestration dispatch', + 'orchestration ask', + 'orchestration dispatch-show', + 'orchestration run', + 'orchestration run-stop', + 'orchestration gate-create', + 'orchestration gate-resolve', + 'orchestration gate-list', + 'orchestration reset' + ], + load: async () => (await import('./handlers/orchestration.js')).ORCHESTRATION_HANDLERS + }, + { + name: 'emulator', + keys: [ + 'emulator list', + 'emulator devices', + 'emulator attach', + 'emulator tap', + 'emulator type', + 'emulator gesture', + 'emulator button', + 'emulator rotate', + 'emulator exec', + 'emulator kill', + 'emulator shutdown', + 'emulator install', + 'emulator launch', + 'emulator permissions', + 'emulator ax', + 'emulator logcat' + ], + load: async () => (await import('./handlers/emulator.js')).EMULATOR_HANDLERS + }, + { + name: 'computer', + keys: [ + 'computer capabilities', + 'computer list-apps', + 'computer permissions', + 'computer list-windows', + 'computer get-app-state', + 'computer click', + 'computer perform-secondary-action', + 'computer scroll', + 'computer drag', + 'computer type-text', + 'computer press-key', + 'computer hotkey', + 'computer paste-text', + 'computer set-value' + ], + load: async () => (await import('./handlers/computer.js')).COMPUTER_HANDLERS + }, + { + name: 'agent-hooks', + keys: ['agent hooks status', 'agent hooks off', 'agent hooks on'], + load: async () => (await import('./handlers/agent-hooks.js')).AGENT_HOOK_HANDLERS + }, + { + name: 'diagnostics', + keys: ['diagnostics memory'], + load: async () => (await import('./handlers/diagnostics.js')).DIAGNOSTICS_HANDLERS + }, + { + name: 'introspection', + keys: ['agent-context'], + load: async () => (await import('./handlers/introspection.js')).INTROSPECTION_HANDLERS + }, + { + name: 'environment', + keys: ['environment add', 'environment list', 'environment show', 'environment rm'], + load: async () => (await import('./handlers/environment.js')).ENVIRONMENT_HANDLERS + }, + { + name: 'linear', + keys: [ + 'linear save-issue', + 'linear list-issues', + 'linear relation add', + 'linear relation remove', + 'linear issue', + 'linear search', + 'linear team list', + 'linear team members', + 'linear team states', + 'linear team labels', + 'linear project list', + 'linear list', + 'linear status set', + 'linear assignee set', + 'linear assignee clear', + 'linear priority set', + 'linear priority clear', + 'linear estimate set', + 'linear estimate clear', + 'linear due-date set', + 'linear due-date clear', + 'linear label add', + 'linear label remove', + 'linear label set', + 'linear comment add', + 'linear attach', + 'linear create' + ], + load: async () => (await import('./handlers/linear.js')).LINEAR_HANDLERS + }, + { + name: 'vm', + keys: ['vm recipe doctor'], + load: async () => (await import('./handlers/vm.js')).VM_HANDLERS + }, + { + name: 'skills', + keys: ['skills list', 'skills get'], + load: async () => (await import('./handlers/skills.js')).SKILL_HANDLERS + } +]