Skip to content

perf(server): skip CLI loading for version probes - #29246

Open
voltcrash wants to merge 2 commits into
pingdotgg:mainfrom
voltcrash:voltcrash/cli-version-fast-path
Open

voltcrash wants to merge 2 commits into
pingdotgg:mainfrom
voltcrash:voltcrash/cli-version-fast-path

Conversation

@voltcrash

Copy link
Copy Markdown
Contributor

Problem

SSH setup runs t3 --version to probe the remote binary, but the query evaluates the full CLI graph before printing a version. On Linux arm64, this loads 123 file modules totaling 8.46 MB for a metadata query.

Change

Exact top-level --version prints the existing packaged version before loading that graph. Terminal styling, NO_COLOR, output text, and exit status are preserved. Help, extra arguments, and subcommands retain normal CLI dispatch. SEA still parses its single bundle; its version probe skips command-graph evaluation.

Scope and approval

This is a small, focused performance fix for unnecessary work in an existing SSH setup probe, qualifying for the focused-fix exception without a prior issue. It changes only entrypoint dispatch and its subprocess regression coverage. The version probe validates binary startup/version and intentionally stops evaluating the command graph.

Verification

  • The subprocess guard regression failed before the fix with CLI_GRAPH_LOADED. Final vp run check:changed --concurrency 1 passed, including subprocess coverage for exact output, TTY/NO_COLOR, and argument fallthrough. A TTY byte mismatch was reproduced before preserving its ANSI output.
  • vp pack passed. The rebuilt Node bundle reports its embedded version after relocation without dependencies beside a deliberately different package manifest.
  • Linux arm64, 16 KiB pages: nine alternating Node-distribution runs with compile caching disabled had medians of 250 → 23 ms wall time, 330 → 10 ms CPU, and 149 → 48 MiB peak RSS. File modules loaded fell from 123 (8.46 MB) to 4 (5.7 KB). These are local observations, not a cross-platform speedup guarantee.
  • Native arm64 SEA built with the repository's pinned Node 26.12.1 and ran from an isolated directory: stdout t3 v0.1.6\n, empty stderr, exit 0, on aarch64 with 16 KiB pages. x86_64 execution remains pending CI/maintainer verification.

Reviewed with review-loop: 2 completed rounds, Claude Opus 5.5 on high and GPT 6.1 Sol on xhigh. Reviewer verdict: approved. The final round used GPT Sol at the user's request after Claude quota failures returned no verdict.

Created with GPT 6.1 Sol in Codex.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:S 10-29 changed lines (additions + deletions). labels Nov 2, 2026
@macroscopeapp

macroscopeapp Bot commented Nov 2, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 4c765cd

Macroscope's review found this PR approvable — Exact t3 --version probes now print the packaged version without loading the full CLI graph, while other invocations retain their existing dispatch. The limited metadata-only behavior and reported subprocess and packaging verification indicate low material risk.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

The server now prints the package version for an exact top-level --version invocation before loading the CLI graph. A subprocess test checks output across TTY and color settings, and verifies that other argument forms continue to load the CLI graph.

Changes: CLI version handling

Layer / File(s) Summary
Version fast path and argument-boundary tests
apps/server/src/binMain.ts, apps/server/src/cliCompileCache.test.ts
binMain.ts prints the package version for an exact top-level --version argument before the remaining CLI setup. The subprocess test checks plain and TTY output, NO_COLOR, and argument forms that load the CLI graph.

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

Change: Refactor

Possibly related PRs

  • pingdotgg/t3code#27433: This PR changes the normal CLI path that loads the command graph, which the version fast path bypasses.
  • pingdotgg/t3code#29284: This PR moves CLI dispatch into runCliCommand in binMain.ts, where the version fast path runs.

Suggested reviewers: maria-rcks

Merge Risk: Minimal · up to 3f1b7

The version fast path is mergeable after normal checks; no material issue remains established.

Pre-merge checks: Passed 4

Check name Status Explanation
Title check Passed The title clearly describes the version probe fast path.
Description check Passed The description covers the problem, change, scope, approval exception, verification results, and pending x86_64 SEA verification. It also identifies the model and harness used.
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.
Finishing Touches

Generate unit tests (beta)

  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Recent review info

Run configuration

Commits

Reviewing files that changed from the base of the PR and between 4c765cd and 3f1b7f9.

Files selected for processing: 2

  • apps/server/src/binMain.ts
  • apps/server/src/cliCompileCache.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 6 remain after this review.

@maria-rcks

maria-rcks commented Nov 2, 2026 •

Copy link
Copy Markdown
Collaborator

Note

Written by claude-opus-5-5 on behalf of Maria

Merged main (3f1b7f964b)

main moved the fast paths out of bin.ts into binMain.ts in #29284, which conflicted with this pr. i took main's bin.ts unchanged and moved the --version branch to the top of runCliCommand in binMain.ts. it returns before the socket guard and every other import. the version test sits beside main's renamed cache test in cliCompileCache.test.ts. no other changes.

  • node src/bin.ts --version prints t3 v0.1.7 in 0.03s using 68 MB. --version --help goes through the full cli: 1.09s, 341 MB.
  • blacksmith vp run check:changed against main passed format, lint, knip, graphql, typecheck (3 packages), and tests (2 files, including both cli cache tests).

earlier verification of 4c765cd780, before the merge:

Verified in a real dev environment

verified unchanged head 4c765cd78081c7c4912292fb610f871c5a27cd8e on linux x86_64 with node v24.22.1.

  • reproduced on github/main (a036489016) in a separate worktree after vp i: exact --version loaded binCli.ts and 1,253 file modules. the pr loaded five file modules and skipped binCli.ts. a transparent node loader hook logged imports without blocking or replacing modules. both printed t3 v0.1.6 and exited 0. this verifies avoided module loading; it makes no latency speedup claim.
  • started main and pr dev servers sequentially in the background. the pr used isolated .t3 state, server port 13773 and web port 5733. created a project pointing at the real repo through the cli, then started a real codex gpt-6.1-sol thread through the running server. playwright with /usr/bin/google-chrome submitted a follow-up in the web client. both turns completed; the provider ran the version command and returned exit status 0.
  • plain output, actual pseudo-terminal ansi bytes, and NO_COLOR=1 output matched main. --version --help still displayed help, serve --version still printed the version, and thread help/start/show worked through normal dispatch.
  • blacksmith vp run check:changed 47f2c8d29e981c7723467efe137ee19e91df924e passed format, lint, knip, graphql checks, affected typechecks, and three tests in two files. five neighboring deferred-import tests also passed. all 31 github ci checks were successful at inspection.

command transcript

real codex thread returns the version and exit status zero

real web client codex turn runs the version command

Not verified

packaged node distribution and sea executables were not built in this pass; execution used the source entrypoint. the full remote ssh download/bootstrap flow was not exercised against a second host. native desktop/mobile, macos/windows, and linux arm64 were not exercised on this linux x86_64 vm. no code changes were needed.

…-fast-path

# Conflicts:
#	apps/server/src/bin.ts
#	apps/server/src/cliCompileCache.test.ts

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:S 10-29 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants