From 9b1a64e4a9423b077eb5cf535996b991828dd0ac Mon Sep 17 00:00:00 2001 From: Theo Browne Date: Fri, 24 Jul 2026 16:55:50 -0700 Subject: [PATCH 1/5] Make dev servers shareable from other devices Browser dev is now single-origin: Vite proxies /api, /ws, /oauth, and /.well-known to the backend, and dev/dev:web no longer bake VITE_HTTP_URL/VITE_WS_URL into the bundle. Absolute localhost URLs in the bundle sent any remote browser to its own machine, which no amount of external path mapping could fix. - `dev --share` publishes the web port on the tailnet and logs a pairing URL already built against that origin - `dev:pair` mints a pairing URL for a running server, resolving port and state dir from server-runtime.json - Ports derive from the worktree path, so each worktree is stable across restarts; the availability probe is loopback-only so `tailscale serve` can't push the port - Web session cookies are port-scoped like desktop's, ending the cross-instance "Invalid session token signature" loop - Dev startup pairing tokens last 24h instead of 5m; user-issued links keep the 5m default Co-Authored-By: Claude Opus 5 (1M context) --- .agents/skills/test-t3-app/SKILL.md | 69 +++++-- AGENTS.md | 8 + apps/server/src/auth/EnvironmentAuth.test.ts | 9 +- apps/server/src/auth/EnvironmentAuth.ts | 4 +- .../src/auth/EnvironmentAuthPolicy.test.ts | 5 +- apps/server/src/auth/EnvironmentAuthPolicy.ts | 5 +- apps/server/src/auth/PairingGrantStore.ts | 16 +- apps/server/src/auth/SessionStore.ts | 5 +- apps/server/src/auth/utils.ts | 17 +- apps/server/src/cli/auth.ts | 74 +++++++- apps/server/src/http.ts | 28 ++- apps/server/src/serverRuntimeState.ts | 9 +- apps/web/vite.config.ts | 61 ++++-- docs/reference/scripts.md | 29 ++- package.json | 2 + scripts/dev-runner.test.ts | 55 +++++- scripts/dev-runner.ts | 133 ++++++++++++-- scripts/lib/dev-share.ts | 173 ++++++++++++++++++ 18 files changed, 636 insertions(+), 66 deletions(-) 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 b59ed9ddfe8..85d92c21718 100644 --- a/.agents/skills/test-t3-app/SKILL.md +++ b/.agents/skills/test-t3-app/SKILL.md @@ -13,9 +13,16 @@ 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 --home-dir `. Add `--share` + to also publish it on the tailnet so the user can open it from another device + (see "Share a dev server with the user" below). 4. Keep the terminal session alive and read the selected server port, web port, base directory, and pairing URL from its output. +Ports are derived from the worktree path, so each worktree gets its own stable +pair that survives restarts. Read them from the dev-runner line rather than +assuming `5733`/`13773`; `node scripts/dev-runner.ts dev --dry-run` prints them +without starting anything. + 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 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. @@ -42,20 +49,56 @@ Treat pairing URLs as secrets. Do not copy them into final responses, screenshot ## Recover a consumed or expired pairing token -Create another token against the same database and web URL as the running dev server: +Ask the running server for a fresh pairing URL: ```bash -T3CODE_PORT= node apps/server/src/bin.ts auth pairing create \ - --base-dir \ - --dev-url \ - --base-url \ - --ttl 15m \ - --label agent-ui-test +bun run dev:pair # implicit ~/.t3 home +bun run dev:pair -- --base-dir # explicit --home-dir environment ``` -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. +It reads the running server's own `server-runtime.json`, so it needs no port and +builds the URL against the web origin already in use — including a `--share` +tailnet URL. Pass `--base-dir` only when the dev server was started with +`--home-dir`, and pass exactly the same path: `--base-dir` switches the state +directory from `/dev` to `/userdata`, and a mismatch writes the token +to a database the server isn't reading. Add `--ttl 1h` for a longer window or +`--json` for machine-readable output. + +Use the printed `Pair URL` once. If the command reports no running server, the +dev process has exited — restart it and use the URL from its own startup log. + +Use `auth pairing list` to inspect active token metadata; it intentionally cannot +reveal token secrets. `auth pairing create` remains available for scripted cases +that need explicit control over every flag. + +## Share a dev server with the user + +When the user wants to try the change themselves — from their laptop, their +phone, or anything else on their tailnet — start the stack with `--share`: + +```bash +bun run dev:share # or: vp run dev --share --home-dir +``` + +This publishes the web port over HTTPS on the machine's tailnet and prints +`[dev-runner] shared on tailnet: https://:/`. The pairing URL logged +right after is already built against that origin, so give the user that URL +verbatim — it is the deliverable, and it works unchanged on any of their devices. + +Dev is single-origin: Vite proxies `/api`, `/ws`, `/oauth`, and `/.well-known` to +the backend, so one shared port covers the whole app. Do not map backend paths by +hand, and do not set `VITE_HTTP_URL`/`VITE_WS_URL` for a shared server — absolute +localhost URLs get compiled into the bundle and send the remote browser to its +own machine. + +The mapping is removed when the dev server exits, and re-running `--share` +replaces any mapping left behind by a run that was killed. If the tailnet is +unavailable the dev server still starts and serves locally; the warning explains +what to fix. -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. +Expect one failed `ws://localhost:` HMR attempt in the remote console +before Vite falls back to the page origin. That is Vite's own hot-reload socket, +not the app's connection — ignore it. ## Inspect or seed SQLite state @@ -83,7 +126,9 @@ If completion is uncertain, keep the environment alive and mention that it is re ## Troubleshoot predictably - If the browser shows an unauthenticated pairing screen, issue a new token instead of retrying the consumed URL. -- If the pairing URL is no longer visible, create a replacement token with both `--dev-url` and `--base-url`. -- If the replacement token is rejected, verify that the CLI and server use the identical absolute base directory and web URL. +- If the pairing URL is no longer visible, run `bun run dev:pair`. +- If the replacement token is rejected, verify that the CLI and server resolve the same state directory — pass `--base-dir` only when the server was started with `--home-dir`, and use the same path. - If the UI shows unexpected data, verify that every command uses the identical explicit base directory before editing anything. - If ports move because another instance is running, trust the current dev-runner output rather than assuming ports `13773` and `5733`. +- If a remote browser reaches the page but the app cannot connect, check that the served bundle has no absolute `localhost` backend URL baked in: `VITE_HTTP_URL` and `VITE_WS_URL` must be unset for `dev`/`dev:web`. +- If a shared URL returns 403 with "host is not allowed", the hostname is outside `*.ts.net`; add it to `T3CODE_DEV_ALLOWED_HOSTS` (comma-separated). diff --git a/AGENTS.md b/AGENTS.md index ef69591a340..a08ffc25ade 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -13,6 +13,14 @@ - 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 + +- Start the web stack with `bun run dev` (equivalently `vp run dev`). Never run `vp dev` from the repo root — that starts a bare Vite with no backend. +- Ports are derived from the worktree path, so each worktree has its own stable pair. Read them from the `[dev-runner] …` line; `--dry-run` prints them without starting anything. +- To let the user try a change on their own device, use `bun run dev:share`. It publishes the web port on the tailnet and logs a pairing URL already built against that origin — hand them that URL as-is. Details and troubleshooting live in the `test-t3-app` skill. +- Dev is single-origin: Vite proxies `/api`, `/ws`, `/oauth`, and `/.well-known` to the backend. Do not set `VITE_HTTP_URL`/`VITE_WS_URL` for `dev`/`dev:web` — they compile absolute localhost URLs into the bundle and break every non-localhost origin. +- Need a pairing URL for a server that is already running? `bun run dev:pair`. It finds the server itself; no ports or flags to get wrong. + ## 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/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..8e8d3b41b45 100644 --- a/apps/server/src/auth/EnvironmentAuth.ts +++ b/apps/server/src/auth/EnvironmentAuth.ts @@ -36,7 +36,9 @@ import { verifyRequestDpopProof } from "./dpop.ts"; import { layerConfig as SqlitePersistenceLayer } from "../persistence/Layers/Sqlite.ts"; export const DEFAULT_SESSION_SUBJECT = "cli-issued-session"; -export const INTERNAL_ADMINISTRATIVE_BOOTSTRAP_SUBJECT = "administrative-bootstrap"; +// Re-exported from PairingGrantStore, which keys the dev startup-token TTL off it. +export const INTERNAL_ADMINISTRATIVE_BOOTSTRAP_SUBJECT = + PairingGrantStore.INTERNAL_ADMINISTRATIVE_BOOTSTRAP_SUBJECT; export interface IssuedPairingLink { readonly id: string; 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..ccc673b26e0 100644 --- a/apps/server/src/auth/PairingGrantStore.ts +++ b/apps/server/src/auth/PairingGrantStore.ts @@ -243,6 +243,16 @@ 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); +export const INTERNAL_ADMINISTRATIVE_BOOTSTRAP_SUBJECT = "administrative-bootstrap"; const PAIRING_TOKEN_ALPHABET = "23456789ABCDEFGHJKLMNPQRSTUVWXYZ"; const PAIRING_TOKEN_LENGTH = 12; const PAIRING_TOKEN_REJECTION_LIMIT = @@ -371,7 +381,11 @@ 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?.subject === INTERNAL_ADMINISTRATIVE_BOOTSTRAP_SUBJECT; + 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/cli/auth.ts b/apps/server/src/cli/auth.ts index 1b349111811..50b278687d4 100644 --- a/apps/server/src/cli/auth.ts +++ b/apps/server/src/cli/auth.ts @@ -8,6 +8,7 @@ import * as Effect from "effect/Effect"; import * as Layer from "effect/Layer"; import * as Option from "effect/Option"; import * as References from "effect/References"; +import * as Schema from "effect/Schema"; import { Argument, Command, Flag, GlobalFlag } from "effect/unstable/cli"; import * as EnvironmentAuth from "../auth/EnvironmentAuth.ts"; @@ -19,6 +20,7 @@ import { formatSessionList, } from "../cliAuthFormat.ts"; import * as ServerConfig from "../config.ts"; +import { readPersistedServerRuntimeState } from "../serverRuntimeState.ts"; import { authLocationFlags, type CliAuthLocationFlags, @@ -26,6 +28,19 @@ import { resolveCliAuthConfig, } from "./config.ts"; +class NoRunningServerError extends Schema.TaggedErrorClass()( + "NoRunningServerError", + { statePath: Schema.String }, +) { + override get message(): string { + return [ + "No running T3 Code server was found for this data directory.", + `Looked for: ${this.statePath}`, + "Start one with `bun run dev`, or point at another directory with --base-dir.", + ].join("\n"); + } +} + const runWithEnvironmentAuth = ( flags: CliAuthLocationFlags, run: (environmentAuth: EnvironmentAuth.EnvironmentAuth["Service"]) => Effect.Effect, @@ -113,6 +128,58 @@ const pairingCreateCommand = Command.make("create", { ), ); +/** + * `t3 auth pairing url` — print a ready-to-open pairing link for a dev server + * that is already running, without having to know its port, its state + * directory, or which of those two the `--base-dir`/`--dev-url` combination + * happens to select. The running server records all of it in + * `server-runtime.json`; this reads that back. + * + * The startup link printed in the server's own log is usually enough. This is + * for when it has been consumed, scrolled away, or the log isn't at hand. + */ +const pairingUrlCommand = Command.make("url", { + ...authLocationFlags, + ttl: ttlFlag, + json: jsonFlag, +}).pipe( + Command.withDescription( + "Print a pairing URL for the dev server running against this data directory.", + ), + Command.withHandler((flags) => + Effect.gen(function* () { + const logLevel = yield* GlobalFlag.LogLevel; + const config = yield* resolveCliAuthConfig(flags, logLevel); + const runtimeState = yield* readPersistedServerRuntimeState(config.serverRuntimeStatePath); + + if (Option.isNone(runtimeState)) { + return yield* new NoRunningServerError({ statePath: config.serverRuntimeStatePath }); + } + + // Prefer the web origin the user actually opens; a server with no dev URL + // serves the app itself, so its own origin is the right target. + const baseUrl = runtimeState.value.devUrl ?? runtimeState.value.origin; + + return yield* runWithEnvironmentAuth( + flags, + (environmentAuth) => + Effect.gen(function* () { + const issued = yield* environmentAuth.createPairingLink({ + scopes: AuthStandardClientScopes, + subject: "one-time-token", + ...(Option.isSome(flags.ttl) ? { ttl: flags.ttl.value } : {}), + label: "cli-issued pairing url", + }); + yield* Console.log( + formatIssuedPairingCredential(issued, { json: flags.json, baseUrl }), + ); + }), + { quietLogs: flags.json }, + ); + }), + ), +); + const pairingListCommand = Command.make("list", { ...authLocationFlags, json: jsonFlag, @@ -156,7 +223,12 @@ const pairingRevokeCommand = Command.make("revoke", { const pairingCommand = Command.make("pairing").pipe( Command.withDescription("Manage one-time client pairing tokens."), - Command.withSubcommands([pairingCreateCommand, pairingListCommand, pairingRevokeCommand]), + Command.withSubcommands([ + pairingCreateCommand, + pairingUrlCommand, + pairingListCommand, + pairingRevokeCommand, + ]), ); const sessionIssueCommand = Command.make("issue", { diff --git a/apps/server/src/http.ts b/apps/server/src/http.ts index b9bb40f372d..7f31c7fbbbe 100644 --- a/apps/server/src/http.ts +++ b/apps/server/src/http.ts @@ -41,6 +41,16 @@ import { browserApiCorsAllowedHeaders, browserApiCorsAllowedMethods } from "./ht const OTLP_TRACES_PROXY_PATH = "/api/observability/v1/traces"; const LOOPBACK_HOSTNAMES = new Set(["127.0.0.1", "::1", "localhost"]); const DESKTOP_RENDERER_ORIGINS = ["t3code://app", "t3code-dev://app"]; +// Paths the web dev server proxies to us. Bouncing an unmatched one back to +// Vite would loop forever (Vite proxies it straight back), and it would answer +// an API call with index.html — so these 404 instead. +const DEV_PROXIED_PATH_PREFIXES = ["/api/", "/oauth/", "/.well-known/", "/ws"] as const; + +function isDevProxiedPath(pathname: string): boolean { + return DEV_PROXIED_PATH_PREFIXES.some( + (prefix) => pathname === prefix.replace(/\/$/, "") || pathname.startsWith(prefix), + ); +} export const browserApiCorsLayer = Layer.unwrap( Effect.gen(function* () { @@ -48,9 +58,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 +238,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/server/src/serverRuntimeState.ts b/apps/server/src/serverRuntimeState.ts index 329b000369a..ccd6038ceca 100644 --- a/apps/server/src/serverRuntimeState.ts +++ b/apps/server/src/serverRuntimeState.ts @@ -14,6 +14,12 @@ export const PersistedServerRuntimeState = Schema.Struct({ host: Schema.optional(Schema.String), port: Schema.Int, origin: Schema.String, + /** + * The web origin users actually open in dev — the Vite server, or the shared + * tailnet URL when the dev runner published one. Recorded so tooling can mint + * a pairing URL for a running server without being told where it lives. + */ + devUrl: Schema.optional(Schema.String), startedAt: Schema.String, }); export type PersistedServerRuntimeState = typeof PersistedServerRuntimeState.Type; @@ -45,7 +51,7 @@ const runtimeOriginForConfig = ( }; export const makePersistedServerRuntimeState = (input: { - readonly config: Pick; + readonly config: Pick; readonly port: number; }): Effect.Effect => Effect.map(DateTime.now, (now) => ({ @@ -54,6 +60,7 @@ export const makePersistedServerRuntimeState = (input: { ...(input.config.host ? { host: input.config.host } : {}), port: input.port, origin: runtimeOriginForConfig(input.config, input.port), + ...(input.config.devUrl ? { devUrl: input.config.devUrl.toString() } : {}), startedAt: DateTime.formatIso(now), })); diff --git a/apps/web/vite.config.ts b/apps/web/vite.config.ts index 6e5b532b58a..cb24cf55a9e 100644 --- a/apps/web/vite.config.ts +++ b/apps/web/vite.config.ts @@ -65,7 +65,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 +99,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,6 +168,7 @@ export default defineConfig(() => { host, port, strictPort: true, + allowedHosts, ...(devProxyTarget ? { proxy: { @@ -156,21 +180,36 @@ export default defineConfig(() => { target: devProxyTarget, changeOrigin: true, }, - "/attachments": { + "/oauth": { + target: devProxyTarget, + changeOrigin: true, + }, + // 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. + "/ws": { target: devProxyTarget, changeOrigin: true, + ws: true, }, }, } : {}), - 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, - }, + // 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".) + ...(process.env.HOST?.trim() + ? { + hmr: { + protocol: "ws", + host, + clientPort: port, + }, + } + : {}), }, build: { outDir: "dist", diff --git a/docs/reference/scripts.md b/docs/reference/scripts.md index 746aa66d563..27845797df3 100644 --- a/docs/reference/scripts.md +++ b/docs/reference/scripts.md @@ -1,6 +1,8 @@ # Scripts - `vp run dev` — Starts contracts, server, and web in watch mode. +- `vp run dev --share` (or `vp run dev:share`) — Same, plus publishes the web port on this machine's tailnet over HTTPS via `tailscale serve`. Prints the shareable URL, and the pairing URL is built against it so it can be opened from a phone or another laptop as-is. The mapping is removed on exit; if the tailnet is unavailable, the dev server still starts locally and logs a warning. +- `vp run dev:pair` — Prints a fresh pairing URL for the dev server already running against this data directory, resolving its port and web origin from `server-runtime.json`. Add `--base-dir ` only when the server was started with `--home-dir`. - `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. @@ -38,10 +40,27 @@ ## Running multiple dev instances -Set `T3CODE_DEV_INSTANCE` to any value to deterministically shift all dev ports together. +Ports resolve in this order, first match winning: -- Default ports: server `13773`, web `5733` -- Shifted ports: `base + offset` (offset is hashed from `T3CODE_DEV_INSTANCE`) -- Example: `T3CODE_DEV_INSTANCE=branch-a vp run dev:desktop` +1. `T3CODE_PORT_OFFSET=` — exact numeric offset, full control. +2. `T3CODE_DEV_INSTANCE=` — numeric offset, or a hashed one for non-numeric values. Example: `T3CODE_DEV_INSTANCE=branch-a vp run dev:desktop` +3. **Git worktree** — the offset is hashed from the worktree path, so every worktree gets its own stable pair that survives restarts and doesn't collide with its siblings. +4. Otherwise the defaults: server `13773`, web `5733`. -If you want full control instead of hashing, set `T3CODE_PORT_OFFSET` to a numeric offset. +Whatever the source, both ports are then checked on loopback and shifted together if either is taken. The resolved values are printed on the `[dev-runner] …` line; `--dry-run` prints them without starting anything. Read them from there rather than assuming the defaults. + +## Browser dev is single-origin + +`dev` and `dev:web` deliberately leave `VITE_HTTP_URL` and `VITE_WS_URL` unset so +the client resolves its backend from `window.location.origin`, with Vite proxying +`/api`, `/ws`, `/oauth`, and `/.well-known` to the server. That is what lets a dev +server work unchanged from a tailnet name, a LAN IP, or a phone. + +Setting those variables for web dev compiles absolute `localhost` URLs into the +bundle, and any browser that isn't on this machine will then try to reach its own +localhost. `dev:desktop` still sets them, because the Electron renderer talks to +the backend directly. + +Non-`.ts.net` hostnames need `T3CODE_DEV_ALLOWED_HOSTS` (comma-separated) to pass +Vite's host check; `T3CODE_DEV_ALLOWED_ORIGINS` does the same for the server's +CORS allowlist. diff --git a/package.json b/package.json index 3c0a8cc6b77..3dcfef804aa 100644 --- a/package.json +++ b/package.json @@ -5,8 +5,10 @@ "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:pair": "node apps/server/src/bin.ts auth pairing url", "dev:marketing": "vp run --filter @t3tools/marketing dev", "dev:desktop": "node scripts/dev-runner.ts dev:desktop", "start": "vp run --filter t3 start", diff --git a/scripts/dev-runner.test.ts b/scripts/dev-runner.test.ts index 3b79db49f5b..3f0767fabe7 100644 --- a/scripts/dev-runner.test.ts +++ b/scripts/dev-runner.test.ts @@ -58,6 +58,7 @@ const devServerInput = { port: 13_773, devUrl: undefined, dryRun: false, + share: false, runArgs: ["--inspect", "secret-token-value"], } as const; @@ -336,8 +337,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"); }), ); }); diff --git a/scripts/dev-runner.ts b/scripts/dev-runner.ts index 1938232300f..2ac3dc883c7 100644 --- a/scripts/dev-runner.ts +++ b/scripts/dev-runner.ts @@ -9,6 +9,7 @@ import { HostProcessEnvironment } from "@t3tools/shared/hostProcess"; import { resolveSpawnCommand } from "@t3tools/shared/shell"; import * as Config from "effect/Config"; import * as Effect from "effect/Effect"; +import * as FileSystem from "effect/FileSystem"; import * as Hash from "effect/Hash"; import * as Layer from "effect/Layer"; import * as Logger from "effect/Logger"; @@ -18,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()); @@ -27,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"), @@ -166,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 @@ -187,19 +195,46 @@ 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" }); +} + +/** + * The path of the linked git worktree we're running in, or undefined for the + * main checkout. Git marks a linked worktree by making `.git` a file + * (`gitdir: …`) rather than a directory. + */ +export function resolveWorktreePath( + cwd: string, +): Effect.Effect { + return Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const info = yield* fs.stat(path.join(cwd, ".git")).pipe(Effect.option); + return Option.isSome(info) && info.value.type === "File" ? cwd : undefined; + }); } function resolveBaseDir(baseDir: string | undefined): Effect.Effect { @@ -265,8 +300,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}`; @@ -480,6 +527,7 @@ interface DevRunnerCliInput { readonly port: number | undefined; readonly devUrl: URL | undefined; readonly dryRun: boolean; + readonly share: boolean; readonly runArgs: ReadonlyArray; } @@ -495,7 +543,11 @@ export function runDevRunnerWithInput(input: DevRunnerCliInput) { ), ); - const { offset, source } = yield* resolveOffset({ portOffset, devInstance }); + const { offset, source } = yield* resolveOffset({ + portOffset, + devInstance, + worktreePath: yield* resolveWorktreePath(process.cwd()), + }); const { serverOffset, webOffset } = yield* resolveModePortOffsets({ mode: input.mode, @@ -529,6 +581,55 @@ export function runDevRunnerWithInput(input: DevRunnerCliInput) { `[dev-runner] mode=${input.mode} source=${source}${selectionSuffix} serverPort=${String(env.T3CODE_PORT)} webPort=${String(env.PORT)} baseDir=${baseDir}`, ); + 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 }), + () => unshareDevServer(sharedWebPort), + ).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(","); + env.T3CODE_DEV_SHARE_URL = shared.url; + // 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}`); + } + } + } + if (input.dryRun) { return; } @@ -631,6 +732,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.ts b/scripts/lib/dev-share.ts new file mode 100644 index 00000000000..e4ea6617c52 --- /dev/null +++ b/scripts/lib/dev-share.ts @@ -0,0 +1,173 @@ +/** + * 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. + * + * 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 * as Effect from "effect/Effect"; +import * as Schema from "effect/Schema"; +import * as Stream from "effect/Stream"; +import { ChildProcess, ChildProcessSpawner } from "effect/unstable/process"; + +export class DevShareError extends Schema.TaggedErrorClass()("DevShareError", { + reason: Schema.Literals([ + "tailscale-missing", + "status-failed", + "status-unreadable", + "no-tailnet-name", + "serve-failed", + ]), + detail: Schema.optional(Schema.String), +}) { + override get message(): string { + const base = { + "tailscale-missing": "tailscale is not installed or not on PATH", + "status-failed": "could not read tailscale status", + "status-unreadable": "could not parse tailscale status output", + "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 { + switch (this.reason) { + case "tailscale-missing": + return "Install Tailscale, or drop --share and open the printed localhost URL."; + case "status-failed": + return "Is tailscaled running? Try `tailscale status`."; + case "no-tailnet-name": + return "Run `tailscale up` and make sure MagicDNS is enabled."; + default: + return undefined; + } + } +} + +/** The one field we need out of `tailscale status --json`. */ +const TailscaleStatus = Schema.fromJsonString( + Schema.Struct({ + Self: Schema.Struct({ + DNSName: Schema.String, + }), + }), +); +const decodeTailscaleStatus = Schema.decodeUnknownEffect(TailscaleStatus); + +const collectStreamAsString = (stream: Stream.Stream): Effect.Effect => + stream.pipe( + Stream.decodeText(), + Stream.runFold( + () => "", + (accumulated, chunk) => accumulated + chunk, + ), + ); + +interface TailscaleResult { + readonly exitCode: number; + readonly stdout: string; + readonly stderr: string; +} + +const runTailscale = Effect.fn("devShare.runTailscale")(function* ( + args: ReadonlyArray, + spawnFailureReason: DevShareError["reason"], +) { + const spawner = yield* ChildProcessSpawner.ChildProcessSpawner; + const child = yield* spawner + .spawn(ChildProcess.make("tailscale", args)) + .pipe(Effect.mapError(() => new DevShareError({ reason: "tailscale-missing" }))); + + const [stdout, stderr, exitCode] = yield* Effect.all( + [ + collectStreamAsString(child.stdout), + collectStreamAsString(child.stderr), + child.exitCode.pipe(Effect.map(Number)), + ], + { concurrency: "unbounded" }, + ).pipe( + Effect.mapError( + (cause) => new DevShareError({ reason: spawnFailureReason, detail: String(cause) }), + ), + ); + + return { exitCode, stdout, stderr } satisfies TailscaleResult; +}); + +/** The tailnet DNS name of this machine, e.g. `bb-1.example.ts.net`. */ +export const resolveTailnetHost = Effect.fn("devShare.resolveTailnetHost")(function* () { + const status = yield* runTailscale(["status", "--json"], "status-failed"); + if (status.exitCode !== 0) { + return yield* new DevShareError({ + reason: "status-failed", + ...(status.stderr.trim() ? { detail: status.stderr.trim() } : {}), + }); + } + + const decoded = yield* decodeTailscaleStatus(status.stdout).pipe( + Effect.mapError(() => new DevShareError({ reason: "status-unreadable" })), + ); + + // MagicDNS names come back fully qualified, with the trailing dot. + const host = decoded.Self.DNSName.replace(/\.$/, ""); + if (!host) { + return yield* new DevShareError({ reason: "no-tailnet-name" }); + } + return host; +}); + +export interface DevShareResult { + readonly url: string; + readonly host: string; +} + +/** + * Removes a mapping created by {@link shareDevServer}. Best-effort. + * + * Runs uninterruptibly with its own scope: this is called from a finalizer on + * the way out of an interrupted program, and spawning the cleanup subprocess + * under the dying scope would cancel it before `tailscale` ever ran — leaving + * exactly the stale mapping it exists to remove. + */ +export const unshareDevServer = (webPort: number) => + runTailscale(["serve", `--https=${String(webPort)}`, "off"], "serve-failed").pipe( + Effect.scoped, + Effect.ignore, + 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. + */ +export const shareDevServer = Effect.fn("devShare.shareDevServer")(function* (input: { + readonly webPort: number; +}) { + const host = yield* resolveTailnetHost(); + const port = String(input.webPort); + + // 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). + yield* unshareDevServer(input.webPort); + + const serve = yield* runTailscale( + ["serve", "--bg", `--https=${port}`, `http://127.0.0.1:${port}`], + "serve-failed", + ); + + if (serve.exitCode !== 0) { + return yield* new DevShareError({ + reason: "serve-failed", + detail: serve.stderr.trim() || `exit code ${String(serve.exitCode)} for port ${port}`, + }); + } + + return { url: `https://${host}:${port}/`, host } satisfies DevShareResult; +}); From 36fe3b31a39f3c43a46adea2660335495651ea23 Mon Sep 17 00:00:00 2001 From: Theo Browne Date: Fri, 24 Jul 2026 17:15:38 -0700 Subject: [PATCH 2/5] Fix review findings in dev sharing - `auth pairing url` resolved the state directory with the same flag heuristic the command exists to hide, so without --dev-url it read `userdata` while `bun run dev` writes to `dev`. It then minted into the wrong database and printed a URL the live server rejects with `invalid_credential`. It now searches both state directories and issues against whichever one the running server actually uses. - A server killed with SIGKILL leaves `server-runtime.json` behind, and that was taken as proof it was still up. Verify the recorded pid is alive before minting. - `--dry-run --share` replaced and then tore down the port's existing tailscale mapping. Dry run now returns before the share block. deriveServerPaths takes an optional explicit stateDir for callers that have already located a running server and must target it exactly. Co-Authored-By: Claude Opus 5 (1M context) --- .agents/skills/test-t3-app/SKILL.md | 13 ++- apps/server/src/cli/auth.ts | 93 ++++++++++++++++------ apps/server/src/config.ts | 14 +++- apps/server/src/serverRuntimeState.test.ts | 29 +++++++ apps/server/src/serverRuntimeState.ts | 25 ++++++ docs/reference/scripts.md | 2 +- scripts/dev-runner.test.ts | 28 +++++++ scripts/dev-runner.ts | 11 ++- 8 files changed, 176 insertions(+), 39 deletions(-) diff --git a/.agents/skills/test-t3-app/SKILL.md b/.agents/skills/test-t3-app/SKILL.md index 85d92c21718..6db626a30bc 100644 --- a/.agents/skills/test-t3-app/SKILL.md +++ b/.agents/skills/test-t3-app/SKILL.md @@ -56,13 +56,12 @@ bun run dev:pair # implicit ~/.t3 home bun run dev:pair -- --base-dir # explicit --home-dir environment ``` -It reads the running server's own `server-runtime.json`, so it needs no port and -builds the URL against the web origin already in use — including a `--share` -tailnet URL. Pass `--base-dir` only when the dev server was started with -`--home-dir`, and pass exactly the same path: `--base-dir` switches the state -directory from `/dev` to `/userdata`, and a mismatch writes the token -to a database the server isn't reading. Add `--ttl 1h` for a longer window or -`--json` for machine-readable output. +It finds the running server's own `server-runtime.json` under the base directory +— checking both the `dev` and `userdata` state directories and verifying the +recorded process is alive — so it needs no port and builds the URL against the +web origin already in use, including a `--share` tailnet URL. Pass `--base-dir` +only when the dev server was started with `--home-dir`. Add `--ttl 1h` for a +longer window or `--json` for machine-readable output. Use the printed `Pair URL` once. If the command reports no running server, the dev process has exited — restart it and use the URL from its own startup log. diff --git a/apps/server/src/cli/auth.ts b/apps/server/src/cli/auth.ts index 50b278687d4..85450df9aff 100644 --- a/apps/server/src/cli/auth.ts +++ b/apps/server/src/cli/auth.ts @@ -7,6 +7,7 @@ import * as Console from "effect/Console"; import * as Effect from "effect/Effect"; import * as Layer from "effect/Layer"; import * as Option from "effect/Option"; +import * as Path from "effect/Path"; import * as References from "effect/References"; import * as Schema from "effect/Schema"; import { Argument, Command, Flag, GlobalFlag } from "effect/unstable/cli"; @@ -20,7 +21,11 @@ import { formatSessionList, } from "../cliAuthFormat.ts"; import * as ServerConfig from "../config.ts"; -import { readPersistedServerRuntimeState } from "../serverRuntimeState.ts"; +import { + isPersistedServerRuntimeStateLive, + type PersistedServerRuntimeState, + readPersistedServerRuntimeState, +} from "../serverRuntimeState.ts"; import { authLocationFlags, type CliAuthLocationFlags, @@ -30,12 +35,11 @@ import { class NoRunningServerError extends Schema.TaggedErrorClass()( "NoRunningServerError", - { statePath: Schema.String }, + { baseDir: Schema.String }, ) { override get message(): string { return [ - "No running T3 Code server was found for this data directory.", - `Looked for: ${this.statePath}`, + `No running T3 Code server was found under ${this.baseDir}.`, "Start one with `bun run dev`, or point at another directory with --base-dir.", ].join("\n"); } @@ -128,6 +132,40 @@ const pairingCreateCommand = Command.make("create", { ), ); +/** + * The state directory is `/dev` for an implicit dev home and + * `/userdata` otherwise — a split that depends on flags the caller of + * this command should not have to reason about, and which silently mints + * tokens into a database the running server never reads when guessed wrong. + * So check both, and let the live server decide which one is right. + */ +const findLiveServerRuntimeState = Effect.fn("auth.findLiveServerRuntimeState")(function* ( + config: ServerConfig.ServerConfig["Service"], +) { + const path = yield* Path.Path; + const candidates = [ + config.serverRuntimeStatePath, + ...(["dev", "userdata"] as const).map((stateDir) => + path.join(config.baseDir, stateDir, "server-runtime.json"), + ), + ].filter((candidate, index, all) => all.indexOf(candidate) === index); + + for (const statePath of candidates) { + const state = yield* readPersistedServerRuntimeState(statePath); + if (Option.isNone(state)) { + continue; + } + // A file left behind by a killed or crashed server describes a port + // nothing is listening on. Minting against it produces a token the live + // server rejects with `invalid_credential`. + if (yield* isPersistedServerRuntimeStateLive(state.value)) { + return Option.some({ stateDir: path.dirname(statePath), state: state.value }); + } + } + + return Option.none<{ readonly stateDir: string; readonly state: PersistedServerRuntimeState }>(); +}); + /** * `t3 auth pairing url` — print a ready-to-open pairing link for a dev server * that is already running, without having to know its port, its state @@ -150,31 +188,40 @@ const pairingUrlCommand = Command.make("url", { Effect.gen(function* () { const logLevel = yield* GlobalFlag.LogLevel; const config = yield* resolveCliAuthConfig(flags, logLevel); - const runtimeState = yield* readPersistedServerRuntimeState(config.serverRuntimeStatePath); + const live = yield* findLiveServerRuntimeState(config); - if (Option.isNone(runtimeState)) { - return yield* new NoRunningServerError({ statePath: config.serverRuntimeStatePath }); + if (Option.isNone(live)) { + return yield* new NoRunningServerError({ baseDir: config.baseDir }); } + // Issue against the same state directory the server is using, whichever + // of the two it turned out to be. + const derivedPaths = yield* ServerConfig.deriveServerPaths(config.baseDir, config.devUrl, { + stateDir: live.value.stateDir, + }); + // Prefer the web origin the user actually opens; a server with no dev URL // serves the app itself, so its own origin is the right target. - const baseUrl = runtimeState.value.devUrl ?? runtimeState.value.origin; + const baseUrl = live.value.state.devUrl ?? live.value.state.origin; - return yield* runWithEnvironmentAuth( - flags, - (environmentAuth) => - Effect.gen(function* () { - const issued = yield* environmentAuth.createPairingLink({ - scopes: AuthStandardClientScopes, - subject: "one-time-token", - ...(Option.isSome(flags.ttl) ? { ttl: flags.ttl.value } : {}), - label: "cli-issued pairing url", - }); - yield* Console.log( - formatIssuedPairingCredential(issued, { json: flags.json, baseUrl }), - ); - }), - { quietLogs: flags.json }, + return yield* Effect.gen(function* () { + const environmentAuth = yield* EnvironmentAuth.EnvironmentAuth; + const issued = yield* environmentAuth.createPairingLink({ + scopes: AuthStandardClientScopes, + subject: "one-time-token", + ...(Option.isSome(flags.ttl) ? { ttl: flags.ttl.value } : {}), + label: "cli-issued pairing url", + }); + yield* Console.log(formatIssuedPairingCredential(issued, { json: flags.json, baseUrl })); + }).pipe( + Effect.provide( + Layer.mergeAll(EnvironmentAuth.runtimeLayer).pipe( + Layer.provide(ServerConfig.layer({ ...config, ...derivedPaths })), + Layer.provide( + Layer.succeed(References.MinimumLogLevel, flags.json ? "Error" : config.logLevel), + ), + ), + ), ); }), ), diff --git a/apps/server/src/config.ts b/apps/server/src/config.ts index 3b081c95c34..b82b8767f88 100644 --- a/apps/server/src/config.ts +++ b/apps/server/src/config.ts @@ -47,6 +47,13 @@ export interface ServerDerivedPaths { export interface DeriveServerPathsOptions { readonly baseDirIsExplicit?: boolean; + /** + * Use this state directory verbatim instead of deriving `dev` vs `userdata`. + * For tooling that has already located a running server's state directory and + * must target it exactly, rather than re-running the heuristic and risking a + * different answer. + */ + readonly stateDir?: string; } /** @@ -98,10 +105,9 @@ export const deriveServerPaths = Effect.fn(function* ( options: DeriveServerPathsOptions = {}, ): Effect.fn.Return { const { join } = yield* Path.Path; - const stateDir = join( - baseDir, - devUrl !== undefined && !options.baseDirIsExplicit ? "dev" : "userdata", - ); + const stateDir = + options.stateDir ?? + join(baseDir, devUrl !== undefined && !options.baseDirIsExplicit ? "dev" : "userdata"); const dbPath = join(stateDir, "state.sqlite"); const attachmentsDir = join(stateDir, "attachments"); const logsDir = join(stateDir, "logs"); diff --git a/apps/server/src/serverRuntimeState.test.ts b/apps/server/src/serverRuntimeState.test.ts index 749fd3062e9..8450ed78322 100644 --- a/apps/server/src/serverRuntimeState.test.ts +++ b/apps/server/src/serverRuntimeState.test.ts @@ -43,6 +43,35 @@ describe("serverRuntimeState", () => { }).pipe(Effect.provide(NodeServices.layer)), ); + // A server killed with SIGKILL never runs its release finalizer, so the file + // survives it. Trusting a leftover file points callers at a dead port. + describe("isPersistedServerRuntimeStateLive", () => { + const stateForPid = (pid: number): ServerRuntimeState.PersistedServerRuntimeState => ({ + version: 1, + pid, + port: 4_971, + origin: "http://127.0.0.1:4971", + startedAt: "2026-06-20T00:00:00.000Z", + }); + + it.effect("recognizes the current process as live", () => + Effect.gen(function* () { + assert.isTrue( + yield* ServerRuntimeState.isPersistedServerRuntimeStateLive(stateForPid(process.pid)), + ); + }), + ); + + it.effect("reports a pid that no longer exists as dead", () => + Effect.gen(function* () { + // Above the default pid_max, so it cannot be a live process. + assert.isFalse( + yield* ServerRuntimeState.isPersistedServerRuntimeStateLive(stateForPid(0x7ffffffe)), + ); + }), + ); + }); + it.effect("treats a missing runtime state file as absent", () => Effect.gen(function* () { const fileSystem = yield* FileSystem.FileSystem; diff --git a/apps/server/src/serverRuntimeState.ts b/apps/server/src/serverRuntimeState.ts index ccd6038ceca..b4d75bad7a8 100644 --- a/apps/server/src/serverRuntimeState.ts +++ b/apps/server/src/serverRuntimeState.ts @@ -107,6 +107,31 @@ export const clearPersistedServerRuntimeState = (path: string) => ); }); +/** + * Whether the process that wrote a runtime-state file is still alive. + * + * The file is removed by a release finalizer, so a server killed with SIGKILL — + * or one that crashed — leaves it behind. Treating a leftover file as "the + * server is up" points callers at a port nothing is listening on. Signal 0 + * performs the permission and existence checks without delivering a signal. + * + * PIDs are reused eventually, so a false positive is possible in principle; + * that is acceptable for local dev tooling, where the alternative is an + * HTTP probe that has to guess how long to wait for a starting server. + */ +export const isPersistedServerRuntimeStateLive = ( + state: PersistedServerRuntimeState, +): Effect.Effect => + Effect.sync(() => { + try { + process.kill(state.pid, 0); + return true; + } catch (cause) { + // EPERM means the process exists but belongs to another user. + return (cause as NodeJS.ErrnoException).code === "EPERM"; + } + }); + export const readPersistedServerRuntimeState = (path: string) => Effect.gen(function* () { const fs = yield* FileSystem.FileSystem; diff --git a/docs/reference/scripts.md b/docs/reference/scripts.md index 27845797df3..8409c442bad 100644 --- a/docs/reference/scripts.md +++ b/docs/reference/scripts.md @@ -2,7 +2,7 @@ - `vp run dev` — Starts contracts, server, and web in watch mode. - `vp run dev --share` (or `vp run dev:share`) — Same, plus publishes the web port on this machine's tailnet over HTTPS via `tailscale serve`. Prints the shareable URL, and the pairing URL is built against it so it can be opened from a phone or another laptop as-is. The mapping is removed on exit; if the tailnet is unavailable, the dev server still starts locally and logs a warning. -- `vp run dev:pair` — Prints a fresh pairing URL for the dev server already running against this data directory, resolving its port and web origin from `server-runtime.json`. Add `--base-dir ` only when the server was started with `--home-dir`. +- `vp run dev:pair` — Prints a fresh pairing URL for the dev server already running against this data directory, resolving its port and web origin from `server-runtime.json`. Searches both the `dev` and `userdata` state directories and requires the recorded process to still be alive, so a crashed server's leftover state file is not mistaken for a running one. Add `--base-dir ` only when the server was started with `--home-dir`. - `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. diff --git a/scripts/dev-runner.test.ts b/scripts/dev-runner.test.ts index 3f0767fabe7..197a1e45247 100644 --- a/scripts/dev-runner.test.ts +++ b/scripts/dev-runner.test.ts @@ -620,6 +620,34 @@ 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. + 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 2ac3dc883c7..705d0524890 100644 --- a/scripts/dev-runner.ts +++ b/scripts/dev-runner.ts @@ -581,6 +581,13 @@ 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") { @@ -630,10 +637,6 @@ export function runDevRunnerWithInput(input: DevRunnerCliInput) { } } - if (input.dryRun) { - return; - } - const spawnCommand = yield* resolveSpawnCommand( "vp", [...MODE_ARGS[input.mode], ...input.runArgs], From 0555604ded6eaf5582289578a0e88df89f5dd511 Mon Sep 17 00:00:00 2001 From: Theo Browne Date: Fri, 24 Jul 2026 17:27:13 -0700 Subject: [PATCH 3/5] Prefer the dev server when several are live under one base dir MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `findLiveServerRuntimeState` checked the configured state path first, which without --dev-url resolves to `userdata`. With both a userdata and a dev server running under the same base directory, `auth pairing url` would mint against the userdata server and print its origin — not the dev server this command exists to pair with. Rank candidates by the `devUrl` field instead, which only a server fronted by a web dev server records. Falls back to any live server when no dev server is running. Co-Authored-By: Claude Opus 5 (1M context) --- apps/server/src/cli/auth.test.ts | 138 +++++++++++++++++++++++++++++++ apps/server/src/cli/auth.ts | 24 ++++-- 2 files changed, 156 insertions(+), 6 deletions(-) create mode 100644 apps/server/src/cli/auth.test.ts diff --git a/apps/server/src/cli/auth.test.ts b/apps/server/src/cli/auth.test.ts new file mode 100644 index 00000000000..7466a6d5276 --- /dev/null +++ b/apps/server/src/cli/auth.test.ts @@ -0,0 +1,138 @@ +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 Option from "effect/Option"; +import * as Path from "effect/Path"; + +import { persistServerRuntimeState } from "../serverRuntimeState.ts"; +import { findLiveServerRuntimeState } from "./auth.ts"; + +/** A pid that cannot belong to a live process (above the usual pid_max). */ +const DEAD_PID = 0x7ffffffe; + +const writeRuntimeState = Effect.fn(function* (input: { + readonly baseDir: string; + readonly stateDir: "dev" | "userdata"; + readonly pid: number; + readonly port: number; + readonly devUrl?: string; +}) { + const path = yield* Path.Path; + yield* persistServerRuntimeState({ + path: path.join(input.baseDir, input.stateDir, "server-runtime.json"), + state: { + version: 1, + pid: input.pid, + port: input.port, + origin: `http://127.0.0.1:${String(input.port)}`, + ...(input.devUrl ? { devUrl: input.devUrl } : {}), + startedAt: "2026-07-20T00:00:00.000Z", + }, + }); +}); + +const makeBaseDir = Effect.fn(function* () { + const fileSystem = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const baseDir = yield* fileSystem.makeTempDirectoryScoped({ prefix: "t3-auth-cli-test-" }); + return { + baseDir, + // What the flag heuristic resolves to without --dev-url: the userdata dir. + serverRuntimeStatePath: path.join(baseDir, "userdata", "server-runtime.json"), + }; +}); + +describe("findLiveServerRuntimeState", () => { + it.effect("prefers the dev server when both state directories are live", () => + Effect.gen(function* () { + const path = yield* Path.Path; + const config = yield* makeBaseDir(); + // The configured path points at userdata, but this command is about dev. + yield* writeRuntimeState({ + baseDir: config.baseDir, + stateDir: "userdata", + pid: process.pid, + port: 16_601, + }); + yield* writeRuntimeState({ + baseDir: config.baseDir, + stateDir: "dev", + pid: process.pid, + port: 16_602, + devUrl: "http://localhost:8888/", + }); + + const found = yield* findLiveServerRuntimeState(config); + + const live = Option.getOrThrow(found); + assert.equal(live.stateDir, path.join(config.baseDir, "dev")); + assert.equal(live.state.devUrl, "http://localhost:8888/"); + }).pipe(Effect.provide(NodeServices.layer)), + ); + + it.effect("falls back to a live non-dev server when no dev server is running", () => + Effect.gen(function* () { + const path = yield* Path.Path; + const config = yield* makeBaseDir(); + yield* writeRuntimeState({ + baseDir: config.baseDir, + stateDir: "userdata", + pid: process.pid, + port: 16_601, + }); + + const found = yield* findLiveServerRuntimeState(config); + + const live = Option.getOrThrow(found); + assert.equal(live.stateDir, path.join(config.baseDir, "userdata")); + assert.equal(live.state.port, 16_601); + }).pipe(Effect.provide(NodeServices.layer)), + ); + + // A server killed with SIGKILL never clears its state file. + it.effect("ignores state files whose process is gone", () => + Effect.gen(function* () { + const config = yield* makeBaseDir(); + yield* writeRuntimeState({ + baseDir: config.baseDir, + stateDir: "dev", + pid: DEAD_PID, + port: 16_602, + devUrl: "http://localhost:8888/", + }); + + assert.isTrue(Option.isNone(yield* findLiveServerRuntimeState(config))); + }).pipe(Effect.provide(NodeServices.layer)), + ); + + it.effect("skips a stale dev server in favor of a live one elsewhere", () => + Effect.gen(function* () { + const path = yield* Path.Path; + const config = yield* makeBaseDir(); + yield* writeRuntimeState({ + baseDir: config.baseDir, + stateDir: "dev", + pid: DEAD_PID, + port: 16_602, + devUrl: "http://localhost:8888/", + }); + yield* writeRuntimeState({ + baseDir: config.baseDir, + stateDir: "userdata", + pid: process.pid, + port: 16_601, + }); + + const live = Option.getOrThrow(yield* findLiveServerRuntimeState(config)); + assert.equal(live.stateDir, path.join(config.baseDir, "userdata")); + }).pipe(Effect.provide(NodeServices.layer)), + ); + + it.effect("reports nothing when no state files exist", () => + Effect.gen(function* () { + const config = yield* makeBaseDir(); + assert.isTrue(Option.isNone(yield* findLiveServerRuntimeState(config))); + }).pipe(Effect.provide(NodeServices.layer)), + ); +}); diff --git a/apps/server/src/cli/auth.ts b/apps/server/src/cli/auth.ts index 85450df9aff..1096ce61e4e 100644 --- a/apps/server/src/cli/auth.ts +++ b/apps/server/src/cli/auth.ts @@ -139,18 +139,23 @@ const pairingCreateCommand = Command.make("create", { * tokens into a database the running server never reads when guessed wrong. * So check both, and let the live server decide which one is right. */ -const findLiveServerRuntimeState = Effect.fn("auth.findLiveServerRuntimeState")(function* ( - config: ServerConfig.ServerConfig["Service"], +export const findLiveServerRuntimeState = Effect.fn("auth.findLiveServerRuntimeState")(function* ( + config: Pick, ) { const path = yield* Path.Path; - const candidates = [ + const candidatePaths = [ config.serverRuntimeStatePath, ...(["dev", "userdata"] as const).map((stateDir) => path.join(config.baseDir, stateDir, "server-runtime.json"), ), ].filter((candidate, index, all) => all.indexOf(candidate) === index); - for (const statePath of candidates) { + const live: Array<{ + readonly stateDir: string; + readonly state: PersistedServerRuntimeState; + }> = []; + + for (const statePath of candidatePaths) { const state = yield* readPersistedServerRuntimeState(statePath); if (Option.isNone(state)) { continue; @@ -159,11 +164,18 @@ const findLiveServerRuntimeState = Effect.fn("auth.findLiveServerRuntimeState")( // nothing is listening on. Minting against it produces a token the live // server rejects with `invalid_credential`. if (yield* isPersistedServerRuntimeStateLive(state.value)) { - return Option.some({ stateDir: path.dirname(statePath), state: state.value }); + live.push({ stateDir: path.dirname(statePath), state: state.value }); } } - return Option.none<{ readonly stateDir: string; readonly state: PersistedServerRuntimeState }>(); + // `devUrl` is only recorded by a server fronted by a web dev server, so it + // distinguishes the dev instance from a production-style one when both are + // running under the same base directory. This command is about dev, so a dev + // server wins regardless of which state directory the flags happened to + // resolve to. + return Option.fromNullishOr( + live.find((candidate) => candidate.state.devUrl !== undefined) ?? live[0], + ); }); /** From 827469be4dec3ebda50cd3e21f1ffbaf916f65d1 Mon Sep 17 00:00:00 2001 From: Theo Browne Date: Fri, 24 Jul 2026 17:38:23 -0700 Subject: [PATCH 4/5] Address CodeRabbit review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - The skill told agents both to never put a pairing URL in a response and to hand the shared one to the user. State the rule and its single exception explicitly: give the URL to the person who asked for access, never put it anywhere durable. - A failed `tailscale serve` left the port unshared, because the stale- mapping clear had already run. The clear is still required (old mappings carry path routes that serving "/" would not replace), so say so in the error instead of letting an operator assume the previous mapping survived. - Worktree-derived ports were described as collision-free. They are a preferred offset, not a guarantee — hashes can collide and occupied ports shift both values. Qualified in AGENTS.md, the skill, and docs/reference/scripts.md. CodeRabbit also asked for tests around the pairing-url command; those landed in 0555604de, which it had not yet seen. Co-Authored-By: Claude Opus 5 (1M context) --- .agents/skills/test-t3-app/SKILL.md | 16 +++++-- AGENTS.md | 2 +- docs/reference/scripts.md | 6 ++- scripts/lib/dev-share.test.ts | 71 +++++++++++++++++++++++++++++ scripts/lib/dev-share.ts | 11 +++-- 5 files changed, 95 insertions(+), 11 deletions(-) create mode 100644 scripts/lib/dev-share.test.ts diff --git a/.agents/skills/test-t3-app/SKILL.md b/.agents/skills/test-t3-app/SKILL.md index 6db626a30bc..8c9ffa2296f 100644 --- a/.agents/skills/test-t3-app/SKILL.md +++ b/.agents/skills/test-t3-app/SKILL.md @@ -18,10 +18,12 @@ Use this skill for the web client. For iOS Simulator, Android Emulator, or physi (see "Share a dev server with the user" below). 4. Keep the terminal session alive and read the selected server port, web port, base directory, and pairing URL from its output. -Ports are derived from the worktree path, so each worktree gets its own stable -pair that survives restarts. Read them from the dev-runner line rather than -assuming `5733`/`13773`; `node scripts/dev-runner.ts dev --dry-run` prints them -without starting anything. +Ports are derived from the worktree path, so a worktree prefers the same pair on +every run rather than racing others for the default. That is a preference, not a +guarantee — the runner shifts both ports when either is already taken. Read the +actual values from the dev-runner line rather than assuming `5733`/`13773`, and +re-read them after a restart; `node scripts/dev-runner.ts dev --dry-run` prints +them without starting anything. 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. @@ -45,7 +47,9 @@ Treat the overall testing or implementation loop—not an assistant turn or one 4. Wait for the pairing exchange and redirect to finish before navigating elsewhere. 5. Continue in the same browser context so its stored bearer session remains available. -Treat pairing URLs as secrets. Do not copy them into final responses, screenshots, committed files, or durable logs. A pairing token is short-lived and single-use; opening the URL in another browser or opening it twice can consume it. +Treat pairing URLs as secrets: they grant access to the environment. Never put one anywhere it outlives the moment — screenshots, committed files, durable logs, or a PR description. + +The one exception is handing access to the user who asked for it: when they want to open the app themselves (see "Share a dev server with the user"), the URL is the deliverable, so give it to them directly in the response. Do not volunteer one otherwise. A pairing token is short-lived and single-use; opening the URL in another browser or opening it twice can consume it. ## Recover a consumed or expired pairing token @@ -83,6 +87,8 @@ This publishes the web port over HTTPS on the machine's tailnet and prints `[dev-runner] shared on tailnet: https://:/`. The pairing URL logged right after is already built against that origin, so give the user that URL verbatim — it is the deliverable, and it works unchanged on any of their devices. +This is the one case where a pairing URL belongs in a response: it is going to +the person who asked for it. It still must not go anywhere durable. Dev is single-origin: Vite proxies `/api`, `/ws`, `/oauth`, and `/.well-known` to the backend, so one shared port covers the whole app. Do not map backend paths by diff --git a/AGENTS.md b/AGENTS.md index a08ffc25ade..bf499f6a4a7 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -16,7 +16,7 @@ ## Dev Servers - Start the web stack with `bun run dev` (equivalently `vp run dev`). Never run `vp dev` from the repo root — that starts a bare Vite with no backend. -- Ports are derived from the worktree path, so each worktree has its own stable pair. Read them from the `[dev-runner] …` line; `--dry-run` prints them without starting anything. +- Ports are derived from the worktree path, so a worktree prefers the same pair every run instead of everyone racing for the default. They still shift when something already holds them, so read the actual values from the `[dev-runner] …` line rather than assuming them. `--dry-run` prints them without starting anything. - To let the user try a change on their own device, use `bun run dev:share`. It publishes the web port on the tailnet and logs a pairing URL already built against that origin — hand them that URL as-is. Details and troubleshooting live in the `test-t3-app` skill. - Dev is single-origin: Vite proxies `/api`, `/ws`, `/oauth`, and `/.well-known` to the backend. Do not set `VITE_HTTP_URL`/`VITE_WS_URL` for `dev`/`dev:web` — they compile absolute localhost URLs into the bundle and break every non-localhost origin. - Need a pairing URL for a server that is already running? `bun run dev:pair`. It finds the server itself; no ports or flags to get wrong. diff --git a/docs/reference/scripts.md b/docs/reference/scripts.md index 8409c442bad..67566592649 100644 --- a/docs/reference/scripts.md +++ b/docs/reference/scripts.md @@ -44,10 +44,12 @@ Ports resolve in this order, first match winning: 1. `T3CODE_PORT_OFFSET=` — exact numeric offset, full control. 2. `T3CODE_DEV_INSTANCE=` — numeric offset, or a hashed one for non-numeric values. Example: `T3CODE_DEV_INSTANCE=branch-a vp run dev:desktop` -3. **Git worktree** — the offset is hashed from the worktree path, so every worktree gets its own stable pair that survives restarts and doesn't collide with its siblings. +3. **Git worktree** — the offset is hashed from the worktree path, so a worktree gets the same preferred pair every time instead of everyone starting at the default and racing for it. 4. Otherwise the defaults: server `13773`, web `5733`. -Whatever the source, both ports are then checked on loopback and shifted together if either is taken. The resolved values are printed on the `[dev-runner] …` line; `--dry-run` prints them without starting anything. Read them from there rather than assuming the defaults. +Whatever the source, this only picks a _preferred_ offset. Both ports are then checked on loopback and shifted together if either is taken, so two worktrees whose hashes collide — or one whose ports something else already holds — still start, just not on the offset they asked for. Ports are stable across restarts in practice, not guaranteed. + +Which means: read the resolved values from the `[dev-runner] …` line rather than assuming them, and re-read them after a restart. `--dry-run` prints them without starting anything (it resolves only — it will not touch a `--share` mapping). ## Browser dev is single-origin diff --git a/scripts/lib/dev-share.test.ts b/scripts/lib/dev-share.test.ts new file mode 100644 index 00000000000..4388252ff4e --- /dev/null +++ b/scripts/lib/dev-share.test.ts @@ -0,0 +1,71 @@ +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 } from "./dev-share.ts"; + +const TAILNET_STATUS = JSON.stringify({ Self: { DNSName: "host.example.ts.net." } }); + +/** + * Answers `tailscale status --json` with a valid tailnet name, and lets each + * test decide how the `serve` call behaves. + */ +const spawnerLayer = (serve: { readonly exitCode: number; readonly stderr?: string }) => + Layer.succeed( + ChildProcessSpawner.ChildProcessSpawner, + ChildProcessSpawner.make((command) => { + const args = "args" in command ? (command.args as ReadonlyArray) : []; + const isStatus = args.includes("status"); + return Effect.succeed( + ChildProcessSpawner.makeHandle({ + pid: ChildProcessSpawner.ProcessId(1), + exitCode: Effect.succeed(ChildProcessSpawner.ExitCode(isStatus ? 0 : serve.exitCode)), + isRunning: Effect.succeed(false), + kill: () => Effect.void, + unref: Effect.succeed(Effect.void), + stdin: Sink.drain, + stdout: isStatus ? Stream.make(new TextEncoder().encode(TAILNET_STATUS)) : Stream.empty, + stderr: + !isStatus && serve.stderr + ? Stream.make(new TextEncoder().encode(serve.stderr)) + : Stream.empty, + all: Stream.empty, + getInputFd: () => Sink.drain, + getOutputFd: () => Stream.empty, + }), + ); + }), + ); + +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({ exitCode: 0 })), + ); + + 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({ 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"); + }), + ); +}); diff --git a/scripts/lib/dev-share.ts b/scripts/lib/dev-share.ts index e4ea6617c52..51e9bcad001 100644 --- a/scripts/lib/dev-share.ts +++ b/scripts/lib/dev-share.ts @@ -153,8 +153,9 @@ export const shareDevServer = Effect.fn("devShare.shareDevServer")(function* (in // 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). + // 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. yield* unshareDevServer(input.webPort); const serve = yield* runTailscale( @@ -163,9 +164,13 @@ export const shareDevServer = Effect.fn("devShare.shareDevServer")(function* (in ); if (serve.exitCode !== 0) { + // The clear above already happened, so say so: on a re-share this port is + // now serving nothing, and an operator who only saw "serve failed" would + // reasonably assume the previous mapping survived. + const cause = serve.stderr.trim() || `exit code ${String(serve.exitCode)}`; return yield* new DevShareError({ reason: "serve-failed", - detail: serve.stderr.trim() || `exit code ${String(serve.exitCode)} for port ${port}`, + detail: `${cause} (port ${port} is no longer served; any previous mapping for it was cleared before this attempt)`, }); } From 2682f8aafc52d6d1a06040d055ae42cb719e9887 Mon Sep 17 00:00:00 2001 From: Theo Browne Date: Fri, 24 Jul 2026 17:44:39 -0700 Subject: [PATCH 5/5] Verify the pre-clear actually cleared the mapping MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `unshareDevServer` applied Effect.ignore, so a failed removal was indistinguishable from a successful one. `shareDevServer` then served over routes it had not removed: stale path entries from the older `tsdev t3` layout (/ws, /api pointing at a separate backend port) would survive, producing a URL that loads while its API calls resolve to a dead port. The serve-failed message also asserted the port was cleared without knowing it. It now reports whether the port is clear, and sharing refuses when it is not. `tailscale serve … off` exits nonzero with "handler does not exist" when nothing was mapped — the normal first-share case — so that counts as cleared; anything else does not. The dev-runner finalizer warns, with the command to run, when cleanup does not take. Co-Authored-By: Claude Opus 5 (1M context) --- scripts/dev-runner.ts | 15 +++++- scripts/lib/dev-share.test.ts | 88 ++++++++++++++++++++++++++++++----- scripts/lib/dev-share.ts | 46 ++++++++++++++++-- 3 files changed, 132 insertions(+), 17 deletions(-) diff --git a/scripts/dev-runner.ts b/scripts/dev-runner.ts index 705d0524890..78f61e70270 100644 --- a/scripts/dev-runner.ts +++ b/scripts/dev-runner.ts @@ -602,7 +602,20 @@ export function runDevRunnerWithInput(input: DevRunnerCliInput) { // warn, and carry on serving locally. const shared = yield* Effect.acquireRelease( shareDevServer({ webPort: sharedWebPort }), - () => unshareDevServer(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( diff --git a/scripts/lib/dev-share.test.ts b/scripts/lib/dev-share.test.ts index 4388252ff4e..bdd3e5b636c 100644 --- a/scripts/lib/dev-share.test.ts +++ b/scripts/lib/dev-share.test.ts @@ -5,33 +5,43 @@ import * as Sink from "effect/Sink"; import * as Stream from "effect/Stream"; import { ChildProcessSpawner } from "effect/unstable/process"; -import { DevShareError, shareDevServer } from "./dev-share.ts"; +import { DevShareError, shareDevServer, unshareDevServer } from "./dev-share.ts"; const TAILNET_STATUS = JSON.stringify({ Self: { DNSName: "host.example.ts.net." } }); +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 decide how the `serve` call behaves. + * 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 = (serve: { readonly exitCode: number; readonly stderr?: string }) => +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 isStatus = args.includes("status"); + 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(isStatus ? 0 : serve.exitCode)), + exitCode: Effect.succeed(ChildProcessSpawner.ExitCode(result.exitCode)), isRunning: Effect.succeed(false), kill: () => Effect.void, unref: Effect.succeed(Effect.void), stdin: Sink.drain, - stdout: isStatus ? Stream.make(new TextEncoder().encode(TAILNET_STATUS)) : Stream.empty, - stderr: - !isStatus && serve.stderr - ? Stream.make(new TextEncoder().encode(serve.stderr)) - : Stream.empty, + 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, @@ -40,11 +50,50 @@ const spawnerLayer = (serve: { readonly exitCode: number; readonly stderr?: stri }), ); +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: "error: failed to remove web serve: handler does not exist", + }, + }), + ), + ); + 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.equal(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({ exitCode: 0 })), + Effect.provide(spawnerLayer({})), ); assert.equal(shared.host, "host.example.ts.net"); @@ -58,7 +107,7 @@ describe("shareDevServer", () => { 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({ exitCode: 1, stderr: "port already in use" })), + Effect.provide(spawnerLayer({ serve: { exitCode: 1, stderr: "port already in use" } })), Effect.flip, ); @@ -68,4 +117,19 @@ describe("shareDevServer", () => { 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 index 51e9bcad001..89f4b62a46c 100644 --- a/scripts/lib/dev-share.ts +++ b/scripts/lib/dev-share.ts @@ -127,17 +127,44 @@ export interface DevShareResult { } /** - * Removes a mapping created by {@link shareDevServer}. Best-effort. + * `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 with its own scope: this is called from a finalizer on * the way out of an interrupted program, and spawning the cleanup subprocess * under the dying scope would cancel it before `tailscale` ever ran — leaving * exactly the stale mapping it exists to remove. */ -export const unshareDevServer = (webPort: number) => +export const unshareDevServer = ( + webPort: number, +): Effect.Effect< + { readonly cleared: boolean; readonly detail?: string | undefined }, + never, + ChildProcessSpawner.ChildProcessSpawner +> => runTailscale(["serve", `--https=${String(webPort)}`, "off"], "serve-failed").pipe( + Effect.map((result) => { + if (result.exitCode === 0) { + return { cleared: true } as const; + } + const stderr = result.stderr.trim(); + // Nothing was mapped, so the port is clear either way. + if (NO_EXISTING_HANDLER_PATTERN.test(stderr)) { + return { cleared: true } as const; + } + return { cleared: false, detail: stderr || `exit code ${String(result.exitCode)}` } as const; + }), + // A spawn failure (no tailscale on PATH) also means we cannot vouch for the + // port being clear. + Effect.catch((error: DevShareError) => + Effect.succeed({ cleared: false, detail: error.message } as const), + ), Effect.scoped, - Effect.ignore, Effect.uninterruptible, ); @@ -156,7 +183,18 @@ export const shareDevServer = Effect.fn("devShare.shareDevServer")(function* (in // 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. - yield* unshareDevServer(input.webPort); + 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 ${port}${ + cleared.detail ? `: ${cleared.detail}` : "" + }. Run \`tailscale serve --https=${port} off\` and retry.`, + }); + } const serve = yield* runTailscale( ["serve", "--bg", `--https=${port}`, `http://127.0.0.1:${port}`],