perf: lazy load daemon handlers and report bundle size#608
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 97f6f45227
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| #!/usr/bin/env node | ||
| import fs from 'node:fs'; | ||
| import path from 'node:path'; | ||
| import { spawnSync } from 'node:child_process'; |
There was a problem hiding this comment.
Use the approved exec helper path for process execution
This new script imports spawnSync directly, but the repo's AGENTS.md Hard Rules say not to import raw spawn/spawnSync outside src/utils/exec.ts, and that plain .mjs scripts should prefer execFile/execFileSync instead. Keeping this as a direct spawn creates a second process-execution path outside the repo's audited helper conventions; switch this npm invocation to execFileSync/execFile or route it through an approved helper.
Useful? React with 👍 / 👎.
97f6f45 to
320343a
Compare
|
Size Report
Startup median (7 runs, lower is better):
Top changed chunks:
|
eb26072 to
dc9296c
Compare
dc9296c to
bb10697
Compare
Summary
Lazy-load daemon handler, platform, and command execution boundaries behind command-catalog-owned routing groups, keeping request admission, locking, provider scoping, and generic platform fallback unchanged.
Split command metadata from executable command/runtime modules so CLI help and MCP
tools/listcan enumerate command schemas without pulling daemon projection/runtime code.Add a lightweight size reporting script and same-repo PR workflow that reports raw JS, gzip JS, npm tarball/unpacked size, and top changed chunks as a PR comment.
Review follow-up:
runtimenow uses a command-catalog constant, matched handlers returningnullraise an internal routing mismatch, the handler-chain test covers a session-family command, and the size script usesexecFileSyncfor npm dry-run execution.Scope: 64 source files touched. Scope expanded from daemon routing into command metadata and provider seams to keep CLI, MCP, and Node.js boundaries thin.
Total Benefits
Cold CLI startup paths from the initial lazy-routing pass, measured against the PR merge-base (
5083d0456) and PR head (8582d0d02) from builtdistoutput on the same local machine. Each row is a freshnode bin/agent-device.mjs ...process, 35 measured runs after 5 warmups.--version--helphelphelp openopen --helphelp snapshotsnapshot --helpFollow-up optimization pass measured against the previous PR head before the command metadata/platform split:
--version--helphelp snapshottools/listDependency graph impact from dependency-cruiser:
command-metadata.tsstatic closuremcp/command-tools.tsstatic closureutils/args.tsstatic closureSize tradeoff versus the pre-lazy baseline:
This is a startup/runtime-boundary win rather than a total package-size win. The package grows slightly because previously shared code is split into lazy chunks, but hot CLI/MCP paths load less work.
Validation
Verified with
pnpm format,pnpm check:quick,pnpm check:fallow --base origin/main,pnpm build, dependency-cruiser, focused request-handler-chain Vitest, focused batch/MCP/help Vitest, focused provider Vitest, smoke CLI tests, andpnpm size:markdown.Previous validation on this PR also passed
pnpm check:mcp-metadata.pnpm check:unitstill fails in this sandbox on pre-existing environment-shaped failures: local serverlisten EPERM, Swift script typecheck exits, and one child-process replay test.