Skip to content

fix(server): the suggested t3 command names this install, not another t3 on PATH - #22639

Open
Gourav224 wants to merge 2 commits into
pingdotgg:mainfrom
Gourav224:fix/cli-command-names-own-install
Open

Gourav224 wants to merge 2 commits into
pingdotgg:mainfrom
Gourav224:fix/cli-command-names-own-install

Conversation

@Gourav224

Copy link
Copy Markdown
Contributor

Fixes the "wrong t3 in the suggested command" half of #22626. The onboarding step it also asks for is a new-behaviour idea and is not part of this PR.

resolveHostCliCommand returned a bare t3 whenever any t3 was on PATH, without asking whether it was this install's. On a machine with the desktop app and an older npm global, the AppArmor message told the agent to run sudo env "PATH=$PATH" t3 browser setup, which runs the older install's CLI.

When the app names its launcher (T3CODE_CLI_PATH), a t3 on PATH now counts as this install's only if it resolves to that launcher, which is true for the link the app installs onto PATH. Any other t3 is passed over for the launcher's absolute path. Without a launcher to compare against (npm global, standalone, source) nothing changes.

The existing launcher test asserted the old behaviour (an unrelated t3 on PATH wins); it now expects the launcher, and a new case covers a symlink to the launcher staying plain t3. That test fails without the change and passes with it.

Two other tests in invocation.test.ts ("requires the npm prefix's bin…", "proves the downloaded executable…") fail on macOS before and after this change because the temp directory sits behind a /var symlink; I didn't touch them. Not exercised on a real Ubuntu desktop install.

🤖 Generated with Claude Code

… t3 on PATH

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@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 Oct 21, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 21, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 85c3e95

Macroscope's review found this PR approvable — This focused bug fix ensures generated t3 commands target the owning installation instead of an unrelated executable on PATH, while preserving the existing behavior for valid PATH links and launcher-less installs. The accompanying tests cover the changed cases, and no broader product or infrastructure behavior is introduced.

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

@coderabbitai

coderabbitai Bot commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Warning

Review limit reached

Only developers with an assigned seat can use this organization's usage-based review budget, and seats here are assigned manually. Ask an admin to assign a seat, or change the review continuation mode in Billing.

Next included review available in 33 minutes.

Check out review usage here.

View limit details

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

Learn how review limits work.

Review configuration:

Run configuration
Commits

Reviewing files that changed from the base of the PR and between 85c3e95 and 0f8639e.

Files selected for processing: 2
  • apps/server/src/cli/invocation.test.ts
  • apps/server/src/cli/invocation.ts

No actionable comments were generated in the recent review.

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Path: .coderabbit.config.ts
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 018f1172-6756-459e-8011-a20aad5896ba
📥 Commits

Reviewing files that changed from the base of the PR and between 8a9d0e0 and 85c3e95.

📒 Files selected for processing: 2
  • apps/server/src/cli/invocation.test.ts
  • apps/server/src/cli/invocation.ts

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

Walkthrough

The host CLI command resolver now checks whether a PATH-provided t3 resolves to a different installation than the desktop launcher. Tests cover both an unrelated PATH command and a symlink to the desktop launcher.

Changes: Host CLI command resolution

Layer / File(s) Summary
Resolve and select the host command
apps/server/src/cli/invocation.ts, apps/server/src/cli/invocation.test.ts
The resolver compares the resolved PATH command with the desktop launcher. It uses the launcher when PATH points to a different installation, and uses t3 when the paths match or the installation cannot be identified. Tests cover an unrelated PATH command and a symlink to the launcher.

Priority: Low

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

Change: Bug fix

Suggested reviewers: juliusmarminge

Merge Risk: Minimal · up to 85c3e

The change makes the suggested t3 command point at this installation instead of an unrelated t3 on PATH. No actionable merge-blocking risk was found.

Architecture risk: Medium · up to 85c3e

The change affects 1 system.

Changed systems: apps/server

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — apps/server (service) was modified; 2 changed files map to changed impact.

Before / after behavior

  • observed — Modified behavior in apps/server/src/cli/invocation.test.ts: Replaced the prior PATH-based launcher expectation with tests confirming that an unrelated t3 preserves the desktop shim, while a symlink to that shim selects plain t3.
  • observed — Modified behavior in apps/server/src/cli/invocation.ts: Replaced the shell availability import with resolveCommandPath.
  • observed — Modified behavior in apps/server/src/cli/invocation.ts: Added pathCommandIsOtherInstall, which resolves the configured shim and PATH command and identifies a different install when both real paths exist and differ. Updated the host-command documentation to include this install-ownership check.
  • observed — Modified behavior in apps/server/src/cli/invocation.ts: Replaced the unconditional isCommandAvailable("t3") check with PATH resolution plus rejection of a different install; matching or unidentifiable installs continue using the PATH command.

Reliability and maintainability

  • inferred — Risk-relevant change factors for apps/server: blast_radius_1; direct_dependents_1
Pre-merge checks: Passed 4
Check name Status Explanation
Title check Passed The title clearly and concisely describes the primary fix: preventing the suggested t3 command from selecting another installation on PATH.
Description check Passed The description explains the problem, implementation, tests, known macOS limitations, and untested Ubuntu installation. It references issue #22626, but it does not provide explicit maintainer approval…
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 explains the problem, implementation, tests, known macOS limitations, and untested Ubuntu installation. It references issue #22626, but it does not provide explicit maintainer approval or explain the small-fix exemption in detail.

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

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

Copy link
Copy Markdown
Collaborator

Note

Written by gpt-6.1-sol on behalf of Maria

kept @Gourav224's implementation and regression coverage, merged current main, and resolved the conflict. inlined the realpath comparison into the existing resolver: the final diff is two files, 20 additions and 3 deletions. matching symlinks still use plain t3; a different desktop install uses its quoted launcher. runner precedence and launcher-less fallbacks stay unchanged.

verification:

  • the contributor's regression fails against main's resolver with the exact wrong sudo env "PATH=$PATH" t3 browser setup suggestion.
  • blacksmith: 9 focused invocation/browser-diagnostic tests pass. vp run check:changed passes format, lint, both knip passes, graphql checks, affected typechecks, and invocation tests.
  • a local runtime probe with real files confirms foreign-PATH fallback, matching symlinks, and launcher paths containing spaces and an apostrophe. the actual browser setup CLI also runs successfully on this host.
  • two independent read-only reviewers approved 0f8639e4ff935b88e1ce454bba463a85307c08f3 with no findings.

unverified: the native desktop AppArmor error screen, macOS, and native Windows execution. this VM launches sandboxed Chrome successfully, so it does not reproduce that error screen; no before/after UI captures are claimed. CodeRabbit's automatic review was rate limited on this head; its earlier review covered the contributor's original commit.

ci: all current-head checks passed or were skipped, including build/typecheck, lint, general tests, all eight server shards, and the final Check gate. no new actionable bot findings. no labels were changed manually, and no merge was performed.

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