Skip to content

bridge: RQ-STACK-CLI-COLDSTART - #130

Merged
masonwyatt23 merged 1 commit into
ashlrai:mainfrom
masonwyatt23:codex-bridge/RQ-STACK-CLI-COLDSTART
Oct 27, 2026
Merged

masonwyatt23 merged 1 commit into
ashlrai:mainfrom
masonwyatt23:codex-bridge/RQ-STACK-CLI-COLDSTART

Conversation

@masonwyatt23

@masonwyatt23 masonwyatt23 commented Oct 26, 2026 •

Copy link
Copy Markdown
Member

Automated bridge job RQ-STACK-CLI-COLDSTART (pool: codex-personal), single commit on base c3da770f2.

Tests: no test command

Tier-1 (CODEOWNERS) paths, async post-merge review: packages/cli/src/tests/run-cli.test.ts packages/cli/src/index.ts

Agent notes

RQ-STACK-CLI-COLDSTART

Implemented lazy resolvers for all 27 CLI command modules in packages/cli/src/index.ts.
Removed the eager command imports. Existing citty dispatch and argument validation resolve
only the selected command; root help can still resolve command metadata normally.

Added subprocess regression coverage for --version, -v, and --version --json.
A Bun preload plugin rejects command-module loads, and a selected list command
proves that the guard actually intercepts those modules. Added a 5,000 ms startup
ceiling including process creation, module loading, and successful version output.
The guard test permits 20 seconds for its four subprocesses on shared runners.

PR-ready description

Load CLI commands only when selected so version requests skip all command modules.
Preserve command dispatch, nested commands, root help, and error handling.

Measured with Bun 1.3.14 after bun install --frozen-lockfile: 20 fresh subprocesses
per version, one discarded warmup per version, temporary HOME, update checking and
telemetry disabled. Original source was read from HEAD into a temporary sibling
entry point and removed after measurement. Source CLI invocation: bun <entry> --version.

Source Median Minimum Maximum
Before 1,413.22 ms 909.30 ms 2,740.33 ms
After 758.11 ms 349.06 ms 1,248.13 ms

Median improvement: 46.4%. These are local elapsed startup measurements under
shared machine load, not filesystem-cache-flushed measurements. Earlier runs
before dependency installation were discarded because runtime package resolution
made the comparison unreliable.

Validation

All test runs used temporary HOME directories. No credentials were needed.

  • bun install --frozen-lockfile: passed, 1,199 packages installed, lockfile unchanged.

  • HOME=$(mktemp -d) bun test packages/cli/src/__tests__/run-cli.test.ts: 7 pass, 0 fail, 30 assertions.

  • HOME=$(mktemp -d) bun run typecheck: passed after locked dependency installation.

  • HOME=$(mktemp -d) bun run lint: passed, 253 files checked after restoring generated site churn.

  • HOME=$(mktemp -d) bun run --cwd packages/cli build: passed, 142 modules bundled.

  • Built Node bundle smoke checks: --version and templates list --json passed.

  • git diff --check: passed.

  • HOME=$(mktemp -d) TMPDIR=$(realpath "${TMPDIR:-/tmp}") bun test: 968 pass, 10 skip, 0 fail, 7,645 assertions across 109 files (978 tests), 309.71 seconds.

  • No test was weakened, skipped, or deleted by this change; the 10 skips already exist in the suite.

Earlier validation attempts are retained here for completeness:

  • Before dependencies were installed, typecheck failed due to missing Bun types and an unpinned fallback TypeScript; the first suite attempt ended with 635 pass, 10 skip, 67 fail, and 37 errors. The focused run also timed out during runtime package resolution.
  • The first dependency-installed full run ended with 966 pass, 10 skip, 2 fail: a core test compared the macOS /var alias to /private/var, and a doctor integration test exceeded its existing timeout while another suite was running. Canonicalizing TMPDIR and letting the suite finish yielded the green final run above.
  • An intermediate lint check failed on site files regenerated by the suite. Those unrelated generated changes were restored, then lint passed.

Diff accounting

Product and test changes: 74 added lines, 57 removed lines, net +17 LOC.
Bridge notes: 63 added lines. Total: 137 additions, 57 deletions, net +80 LOC across three files.

Delivery

Changes remain uncommitted. No commit, push, PR, tag, or publication was performed,
as explicitly instructed. scripts/publish.sh publishes npm packages and tags;
it was inspected but not run. No .github/, CODEOWNERS, release configuration,
or secrets were changed. Site build output generated by tests is restored.

