Skip to content

CLI: Load each command's implementation when the command runs - #5495

Merged
chubes4 merged 4 commits into
trunkfrom
cli/lazy-command-handlers
Oct 27, 2026
Merged

chubes4 merged 4 commits into
trunkfrom
cli/lazy-command-handlers

Conversation

@chubes4

@chubes4 chubes4 commented Oct 27, 2026

Copy link
Copy Markdown
Contributor

Related issues

How AI was used in this PR

Claude (via the Pi coding agent in a Roadie session) profiled CLI startup, wrote the change and the test, and measured the before/after timings. I reviewed the diff and the measurements.

Proposed Changes

apps/cli/index.ts statically imported every top-level command module, and each module held the command's whole implementation. So every studio invocation loaded the implementation of every command before running one, including the export, import, push and pull code and its archive, upload and certificate libraries: 1,375 dependency files from 109 packages for studio --version.

Each top-level command now has a .command module that defines the command (name, description, options) and loads the implementation with await import() in its handler, the pattern wp and _events already use. The implementation modules keep their paths and their runCommand exports, so their other callers are unchanged; each one's former handler body is now its exported commandHandler.

  • studio list, start, stop, create and wp load 520 dependency files at startup instead of 1,375.
  • Help output, unknown-command errors and option validation are byte-identical to trunk.
  • A CommandArgs< typeof builder > type keeps each handler typed from its command's options.

Local timings on macOS (median of 8 alternating runs, Node 24):

Command trunk this PR
studio --version 0.40 s 0.21 s
studio list 0.39 s 0.23 s
studio status 0.42 s 0.24 s
studio wp option get blogname 0.67 s 0.48 s
studio stop / start (already in that state) 0.44 s 0.26 s

The saving is a fixed cost per CLI process, so it applies to every command the desktop app and Studio Code run through the CLI.

Testing Instructions

  • npx vitest run --project cli apps/cli/tests/startup-imports.test.ts: loading what index.ts imports statically must not load any command implementation. Making a .command module import its implementation statically fails it.
  • Build the CLI and compare studio --help, studio create --help and studio foo with trunk's output.
  • Time studio list or studio wp option get blogname against trunk.

Pre-merge Checklist

  • Have you checked for TypeScript, React or other console errors?

@wpmobilebot

wpmobilebot commented Oct 27, 2026 •

Copy link
Copy Markdown
Collaborator

📊 Performance Test Results

Comparing 0be02f6 vs trunk

app-size

Metric trunk 0be02f6 Diff Change
App Size (Mac) 954.28 MB 954.28 MB 0.00 MB ⚪ 0.0%

site-editor

Metric trunk 0be02f6 Diff Change
load 1233 ms 1232 ms 1 ms ⚪ 0.0%

site-startup

Metric trunk 0be02f6 Diff Change
siteCreation 5149 ms 5517 ms +368 ms 🔴 7.1%
siteStartup 2371 ms 2377 ms +6 ms ⚪ 0.0%

Results are median values from multiple test runs.

Legend: 🟢 Improvement (faster) | 🔴 Regression (slower) | ⚪ No change (<50ms diff)

chubes4 added a commit that referenced this pull request Oct 27, 2026
…ws CI's time limit (#5499)

## Related issues

- Follows #5314, which added this test.

## How AI was used in this PR

Claude (via the Pi coding agent in a Roadie session) found the Windows
timeouts in CI logs and wrote the change. I reviewed it.

## Proposed Changes

The test from #5314 runs eight concurrent installs of a 400-file
WordPress copy. It takes about 1 s on macOS but sometimes exceeds
Vitest's default 5 s on the Windows Unit Tests agent, which failed
Windows Unit Tests on #5494 (build 24210) and #5495 (build 24211) with
`Test timed out in 5000ms`. Its trunk runs passed, so it is
intermittent.

The size adds no detection. Run 5 times each on macOS against #5314's
fix and against the code before it:

| Size | With the fix | Before the fix |
| --- | --- | --- |
| 8 installs × 400 files | 5/5 pass | 5/5 fail |
| 4 installs × 100 files | 5/5 pass | 5/5 fail |
| 2 installs × 50 files | 5/5 pass | 5/5 fail |

Before the fix, the failing check is the one this test exists for:
`latest` shows `version.php` before all its files are there. This
shrinks the test to two installs of 50 files, which keeps the concurrent
and interrupted-copy paths at about 1/30 of the file work.

## Testing Instructions

- `npx vitest run --project cli
apps/cli/lib/dependency-management/tests/setup-server-files.test.ts`
passes. With `apps/cli/lib/dependency-management/setup.ts` from before
#5314, it fails.
- Windows Unit Tests pass in CI.

## Pre-merge Checklist

- [x] Have you checked for TypeScript, React or other console errors?
@chubes4
chubes4 merged commit 55f2262 into trunk Oct 27, 2026
8 of 12 checks passed
@chubes4
chubes4 deleted the cli/lazy-command-handlers branch October 27, 2026 20:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants