From 3bfbc37dbf37095816eda500f3f1e05cb98bb95b Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Thu, 8 Oct 2026 16:39:48 -0700 Subject: [PATCH] fix(desktop): the t3 command warns instead of installing behind another t3 (#17351) Co-authored-by: Claude Opus 5.5 --- .../desktop/src/app/DesktopCliCommand.test.ts | 32 ++++++++++++++----- apps/desktop/src/app/DesktopCliCommand.ts | 23 +++++++++++-- .../settings/CliCommandSettingsRow.tsx | 12 ++++--- packages/contracts/src/ipc.ts | 5 +++ 4 files changed, 57 insertions(+), 15 deletions(-) diff --git a/apps/desktop/src/app/DesktopCliCommand.test.ts b/apps/desktop/src/app/DesktopCliCommand.test.ts index dc2e7fb962d18e3651c4dd8000b63853d0c9eff7..b2305fe9840ac841dd90101129138ab7bcf73522 100644 --- a/apps/desktop/src/app/DesktopCliCommand.test.ts +++ b/apps/desktop/src/app/DesktopCliCommand.test.ts @@ -1,5 +1,5 @@ import * as NodeServices from "@effect/platform-node/NodeServices"; -import { expect, it } from "@effect/vitest"; +import { afterEach, beforeEach, expect, it } from "@effect/vitest"; import * as Effect from "effect/Effect"; import * as FileSystem from "effect/FileSystem"; import * as Option from "effect/Option"; @@ -85,6 +85,15 @@ const fakePowerShell = (initial: string) => { return { registry, spawner }; }; +// The machine's own PATH may already hold a t3; each test sets the PATH it means. +const machinePath = process.env.PATH; +beforeEach(() => { + process.env.PATH = ""; +}); +afterEach(() => { + process.env.PATH = machinePath; +}); + it.layer(NodeServices.layer)("DesktopCliCommand", (it) => { it.effect("links the launcher onto PATH and removes only that link", () => Effect.gen(function* () { @@ -165,7 +174,7 @@ it.layer(NodeServices.layer)("DesktopCliCommand", (it) => { }).pipe(Effect.scoped), ); - it.effect("reports when another t3 earlier on PATH would run instead", () => + it.effect("refuses to install behind another t3 that runs first", () => Effect.gen(function* () { const fs = yield* FileSystem.FileSystem; const path = yield* Path.Path; @@ -173,16 +182,23 @@ it.layer(NodeServices.layer)("DesktopCliCommand", (it) => { const shadow = path.join(home, "shadow"); yield* fs.makeDirectory(shadow); yield* fs.writeFileString(path.join(shadow, "t3"), "#!/bin/sh\n", { mode: 0o755 }); - const previous = process.env.PATH; process.env.PATH = [shadow, path.join(home, ".local", "bin")].join(":"); - yield* Effect.addFinalizer(() => Effect.sync(() => (process.env.PATH = previous))); const command = yield* commandIn({ home }); + const theirs = path.join(shadow, "t3"); + expect((yield* command.state).shadowedBy).toBe(theirs); + // A link behind it would never run, so nothing is created. + const error = yield* Effect.flip(command.install); + expect(error.message).toContain(theirs); + expect(yield* fs.exists(path.join(home, ".local", "bin", "t3"))).toBe(false); + + yield* fs.remove(theirs); const installed = yield* command.install; - expect(installed.installedPath).toBe(path.join(home, ".local", "bin", "t3")); - expect(installed.onPath).toBe(false); - yield* fs.remove(path.join(shadow, "t3")); - expect((yield* command.state).onPath).toBe(true); + expect(installed).toMatchObject({ + onPath: true, + installedPath: path.join(home, ".local", "bin", "t3"), + }); + expect(installed.shadowedBy).toBeUndefined(); }).pipe(Effect.scoped), ); diff --git a/apps/desktop/src/app/DesktopCliCommand.ts b/apps/desktop/src/app/DesktopCliCommand.ts index 534af3716db7aaadb93db2ab9fc2d49ff0f08580..d79cab92f7ccc495fd08740b3377b007f415f066 100644 --- a/apps/desktop/src/app/DesktopCliCommand.ts +++ b/apps/desktop/src/app/DesktopCliCommand.ts @@ -146,6 +146,14 @@ export const make = Effect.gen(function* () { return Option.none(); }); + /** The `t3` a new shell runs when it is not this app's, on Unix. */ + const foreignFirstOnPath = Effect.gen(function* () { + if (windows) return Option.none(); + const first = yield* firstOnPath; + if (Option.isNone(first) || (yield* isOurLink(first.value))) return Option.none(); + return first; + }); + /** Where this app's command is installed now, if anywhere. */ const installedAt = Effect.gen(function* () { if (windows) { @@ -167,12 +175,16 @@ export const make = Effect.gen(function* () { return { supported: false, installedPath: null, onPath: false } as const; } const installed = yield* installedAt; - if (Option.isNone(installed)) return { supported: true, installedPath: null, onPath: false }; + const shadowedBy = yield* foreignFirstOnPath; + const shadow = Option.isSome(shadowedBy) ? { shadowedBy: shadowedBy.value } : {}; + if (Option.isNone(installed)) { + return { supported: true, installedPath: null, onPath: false, ...shadow }; + } // On Windows only terminals opened after the change see it. On Unix the // first `t3` on PATH must be ours; a `t3` earlier on PATH would shadow it. const first = yield* firstOnPath; const onPath = windows || (Option.isSome(first) && (yield* isOurLink(first.value))); - return { supported: true, installedPath: installed.value, onPath }; + return { supported: true, installedPath: installed.value, onPath, ...shadow }; }).pipe(Effect.orElseSucceed(() => ({ supported: false, installedPath: null, onPath: false }))); /** Writes the launcher if the app has not yet, e.g. when no local backend runs. */ @@ -209,6 +221,13 @@ export const make = Effect.gen(function* () { .remove(existing.value) .pipe(Effect.mapError(() => fail(`Could not replace ${existing.value}.`))); } + // A link behind another `t3` never runs, so installing one would only hide the problem. + const shadowedBy = yield* foreignFirstOnPath; + if (Option.isSome(shadowedBy)) { + return yield* fail( + `Another t3 at ${shadowedBy.value} runs first in a new terminal. Remove it, or run the launcher directly at ${launcher}.`, + ); + } const onPath = pathEntries(process.env.PATH, ":"); const candidates = unixCandidates(environment.homeDirectory, environment.platform); // Prefer a folder already on PATH that the user can write to without admin rights. diff --git a/apps/web/src/components/settings/CliCommandSettingsRow.tsx b/apps/web/src/components/settings/CliCommandSettingsRow.tsx index f7c12345d7db30f8b0276b81212b3bc5fd1fc702..6940a5bf39533c56170d02de665d35a94f24b769 100644 --- a/apps/web/src/components/settings/CliCommandSettingsRow.tsx +++ b/apps/web/src/components/settings/CliCommandSettingsRow.tsx @@ -51,11 +51,13 @@ export function CliCommandSettingsRow() { if (!bridge || !state?.supported) return null; const installed = state.installedPath !== null; - const description = !installed - ? "Run T3 Code's CLI as `t3` from any terminal." - : state.onPath - ? `Installed at ${state.installedPath}. Open a new terminal to use it.` - : `Installed at ${state.installedPath}, which is not on your PATH yet. Add its folder to your PATH to run \`t3\`.`; + const description = state.shadowedBy + ? `Another t3 at ${state.shadowedBy} runs first in a new terminal. Remove it to use T3 Code's.` + : !installed + ? "Run T3 Code's CLI as `t3` from any terminal." + : state.onPath + ? `Installed at ${state.installedPath}. Open a new terminal to use it.` + : `Installed at ${state.installedPath}, which is not on your PATH yet. Add its folder to your PATH to run \`t3\`.`; return (