perf(cli): keep node:http and ICU off the CLI startup path (-54% warm call) (#1681)

* perf(cli): keep node:http and ICU off the CLI startup path (-54% warm call)

A warm `appstate` cost 146ms; 79ms of that was startup work no local
command needs.

`node:http` was value-imported by four modules inside the eager import
closure of `src/cli.ts`, so every invocation initialized undici -- and,
under `NODE_USE_SYSTEM_CA=1`, the macOS trust store behind it. That is
~70ms of mostly off-CPU time (a blocking trustd lookup, invisible to
--cpu-prof), paid by commands that never speak HTTP: the default daemon
transport is a plain `node:net` socket. Cheap daemon-only commands paid
it in full; only commands slow enough to absorb it, like `snapshot`,
did not.

Separately, `option-schema.ts` sorted its specs with `localeCompare` at
module scope. The first `localeCompare` in a process costs ~5.4ms of ICU
collator initialization (every later one is ~0.002ms), so the CLI paid
ICU init purely to alphabetize an internal list. A case-insensitive
comparator reproduces the exact same order for the ASCII flag-key
vocabulary -- verified against all 159 keys.

Both HTTP modules are now loaded on demand, via `.default` so they stay
the same mutable module object a static import yields (the transport
tests stub `http.request` on it). Behavior is unchanged: the HTTP
transport, remote daemons and artifact upload/download all still work.

Interleaved A/B, alternating prebuilt dists in one loop, 5-min load avg
< 10, n=30 per arm:

  appstate      146.4ms -> 67.6ms   -53.8%
  session list  137.9ms -> 57.7ms   -58.2%
  snapshot -i   195.5ms -> 188.8ms   -3.4%  (device-work bound)

Without NODE_USE_SYSTEM_CA the appstate win is 73.9ms -> 57.9ms (-22%),
so this is not purely an artifact of that setting.

Measured and rejected: splitting command metadata from runtime handlers
(the facet restructure deferred by #1660). registry.js is 142kB of the
643kB eager closure but only ~1.6ms of module evaluation, and ALL module
compilation is ~9.5ms -- the whole refactor could not clear the 20ms bar
set for attempting it.

A unit test walks the eager closure of src/cli.ts and fails if any file
in it value-imports node:http/node:https again; type-only imports stay
allowed.

* test(cli): classify startup-closure imports with the ES module record

The regex-based guard only recognized `import ... from` / `export ... from`
forms, so a bare side-effect `import 'node:http'` anywhere in the eager
closure would reintroduce the full undici + system-CA initialization cost
while the guard stayed green -- the one form that binds nothing yet still
evaluates the module.

Classification now comes from oxc-parser's ES module record, the parser
this repo already uses for import analysis in scripts/lib/shipped-imports.
A side-effect import is exactly an entry-less static import, so it falls
out of the record rather than needing another regex branch; type-only
imports and re-exports are identified by their own `isType` flag instead
of being pattern-matched.

A fixture table now pins all eleven import forms -- side-effect, default,
namespace, named, value re-export, star re-export, mixed value+type,
type-only default/named/re-export, and dynamic -- so the classifier's
contract is asserted directly rather than only through the closure walk.
Verified red end to end: a side-effect import inserted into a real closure
module fails the guard and names the file.

* test(cli): follow workspace subpath imports in the startup-closure guard

The walk stopped at the package boundary, so a `node:http` import inside
any @agent-device/* module the CLI evaluates would have reintroduced the
cost invisibly -- the same blind spot the module-record rewrite closed for
side-effect imports, just relocated. Workspace specifiers now resolve
through each package's exports map.

The anti-vacuity assertion checks the closure actually contains package
files, so a resolver that silently returned null for every workspace
specifier would fail rather than leave the guard green over a src-only
closure. Verified red both ways: a side-effect import inserted into
packages/kernel/src/errors.ts, and one in a src/ module, each fail the
guard and name the offending file.

* test(cli): drop a redundant cast in the workspace exports lookup

* test(cli): classify a top-level dynamic import as eager, and share one HTTP loader

"On demand" is a claim about SCOPE, not syntax, and the guard only knew
syntax. `import('node:http')` at MODULE TOP LEVEL runs during module
evaluation -- so a top-level `await import(...)`, an `import(...).then(...)`,
or an immediately-invoked top-level function reintroduced the whole undici
plus trust-store cost while the guard stayed green. A dynamic import is lazy
only when its nearest enclosing function scope is not the module itself.

The guard now walks each closure file's OXC AST carrying an `eager` flag that
drops on entry to a FunctionDeclaration/FunctionExpression/ArrowFunction body,
and reports `import()` calls still reached with it set. Immediately-invoked
top-level functions are detected by unwrapping the callee's
ParenthesizedExpression, so `(async () => { ... })()` is eager too. The one
shape that still reads as lazy -- a top-level function invoked indirectly at
load time -- is stated as a known limitation in the test rather than left
implied.

Verified red by inserting each shape into a REACHABLE closure module
(daemon-client-transport.ts): top-level await, `.then(...)`, and the
top-level IIFE each fail the guard and name the file. The lazy case is
proven by the suite itself rather than a fixture alone -- the shared loader
below lives in the closure, so its function-local `await import('node:http')`
would turn the guard red if scope tracking were wrong.

The three lazy-load sites had drifted into three copies of the same ternary.
They now share `loadNodeHttpRequester` in src/utils/node-http.ts, next to the
other node:http helpers, which is where the reason for the indirection is
documented once. This also clears the Fallow complexity finding the previous
push introduced.

Fixture table extended to 19 cases covering both axes: static form
(side-effect, default, namespace, named, value/star re-export, mixed,
type-only x3) and dynamic scope (top-level x3, function/arrow/method-local,
and the exact ternary the shipped loader uses).

Author: thymikee

src/__tests__/cli-startup-import-closure.test.ts

@@ -0,0 +1,271 @@
+import { expect, test } from 'vitest';
+import fs from 'node:fs';
+import path from 'node:path';
+import { parseSync } from 'oxc-parser';
+
+/**
+ * Every CLI invocation eagerly evaluates the static import closure of
+ * `src/cli.ts`. Importing `node:http` (or `node:https`) for a VALUE inside that
+ * closure initializes undici and, under `NODE_USE_SYSTEM_CA=1`, the platform
+ * trust store as well -- ~79ms added to every warm command on macOS, including
+ * ones that never speak HTTP (the default daemon transport is a `node:net`
+ * socket). The HTTP daemon transport, remote-artifact upload and download all
+ * load these modules on demand instead.
+ *
+ * "On demand" is a claim about SCOPE, not about syntax, which is why this reads
+ * the AST rather than matching import forms. Two shapes evaluate the module
+ * during module evaluation while looking lazy or looking like nothing at all:
+ * a bare side-effect `import 'node:http'` (binds no names, still evaluates),
+ * and a dynamic `import('node:http')` sitting at module top level rather than
+ * inside a function -- including `.then(...)` and an immediately-invoked
+ * top-level function. A dynamic import is lazy only when its nearest enclosing
+ * function scope is not the module itself. Type-only imports and re-exports are
+ * erased at build and stay allowed.
+ *
+ * Known limitation: a top-level function that is invoked indirectly at load
+ * time (stored, then called by another top-level statement) reads as lazy here.
+ * Direct top-level invocation -- `(() => { ... })()` -- is detected.
+ */
+
+const srcRoot = path.resolve(import.meta.dirname, '..');
+const LAZY_HTTP_MODULES = new Set(['node:http', 'node:https']);
+const FUNCTION_NODES = new Set([
+  'FunctionDeclaration',
+  'FunctionExpression',
+  'ArrowFunctionExpression',
+]);
+const WORKSPACE_SPECIFIER = /^(@agent-device\/[^/]+)(\/.*)?$/;
+
+type AstNode = { type?: string; [key: string]: unknown };
+
+type ParsedModuleRecord = ReturnType<typeof parseSync>['module'];
+
+/** Static specifiers this file causes to be EVALUATED (not type-only). */
+function staticEvaluatedRefs(module: ParsedModuleRecord): string[] {
+  const refs: string[] = [];
+  for (const staticImport of module.staticImports) {
+    // No entries at all is a side-effect import (`import 'x'`), which always
+    // evaluates. Otherwise it evaluates unless every binding is type-only.
+    const evaluates =
+      staticImport.entries.length === 0 || staticImport.entries.some((entry) => !entry.isType);
+    if (evaluates) refs.push(staticImport.moduleRequest.value);
+  }
+  for (const staticExport of module.staticExports) {
+    for (const entry of staticExport.entries) {
+      if (entry.moduleRequest && !entry.isType) refs.push(entry.moduleRequest.value);
+    }
+  }
+  return refs;
+}
+
+function unwrapParentheses(node: unknown): AstNode | null {
+  let current = node as AstNode | null;
+  while (current?.type === 'ParenthesizedExpression')
+    current = current.expression as AstNode | null;
+  return current;
+}
+
+/** The body of a function invoked right where it is defined, if this is that call. */
+function immediatelyInvokedBody(node: AstNode): unknown {
+  if (node.type !== 'CallExpression') return null;
+  const callee = unwrapParentheses(node.callee);
+  return callee && FUNCTION_NODES.has(String(callee.type)) ? callee.body : null;
+}
+
+function dynamicImportSpecifier(node: AstNode): string | null {
+  if (node.type !== 'ImportExpression') return null;
+  const source = node.source as { type?: string; value?: unknown } | undefined;
+  return source?.type === 'Literal' && typeof source.value === 'string' ? source.value : null;
+}
+
+function recordDynamicImport(record: AstNode, eager: boolean, found: string[]): void {
+  if (!eager) return;
+  const specifier = dynamicImportSpecifier(record);
+  if (specifier !== null) found.push(specifier);
+}
+
+/** Descends into a node's children, dropping `eager` on the way into a function body. */
+function visitChildren(record: AstNode, eager: boolean, found: string[]): void {
+  const childEager = eager && !FUNCTION_NODES.has(String(record.type));
+  for (const [key, value] of Object.entries(record)) {
+    if (key !== 'type') collectEagerDynamicImports(value, childEager, found);
+  }
+}
+
+/** Walks the AST, collecting only `import()` calls reached without entering a function. */
+function collectEagerDynamicImports(node: unknown, eager: boolean, found: string[]): void {
+  if (Array.isArray(node)) {
+    for (const child of node) collectEagerDynamicImports(child, eager, found);
+    return;
+  }
+  if (!node || typeof node !== 'object') return;
+  const record = node as AstNode;
+  recordDynamicImport(record, eager, found);
+  // An immediately-invoked function runs now, so its body inherits `eager`.
+  const invokedBody = immediatelyInvokedBody(record);
+  if (invokedBody) collectEagerDynamicImports(invokedBody, eager, found);
+  visitChildren(record, eager, found);
+}
+
+/** Every specifier this file evaluates at load time, static or dynamic. */
+function eagerlyEvaluatedModules(fileName: string, source: string): string[] {
+  const parsed = parseSync(fileName, source);
+  const dynamic: string[] = [];
+  collectEagerDynamicImports(parsed.program, true, dynamic);
+  return [...new Set([...staticEvaluatedRefs(parsed.module), ...dynamic])];
+}
+
+function resolveRelative(fromFile: string, specifier: string): string | null {
+  const candidate = path.resolve(path.dirname(fromFile), specifier);
+  if (fs.existsSync(candidate) && fs.statSync(candidate).isFile()) return candidate;
+  for (const suffix of ['.ts', '.tsx', '/index.ts']) {
+    if (fs.existsSync(`${candidate}${suffix}`)) return `${candidate}${suffix}`;
+  }
+  return null;
+}
+
+/** `@agent-device/<pkg>` -> that package's directory, keyed by its declared name. */
+function readWorkspacePackageDirs(): Map<string, string> {
+  const packagesRoot = path.resolve(srcRoot, '..', 'packages');
+  const dirs = new Map<string, string>();
+  for (const entry of fs.readdirSync(packagesRoot)) {
+    const manifestPath = path.join(packagesRoot, entry, 'package.json');
+    if (!fs.existsSync(manifestPath)) continue;
+    const manifest = JSON.parse(fs.readFileSync(manifestPath, 'utf8')) as { name?: string };
+    if (manifest.name) dirs.set(manifest.name, path.join(packagesRoot, entry));
+  }
+  return dirs;
+}
+
+type ExportTarget = { default?: string; types?: string } | string;
+
+function readExportTarget(packageDir: string, subpath: string): string | undefined {
+  const manifest = JSON.parse(fs.readFileSync(path.join(packageDir, 'package.json'), 'utf8')) as {
+    exports?: Record<string, ExportTarget>;
+  };
+  const target = manifest.exports?.[subpath];
+  if (typeof target === 'string') return target;
+  return target?.default ?? target?.types;
+}
+
+/**
+ * Workspace subpath imports are followed too: a package the CLI evaluates can
+ * pull `node:http` in just as effectively as a file under src/, and stopping the
+ * walk at the package boundary would be the same blind spot in a new place.
+ */
+function resolveWorkspace(specifier: string, packageDirs: Map<string, string>): string | null {
+  const match = WORKSPACE_SPECIFIER.exec(specifier);
+  const packageName = match?.[1];
+  const packageDir = packageName ? packageDirs.get(packageName) : undefined;
+  if (!packageDir) return null;
+  const target = readExportTarget(packageDir, `.${match?.[2] ?? ''}`);
+  if (!target) return null;
+  const resolved = path.resolve(packageDir, target);
+  return fs.existsSync(resolved) ? resolved : null;
+}
+
+/** Every repo file evaluated as a consequence of importing `src/cli.ts`. */
+function eagerClosureOfCli(): string[] {
+  const packageDirs = readWorkspacePackageDirs();
+  const queue = [path.join(srcRoot, 'cli.ts')];
+  const visited = new Set<string>();
+  while (queue.length > 0) {
+    const current = queue.pop();
+    if (!current || visited.has(current)) continue;
+    visited.add(current);
+    for (const specifier of eagerlyEvaluatedModules(current, fs.readFileSync(current, 'utf8'))) {
+      const resolved = specifier.startsWith('.')
+        ? resolveRelative(current, specifier)
+        : resolveWorkspace(specifier, packageDirs);
+      if (resolved) queue.push(resolved);
+    }
+  }
+  return [...visited];
+}
+
+test.for([
+  // --- static forms ---
+  { form: 'side-effect import', code: `import 'node:http';`, eager: true },
+  { form: 'default value import', code: `import http from 'node:http';`, eager: true },
+  { form: 'namespace import', code: `import * as http from 'node:http';`, eager: true },
+  { form: 'named value import', code: `import { request } from 'node:http';`, eager: true },
+  { form: 'value re-export', code: `export { request } from 'node:http';`, eager: true },
+  { form: 'star re-export', code: `export * from 'node:http';`, eager: true },
+  {
+    form: 'mixed value + type import',
+    code: `import http, { type IncomingMessage } from 'node:http';`,
+    eager: true,
+  },
+  { form: 'type-only default import', code: `import type http from 'node:http';`, eager: false },
+  {
+    form: 'type-only named import',
+    code: `import { type IncomingMessage } from 'node:http';`,
+    eager: false,
+  },
+  {
+    form: 'type-only re-export',
+    code: `export type { IncomingMessage } from 'node:http';`,
+    eager: false,
+  },
+  // --- dynamic import: scope decides, not syntax ---
+  { form: 'top-level await import', code: `const m = await import('node:http');`, eager: true },
+  { form: 'top-level import().then', code: `import('node:http').then((m) => m);`, eager: true },
+  {
+    form: 'top-level immediately-invoked arrow',
+    code: `(async () => { await import('node:http'); })();`,
+    eager: true,
+  },
+  {
+    form: 'function-declaration-local import',
+    code: `async function load() { return await import('node:http'); }`,
+    eager: false,
+  },
+  {
+    form: 'arrow-local import',
+    code: `const load = async () => await import('node:http');`,
+    eager: false,
+  },
+  {
+    form: 'method-local import',
+    code: `class K { async load() { await import('node:http'); } }`,
+    eager: false,
+  },
+  {
+    form: 'ternary inside a function (the shipped lazy-load shape)',
+    code: `async function load(s: boolean) {
+      return s ? (await import('node:https')).default : (await import('node:http')).default;
+    }`,
+    eager: false,
+  },
+])('$form is eager=$eager', ({ code, eager }) => {
+  const refs = eagerlyEvaluatedModules('fixture.ts', code);
+  expect(refs.includes('node:http') || refs.includes('node:https')).toBe(eager);
+});
+
+test('the CLI startup import closure never evaluates node:http or node:https', () => {
+  const offenders: string[] = [];
+  for (const file of eagerClosureOfCli()) {
+    for (const specifier of eagerlyEvaluatedModules(file, fs.readFileSync(file, 'utf8'))) {
+      if (LAZY_HTTP_MODULES.has(specifier)) {
+        offenders.push(`${path.relative(srcRoot, file)} -> ${specifier}`);
+      }
+    }
+  }
+
+  expect(
+    offenders,
+    'Load node:http / node:https on demand instead: evaluating either one here costs every warm ' +
+      'CLI command ~79ms of undici + system-CA initialization.',
+  ).toEqual([]);
+});
+
+test('the CLI startup import closure is reachable and crosses the package boundary', () => {
+  // Guards the test above from silently passing because the walk found nothing:
+  // a resolver that returned null for everything would leave both the src side
+  // and the workspace side of the closure empty while the guard stayed green.
+  const closure = eagerClosureOfCli();
+  expect(closure.length).toBeGreaterThan(50);
+  expect(
+    closure.filter((file) => file.includes(`${path.sep}packages${path.sep}`)).length,
+  ).toBeGreaterThan(0);
+});

src/cli-schema/option-schema.ts

@@ -91,7 +91,20 @@ function buildOptionSpecs(): OptionSpec[] {
         return supported.has(command);
       },
     }))