Summary by CodeRabbit

  • Improvements
    • The CLI now loads commands only when selected, helping version checks start without loading command functionality.
    • Plain version output completes in under five seconds in cold-start testing.

@masonwyatt23
masonwyatt23 enabled auto-merge (squash) October 26, 2026 23:41
@vercel

vercel Bot commented Oct 26, 2026

Copy link
Copy Markdown

@masonwyatt23 is attempting to deploy a commit to the Evero Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai

coderabbitai Bot commented Oct 26, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between 7f6726b and 8575426.

Files selected for processing: 2
  • packages/cli/src/__tests__/run-cli.test.ts
  • packages/cli/src/index.ts

Limit details: You’ve used all 10 included reviews currently available.

Walkthrough

The CLI now dynamically imports subcommand modules when selected instead of importing them eagerly. New tests check version command cold starts, loading behavior for list, and plain version startup time.

Changes: CLI lazy loading

Layer / File(s) Summary
Lazy command loading and cold-start tests
packages/cli/src/index.ts, packages/cli/src/__tests__/run-cli.test.ts
Subcommand registrations now dynamically import command modules when selected. Tests check that version commands do not load command modules, that selecting list does, and that plain version startup takes less than five seconds.

Priority: Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Refactor

Possibly related PRs

  • ashlrai/ashlr-stack#119: Introduced lazy command loaders and rewired CLI dispatch, which is directly related to this CLI command-loading change.
Merge Risk: Minimal · up to 85754

No actionable issue remains from the reviewed change; it is mergeable after normal checks.

Security architecture risk: Minimal · up to 85754

The change defers loading existing commands without expanding command selection or weakening the inspected security checks. No material security risk introduced or worsened by this change was identified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The reviewed change affects command-loading timing within the existing local CLI process. It does not add command targets or grant additional authority to the inspected handlers; their existing privileged capabilities remain reachable through the same command registry.

Trust Boundaries and Controls

  • observed — User-supplied arguments select registry entries, while the new module specifiers remain source literals. The change does not create an argument-to-import-path interpolation path, and unknown-flag rejection remains before ordinary handler execution.

Resilience and Maintainability Implications

  • inferred — The changed resolver stage loads command definitions rather than performing the handlers' persistent operations. Failure during preliminary resolution stops dispatch before those operations begin, so this change does not introduce a new application-state rollback or recovery protocol.
Pre-merge checks: Passed 3 · Failed 1 · Inconclusive 1

Failed Failed checks: 1 warning · 1 inconclusive

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check Inconclusive The title identifies the bridge job but does not describe the main change: lazy CLI command loading and faster cold starts. Replace the title with a concise description of the change, such as "Load CLI commands lazily to reduce version startup time".
Passed Passed checks: 3 passed
Check name Status Explanation
Description check Passed The description provides detailed change rationale, validation results, performance measurements, and reviewer notes. It does not use the template headings or explicitly document checklist status, but…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

The description provides detailed change rationale, validation results, performance measurements, and reviewer notes. It does not use the template headings or explicitly document checklist status, but it contains most required information.

Finishing Touches: 💡 1
🧪 Generate unit tests (beta)
  • Create a new PR
🛠️ Fix failing CI checks: 💡
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Usage-based review receipt

  • Mode: Continue automatically
  • Reviewed files: 2
  • Waived: $0.50 (charged $0.00)
  • View usage details

Note

This review exceeded your plan’s limits and used usage-based reviews—free during trial, billed after paid activation unless disabled. Manage usage-based reviews.

Comment @coderabbitai help to get the list of available commands.

@masonwyatt23
masonwyatt23 merged commit 62177b3 into ashlrai:main Oct 27, 2026
5 of 6 checks passed
@masonwyatt23

Copy link
Copy Markdown
Member Author

Post-merge Tier-1 security review: no issues.

  • packages/cli/src/index.ts:71-98: the lazy resolvers map the same 27 commands to the same modules and exports as the removed eager imports. Specifiers are string literals, so there is no user-controlled module path.
  • Dispatch, telemetry and update-check wiring are unchanged (index.ts:2-8, 116-117).
  • run-cli.test.ts:81-122: the preload guard lives in a mkdtemp dir and the child env stays minimal.

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.

1 participant