From 2654583a9ffc2bf0e943318b74fe45623dd93e51 Mon Sep 17 00:00:00 2001 From: Theo Browne Date: Sat, 25 Jul 2026 22:12:09 -0700 Subject: [PATCH 01/23] Keep worktree dev state isolated --- .agents/skills/test-t3-app/SKILL.md | 4 +- AGENTS.md | 4 ++ docs/reference/scripts.md | 3 +- packages/shared/package.json | 4 ++ packages/shared/src/devHome.test.ts | 59 +++++++++++++++++++++++++++ packages/shared/src/devHome.ts | 62 +++++++++++++++++++++++++++++ scripts/dev-runner.ts | 18 +++++++-- 7 files changed, 148 insertions(+), 6 deletions(-) create mode 100644 packages/shared/src/devHome.test.ts create mode 100644 packages/shared/src/devHome.ts diff --git a/.agents/skills/test-t3-app/SKILL.md b/.agents/skills/test-t3-app/SKILL.md index b59ed9ddfe8..86a084bca48 100644 --- a/.agents/skills/test-t3-app/SKILL.md +++ b/.agents/skills/test-t3-app/SKILL.md @@ -13,11 +13,13 @@ Use this skill for the web client. For iOS Simulator, Android Emulator, or physi 2. Choose a base directory that belongs only to the current worktree or test: - Use the repository's ignored `.t3` directory for reusable worktree-local state. - Use `mktemp -d /tmp/t3code-test.XXXXXX` for disposable state and retain the printed absolute path. -3. Start the full web stack with `vp run dev --home-dir `. +3. Start the full web stack with `vp run dev`. In a linked worktree it defaults to that worktree's gitignored `.t3`; pass `--home-dir ` only when the test needs a different isolated directory. 4. Keep the terminal session alive and read the selected server port, web port, base directory, and pairing URL from its output. Treat a base directory as disposable only when it was created or deliberately selected for the current test. Never delete or directly seed the shared `~/.t3` directory. Prefer starting with a new temporary base directory over clearing state of uncertain ownership. +The worktree-local default deliberately outranks an ambient `T3CODE_HOME`; do not pass the shared home through to a worktree dev server. + The dev runner disables browser auto-open by default. Do not pass `--browser` during automated testing: an automatically opened page can consume the one-time bootstrap token before the controlled browser uses it. ## Preserve the environment while iterating diff --git a/AGENTS.md b/AGENTS.md index ef69591a340..7b67ebb7bd5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -13,6 +13,10 @@ - Subagents must not independently launch dev servers or repeat integrated client verification unless their delegated task explicitly requires it. - Stop dev servers, watchers, and other long-running verification processes when the focused verification is complete. +## Dev Servers + +- In a linked git worktree, dev state defaults to that worktree's gitignored `.t3`. This deliberately outranks an ambient `T3CODE_HOME`, which could otherwise select the installed app's live `~/.t3/userdata` database. An explicit `--home-dir` still wins. + ## Package Roles - `apps/server`: Node.js WebSocket server. Wraps Codex app-server (JSON-RPC over stdio), serves the React web app, and manages provider sessions. diff --git a/docs/reference/scripts.md b/docs/reference/scripts.md index 746aa66d563..df6907c2651 100644 --- a/docs/reference/scripts.md +++ b/docs/reference/scripts.md @@ -3,7 +3,8 @@ - `vp run dev` — Starts contracts, server, and web in watch mode. - `vp run dev:server` — Starts just the WebSocket server. The server process runs on Bun (`@effect/platform-bun` + `BunPtyAdapter`), but task running uses `vp run`. - `vp run dev:web` — Starts just the Vite dev server for the web app. -- Dev commands implicitly use `~/.t3/dev`, keeping development state separate from `~/.t3/userdata`. An explicit `--home-dir ` stores state under `/userdata`; the base directory remains available for caches, worktrees, and other shared data. +- Dev commands run from a linked **git worktree** default to that worktree's gitignored `.t3`, even when `T3CODE_HOME` is set. Pass `--home-dir ` to choose another isolated directory explicitly. +- From the **main checkout**, dev commands implicitly use `~/.t3/dev`, keeping development state separate from `~/.t3/userdata`. An explicit `--home-dir ` stores state under `/userdata`; the base directory remains available for caches, worktrees, and other shared data. - Web dev commands do not auto-open a browser. Open the one-time pairing URL printed by the server so the first browser navigation is authenticated. Set `T3CODE_NO_BROWSER=0` only when interactive auto-open is intentional. - Pass dev-runner flags directly after the root task name, for example: `vp run dev --home-dir /tmp/t3code-dev` diff --git a/packages/shared/package.json b/packages/shared/package.json index 7d0cc5eb820..b83ecc53624 100644 --- a/packages/shared/package.json +++ b/packages/shared/package.json @@ -194,6 +194,10 @@ "./httpReadiness": { "types": "./src/httpReadiness.ts", "import": "./src/httpReadiness.ts" + }, + "./devHome": { + "types": "./src/devHome.ts", + "import": "./src/devHome.ts" } }, "scripts": { diff --git a/packages/shared/src/devHome.test.ts b/packages/shared/src/devHome.test.ts new file mode 100644 index 00000000000..5ecf19c3020 --- /dev/null +++ b/packages/shared/src/devHome.test.ts @@ -0,0 +1,59 @@ +// @effect-diagnostics nodeBuiltinImport:off - builds real worktree layouts on disk. +import * as NodeServices from "@effect/platform-node/NodeServices"; +import { assert, describe, it } from "@effect/vitest"; +import * as NodeFS from "node:fs"; +import * as NodeOS from "node:os"; +import * as NodePath from "node:path"; +import * as Effect from "effect/Effect"; + +import { resolveGitWorktreePath, resolveWorktreeT3Home } from "./devHome.ts"; + +const makeRepo = (kind: "worktree" | "checkout" | "bare") => + Effect.acquireRelease( + Effect.sync(() => { + const root = NodeFS.mkdtempSync(NodePath.join(NodeOS.tmpdir(), "t3-devhome-")); + if (kind === "worktree") { + NodeFS.writeFileSync(NodePath.join(root, ".git"), "gitdir: /elsewhere/.git/worktrees/x\n"); + } else if (kind === "checkout") { + NodeFS.mkdirSync(NodePath.join(root, ".git")); + } + const nested = NodePath.join(root, "apps", "web", "src"); + NodeFS.mkdirSync(nested, { recursive: true }); + return { root, nested }; + }), + ({ root }) => Effect.sync(() => NodeFS.rmSync(root, { recursive: true, force: true })), + ); + +describe("resolveGitWorktreePath", () => { + it.effect("finds a worktree root from a nested directory", () => + Effect.gen(function* () { + const { root, nested } = yield* makeRepo("worktree"); + assert.equal(yield* resolveGitWorktreePath(nested), NodePath.resolve(root)); + }).pipe(Effect.scoped, Effect.provide(NodeServices.layer)), + ); + + it.effect("reports a main checkout as not a linked worktree", () => + Effect.gen(function* () { + const { nested } = yield* makeRepo("checkout"); + assert.equal(yield* resolveGitWorktreePath(nested), undefined); + }).pipe(Effect.scoped, Effect.provide(NodeServices.layer)), + ); + + it.effect("reports a directory outside a repository", () => + Effect.gen(function* () { + const { nested } = yield* makeRepo("bare"); + assert.equal(yield* resolveGitWorktreePath(nested), undefined); + }).pipe(Effect.scoped, Effect.provide(NodeServices.layer)), + ); +}); + +describe("resolveWorktreeT3Home", () => { + it.effect("answers with .t3 before the dev runner creates it", () => + Effect.gen(function* () { + const { root, nested } = yield* makeRepo("worktree"); + const home = yield* resolveWorktreeT3Home(nested); + assert.equal(home, NodePath.join(NodePath.resolve(root), ".t3")); + assert.isFalse(NodeFS.existsSync(home ?? "")); + }).pipe(Effect.scoped, Effect.provide(NodeServices.layer)), + ); +}); diff --git a/packages/shared/src/devHome.ts b/packages/shared/src/devHome.ts new file mode 100644 index 00000000000..618b0dc9554 --- /dev/null +++ b/packages/shared/src/devHome.ts @@ -0,0 +1,62 @@ +/** + * Where development state lives, and how to keep it away from the shared + * `~/.t3` that a user's installed T3 Code runs against. + * + * A linked git worktree gets its own (gitignored) `.t3`: feature work in a + * throwaway branch must not share a database with the real app, and an ambient + * `T3CODE_HOME` counts as an explicit base dir — flipping the state directory + * from `/dev` to `/userdata`, the live production database. + */ + +import * as Effect from "effect/Effect"; +import * as FileSystem from "effect/FileSystem"; +import * as Option from "effect/Option"; +import * as Path from "effect/Path"; + +/** + * The path of the linked git worktree containing `cwd`, or undefined when + * `cwd` is not inside one. Git marks a linked worktree by making `.git` a file + * (`gitdir: …`) rather than a directory. + * + * Walks up to the repository root, so running from a subdirectory resolves the + * same worktree as running from the top. + */ +export const resolveGitWorktreePath = ( + cwd: string, +): Effect.Effect => + Effect.gen(function* () { + const fileSystem = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + + let directory = path.resolve(cwd); + for (;;) { + const info = yield* fileSystem.stat(path.join(directory, ".git")).pipe(Effect.option); + if (Option.isSome(info)) { + // A directory means the main checkout. Stop either way: nesting one + // repository inside another does not make the outer one this root. + return info.value.type === "File" ? directory : undefined; + } + const parent = path.dirname(directory); + if (parent === directory) { + return undefined; + } + directory = parent; + } + }); + +/** + * The worktree-local data directory for `cwd`, or undefined outside a linked + * worktree. Deliberately does not require the directory to exist yet: falling + * back because it is missing would send callers at the shared home. + */ +export const resolveWorktreeT3Home = ( + cwd: string, +): Effect.Effect => + Effect.gen(function* () { + const worktreePath = yield* resolveGitWorktreePath(cwd); + if (worktreePath === undefined) { + return undefined; + } + const path = yield* Path.Path; + return path.join(worktreePath, ".t3"); + }); diff --git a/scripts/dev-runner.ts b/scripts/dev-runner.ts index 1938232300f..d6aec7daf36 100644 --- a/scripts/dev-runner.ts +++ b/scripts/dev-runner.ts @@ -5,6 +5,7 @@ import * as NodeOS from "node:os"; import * as NodeRuntime from "@effect/platform-node/NodeRuntime"; import * as NodeServices from "@effect/platform-node/NodeServices"; import * as NetService from "@t3tools/shared/Net"; +import { resolveWorktreeT3Home } from "@t3tools/shared/devHome"; import { HostProcessEnvironment } from "@t3tools/shared/hostProcess"; import { resolveSpawnCommand } from "@t3tools/shared/shell"; import * as Config from "effect/Config"; @@ -245,7 +246,9 @@ export function createDevRunnerEnv({ return Effect.gen(function* () { const serverPort = port ?? BASE_SERVER_PORT + serverOffset; const webPort = BASE_WEB_PORT + webOffset; - const configuredBaseDir = t3Home?.trim() || baseEnv.T3CODE_HOME?.trim() || undefined; + // Precedence is resolved by the caller. An unset t3Home here genuinely + // means "use the default" rather than inheriting an ambient value. + const configuredBaseDir = t3Home?.trim() || undefined; const resolvedBaseDir = yield* resolveBaseDir(configuredBaseDir); const isDesktopMode = mode === "dev:desktop"; @@ -505,12 +508,18 @@ export function runDevRunnerWithInput(input: DevRunnerCliInput) { }); const hostEnvironment = yield* HostProcessEnvironment; + // A worktree defaults to its own gitignored `.t3`. This deliberately + // outranks ambient T3CODE_HOME, which otherwise selects the installed + // app's live userdata database. An explicit --home-dir still wins. + const worktreeHome = yield* resolveWorktreeT3Home(process.cwd()); + const resolvedT3Home = + input.t3Home ?? worktreeHome ?? (hostEnvironment.T3CODE_HOME?.trim() || undefined); const env = yield* createDevRunnerEnv({ mode: input.mode, baseEnv: hostEnvironment, serverOffset, webOffset, - t3Home: input.t3Home, + t3Home: resolvedT3Home, browser: input.browser, autoBootstrapProjectFromCwd: input.autoBootstrapProjectFromCwd, logWebSocketEvents: input.logWebSocketEvents, @@ -592,9 +601,10 @@ const devRunnerCli = Command.make("dev-runner", { ), t3Home: Flag.string("home-dir").pipe( Flag.withDescription( - "Explicit T3 Code data directory; runtime state is stored under userdata (equivalent to T3CODE_HOME).", + "Explicit T3 Code data directory; runtime state is stored under userdata. Inside a git worktree this defaults to that worktree's own .t3.", ), - Flag.withFallbackConfig(optionalStringConfig("T3CODE_HOME")), + Flag.optional, + Flag.map(Option.getOrUndefined), ), browser: Flag.boolean("browser").pipe( Flag.withDescription("Open a browser automatically (disabled by default for web dev)."), From e52d104095ab4e4df380a7c6ad2f7683d29c0694 Mon Sep 17 00:00:00 2001 From: Theo Browne Date: Sat, 25 Jul 2026 22:40:02 -0700 Subject: [PATCH 02/23] Distinguish worktrees from submodules, cover precedence A submodule's `.git` is also a file, so the file-vs-directory check alone classified every submodule as a linked worktree and handed it a worktree-local `.t3`. Read the `gitdir:` pointer and require it to land in `.git/worktrees` (compared as path segments, not a substring). Resolve cwd through a HostProcessWorkingDirectory reference rather than calling process.cwd() directly, so the precedence chain is testable. Add tests for the submodule and malformed-`.git`-file cases, plus end-to-end coverage of all four home-precedence outcomes, which had no test before. Document that worktree state lands in `.t3/userdata`. Co-Authored-By: Claude Opus 5 (1M context) --- .agents/skills/test-t3-app/SKILL.md | 2 +- docs/reference/scripts.md | 2 +- packages/shared/src/devHome.test.ts | 20 +++++- packages/shared/src/devHome.ts | 38 +++++++++- packages/shared/src/hostProcess.ts | 7 ++ scripts/dev-runner.test.ts | 105 +++++++++++++++++++++++++++- scripts/dev-runner.ts | 4 +- 7 files changed, 169 insertions(+), 9 deletions(-) diff --git a/.agents/skills/test-t3-app/SKILL.md b/.agents/skills/test-t3-app/SKILL.md index 86a084bca48..f26270cd06d 100644 --- a/.agents/skills/test-t3-app/SKILL.md +++ b/.agents/skills/test-t3-app/SKILL.md @@ -57,7 +57,7 @@ T3CODE_PORT= node apps/server/src/bin.ts auth pairing create \ Use the `Pair URL` from this command once. Derive `` and `` from the current dev-runner output, including any automatically selected port offset. Setting `T3CODE_PORT` keeps the administrative CLI from probing for an unrelated free port. -Always pass `--dev-url` for a dev-runner environment so the generated pairing URL uses the current web origin. An explicit base directory stores runtime state in `/userdata`; the `/dev` fallback is only used by an implicit dev home. Use `auth pairing list` to inspect active token metadata; it intentionally cannot reveal token secrets. +Always pass `--dev-url` for a dev-runner environment so the generated pairing URL uses the current web origin. An explicit base directory stores runtime state in `/userdata`; the `/dev` fallback is only used by an implicit dev home. A worktree-local `.t3` counts as explicit, so its state lives in `/.t3/userdata`. Use `auth pairing list` to inspect active token metadata; it intentionally cannot reveal token secrets. ## Inspect or seed SQLite state diff --git a/docs/reference/scripts.md b/docs/reference/scripts.md index df6907c2651..b985c9c9e3d 100644 --- a/docs/reference/scripts.md +++ b/docs/reference/scripts.md @@ -3,7 +3,7 @@ - `vp run dev` — Starts contracts, server, and web in watch mode. - `vp run dev:server` — Starts just the WebSocket server. The server process runs on Bun (`@effect/platform-bun` + `BunPtyAdapter`), but task running uses `vp run`. - `vp run dev:web` — Starts just the Vite dev server for the web app. -- Dev commands run from a linked **git worktree** default to that worktree's gitignored `.t3`, even when `T3CODE_HOME` is set. Pass `--home-dir ` to choose another isolated directory explicitly. +- Dev commands run from a linked **git worktree** default to that worktree's gitignored `.t3`, even when `T3CODE_HOME` is set, storing state in `/.t3/userdata`. Pass `--home-dir ` to choose another isolated directory explicitly. Submodules are not worktrees and keep the normal precedence. - From the **main checkout**, dev commands implicitly use `~/.t3/dev`, keeping development state separate from `~/.t3/userdata`. An explicit `--home-dir ` stores state under `/userdata`; the base directory remains available for caches, worktrees, and other shared data. - Web dev commands do not auto-open a browser. Open the one-time pairing URL printed by the server so the first browser navigation is authenticated. Set `T3CODE_NO_BROWSER=0` only when interactive auto-open is intentional. - Pass dev-runner flags directly after the root task name, for example: diff --git a/packages/shared/src/devHome.test.ts b/packages/shared/src/devHome.test.ts index 5ecf19c3020..184768a7a2c 100644 --- a/packages/shared/src/devHome.test.ts +++ b/packages/shared/src/devHome.test.ts @@ -8,12 +8,16 @@ import * as Effect from "effect/Effect"; import { resolveGitWorktreePath, resolveWorktreeT3Home } from "./devHome.ts"; -const makeRepo = (kind: "worktree" | "checkout" | "bare") => +const makeRepo = (kind: "worktree" | "checkout" | "bare" | "submodule" | "unreadable-git-file") => Effect.acquireRelease( Effect.sync(() => { const root = NodeFS.mkdtempSync(NodePath.join(NodeOS.tmpdir(), "t3-devhome-")); if (kind === "worktree") { NodeFS.writeFileSync(NodePath.join(root, ".git"), "gitdir: /elsewhere/.git/worktrees/x\n"); + } else if (kind === "submodule") { + NodeFS.writeFileSync(NodePath.join(root, ".git"), "gitdir: ../.git/modules/sub\n"); + } else if (kind === "unreadable-git-file") { + NodeFS.writeFileSync(NodePath.join(root, ".git"), "not a gitdir pointer\n"); } else if (kind === "checkout") { NodeFS.mkdirSync(NodePath.join(root, ".git")); } @@ -45,6 +49,20 @@ describe("resolveGitWorktreePath", () => { assert.equal(yield* resolveGitWorktreePath(nested), undefined); }).pipe(Effect.scoped, Effect.provide(NodeServices.layer)), ); + + it.effect("reports a submodule as not a linked worktree", () => + Effect.gen(function* () { + const { nested } = yield* makeRepo("submodule"); + assert.equal(yield* resolveGitWorktreePath(nested), undefined); + }).pipe(Effect.scoped, Effect.provide(NodeServices.layer)), + ); + + it.effect("reports a .git file without a usable gitdir pointer", () => + Effect.gen(function* () { + const { nested } = yield* makeRepo("unreadable-git-file"); + assert.equal(yield* resolveGitWorktreePath(nested), undefined); + }).pipe(Effect.scoped, Effect.provide(NodeServices.layer)), + ); }); describe("resolveWorktreeT3Home", () => { diff --git a/packages/shared/src/devHome.ts b/packages/shared/src/devHome.ts index 618b0dc9554..99aabca9f37 100644 --- a/packages/shared/src/devHome.ts +++ b/packages/shared/src/devHome.ts @@ -13,10 +13,33 @@ import * as FileSystem from "effect/FileSystem"; import * as Option from "effect/Option"; import * as Path from "effect/Path"; +/** + * A `.git` file points at the real git directory. A linked worktree's lives + * under `/.git/worktrees/`; a submodule's under + * `/.git/modules/`. Both are files, so the pointer — not the + * file-vs-directory distinction alone — is what identifies a worktree. + */ +const pointsAtLinkedWorktree = (gitFileContents: string, path: Path.Path): boolean => { + const gitdir = gitFileContents + .split(/\r?\n/) + .map((line) => line.trim()) + .find((line) => line.startsWith("gitdir:")) + ?.slice("gitdir:".length) + .trim(); + if (gitdir === undefined || gitdir.length === 0) { + return false; + } + // Normalize so `.git/worktrees` is matched as path segments rather than as a + // substring of some directory that merely happens to be named that way. + const segments = path.normalize(gitdir.replaceAll("\\", "/")).split(/[/\\]/); + const gitIndex = segments.lastIndexOf(".git"); + return gitIndex !== -1 && segments[gitIndex + 1] === "worktrees"; +}; + /** * The path of the linked git worktree containing `cwd`, or undefined when * `cwd` is not inside one. Git marks a linked worktree by making `.git` a file - * (`gitdir: …`) rather than a directory. + * whose `gitdir:` points into the repository's `.git/worktrees`. * * Walks up to the repository root, so running from a subdirectory resolves the * same worktree as running from the top. @@ -30,11 +53,20 @@ export const resolveGitWorktreePath = ( let directory = path.resolve(cwd); for (;;) { - const info = yield* fileSystem.stat(path.join(directory, ".git")).pipe(Effect.option); + const gitPath = path.join(directory, ".git"); + const info = yield* fileSystem.stat(gitPath).pipe(Effect.option); if (Option.isSome(info)) { // A directory means the main checkout. Stop either way: nesting one // repository inside another does not make the outer one this root. - return info.value.type === "File" ? directory : undefined; + if (info.value.type !== "File") { + return undefined; + } + // A submodule also has a `.git` file, but it is not a worktree of this + // repository and gets no worktree-local home. + const contents = yield* fileSystem + .readFileString(gitPath) + .pipe(Effect.orElseSucceed(() => "")); + return pointsAtLinkedWorktree(contents, path) ? directory : undefined; } const parent = path.dirname(directory); if (parent === directory) { diff --git a/packages/shared/src/hostProcess.ts b/packages/shared/src/hostProcess.ts index baff2c5ffed..4885072a072 100644 --- a/packages/shared/src/hostProcess.ts +++ b/packages/shared/src/hostProcess.ts @@ -30,6 +30,13 @@ export const HostProcessEnvironment = Context.Reference( }, ); +export const HostProcessWorkingDirectory = Context.Reference( + "@t3tools/shared/hostProcess/HostProcessWorkingDirectory", + { + defaultValue: () => process.cwd(), + }, +); + export const HostProcessExecutablePath = Context.Reference( "@t3tools/shared/hostProcess/HostProcessExecutablePath", { diff --git a/scripts/dev-runner.test.ts b/scripts/dev-runner.test.ts index 3b79db49f5b..00e9adcb574 100644 --- a/scripts/dev-runner.test.ts +++ b/scripts/dev-runner.test.ts @@ -1,6 +1,14 @@ +// @effect-diagnostics nodeBuiltinImport:off - builds real worktree layouts on disk. import * as NodeServices from "@effect/platform-node/NodeServices"; +import * as NodeFS from "node:fs"; +import * as NodeOS from "node:os"; +import * as NodePath from "node:path"; import * as NetService from "@t3tools/shared/Net"; -import { HostProcessPlatform } from "@t3tools/shared/hostProcess"; +import { + HostProcessEnvironment, + HostProcessPlatform, + HostProcessWorkingDirectory, +} from "@t3tools/shared/hostProcess"; import { assert, describe, it } from "@effect/vitest"; import * as ConfigProvider from "effect/ConfigProvider"; import * as Effect from "effect/Effect"; @@ -629,5 +637,100 @@ it.layer(NodeServices.layer)("dev-runner", (it) => { assert.notInclude(error.message, "secret-token-value"); }); }); + + describe("t3 home precedence", () => { + const makeWorktree = Effect.acquireRelease( + Effect.sync(() => { + const root = NodeFS.mkdtempSync(NodePath.join(NodeOS.tmpdir(), "t3-devrunner-")); + NodeFS.writeFileSync( + NodePath.join(root, ".git"), + "gitdir: /elsewhere/.git/worktrees/x\n", + ); + return root; + }), + (root) => Effect.sync(() => NodeFS.rmSync(root, { recursive: true, force: true })), + ); + + const spawnedHome = (input: { + readonly t3Home: string | undefined; + readonly cwd: string; + readonly ambientHome: string | undefined; + }) => + Effect.gen(function* () { + let captured: Record | undefined; + const spawnerLayer = Layer.succeed( + ChildProcessSpawner.ChildProcessSpawner, + ChildProcessSpawner.make((command) => { + captured = ( + command as { + readonly options?: { readonly env?: Record }; + } + ).options?.env; + return Effect.succeed(mockProcess(0)); + }), + ); + + yield* runDevRunnerWithInput({ ...devServerInput, t3Home: input.t3Home }).pipe( + Effect.provide(Layer.mergeAll(emptyConfigLayer, netServiceLayer, spawnerLayer)), + Effect.provideService(HostProcessPlatform, "linux"), + Effect.provideService(HostProcessWorkingDirectory, input.cwd), + Effect.provideService( + HostProcessEnvironment, + input.ambientHome === undefined ? {} : { T3CODE_HOME: input.ambientHome }, + ), + ); + + return captured?.T3CODE_HOME; + }); + + it.effect("prefers an explicit --home-dir over the worktree default", () => + Effect.gen(function* () { + const path = yield* Path.Path; + const root = yield* makeWorktree; + const home = yield* spawnedHome({ + t3Home: "/tmp/explicit-home", + cwd: root, + ambientHome: "/home/user/.t3", + }); + assert.equal(home, path.resolve("/tmp/explicit-home")); + }).pipe(Effect.scoped), + ); + + it.effect("prefers the worktree .t3 over an ambient T3CODE_HOME", () => + Effect.gen(function* () { + const path = yield* Path.Path; + const root = yield* makeWorktree; + const home = yield* spawnedHome({ + t3Home: undefined, + cwd: root, + ambientHome: "/home/user/.t3", + }); + assert.equal(home, path.join(path.resolve(root), ".t3")); + }).pipe(Effect.scoped), + ); + + it.effect("falls back to an ambient T3CODE_HOME outside a worktree", () => + Effect.gen(function* () { + const path = yield* Path.Path; + const home = yield* spawnedHome({ + t3Home: undefined, + cwd: NodeOS.tmpdir(), + ambientHome: "/home/user/.t3", + }); + assert.equal(home, path.resolve("/home/user/.t3")); + }), + ); + + it.effect("leaves the home implicit with no worktree and no ambient value", () => + Effect.gen(function* () { + const home = yield* spawnedHome({ + t3Home: undefined, + cwd: NodeOS.tmpdir(), + ambientHome: undefined, + }); + assert.equal(home, undefined); + }), + ); + }); }); }); diff --git a/scripts/dev-runner.ts b/scripts/dev-runner.ts index d6aec7daf36..980b86e44f9 100644 --- a/scripts/dev-runner.ts +++ b/scripts/dev-runner.ts @@ -6,7 +6,7 @@ import * as NodeRuntime from "@effect/platform-node/NodeRuntime"; import * as NodeServices from "@effect/platform-node/NodeServices"; import * as NetService from "@t3tools/shared/Net"; import { resolveWorktreeT3Home } from "@t3tools/shared/devHome"; -import { HostProcessEnvironment } from "@t3tools/shared/hostProcess"; +import { HostProcessEnvironment, HostProcessWorkingDirectory } from "@t3tools/shared/hostProcess"; import { resolveSpawnCommand } from "@t3tools/shared/shell"; import * as Config from "effect/Config"; import * as Effect from "effect/Effect"; @@ -511,7 +511,7 @@ export function runDevRunnerWithInput(input: DevRunnerCliInput) { // A worktree defaults to its own gitignored `.t3`. This deliberately // outranks ambient T3CODE_HOME, which otherwise selects the installed // app's live userdata database. An explicit --home-dir still wins. - const worktreeHome = yield* resolveWorktreeT3Home(process.cwd()); + const worktreeHome = yield* resolveWorktreeT3Home(yield* HostProcessWorkingDirectory); const resolvedT3Home = input.t3Home ?? worktreeHome ?? (hostEnvironment.T3CODE_HOME?.trim() || undefined); const env = yield* createDevRunnerEnv({ From 6404fc1a8e042e3ec079ea749413602e79e7002d Mon Sep 17 00:00:00 2001 From: Theo Browne Date: Sat, 25 Jul 2026 22:43:49 -0700 Subject: [PATCH 03/23] Match worktrees by their gitdir tail, ignore blank --home-dir MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `.git/worktrees` segment check rejected worktrees whose common dir is not named `.git`. A worktree of a bare repo points at `.git/worktrees/`, and $GIT_COMMON_DIR can be anything. Git always uses the `worktrees/` tail, so match on that instead; `/modules/` still fails it. A blank `--home-dir` also counted as a selection under `??`, skipping the worktree default and landing on the shared home — the outcome this precedence exists to prevent. Trim before choosing. Co-Authored-By: Claude Opus 5 (1M context) --- packages/shared/src/devHome.test.ts | 31 ++++++++++++++++++++++++++++- packages/shared/src/devHome.ts | 25 +++++++++++++++-------- scripts/dev-runner.test.ts | 13 ++++++++++++ scripts/dev-runner.ts | 7 ++++++- 4 files changed, 66 insertions(+), 10 deletions(-) diff --git a/packages/shared/src/devHome.test.ts b/packages/shared/src/devHome.test.ts index 184768a7a2c..812c34e3cce 100644 --- a/packages/shared/src/devHome.test.ts +++ b/packages/shared/src/devHome.test.ts @@ -8,12 +8,27 @@ import * as Effect from "effect/Effect"; import { resolveGitWorktreePath, resolveWorktreeT3Home } from "./devHome.ts"; -const makeRepo = (kind: "worktree" | "checkout" | "bare" | "submodule" | "unreadable-git-file") => +const makeRepo = ( + kind: + | "worktree" + | "checkout" + | "bare" + | "submodule" + | "unreadable-git-file" + | "bare-repo-worktree" + | "custom-common-dir-worktree", +) => Effect.acquireRelease( Effect.sync(() => { const root = NodeFS.mkdtempSync(NodePath.join(NodeOS.tmpdir(), "t3-devhome-")); if (kind === "worktree") { NodeFS.writeFileSync(NodePath.join(root, ".git"), "gitdir: /elsewhere/.git/worktrees/x\n"); + } else if (kind === "bare-repo-worktree") { + // `git worktree add` from a bare repo: the common dir is `.git`. + NodeFS.writeFileSync(NodePath.join(root, ".git"), "gitdir: /srv/myrepo.git/worktrees/x\n"); + } else if (kind === "custom-common-dir-worktree") { + // $GIT_COMMON_DIR need not be named `.git` at all. + NodeFS.writeFileSync(NodePath.join(root, ".git"), "gitdir: /srv/store/worktrees/x\n"); } else if (kind === "submodule") { NodeFS.writeFileSync(NodePath.join(root, ".git"), "gitdir: ../.git/modules/sub\n"); } else if (kind === "unreadable-git-file") { @@ -63,6 +78,20 @@ describe("resolveGitWorktreePath", () => { assert.equal(yield* resolveGitWorktreePath(nested), undefined); }).pipe(Effect.scoped, Effect.provide(NodeServices.layer)), ); + + it.effect("finds a worktree of a bare repository", () => + Effect.gen(function* () { + const { root, nested } = yield* makeRepo("bare-repo-worktree"); + assert.equal(yield* resolveGitWorktreePath(nested), NodePath.resolve(root)); + }).pipe(Effect.scoped, Effect.provide(NodeServices.layer)), + ); + + it.effect("finds a worktree whose common dir is not named .git", () => + Effect.gen(function* () { + const { root, nested } = yield* makeRepo("custom-common-dir-worktree"); + assert.equal(yield* resolveGitWorktreePath(nested), NodePath.resolve(root)); + }).pipe(Effect.scoped, Effect.provide(NodeServices.layer)), + ); }); describe("resolveWorktreeT3Home", () => { diff --git a/packages/shared/src/devHome.ts b/packages/shared/src/devHome.ts index 99aabca9f37..1fde5409f5c 100644 --- a/packages/shared/src/devHome.ts +++ b/packages/shared/src/devHome.ts @@ -14,10 +14,15 @@ import * as Option from "effect/Option"; import * as Path from "effect/Path"; /** - * A `.git` file points at the real git directory. A linked worktree's lives - * under `/.git/worktrees/`; a submodule's under - * `/.git/modules/`. Both are files, so the pointer — not the + * A `.git` file points at the real git directory. A linked worktree's lives at + * `/worktrees/`; a submodule's at + * `/modules/`. Both are files, so the pointer — not the * file-vs-directory distinction alone — is what identifies a worktree. + * + * The common dir is not necessarily named `.git`: a worktree of a bare repo + * points at `.git/worktrees/`, and `$GIT_COMMON_DIR` can be + * anything. So match on the `worktrees/` tail, which git always uses, + * rather than on the name of the directory containing it. */ const pointsAtLinkedWorktree = (gitFileContents: string, path: Path.Path): boolean => { const gitdir = gitFileContents @@ -29,11 +34,15 @@ const pointsAtLinkedWorktree = (gitFileContents: string, path: Path.Path): boole if (gitdir === undefined || gitdir.length === 0) { return false; } - // Normalize so `.git/worktrees` is matched as path segments rather than as a - // substring of some directory that merely happens to be named that way. - const segments = path.normalize(gitdir.replaceAll("\\", "/")).split(/[/\\]/); - const gitIndex = segments.lastIndexOf(".git"); - return gitIndex !== -1 && segments[gitIndex + 1] === "worktrees"; + // Compare as path segments so a directory merely named `…worktrees…` cannot + // match as a substring. Trailing separators normalize away first. + const segments = path + .normalize(gitdir.replaceAll("\\", "/")) + .split(/[/\\]/) + .filter((segment) => segment.length > 0); + // `/worktrees/`: `worktrees` is the penultimate segment, + // and something must precede it. This excludes `/modules/`. + return segments.length >= 3 && segments.at(-2) === "worktrees"; }; /** diff --git a/scripts/dev-runner.test.ts b/scripts/dev-runner.test.ts index 00e9adcb574..ad95a49832e 100644 --- a/scripts/dev-runner.test.ts +++ b/scripts/dev-runner.test.ts @@ -696,6 +696,19 @@ it.layer(NodeServices.layer)("dev-runner", (it) => { }).pipe(Effect.scoped), ); + it.effect("treats a blank --home-dir as unset rather than as a selection", () => + Effect.gen(function* () { + const path = yield* Path.Path; + const root = yield* makeWorktree; + const home = yield* spawnedHome({ + t3Home: " ", + cwd: root, + ambientHome: "/home/user/.t3", + }); + assert.equal(home, path.join(path.resolve(root), ".t3")); + }).pipe(Effect.scoped), + ); + it.effect("prefers the worktree .t3 over an ambient T3CODE_HOME", () => Effect.gen(function* () { const path = yield* Path.Path; diff --git a/scripts/dev-runner.ts b/scripts/dev-runner.ts index 980b86e44f9..2875cbe88d3 100644 --- a/scripts/dev-runner.ts +++ b/scripts/dev-runner.ts @@ -512,8 +512,13 @@ export function runDevRunnerWithInput(input: DevRunnerCliInput) { // outranks ambient T3CODE_HOME, which otherwise selects the installed // app's live userdata database. An explicit --home-dir still wins. const worktreeHome = yield* resolveWorktreeT3Home(yield* HostProcessWorkingDirectory); + // Trim before choosing: `--home-dir ""` is not a selection, and treating it + // as one would skip the worktree default and land on the shared home — + // exactly the outcome this precedence exists to prevent. const resolvedT3Home = - input.t3Home ?? worktreeHome ?? (hostEnvironment.T3CODE_HOME?.trim() || undefined); + (input.t3Home?.trim() || undefined) ?? + worktreeHome ?? + (hostEnvironment.T3CODE_HOME?.trim() || undefined); const env = yield* createDevRunnerEnv({ mode: input.mode, baseEnv: hostEnvironment, From 8684ca4d172c3f84a6e5367d7d43751d856757c4 Mon Sep 17 00:00:00 2001 From: Theo Browne Date: Sat, 25 Jul 2026 22:17:15 -0700 Subject: [PATCH 04/23] Make browser dev shareable over Tailscale --- .agents/skills/test-t3-app/SKILL.md | 6 +- AGENTS.md | 3 + apps/server/src/auth/EnvironmentAuth.test.ts | 9 +- apps/server/src/auth/EnvironmentAuth.ts | 5 + .../src/auth/EnvironmentAuthPolicy.test.ts | 5 +- apps/server/src/auth/EnvironmentAuthPolicy.ts | 5 +- apps/server/src/auth/PairingGrantStore.ts | 19 ++- apps/server/src/auth/SessionStore.ts | 5 +- apps/server/src/auth/utils.ts | 17 +- apps/server/src/http.ts | 19 ++- apps/web/vite.config.ts | 83 +++++++--- docs/reference/scripts.md | 8 + package.json | 1 + packages/shared/package.json | 4 + packages/shared/src/devProxy.ts | 17 ++ packages/tailscale/src/tailscale.ts | 16 ++ pnpm-lock.yaml | 3 + scripts/dev-runner.test.ts | 107 ++++++++++++- scripts/dev-runner.ts | 147 +++++++++++++++--- scripts/lib/dev-share.test.ts | 134 ++++++++++++++++ scripts/lib/dev-share.ts | 146 +++++++++++++++++ scripts/package.json | 1 + 22 files changed, 693 insertions(+), 67 deletions(-) create mode 100644 packages/shared/src/devProxy.ts create mode 100644 scripts/lib/dev-share.test.ts create mode 100644 scripts/lib/dev-share.ts diff --git a/.agents/skills/test-t3-app/SKILL.md b/.agents/skills/test-t3-app/SKILL.md index f26270cd06d..7bc0d86498a 100644 --- a/.agents/skills/test-t3-app/SKILL.md +++ b/.agents/skills/test-t3-app/SKILL.md @@ -13,13 +13,17 @@ Use this skill for the web client. For iOS Simulator, Android Emulator, or physi 2. Choose a base directory that belongs only to the current worktree or test: - Use the repository's ignored `.t3` directory for reusable worktree-local state. - Use `mktemp -d /tmp/t3code-test.XXXXXX` for disposable state and retain the printed absolute path. -3. Start the full web stack with `vp run dev`. In a linked worktree it defaults to that worktree's gitignored `.t3`; pass `--home-dir ` only when the test needs a different isolated directory. +3. Start the full web stack with `vp run dev`. Add `--share` when the user needs to open it from another tailnet device. In a linked worktree it defaults to that worktree's gitignored `.t3`; pass `--home-dir ` only when the test needs a different isolated directory. 4. Keep the terminal session alive and read the selected server port, web port, base directory, and pairing URL from its output. Treat a base directory as disposable only when it was created or deliberately selected for the current test. Never delete or directly seed the shared `~/.t3` directory. Prefer starting with a new temporary base directory over clearing state of uncertain ownership. The worktree-local default deliberately outranks an ambient `T3CODE_HOME`; do not pass the shared home through to a worktree dev server. +Ports are derived from the worktree path but can shift when occupied. Always read the actual values from the `[dev-runner]` line. + +Shared browser dev is single-origin: Vite proxies the backend paths, so never set `VITE_HTTP_URL` or `VITE_WS_URL` for `dev`/`dev:web`. + The dev runner disables browser auto-open by default. Do not pass `--browser` during automated testing: an automatically opened page can consume the one-time bootstrap token before the controlled browser uses it. ## Preserve the environment while iterating diff --git a/AGENTS.md b/AGENTS.md index 7b67ebb7bd5..fd79e04d36c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -16,6 +16,9 @@ ## Dev Servers - In a linked git worktree, dev state defaults to that worktree's gitignored `.t3`. This deliberately outranks an ambient `T3CODE_HOME`, which could otherwise select the installed app's live `~/.t3/userdata` database. An explicit `--home-dir` still wins. +- Start the web stack with `bun run dev`. Use `bun run dev:share` when someone needs to open it from another device on the tailnet. +- Browser dev is single-origin: Vite proxies `/api`, `/ws`, `/oauth`, and `/.well-known` to the backend. Do not set `VITE_HTTP_URL` or `VITE_WS_URL` for `dev`/`dev:web`. +- Worktree paths supply stable preferred port offsets. Read the actual server and web ports from the `[dev-runner]` line because occupied ports can still shift them. ## Package Roles diff --git a/apps/server/src/auth/EnvironmentAuth.test.ts b/apps/server/src/auth/EnvironmentAuth.test.ts index 335e0685197..1970948088a 100644 --- a/apps/server/src/auth/EnvironmentAuth.test.ts +++ b/apps/server/src/auth/EnvironmentAuth.test.ts @@ -8,9 +8,13 @@ import * as ServerConfig from "../config.ts"; import { SqlitePersistenceMemory } from "../persistence/Layers/Sqlite.ts"; import * as PairingGrantStore from "./PairingGrantStore.ts"; import * as EnvironmentAuth from "./EnvironmentAuth.ts"; +import { resolveSessionCookieName } from "./utils.ts"; import * as ServerSecretStore from "./ServerSecretStore.ts"; +/** Pinned so the session cookie name (which is port-scoped) is predictable. */ +const TEST_SERVER_PORT = 13_773; + const makeServerConfigLayer = (overrides?: Partial) => Layer.effect( ServerConfig.ServerConfig, @@ -18,6 +22,7 @@ const makeServerConfigLayer = (overrides?: Partial[0] => ({ cookies: { - t3_session: sessionToken, + // Derived, not hardcoded: the name is port-scoped so concurrent servers + // on one hostname don't share a cookie. + [resolveSessionCookieName({ port: TEST_SERVER_PORT })]: sessionToken, }, headers: {}, }) as unknown as Parameters< diff --git a/apps/server/src/auth/EnvironmentAuth.ts b/apps/server/src/auth/EnvironmentAuth.ts index dd53a83ca95..eb056342140 100644 --- a/apps/server/src/auth/EnvironmentAuth.ts +++ b/apps/server/src/auth/EnvironmentAuth.ts @@ -438,6 +438,7 @@ export class EnvironmentAuth extends Context.Service< readonly scopes?: ReadonlyArray; readonly subject?: string; readonly proofKeyThumbprint?: string; + readonly purpose?: "startup"; }) => Effect.Effect; readonly issuePairingCredential: ( input?: AuthCreatePairingCredentialInput, @@ -746,11 +747,13 @@ export const make = Effect.gen(function* () { readonly scopes: ReadonlyArray; readonly subject: string; readonly label?: string; + readonly purpose?: "startup"; }) => createPairingLink({ scopes: input.scopes, subject: input.subject, ...(input.label ? { label: input.label } : {}), + ...(input.purpose ? { purpose: input.purpose } : {}), }).pipe( Effect.map( (issued) => @@ -774,6 +777,7 @@ export const make = Effect.gen(function* () { ...(input?.ttl ? { ttl: input.ttl } : {}), ...(input?.label ? { label: input.label } : {}), ...(input?.proofKeyThumbprint ? { proofKeyThumbprint: input.proofKeyThumbprint } : {}), + ...(input?.purpose ? { purpose: input.purpose } : {}), }); return { id: issued.id, @@ -872,6 +876,7 @@ export const make = Effect.gen(function* () { issuePairingCredentialForSubject({ scopes: AuthAdministrativeScopes, subject: INTERNAL_ADMINISTRATIVE_BOOTSTRAP_SUBJECT, + purpose: "startup", }).pipe(Effect.withSpan("EnvironmentAuth.issueStartupPairingCredential")); const listClientSessions: EnvironmentAuth["Service"]["listClientSessions"] = (currentSessionId) => diff --git a/apps/server/src/auth/EnvironmentAuthPolicy.test.ts b/apps/server/src/auth/EnvironmentAuthPolicy.test.ts index 95269fb6c37..55f1a99b205 100644 --- a/apps/server/src/auth/EnvironmentAuthPolicy.test.ts +++ b/apps/server/src/auth/EnvironmentAuthPolicy.test.ts @@ -69,12 +69,15 @@ it.layer(NodeServices.layer)("EnvironmentAuthPolicy.layer", (it) => { expect(descriptor.policy).toBe("loopback-browser"); expect(descriptor.bootstrapMethods).toEqual(["one-time-token"]); - expect(descriptor.sessionCookieName).toBe("t3_session"); + // Port-scoped in web mode too: cookies ignore ports, so two dev servers + // on one hostname would otherwise clobber each other's session. + expect(descriptor.sessionCookieName).toBe("t3_session_13773"); }).pipe( Effect.provide( makeEnvironmentAuthPolicyLayer({ mode: "web", host: "127.0.0.1", + port: 13773, }), ), ), diff --git a/apps/server/src/auth/EnvironmentAuthPolicy.ts b/apps/server/src/auth/EnvironmentAuthPolicy.ts index 7ffef0ff0a5..1d7a1dd4ad3 100644 --- a/apps/server/src/auth/EnvironmentAuthPolicy.ts +++ b/apps/server/src/auth/EnvironmentAuthPolicy.ts @@ -38,10 +38,7 @@ export const make = Effect.gen(function* () { policy, bootstrapMethods, sessionMethods: ["browser-session-cookie", "bearer-access-token", "dpop-access-token"], - sessionCookieName: resolveSessionCookieName({ - mode: config.mode, - port: config.port, - }), + sessionCookieName: resolveSessionCookieName({ port: config.port }), }; return EnvironmentAuthPolicy.of({ diff --git a/apps/server/src/auth/PairingGrantStore.ts b/apps/server/src/auth/PairingGrantStore.ts index 588d5e3775f..057a257ba66 100644 --- a/apps/server/src/auth/PairingGrantStore.ts +++ b/apps/server/src/auth/PairingGrantStore.ts @@ -202,6 +202,11 @@ export class PairingGrantStore extends Context.Service< readonly subject?: string; readonly label?: string; readonly proofKeyThumbprint?: string; + /** + * "startup" marks the credential the server mints for itself at boot, + * which gets the long dev TTL when a dev URL is configured. + */ + readonly purpose?: "startup"; }) => Effect.Effect; readonly listActive: () => Effect.Effect< ReadonlyArray, @@ -243,6 +248,15 @@ const DEFAULT_ONE_TIME_TOKEN_TTL_MINUTES = Duration.minutes(5); // window can still recover by re-bootstrapping rather than locking // the user out of the backend. const DESKTOP_BOOTSTRAP_TTL_HOURS = Duration.hours(24); +// A dev server's startup token is read off a log by whoever (or whatever) is +// driving the session, often minutes later — after a `node --watch` restart, a +// detour into another task, or a hand-off to the person actually doing the +// testing. Five minutes turns that into a restart-the-server loop for no +// security benefit: the token only unlocks a local dev backend, and its holder +// could read the log anyway. Same reasoning (and duration) as the desktop +// bootstrap grant above. Only applies when a dev URL is configured; user-issued +// pairing links and real servers keep the 5-minute default. +const DEV_STARTUP_TTL_HOURS = Duration.hours(24); const PAIRING_TOKEN_ALPHABET = "23456789ABCDEFGHJKLMNPQRSTUVWXYZ"; const PAIRING_TOKEN_LENGTH = 12; const PAIRING_TOKEN_REJECTION_LIMIT = @@ -371,7 +385,10 @@ export const make = Effect.gen(function* () { ), ); const credential = yield* generatePairingToken; - const ttl = input?.ttl ?? DEFAULT_ONE_TIME_TOKEN_TTL_MINUTES; + const isDevStartupToken = config.devUrl !== undefined && input?.purpose === "startup"; + const ttl = + input?.ttl ?? + (isDevStartupToken ? DEV_STARTUP_TTL_HOURS : DEFAULT_ONE_TIME_TOKEN_TTL_MINUTES); const now = yield* DateTime.now; const expiresAt = DateTime.add(now, { milliseconds: Duration.toMillis(ttl) }); const issued: IssuedBootstrapCredential = { diff --git a/apps/server/src/auth/SessionStore.ts b/apps/server/src/auth/SessionStore.ts index 12ecb7dba4d..ed113470b24 100644 --- a/apps/server/src/auth/SessionStore.ts +++ b/apps/server/src/auth/SessionStore.ts @@ -467,10 +467,7 @@ export const make = Effect.gen(function* () { const signingSecret = yield* secretStore.getOrCreateRandom(SIGNING_SECRET_NAME, 32); const connectedSessionsRef = yield* Ref.make(new Map()); const changesPubSub = yield* PubSub.unbounded(); - const cookieName = resolveSessionCookieName({ - mode: serverConfig.mode, - port: serverConfig.port, - }); + const cookieName = resolveSessionCookieName({ port: serverConfig.port }); const emitUpsert = (clientSession: AuthClientSession) => PubSub.publish(changesPubSub, { diff --git a/apps/server/src/auth/utils.ts b/apps/server/src/auth/utils.ts index 39f04988ac5..0bb03099fa5 100644 --- a/apps/server/src/auth/utils.ts +++ b/apps/server/src/auth/utils.ts @@ -10,14 +10,15 @@ import * as Result from "effect/Result"; const SESSION_COOKIE_NAME = "t3_session"; -export function resolveSessionCookieName(input: { - readonly mode: "web" | "desktop"; - readonly port: number; -}): string { - if (input.mode !== "desktop") { - return SESSION_COOKIE_NAME; - } - +/** + * Cookies are scoped by host but *not* by port, so every server reachable at a + * given hostname shares one cookie jar. Suffixing the port keeps concurrent + * instances from overwriting each other's session — otherwise two dev servers + * (different worktrees, or several ports behind one tailnet name) fight over + * `t3_session`, and whichever wrote last makes every other one reject the + * cookie with "Invalid session token signature" until it's cleared by hand. + */ +export function resolveSessionCookieName(input: { readonly port: number }): string { return `${SESSION_COOKIE_NAME}_${input.port}`; } diff --git a/apps/server/src/http.ts b/apps/server/src/http.ts index b9bb40f372d..bfb2caf6bde 100644 --- a/apps/server/src/http.ts +++ b/apps/server/src/http.ts @@ -4,6 +4,7 @@ import { AuthOrchestrationReadScope, EnvironmentHttpApi, } from "@t3tools/contracts"; +import { isDevProxiedPath } from "@t3tools/shared/devProxy"; import { decodeOtlpTraceRecords } from "@t3tools/shared/observability"; import * as Data from "effect/Data"; import * as Effect from "effect/Effect"; @@ -48,9 +49,21 @@ export const browserApiCorsLayer = Layer.unwrap( const devOrigin = config.devUrl?.origin; // Dev uses credentialed requests from Vite or the Electron custom origin, so both must be // explicit. Packaged desktop omits credentials and uses Effect's default wildcard origin. + // + // T3CODE_DEV_ALLOWED_ORIGINS covers dev servers reached from a second + // origin — a tailnet name, a LAN IP, a phone. Browser dev normally proxies + // through Vite and is same-origin (no preflight at all), so this is a + // safety net for the desktop renderer and any direct-to-backend caller. + const extraDevOrigins = (process.env.T3CODE_DEV_ALLOWED_ORIGINS ?? "") + .split(",") + .map((entry) => entry.trim()) + .filter((entry) => entry.length > 0); return HttpRouter.cors({ ...(devOrigin - ? { allowedOrigins: [devOrigin, ...DESKTOP_RENDERER_ORIGINS], credentials: true } + ? { + allowedOrigins: [devOrigin, ...DESKTOP_RENDERER_ORIGINS, ...extraDevOrigins], + credentials: true, + } : {}), allowedMethods: browserApiCorsAllowedMethods, allowedHeaders: browserApiCorsAllowedHeaders, @@ -216,6 +229,10 @@ export const staticAndDevRouteLayer = HttpRouter.add( } const config = yield* ServerConfig.ServerConfig; + if (config.devUrl && isDevProxiedPath(url.value.pathname)) { + return HttpServerResponse.text("Not Found", { status: 404 }); + } + if (config.devUrl && isLoopbackHostname(url.value.hostname)) { return HttpServerResponse.redirect(resolveDevRedirectUrl(config.devUrl, url.value), { status: 302, diff --git a/apps/web/vite.config.ts b/apps/web/vite.config.ts index 6e5b532b58a..c8e0c7abae8 100644 --- a/apps/web/vite.config.ts +++ b/apps/web/vite.config.ts @@ -7,13 +7,16 @@ import "vite-plus/test/config"; import { defineConfig } from "vite-plus"; import pkg from "./package.json" with { type: "json" }; +import { DEV_PROXIED_PATH_PREFIXES } from "@t3tools/shared/devProxy"; + import { loadRepoEnv } from "../../scripts/lib/public-config"; const repoEnv = loadRepoEnv(); Object.assign(process.env, repoEnv); const port = Number(process.env.PORT ?? 5733); -const host = process.env.HOST?.trim() || "localhost"; +const explicitHost = process.env.HOST?.trim(); +const host = explicitHost || "localhost"; const configuredWsUrl = process.env.VITE_WS_URL?.trim(); const configuredRelayUrl = repoEnv.VITE_T3CODE_RELAY_URL?.trim() || ""; const configuredClerkPublishableKey = repoEnv.VITE_CLERK_PUBLISHABLE_KEY?.trim() || ""; @@ -65,7 +68,20 @@ const unitTestProject = { }, } satisfies TestProjectInlineConfiguration; -function resolveDevProxyTarget(wsUrl: string | undefined): string | undefined { +function resolveDevProxyTarget( + backendPort: string | undefined, + wsUrl: string | undefined, +): string | undefined { + // Browser dev is single-origin: the backend port is proxied through this + // server so the app works from any origin (localhost, tailnet, LAN, phone). + // T3CODE_PORT is set by scripts/dev-runner.ts for every non-desktop mode. + const port = Number(backendPort?.trim()); + if (Number.isInteger(port) && port > 0) { + return `http://localhost:${port}/`; + } + + // dev:desktop still points the renderer straight at the backend, so fall + // back to deriving the target from the explicit websocket URL. if (!wsUrl) { return undefined; } @@ -86,7 +102,17 @@ function resolveDevProxyTarget(wsUrl: string | undefined): string | undefined { } } -const devProxyTarget = resolveDevProxyTarget(configuredWsUrl); +const devProxyTarget = resolveDevProxyTarget(process.env.T3CODE_PORT, configuredWsUrl); + +// Vite rejects requests whose Host header isn't localhost, which blocks sharing +// a dev server over Tailscale/LAN. Tailnet names are safe to allow wholesale: +// the DNS is controlled by tailscale, so they can't be rebound by an attacker. +// Anything else (ngrok, a LAN IP alias) goes through the env var. +const configuredAllowedHosts = (process.env.T3CODE_DEV_ALLOWED_HOSTS ?? "") + .split(",") + .map((entry) => entry.trim()) + .filter((entry) => entry.length > 0); +const allowedHosts = [".ts.net", ...configuredAllowedHosts]; export default defineConfig(() => { return { @@ -145,32 +171,41 @@ export default defineConfig(() => { host, port, strictPort: true, + allowedHosts, ...(devProxyTarget ? { - proxy: { - "/.well-known": { - target: devProxyTarget, - changeOrigin: true, - }, - "/api": { - target: devProxyTarget, - changeOrigin: true, - }, - "/attachments": { - target: devProxyTarget, - changeOrigin: true, - }, + // One entry per shared prefix; the server's dev catch-all 404s the + // same list, so the two sides cannot drift. `/ws` is the app's own + // socket — Vite's HMR socket is matched separately and exactly + // (path "/" plus a vite-hmr subprotocol), so the two upgrade + // handlers don't collide. + proxy: Object.fromEntries( + DEV_PROXIED_PATH_PREFIXES.map((prefix) => [ + prefix, + { + target: devProxyTarget, + changeOrigin: true, + ...(prefix === "/ws" ? { ws: true } : {}), + }, + ]), + ), + } + : {}), + // Electron's BrowserWindow needs the HMR socket pinned to an explicit + // host to connect reliably; dev:desktop is the only mode that sets HOST. + // Everywhere else, leaving this unset lets the client derive it from the + // page origin, which is what makes HMR work over Tailscale/LAN instead of + // failing an attempt against the wrong machine's localhost first. + // (Vite 8 logs connection state via console.debug — enable "Verbose".) + ...(explicitHost + ? { + hmr: { + protocol: "ws", + host: explicitHost, + clientPort: port, }, } : {}), - hmr: { - // Explicit config so Vite's HMR WebSocket connects reliably - // inside Electron's BrowserWindow. Vite 8 uses console.debug for - // connection logs — enable "Verbose" in DevTools to see them. - protocol: "ws", - host, - clientPort: port, - }, }, build: { outDir: "dist", diff --git a/docs/reference/scripts.md b/docs/reference/scripts.md index b985c9c9e3d..2697af9ea15 100644 --- a/docs/reference/scripts.md +++ b/docs/reference/scripts.md @@ -1,6 +1,7 @@ # Scripts - `vp run dev` — Starts contracts, server, and web in watch mode. +- `vp run dev --share` (or `vp run dev:share`) — Also publishes the web port over HTTPS on this machine's tailnet. The startup pairing URL is built against the shared origin, and the mapping is removed on exit. - `vp run dev:server` — Starts just the WebSocket server. The server process runs on Bun (`@effect/platform-bun` + `BunPtyAdapter`), but task running uses `vp run`. - `vp run dev:web` — Starts just the Vite dev server for the web app. - Dev commands run from a linked **git worktree** default to that worktree's gitignored `.t3`, even when `T3CODE_HOME` is set, storing state in `/.t3/userdata`. Pass `--home-dir ` to choose another isolated directory explicitly. Submodules are not worktrees and keep the normal precedence. @@ -24,6 +25,7 @@ - Default build is unsigned/not notarized for local sharing. - The DMG build uses `assets/prod/black-macos-1024.png` as the production app icon source. - Desktop production windows load the bundled UI from `t3code://app/index.html` (not a `127.0.0.1` document URL). + - Desktop packaging includes `apps/server/dist` (the `t3` backend) and starts it on loopback with an auth token for WebSocket/API traffic. - Your tester can still open it on macOS by right-clicking the app and choosing **Open** on first launch. - To keep staging files for debugging package contents, run: `vp run dist:desktop:dmg -- --keep-stage` @@ -37,6 +39,12 @@ - Azure authentication env vars are also required (for example service principal with secret): `AZURE_TENANT_ID`, `AZURE_CLIENT_ID`, `AZURE_CLIENT_SECRET`. +## Browser development + +`dev` and `dev:web` leave `VITE_HTTP_URL` and `VITE_WS_URL` unset so the browser resolves the backend from `window.location.origin`. Vite proxies `/api`, `/ws`, `/oauth`, and `/.well-known` to the server, allowing the same bundle to work from localhost or a tailnet hostname. + +Worktrees derive a preferred port offset from their path. The runner shifts both ports together when either is occupied, so treat the `[dev-runner]` output as authoritative. + ## Running multiple dev instances Set `T3CODE_DEV_INSTANCE` to any value to deterministically shift all dev ports together. diff --git a/package.json b/package.json index 3c0a8cc6b77..e2492914d6d 100644 --- a/package.json +++ b/package.json @@ -5,6 +5,7 @@ "scripts": { "prepare": "effect-tsgo patch && vp config --no-agent", "dev": "node scripts/dev-runner.ts dev", + "dev:share": "node scripts/dev-runner.ts dev --share", "dev:server": "node scripts/dev-runner.ts dev:server", "dev:web": "node scripts/dev-runner.ts dev:web", "dev:marketing": "vp run --filter @t3tools/marketing dev", diff --git a/packages/shared/package.json b/packages/shared/package.json index b83ecc53624..8a45591fd36 100644 --- a/packages/shared/package.json +++ b/packages/shared/package.json @@ -198,6 +198,10 @@ "./devHome": { "types": "./src/devHome.ts", "import": "./src/devHome.ts" + }, + "./devProxy": { + "types": "./src/devProxy.ts", + "import": "./src/devProxy.ts" } }, "scripts": { diff --git a/packages/shared/src/devProxy.ts b/packages/shared/src/devProxy.ts new file mode 100644 index 00000000000..13336cb48a1 --- /dev/null +++ b/packages/shared/src/devProxy.ts @@ -0,0 +1,17 @@ +/** + * Backend paths the web dev server proxies in single-origin browser dev. + * + * Two consumers must agree on this list: the Vite proxy map + * (apps/web/vite.config.ts) that forwards these to the backend, and the + * server's dev catch-all (apps/server/src/http.ts) that 404s them instead of + * redirecting back to Vite. Drift is silent and nasty in both directions — a + * prefix only Vite knows gets answered with index.html; a prefix only the + * server knows redirect-loops through the proxy. + */ +export const DEV_PROXIED_PATH_PREFIXES = ["/api", "/oauth", "/.well-known", "/ws"] as const; + +export function isDevProxiedPath(pathname: string): boolean { + return DEV_PROXIED_PATH_PREFIXES.some( + (prefix) => pathname === prefix || pathname.startsWith(`${prefix}/`), + ); +} diff --git a/packages/tailscale/src/tailscale.ts b/packages/tailscale/src/tailscale.ts index 27761490af0..b40ead671bc 100644 --- a/packages/tailscale/src/tailscale.ts +++ b/packages/tailscale/src/tailscale.ts @@ -54,6 +54,9 @@ export class TailscaleCommandExitError extends Schema.TaggedErrorClass { + const trimmed = stderr.trim(); + return trimmed.length > 0 ? trimmed.slice(0, STDERR_PREVIEW_LIMIT) : undefined; +}; + export class TailscaleCommandTimeoutError extends Schema.TaggedErrorClass()( "TailscaleCommandTimeoutError", { @@ -212,6 +222,9 @@ export const readTailscaleStatus = Effect.gen(function* () { exitCode, stdoutLength: stdout.length, stderrLength: stderr.length, + ...(stderrPreviewOf(stderr) !== undefined + ? { stderrPreview: stderrPreviewOf(stderr) } + : {}), }); } return yield* parseTailscaleStatus(stdout); @@ -275,6 +288,9 @@ const runTailscaleCommand = ( ...commandContext, exitCode, stderrLength: stderr.length, + ...(stderrPreviewOf(stderr) !== undefined + ? { stderrPreview: stderrPreviewOf(stderr) } + : {}), }); } }).pipe( diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index bea636d3807..d47c81d4ba4 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -905,6 +905,9 @@ importers: '@t3tools/shared': specifier: workspace:* version: link:../packages/shared + '@t3tools/tailscale': + specifier: workspace:* + version: link:../packages/tailscale effect: specifier: 4.0.0-beta.78 version: 4.0.0-beta.78(patch_hash=c502bc684210b707dfceb87d8fe6ad6843395af6e19cfc02cd65854898bde2c5) diff --git a/scripts/dev-runner.test.ts b/scripts/dev-runner.test.ts index ad95a49832e..dba2c4de087 100644 --- a/scripts/dev-runner.test.ts +++ b/scripts/dev-runner.test.ts @@ -66,6 +66,7 @@ const devServerInput = { port: 13_773, devUrl: undefined, dryRun: false, + share: false, runArgs: ["--inspect", "secret-token-value"], } as const; @@ -344,8 +345,58 @@ it.layer(NodeServices.layer)("dev-runner", (it) => { }); assert.equal(env.T3CODE_PORT, "13773"); - assert.equal(env.VITE_HTTP_URL, "http://localhost:13773"); - assert.equal(env.VITE_WS_URL, "ws://localhost:13773"); + assert.equal(env.PORT, "5733"); + }), + ); + + // Browser dev is single-origin: Vite proxies the backend, and the client + // resolves it from window.location.origin. Baking a localhost URL here is + // what breaks sharing a dev server to another device. + for (const mode of ["dev", "dev:web"] as const) { + it.effect(`leaves the client backend URLs unset in ${mode} mode`, () => + Effect.gen(function* () { + const env = yield* createDevRunnerEnv({ + mode, + baseEnv: { + VITE_HTTP_URL: "http://localhost:1234", + VITE_WS_URL: "ws://localhost:1234", + }, + serverOffset: 0, + webOffset: 0, + t3Home: undefined, + browser: undefined, + autoBootstrapProjectFromCwd: undefined, + logWebSocketEvents: undefined, + host: undefined, + port: undefined, + devUrl: undefined, + }); + + assert.equal(env.VITE_HTTP_URL, undefined); + assert.equal(env.VITE_WS_URL, undefined); + assert.equal(env.T3CODE_PORT, "13773"); + }), + ); + } + + it.effect("keeps explicit backend URLs for the desktop renderer", () => + Effect.gen(function* () { + const env = yield* createDevRunnerEnv({ + mode: "dev:desktop", + baseEnv: {}, + serverOffset: 0, + webOffset: 0, + t3Home: undefined, + browser: undefined, + autoBootstrapProjectFromCwd: undefined, + logWebSocketEvents: undefined, + host: undefined, + port: undefined, + devUrl: undefined, + }); + + assert.equal(env.VITE_HTTP_URL, "http://127.0.0.1:13773"); + assert.equal(env.VITE_WS_URL, "ws://127.0.0.1:13773"); }), ); }); @@ -577,6 +628,58 @@ it.layer(NodeServices.layer)("dev-runner", (it) => { }); }); + // `tailscale serve` config outlives the process, so a dry run that shared + // would replace and then tear down whatever mapping the port already had. + // Base-dir precedence (--home-dir > worktree .t3 > ambient T3CODE_HOME) + // lives in runDevRunnerWithInput; the env builder must not consult the + // ambient variable on its own, or it would silently outrank the worktree + // default and land dev state on the user's real database. + it.effect("ignores an ambient T3CODE_HOME when no home is resolved", () => + Effect.gen(function* () { + const env = yield* createDevRunnerEnv({ + mode: "dev", + baseEnv: { T3CODE_HOME: "/home/user/.t3" }, + serverOffset: 0, + webOffset: 0, + t3Home: undefined, + browser: undefined, + autoBootstrapProjectFromCwd: undefined, + logWebSocketEvents: undefined, + host: undefined, + port: undefined, + devUrl: undefined, + }); + + assert.equal(env.T3CODE_HOME, undefined); + }), + ); + + it.effect("spawns nothing when --dry-run is combined with --share", () => { + let spawnCount = 0; + const spawnerLayer = Layer.succeed( + ChildProcessSpawner.ChildProcessSpawner, + ChildProcessSpawner.make(() => { + spawnCount += 1; + return Effect.succeed(mockProcess(0)); + }), + ); + + return Effect.gen(function* () { + yield* runDevRunnerWithInput({ + ...devServerInput, + mode: "dev", + port: undefined, + dryRun: true, + share: true, + }).pipe( + Effect.provide(Layer.mergeAll(emptyConfigLayer, netServiceLayer, spawnerLayer)), + Effect.provideService(HostProcessPlatform, "linux"), + ); + + assert.equal(spawnCount, 0); + }); + }); + it.effect("reports non-zero exits without manufacturing a cause", () => { const spawnerLayer = Layer.succeed( ChildProcessSpawner.ChildProcessSpawner, diff --git a/scripts/dev-runner.ts b/scripts/dev-runner.ts index 2875cbe88d3..973c3b0afd5 100644 --- a/scripts/dev-runner.ts +++ b/scripts/dev-runner.ts @@ -5,7 +5,7 @@ import * as NodeOS from "node:os"; import * as NodeRuntime from "@effect/platform-node/NodeRuntime"; import * as NodeServices from "@effect/platform-node/NodeServices"; import * as NetService from "@t3tools/shared/Net"; -import { resolveWorktreeT3Home } from "@t3tools/shared/devHome"; +import { resolveGitWorktreePath, resolveWorktreeT3Home } from "@t3tools/shared/devHome"; import { HostProcessEnvironment, HostProcessWorkingDirectory } from "@t3tools/shared/hostProcess"; import { resolveSpawnCommand } from "@t3tools/shared/shell"; import * as Config from "effect/Config"; @@ -19,6 +19,7 @@ import * as Schema from "effect/Schema"; import { Argument, Command, Flag } from "effect/unstable/cli"; import { ChildProcess } from "effect/unstable/process"; +import { DevShareError, shareDevServer, unshareDevServer } from "./lib/dev-share.ts"; import { loadRepoEnv } from "./lib/public-config.ts"; Object.assign(process.env, loadRepoEnv()); @@ -28,7 +29,12 @@ const BASE_WEB_PORT = 5733; const MAX_HASH_OFFSET = 3000; const MAX_PORT = 65535; const DESKTOP_DEV_LOOPBACK_HOST = "127.0.0.1"; -const DEV_PORT_PROBE_HOSTS = ["127.0.0.1", "0.0.0.0", "::1", "::"] as const; +// Dev servers bind loopback, so loopback is the only interface whose +// availability decides whether we can use a port. Probing wildcards too made +// the runner walk away from a perfectly free port whenever something else held +// the same number on another interface — `tailscale serve` does exactly that, +// which silently moved the ports out from under a URL that had just been shared. +const DEV_PORT_PROBE_HOSTS = ["127.0.0.1", "::1"] as const; export const DEFAULT_T3_HOME = Effect.map(Effect.service(Path.Path), (path) => path.join(NodeOS.homedir(), ".t3"), @@ -167,6 +173,7 @@ const OffsetConfig = Config.all({ export function resolveOffset(config: { readonly portOffset: number | undefined; readonly devInstance: string | undefined; + readonly worktreePath?: string | undefined; }): Effect.Effect< { readonly offset: number; readonly source: string }, DevRunnerInvalidPortOffsetError @@ -188,19 +195,30 @@ export function resolveOffset(config: { } const seed = config.devInstance?.trim(); - if (!seed) { - return Effect.succeed({ offset: 0, source: "default ports" }); + if (seed) { + if (/^\d+$/.test(seed)) { + return Effect.succeed({ + offset: Number(seed), + source: `numeric T3CODE_DEV_INSTANCE=${seed}`, + }); + } + + const offset = ((Hash.string(seed) >>> 0) % MAX_HASH_OFFSET) + 1; + return Effect.succeed({ offset, source: `hashed T3CODE_DEV_INSTANCE=${seed}` }); } - if (/^\d+$/.test(seed)) { - return Effect.succeed({ - offset: Number(seed), - source: `numeric T3CODE_DEV_INSTANCE=${seed}`, - }); + // Worktrees get ports derived from their path so each one is stable across + // restarts and distinct from its siblings. Without this every worktree starts + // at offset 0 and scan-collides onto whatever happens to be free that minute, + // so ports move under you between runs — which breaks any URL you already + // shared. The main checkout keeps the documented 5733/13773. + const worktreePath = config.worktreePath?.trim(); + if (worktreePath) { + const offset = ((Hash.string(worktreePath) >>> 0) % MAX_HASH_OFFSET) + 1; + return Effect.succeed({ offset, source: `worktree ${worktreePath}` }); } - const offset = ((Hash.string(seed) >>> 0) % MAX_HASH_OFFSET) + 1; - return Effect.succeed({ offset, source: `hashed T3CODE_DEV_INSTANCE=${seed}` }); + return Effect.succeed({ offset: 0, source: "default ports" }); } function resolveBaseDir(baseDir: string | undefined): Effect.Effect { @@ -246,8 +264,8 @@ export function createDevRunnerEnv({ return Effect.gen(function* () { const serverPort = port ?? BASE_SERVER_PORT + serverOffset; const webPort = BASE_WEB_PORT + webOffset; - // Precedence is resolved by the caller. An unset t3Home here genuinely - // means "use the default" rather than inheriting an ambient value. + // Precedence (--home-dir > worktree .t3 > ambient T3CODE_HOME) is resolved + // by the caller; an unset t3Home here genuinely means "use the default". const configuredBaseDir = t3Home?.trim() || undefined; const resolvedBaseDir = yield* resolveBaseDir(configuredBaseDir); const isDesktopMode = mode === "dev:desktop"; @@ -268,8 +286,20 @@ export function createDevRunnerEnv({ if (!isDesktopMode) { output.T3CODE_PORT = String(serverPort); - output.VITE_HTTP_URL = `http://localhost:${serverPort}`; - output.VITE_WS_URL = `ws://localhost:${serverPort}`; + if (mode === "dev" || mode === "dev:web") { + // Browser dev is single-origin: everything (including /ws) is proxied + // through Vite, so the client must resolve its backend from + // window.location.origin rather than a baked-in localhost URL. See + // resolveConfiguredPrimaryTarget in apps/web/src/environments/primary/target.ts + // — it only defers to the origin when both of these are absent. Baking + // localhost here is what breaks any non-localhost origin (tailnet, LAN, + // phone): the remote browser dials its own machine. + delete output.VITE_HTTP_URL; + delete output.VITE_WS_URL; + } else { + output.VITE_HTTP_URL = `http://localhost:${serverPort}`; + output.VITE_WS_URL = `ws://localhost:${serverPort}`; + } } else { output.T3CODE_PORT = String(serverPort); output.VITE_HTTP_URL = `http://${DESKTOP_DEV_LOOPBACK_HOST}:${serverPort}`; @@ -483,6 +513,7 @@ interface DevRunnerCliInput { readonly port: number | undefined; readonly devUrl: URL | undefined; readonly dryRun: boolean; + readonly share: boolean; readonly runArgs: ReadonlyArray; } @@ -498,7 +529,13 @@ export function runDevRunnerWithInput(input: DevRunnerCliInput) { ), ); - const { offset, source } = yield* resolveOffset({ portOffset, devInstance }); + const worktreePath = yield* resolveGitWorktreePath(yield* HostProcessWorkingDirectory); + + const { offset, source } = yield* resolveOffset({ + portOffset, + devInstance, + worktreePath, + }); const { serverOffset, webOffset } = yield* resolveModePortOffsets({ mode: input.mode, @@ -508,9 +545,9 @@ export function runDevRunnerWithInput(input: DevRunnerCliInput) { }); const hostEnvironment = yield* HostProcessEnvironment; - // A worktree defaults to its own gitignored `.t3`. This deliberately - // outranks ambient T3CODE_HOME, which otherwise selects the installed - // app's live userdata database. An explicit --home-dir still wins. + // A dev server started inside a worktree defaults to that worktree's own + // (gitignored) `.t3` — see @t3tools/shared/devHome for why this must + // outrank an ambient T3CODE_HOME. `--home-dir` still wins. const worktreeHome = yield* resolveWorktreeT3Home(yield* HostProcessWorkingDirectory); // Trim before choosing: `--home-dir ""` is not a selection, and treating it // as one would skip the worktree default and land on the shared home — @@ -543,10 +580,74 @@ export function runDevRunnerWithInput(input: DevRunnerCliInput) { `[dev-runner] mode=${input.mode} source=${source}${selectionSuffix} serverPort=${String(env.T3CODE_PORT)} webPort=${String(env.PORT)} baseDir=${baseDir}`, ); + // Before the share block: --dry-run only resolves and prints. Sharing would + // replace, then tear down, whatever mapping the port already had — a + // surprising side effect from a command documented as inert. if (input.dryRun) { return; } + const sharedWebPort = BASE_WEB_PORT + webOffset; + if (input.share) { + if (input.mode === "dev:server") { + yield* Effect.logInfo("[dev-runner] --share has no effect for dev:server (no web server)."); + } else { + // acquireRelease, not share-then-addFinalizer: the mapping outlives this + // process (and reboots), so the cleanup has to be registered atomically + // with creating it. An interrupt landing in between would otherwise + // leave a mapping pointing at a port nothing is listening on. + // + // A tailnet that isn't up shouldn't stop the dev server from starting — + // warn, and carry on serving locally. + const shared = yield* Effect.acquireRelease( + shareDevServer({ webPort: sharedWebPort }), + () => + // Serve config outlives this process, so a cleanup that did not + // take leaves a tailnet URL pointing at a port nothing serves. + unshareDevServer(sharedWebPort).pipe( + Effect.flatMap((result) => + result.cleared + ? Effect.void + : Effect.logWarning( + `[dev-runner] could not remove the tailnet mapping for port ${String(sharedWebPort)}${ + result.detail ? `: ${result.detail}` : "" + }. Remove it with \`tailscale serve --https=${String(sharedWebPort)} off\`.`, + ), + ), + ), + ).pipe( + Effect.tapError((error: DevShareError) => + Effect.logWarning( + `[dev-runner] could not share on the tailnet: ${error.message}${ + error.hint ? ` — ${error.hint}` : "" + }`, + ), + ), + Effect.option, + Effect.map(Option.getOrUndefined), + ); + + if (shared) { + // The app is reached from the tailnet origin. Vite already allows + // *.ts.net hosts; the backend needs the origin for credentialed + // requests that bypass the proxy (desktop renderer, direct calls). + env.T3CODE_DEV_ALLOWED_ORIGINS = [ + env.T3CODE_DEV_ALLOWED_ORIGINS, + new URL(shared.url).origin, + ] + .filter((entry) => entry && entry.length > 0) + .join(","); + // The server builds its pairing URL from this, so the URL printed at + // startup is already the shareable one — no rewriting by hand. An + // explicit --dev-url still wins. + if (input.devUrl === undefined) { + env.VITE_DEV_SERVER_URL = shared.url; + } + yield* Effect.logInfo(`[dev-runner] shared on tailnet: ${shared.url}`); + } + } + } + const spawnCommand = yield* resolveSpawnCommand( "vp", [...MODE_ARGS[input.mode], ...input.runArgs], @@ -606,7 +707,7 @@ const devRunnerCli = Command.make("dev-runner", { ), t3Home: Flag.string("home-dir").pipe( Flag.withDescription( - "Explicit T3 Code data directory; runtime state is stored under userdata. Inside a git worktree this defaults to that worktree's own .t3.", + "Explicit T3 Code data directory; runtime state is stored under userdata (equivalent to T3CODE_HOME). Inside a git worktree this defaults to that worktree's own .t3 so dev state stays off the shared home.", ), Flag.optional, Flag.map(Option.getOrUndefined), @@ -646,6 +747,12 @@ const devRunnerCli = Command.make("dev-runner", { Flag.withDescription("Resolve mode/ports/env and print, but do not spawn Vite+."), Flag.withDefault(false), ), + share: Flag.boolean("share").pipe( + Flag.withDescription( + "Publish the web dev server on this machine's tailnet over HTTPS (via `tailscale serve`) and print the pairing URL for it. Removed again on exit.", + ), + Flag.withDefault(false), + ), runArgs: Argument.string("run-arg").pipe( Argument.withDescription("Additional Vite+ run args (pass after `--`)."), Argument.variadic(), diff --git a/scripts/lib/dev-share.test.ts b/scripts/lib/dev-share.test.ts new file mode 100644 index 00000000000..89fc4eb564e --- /dev/null +++ b/scripts/lib/dev-share.test.ts @@ -0,0 +1,134 @@ +import { assert, describe, it } from "@effect/vitest"; +import * as Effect from "effect/Effect"; +import * as Layer from "effect/Layer"; +import * as Sink from "effect/Sink"; +import * as Stream from "effect/Stream"; +import { ChildProcessSpawner } from "effect/unstable/process"; + +import { DevShareError, shareDevServer, unshareDevServer } from "./dev-share.ts"; + +const TAILNET_STATUS = JSON.stringify({ Self: { DNSName: "host.example.ts.net." } }); +const NO_HANDLER_STDERR = "error: failed to remove web serve: handler does not exist"; + +interface CallResult { + readonly exitCode: number; + readonly stderr?: string; +} + +const encode = (value: string) => Stream.make(new TextEncoder().encode(value)); + +/** + * Answers `tailscale status --json` with a valid tailnet name, and lets each + * test set the outcome of the `off` (pre-clear) and `serve` calls separately — + * they are the same subcommand and are told apart by the trailing `off`. + */ +const spawnerLayer = (input: { readonly off?: CallResult; readonly serve?: CallResult }) => + Layer.succeed( + ChildProcessSpawner.ChildProcessSpawner, + ChildProcessSpawner.make((command) => { + const args = "args" in command ? (command.args as ReadonlyArray) : []; + const result: CallResult = args.includes("status") + ? { exitCode: 0 } + : args.includes("off") + ? (input.off ?? { exitCode: 0 }) + : (input.serve ?? { exitCode: 0 }); + + return Effect.succeed( + ChildProcessSpawner.makeHandle({ + pid: ChildProcessSpawner.ProcessId(1), + exitCode: Effect.succeed(ChildProcessSpawner.ExitCode(result.exitCode)), + isRunning: Effect.succeed(false), + kill: () => Effect.void, + unref: Effect.succeed(Effect.void), + stdin: Sink.drain, + stdout: args.includes("status") ? encode(TAILNET_STATUS) : Stream.empty, + stderr: result.stderr ? encode(result.stderr) : Stream.empty, + all: Stream.empty, + getInputFd: () => Sink.drain, + getOutputFd: () => Stream.empty, + }), + ); + }), + ); + +describe("unshareDevServer", () => { + it.effect("treats a removed mapping as cleared", () => + Effect.gen(function* () { + const result = yield* unshareDevServer(5788).pipe( + Effect.provide(spawnerLayer({ off: { exitCode: 0 } })), + ); + assert.isTrue(result.cleared); + }), + ); + + // `tailscale serve … off` exits 1 when the port had no mapping, which is the + // normal first-share case — the port is clear, so this must not be an error. + it.effect("treats a missing handler as cleared", () => + Effect.gen(function* () { + const result = yield* unshareDevServer(5788).pipe( + Effect.provide(spawnerLayer({ off: { exitCode: 1, stderr: NO_HANDLER_STDERR } })), + ); + assert.isTrue(result.cleared); + }), + ); + + it.effect("reports a genuine removal failure as not cleared", () => + Effect.gen(function* () { + const result = yield* unshareDevServer(5788).pipe( + Effect.provide(spawnerLayer({ off: { exitCode: 1, stderr: "permission denied" } })), + ); + assert.isFalse(result.cleared); + assert.include(result.detail, "permission denied"); + }), + ); +}); + +describe("shareDevServer", () => { + it.effect("returns the tailnet URL for the same port", () => + Effect.gen(function* () { + const shared = yield* shareDevServer({ webPort: 5788 }).pipe( + Effect.provide(spawnerLayer({ off: { exitCode: 1, stderr: NO_HANDLER_STDERR } })), + ); + + assert.equal(shared.host, "host.example.ts.net"); + assert.equal(shared.url, "https://host.example.ts.net:5788/"); + }), + ); + + // The stale-mapping clear runs before serve, so a failure here leaves the + // port serving nothing. Saying only "serve failed" would let an operator + // assume their previous mapping survived. + it.effect("reports that the prior mapping was cleared when serve fails", () => + Effect.gen(function* () { + const error: DevShareError = yield* shareDevServer({ webPort: 5788 }).pipe( + Effect.provide( + spawnerLayer({ + off: { exitCode: 0 }, + serve: { exitCode: 1, stderr: "port already in use" }, + }), + ), + Effect.flip, + ); + + assert.equal(error.reason, "serve-failed"); + assert.include(error.message, "port already in use"); + assert.include(error.message, "no longer served"); + assert.include(error.message, "5788"); + }), + ); + + // Serving over routes we could not remove yields a URL that loads but whose + // /ws and /api quietly point at a dead backend. + it.effect("refuses to serve when the existing mapping could not be cleared", () => + Effect.gen(function* () { + const error: DevShareError = yield* shareDevServer({ webPort: 5788 }).pipe( + Effect.provide(spawnerLayer({ off: { exitCode: 1, stderr: "permission denied" } })), + Effect.flip, + ); + + assert.equal(error.reason, "serve-failed"); + assert.include(error.message, "could not clear the existing mapping"); + assert.include(error.message, "permission denied"); + }), + ); +}); diff --git a/scripts/lib/dev-share.ts b/scripts/lib/dev-share.ts new file mode 100644 index 00000000000..863365fe42e --- /dev/null +++ b/scripts/lib/dev-share.ts @@ -0,0 +1,146 @@ +/** + * Shares a running dev server on the local tailnet via `tailscale serve`, so it + * can be opened from a phone, another laptop, or by whoever is reviewing the + * work. + * + * Thin wrapper over `@t3tools/tailscale` (the same client the server's own + * `--tailscale-serve` uses). What it adds is dev-share semantics: replacing a + * stale mapping left by a killed run, and refusing to serve over routes it + * could not remove. + * + * Because browser dev is single-origin (Vite proxies the backend — see + * `resolveDevProxyTarget` in apps/web/vite.config.ts), one proxy rule covering + * the web port is enough; the backend needs no mapping of its own. + */ + +import { + buildTailscaleHttpsBaseUrl, + disableTailscaleServe, + ensureTailscaleServe, + readTailscaleStatus, + type TailscaleCommandError, +} from "@t3tools/tailscale"; +import * as Effect from "effect/Effect"; +import * as Schema from "effect/Schema"; +import type { ChildProcessSpawner } from "effect/unstable/process"; + +export class DevShareError extends Schema.TaggedErrorClass()("DevShareError", { + reason: Schema.Literals(["tailscale-unavailable", "no-tailnet-name", "serve-failed"]), + detail: Schema.optional(Schema.String), +}) { + override get message(): string { + const base = { + "tailscale-unavailable": "could not talk to tailscale", + "no-tailnet-name": "this machine has no tailnet DNS name", + "serve-failed": "tailscale serve failed", + }[this.reason]; + return this.detail ? `${base}: ${this.detail}` : base; + } + + /** What the user can actually do about it. */ + get hint(): string | undefined { + return { + "tailscale-unavailable": + "Is Tailscale installed and tailscaled running? Try `tailscale status` — or drop --share and open the printed localhost URL.", + "no-tailnet-name": "Run `tailscale up` and make sure MagicDNS is enabled.", + "serve-failed": undefined, + }[this.reason]; + } +} + +const commandDetail = (error: TailscaleCommandError): string => + error._tag === "TailscaleCommandExitError" && error.stderrPreview !== undefined + ? `${error.message} ${error.stderrPreview}` + : error.message; + +/** + * `tailscale serve … off` exits nonzero with this when the port had no mapping, + * which is the normal case for a first-time share — not a failure. + */ +const NO_EXISTING_HANDLER_PATTERN = /handler does not exist/i; + +/** + * Removes any mapping for `webPort`, reporting whether the port is now clear. + * + * Runs uninterruptibly: this is called from a finalizer on the way out of an + * interrupted program, and cancelling the cleanup subprocess would leave + * exactly the stale mapping it exists to remove. + */ +export const unshareDevServer = ( + webPort: number, +): Effect.Effect< + { readonly cleared: boolean; readonly detail?: string | undefined }, + never, + ChildProcessSpawner.ChildProcessSpawner +> => + disableTailscaleServe({ servePort: webPort }).pipe( + Effect.as({ cleared: true } as const), + Effect.catch((error: TailscaleCommandError) => + Effect.succeed( + // "Nothing was mapped" leaves the port clear either way. + error._tag === "TailscaleCommandExitError" && + NO_EXISTING_HANDLER_PATTERN.test(error.stderrPreview ?? "") + ? ({ cleared: true } as const) + : ({ cleared: false, detail: commandDetail(error) } as const), + ), + ), + Effect.uninterruptible, + ); + +export interface DevShareResult { + readonly url: string; + readonly host: string; +} + +/** + * Publishes `webPort` on the tailnet at the same port number and returns the + * resulting HTTPS URL. Idempotent: re-running replaces any existing mapping. + */ +export const shareDevServer = Effect.fn("devShare.shareDevServer")(function* (input: { + readonly webPort: number; +}) { + const status = yield* readTailscaleStatus.pipe( + Effect.mapError( + (error) => new DevShareError({ reason: "tailscale-unavailable", detail: error.message }), + ), + ); + if (status.magicDnsName === null) { + return yield* new DevShareError({ reason: "no-tailnet-name" }); + } + + // Clear any mapping left behind by a run that was killed before its finalizer + // could fire. Serve config survives both the process and a reboot, and a + // stale entry may carry path routes we no longer want — older versions mapped + // /ws, /api and friends to a separate backend port, and serving "/" alone + // would leave those pointing at a port nothing is listening on. + const cleared = yield* unshareDevServer(input.webPort); + if (!cleared.cleared) { + // Serving over routes we failed to remove would hand out a URL that is + // broken in a way the user cannot see: the page loads while /ws and /api + // silently resolve to a dead backend. Better to refuse and say why. + return yield* new DevShareError({ + reason: "serve-failed", + detail: `could not clear the existing mapping for port ${String(input.webPort)}${ + cleared.detail ? `: ${cleared.detail}` : "" + }. Run \`tailscale serve --https=${String(input.webPort)} off\` and retry.`, + }); + } + + yield* ensureTailscaleServe({ localPort: input.webPort, servePort: input.webPort }).pipe( + Effect.mapError( + (error) => + new DevShareError({ + reason: "serve-failed", + detail: `${commandDetail(error)} (port ${String(input.webPort)} is no longer served; any previous mapping for it was cleared before this attempt)`, + }), + ), + ); + + return { + url: buildTailscaleHttpsBaseUrl({ + magicDnsName: status.magicDnsName, + servePort: input.webPort, + }), + host: status.magicDnsName, + } satisfies DevShareResult; +}); diff --git a/scripts/package.json b/scripts/package.json index 629163d688e..457a8f0d3a3 100644 --- a/scripts/package.json +++ b/scripts/package.json @@ -10,6 +10,7 @@ "@effect/platform-node": "catalog:", "@t3tools/contracts": "workspace:*", "@t3tools/shared": "workspace:*", + "@t3tools/tailscale": "workspace:*", "effect": "catalog:", "pngjs": "7.0.0", "yaml": "catalog:" From dbad3bc9f4f97704dd07cedba8bf2f8a18fdb165 Mon Sep 17 00:00:00 2001 From: Theo Browne Date: Sat, 25 Jul 2026 23:52:43 -0700 Subject: [PATCH 05/23] Decline --share for dev:desktop Desktop dev is not single-origin: the renderer gets VITE_HTTP_URL and VITE_WS_URL baked to loopback, so a tailnet visitor would load the UI and then watch it dial its own 127.0.0.1 for the backend. Sharing also overwrote VITE_DEV_SERVER_URL, which is the origin Electron loads the renderer from. Warn and carry on serving locally instead of handing out a URL that is broken in a way the user cannot see. Co-Authored-By: Claude Opus 5 (1M context) --- scripts/dev-runner.test.ts | 28 ++++++++++++++++++++++++++++ scripts/dev-runner.ts | 10 ++++++++++ 2 files changed, 38 insertions(+) diff --git a/scripts/dev-runner.test.ts b/scripts/dev-runner.test.ts index dba2c4de087..cd82f1e448d 100644 --- a/scripts/dev-runner.test.ts +++ b/scripts/dev-runner.test.ts @@ -654,6 +654,34 @@ it.layer(NodeServices.layer)("dev-runner", (it) => { }), ); + // Sharing dev:desktop would publish a URL whose renderer dials the + // visitor's own loopback, and would clobber the VITE_DEV_SERVER_URL that + // Electron loads from. It must decline, not half-work. + it.effect("declines to share for dev:desktop and still starts the stack", () => { + let spawnCount = 0; + const spawnerLayer = Layer.succeed( + ChildProcessSpawner.ChildProcessSpawner, + ChildProcessSpawner.make(() => { + spawnCount += 1; + return Effect.succeed(mockProcess(0)); + }), + ); + + return Effect.gen(function* () { + yield* runDevRunnerWithInput({ + ...devServerInput, + mode: "dev:desktop", + port: undefined, + share: true, + }).pipe( + Effect.provide(Layer.mergeAll(emptyConfigLayer, netServiceLayer, spawnerLayer)), + Effect.provideService(HostProcessPlatform, "linux"), + ); + + assert.equal(spawnCount, 1); + }); + }); + it.effect("spawns nothing when --dry-run is combined with --share", () => { let spawnCount = 0; const spawnerLayer = Layer.succeed( diff --git a/scripts/dev-runner.ts b/scripts/dev-runner.ts index 973c3b0afd5..f8f3843334c 100644 --- a/scripts/dev-runner.ts +++ b/scripts/dev-runner.ts @@ -591,6 +591,16 @@ export function runDevRunnerWithInput(input: DevRunnerCliInput) { if (input.share) { if (input.mode === "dev:server") { yield* Effect.logInfo("[dev-runner] --share has no effect for dev:server (no web server)."); + } else if (input.mode === "dev:desktop") { + // Desktop is not single-origin: the renderer gets VITE_HTTP_URL and + // VITE_WS_URL baked to loopback, so a tailnet visitor would load the UI + // and then watch it dial its own 127.0.0.1 for the backend. Worse, + // sharing would overwrite VITE_DEV_SERVER_URL, which is the origin + // Electron itself loads the renderer from. Refuse rather than hand out + // a URL that is broken in a way the user cannot see. + yield* Effect.logWarning( + "[dev-runner] --share is not supported for dev:desktop (the renderer is pinned to loopback). Use `dev` or `dev:web`.", + ); } else { // acquireRelease, not share-then-addFinalizer: the mapping outlives this // process (and reboots), so the cleanup has to be registered atomically From ce4664d9244828f632a62e3c2f32ee8b8c9d824f Mon Sep 17 00:00:00 2001 From: Theo Browne Date: Sun, 26 Jul 2026 00:00:48 -0700 Subject: [PATCH 06/23] Classify tailscale stderr instead of quoting it; probe the bind host MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two findings from review. stderrPreview carried the first 200 chars of raw CLI stderr into DevShareError.detail and on into dev-runner's logs. tailscale prints auth keys (tskey-...) and node names to stderr, and the package's own tests already seed a token there and assert it does not leak — stderrPreview walked straight past that invariant. Replace it with stderrDiagnostic, a closed set of labels (no-existing-handler, not-logged-in, permission-denied, unknown); callers match on the label and render our own wording. Unrecognized stderr degrades to "unknown" rather than falling back to text. Port probing was narrowed to loopback, but --host/T3CODE_HOST moves the backend onto another interface, and that interface decides whether the bind succeeds. A port free on loopback and taken there was selected and then failed to bind. Probe the configured host alongside loopback. Co-Authored-By: Claude Opus 5 (1M context) --- packages/tailscale/src/tailscale.test.ts | 38 ++++++++++++++++ packages/tailscale/src/tailscale.ts | 55 ++++++++++++++++++------ scripts/dev-runner.test.ts | 29 +++++++++++++ scripts/dev-runner.ts | 40 ++++++++++++++--- scripts/lib/dev-share.test.ts | 29 ++++++++++++- scripts/lib/dev-share.ts | 30 +++++++++---- 6 files changed, 190 insertions(+), 31 deletions(-) diff --git a/packages/tailscale/src/tailscale.test.ts b/packages/tailscale/src/tailscale.test.ts index 853bb1f81c2..a1c16232157 100644 --- a/packages/tailscale/src/tailscale.test.ts +++ b/packages/tailscale/src/tailscale.test.ts @@ -25,6 +25,20 @@ import { } from "./tailscale.ts"; const encoder = new TextEncoder(); + +/** + * Asserts no field of `error` (nor its message) contains `secret`. Walks the + * own enumerable values rather than serializing, so it holds for any field + * added later without the test needing to know the shape. + */ +function assertCarriesNoSecret(error: object, secret: string): void { + assert.notInclude(String((error as { message?: unknown }).message ?? ""), secret); + for (const [key, value] of Object.entries(error)) { + if (typeof value === "string") { + assert.notInclude(value, secret, `field "${key}" leaked stderr`); + } + } +} const tailscaleStatusJson = `{"Self":{"DNSName":"desktop.tail.ts.net.","TailscaleIPs":["100.100.100.100","fd7a:115c:a1e0::1","192.168.1.20"]}}`; const tailscaleStatusWithSingleIpJson = `{"Self":{"DNSName":"desktop.tail.ts.net.","TailscaleIPs":["100.90.1.2"]}}`; @@ -194,6 +208,26 @@ describe("tailscale", () => { assert.notProperty(error, "stderr"); assert.notInclude(error.message, "tskey-auth-secret-token-value"); assert.equal(error.message, "tailscale status exited with code 7."); + assert.equal(error.stderrDiagnostic, "not-logged-in"); + assertCarriesNoSecret(error, "tskey-auth-secret-token-value"); + }); + }); + + it.effect("classifies unrecognized stderr without quoting it", () => { + const layer = mockSpawnerLayer(() => ({ + code: 3, + stderr: "something novel went wrong for node fluffy-badger tskey-auth-secret-token-value", + })); + + return Effect.gen(function* () { + const error = yield* readTailscaleStatus.pipe(Effect.flip, Effect.provide(layer)); + + assert.instanceOf(error, TailscaleCommandExitError); + // Unmatched stderr degrades to "unknown" rather than passing text + // through — that fallback is what keeps novel output from leaking. + assert.equal(error.stderrDiagnostic, "unknown"); + assertCarriesNoSecret(error, "tskey-auth-secret-token-value"); + assertCarriesNoSecret(error, "fluffy-badger"); }); }); @@ -253,6 +287,10 @@ describe("tailscale", () => { assert.notProperty(error, "command"); assert.notProperty(error, "stderr"); assert.notInclude(error.message, "tskey-auth-secret-token-value"); + // The diagnostic classifies the failure without quoting stderr, so the + // key cannot reach a log through it either. + assert.equal(error.stderrDiagnostic, "permission-denied"); + assertCarriesNoSecret(error, "tskey-auth-secret-token-value"); }); }); diff --git a/packages/tailscale/src/tailscale.ts b/packages/tailscale/src/tailscale.ts index b40ead671bc..7260a9de11b 100644 --- a/packages/tailscale/src/tailscale.ts +++ b/packages/tailscale/src/tailscale.ts @@ -23,6 +23,37 @@ const TailscaleCommandContext = { argumentCount: Schema.Number, }; +/** + * Failure kinds we can name without quoting the CLI. Anything unrecognized + * becomes "unknown" rather than falling back to raw text — stderr can contain + * auth keys (`tskey-…`) and node names, and these labels are logged. + */ +export const TailscaleStderrDiagnostic = Schema.Literals([ + "no-existing-handler", + "not-logged-in", + "permission-denied", + "unknown", +]); +export type TailscaleStderrDiagnostic = typeof TailscaleStderrDiagnostic.Type; + +// Matched against stderr, most specific first. Patterns are deliberately short +// and anchored on tailscale's own wording. +const STDERR_DIAGNOSTIC_PATTERNS: ReadonlyArray< + readonly [RegExp, Exclude] +> = [ + [/handler does not exist/i, "no-existing-handler"], + [/not logged in|logged out|needs? login/i, "not-logged-in"], + [/permission denied|access denied|must be root|operation not permitted/i, "permission-denied"], +]; + +/** Classifies stderr into a safe label, dropping the text itself. */ +export const stderrDiagnosticOf = (stderr: string): TailscaleStderrDiagnostic | undefined => { + if (stderr.trim().length === 0) { + return undefined; + } + return STDERR_DIAGNOSTIC_PATTERNS.find(([pattern]) => pattern.test(stderr))?.[1] ?? "unknown"; +}; + export class TailscaleCommandSpawnError extends Schema.TaggedErrorClass()( "TailscaleCommandSpawnError", { @@ -54,9 +85,12 @@ export class TailscaleCommandExitError extends Schema.TaggedErrorClass { - const trimmed = stderr.trim(); - return trimmed.length > 0 ? trimmed.slice(0, STDERR_PREVIEW_LIMIT) : undefined; -}; - export class TailscaleCommandTimeoutError extends Schema.TaggedErrorClass()( "TailscaleCommandTimeoutError", { @@ -222,8 +249,8 @@ export const readTailscaleStatus = Effect.gen(function* () { exitCode, stdoutLength: stdout.length, stderrLength: stderr.length, - ...(stderrPreviewOf(stderr) !== undefined - ? { stderrPreview: stderrPreviewOf(stderr) } + ...(stderrDiagnosticOf(stderr) !== undefined + ? { stderrDiagnostic: stderrDiagnosticOf(stderr) } : {}), }); } @@ -288,8 +315,8 @@ const runTailscaleCommand = ( ...commandContext, exitCode, stderrLength: stderr.length, - ...(stderrPreviewOf(stderr) !== undefined - ? { stderrPreview: stderrPreviewOf(stderr) } + ...(stderrDiagnosticOf(stderr) !== undefined + ? { stderrDiagnostic: stderrDiagnosticOf(stderr) } : {}), }); } diff --git a/scripts/dev-runner.test.ts b/scripts/dev-runner.test.ts index cd82f1e448d..8d8816addc1 100644 --- a/scripts/dev-runner.test.ts +++ b/scripts/dev-runner.test.ts @@ -22,6 +22,7 @@ import { ChildProcessSpawner } from "effect/unstable/process"; import { checkPortAvailabilityOnHosts, createDevRunnerEnv, + devPortProbeHosts, findFirstAvailableOffset, getDevRunnerModeArgs, resolveModePortOffsets, @@ -495,6 +496,34 @@ it.layer(NodeServices.layer)("dev-runner", (it) => { ); }); + describe("devPortProbeHosts", () => { + it.effect("probes loopback only when no bind host is configured", () => + Effect.sync(() => { + assert.deepStrictEqual(devPortProbeHosts(undefined), ["127.0.0.1", "::1"]); + assert.deepStrictEqual(devPortProbeHosts(" "), ["127.0.0.1", "::1"]); + }), + ); + + // A port free on loopback can be taken on the interface the server will + // actually bind, so --host/T3CODE_HOST has to be probed as well. + it.effect("adds a non-loopback bind host to the probe list", () => + Effect.sync(() => { + assert.deepStrictEqual(devPortProbeHosts("0.0.0.0"), ["127.0.0.1", "::1", "0.0.0.0"]); + assert.deepStrictEqual(devPortProbeHosts("192.168.1.10"), [ + "127.0.0.1", + "::1", + "192.168.1.10", + ]); + }), + ); + + it.effect("does not probe loopback twice when it is the configured host", () => + Effect.sync(() => { + assert.deepStrictEqual(devPortProbeHosts("127.0.0.1"), ["127.0.0.1", "::1"]); + }), + ); + }); + describe("resolveModePortOffsets", () => { it.effect("uses a shared fallback offset for dev mode", () => Effect.gen(function* () { diff --git a/scripts/dev-runner.ts b/scripts/dev-runner.ts index f8f3843334c..3e23848a6a7 100644 --- a/scripts/dev-runner.ts +++ b/scripts/dev-runner.ts @@ -374,13 +374,36 @@ export function checkPortAvailabilityOnHosts( }); } -const defaultCheckPortAvailability: PortAvailabilityCheck = (port) => - Effect.gen(function* () { - const net = yield* NetService.NetService; - return yield* checkPortAvailabilityOnHosts(port, DEV_PORT_PROBE_HOSTS, (candidatePort, host) => - net.canListenOnHost(candidatePort, host), - ); - }); +/** + * Hosts to probe for a dev server bound to `configuredHost`. + * + * Loopback is always checked because the web server and the desktop renderer + * target reach it there. When `--host`/`T3CODE_HOST` moves the backend onto + * another interface, that interface decides whether the bind actually + * succeeds — probing only loopback would hand back a port that is free here + * and taken there, and the server would fail to start. + */ +export function devPortProbeHosts(configuredHost: string | undefined): ReadonlyArray { + const host = configuredHost?.trim(); + if (!host || DEV_PORT_PROBE_HOSTS.includes(host as (typeof DEV_PORT_PROBE_HOSTS)[number])) { + return DEV_PORT_PROBE_HOSTS; + } + return [...DEV_PORT_PROBE_HOSTS, host]; +} + +const makeDefaultCheckPortAvailability = + (configuredHost: string | undefined): PortAvailabilityCheck => + (port) => + Effect.gen(function* () { + const net = yield* NetService.NetService; + return yield* checkPortAvailabilityOnHosts( + port, + devPortProbeHosts(configuredHost), + (candidatePort, host) => net.canListenOnHost(candidatePort, host), + ); + }); + +const defaultCheckPortAvailability = makeDefaultCheckPortAvailability(undefined); interface FindFirstAvailableOffsetInput { readonly startOffset: number; @@ -542,6 +565,9 @@ export function runDevRunnerWithInput(input: DevRunnerCliInput) { startOffset: offset, hasExplicitServerPort: input.port !== undefined, hasExplicitDevUrl: input.devUrl !== undefined, + // A non-loopback bind host decides whether the backend can actually take + // the port, so it has to be probed alongside loopback. + checkPortAvailability: makeDefaultCheckPortAvailability(input.host), }); const hostEnvironment = yield* HostProcessEnvironment; diff --git a/scripts/lib/dev-share.test.ts b/scripts/lib/dev-share.test.ts index 89fc4eb564e..c974ac916f9 100644 --- a/scripts/lib/dev-share.test.ts +++ b/scripts/lib/dev-share.test.ts @@ -111,12 +111,39 @@ describe("shareDevServer", () => { ); assert.equal(error.reason, "serve-failed"); - assert.include(error.message, "port already in use"); + // Unclassifiable stderr is never quoted (it can carry auth keys), so the + // message points at the command instead of echoing the CLI. + assert.notInclude(error.message, "port already in use"); + assert.include(error.message, "Run the command by hand"); assert.include(error.message, "no longer served"); assert.include(error.message, "5788"); }), ); + // A recognized failure gets our own wording for it — enough to act on + // without passing CLI text through. + it.effect("explains a recognized serve failure without quoting stderr", () => + Effect.gen(function* () { + const error: DevShareError = yield* shareDevServer({ webPort: 5788 }).pipe( + Effect.provide( + spawnerLayer({ + off: { exitCode: 0 }, + serve: { + exitCode: 1, + stderr: "permission denied for tskey-auth-secret-token-value", + }, + }), + ), + Effect.flip, + ); + + assert.equal(error.reason, "serve-failed"); + assert.include(error.message, "permission denied"); + assert.include(error.message, "elevated privileges"); + assert.notInclude(error.message, "tskey-auth-secret-token-value"); + }), + ); + // Serving over routes we could not remove yields a URL that loads but whose // /ws and /api quietly point at a dead backend. it.effect("refuses to serve when the existing mapping could not be cleared", () => diff --git a/scripts/lib/dev-share.ts b/scripts/lib/dev-share.ts index 863365fe42e..a82f92e26cb 100644 --- a/scripts/lib/dev-share.ts +++ b/scripts/lib/dev-share.ts @@ -19,6 +19,7 @@ import { ensureTailscaleServe, readTailscaleStatus, type TailscaleCommandError, + type TailscaleStderrDiagnostic, } from "@t3tools/tailscale"; import * as Effect from "effect/Effect"; import * as Schema from "effect/Schema"; @@ -48,16 +49,27 @@ export class DevShareError extends Schema.TaggedErrorClass()("Dev } } -const commandDetail = (error: TailscaleCommandError): string => - error._tag === "TailscaleCommandExitError" && error.stderrPreview !== undefined - ? `${error.message} ${error.stderrPreview}` - : error.message; - /** - * `tailscale serve … off` exits nonzero with this when the port had no mapping, - * which is the normal case for a first-time share — not a failure. + * Human-readable gloss for each diagnostic. Deliberately our own words rather + * than the CLI's: tailscale prints auth keys and node names into stderr, and + * this string is logged. */ -const NO_EXISTING_HANDLER_PATTERN = /handler does not exist/i; +const DIAGNOSTIC_EXPLANATIONS: Record = { + "no-existing-handler": "no mapping existed for that port", + "not-logged-in": "this machine is not logged into a tailnet — run `tailscale up`", + "permission-denied": "permission denied — `tailscale serve` may need elevated privileges", + unknown: undefined, +}; + +const commandDetail = (error: TailscaleCommandError): string => { + if (error._tag !== "TailscaleCommandExitError" || error.stderrDiagnostic === undefined) { + return error.message; + } + const explanation = DIAGNOSTIC_EXPLANATIONS[error.stderrDiagnostic]; + return explanation === undefined + ? `${error.message} Run the command by hand to see why.` + : `${error.message} ${explanation}.`; +}; /** * Removes any mapping for `webPort`, reporting whether the port is now clear. @@ -79,7 +91,7 @@ export const unshareDevServer = ( Effect.succeed( // "Nothing was mapped" leaves the port clear either way. error._tag === "TailscaleCommandExitError" && - NO_EXISTING_HANDLER_PATTERN.test(error.stderrPreview ?? "") + error.stderrDiagnostic === "no-existing-handler" ? ({ cleared: true } as const) : ({ cleared: false, detail: commandDetail(error) } as const), ), From 2869a9e848c11db1904b1e4d0f0bf4fc605dd182 Mon Sep 17 00:00:00 2001 From: Theo Browne Date: Sun, 26 Jul 2026 00:05:10 -0700 Subject: [PATCH 07/23] Signal single-origin dev positively so .env cannot revive backend URLs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The runner deleted VITE_HTTP_URL/VITE_WS_URL for dev and dev:web, but vite.config.ts calls loadRepoEnv, which merges .env/.env.local underneath the process env. A developer with either URL in their .env got it back, and Vite baked it into the bundle — pinning the client to localhost and breaking every non-localhost origin. Invisible, too: the page still loads, then dials the visitor's own machine. Absence is not a usable signal, so state the intent: the runner sets T3CODE_SINGLE_ORIGIN_DEV=1, and Vite ignores both URLs when it is set. VITE_HTTP_URL is now pinned in `define` as well, since Vite's automatic VITE_ exposure would otherwise leak it past the check. Verified by A/B with a .env setting both URLs: the served module carries "http://localhost:13773" without this change and "" with it, proxying still 200. Also point the dev:desktop --share refusal at `dev` rather than `dev:web`, which starts no backend of its own. Co-Authored-By: Claude Opus 5 (1M context) --- apps/web/vite.config.ts | 16 ++++++++++++- scripts/dev-runner.test.ts | 48 ++++++++++++++++++++++++++++++++++++++ scripts/dev-runner.ts | 12 +++++++++- 3 files changed, 74 insertions(+), 2 deletions(-) diff --git a/apps/web/vite.config.ts b/apps/web/vite.config.ts index c8e0c7abae8..e1d590ecdf2 100644 --- a/apps/web/vite.config.ts +++ b/apps/web/vite.config.ts @@ -14,10 +14,20 @@ import { loadRepoEnv } from "../../scripts/lib/public-config"; const repoEnv = loadRepoEnv(); Object.assign(process.env, repoEnv); +// Single-origin dev is signalled positively, because it cannot be inferred +// from the absence of VITE_HTTP_URL/VITE_WS_URL: the runner deletes those keys +// but `loadRepoEnv` merges `.env`/`.env.local` *underneath* the process env, so +// a developer with either URL in their `.env` gets it back here. Baking it then +// pins the client to localhost and breaks every non-localhost origin — the +// exact failure single-origin mode exists to prevent, and an invisible one +// since the page still loads. +const isSingleOriginDev = process.env.T3CODE_SINGLE_ORIGIN_DEV === "1"; + const port = Number(process.env.PORT ?? 5733); const explicitHost = process.env.HOST?.trim(); const host = explicitHost || "localhost"; -const configuredWsUrl = process.env.VITE_WS_URL?.trim(); +const configuredWsUrl = isSingleOriginDev ? undefined : process.env.VITE_WS_URL?.trim(); +const configuredHttpUrl = isSingleOriginDev ? undefined : process.env.VITE_HTTP_URL?.trim(); const configuredRelayUrl = repoEnv.VITE_T3CODE_RELAY_URL?.trim() || ""; const configuredClerkPublishableKey = repoEnv.VITE_CLERK_PUBLISHABLE_KEY?.trim() || ""; const configuredClerkJwtTemplate = repoEnv.VITE_CLERK_JWT_TEMPLATE?.trim() || ""; @@ -145,6 +155,10 @@ export default defineConfig(() => { define: { // In dev mode, tell the web app where the WebSocket server lives "import.meta.env.VITE_WS_URL": JSON.stringify(configuredWsUrl ?? ""), + // Pinned explicitly rather than left to Vite's automatic VITE_ exposure: + // under single-origin dev this must stay empty even when a `.env` + // supplies it, so the client falls back to window.location.origin. + "import.meta.env.VITE_HTTP_URL": JSON.stringify(configuredHttpUrl ?? ""), "import.meta.env.VITE_T3CODE_RELAY_URL": JSON.stringify(configuredRelayUrl), "import.meta.env.VITE_CLERK_PUBLISHABLE_KEY": JSON.stringify(configuredClerkPublishableKey), "import.meta.env.VITE_CLERK_JWT_TEMPLATE": JSON.stringify(configuredClerkJwtTemplate), diff --git a/scripts/dev-runner.test.ts b/scripts/dev-runner.test.ts index 8d8816addc1..c56cdfe6626 100644 --- a/scripts/dev-runner.test.ts +++ b/scripts/dev-runner.test.ts @@ -376,10 +376,58 @@ it.layer(NodeServices.layer)("dev-runner", (it) => { assert.equal(env.VITE_HTTP_URL, undefined); assert.equal(env.VITE_WS_URL, undefined); assert.equal(env.T3CODE_PORT, "13773"); + // Deleting the keys is not sufficient — vite.config.ts merges + // `.env`/`.env.local` underneath this env and would revive them, so + // the intent has to be stated positively. + assert.equal(env.T3CODE_SINGLE_ORIGIN_DEV, "1"); }), ); } + // Desktop pins the renderer at loopback deliberately; an ambient marker + // must not make Vite discard those URLs. + it.effect("clears the single-origin marker in dev:desktop mode", () => + Effect.gen(function* () { + const env = yield* createDevRunnerEnv({ + mode: "dev:desktop", + baseEnv: { T3CODE_SINGLE_ORIGIN_DEV: "1" }, + serverOffset: 0, + webOffset: 0, + t3Home: undefined, + browser: undefined, + autoBootstrapProjectFromCwd: undefined, + logWebSocketEvents: undefined, + host: undefined, + port: undefined, + devUrl: undefined, + }); + + assert.equal(env.T3CODE_SINGLE_ORIGIN_DEV, undefined); + assert.equal(env.VITE_HTTP_URL, "http://127.0.0.1:13773"); + }), + ); + + it.effect("clears the single-origin marker in dev:server mode", () => + Effect.gen(function* () { + const env = yield* createDevRunnerEnv({ + mode: "dev:server", + baseEnv: { T3CODE_SINGLE_ORIGIN_DEV: "1" }, + serverOffset: 0, + webOffset: 0, + t3Home: undefined, + browser: undefined, + autoBootstrapProjectFromCwd: undefined, + logWebSocketEvents: undefined, + host: undefined, + port: undefined, + devUrl: undefined, + }); + + assert.equal(env.T3CODE_SINGLE_ORIGIN_DEV, undefined); + assert.equal(env.VITE_HTTP_URL, "http://localhost:13773"); + }), + ); + it.effect("keeps explicit backend URLs for the desktop renderer", () => Effect.gen(function* () { const env = yield* createDevRunnerEnv({ diff --git a/scripts/dev-runner.ts b/scripts/dev-runner.ts index 3e23848a6a7..7e5e19f7927 100644 --- a/scripts/dev-runner.ts +++ b/scripts/dev-runner.ts @@ -296,14 +296,24 @@ export function createDevRunnerEnv({ // phone): the remote browser dials its own machine. delete output.VITE_HTTP_URL; delete output.VITE_WS_URL; + // Deleting is not enough on its own: vite.config.ts calls loadRepoEnv, + // which merges `.env`/`.env.local` *under* this env, so a developer + // with either URL in their `.env` would get it back and silently lose + // single-origin mode. This states the intent positively so Vite can + // ignore those values rather than infer from their absence. + output.T3CODE_SINGLE_ORIGIN_DEV = "1"; } else { output.VITE_HTTP_URL = `http://localhost:${serverPort}`; output.VITE_WS_URL = `ws://localhost:${serverPort}`; + delete output.T3CODE_SINGLE_ORIGIN_DEV; } } else { output.T3CODE_PORT = String(serverPort); output.VITE_HTTP_URL = `http://${DESKTOP_DEV_LOOPBACK_HOST}:${serverPort}`; output.VITE_WS_URL = `ws://${DESKTOP_DEV_LOOPBACK_HOST}:${serverPort}`; + // Desktop pins the renderer to loopback on purpose; an ambient marker + // must not make Vite drop those URLs. + delete output.T3CODE_SINGLE_ORIGIN_DEV; delete output.T3CODE_MODE; delete output.T3CODE_NO_BROWSER; delete output.T3CODE_HOST; @@ -625,7 +635,7 @@ export function runDevRunnerWithInput(input: DevRunnerCliInput) { // Electron itself loads the renderer from. Refuse rather than hand out // a URL that is broken in a way the user cannot see. yield* Effect.logWarning( - "[dev-runner] --share is not supported for dev:desktop (the renderer is pinned to loopback). Use `dev` or `dev:web`.", + "[dev-runner] --share is not supported for dev:desktop (the renderer is pinned to loopback). Use `dev`, which runs the whole browser stack.", ); } else { // acquireRelease, not share-then-addFinalizer: the mapping outlives this From 37e1c7978ce827fa4c8fd5cf1585be4969dd4a52 Mon Sep 17 00:00:00 2001 From: Theo Browne Date: Sun, 26 Jul 2026 00:07:37 -0700 Subject: [PATCH 08/23] Apply the bind host to the server port only --host/T3CODE_HOST configures the backend. Vite takes its bind address from HOST, which the runner sets for desktop alone, so the web port stays on loopback. Probing it against the backend's interface could reject a port that was free where Vite would actually bind, shifting offsets or failing startup for no reason. The checker now takes the port's role. Passed explicitly rather than inferred from the port number, which stops discriminating once a large T3CODE_PORT_OFFSET pushes the web port past the server base. Co-Authored-By: Claude Opus 5 (1M context) --- scripts/dev-runner.test.ts | 25 +++++++++++++++++++++++++ scripts/dev-runner.ts | 29 +++++++++++++++++++++-------- 2 files changed, 46 insertions(+), 8 deletions(-) diff --git a/scripts/dev-runner.test.ts b/scripts/dev-runner.test.ts index c56cdfe6626..47cce2b26ed 100644 --- a/scripts/dev-runner.test.ts +++ b/scripts/dev-runner.test.ts @@ -570,6 +570,31 @@ it.layer(NodeServices.layer)("dev-runner", (it) => { assert.deepStrictEqual(devPortProbeHosts("127.0.0.1"), ["127.0.0.1", "::1"]); }), ); + + // Only the backend honours --host/T3CODE_HOST. Vite reads HOST (set for + // desktop only), so judging the web port against the backend's interface + // would reject ports for a server that never binds there. + it.effect("passes the port role so only the server port sees the bind host", () => + Effect.gen(function* () { + const probed: Array<{ port: number; role: string | undefined }> = []; + + yield* resolveModePortOffsets({ + mode: "dev", + startOffset: 0, + hasExplicitServerPort: false, + hasExplicitDevUrl: false, + checkPortAvailability: (port, role) => { + probed.push({ port, role }); + return Effect.succeed(true); + }, + }); + + assert.deepStrictEqual(probed, [ + { port: 13_773, role: "server" }, + { port: 5733, role: "web" }, + ]); + }), + ); }); describe("resolveModePortOffsets", () => { diff --git a/scripts/dev-runner.ts b/scripts/dev-runner.ts index 7e5e19f7927..c4e530252ff 100644 --- a/scripts/dev-runner.ts +++ b/scripts/dev-runner.ts @@ -55,7 +55,15 @@ const MODE_ARGS = { } as const satisfies Record>; type DevMode = keyof typeof MODE_ARGS; -type PortAvailabilityCheck = (port: number) => Effect.Effect; +/** + * `role` matters because only the backend honours `--host`/`T3CODE_HOST`; the + * web port is always loopback. Passed explicitly rather than inferred from the + * port number, which stops distinguishing them under a large port offset. + */ +type PortAvailabilityCheck = ( + port: number, + role?: "server" | "web", +) => Effect.Effect; const DEV_RUNNER_MODES = Object.keys(MODE_ARGS) as Array; @@ -392,6 +400,12 @@ export function checkPortAvailabilityOnHosts( * another interface, that interface decides whether the bind actually * succeeds — probing only loopback would hand back a port that is free here * and taken there, and the server would fail to start. + * + * `configuredHost` applies to the *backend* only. Vite takes its bind address + * from `HOST`, which the runner sets for desktop alone, so the web port stays + * on loopback and must not be judged against the backend's interface — + * a port free on loopback but busy on that interface would otherwise be + * rejected for a server that was never going to bind there. */ export function devPortProbeHosts(configuredHost: string | undefined): ReadonlyArray { const host = configuredHost?.trim(); @@ -403,13 +417,12 @@ export function devPortProbeHosts(configuredHost: string | undefined): ReadonlyA const makeDefaultCheckPortAvailability = (configuredHost: string | undefined): PortAvailabilityCheck => - (port) => + (port, role) => Effect.gen(function* () { const net = yield* NetService.NetService; - return yield* checkPortAvailabilityOnHosts( - port, - devPortProbeHosts(configuredHost), - (candidatePort, host) => net.canListenOnHost(candidatePort, host), + const hosts = role === "web" ? DEV_PORT_PROBE_HOSTS : devPortProbeHosts(configuredHost); + return yield* checkPortAvailabilityOnHosts(port, hosts, (candidatePort, host) => + net.canListenOnHost(candidatePort, host), ); }); @@ -447,10 +460,10 @@ export function findFirstAvailableOffset({ const checks: Array> = []; if (requireServerPort) { - checks.push(checkPort(serverPort)); + checks.push(checkPort(serverPort, "server")); } if (requireWebPort) { - checks.push(checkPort(webPort)); + checks.push(checkPort(webPort, "web")); } if (checks.length === 0) { From b50efd54689b1932eca31a7cf6db98758b775dc6 Mon Sep 17 00:00:00 2001 From: Theo Browne Date: Sun, 26 Jul 2026 00:34:40 -0700 Subject: [PATCH 09/23] Address review: recursive secret check, pinned test port, bun docs Make assertCarriesNoSecret recurse. It checked only top-level strings, so a leak inside a nested cause or an array would have passed while the comment claimed coverage for fields added later. Verified the gap is real: the shallow version passes on both a nested cause and an array carrying the token, and the recursive one fails on each. Walks message/cause explicitly (getters on Error subclasses are not own enumerable props) and tracks visited objects so cyclic causes terminate. Pin port after overrides in makeServerConfigLayer. makeCookieRequest builds the cookie name from TEST_SERVER_PORT, so an override that changed the port would leave the server reading one cookie name while requests sent another. Use bun run in docs/reference/scripts.md for the dev and dev:share lines, matching AGENTS.md. Confirmed `bun run dev --share` forwards the flag. Co-Authored-By: Claude Opus 5 (1M context) --- apps/server/src/auth/EnvironmentAuth.test.ts | 7 +++- docs/reference/scripts.md | 4 +-- packages/tailscale/src/tailscale.test.ts | 38 ++++++++++++++++---- 3 files changed, 39 insertions(+), 10 deletions(-) diff --git a/apps/server/src/auth/EnvironmentAuth.test.ts b/apps/server/src/auth/EnvironmentAuth.test.ts index 1970948088a..ba3f67c237e 100644 --- a/apps/server/src/auth/EnvironmentAuth.test.ts +++ b/apps/server/src/auth/EnvironmentAuth.test.ts @@ -22,8 +22,13 @@ const makeServerConfigLayer = (overrides?: Partial while every request still sent t3_session_13773, + // and the tests would fail for a reason unrelated to what they assert. + port: TEST_SERVER_PORT, } satisfies ServerConfig.ServerConfig["Service"]; }), ).pipe(Layer.provide(ServerConfig.layerTest(process.cwd(), { prefix: "t3-auth-server-test-" }))); diff --git a/docs/reference/scripts.md b/docs/reference/scripts.md index 2697af9ea15..b96186d9322 100644 --- a/docs/reference/scripts.md +++ b/docs/reference/scripts.md @@ -1,7 +1,7 @@ # Scripts -- `vp run dev` — Starts contracts, server, and web in watch mode. -- `vp run dev --share` (or `vp run dev:share`) — Also publishes the web port over HTTPS on this machine's tailnet. The startup pairing URL is built against the shared origin, and the mapping is removed on exit. +- `bun run dev` — Starts contracts, server, and web in watch mode. +- `bun run dev --share` (or `bun run dev:share`) — Also publishes the web port over HTTPS on this machine's tailnet. The startup pairing URL is built against the shared origin, and the mapping is removed on exit. - `vp run dev:server` — Starts just the WebSocket server. The server process runs on Bun (`@effect/platform-bun` + `BunPtyAdapter`), but task running uses `vp run`. - `vp run dev:web` — Starts just the Vite dev server for the web app. - Dev commands run from a linked **git worktree** default to that worktree's gitignored `.t3`, even when `T3CODE_HOME` is set, storing state in `/.t3/userdata`. Pass `--home-dir ` to choose another isolated directory explicitly. Submodules are not worktrees and keep the normal precedence. diff --git a/packages/tailscale/src/tailscale.test.ts b/packages/tailscale/src/tailscale.test.ts index a1c16232157..24c22454d9d 100644 --- a/packages/tailscale/src/tailscale.test.ts +++ b/packages/tailscale/src/tailscale.test.ts @@ -27,17 +27,41 @@ import { const encoder = new TextEncoder(); /** - * Asserts no field of `error` (nor its message) contains `secret`. Walks the - * own enumerable values rather than serializing, so it holds for any field - * added later without the test needing to know the shape. + * Asserts nothing reachable from `error` contains `secret`. Recurses through + * nested objects, arrays, and `cause` chains rather than checking only + * top-level strings: a leak one level down (say, a wrapped cause carrying raw + * stderr) is just as visible in a log, and a shallow check would pass it. + * + * Walks values instead of serializing so it holds for fields added later, and + * tracks visited objects so a cyclic cause chain terminates. */ function assertCarriesNoSecret(error: object, secret: string): void { - assert.notInclude(String((error as { message?: unknown }).message ?? ""), secret); - for (const [key, value] of Object.entries(error)) { + const seen = new WeakSet(); + + const walk = (value: unknown, path: string): void => { if (typeof value === "string") { - assert.notInclude(value, secret, `field "${key}" leaked stderr`); + assert.notInclude(value, secret, `${path} leaked stderr`); + return; + } + if (typeof value !== "object" || value === null || seen.has(value)) { + return; + } + seen.add(value); + + if (Array.isArray(value)) { + value.forEach((entry, index) => walk(entry, `${path}[${String(index)}]`)); + return; + } + // `message` and `cause` are getters on Error subclasses, so they are not + // own enumerable properties and Object.entries alone would skip them. + walk((value as { message?: unknown }).message, `${path}.message`); + walk((value as { cause?: unknown }).cause, `${path}.cause`); + for (const [key, nested] of Object.entries(value)) { + walk(nested, `${path}.${key}`); } - } + }; + + walk(error, "error"); } const tailscaleStatusJson = `{"Self":{"DNSName":"desktop.tail.ts.net.","TailscaleIPs":["100.100.100.100","fd7a:115c:a1e0::1","192.168.1.20"]}}`; const tailscaleStatusWithSingleIpJson = `{"Self":{"DNSName":"desktop.tail.ts.net.","TailscaleIPs":["100.90.1.2"]}}`; From 2db512ad438ec09b64dd7e4889ad7b81ee70eecc Mon Sep 17 00:00:00 2001 From: Theo Browne Date: Sun, 26 Jul 2026 01:20:12 -0700 Subject: [PATCH 10/23] Split DevShareError into three classes and keep the cause MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Effect service conventions check was failing on this, correctly. The single DevShareError violated four rules at once: a `reason` discriminator chose both the user-facing message and the caller's remedy, the `message` getter was a lookup table over it, `detail` copied `cause.message` and was then used to build that message, and the underlying TailscaleCommandError was discarded entirely so the error chain ended at the wrapper. Now three classes, one per genuinely distinct failure: TailscaleUnavailableError and TailnetNameMissingError each have one fixed message and one remedy; DevServeFailedError keeps a `stage` discriminator, which is legitimate — both stages share the same semantics (a serve invocation failed for this port) and differ only in which one. Each wrapping construction now preserves the immediate error as `cause`, and every message derives solely from structural fields. unshareDevServer returns the structured cause rather than a flattened string so its caller can do the same. TailnetNameMissingError has no cause because none exists — the status read succeeded and simply had no name. Verified the chain end to end: the wrapped TailscaleCommandExitError is reachable with exitCode and stderrDiagnostic intact, and the auth token in stderr still never reaches the message. Co-Authored-By: Claude Opus 5 (1M context) --- scripts/dev-runner.ts | 4 +- scripts/lib/dev-share.test.ts | 29 +++++-- scripts/lib/dev-share.ts | 159 +++++++++++++++++++++++----------- 3 files changed, 134 insertions(+), 58 deletions(-) diff --git a/scripts/dev-runner.ts b/scripts/dev-runner.ts index c4e530252ff..89c4a51c740 100644 --- a/scripts/dev-runner.ts +++ b/scripts/dev-runner.ts @@ -19,7 +19,7 @@ import * as Schema from "effect/Schema"; import { Argument, Command, Flag } from "effect/unstable/cli"; import { ChildProcess } from "effect/unstable/process"; -import { DevShareError, shareDevServer, unshareDevServer } from "./lib/dev-share.ts"; +import { type DevShareError, shareDevServer, unshareDevServer } from "./lib/dev-share.ts"; import { loadRepoEnv } from "./lib/public-config.ts"; Object.assign(process.env, loadRepoEnv()); @@ -669,7 +669,7 @@ export function runDevRunnerWithInput(input: DevRunnerCliInput) { ? Effect.void : Effect.logWarning( `[dev-runner] could not remove the tailnet mapping for port ${String(sharedWebPort)}${ - result.detail ? `: ${result.detail}` : "" + result.explanation ? `: ${result.explanation}` : "" }. Remove it with \`tailscale serve --https=${String(sharedWebPort)} off\`.`, ), ), diff --git a/scripts/lib/dev-share.test.ts b/scripts/lib/dev-share.test.ts index c974ac916f9..b2dfba3585e 100644 --- a/scripts/lib/dev-share.test.ts +++ b/scripts/lib/dev-share.test.ts @@ -5,7 +5,12 @@ import * as Sink from "effect/Sink"; import * as Stream from "effect/Stream"; import { ChildProcessSpawner } from "effect/unstable/process"; -import { DevShareError, shareDevServer, unshareDevServer } from "./dev-share.ts"; +import { + type DevShareError, + DevServeFailedError, + shareDevServer, + unshareDevServer, +} from "./dev-share.ts"; const TAILNET_STATUS = JSON.stringify({ Self: { DNSName: "host.example.ts.net." } }); const NO_HANDLER_STDERR = "error: failed to remove web serve: handler does not exist"; @@ -78,7 +83,9 @@ describe("unshareDevServer", () => { Effect.provide(spawnerLayer({ off: { exitCode: 1, stderr: "permission denied" } })), ); assert.isFalse(result.cleared); - assert.include(result.detail, "permission denied"); + assert.include(result.explanation, "permission denied"); + // Structured, so a wrapping error can keep the real chain. + assert.equal(result.cause?._tag, "TailscaleCommandExitError"); }), ); }); @@ -110,11 +117,18 @@ describe("shareDevServer", () => { Effect.flip, ); - assert.equal(error.reason, "serve-failed"); + assert.instanceOf(error, DevServeFailedError); + assert.equal(error.stage, "serve"); + assert.equal(error.webPort, 5788); + // The underlying failure is preserved rather than flattened to a string. + assert.equal( + (error.cause as { _tag?: string } | undefined)?._tag, + "TailscaleCommandExitError", + ); // Unclassifiable stderr is never quoted (it can carry auth keys), so the // message points at the command instead of echoing the CLI. assert.notInclude(error.message, "port already in use"); - assert.include(error.message, "Run the command by hand"); + assert.include(error.message, "run the command by hand"); assert.include(error.message, "no longer served"); assert.include(error.message, "5788"); }), @@ -137,7 +151,8 @@ describe("shareDevServer", () => { Effect.flip, ); - assert.equal(error.reason, "serve-failed"); + assert.instanceOf(error, DevServeFailedError); + assert.equal(error.stage, "serve"); assert.include(error.message, "permission denied"); assert.include(error.message, "elevated privileges"); assert.notInclude(error.message, "tskey-auth-secret-token-value"); @@ -153,7 +168,9 @@ describe("shareDevServer", () => { Effect.flip, ); - assert.equal(error.reason, "serve-failed"); + assert.instanceOf(error, DevServeFailedError); + // A distinct stage: the prior mapping survived, so nothing was replaced. + assert.equal(error.stage, "clear-existing"); assert.include(error.message, "could not clear the existing mapping"); assert.include(error.message, "permission denied"); }), diff --git a/scripts/lib/dev-share.ts b/scripts/lib/dev-share.ts index a82f92e26cb..0f843b3ba91 100644 --- a/scripts/lib/dev-share.ts +++ b/scripts/lib/dev-share.ts @@ -25,30 +25,6 @@ import * as Effect from "effect/Effect"; import * as Schema from "effect/Schema"; import type { ChildProcessSpawner } from "effect/unstable/process"; -export class DevShareError extends Schema.TaggedErrorClass()("DevShareError", { - reason: Schema.Literals(["tailscale-unavailable", "no-tailnet-name", "serve-failed"]), - detail: Schema.optional(Schema.String), -}) { - override get message(): string { - const base = { - "tailscale-unavailable": "could not talk to tailscale", - "no-tailnet-name": "this machine has no tailnet DNS name", - "serve-failed": "tailscale serve failed", - }[this.reason]; - return this.detail ? `${base}: ${this.detail}` : base; - } - - /** What the user can actually do about it. */ - get hint(): string | undefined { - return { - "tailscale-unavailable": - "Is Tailscale installed and tailscaled running? Try `tailscale status` — or drop --share and open the printed localhost URL.", - "no-tailnet-name": "Run `tailscale up` and make sure MagicDNS is enabled.", - "serve-failed": undefined, - }[this.reason]; - } -} - /** * Human-readable gloss for each diagnostic. Deliberately our own words rather * than the CLI's: tailscale prints auth keys and node names into stderr, and @@ -61,15 +37,86 @@ const DIAGNOSTIC_EXPLANATIONS: Record { - if (error._tag !== "TailscaleCommandExitError" || error.stderrDiagnostic === undefined) { - return error.message; +/** + * Our own wording for why a tailscale command failed, derived from the + * classified diagnostic. Never the CLI's text — see `stderrDiagnosticOf`. + */ +const explainCommandFailure = (error: TailscaleCommandError): string | undefined => + error._tag === "TailscaleCommandExitError" && error.stderrDiagnostic !== undefined + ? (DIAGNOSTIC_EXPLANATIONS[error.stderrDiagnostic] ?? "run the command by hand to see why") + : undefined; + +/** + * Three distinct failures, three classes: each has its own caller-visible + * message and its own remedy, and `shareDevServer` chooses between them + * structurally. A single error with a `reason` discriminator would encode that + * distinction twice and put a lookup table in the `message` getter. + * + * Each wraps a real underlying failure and so keeps it as `cause`; the message + * is derived only from the structural fields, never from `cause.message`. + */ +export class TailscaleUnavailableError extends Schema.TaggedErrorClass()( + "TailscaleUnavailableError", + { cause: Schema.Defect() }, +) { + override get message(): string { + return "could not talk to tailscale"; } - const explanation = DIAGNOSTIC_EXPLANATIONS[error.stderrDiagnostic]; - return explanation === undefined - ? `${error.message} Run the command by hand to see why.` - : `${error.message} ${explanation}.`; -}; + + get hint(): string { + return "Is Tailscale installed and tailscaled running? Try `tailscale status` — or drop --share and open the printed localhost URL."; + } +} + +/** No underlying failure: the status read succeeded and simply had no name. */ +export class TailnetNameMissingError extends Schema.TaggedErrorClass()( + "TailnetNameMissingError", + {}, +) { + override get message(): string { + return "this machine has no tailnet DNS name"; + } + + get hint(): string { + return "Run `tailscale up` and make sure MagicDNS is enabled."; + } +} + +/** + * `stage` is a genuine multi-value discriminator: both stages share the same + * semantics (a `tailscale serve` invocation failed for this port) and differ + * only in which one, which the message states plainly. + */ +export class DevServeFailedError extends Schema.TaggedErrorClass()( + "DevServeFailedError", + { + stage: Schema.Literals(["clear-existing", "serve"]), + webPort: Schema.Number, + explanation: Schema.optional(Schema.String), + cause: Schema.optional(Schema.Defect()), + }, +) { + override get message(): string { + const port = String(this.webPort); + const base = + this.stage === "clear-existing" + ? `could not clear the existing mapping for port ${port}. Run \`tailscale serve --https=${port} off\` and retry` + : `could not serve port ${port} on the tailnet (it is no longer served; any previous mapping for it was cleared before this attempt)`; + return this.explanation ? `${base}: ${this.explanation}` : base; + } + + get hint(): undefined { + return undefined; + } +} + +export const DevShareError = Schema.Union([ + TailscaleUnavailableError, + TailnetNameMissingError, + DevServeFailedError, +]); +export type DevShareError = typeof DevShareError.Type; +export const isDevShareError = Schema.is(DevShareError); /** * Removes any mapping for `webPort`, reporting whether the port is now clear. @@ -81,7 +128,13 @@ const commandDetail = (error: TailscaleCommandError): string => { export const unshareDevServer = ( webPort: number, ): Effect.Effect< - { readonly cleared: boolean; readonly detail?: string | undefined }, + { + readonly cleared: boolean; + readonly explanation?: string | undefined; + // Kept structured so a caller wrapping this can preserve the real error + // chain rather than a flattened string. + readonly cause?: TailscaleCommandError | undefined; + }, never, ChildProcessSpawner.ChildProcessSpawner > => @@ -93,7 +146,13 @@ export const unshareDevServer = ( error._tag === "TailscaleCommandExitError" && error.stderrDiagnostic === "no-existing-handler" ? ({ cleared: true } as const) - : ({ cleared: false, detail: commandDetail(error) } as const), + : ({ + cleared: false, + ...(explainCommandFailure(error) !== undefined + ? { explanation: explainCommandFailure(error) } + : {}), + cause: error, + } as const), ), ), Effect.uninterruptible, @@ -112,12 +171,10 @@ export const shareDevServer = Effect.fn("devShare.shareDevServer")(function* (in readonly webPort: number; }) { const status = yield* readTailscaleStatus.pipe( - Effect.mapError( - (error) => new DevShareError({ reason: "tailscale-unavailable", detail: error.message }), - ), + Effect.mapError((error) => new TailscaleUnavailableError({ cause: error })), ); if (status.magicDnsName === null) { - return yield* new DevShareError({ reason: "no-tailnet-name" }); + return yield* new TailnetNameMissingError(); } // Clear any mapping left behind by a run that was killed before its finalizer @@ -130,22 +187,24 @@ export const shareDevServer = Effect.fn("devShare.shareDevServer")(function* (in // Serving over routes we failed to remove would hand out a URL that is // broken in a way the user cannot see: the page loads while /ws and /api // silently resolve to a dead backend. Better to refuse and say why. - return yield* new DevShareError({ - reason: "serve-failed", - detail: `could not clear the existing mapping for port ${String(input.webPort)}${ - cleared.detail ? `: ${cleared.detail}` : "" - }. Run \`tailscale serve --https=${String(input.webPort)} off\` and retry.`, + return yield* new DevServeFailedError({ + stage: "clear-existing", + webPort: input.webPort, + ...(cleared.explanation !== undefined ? { explanation: cleared.explanation } : {}), + ...(cleared.cause !== undefined ? { cause: cleared.cause } : {}), }); } yield* ensureTailscaleServe({ localPort: input.webPort, servePort: input.webPort }).pipe( - Effect.mapError( - (error) => - new DevShareError({ - reason: "serve-failed", - detail: `${commandDetail(error)} (port ${String(input.webPort)} is no longer served; any previous mapping for it was cleared before this attempt)`, - }), - ), + Effect.mapError((error) => { + const explanation = explainCommandFailure(error); + return new DevServeFailedError({ + stage: "serve", + webPort: input.webPort, + ...(explanation !== undefined ? { explanation } : {}), + cause: error, + }); + }), ); return { From 4333a66fd762da9f259eb2e8357d2b14f73e4fda Mon Sep 17 00:00:00 2001 From: Theo Browne Date: Sun, 26 Jul 2026 15:38:56 -0700 Subject: [PATCH 11/23] Fix dev sharing across hosted and restarted servers --- apps/server/src/auth/EnvironmentAuth.test.ts | 4 +- .../src/auth/EnvironmentAuthPolicy.test.ts | 23 ++++- apps/server/src/auth/EnvironmentAuthPolicy.ts | 2 +- apps/server/src/auth/SessionStore.ts | 5 +- apps/server/src/auth/utils.ts | 17 ++-- apps/server/src/bin.test.ts | 1 + apps/server/src/cli/config.test.ts | 4 + apps/server/src/cli/config.ts | 10 ++ apps/server/src/config.ts | 2 + .../src/environment/ServerEnvironment.test.ts | 1 + apps/server/src/http.ts | 6 +- apps/server/src/server.test.ts | 29 ++++++ scripts/dev-runner.ts | 32 ++++--- scripts/lib/dev-share.test.ts | 65 ++++++++++++- scripts/lib/dev-share.ts | 92 +++++++++++++++++++ 15 files changed, 258 insertions(+), 35 deletions(-) diff --git a/apps/server/src/auth/EnvironmentAuth.test.ts b/apps/server/src/auth/EnvironmentAuth.test.ts index ba3f67c237e..9eaac9eda18 100644 --- a/apps/server/src/auth/EnvironmentAuth.test.ts +++ b/apps/server/src/auth/EnvironmentAuth.test.ts @@ -12,7 +12,7 @@ import { resolveSessionCookieName } from "./utils.ts"; import * as ServerSecretStore from "./ServerSecretStore.ts"; -/** Pinned so the session cookie name (which is port-scoped) is predictable. */ +/** Pinned so dev-mode cookie tests can assert the port-scoped name. */ const TEST_SERVER_PORT = 13_773; const makeServerConfigLayer = (overrides?: Partial) => @@ -47,7 +47,7 @@ const makeCookieRequest = ( cookies: { // Derived, not hardcoded: the name is port-scoped so concurrent servers // on one hostname don't share a cookie. - [resolveSessionCookieName({ port: TEST_SERVER_PORT })]: sessionToken, + [resolveSessionCookieName({ port: TEST_SERVER_PORT, devUrl: undefined })]: sessionToken, }, headers: {}, }) as unknown as Parameters< diff --git a/apps/server/src/auth/EnvironmentAuthPolicy.test.ts b/apps/server/src/auth/EnvironmentAuthPolicy.test.ts index 55f1a99b205..9f19f5d5f74 100644 --- a/apps/server/src/auth/EnvironmentAuthPolicy.test.ts +++ b/apps/server/src/auth/EnvironmentAuthPolicy.test.ts @@ -34,7 +34,7 @@ it.layer(NodeServices.layer)("EnvironmentAuthPolicy.layer", (it) => { expect(descriptor.policy).toBe("desktop-managed-local"); expect(descriptor.bootstrapMethods).toEqual(["desktop-bootstrap"]); - expect(descriptor.sessionCookieName).toBe("t3_session_3773"); + expect(descriptor.sessionCookieName).toBe("t3_session"); }).pipe( Effect.provide( makeEnvironmentAuthPolicyLayer({ @@ -69,9 +69,7 @@ it.layer(NodeServices.layer)("EnvironmentAuthPolicy.layer", (it) => { expect(descriptor.policy).toBe("loopback-browser"); expect(descriptor.bootstrapMethods).toEqual(["one-time-token"]); - // Port-scoped in web mode too: cookies ignore ports, so two dev servers - // on one hostname would otherwise clobber each other's session. - expect(descriptor.sessionCookieName).toBe("t3_session_13773"); + expect(descriptor.sessionCookieName).toBe("t3_session"); }).pipe( Effect.provide( makeEnvironmentAuthPolicyLayer({ @@ -83,6 +81,23 @@ it.layer(NodeServices.layer)("EnvironmentAuthPolicy.layer", (it) => { ), ); + it.effect("scopes session cookies by port only in development", () => + Effect.gen(function* () { + const policy = yield* EnvironmentAuthPolicy.EnvironmentAuthPolicy; + const descriptor = yield* policy.getDescriptor(); + + expect(descriptor.sessionCookieName).toBe("t3_session_13773"); + }).pipe( + Effect.provide( + makeEnvironmentAuthPolicyLayer({ + mode: "web", + port: 13773, + devUrl: new URL("http://127.0.0.1:5733"), + }), + ), + ), + ); + it.effect("uses remote-reachable policy for wildcard web hosts", () => Effect.gen(function* () { const policy = yield* EnvironmentAuthPolicy.EnvironmentAuthPolicy; diff --git a/apps/server/src/auth/EnvironmentAuthPolicy.ts b/apps/server/src/auth/EnvironmentAuthPolicy.ts index 1d7a1dd4ad3..d43db0d0ba9 100644 --- a/apps/server/src/auth/EnvironmentAuthPolicy.ts +++ b/apps/server/src/auth/EnvironmentAuthPolicy.ts @@ -38,7 +38,7 @@ export const make = Effect.gen(function* () { policy, bootstrapMethods, sessionMethods: ["browser-session-cookie", "bearer-access-token", "dpop-access-token"], - sessionCookieName: resolveSessionCookieName({ port: config.port }), + sessionCookieName: resolveSessionCookieName({ port: config.port, devUrl: config.devUrl }), }; return EnvironmentAuthPolicy.of({ diff --git a/apps/server/src/auth/SessionStore.ts b/apps/server/src/auth/SessionStore.ts index ed113470b24..b8d3be63483 100644 --- a/apps/server/src/auth/SessionStore.ts +++ b/apps/server/src/auth/SessionStore.ts @@ -467,7 +467,10 @@ export const make = Effect.gen(function* () { const signingSecret = yield* secretStore.getOrCreateRandom(SIGNING_SECRET_NAME, 32); const connectedSessionsRef = yield* Ref.make(new Map()); const changesPubSub = yield* PubSub.unbounded(); - const cookieName = resolveSessionCookieName({ port: serverConfig.port }); + const cookieName = resolveSessionCookieName({ + port: serverConfig.port, + devUrl: serverConfig.devUrl, + }); const emitUpsert = (clientSession: AuthClientSession) => PubSub.publish(changesPubSub, { diff --git a/apps/server/src/auth/utils.ts b/apps/server/src/auth/utils.ts index 0bb03099fa5..060f1d29f0a 100644 --- a/apps/server/src/auth/utils.ts +++ b/apps/server/src/auth/utils.ts @@ -11,15 +11,16 @@ import * as Result from "effect/Result"; const SESSION_COOKIE_NAME = "t3_session"; /** - * Cookies are scoped by host but *not* by port, so every server reachable at a - * given hostname shares one cookie jar. Suffixing the port keeps concurrent - * instances from overwriting each other's session — otherwise two dev servers - * (different worktrees, or several ports behind one tailnet name) fight over - * `t3_session`, and whichever wrote last makes every other one reject the - * cookie with "Invalid session token signature" until it's cleared by hand. + * Cookies are scoped by host but *not* by port, so concurrent dev servers on a + * hostname need separate names. Hosted deployments keep the stable production + * name: their public port can change between releases, and scoping it would log + * every user out. */ -export function resolveSessionCookieName(input: { readonly port: number }): string { - return `${SESSION_COOKIE_NAME}_${input.port}`; +export function resolveSessionCookieName(input: { + readonly port: number; + readonly devUrl: URL | undefined; +}): string { + return input.devUrl === undefined ? SESSION_COOKIE_NAME : `${SESSION_COOKIE_NAME}_${input.port}`; } export function base64UrlEncode(input: string | Uint8Array): string { diff --git a/apps/server/src/bin.test.ts b/apps/server/src/bin.test.ts index 91006a9bece..fcb662b9b78 100644 --- a/apps/server/src/bin.test.ts +++ b/apps/server/src/bin.test.ts @@ -84,6 +84,7 @@ const makeCliTestServerConfig = (baseDir: string) => ...derivedPaths, staticDir: undefined, devUrl: undefined, + devAllowedOrigins: [], noBrowser: true, startupPresentation: "browser", desktopBootstrapToken: undefined, diff --git a/apps/server/src/cli/config.test.ts b/apps/server/src/cli/config.test.ts index f6c2a63e192..39c00053fef 100644 --- a/apps/server/src/cli/config.test.ts +++ b/apps/server/src/cli/config.test.ts @@ -48,6 +48,7 @@ it.layer(NodeServices.layer)("cli config resolution", (it) => { otlpMetricsUrl: undefined, otlpExportIntervalMs: 10_000, otlpServiceName: "t3-server", + devAllowedOrigins: [], } as const; const openBootstrapFd = Effect.fn(function* (payload: DesktopBackendBootstrapValue) { @@ -95,6 +96,8 @@ it.layer(NodeServices.layer)("cli config resolution", (it) => { T3CODE_HOST: "0.0.0.0", T3CODE_HOME: baseDir, VITE_DEV_SERVER_URL: "http://127.0.0.1:5173", + T3CODE_DEV_ALLOWED_ORIGINS: + "https://host.example.ts.net, https://phone.example.ts.net ", T3CODE_NO_BROWSER: "true", T3CODE_AUTO_BOOTSTRAP_PROJECT_FROM_CWD: "false", T3CODE_LOG_WS_EVENTS: "true", @@ -117,6 +120,7 @@ it.layer(NodeServices.layer)("cli config resolution", (it) => { host: "0.0.0.0", staticDir: undefined, devUrl: new URL("http://127.0.0.1:5173"), + devAllowedOrigins: ["https://host.example.ts.net", "https://phone.example.ts.net"], noBrowser: true, startupPresentation: "browser", desktopBootstrapToken: undefined, diff --git a/apps/server/src/cli/config.ts b/apps/server/src/cli/config.ts index 5a4cde0a6fd..e3f231cca29 100644 --- a/apps/server/src/cli/config.ts +++ b/apps/server/src/cli/config.ts @@ -106,6 +106,15 @@ const EnvServerConfig = Config.all({ host: Config.string("T3CODE_HOST").pipe(Config.option, Config.map(Option.getOrUndefined)), t3Home: Config.string("T3CODE_HOME").pipe(Config.option, Config.map(Option.getOrUndefined)), devUrl: Config.url("VITE_DEV_SERVER_URL").pipe(Config.option, Config.map(Option.getOrUndefined)), + devAllowedOrigins: Config.string("T3CODE_DEV_ALLOWED_ORIGINS").pipe( + Config.withDefault(""), + Config.map((value) => + value + .split(",") + .map((entry) => entry.trim()) + .filter((entry) => entry.length > 0), + ), + ), noBrowser: Config.boolean("T3CODE_NO_BROWSER").pipe( Config.option, Config.map(Option.getOrUndefined), @@ -363,6 +372,7 @@ export const resolveServerConfig = ( host, staticDir, devUrl, + devAllowedOrigins: env.devAllowedOrigins, noBrowser, startupPresentation, desktopBootstrapToken, diff --git a/apps/server/src/config.ts b/apps/server/src/config.ts index 3b081c95c34..5a16e144ee4 100644 --- a/apps/server/src/config.ts +++ b/apps/server/src/config.ts @@ -72,6 +72,7 @@ export class ServerConfig extends Context.Service< readonly baseDir: string; readonly staticDir: string | undefined; readonly devUrl: URL | undefined; + readonly devAllowedOrigins: ReadonlyArray; readonly noBrowser: boolean; readonly startupPresentation: StartupPresentation; readonly desktopBootstrapToken: string | undefined; @@ -187,6 +188,7 @@ const makeTest = Effect.fn("ServerConfig.makeTest")(function* ( desktopBootstrapToken: undefined, staticDir: undefined, devUrl, + devAllowedOrigins: [], noBrowser: false, startupPresentation: "browser", }); diff --git a/apps/server/src/environment/ServerEnvironment.test.ts b/apps/server/src/environment/ServerEnvironment.test.ts index 61892d53d63..e9dbc4a5956 100644 --- a/apps/server/src/environment/ServerEnvironment.test.ts +++ b/apps/server/src/environment/ServerEnvironment.test.ts @@ -43,6 +43,7 @@ const makeServerConfig = Effect.fn(function* (baseDir: string) { desktopBootstrapToken: undefined, staticDir: undefined, devUrl: undefined, + devAllowedOrigins: [], noBrowser: false, startupPresentation: "browser", } satisfies ServerConfig.ServerConfig["Service"]; diff --git a/apps/server/src/http.ts b/apps/server/src/http.ts index bfb2caf6bde..77eef955d55 100644 --- a/apps/server/src/http.ts +++ b/apps/server/src/http.ts @@ -54,14 +54,10 @@ export const browserApiCorsLayer = Layer.unwrap( // origin — a tailnet name, a LAN IP, a phone. Browser dev normally proxies // through Vite and is same-origin (no preflight at all), so this is a // safety net for the desktop renderer and any direct-to-backend caller. - const extraDevOrigins = (process.env.T3CODE_DEV_ALLOWED_ORIGINS ?? "") - .split(",") - .map((entry) => entry.trim()) - .filter((entry) => entry.length > 0); return HttpRouter.cors({ ...(devOrigin ? { - allowedOrigins: [devOrigin, ...DESKTOP_RENDERER_ORIGINS, ...extraDevOrigins], + allowedOrigins: [devOrigin, ...DESKTOP_RENDERER_ORIGINS, ...config.devAllowedOrigins], credentials: true, } : {}), diff --git a/apps/server/src/server.test.ts b/apps/server/src/server.test.ts index 871b79eca90..9510c7720c1 100644 --- a/apps/server/src/server.test.ts +++ b/apps/server/src/server.test.ts @@ -380,6 +380,7 @@ const buildAppUnderTest = (options?: { ...derivedPaths, staticDir: undefined, devUrl, + devAllowedOrigins: [], noBrowser: true, startupPresentation: "browser", desktopBootstrapToken: defaultDesktopBootstrapToken, @@ -3246,6 +3247,34 @@ it.layer(NodeServices.layer)("server router seam", (it) => { }).pipe(Effect.provide(NodeHttpServer.layerTest)), ); + it.effect("allows configured development origins through ServerConfig", () => + Effect.gen(function* () { + const tailnetOrigin = "https://host.example.ts.net"; + yield* buildAppUnderTest({ + config: { + devUrl: new URL(crossOriginClientOrigin), + devAllowedOrigins: [tailnetOrigin], + }, + }); + + const sessionUrl = yield* getHttpServerUrl("/api/auth/session"); + const response = yield* fetchEffect(sessionUrl, { + method: "OPTIONS", + headers: { + origin: tailnetOrigin, + "access-control-request-method": "GET", + "access-control-request-headers": "content-type", + }, + }); + + assert.equal(response.status, 204); + assertBrowserApiCorsPreflightHeaders(response.headers, { + origin: tailnetOrigin, + credentials: true, + }); + }).pipe(Effect.provide(NodeHttpServer.layerTest)), + ); + for (const desktopOrigin of ["t3code://app", "t3code-dev://app"]) { it.effect(`allows credentialed preflights from ${desktopOrigin} in development`, () => Effect.gen(function* () { diff --git a/scripts/dev-runner.ts b/scripts/dev-runner.ts index 89c4a51c740..4448c085d5e 100644 --- a/scripts/dev-runner.ts +++ b/scripts/dev-runner.ts @@ -1,6 +1,7 @@ #!/usr/bin/env node import * as NodeOS from "node:os"; +import * as NodeCrypto from "node:crypto"; import * as NodeRuntime from "@effect/platform-node/NodeRuntime"; import * as NodeServices from "@effect/platform-node/NodeServices"; @@ -19,7 +20,12 @@ import * as Schema from "effect/Schema"; import { Argument, Command, Flag } from "effect/unstable/cli"; import { ChildProcess } from "effect/unstable/process"; -import { type DevShareError, shareDevServer, unshareDevServer } from "./lib/dev-share.ts"; +import { + claimDevShareLease, + cleanupOwnedDevShare, + type DevShareError, + shareDevServer, +} from "./lib/dev-share.ts"; import { loadRepoEnv } from "./lib/public-config.ts"; Object.assign(process.env, loadRepoEnv()); @@ -651,6 +657,7 @@ export function runDevRunnerWithInput(input: DevRunnerCliInput) { "[dev-runner] --share is not supported for dev:desktop (the renderer is pinned to loopback). Use `dev`, which runs the whole browser stack.", ); } else { + const path = yield* Path.Path; // acquireRelease, not share-then-addFinalizer: the mapping outlives this // process (and reboots), so the cleanup has to be registered atomically // with creating it. An interrupt landing in between would otherwise @@ -658,23 +665,24 @@ export function runDevRunnerWithInput(input: DevRunnerCliInput) { // // A tailnet that isn't up shouldn't stop the dev server from starting — // warn, and carry on serving locally. - const shared = yield* Effect.acquireRelease( - shareDevServer({ webPort: sharedWebPort }), - () => - // Serve config outlives this process, so a cleanup that did not - // take leaves a tailnet URL pointing at a port nothing serves. - unshareDevServer(sharedWebPort).pipe( + const shared = yield* Effect.gen(function* () { + const lease = yield* claimDevShareLease({ + leasePath: path.join(baseDir, "dev-share", `${String(sharedWebPort)}.owner`), + ownerId: `${String(process.pid)}:${NodeCrypto.randomUUID()}`, + webPort: sharedWebPort, + }); + return yield* Effect.acquireRelease(shareDevServer({ webPort: sharedWebPort }), () => + cleanupOwnedDevShare(lease).pipe( Effect.flatMap((result) => - result.cleared + result.status !== "failed" ? Effect.void : Effect.logWarning( - `[dev-runner] could not remove the tailnet mapping for port ${String(sharedWebPort)}${ - result.explanation ? `: ${result.explanation}` : "" - }. Remove it with \`tailscale serve --https=${String(sharedWebPort)} off\`.`, + `[dev-runner] could not safely clean up the tailnet mapping for port ${String(sharedWebPort)}: ${result.explanation}`, ), ), ), - ).pipe( + ); + }).pipe( Effect.tapError((error: DevShareError) => Effect.logWarning( `[dev-runner] could not share on the tailnet: ${error.message}${ diff --git a/scripts/lib/dev-share.test.ts b/scripts/lib/dev-share.test.ts index b2dfba3585e..a40a38d951a 100644 --- a/scripts/lib/dev-share.test.ts +++ b/scripts/lib/dev-share.test.ts @@ -1,11 +1,15 @@ +import * as NodeServices from "@effect/platform-node/NodeServices"; import { assert, describe, it } from "@effect/vitest"; import * as Effect from "effect/Effect"; +import * as FileSystem from "effect/FileSystem"; import * as Layer from "effect/Layer"; import * as Sink from "effect/Sink"; import * as Stream from "effect/Stream"; import { ChildProcessSpawner } from "effect/unstable/process"; import { + claimDevShareLease, + cleanupOwnedDevShare, type DevShareError, DevServeFailedError, shareDevServer, @@ -27,11 +31,17 @@ const encode = (value: string) => Stream.make(new TextEncoder().encode(value)); * test set the outcome of the `off` (pre-clear) and `serve` calls separately — * they are the same subcommand and are told apart by the trailing `off`. */ -const spawnerLayer = (input: { readonly off?: CallResult; readonly serve?: CallResult }) => +const spawnerLayer = (input: { + readonly off?: CallResult; + readonly serve?: CallResult; + readonly onOff?: Effect.Effect; + readonly calls?: Array>; +}) => Layer.succeed( ChildProcessSpawner.ChildProcessSpawner, ChildProcessSpawner.make((command) => { const args = "args" in command ? (command.args as ReadonlyArray) : []; + input.calls?.push(args); const result: CallResult = args.includes("status") ? { exitCode: 0 } : args.includes("off") @@ -41,7 +51,9 @@ const spawnerLayer = (input: { readonly off?: CallResult; readonly serve?: CallR return Effect.succeed( ChildProcessSpawner.makeHandle({ pid: ChildProcessSpawner.ProcessId(1), - exitCode: Effect.succeed(ChildProcessSpawner.ExitCode(result.exitCode)), + exitCode: (args.includes("off") ? (input.onOff ?? Effect.void) : Effect.void).pipe( + Effect.as(ChildProcessSpawner.ExitCode(result.exitCode)), + ), isRunning: Effect.succeed(false), kill: () => Effect.void, unref: Effect.succeed(Effect.void), @@ -176,3 +188,52 @@ describe("shareDevServer", () => { }), ); }); + +it.layer(NodeServices.layer)("dev share cleanup ownership", (it) => { + it.effect("restores a newer runner's mapping when ownership changes during cleanup", () => + Effect.gen(function* () { + const fileSystem = yield* FileSystem.FileSystem; + const directory = yield* fileSystem.makeTempDirectoryScoped({ + prefix: "t3-dev-share-lease-", + }); + const leasePath = `${directory}/5788.owner`; + const oldLease = { leasePath, ownerId: "old-runner", webPort: 5788 }; + const calls: Array> = []; + + yield* claimDevShareLease(oldLease); + const result = yield* cleanupOwnedDevShare(oldLease).pipe( + Effect.provide( + spawnerLayer({ + calls, + onOff: fileSystem.writeFileString(leasePath, "new-runner").pipe(Effect.orDie), + }), + ), + ); + + assert.equal(result.status, "restored"); + assert.isTrue(calls.some((args) => args.includes("off"))); + assert.isTrue(calls.some((args) => args.includes("http://127.0.0.1:5788"))); + }), + ); + + it.effect("leaves the mapping alone when a newer runner already owns it", () => + Effect.gen(function* () { + const fileSystem = yield* FileSystem.FileSystem; + const directory = yield* fileSystem.makeTempDirectoryScoped({ + prefix: "t3-dev-share-lease-", + }); + const leasePath = `${directory}/5788.owner`; + const oldLease = { leasePath, ownerId: "old-runner", webPort: 5788 }; + const calls: Array> = []; + + yield* claimDevShareLease(oldLease); + yield* claimDevShareLease({ ...oldLease, ownerId: "new-runner" }); + const result = yield* cleanupOwnedDevShare(oldLease).pipe( + Effect.provide(spawnerLayer({ calls })), + ); + + assert.equal(result.status, "superseded"); + assert.isEmpty(calls); + }), + ); +}); diff --git a/scripts/lib/dev-share.ts b/scripts/lib/dev-share.ts index 0f843b3ba91..d6c87abd8c2 100644 --- a/scripts/lib/dev-share.ts +++ b/scripts/lib/dev-share.ts @@ -22,6 +22,9 @@ import { type TailscaleStderrDiagnostic, } from "@t3tools/tailscale"; import * as Effect from "effect/Effect"; +import * as FileSystem from "effect/FileSystem"; +import * as Option from "effect/Option"; +import * as Path from "effect/Path"; import * as Schema from "effect/Schema"; import type { ChildProcessSpawner } from "effect/unstable/process"; @@ -110,10 +113,24 @@ export class DevServeFailedError extends Schema.TaggedErrorClass()( + "DevShareLeaseClaimError", + { leasePath: Schema.String, cause: Schema.Defect() }, +) { + override get message(): string { + return `could not claim cleanup ownership at ${this.leasePath}`; + } + + get hint(): string { + return "Check that the dev data directory is writable."; + } +} + export const DevShareError = Schema.Union([ TailscaleUnavailableError, TailnetNameMissingError, DevServeFailedError, + DevShareLeaseClaimError, ]); export type DevShareError = typeof DevShareError.Type; export const isDevShareError = Schema.is(DevShareError); @@ -163,6 +180,81 @@ export interface DevShareResult { readonly host: string; } +export interface DevShareLease { + readonly leasePath: string; + readonly ownerId: string; + readonly webPort: number; +} + +export const claimDevShareLease = Effect.fn("devShare.claimDevShareLease")(function* ( + lease: DevShareLease, +) { + const fileSystem = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + yield* fileSystem.makeDirectory(path.dirname(lease.leasePath), { recursive: true }).pipe( + Effect.andThen(fileSystem.writeFileString(lease.leasePath, lease.ownerId)), + Effect.mapError((cause) => new DevShareLeaseClaimError({ leasePath: lease.leasePath, cause })), + ); + return lease; +}); + +export type DevShareCleanupResult = + | { readonly status: "cleared" | "superseded" | "restored" } + | { readonly status: "failed"; readonly explanation: string }; + +/** + * Clears only the mapping this runner owns. If a successor claims the lease + * while `tailscale serve off` is in flight, restore the same port mapping so + * the old finalizer cannot silently disconnect the new runner. + */ +export const cleanupOwnedDevShare = Effect.fn("devShare.cleanupOwnedDevShare")(function* ( + lease: DevShareLease, +): Effect.fn.Return< + DevShareCleanupResult, + never, + FileSystem.FileSystem | ChildProcessSpawner.ChildProcessSpawner +> { + const fileSystem = yield* FileSystem.FileSystem; + const readOwner = fileSystem + .readFileString(lease.leasePath) + .pipe(Effect.option, Effect.map(Option.getOrUndefined)); + const ownerBefore = yield* readOwner; + if (ownerBefore !== lease.ownerId) { + return { status: "superseded" }; + } + + const result = yield* unshareDevServer(lease.webPort); + if (!result.cleared) { + return { + status: "failed", + explanation: + result.explanation ?? + `could not remove the tailnet mapping for port ${String(lease.webPort)}`, + }; + } + + const ownerAfter = yield* readOwner; + if (ownerAfter === lease.ownerId) { + return { status: "cleared" }; + } + + return yield* ensureTailscaleServe({ + localPort: lease.webPort, + servePort: lease.webPort, + }).pipe( + Effect.as({ status: "restored" } as const), + Effect.catch((cause: TailscaleCommandError) => + Effect.succeed({ + status: "failed", + explanation: + explainCommandFailure(cause) ?? + `a newer runner claimed port ${String(lease.webPort)}, but its mapping could not be restored`, + } as const), + ), + Effect.uninterruptible, + ); +}); + /** * Publishes `webPort` on the tailnet at the same port number and returns the * resulting HTTPS URL. Idempotent: re-running replaces any existing mapping. From f48ee4af68256701c05d6dd0dbe9ff50cbecb507 Mon Sep 17 00:00:00 2001 From: Theo Browne Date: Sun, 26 Jul 2026 15:44:58 -0700 Subject: [PATCH 12/23] Update hosted cookie test expectation --- apps/server/src/server.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/server/src/server.test.ts b/apps/server/src/server.test.ts index 9510c7720c1..948c2b4ff45 100644 --- a/apps/server/src/server.test.ts +++ b/apps/server/src/server.test.ts @@ -1329,7 +1329,7 @@ it.layer(NodeServices.layer)("server router seam", (it) => { "bearer-access-token", "dpop-access-token", ]); - assert.isTrue(body.auth.sessionCookieName.startsWith("t3_session_")); + assert.equal(body.auth.sessionCookieName, "t3_session"); }).pipe(Effect.provide(NodeHttpServer.layerTest)), ); From 8ba59f6d164c73c1dc4dc6131c9ff00283947f88 Mon Sep 17 00:00:00 2001 From: Theo Browne Date: Sun, 26 Jul 2026 15:49:24 -0700 Subject: [PATCH 13/23] Claim dev share ownership after serving --- scripts/dev-runner.ts | 13 ++++--------- scripts/lib/dev-share.test.ts | 28 ++++++++++++++++++++++++++++ scripts/lib/dev-share.ts | 13 +++++++++++++ 3 files changed, 45 insertions(+), 9 deletions(-) diff --git a/scripts/dev-runner.ts b/scripts/dev-runner.ts index 4448c085d5e..8d148df883e 100644 --- a/scripts/dev-runner.ts +++ b/scripts/dev-runner.ts @@ -20,12 +20,7 @@ import * as Schema from "effect/Schema"; import { Argument, Command, Flag } from "effect/unstable/cli"; import { ChildProcess } from "effect/unstable/process"; -import { - claimDevShareLease, - cleanupOwnedDevShare, - type DevShareError, - shareDevServer, -} from "./lib/dev-share.ts"; +import { acquireDevShare, cleanupOwnedDevShare, type DevShareError } from "./lib/dev-share.ts"; import { loadRepoEnv } from "./lib/public-config.ts"; Object.assign(process.env, loadRepoEnv()); @@ -666,12 +661,12 @@ export function runDevRunnerWithInput(input: DevRunnerCliInput) { // A tailnet that isn't up shouldn't stop the dev server from starting — // warn, and carry on serving locally. const shared = yield* Effect.gen(function* () { - const lease = yield* claimDevShareLease({ + const lease = { leasePath: path.join(baseDir, "dev-share", `${String(sharedWebPort)}.owner`), ownerId: `${String(process.pid)}:${NodeCrypto.randomUUID()}`, webPort: sharedWebPort, - }); - return yield* Effect.acquireRelease(shareDevServer({ webPort: sharedWebPort }), () => + }; + return yield* Effect.acquireRelease(acquireDevShare(lease), () => cleanupOwnedDevShare(lease).pipe( Effect.flatMap((result) => result.status !== "failed" diff --git a/scripts/lib/dev-share.test.ts b/scripts/lib/dev-share.test.ts index a40a38d951a..d4539b8444b 100644 --- a/scripts/lib/dev-share.test.ts +++ b/scripts/lib/dev-share.test.ts @@ -8,6 +8,7 @@ import * as Stream from "effect/Stream"; import { ChildProcessSpawner } from "effect/unstable/process"; import { + acquireDevShare, claimDevShareLease, cleanupOwnedDevShare, type DevShareError, @@ -190,6 +191,33 @@ describe("shareDevServer", () => { }); it.layer(NodeServices.layer)("dev share cleanup ownership", (it) => { + it.effect("keeps the prior owner when a replacement share fails", () => + Effect.gen(function* () { + const fileSystem = yield* FileSystem.FileSystem; + const directory = yield* fileSystem.makeTempDirectoryScoped({ + prefix: "t3-dev-share-lease-", + }); + const leasePath = `${directory}/5788.owner`; + + yield* fileSystem.writeFileString(leasePath, "old-runner"); + yield* acquireDevShare({ + leasePath, + ownerId: "new-runner", + webPort: 5788, + }).pipe( + Effect.provide( + spawnerLayer({ + off: { exitCode: 0 }, + serve: { exitCode: 1, stderr: "permission denied" }, + }), + ), + Effect.flip, + ); + + assert.equal(yield* fileSystem.readFileString(leasePath), "old-runner"); + }), + ); + it.effect("restores a newer runner's mapping when ownership changes during cleanup", () => Effect.gen(function* () { const fileSystem = yield* FileSystem.FileSystem; diff --git a/scripts/lib/dev-share.ts b/scripts/lib/dev-share.ts index d6c87abd8c2..370b9ae7fb9 100644 --- a/scripts/lib/dev-share.ts +++ b/scripts/lib/dev-share.ts @@ -307,3 +307,16 @@ export const shareDevServer = Effect.fn("devShare.shareDevServer")(function* (in host: status.magicDnsName, } satisfies DevShareResult; }); + +/** + * Claims cleanup ownership only after the tailnet mapping exists. A failed + * replacement must leave the prior runner as the owner so its finalizer can + * still remove any mapping that survived the attempt. + */ +export const acquireDevShare = Effect.fn("devShare.acquireDevShare")(function* ( + lease: DevShareLease, +) { + const shared = yield* shareDevServer({ webPort: lease.webPort }); + yield* claimDevShareLease(lease); + return shared; +}); From 3b4765b42231d77fc3f6124afcc5199acf4d44ae Mon Sep 17 00:00:00 2001 From: Theo Browne Date: Sun, 26 Jul 2026 15:55:24 -0700 Subject: [PATCH 14/23] Make dev share acquisition transactional --- scripts/lib/dev-share.test.ts | 7 +++++-- scripts/lib/dev-share.ts | 12 +++++++----- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/scripts/lib/dev-share.test.ts b/scripts/lib/dev-share.test.ts index d4539b8444b..0ab0b2f72a6 100644 --- a/scripts/lib/dev-share.test.ts +++ b/scripts/lib/dev-share.test.ts @@ -191,13 +191,14 @@ describe("shareDevServer", () => { }); it.layer(NodeServices.layer)("dev share cleanup ownership", (it) => { - it.effect("keeps the prior owner when a replacement share fails", () => + it.effect("cleans up a claimed mapping when replacement sharing fails", () => Effect.gen(function* () { const fileSystem = yield* FileSystem.FileSystem; const directory = yield* fileSystem.makeTempDirectoryScoped({ prefix: "t3-dev-share-lease-", }); const leasePath = `${directory}/5788.owner`; + const calls: Array> = []; yield* fileSystem.writeFileString(leasePath, "old-runner"); yield* acquireDevShare({ @@ -207,6 +208,7 @@ it.layer(NodeServices.layer)("dev share cleanup ownership", (it) => { }).pipe( Effect.provide( spawnerLayer({ + calls, off: { exitCode: 0 }, serve: { exitCode: 1, stderr: "permission denied" }, }), @@ -214,7 +216,8 @@ it.layer(NodeServices.layer)("dev share cleanup ownership", (it) => { Effect.flip, ); - assert.equal(yield* fileSystem.readFileString(leasePath), "old-runner"); + assert.equal(yield* fileSystem.readFileString(leasePath), "new-runner"); + assert.equal(calls.filter((args) => args.includes("off")).length, 2); }), ); diff --git a/scripts/lib/dev-share.ts b/scripts/lib/dev-share.ts index 370b9ae7fb9..324cac54de7 100644 --- a/scripts/lib/dev-share.ts +++ b/scripts/lib/dev-share.ts @@ -309,14 +309,16 @@ export const shareDevServer = Effect.fn("devShare.shareDevServer")(function* (in }); /** - * Claims cleanup ownership only after the tailnet mapping exists. A failed - * replacement must leave the prior runner as the owner so its finalizer can - * still remove any mapping that survived the attempt. + * Claims cleanup ownership before publishing, then cleans up immediately if + * publishing fails. This keeps the handoff atomic from the runners' point of + * view: the prior runner can stop cleaning as soon as the successor claims, + * because that successor owns both the attempted mapping and its rollback. */ export const acquireDevShare = Effect.fn("devShare.acquireDevShare")(function* ( lease: DevShareLease, ) { - const shared = yield* shareDevServer({ webPort: lease.webPort }); yield* claimDevShareLease(lease); - return shared; + return yield* shareDevServer({ webPort: lease.webPort }).pipe( + Effect.tapError(() => cleanupOwnedDevShare(lease)), + ); }); From 5f48002f25e9e866799aff49e0b034d79e047560 Mon Sep 17 00:00:00 2001 From: Theo Browne Date: Sun, 26 Jul 2026 16:00:34 -0700 Subject: [PATCH 15/23] Handoff dev share leases between clear and serve --- scripts/lib/dev-share.test.ts | 30 +++++++++++++++++++++-- scripts/lib/dev-share.ts | 45 +++++++++++++++++++++++------------ 2 files changed, 58 insertions(+), 17 deletions(-) diff --git a/scripts/lib/dev-share.test.ts b/scripts/lib/dev-share.test.ts index 0ab0b2f72a6..47c3188ae2f 100644 --- a/scripts/lib/dev-share.test.ts +++ b/scripts/lib/dev-share.test.ts @@ -191,7 +191,7 @@ describe("shareDevServer", () => { }); it.layer(NodeServices.layer)("dev share cleanup ownership", (it) => { - it.effect("cleans up a claimed mapping when replacement sharing fails", () => + it.effect("keeps the prior owner when clearing its mapping fails", () => Effect.gen(function* () { const fileSystem = yield* FileSystem.FileSystem; const directory = yield* fileSystem.makeTempDirectoryScoped({ @@ -209,6 +209,33 @@ it.layer(NodeServices.layer)("dev share cleanup ownership", (it) => { Effect.provide( spawnerLayer({ calls, + off: { exitCode: 1, stderr: "permission denied" }, + }), + ), + Effect.flip, + ); + + assert.equal(yield* fileSystem.readFileString(leasePath), "old-runner"); + assert.equal(calls.filter((args) => args.includes("off")).length, 1); + }), + ); + + it.effect("claims ownership after clearing and before publishing", () => + Effect.gen(function* () { + const fileSystem = yield* FileSystem.FileSystem; + const directory = yield* fileSystem.makeTempDirectoryScoped({ + prefix: "t3-dev-share-lease-", + }); + const leasePath = `${directory}/5788.owner`; + + yield* fileSystem.writeFileString(leasePath, "old-runner"); + yield* acquireDevShare({ + leasePath, + ownerId: "new-runner", + webPort: 5788, + }).pipe( + Effect.provide( + spawnerLayer({ off: { exitCode: 0 }, serve: { exitCode: 1, stderr: "permission denied" }, }), @@ -217,7 +244,6 @@ it.layer(NodeServices.layer)("dev share cleanup ownership", (it) => { ); assert.equal(yield* fileSystem.readFileString(leasePath), "new-runner"); - assert.equal(calls.filter((args) => args.includes("off")).length, 2); }), ); diff --git a/scripts/lib/dev-share.ts b/scripts/lib/dev-share.ts index 324cac54de7..5cf012a0bf0 100644 --- a/scripts/lib/dev-share.ts +++ b/scripts/lib/dev-share.ts @@ -255,11 +255,7 @@ export const cleanupOwnedDevShare = Effect.fn("devShare.cleanupOwnedDevShare")(f ); }); -/** - * Publishes `webPort` on the tailnet at the same port number and returns the - * resulting HTTPS URL. Idempotent: re-running replaces any existing mapping. - */ -export const shareDevServer = Effect.fn("devShare.shareDevServer")(function* (input: { +const prepareDevShare = Effect.fn("devShare.prepareDevShare")(function* (input: { readonly webPort: number; }) { const status = yield* readTailscaleStatus.pipe( @@ -287,7 +283,17 @@ export const shareDevServer = Effect.fn("devShare.shareDevServer")(function* (in }); } - yield* ensureTailscaleServe({ localPort: input.webPort, servePort: input.webPort }).pipe( + return status.magicDnsName; +}); + +const publishDevShare = Effect.fn("devShare.publishDevShare")(function* (input: { + readonly host: string; + readonly webPort: number; +}) { + yield* ensureTailscaleServe({ + localPort: input.webPort, + servePort: input.webPort, + }).pipe( Effect.mapError((error) => { const explanation = explainCommandFailure(error); return new DevServeFailedError({ @@ -301,24 +307,33 @@ export const shareDevServer = Effect.fn("devShare.shareDevServer")(function* (in return { url: buildTailscaleHttpsBaseUrl({ - magicDnsName: status.magicDnsName, + magicDnsName: input.host, servePort: input.webPort, }), - host: status.magicDnsName, + host: input.host, } satisfies DevShareResult; }); /** - * Claims cleanup ownership before publishing, then cleans up immediately if - * publishing fails. This keeps the handoff atomic from the runners' point of - * view: the prior runner can stop cleaning as soon as the successor claims, - * because that successor owns both the attempted mapping and its rollback. + * Publishes `webPort` on the tailnet at the same port number and returns the + * resulting HTTPS URL. Idempotent: re-running replaces any existing mapping. + */ +export const shareDevServer = Effect.fn("devShare.shareDevServer")(function* (input: { + readonly webPort: number; +}) { + const host = yield* prepareDevShare(input); + return yield* publishDevShare({ ...input, host }); +}); + +/** + * Transfers cleanup ownership after the old mapping is known to be gone and + * before the new mapping is published. Failures on either side of that + * boundary therefore cannot leave a surviving mapping without an owner. */ export const acquireDevShare = Effect.fn("devShare.acquireDevShare")(function* ( lease: DevShareLease, ) { + const host = yield* prepareDevShare({ webPort: lease.webPort }); yield* claimDevShareLease(lease); - return yield* shareDevServer({ webPort: lease.webPort }).pipe( - Effect.tapError(() => cleanupOwnedDevShare(lease)), - ); + return yield* publishDevShare({ host, webPort: lease.webPort }); }); From 0a504fa8e9e5ca940b1745107f3d622d11302f74 Mon Sep 17 00:00:00 2001 From: Theo Browne Date: Sun, 26 Jul 2026 16:06:14 -0700 Subject: [PATCH 16/23] Restore dev sharing when lease handoff fails --- scripts/lib/dev-share.test.ts | 23 +++++++++++++++++++++ scripts/lib/dev-share.ts | 38 +++++++++++++++++++++++++++++------ 2 files changed, 55 insertions(+), 6 deletions(-) diff --git a/scripts/lib/dev-share.test.ts b/scripts/lib/dev-share.test.ts index 47c3188ae2f..e3e806e60a7 100644 --- a/scripts/lib/dev-share.test.ts +++ b/scripts/lib/dev-share.test.ts @@ -12,6 +12,7 @@ import { claimDevShareLease, cleanupOwnedDevShare, type DevShareError, + DevShareLeaseClaimError, DevServeFailedError, shareDevServer, unshareDevServer, @@ -247,6 +248,28 @@ it.layer(NodeServices.layer)("dev share cleanup ownership", (it) => { }), ); + it.effect("restores the mapping when ownership cannot be claimed", () => + Effect.gen(function* () { + const fileSystem = yield* FileSystem.FileSystem; + const directory = yield* fileSystem.makeTempDirectoryScoped({ + prefix: "t3-dev-share-lease-", + }); + const blockedDirectory = `${directory}/not-a-directory`; + const calls: Array> = []; + + yield* fileSystem.writeFileString(blockedDirectory, "blocked"); + const error = yield* acquireDevShare({ + leasePath: `${blockedDirectory}/5788.owner`, + ownerId: "new-runner", + webPort: 5788, + }).pipe(Effect.provide(spawnerLayer({ calls })), Effect.flip); + + assert.instanceOf(error, DevShareLeaseClaimError); + assert.isTrue(calls.some((args) => args.includes("off"))); + assert.isTrue(calls.some((args) => args.includes("http://127.0.0.1:5788"))); + }), + ); + it.effect("restores a newer runner's mapping when ownership changes during cleanup", () => Effect.gen(function* () { const fileSystem = yield* FileSystem.FileSystem; diff --git a/scripts/lib/dev-share.ts b/scripts/lib/dev-share.ts index 5cf012a0bf0..67455f5eec6 100644 --- a/scripts/lib/dev-share.ts +++ b/scripts/lib/dev-share.ts @@ -115,14 +115,22 @@ export class DevServeFailedError extends Schema.TaggedErrorClass()( "DevShareLeaseClaimError", - { leasePath: Schema.String, cause: Schema.Defect() }, + { + leasePath: Schema.String, + cause: Schema.Defect(), + restoreCause: Schema.optional(Schema.Defect()), + }, ) { override get message(): string { - return `could not claim cleanup ownership at ${this.leasePath}`; + return this.restoreCause === undefined + ? `could not claim cleanup ownership at ${this.leasePath}` + : `could not claim cleanup ownership at ${this.leasePath} or restore the previous tailnet mapping`; } get hint(): string { - return "Check that the dev data directory is writable."; + return this.restoreCause === undefined + ? "Check that the dev data directory is writable." + : "Check that the dev data directory is writable, then run the share command again."; } } @@ -333,7 +341,25 @@ export const shareDevServer = Effect.fn("devShare.shareDevServer")(function* (in export const acquireDevShare = Effect.fn("devShare.acquireDevShare")(function* ( lease: DevShareLease, ) { - const host = yield* prepareDevShare({ webPort: lease.webPort }); - yield* claimDevShareLease(lease); - return yield* publishDevShare({ host, webPort: lease.webPort }); + return yield* Effect.gen(function* () { + const host = yield* prepareDevShare({ webPort: lease.webPort }); + yield* claimDevShareLease(lease).pipe( + Effect.catch((error: DevShareLeaseClaimError) => + publishDevShare({ host, webPort: lease.webPort }).pipe( + Effect.matchEffect({ + onFailure: (restoreCause) => + Effect.fail( + new DevShareLeaseClaimError({ + leasePath: lease.leasePath, + cause: error.cause, + restoreCause, + }), + ), + onSuccess: () => Effect.fail(error), + }), + ), + ), + ); + return yield* publishDevShare({ host, webPort: lease.webPort }); + }).pipe(Effect.uninterruptible); }); From 2071c986a0c4746de52c27cb8d2fcd70ccdf4cbe Mon Sep 17 00:00:00 2001 From: Theo Browne Date: Sun, 26 Jul 2026 17:34:55 -0700 Subject: [PATCH 17/23] Keep desktop session cookies port-scoped Switching the cookie-name discriminator to devUrl alone dropped the port scope for packaged desktop, reverting #1898: desktop instances scan upward from 3773 for a free port and bind 127.0.0.1, so two of them share a hostname and would clobber each other's t3_session. Scope on devUrl OR desktop mode, and restore the assertions that had been relaxed to match the regression. Co-Authored-By: Claude Fable 5 --- apps/server/src/auth/EnvironmentAuth.test.ts | 6 +++-- .../src/auth/EnvironmentAuthPolicy.test.ts | 23 +++++++++++++++++-- apps/server/src/auth/EnvironmentAuthPolicy.ts | 6 ++++- apps/server/src/auth/SessionStore.ts | 1 + apps/server/src/auth/utils.ts | 22 ++++++++++++++---- apps/server/src/server.test.ts | 4 +++- 6 files changed, 51 insertions(+), 11 deletions(-) diff --git a/apps/server/src/auth/EnvironmentAuth.test.ts b/apps/server/src/auth/EnvironmentAuth.test.ts index 9eaac9eda18..8432f49695a 100644 --- a/apps/server/src/auth/EnvironmentAuth.test.ts +++ b/apps/server/src/auth/EnvironmentAuth.test.ts @@ -46,8 +46,10 @@ const makeCookieRequest = ( ({ cookies: { // Derived, not hardcoded: the name is port-scoped so concurrent servers - // on one hostname don't share a cookie. - [resolveSessionCookieName({ port: TEST_SERVER_PORT, devUrl: undefined })]: sessionToken, + // on one hostname don't share a cookie. Mode and devUrl mirror + // ServerConfig.layerTest, so this resolves to whatever the server reads. + [resolveSessionCookieName({ mode: "web", port: TEST_SERVER_PORT, devUrl: undefined })]: + sessionToken, }, headers: {}, }) as unknown as Parameters< diff --git a/apps/server/src/auth/EnvironmentAuthPolicy.test.ts b/apps/server/src/auth/EnvironmentAuthPolicy.test.ts index 9f19f5d5f74..0e21ef19c90 100644 --- a/apps/server/src/auth/EnvironmentAuthPolicy.test.ts +++ b/apps/server/src/auth/EnvironmentAuthPolicy.test.ts @@ -34,7 +34,10 @@ it.layer(NodeServices.layer)("EnvironmentAuthPolicy.layer", (it) => { expect(descriptor.policy).toBe("desktop-managed-local"); expect(descriptor.bootstrapMethods).toEqual(["desktop-bootstrap"]); - expect(descriptor.sessionCookieName).toBe("t3_session"); + // Packaged desktop has no devUrl, but still needs the port scope: it + // scans upward from 3773 for a free port and binds 127.0.0.1, so a second + // instance shares this one's hostname on a different port. + expect(descriptor.sessionCookieName).toBe("t3_session_3773"); }).pipe( Effect.provide( makeEnvironmentAuthPolicyLayer({ @@ -45,6 +48,22 @@ it.layer(NodeServices.layer)("EnvironmentAuthPolicy.layer", (it) => { ), ); + it.effect("keeps desktop cookies port-scoped on the port a second instance lands on", () => + Effect.gen(function* () { + const policy = yield* EnvironmentAuthPolicy.EnvironmentAuthPolicy; + const descriptor = yield* policy.getDescriptor(); + + expect(descriptor.sessionCookieName).toBe("t3_session_3774"); + }).pipe( + Effect.provide( + makeEnvironmentAuthPolicyLayer({ + mode: "desktop", + port: 3774, + }), + ), + ), + ); + it.effect("uses remote-reachable policy for desktop mode when bound beyond loopback", () => Effect.gen(function* () { const policy = yield* EnvironmentAuthPolicy.EnvironmentAuthPolicy; @@ -81,7 +100,7 @@ it.layer(NodeServices.layer)("EnvironmentAuthPolicy.layer", (it) => { ), ); - it.effect("scopes session cookies by port only in development", () => + it.effect("scopes web session cookies by port only in development", () => Effect.gen(function* () { const policy = yield* EnvironmentAuthPolicy.EnvironmentAuthPolicy; const descriptor = yield* policy.getDescriptor(); diff --git a/apps/server/src/auth/EnvironmentAuthPolicy.ts b/apps/server/src/auth/EnvironmentAuthPolicy.ts index d43db0d0ba9..28e41576769 100644 --- a/apps/server/src/auth/EnvironmentAuthPolicy.ts +++ b/apps/server/src/auth/EnvironmentAuthPolicy.ts @@ -38,7 +38,11 @@ export const make = Effect.gen(function* () { policy, bootstrapMethods, sessionMethods: ["browser-session-cookie", "bearer-access-token", "dpop-access-token"], - sessionCookieName: resolveSessionCookieName({ port: config.port, devUrl: config.devUrl }), + sessionCookieName: resolveSessionCookieName({ + mode: config.mode, + port: config.port, + devUrl: config.devUrl, + }), }; return EnvironmentAuthPolicy.of({ diff --git a/apps/server/src/auth/SessionStore.ts b/apps/server/src/auth/SessionStore.ts index b8d3be63483..efa811302dc 100644 --- a/apps/server/src/auth/SessionStore.ts +++ b/apps/server/src/auth/SessionStore.ts @@ -468,6 +468,7 @@ export const make = Effect.gen(function* () { const connectedSessionsRef = yield* Ref.make(new Map()); const changesPubSub = yield* PubSub.unbounded(); const cookieName = resolveSessionCookieName({ + mode: serverConfig.mode, port: serverConfig.port, devUrl: serverConfig.devUrl, }); diff --git a/apps/server/src/auth/utils.ts b/apps/server/src/auth/utils.ts index 060f1d29f0a..81ef9bffc49 100644 --- a/apps/server/src/auth/utils.ts +++ b/apps/server/src/auth/utils.ts @@ -11,16 +11,28 @@ import * as Result from "effect/Result"; const SESSION_COOKIE_NAME = "t3_session"; /** - * Cookies are scoped by host but *not* by port, so concurrent dev servers on a - * hostname need separate names. Hosted deployments keep the stable production - * name: their public port can change between releases, and scoping it would log - * every user out. + * Cookies are scoped by host but *not* by port, so any two servers that can be + * live on one hostname at once need separate names — otherwise the second + * clobbers the first's session and both sides see "Invalid session token + * signature" until someone clears cookies by hand. + * + * Two populations qualify, for the same reason but from different causes: + * + * - **Dev servers** (`devUrl` set), which run several at a time across worktrees. + * - **Desktop**, which scans upward from 3773 for a free port and binds + * 127.0.0.1, so a second instance lands on a different port and the same host. + * + * Hosted deployments keep the stable production name: their public port can + * change between releases, and scoping it would log every user out. */ export function resolveSessionCookieName(input: { + readonly mode: "web" | "desktop"; readonly port: number; readonly devUrl: URL | undefined; }): string { - return input.devUrl === undefined ? SESSION_COOKIE_NAME : `${SESSION_COOKIE_NAME}_${input.port}`; + return input.devUrl === undefined && input.mode !== "desktop" + ? SESSION_COOKIE_NAME + : `${SESSION_COOKIE_NAME}_${input.port}`; } export function base64UrlEncode(input: string | Uint8Array): string { diff --git a/apps/server/src/server.test.ts b/apps/server/src/server.test.ts index 948c2b4ff45..f06c27a066e 100644 --- a/apps/server/src/server.test.ts +++ b/apps/server/src/server.test.ts @@ -1329,7 +1329,9 @@ it.layer(NodeServices.layer)("server router seam", (it) => { "bearer-access-token", "dpop-access-token", ]); - assert.equal(body.auth.sessionCookieName, "t3_session"); + // Desktop, so port-scoped: instances scan for a free port and share + // 127.0.0.1, and cookies are not scoped by port. + assert.isTrue(body.auth.sessionCookieName.startsWith("t3_session_")); }).pipe(Effect.provide(NodeHttpServer.layerTest)), ); From 63b2613871f522d86141f1e8e97090b4b06da705 Mon Sep 17 00:00:00 2001 From: Theo Browne Date: Sun, 26 Jul 2026 17:34:56 -0700 Subject: [PATCH 18/23] Drop an inherited HOST in browser dev modes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit HOST is Vite's own bind address and gates the HMR pin in vite.config.ts; the runner only sets it for dev:desktop. An exported one (container, CI, HOST=0.0.0.0 habit) survived into dev/dev:web and pinned the HMR socket to that address — invisible over a shared origin, since the page loads and only HMR dials the wrong machine. Clear it alongside the VITE_* URLs. Co-Authored-By: Claude Fable 5 --- scripts/dev-runner.test.ts | 69 ++++++++++++++++++++++++++++++++++++++ scripts/dev-runner.ts | 7 ++++ 2 files changed, 76 insertions(+) diff --git a/scripts/dev-runner.test.ts b/scripts/dev-runner.test.ts index 47cce2b26ed..c2a2decdf2f 100644 --- a/scripts/dev-runner.test.ts +++ b/scripts/dev-runner.test.ts @@ -428,6 +428,75 @@ it.layer(NodeServices.layer)("dev-runner", (it) => { }), ); + // HOST is Vite's bind address and gates the HMR pin in vite.config.ts. An + // inherited one would survive into browser dev and point HMR at the wrong + // interface — invisible over a shared origin, since the page still loads. + for (const mode of ["dev", "dev:web"] as const) { + it.effect(`drops an inherited HOST in ${mode} mode`, () => + Effect.gen(function* () { + const env = yield* createDevRunnerEnv({ + mode, + baseEnv: { HOST: "0.0.0.0" }, + serverOffset: 0, + webOffset: 0, + t3Home: undefined, + browser: undefined, + autoBootstrapProjectFromCwd: undefined, + logWebSocketEvents: undefined, + host: undefined, + port: undefined, + devUrl: undefined, + }); + + assert.equal(env.HOST, undefined); + }), + ); + } + + // --host configures the *backend* (T3CODE_HOST). It must not become Vite's + // bind address by way of an inherited HOST that happens to agree with it. + it.effect("drops an inherited HOST even when --host is given", () => + Effect.gen(function* () { + const env = yield* createDevRunnerEnv({ + mode: "dev", + baseEnv: { HOST: "0.0.0.0" }, + serverOffset: 0, + webOffset: 0, + t3Home: undefined, + browser: undefined, + autoBootstrapProjectFromCwd: undefined, + logWebSocketEvents: undefined, + host: "0.0.0.0", + port: undefined, + devUrl: undefined, + }); + + assert.equal(env.HOST, undefined); + assert.equal(env.T3CODE_HOST, "0.0.0.0"); + }), + ); + + // Desktop sets HOST itself, so the clearing must not reach it. + it.effect("still pins HOST for dev:desktop", () => + Effect.gen(function* () { + const env = yield* createDevRunnerEnv({ + mode: "dev:desktop", + baseEnv: { HOST: "0.0.0.0" }, + serverOffset: 0, + webOffset: 0, + t3Home: undefined, + browser: undefined, + autoBootstrapProjectFromCwd: undefined, + logWebSocketEvents: undefined, + host: undefined, + port: undefined, + devUrl: undefined, + }); + + assert.equal(env.HOST, "127.0.0.1"); + }), + ); + it.effect("keeps explicit backend URLs for the desktop renderer", () => Effect.gen(function* () { const env = yield* createDevRunnerEnv({ diff --git a/scripts/dev-runner.ts b/scripts/dev-runner.ts index 8d148df883e..f9e7c853c27 100644 --- a/scripts/dev-runner.ts +++ b/scripts/dev-runner.ts @@ -295,6 +295,13 @@ export function createDevRunnerEnv({ if (!isDesktopMode) { output.T3CODE_PORT = String(serverPort); + // HOST is Vite's own bind address, and the desktop branch below is the + // only place we set it. An inherited one (an exported HOST, a container, + // a `HOST=0.0.0.0 npm start` habit) would otherwise reach Vite and pin + // its HMR socket to that address — see the `explicitHost` gate in + // apps/web/vite.config.ts. Over a shared origin that is invisible: the + // page loads and only HMR quietly dials the wrong machine. + delete output.HOST; if (mode === "dev" || mode === "dev:web") { // Browser dev is single-origin: everything (including /ws) is proxied // through Vite, so the client must resolve its backend from From 1c8e54057a944d1e62a44369090641cd86f00ad9 Mon Sep 17 00:00:00 2001 From: Theo Browne Date: Sun, 26 Jul 2026 18:11:34 -0700 Subject: [PATCH 19/23] Remove the dev-share lease protocol MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five commits of ownership machinery (lease files, claim/handoff ordering, restore-on-failure) existed to close one race: a fast restart where the old runner's finalizer tears down a mapping a new runner just installed on the same port. For a dev-only convenience that failure is visible — the shared URL stops working — and re-running --share fixes it, so the protocol cost more than the race it prevented. Each hardening round also surfaced new races inside the mechanism itself. Back to the plain shape: clear any stale mapping, serve, unshare on exit, registered atomically via acquireRelease. Stale mappings from killed runs were always handled by the pre-clear on the next share, and still are. Co-Authored-By: Claude Fable 5 --- scripts/dev-runner.ts | 33 +++---- scripts/lib/dev-share.test.ts | 145 +----------------------------- scripts/lib/dev-share.ts | 160 ++-------------------------------- 3 files changed, 26 insertions(+), 312 deletions(-) diff --git a/scripts/dev-runner.ts b/scripts/dev-runner.ts index f9e7c853c27..38b6e31a8b8 100644 --- a/scripts/dev-runner.ts +++ b/scripts/dev-runner.ts @@ -1,7 +1,6 @@ #!/usr/bin/env node import * as NodeOS from "node:os"; -import * as NodeCrypto from "node:crypto"; import * as NodeRuntime from "@effect/platform-node/NodeRuntime"; import * as NodeServices from "@effect/platform-node/NodeServices"; @@ -20,7 +19,7 @@ import * as Schema from "effect/Schema"; import { Argument, Command, Flag } from "effect/unstable/cli"; import { ChildProcess } from "effect/unstable/process"; -import { acquireDevShare, cleanupOwnedDevShare, type DevShareError } from "./lib/dev-share.ts"; +import { type DevShareError, shareDevServer, unshareDevServer } from "./lib/dev-share.ts"; import { loadRepoEnv } from "./lib/public-config.ts"; Object.assign(process.env, loadRepoEnv()); @@ -659,32 +658,36 @@ export function runDevRunnerWithInput(input: DevRunnerCliInput) { "[dev-runner] --share is not supported for dev:desktop (the renderer is pinned to loopback). Use `dev`, which runs the whole browser stack.", ); } else { - const path = yield* Path.Path; // acquireRelease, not share-then-addFinalizer: the mapping outlives this // process (and reboots), so the cleanup has to be registered atomically // with creating it. An interrupt landing in between would otherwise // leave a mapping pointing at a port nothing is listening on. // + // Deliberately no ownership tracking beyond that: if a second runner + // takes this port during a fast restart, the first's exit can briefly + // tear down the new mapping — visible (the URL stops working) and fixed + // by re-running --share. A lease protocol closing that window existed + // and was removed as more machinery than a dev convenience warrants. + // // A tailnet that isn't up shouldn't stop the dev server from starting — // warn, and carry on serving locally. - const shared = yield* Effect.gen(function* () { - const lease = { - leasePath: path.join(baseDir, "dev-share", `${String(sharedWebPort)}.owner`), - ownerId: `${String(process.pid)}:${NodeCrypto.randomUUID()}`, - webPort: sharedWebPort, - }; - return yield* Effect.acquireRelease(acquireDevShare(lease), () => - cleanupOwnedDevShare(lease).pipe( + const shared = yield* Effect.acquireRelease( + shareDevServer({ webPort: sharedWebPort }), + () => + // Serve config outlives this process, so a cleanup that did not + // take leaves a tailnet URL pointing at a port nothing serves. + unshareDevServer(sharedWebPort).pipe( Effect.flatMap((result) => - result.status !== "failed" + result.cleared ? Effect.void : Effect.logWarning( - `[dev-runner] could not safely clean up the tailnet mapping for port ${String(sharedWebPort)}: ${result.explanation}`, + `[dev-runner] could not remove the tailnet mapping for port ${String(sharedWebPort)}${ + result.explanation ? `: ${result.explanation}` : "" + }. Remove it with \`tailscale serve --https=${String(sharedWebPort)} off\`.`, ), ), ), - ); - }).pipe( + ).pipe( Effect.tapError((error: DevShareError) => Effect.logWarning( `[dev-runner] could not share on the tailnet: ${error.message}${ diff --git a/scripts/lib/dev-share.test.ts b/scripts/lib/dev-share.test.ts index e3e806e60a7..b2dfba3585e 100644 --- a/scripts/lib/dev-share.test.ts +++ b/scripts/lib/dev-share.test.ts @@ -1,18 +1,12 @@ -import * as NodeServices from "@effect/platform-node/NodeServices"; import { assert, describe, it } from "@effect/vitest"; import * as Effect from "effect/Effect"; -import * as FileSystem from "effect/FileSystem"; import * as Layer from "effect/Layer"; import * as Sink from "effect/Sink"; import * as Stream from "effect/Stream"; import { ChildProcessSpawner } from "effect/unstable/process"; import { - acquireDevShare, - claimDevShareLease, - cleanupOwnedDevShare, type DevShareError, - DevShareLeaseClaimError, DevServeFailedError, shareDevServer, unshareDevServer, @@ -33,17 +27,11 @@ const encode = (value: string) => Stream.make(new TextEncoder().encode(value)); * test set the outcome of the `off` (pre-clear) and `serve` calls separately — * they are the same subcommand and are told apart by the trailing `off`. */ -const spawnerLayer = (input: { - readonly off?: CallResult; - readonly serve?: CallResult; - readonly onOff?: Effect.Effect; - readonly calls?: Array>; -}) => +const spawnerLayer = (input: { readonly off?: CallResult; readonly serve?: CallResult }) => Layer.succeed( ChildProcessSpawner.ChildProcessSpawner, ChildProcessSpawner.make((command) => { const args = "args" in command ? (command.args as ReadonlyArray) : []; - input.calls?.push(args); const result: CallResult = args.includes("status") ? { exitCode: 0 } : args.includes("off") @@ -53,9 +41,7 @@ const spawnerLayer = (input: { return Effect.succeed( ChildProcessSpawner.makeHandle({ pid: ChildProcessSpawner.ProcessId(1), - exitCode: (args.includes("off") ? (input.onOff ?? Effect.void) : Effect.void).pipe( - Effect.as(ChildProcessSpawner.ExitCode(result.exitCode)), - ), + exitCode: Effect.succeed(ChildProcessSpawner.ExitCode(result.exitCode)), isRunning: Effect.succeed(false), kill: () => Effect.void, unref: Effect.succeed(Effect.void), @@ -190,130 +176,3 @@ describe("shareDevServer", () => { }), ); }); - -it.layer(NodeServices.layer)("dev share cleanup ownership", (it) => { - it.effect("keeps the prior owner when clearing its mapping fails", () => - Effect.gen(function* () { - const fileSystem = yield* FileSystem.FileSystem; - const directory = yield* fileSystem.makeTempDirectoryScoped({ - prefix: "t3-dev-share-lease-", - }); - const leasePath = `${directory}/5788.owner`; - const calls: Array> = []; - - yield* fileSystem.writeFileString(leasePath, "old-runner"); - yield* acquireDevShare({ - leasePath, - ownerId: "new-runner", - webPort: 5788, - }).pipe( - Effect.provide( - spawnerLayer({ - calls, - off: { exitCode: 1, stderr: "permission denied" }, - }), - ), - Effect.flip, - ); - - assert.equal(yield* fileSystem.readFileString(leasePath), "old-runner"); - assert.equal(calls.filter((args) => args.includes("off")).length, 1); - }), - ); - - it.effect("claims ownership after clearing and before publishing", () => - Effect.gen(function* () { - const fileSystem = yield* FileSystem.FileSystem; - const directory = yield* fileSystem.makeTempDirectoryScoped({ - prefix: "t3-dev-share-lease-", - }); - const leasePath = `${directory}/5788.owner`; - - yield* fileSystem.writeFileString(leasePath, "old-runner"); - yield* acquireDevShare({ - leasePath, - ownerId: "new-runner", - webPort: 5788, - }).pipe( - Effect.provide( - spawnerLayer({ - off: { exitCode: 0 }, - serve: { exitCode: 1, stderr: "permission denied" }, - }), - ), - Effect.flip, - ); - - assert.equal(yield* fileSystem.readFileString(leasePath), "new-runner"); - }), - ); - - it.effect("restores the mapping when ownership cannot be claimed", () => - Effect.gen(function* () { - const fileSystem = yield* FileSystem.FileSystem; - const directory = yield* fileSystem.makeTempDirectoryScoped({ - prefix: "t3-dev-share-lease-", - }); - const blockedDirectory = `${directory}/not-a-directory`; - const calls: Array> = []; - - yield* fileSystem.writeFileString(blockedDirectory, "blocked"); - const error = yield* acquireDevShare({ - leasePath: `${blockedDirectory}/5788.owner`, - ownerId: "new-runner", - webPort: 5788, - }).pipe(Effect.provide(spawnerLayer({ calls })), Effect.flip); - - assert.instanceOf(error, DevShareLeaseClaimError); - assert.isTrue(calls.some((args) => args.includes("off"))); - assert.isTrue(calls.some((args) => args.includes("http://127.0.0.1:5788"))); - }), - ); - - it.effect("restores a newer runner's mapping when ownership changes during cleanup", () => - Effect.gen(function* () { - const fileSystem = yield* FileSystem.FileSystem; - const directory = yield* fileSystem.makeTempDirectoryScoped({ - prefix: "t3-dev-share-lease-", - }); - const leasePath = `${directory}/5788.owner`; - const oldLease = { leasePath, ownerId: "old-runner", webPort: 5788 }; - const calls: Array> = []; - - yield* claimDevShareLease(oldLease); - const result = yield* cleanupOwnedDevShare(oldLease).pipe( - Effect.provide( - spawnerLayer({ - calls, - onOff: fileSystem.writeFileString(leasePath, "new-runner").pipe(Effect.orDie), - }), - ), - ); - - assert.equal(result.status, "restored"); - assert.isTrue(calls.some((args) => args.includes("off"))); - assert.isTrue(calls.some((args) => args.includes("http://127.0.0.1:5788"))); - }), - ); - - it.effect("leaves the mapping alone when a newer runner already owns it", () => - Effect.gen(function* () { - const fileSystem = yield* FileSystem.FileSystem; - const directory = yield* fileSystem.makeTempDirectoryScoped({ - prefix: "t3-dev-share-lease-", - }); - const leasePath = `${directory}/5788.owner`; - const oldLease = { leasePath, ownerId: "old-runner", webPort: 5788 }; - const calls: Array> = []; - - yield* claimDevShareLease(oldLease); - yield* claimDevShareLease({ ...oldLease, ownerId: "new-runner" }); - const result = yield* cleanupOwnedDevShare(oldLease).pipe( - Effect.provide(spawnerLayer({ calls })), - ); - - assert.equal(result.status, "superseded"); - assert.isEmpty(calls); - }), - ); -}); diff --git a/scripts/lib/dev-share.ts b/scripts/lib/dev-share.ts index 67455f5eec6..0f843b3ba91 100644 --- a/scripts/lib/dev-share.ts +++ b/scripts/lib/dev-share.ts @@ -22,9 +22,6 @@ import { type TailscaleStderrDiagnostic, } from "@t3tools/tailscale"; import * as Effect from "effect/Effect"; -import * as FileSystem from "effect/FileSystem"; -import * as Option from "effect/Option"; -import * as Path from "effect/Path"; import * as Schema from "effect/Schema"; import type { ChildProcessSpawner } from "effect/unstable/process"; @@ -113,32 +110,10 @@ export class DevServeFailedError extends Schema.TaggedErrorClass()( - "DevShareLeaseClaimError", - { - leasePath: Schema.String, - cause: Schema.Defect(), - restoreCause: Schema.optional(Schema.Defect()), - }, -) { - override get message(): string { - return this.restoreCause === undefined - ? `could not claim cleanup ownership at ${this.leasePath}` - : `could not claim cleanup ownership at ${this.leasePath} or restore the previous tailnet mapping`; - } - - get hint(): string { - return this.restoreCause === undefined - ? "Check that the dev data directory is writable." - : "Check that the dev data directory is writable, then run the share command again."; - } -} - export const DevShareError = Schema.Union([ TailscaleUnavailableError, TailnetNameMissingError, DevServeFailedError, - DevShareLeaseClaimError, ]); export type DevShareError = typeof DevShareError.Type; export const isDevShareError = Schema.is(DevShareError); @@ -188,82 +163,11 @@ export interface DevShareResult { readonly host: string; } -export interface DevShareLease { - readonly leasePath: string; - readonly ownerId: string; - readonly webPort: number; -} - -export const claimDevShareLease = Effect.fn("devShare.claimDevShareLease")(function* ( - lease: DevShareLease, -) { - const fileSystem = yield* FileSystem.FileSystem; - const path = yield* Path.Path; - yield* fileSystem.makeDirectory(path.dirname(lease.leasePath), { recursive: true }).pipe( - Effect.andThen(fileSystem.writeFileString(lease.leasePath, lease.ownerId)), - Effect.mapError((cause) => new DevShareLeaseClaimError({ leasePath: lease.leasePath, cause })), - ); - return lease; -}); - -export type DevShareCleanupResult = - | { readonly status: "cleared" | "superseded" | "restored" } - | { readonly status: "failed"; readonly explanation: string }; - /** - * Clears only the mapping this runner owns. If a successor claims the lease - * while `tailscale serve off` is in flight, restore the same port mapping so - * the old finalizer cannot silently disconnect the new runner. + * Publishes `webPort` on the tailnet at the same port number and returns the + * resulting HTTPS URL. Idempotent: re-running replaces any existing mapping. */ -export const cleanupOwnedDevShare = Effect.fn("devShare.cleanupOwnedDevShare")(function* ( - lease: DevShareLease, -): Effect.fn.Return< - DevShareCleanupResult, - never, - FileSystem.FileSystem | ChildProcessSpawner.ChildProcessSpawner -> { - const fileSystem = yield* FileSystem.FileSystem; - const readOwner = fileSystem - .readFileString(lease.leasePath) - .pipe(Effect.option, Effect.map(Option.getOrUndefined)); - const ownerBefore = yield* readOwner; - if (ownerBefore !== lease.ownerId) { - return { status: "superseded" }; - } - - const result = yield* unshareDevServer(lease.webPort); - if (!result.cleared) { - return { - status: "failed", - explanation: - result.explanation ?? - `could not remove the tailnet mapping for port ${String(lease.webPort)}`, - }; - } - - const ownerAfter = yield* readOwner; - if (ownerAfter === lease.ownerId) { - return { status: "cleared" }; - } - - return yield* ensureTailscaleServe({ - localPort: lease.webPort, - servePort: lease.webPort, - }).pipe( - Effect.as({ status: "restored" } as const), - Effect.catch((cause: TailscaleCommandError) => - Effect.succeed({ - status: "failed", - explanation: - explainCommandFailure(cause) ?? - `a newer runner claimed port ${String(lease.webPort)}, but its mapping could not be restored`, - } as const), - ), - Effect.uninterruptible, - ); -}); - -const prepareDevShare = Effect.fn("devShare.prepareDevShare")(function* (input: { +export const shareDevServer = Effect.fn("devShare.shareDevServer")(function* (input: { readonly webPort: number; }) { const status = yield* readTailscaleStatus.pipe( @@ -291,17 +195,7 @@ const prepareDevShare = Effect.fn("devShare.prepareDevShare")(function* (input: }); } - return status.magicDnsName; -}); - -const publishDevShare = Effect.fn("devShare.publishDevShare")(function* (input: { - readonly host: string; - readonly webPort: number; -}) { - yield* ensureTailscaleServe({ - localPort: input.webPort, - servePort: input.webPort, - }).pipe( + yield* ensureTailscaleServe({ localPort: input.webPort, servePort: input.webPort }).pipe( Effect.mapError((error) => { const explanation = explainCommandFailure(error); return new DevServeFailedError({ @@ -315,51 +209,9 @@ const publishDevShare = Effect.fn("devShare.publishDevShare")(function* (input: return { url: buildTailscaleHttpsBaseUrl({ - magicDnsName: input.host, + magicDnsName: status.magicDnsName, servePort: input.webPort, }), - host: input.host, + host: status.magicDnsName, } satisfies DevShareResult; }); - -/** - * Publishes `webPort` on the tailnet at the same port number and returns the - * resulting HTTPS URL. Idempotent: re-running replaces any existing mapping. - */ -export const shareDevServer = Effect.fn("devShare.shareDevServer")(function* (input: { - readonly webPort: number; -}) { - const host = yield* prepareDevShare(input); - return yield* publishDevShare({ ...input, host }); -}); - -/** - * Transfers cleanup ownership after the old mapping is known to be gone and - * before the new mapping is published. Failures on either side of that - * boundary therefore cannot leave a surviving mapping without an owner. - */ -export const acquireDevShare = Effect.fn("devShare.acquireDevShare")(function* ( - lease: DevShareLease, -) { - return yield* Effect.gen(function* () { - const host = yield* prepareDevShare({ webPort: lease.webPort }); - yield* claimDevShareLease(lease).pipe( - Effect.catch((error: DevShareLeaseClaimError) => - publishDevShare({ host, webPort: lease.webPort }).pipe( - Effect.matchEffect({ - onFailure: (restoreCause) => - Effect.fail( - new DevShareLeaseClaimError({ - leasePath: lease.leasePath, - cause: error.cause, - restoreCause, - }), - ), - onSuccess: () => Effect.fail(error), - }), - ), - ), - ); - return yield* publishDevShare({ host, webPort: lease.webPort }); - }).pipe(Effect.uninterruptible); -}); From faf646707741afe46bf1bcb7dd721513e070e1cd Mon Sep 17 00:00:00 2001 From: Theo Browne Date: Sun, 26 Jul 2026 18:59:41 -0700 Subject: [PATCH 20/23] Print the exact stop command at dev-runner startup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stopping the stack has no obvious right answer, and the wrong answers are severe: killing the runner PID orphans the node --watch server children (vp does not reap them; they hold the port through the next start), and guessing a pkill pattern has twice killed the agent driving the session — its argv contains the worktree path — and can reach other worktrees' servers. Print the one safe command instead: a process-group kill, with the pgid resolved via ps because under `bun run dev` the group leader is bun, not the runner. Verified live: the printed command run verbatim leaves zero survivors with the worktree's T3CODE_HOME. AGENTS.md and the test-t3-app skill now direct agents to that line and name the pattern-kill footgun explicitly. Co-Authored-By: Claude Fable 5 --- .agents/skills/test-t3-app/SKILL.md | 2 ++ AGENTS.md | 1 + scripts/dev-runner.test.ts | 23 ++++++++++++++++++++++ scripts/dev-runner.ts | 30 +++++++++++++++++++++++++++++ 4 files changed, 56 insertions(+) diff --git a/.agents/skills/test-t3-app/SKILL.md b/.agents/skills/test-t3-app/SKILL.md index 7bc0d86498a..2d04d154219 100644 --- a/.agents/skills/test-t3-app/SKILL.md +++ b/.agents/skills/test-t3-app/SKILL.md @@ -22,6 +22,8 @@ The worktree-local default deliberately outranks an ambient `T3CODE_HOME`; do no Ports are derived from the worktree path but can shift when occupied. Always read the actual values from the `[dev-runner]` line. +To stop the stack, run the exact command from the `[dev-runner] stop:` line — it kills the runner's process group, including the watch-mode children that outlive the runner itself. Never kill dev servers by name or path pattern (`pkill -f`, `pgrep | xargs kill`): the pattern matches your own agent process and can take down other worktrees' servers. + Shared browser dev is single-origin: Vite proxies the backend paths, so never set `VITE_HTTP_URL` or `VITE_WS_URL` for `dev`/`dev:web`. The dev runner disables browser auto-open by default. Do not pass `--browser` during automated testing: an automatically opened page can consume the one-time bootstrap token before the controlled browser uses it. diff --git a/AGENTS.md b/AGENTS.md index fd79e04d36c..8075f4a8ade 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -19,6 +19,7 @@ - Start the web stack with `bun run dev`. Use `bun run dev:share` when someone needs to open it from another device on the tailnet. - Browser dev is single-origin: Vite proxies `/api`, `/ws`, `/oauth`, and `/.well-known` to the backend. Do not set `VITE_HTTP_URL` or `VITE_WS_URL` for `dev`/`dev:web`. - Worktree paths supply stable preferred port offsets. Read the actual server and web ports from the `[dev-runner]` line because occupied ports can still shift them. +- Stop a dev server with the exact command from its `[dev-runner] stop:` line (a process-group kill). Never kill by name or path pattern — `pkill -f ` and `pgrep | xargs kill` match the agent's own process and other worktrees' servers. ## Package Roles diff --git a/scripts/dev-runner.test.ts b/scripts/dev-runner.test.ts index c2a2decdf2f..e8cfad61657 100644 --- a/scripts/dev-runner.test.ts +++ b/scripts/dev-runner.test.ts @@ -27,6 +27,7 @@ import { getDevRunnerModeArgs, resolveModePortOffsets, resolveOffset, + resolveStopCommand, runDevRunnerWithInput, } from "./dev-runner.ts"; @@ -72,6 +73,28 @@ const devServerInput = { } as const; it.layer(NodeServices.layer)("dev-runner", (it) => { + describe("resolveStopCommand", () => { + // The printed command must target the process *group*, resolved at kill + // time via ps: under `bun run dev` the group leader is bun, not the + // runner, so a hardcoded `kill -- -` would name a nonexistent + // group. The group is what reaps the `node --watch` children that + // otherwise outlive the runner holding the server port. + it.effect("resolves the group id from the pid rather than assuming them equal", () => + Effect.sync(() => { + assert.strictEqual( + resolveStopCommand(4321, "linux"), + `kill -TERM -"$(ps -o pgid= -p 4321 | tr -d ' ')"`, + ); + }), + ); + + it.effect("uses taskkill's tree flag on Windows, where process groups do not exist", () => + Effect.sync(() => { + assert.strictEqual(resolveStopCommand(4321, "win32"), "taskkill /pid 4321 /T"); + }), + ); + }); + describe("getDevRunnerModeArgs", () => { it.effect("lets Vite+ honor the desktop dev task graph", () => Effect.sync(() => { diff --git a/scripts/dev-runner.ts b/scripts/dev-runner.ts index 38b6e31a8b8..3acd33d6071 100644 --- a/scripts/dev-runner.ts +++ b/scripts/dev-runner.ts @@ -422,6 +422,25 @@ export function devPortProbeHosts(configuredHost: string | undefined): ReadonlyA return [...DEV_PORT_PROBE_HOSTS, host]; } +/** + * The exact command that stops this runner and everything it spawned, printed + * at startup. Exists because the alternative — guessing a `pkill` pattern — + * has twice killed the agent running the session (its argv contains the + * worktree path) and can reach other worktrees' dev servers. + * + * Targets the process group: children are spawned with `detached: false`, so + * the whole stack (including the `node --watch` server processes, which keep + * the group after re-parenting to init) shares it. The pgid is resolved via + * `ps` rather than assumed equal to our PID — under `bun run dev` the group + * leader is bun, not this runner, and `kill -- -` would name a + * group that does not exist. + */ +export function resolveStopCommand(pid: number, platform: NodeJS.Platform): string { + return platform === "win32" + ? `taskkill /pid ${String(pid)} /T` + : `kill -TERM -"$(ps -o pgid= -p ${String(pid)} | tr -d ' ')"`; +} + const makeDefaultCheckPortAvailability = (configuredHost: string | undefined): PortAvailabilityCheck => (port, role) => @@ -636,6 +655,17 @@ export function runDevRunnerWithInput(input: DevRunnerCliInput) { `[dev-runner] mode=${input.mode} source=${source}${selectionSuffix} serverPort=${String(env.T3CODE_PORT)} webPort=${String(env.PORT)} baseDir=${baseDir}`, ); + // The one safe way to stop this stack, printed so nobody has to invent + // one. Name-pattern kills (`pkill -f `, `pgrep | xargs kill`) + // are the documented footgun here: an agent driving this session has the + // worktree path in its own argv and matches itself, and broad patterns + // (`vite`, `node`) reach other worktrees' servers. Killing the process + // group also reaps the `node --watch` children, which vp does not clean + // up and which would otherwise outlive the runner holding the server port. + yield* Effect.logInfo( + `[dev-runner] stop: ${resolveStopCommand(process.pid, process.platform)} (kills this runner and every child; never kill by name pattern)`, + ); + // Before the share block: --dry-run only resolves and prints. Sharing would // replace, then tear down, whatever mapping the port already had — a // surprising side effect from a command documented as inert. From 08d9260817b9e1fae5a8641e6ad8a6499e00bb73 Mon Sep 17 00:00:00 2001 From: Theo Browne Date: Sun, 26 Jul 2026 19:00:45 -0700 Subject: [PATCH 21/23] Use HostProcessPlatform for the stop-command platform check Lint convention: platform comes from the injected reference, not process.platform, so tests can provide it explicitly. Co-Authored-By: Claude Fable 5 --- scripts/dev-runner.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/scripts/dev-runner.ts b/scripts/dev-runner.ts index 3acd33d6071..2dc39d30591 100644 --- a/scripts/dev-runner.ts +++ b/scripts/dev-runner.ts @@ -6,7 +6,11 @@ import * as NodeRuntime from "@effect/platform-node/NodeRuntime"; import * as NodeServices from "@effect/platform-node/NodeServices"; import * as NetService from "@t3tools/shared/Net"; import { resolveGitWorktreePath, resolveWorktreeT3Home } from "@t3tools/shared/devHome"; -import { HostProcessEnvironment, HostProcessWorkingDirectory } from "@t3tools/shared/hostProcess"; +import { + HostProcessEnvironment, + HostProcessPlatform, + HostProcessWorkingDirectory, +} from "@t3tools/shared/hostProcess"; import { resolveSpawnCommand } from "@t3tools/shared/shell"; import * as Config from "effect/Config"; import * as Effect from "effect/Effect"; @@ -662,8 +666,9 @@ export function runDevRunnerWithInput(input: DevRunnerCliInput) { // (`vite`, `node`) reach other worktrees' servers. Killing the process // group also reaps the `node --watch` children, which vp does not clean // up and which would otherwise outlive the runner holding the server port. + const hostPlatform = yield* HostProcessPlatform; yield* Effect.logInfo( - `[dev-runner] stop: ${resolveStopCommand(process.pid, process.platform)} (kills this runner and every child; never kill by name pattern)`, + `[dev-runner] stop: ${resolveStopCommand(process.pid, hostPlatform)} (kills this runner and every child; never kill by name pattern)`, ); // Before the share block: --dry-run only resolves and prints. Sharing would From 9055f7c5d770ce7ea19eee5c697c78ac2394e22d Mon Sep 17 00:00:00 2001 From: Theo Browne Date: Sun, 26 Jul 2026 19:31:42 -0700 Subject: [PATCH 22/23] Revert the printed stop command MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reverts faf646707 and 08d926081. The printed group-kill is unsafe in the launch context it was designed for: a runner spawned by an agent harness or automation tool (no job control) inherits its parent's process group, so `kill -TERM -` reaches the harness and unrelated siblings — the exact collateral the command existed to prevent. My earlier live verification missed this because bash's job control gave the nohup'd runner its own group. Resolving the pgid at kill time also fails or hits a recycled PID once the runner has exited, so it cannot clean up the orphaned watch children either. Doing this safely means setsid at spawn plus a pidfile, which is more machinery than this PR should grow. Out. Co-Authored-By: Claude Fable 5 --- .agents/skills/test-t3-app/SKILL.md | 2 -- AGENTS.md | 1 - scripts/dev-runner.test.ts | 23 ------------------ scripts/dev-runner.ts | 37 +---------------------------- 4 files changed, 1 insertion(+), 62 deletions(-) diff --git a/.agents/skills/test-t3-app/SKILL.md b/.agents/skills/test-t3-app/SKILL.md index 2d04d154219..7bc0d86498a 100644 --- a/.agents/skills/test-t3-app/SKILL.md +++ b/.agents/skills/test-t3-app/SKILL.md @@ -22,8 +22,6 @@ The worktree-local default deliberately outranks an ambient `T3CODE_HOME`; do no Ports are derived from the worktree path but can shift when occupied. Always read the actual values from the `[dev-runner]` line. -To stop the stack, run the exact command from the `[dev-runner] stop:` line — it kills the runner's process group, including the watch-mode children that outlive the runner itself. Never kill dev servers by name or path pattern (`pkill -f`, `pgrep | xargs kill`): the pattern matches your own agent process and can take down other worktrees' servers. - Shared browser dev is single-origin: Vite proxies the backend paths, so never set `VITE_HTTP_URL` or `VITE_WS_URL` for `dev`/`dev:web`. The dev runner disables browser auto-open by default. Do not pass `--browser` during automated testing: an automatically opened page can consume the one-time bootstrap token before the controlled browser uses it. diff --git a/AGENTS.md b/AGENTS.md index 8075f4a8ade..fd79e04d36c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -19,7 +19,6 @@ - Start the web stack with `bun run dev`. Use `bun run dev:share` when someone needs to open it from another device on the tailnet. - Browser dev is single-origin: Vite proxies `/api`, `/ws`, `/oauth`, and `/.well-known` to the backend. Do not set `VITE_HTTP_URL` or `VITE_WS_URL` for `dev`/`dev:web`. - Worktree paths supply stable preferred port offsets. Read the actual server and web ports from the `[dev-runner]` line because occupied ports can still shift them. -- Stop a dev server with the exact command from its `[dev-runner] stop:` line (a process-group kill). Never kill by name or path pattern — `pkill -f ` and `pgrep | xargs kill` match the agent's own process and other worktrees' servers. ## Package Roles diff --git a/scripts/dev-runner.test.ts b/scripts/dev-runner.test.ts index e8cfad61657..c2a2decdf2f 100644 --- a/scripts/dev-runner.test.ts +++ b/scripts/dev-runner.test.ts @@ -27,7 +27,6 @@ import { getDevRunnerModeArgs, resolveModePortOffsets, resolveOffset, - resolveStopCommand, runDevRunnerWithInput, } from "./dev-runner.ts"; @@ -73,28 +72,6 @@ const devServerInput = { } as const; it.layer(NodeServices.layer)("dev-runner", (it) => { - describe("resolveStopCommand", () => { - // The printed command must target the process *group*, resolved at kill - // time via ps: under `bun run dev` the group leader is bun, not the - // runner, so a hardcoded `kill -- -` would name a nonexistent - // group. The group is what reaps the `node --watch` children that - // otherwise outlive the runner holding the server port. - it.effect("resolves the group id from the pid rather than assuming them equal", () => - Effect.sync(() => { - assert.strictEqual( - resolveStopCommand(4321, "linux"), - `kill -TERM -"$(ps -o pgid= -p 4321 | tr -d ' ')"`, - ); - }), - ); - - it.effect("uses taskkill's tree flag on Windows, where process groups do not exist", () => - Effect.sync(() => { - assert.strictEqual(resolveStopCommand(4321, "win32"), "taskkill /pid 4321 /T"); - }), - ); - }); - describe("getDevRunnerModeArgs", () => { it.effect("lets Vite+ honor the desktop dev task graph", () => Effect.sync(() => { diff --git a/scripts/dev-runner.ts b/scripts/dev-runner.ts index 2dc39d30591..38b6e31a8b8 100644 --- a/scripts/dev-runner.ts +++ b/scripts/dev-runner.ts @@ -6,11 +6,7 @@ import * as NodeRuntime from "@effect/platform-node/NodeRuntime"; import * as NodeServices from "@effect/platform-node/NodeServices"; import * as NetService from "@t3tools/shared/Net"; import { resolveGitWorktreePath, resolveWorktreeT3Home } from "@t3tools/shared/devHome"; -import { - HostProcessEnvironment, - HostProcessPlatform, - HostProcessWorkingDirectory, -} from "@t3tools/shared/hostProcess"; +import { HostProcessEnvironment, HostProcessWorkingDirectory } from "@t3tools/shared/hostProcess"; import { resolveSpawnCommand } from "@t3tools/shared/shell"; import * as Config from "effect/Config"; import * as Effect from "effect/Effect"; @@ -426,25 +422,6 @@ export function devPortProbeHosts(configuredHost: string | undefined): ReadonlyA return [...DEV_PORT_PROBE_HOSTS, host]; } -/** - * The exact command that stops this runner and everything it spawned, printed - * at startup. Exists because the alternative — guessing a `pkill` pattern — - * has twice killed the agent running the session (its argv contains the - * worktree path) and can reach other worktrees' dev servers. - * - * Targets the process group: children are spawned with `detached: false`, so - * the whole stack (including the `node --watch` server processes, which keep - * the group after re-parenting to init) shares it. The pgid is resolved via - * `ps` rather than assumed equal to our PID — under `bun run dev` the group - * leader is bun, not this runner, and `kill -- -` would name a - * group that does not exist. - */ -export function resolveStopCommand(pid: number, platform: NodeJS.Platform): string { - return platform === "win32" - ? `taskkill /pid ${String(pid)} /T` - : `kill -TERM -"$(ps -o pgid= -p ${String(pid)} | tr -d ' ')"`; -} - const makeDefaultCheckPortAvailability = (configuredHost: string | undefined): PortAvailabilityCheck => (port, role) => @@ -659,18 +636,6 @@ export function runDevRunnerWithInput(input: DevRunnerCliInput) { `[dev-runner] mode=${input.mode} source=${source}${selectionSuffix} serverPort=${String(env.T3CODE_PORT)} webPort=${String(env.PORT)} baseDir=${baseDir}`, ); - // The one safe way to stop this stack, printed so nobody has to invent - // one. Name-pattern kills (`pkill -f `, `pgrep | xargs kill`) - // are the documented footgun here: an agent driving this session has the - // worktree path in its own argv and matches itself, and broad patterns - // (`vite`, `node`) reach other worktrees' servers. Killing the process - // group also reaps the `node --watch` children, which vp does not clean - // up and which would otherwise outlive the runner holding the server port. - const hostPlatform = yield* HostProcessPlatform; - yield* Effect.logInfo( - `[dev-runner] stop: ${resolveStopCommand(process.pid, hostPlatform)} (kills this runner and every child; never kill by name pattern)`, - ); - // Before the share block: --dry-run only resolves and prints. Sharing would // replace, then tear down, whatever mapping the port already had — a // surprising side effect from a command documented as inert. From 83783d0562db8c69ab55859f5244c92ba4f486cd Mon Sep 17 00:00:00 2001 From: Theo Browne Date: Sun, 26 Jul 2026 19:35:54 -0700 Subject: [PATCH 23/23] Reject a specific non-loopback --host in browser dev; restore vp run docs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Single-origin browser dev proxies backend traffic to localhost. A backend bound only to a specific interface (--host 192.168.1.10) leaves localhost unanswered, so every proxied request fails in a way that reads as a broken server rather than an unsupported flag combination. Reject it at startup with the two working alternatives (wildcard bind, --share). dev:server and dev:desktop don't proxy and keep specific-interface binds. Also returns the docs this PR touched to `vp run dev` — vp is this repo's task runner and forwards --share fine; the `bun run` wording was introduced by this PR itself responding to a bot nudge, matching neither main nor the surrounding lines. Co-Authored-By: Claude Fable 5 --- AGENTS.md | 2 +- docs/reference/scripts.md | 4 +- scripts/dev-runner.test.ts | 84 ++++++++++++++++++++++++++++++++++++++ scripts/dev-runner.ts | 46 +++++++++++++++++++++ 4 files changed, 133 insertions(+), 3 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index fd79e04d36c..b6881156b84 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -16,7 +16,7 @@ ## Dev Servers - In a linked git worktree, dev state defaults to that worktree's gitignored `.t3`. This deliberately outranks an ambient `T3CODE_HOME`, which could otherwise select the installed app's live `~/.t3/userdata` database. An explicit `--home-dir` still wins. -- Start the web stack with `bun run dev`. Use `bun run dev:share` when someone needs to open it from another device on the tailnet. +- Start the web stack with `vp run dev`. Add `--share` when someone needs to open it from another device on the tailnet. - Browser dev is single-origin: Vite proxies `/api`, `/ws`, `/oauth`, and `/.well-known` to the backend. Do not set `VITE_HTTP_URL` or `VITE_WS_URL` for `dev`/`dev:web`. - Worktree paths supply stable preferred port offsets. Read the actual server and web ports from the `[dev-runner]` line because occupied ports can still shift them. diff --git a/docs/reference/scripts.md b/docs/reference/scripts.md index b96186d9322..dac49a8825c 100644 --- a/docs/reference/scripts.md +++ b/docs/reference/scripts.md @@ -1,7 +1,7 @@ # Scripts -- `bun run dev` — Starts contracts, server, and web in watch mode. -- `bun run dev --share` (or `bun run dev:share`) — Also publishes the web port over HTTPS on this machine's tailnet. The startup pairing URL is built against the shared origin, and the mapping is removed on exit. +- `vp run dev` — Starts contracts, server, and web in watch mode. +- `vp run dev --share` — Also publishes the web port over HTTPS on this machine's tailnet. The startup pairing URL is built against the shared origin, and the mapping is removed on exit. - `vp run dev:server` — Starts just the WebSocket server. The server process runs on Bun (`@effect/platform-bun` + `BunPtyAdapter`), but task running uses `vp run`. - `vp run dev:web` — Starts just the Vite dev server for the web app. - Dev commands run from a linked **git worktree** default to that worktree's gitignored `.t3`, even when `T3CODE_HOME` is set, storing state in `/.t3/userdata`. Pass `--home-dir ` to choose another isolated directory explicitly. Submodules are not worktrees and keep the normal precedence. diff --git a/scripts/dev-runner.test.ts b/scripts/dev-runner.test.ts index c2a2decdf2f..c8cc75b0c5f 100644 --- a/scripts/dev-runner.test.ts +++ b/scripts/dev-runner.test.ts @@ -853,6 +853,90 @@ it.layer(NodeServices.layer)("dev-runner", (it) => { }); }); + // Single-origin browser dev proxies the backend at localhost, so a backend + // bound only to a specific interface breaks every proxied request in a way + // that looks like a broken server. Reject the combination up front. + it.effect("rejects a specific non-loopback --host for browser dev modes", () => { + const spawnerLayer = Layer.succeed( + ChildProcessSpawner.ChildProcessSpawner, + ChildProcessSpawner.make(() => Effect.succeed(mockProcess(0))), + ); + + return Effect.gen(function* () { + const error = yield* runDevRunnerWithInput({ + ...devServerInput, + mode: "dev", + port: undefined, + host: "192.168.1.10", + }).pipe( + Effect.provide(Layer.mergeAll(emptyConfigLayer, netServiceLayer, spawnerLayer)), + Effect.provideService(HostProcessPlatform, "linux"), + Effect.flip, + ); + + if (error._tag !== "DevRunnerHostNotProxiableError") { + assert.fail(`Unexpected error: ${error._tag}`); + } + assert.equal(error.mode, "dev"); + assert.equal(error.host, "192.168.1.10"); + assert.include(error.message, "0.0.0.0"); + assert.include(error.message, "--share"); + }); + }); + + // Wildcards keep loopback answering, so the proxy target stays valid and + // the combination must keep working — it is the documented way to serve a + // LAN interface and the browser proxy at once. + it.effect("still spawns the stack for a wildcard --host in dev mode", () => { + let spawnCount = 0; + const spawnerLayer = Layer.succeed( + ChildProcessSpawner.ChildProcessSpawner, + ChildProcessSpawner.make(() => { + spawnCount += 1; + return Effect.succeed(mockProcess(0)); + }), + ); + + return Effect.gen(function* () { + yield* runDevRunnerWithInput({ + ...devServerInput, + mode: "dev", + port: undefined, + host: "0.0.0.0", + }).pipe( + Effect.provide(Layer.mergeAll(emptyConfigLayer, netServiceLayer, spawnerLayer)), + Effect.provideService(HostProcessPlatform, "linux"), + ); + + assert.equal(spawnCount, 1); + }); + }); + + // dev:server does not proxy — the client talks to the backend directly — + // so a specific interface bind stays legitimate there. + it.effect("keeps a specific --host working for dev:server", () => { + let spawnCount = 0; + const spawnerLayer = Layer.succeed( + ChildProcessSpawner.ChildProcessSpawner, + ChildProcessSpawner.make(() => { + spawnCount += 1; + return Effect.succeed(mockProcess(0)); + }), + ); + + return Effect.gen(function* () { + yield* runDevRunnerWithInput({ + ...devServerInput, + host: "192.168.1.10", + }).pipe( + Effect.provide(Layer.mergeAll(emptyConfigLayer, netServiceLayer, spawnerLayer)), + Effect.provideService(HostProcessPlatform, "linux"), + ); + + assert.equal(spawnCount, 1); + }); + }); + it.effect("spawns nothing when --dry-run is combined with --share", () => { let spawnCount = 0; const spawnerLayer = Layer.succeed( diff --git a/scripts/dev-runner.ts b/scripts/dev-runner.ts index 38b6e31a8b8..976951dc543 100644 --- a/scripts/dev-runner.ts +++ b/scripts/dev-runner.ts @@ -36,6 +36,26 @@ const DESKTOP_DEV_LOOPBACK_HOST = "127.0.0.1"; // which silently moved the ports out from under a URL that had just been shared. const DEV_PORT_PROBE_HOSTS = ["127.0.0.1", "::1"] as const; +/** + * Bind hosts on which a backend still answers `http://localhost:`, which + * is where single-origin browser dev proxies to. Loopback and the wildcards + * qualify; a specific interface (e.g. a LAN IP) does not — the OS binds only + * that address and the proxy target goes dark. + */ +export function isProxiableBindHost(host: string): boolean { + const normalized = host.trim(); + return ( + normalized === "" || + normalized === "localhost" || + normalized === "127.0.0.1" || + normalized === "::1" || + normalized === "[::1]" || + normalized === "0.0.0.0" || + normalized === "::" || + normalized === "[::]" + ); +} + export const DEFAULT_T3_HOME = Effect.map(Effect.service(Path.Path), (path) => path.join(NodeOS.homedir(), ".t3"), ); @@ -143,8 +163,21 @@ export class DevRunnerProcessExitError extends Schema.TaggedErrorClass()( + "DevRunnerHostNotProxiableError", + { + mode: Schema.Literals(["dev", "dev:web"]), + host: Schema.String, + }, +) { + override get message(): string { + return `--host ${this.host} cannot be combined with ${this.mode}: single-origin browser dev proxies the backend at localhost, and a backend bound only to ${this.host} leaves localhost unanswered, so every proxied request fails. Use a wildcard (0.0.0.0 or ::) to serve that interface and loopback together, or --share for remote access.`; + } +} + export const DevRunnerError = Schema.Union([ DevRunnerConfigurationError, + DevRunnerHostNotProxiableError, DevRunnerInvalidPortOffsetError, DevRunnerPortExhaustedError, DevRunnerProcessError, @@ -582,6 +615,19 @@ export function runDevRunnerWithInput(input: DevRunnerCliInput) { ), ); + // Single-origin browser dev proxies the backend at localhost. A wildcard + // bind still answers there; a specific non-loopback interface does not, + // which breaks every proxied request in a way that reads as "server is + // broken" rather than "flag combination is unsupported". Reject it up + // front instead. (dev:server and dev:desktop don't proxy — untouched.) + if ( + (input.mode === "dev" || input.mode === "dev:web") && + input.host !== undefined && + !isProxiableBindHost(input.host) + ) { + return yield* new DevRunnerHostNotProxiableError({ mode: input.mode, host: input.host }); + } + const worktreePath = yield* resolveGitWorktreePath(yield* HostProcessWorkingDirectory); const { offset, source } = yield* resolveOffset({