-    .sort((left, right) => left.key.localeCompare(right.key));
+    .sort((left, right) => compareOptionKeys(left.key, right.key));
+}
+
+/**
+ * Case-insensitive-then-codepoint ordering, matching what `localeCompare`
+ * produces for the ASCII flag-key vocabulary. This runs at module load on
+ * every CLI invocation, and the FIRST `localeCompare` in a process pays ~5ms
+ * of ICU collator initialization — a cost the CLI otherwise never incurs.
+ */
+function compareOptionKeys(left: string, right: string): number {
+  const leftLower = left.toLowerCase();
+  const rightLower = right.toLowerCase();
+  if (leftLower !== rightLower) return leftLower < rightLower ? -1 : 1;
+  return left < right ? -1 : left > right ? 1 : 0;
 }
 
 function primaryFlagDefinition(spec: OptionSpec): FlagDefinition {

src/daemon/client/daemon-client-progress.ts

@@ -1,5 +1,7 @@
 import type { RequestProgressEvent, RequestProgressSink } from '@agent-device/contracts/progress';
-import http from 'node:http';
+// Type-only: importing `node:http` for a value eagerly initializes undici
+// (~9ms in a fresh process), which the default socket transport never needs.
+import type http from 'node:http';
 import type { Socket } from 'node:net';
 import { AppError } from '@agent-device/kernel/errors';
 import type { DaemonRequest, DaemonResponse } from '../types.ts';

src/daemon/client/daemon-client-transport.ts

@@ -1,9 +1,7 @@
 import type { RequestProgressSink } from '@agent-device/contracts/progress';
 import net from 'node:net';
-import http from 'node:http';
-import https from 'node:https';
 import { AppError } from '@agent-device/kernel/errors';
-import { readNodeHttpResponseBody } from '../../utils/node-http.ts';
+import { loadNodeHttpRequester, readNodeHttpResponseBody } from '../../utils/node-http.ts';
 import type { DaemonRequest, DaemonResponse } from '../types.ts';
 import { emitDiagnostic } from '../../utils/diagnostics.ts';
 import type { DaemonPaths, DaemonTransportPreference } from '../config.ts';
@@ -102,19 +100,19 @@ export async function readRemoteDaemonHealth(info: DaemonInfo): Promise<RemoteDa
   return health;
 }
 
-function readDaemonHttpHealth(info: DaemonInfo): Promise<RemoteDaemonHealth> {
+async function readDaemonHttpHealth(info: DaemonInfo): Promise<RemoteDaemonHealth> {
   const endpoint = info.baseUrl
     ? buildDaemonHttpUrl(info.baseUrl, 'health')
     : info.httpPort
       ? `http://127.0.0.1:${info.httpPort}/health`
       : null;
-  if (!endpoint) return Promise.resolve({ reachable: false });
+  if (!endpoint) return { reachable: false };
   const url = new URL(endpoint);
-  const transport = url.protocol === 'https:' ? https : http;
+  const transport = await loadNodeHttpRequester(url.protocol);
   const timeoutMs = info.baseUrl
     ? REMOTE_DAEMON_HEALTHCHECK_TIMEOUT_MS
     : LOCAL_DAEMON_HEALTHCHECK_TIMEOUT_MS;
-  return new Promise((resolve) => {
+  return await new Promise((resolve) => {
     const headers = info.baseUrl ? buildDaemonHttpAuthHeaders(info.token) : {};
     const req = transport.request(
       {
@@ -380,9 +378,9 @@ async function sendHttpRequest(
   if (info.baseUrl) {
     Object.assign(headers, buildDaemonHttpAuthHeaders(info.token));
   }
+  const transport = await loadNodeHttpRequester(rpcUrl.protocol);
 
   return await new Promise((resolve, reject) => {
-    const transport = rpcUrl.protocol === 'https:' ? https : http;
     const request = transport.request(
       {
         protocol: rpcUrl.protocol,

src/remote/daemon-artifacts.ts

@@ -1,9 +1,8 @@
 import fs from 'node:fs';
-import http from 'node:http';
-import https from 'node:https';
 import path from 'node:path';
 import { pipeline } from 'node:stream/promises';
 import { AppError } from '@agent-device/kernel/errors';
+import { loadNodeHttpRequester } from '../utils/node-http.ts';
 import type { DaemonArtifact, DaemonRequest, DaemonResponse } from '../daemon/types.ts';
 import {
   buildDaemonHttpAuthHeaders,
@@ -349,7 +348,9 @@ type DownloadRemoteArtifactParams = {
 
 export async function downloadRemoteArtifact(params: DownloadRemoteArtifactParams): Promise<void> {
   const artifactUrl = new URL(buildDaemonArtifactUrl(params.baseUrl, params.artifactId));
-  const transport = artifactUrl.protocol === 'https:' ? https : http;
+  // `prepareRemoteRequestArtifacts` runs on every CLI request, but only a
+  // remote daemon ever downloads an artifact, so the HTTP stack loads here.
+  const transport = await loadNodeHttpRequester(artifactUrl.protocol);
   await fs.promises.mkdir(path.dirname(params.destinationPath), { recursive: true });
   await new Promise<void>((resolve, reject) => {
     let settled = false;

src/remote/upload-stream.ts

@@ -1,10 +1,9 @@
 import fs from 'node:fs';
-import http, { type IncomingHttpHeaders } from 'node:http';
-import https from 'node:https';
+import type { IncomingHttpHeaders } from 'node:http';
 import path from 'node:path';
 import { pipeline } from 'node:stream/promises';
 import { AppError } from '@agent-device/kernel/errors';
-import { readNodeHttpResponseBody } from '../utils/node-http.ts';
+import { loadNodeHttpRequester, readNodeHttpResponseBody } from '../utils/node-http.ts';
 import {
   createUploadProgressTransform,
   type UploadProgressSink,
@@ -65,7 +64,10 @@ async function streamFileToHttpRequestAttempt(options: {
   startOffset: number;
   progress?: UploadStreamProgressOptions;
 }): Promise<UploadStreamResponse> {
-  const transport = options.url.protocol === 'https:' ? https : http;
+  // This module sits in the CLI's eager import closure via the remote-artifact
+  // upload client, but only a remote daemon ever uploads, so the HTTP stack
+  // loads here rather than at import time.
+  const transport = await loadNodeHttpRequester(options.url.protocol);
   const payloadSize = fs.statSync(options.payloadPath).size;
   const headers = buildUploadRequestHeaders(options.headers, options.startOffset, payloadSize);
   emitUploadAttemptStarted(options.progress, options.startOffset, payloadSize);