From d352d83084605397746d45bb3b003b41bfc04f49 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ko=CC=88nig?= Date: Tue, 22 Sep 2026 11:39:08 +0200 Subject: [PATCH] feat(dev): address each component the way its reader can reach it (AP-3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `lt dev` is URL-first: a project lives at `https://.localhost` and Caddy proxies to an opaque internal port. That promise holds for **browsers**. It does not hold for Node — on Windows `*.localhost` subdomains do not resolve at all (`dns.lookup` → ENOTFOUND, `curl` agrees), while Chromium resolves them internally without asking a resolver. So the question at each call site is not "which URL" but **"who reads it"**, and there are four answers, not two: | | example | address | |---|---|---| | a browser resolves it | Playwright `baseURL`, a printed link | the public name | | Node resolves it | readiness probes, the SSR/proxy target | **loopback** | | another binary resolves it | `cloudflared` | loopback (see "not changed") | | nobody resolves it — it is COMPARED | `APP_URL`, the Caddy vhost matcher, the tunnel `Host:` header | the public name, **untouched** | That last row is why "rewrite every internal URL to 127.0.0.1" would be wrong. `APP_URL` is the CORS allow-list and Better-Auth's `trustedOrigins` — matched as a string against the `Origin` header a browser sends, and the browser arrives from `https://.localhost`. Rewriting it would break **every login**, on a platform where only a health probe was broken before. A test pins that. ## What changed - **`internalUrl(port)`** (`dev-env.ts`) — loopback on EVERY platform, not behind an `isWindows()` branch. 127.0.0.1 works everywhere, so the path Windows depends on is the one macOS exercises daily. A branch only the other platform runs is an unchecked branch. - **`NUXT_API_URL` → loopback**, `NUXT_PUBLIC_API_URL` stays the public name. The two already existed side by side and carried the same value; nuxt-extensions' `buildLtApiUrl()` already prefers the server-only one during SSR, so no change is needed there. - **The `lt dev test` readiness probes** (`dev-test-session.ts`) probe loopback. This is the one that blocked the command outright: Node asking a name it cannot resolve timed out against a stack that was up, and bring-up aborted before Playwright started. Probing the port also measures the component rather than Caddy — the better question for "is it alive". The failure MESSAGES keep the public name, because that is what a developer opens. - **`LT_DEV_API_INTERNAL_URL` / `LT_DEV_APP_INTERNAL_URL`** are published and written to the `.lt-dev/.env` bridge, so an external runner's Node-side helper has an address it can reach. - **The consumer CLAUDE.md URL block names both**, with who each is for. It tells an agent never to assume `localhost:3000/3001`, so naming only the `*.localhost` host left an agent with a name that does not resolve for Node. This moved the patch after port allocation — it ran before, where only the name existed, which is why the ports were never passed. ## Not changed, deliberately `tunnel.ts`'s `upstreamUrl` is dialled by `cloudflared`, so it has the same problem. But pointing it at a component's port would **bypass Caddy**, which is the whole point of the tunnel. The right value is `https://127.0.0.1` with the `Host:` header unchanged (`--no-tls-verify` is already passed). I cannot verify that here — it needs cloudflared and a live tunnel — so it is recorded in the inventory rather than changed on a hunch. Two residuals in the same class, both recorded: Playwright's `request` fixture resolves `baseURL` from Node, and nuxt-extensions' `validateSession()` has no SSR guard and fetches the public URL. Both are project-dependent, neither is triggered by the framework itself. Tests: 77 suites / 1198 tests. Mutations, each turning tests red: `NUXT_API_URL` back to the public name; `APP_URL` rewritten to loopback (the mistake this change argues against); the readiness probe back to the name. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01N8cvaEziSrKGHv3Jcp59JH --- __tests__/dev-env.test.ts | 33 +++++++++++++++++- __tests__/dev-test-session.test.ts | 17 ++++++++-- src/commands/dev/up.ts | 30 +++++++++++------ src/lib/dev-env-bridge.ts | 7 ++++ src/lib/dev-env.ts | 54 +++++++++++++++++++++++++++++- src/lib/dev-patches.ts | 25 +++++++++++--- src/lib/dev-test-session.ts | 16 +++++++-- 7 files changed, 160 insertions(+), 22 deletions(-) diff --git a/__tests__/dev-env.test.ts b/__tests__/dev-env.test.ts index 3f43e14..8c639ff 100644 --- a/__tests__/dev-env.test.ts +++ b/__tests__/dev-env.test.ts @@ -47,12 +47,43 @@ describe('dev-env / buildDevEnv', () => { expect(env.api.env.DATABASE_URL).toContain('crm'); expect(env.app.env.PORT).toBe('4011'); - expect(env.app.env.NUXT_API_URL).toBe('https://api.crm.localhost'); expect(env.app.env.NUXT_PUBLIC_API_URL).toBe('https://api.crm.localhost'); expect(env.app.env.NUXT_PUBLIC_SITE_URL).toBe('https://crm.localhost'); expect(env.app.env.NUXT_PUBLIC_STORAGE_PREFIX).toBe('crm'); }); + test('server-side and browser-side API addresses are NOT the same value', () => { + // The split AP-3 exists for. `NUXT_API_URL` is read by Node (the Vite/Nitro + // proxy target, and what nuxt-extensions' `buildLtApiUrl()` prefers during + // SSR); `NUXT_PUBLIC_API_URL` lands in `runtimeConfig.public` and is fetched + // by the BROWSER. They carried the same `*.localhost` value until now, which + // is fine until Node has to resolve it — on Windows it cannot. + const env = buildDevEnv({ apiInternalPort: 4010, appInternalPort: 4011, dbName: 'crm', identity: fullIdentity }); + expect(env.app.env.NUXT_API_URL).toBe('http://127.0.0.1:4010'); + expect(env.app.env.NUXT_PUBLIC_API_URL).toBe('https://api.crm.localhost'); + expect(env.app.env.NUXT_API_URL).not.toBe(env.app.env.NUXT_PUBLIC_API_URL); + // The loopback pair is also published under its own names, for anything + // Node-side that reads the `.lt-dev/.env` bridge. + expect(env.app.env.LT_DEV_API_INTERNAL_URL).toBe('http://127.0.0.1:4010'); + expect(env.app.env.LT_DEV_APP_INTERNAL_URL).toBe('http://127.0.0.1:4011'); + }); + + test('the values that are COMPARED rather than resolved keep the public name', () => { + // `APP_URL` / `NSC__APP_URL` are the CORS allow-list and Better-Auth's + // `trustedOrigins` — matched as a string against the `Origin` header a + // browser sends, and the browser arrives from `https://crm.localhost`. + // Rewriting them to loopback would break every login, on a platform where + // only a health probe was broken before. Same for `BASE_URL`, which is + // declared (OpenAPI `servers[]`, e-mail links), not fetched. + const env = buildDevEnv({ apiInternalPort: 4010, appInternalPort: 4011, dbName: 'crm', identity: fullIdentity }); + for (const key of ['APP_URL', 'NSC__APP_URL'] as const) { + expect([key, env.api.env[key]]).toEqual([key, 'https://crm.localhost']); + } + for (const key of ['BASE_URL', 'NSC__BASE_URL'] as const) { + expect([key, env.api.env[key]]).toEqual([key, 'https://api.crm.localhost']); + } + }); + test('gives the App a session password so logins work on the built server', () => { // Regression: `lt dev test` serves the *built* Nitro server, which reads only `process.env`. // Without a password every login answered 500 ("H3Error: Empty password") and roughly half diff --git a/__tests__/dev-test-session.test.ts b/__tests__/dev-test-session.test.ts index 65d3fc4..d5d65df 100644 --- a/__tests__/dev-test-session.test.ts +++ b/__tests__/dev-test-session.test.ts @@ -393,8 +393,21 @@ describe('dev-test-session', () => { // The app wait used to pass `undefined` and take waitForHttp's default, // which accepts ANY status — Caddy's 502 included. That made the app half // of the check meaningless, and now that it throws it has to be right. - expect(source).toMatch(/waitForHttp\(\s*appUrl,\s*90_000,\s*isStackServing/); - expect(source).toMatch(/waitForHttp\(\s*`\$\{apiUrl\}\/meta`,\s*120_000,\s*isStackServing/); + expect(source).toMatch(/waitForHttp\(\s*appProbeUrl,\s*90_000,\s*isStackServing/); + expect(source).toMatch(/waitForHttp\(\s*`\$\{apiProbeUrl\}\/meta`,\s*120_000,\s*isStackServing/); + }); + + test('both waits probe the LOOPBACK address, not the *.localhost name', () => { + // Node is what asks here, and on Windows `*.localhost` does not resolve for + // Node at all — the waits would time out against a stack that is up, and + // bring-up aborts before Playwright ever starts. Probing the port also + // measures the component rather than Caddy, which is the better question + // for "is it alive". + expect(source).toMatch(/const\s+appProbeUrl\s*=\s*internalUrl\(appPort\)/); + expect(source).toMatch(/const\s+apiProbeUrl\s*=\s*internalUrl\(apiPort\)/); + // …while the human-facing messages keep the name the developer opens. + expect(source).toMatch(/unreachableStackError\('App',\s*appUrl/); + expect(source).toMatch(/unreachableStackError\('API',\s*`\$\{apiUrl\}\/meta`/); }); }); diff --git a/src/commands/dev/up.ts b/src/commands/dev/up.ts index fd89bb8..f46b253 100644 --- a/src/commands/dev/up.ts +++ b/src/commands/dev/up.ts @@ -164,17 +164,6 @@ const UpCommand: GluegunCommand = { // plugin hook surfaces the ticket context per prompt instead (from the // gitignored `.lt-dev/ticket` marker). For the base project we keep the // committed URL block up to date as before. - if (!ticket) { - const claudeCandidates = [ - join(layout.root, 'CLAUDE.md'), - ...(layout.apiDir ? [join(layout.apiDir, 'CLAUDE.md')] : []), - ...(layout.appDir ? [join(layout.appDir, 'CLAUDE.md')] : []), - ]; - const patched = claudeCandidates.map((f) => patchClaudeMd(f, { dbName, identity })).filter((r) => r.patched); - if (patched.length > 0) { - info(colors.dim(`updated CLAUDE.md URL block in ${patched.length} file(s)`)); - } - } // Always keep `.lt-dev/` (state, env bridge, ticket marker) out of git. if (addToGitignore(layout.root, '.lt-dev/')) { info(colors.dim('added `.lt-dev/` to .gitignore')); @@ -287,6 +276,25 @@ const UpCommand: GluegunCommand = { return 'dev up: port in use'; } + // The CLAUDE.md URL block is written HERE, after the internal ports are + // resolved, so it can name both addresses: the `*.localhost` name a browser + // opens and the loopback address a script must use. It used to run before + // allocation, where only the name existed — which is why an agent following + // the block had nothing but a hostname that does not resolve for Node on + // Windows. NEVER for a ticket worktree: that CLAUDE.md is git-tracked and + // would carry per-ticket URLs into a commit. + if (!ticket) { + const claudeCandidates = [ + join(layout.root, 'CLAUDE.md'), + ...(layout.apiDir ? [join(layout.apiDir, 'CLAUDE.md')] : []), + ...(layout.appDir ? [join(layout.appDir, 'CLAUDE.md')] : []), + ]; + const patched = claudeCandidates.map((f) => patchClaudeMd(f, { apiPort, appPort, dbName, identity })).filter((r) => r.patched); + if (patched.length > 0) { + info(colors.dim(`updated CLAUDE.md URL block in ${patched.length} file(s)`)); + } + } + // ── Health-aware (re)start decision ────────────────────────────────────── // Probe the just-resolved ports so we can tell a still-serving component // from a crashed one (supervisor PID alive, port free). Only dead/crashed diff --git a/src/lib/dev-env-bridge.ts b/src/lib/dev-env-bridge.ts index acb38c3..5540c8e 100644 --- a/src/lib/dev-env-bridge.ts +++ b/src/lib/dev-env-bridge.ts @@ -96,6 +96,13 @@ export function writeEnvBridge(projectRoot: string, devEnv: DevEnv, dbName?: str // Legacy aliases — see dev-env.ts for the rationale. 'API_URL', 'SITE_URL', + // The loopback addresses. An external runner that RESOLVES a URL from Node — + // an API helper, a seeding script, a readiness check — needs these: on Windows + // `*.localhost` does not resolve for Node at all, only for browsers. The + // public names above stay, because browser-facing values (a Playwright + // `baseURL`, a link a developer opens) must keep them. See `dev-env.ts#internalUrl`. + 'LT_DEV_API_INTERNAL_URL', + 'LT_DEV_APP_INTERNAL_URL', ]; for (const key of exported) { const v = devEnv.app.env[key as string]; diff --git a/src/lib/dev-env.ts b/src/lib/dev-env.ts index 47e7974..490c46a 100644 --- a/src/lib/dev-env.ts +++ b/src/lib/dev-env.ts @@ -67,6 +67,9 @@ export function buildDevEnv(input: BuildDevEnvInput): DevEnv { const apiUrl = apiSub ? `https://${apiSub.hostname}` : ''; const appUrl = appSub ? `https://${appSub.hostname}` : ''; + // Same two services, addressed the way Node can reach them. See `internalUrl`. + const apiInternal = internalUrl(apiInternalPort); + const appInternal = internalUrl(appInternalPort); const caPath = detectCaddyRootCa(); const sharedKeys: NodeJS.ProcessEnv = { @@ -75,6 +78,11 @@ export function buildDevEnv(input: BuildDevEnvInput): DevEnv { // user-cache) so E2E suites run without a separate VITEST/PLAYWRIGHT flag. // (Also written to the .lt-dev/.env bridge for external test runners.) LT_DEV_ACTIVE: 'true', + // The loopback addresses, named explicitly so anything Node-side (an external + // test runner reading the `.lt-dev/.env` bridge, a project's own helper) can + // reach the components without going through a hostname it may not resolve. + ...(apiInternal ? { LT_DEV_API_INTERNAL_URL: apiInternal } : {}), + ...(appInternal ? { LT_DEV_APP_INTERNAL_URL: appInternal } : {}), ...(apiUrl ? { BASE_URL: apiUrl, NSC__BASE_URL: apiUrl } : {}), ...(appUrl ? { APP_URL: appUrl, NSC__APP_URL: appUrl } : {}), ...(dbName ? { DATABASE_URL: buildPostgresUrl(dbName), NSC__MONGOOSE__URI: `mongodb://127.0.0.1/${dbName}` } : {}), @@ -113,7 +121,14 @@ export function buildDevEnv(input: BuildDevEnvInput): DevEnv { // those projects "just work" under `lt dev up` without code // changes. The `NUXT_*` variants below win at runtime where // both are read. - ...(apiUrl ? { API_URL: apiUrl, NUXT_API_URL: apiUrl, NUXT_PUBLIC_API_URL: apiUrl } : {}), + // `NUXT_API_URL` is the SERVER-side address (the Vite/Nitro proxy target, + // and what `buildLtApiUrl()` prefers during SSR); `NUXT_PUBLIC_API_URL` + // lands in `runtimeConfig.public` and is fetched by the BROWSER. The + // module already distinguishes the two — until now they carried the same + // value, so the distinction bought nothing. `API_URL` is the legacy alias + // projects read into `runtimeConfig.public`, so it stays public. + ...(apiUrl ? { API_URL: apiUrl, NUXT_PUBLIC_API_URL: apiUrl } : {}), + ...(apiInternal || apiUrl ? { NUXT_API_URL: apiInternal || apiUrl } : {}), ...(appUrl ? { NUXT_PUBLIC_SITE_URL: appUrl, SITE_URL: appUrl } : {}), // Vite-API-Proxy is OFF by default in lt dev mode — Caddy serves // both subdomains under HTTPS with shared cookie domain, so @@ -163,6 +178,43 @@ export function buildDevEnv(input: BuildDevEnvInput): DevEnv { }; } +/** + * The loopback address of a component, for anything that RESOLVES a name. + * + * `lt dev` is URL-first: a project is reached at `https://.localhost`, and + * Caddy proxies that to an opaque internal port. That promise holds for + * **browsers**. It does not hold for Node: on Windows `*.localhost` subdomains do + * not resolve at all (`dns.lookup('api.demo.localhost')` → ENOTFOUND, and `curl` + * agrees), while Chromium resolves them internally without asking a resolver. + * + * So the question at every call site is not "which URL" but **"who reads it"**: + * + * | | example | address to use | + * |---|---|---| + * | a browser resolves it | Playwright `baseURL`, a printed link | the public name | + * | Node resolves it | readiness probes, SSR fetches, a proxy target | **this** | + * | another binary resolves it | `cloudflared` | **this** | + * | nobody resolves it — it is COMPARED | `APP_URL` (CORS / `trustedOrigins`), the Caddy vhost matcher, the tunnel `Host:` header | the public name, untouched | + * + * That last row is why "rewrite every internal URL to 127.0.0.1" would be wrong: + * `APP_URL` is matched as a string against the `Origin` header a browser sends, + * and the browser arrives from `https://.localhost`. Rewriting it would + * break every login — on the platform where only a health probe was broken before. + * + * **Loopback on every platform, not only on Windows.** 127.0.0.1 works everywhere, + * so there is no branch to get wrong, and the path Windows depends on is the one + * macOS exercises daily. A branch only the other platform runs is an unchecked + * branch. + * + * One consequence to keep in mind: this goes PAST Caddy. For "is the component + * alive?" that is an advantage — it measures the component, not the proxy. For + * "does the routing work?" it is the wrong question, and that one belongs to + * `lt dev doctor` over the public name. + */ +export function internalUrl(port: number | undefined): string { + return port ? `http://127.0.0.1:${port}` : ''; +} + /** Postgres convenience URL — used by Postgres-based projects (e.g. nest-base). */ function buildPostgresUrl(dbName: string): string { return `postgresql://${dbName}:${dbName}@localhost:5432/${dbName}`; diff --git a/src/lib/dev-patches.ts b/src/lib/dev-patches.ts index e67383e..9ca24e3 100644 --- a/src/lib/dev-patches.ts +++ b/src/lib/dev-patches.ts @@ -128,8 +128,11 @@ export function patchApiConfig(file: string): PatchResult { * URLs is a no-op; re-running with different URLs replaces the block * in place. */ -export function patchClaudeMd(file: string, options: { dbName?: string; identity: DevIdentity }): PatchResult { - const { dbName, identity } = options; +export function patchClaudeMd( + file: string, + options: { apiPort?: number; appPort?: number; dbName?: string; identity: DevIdentity }, +): PatchResult { + const { apiPort, appPort, dbName, identity } = options; const startMarker = ''; const endMarker = ''; @@ -155,8 +158,22 @@ export function patchClaudeMd(file: string, options: { dbName?: string; identity '**Active URLs for THIS project:**', '', ]; - if (appSub) lines.push(`- App: \`https://${appSub.hostname}\``); - if (apiSub) lines.push(`- API: \`https://${apiSub.hostname}\``); + // Both addresses, with who each one is for. The block tells an agent never to + // assume `localhost:3000/3001` — so if it only named the `*.localhost` host, an + // agent following it would `curl` a name that does not resolve for Node on + // Windows, and fail silently. The browser address stays first: it is the one a + // human opens. + if (appSub) { + lines.push(`- App: \`https://${appSub.hostname}\`${appPort ? ` — from a script: \`http://127.0.0.1:${appPort}\`` : ''}`); + } + if (apiSub) { + lines.push(`- API: \`https://${apiSub.hostname}\`${apiPort ? ` — from a script: \`http://127.0.0.1:${apiPort}\`` : ''}`); + } + if (appPort || apiPort) { + lines.push( + '- The `*.localhost` names are resolved by BROWSERS. Node, `curl` and other tools may not resolve them (they do not on Windows) — use the loopback address from a script.', + ); + } if (dbName) lines.push(`- DB: \`mongodb://127.0.0.1/${dbName}\``); lines.push(''); lines.push( diff --git a/src/lib/dev-test-session.ts b/src/lib/dev-test-session.ts index 2b30291..001736e 100644 --- a/src/lib/dev-test-session.ts +++ b/src/lib/dev-test-session.ts @@ -35,7 +35,7 @@ import { join } from 'path'; import { reloadCaddy, removeProjectBlock, upsertProjectBlock } from './caddy'; import { applyPendingMigrations, findCompiledEntry, resolveApiRuntime } from './dev-api-launch'; -import { buildDevEnv } from './dev-env'; +import { buildDevEnv, internalUrl } from './dev-env'; import { clearEnvBridge, writeEnvBridge } from './dev-env-bridge'; import { buildTestIdentity, DevIdentity } from './dev-identity'; import { type PackageManagerCommand, pickPackageManager } from './dev-package-manager'; @@ -480,10 +480,20 @@ export async function bringUpTestSession( // the wait, so this only ever makes the abort arrive sooner. const gone = (pid?: number) => pid === undefined || !isPidAlive(pid); + // The probes go to the LOOPBACK address, not the public name: this is Node + // asking, and on Windows `*.localhost` does not resolve for Node at all — the + // waits would time out against a stack that is up, and `bringUpTestSession` + // aborts before Playwright ever starts. Probing the port directly also measures + // the component rather than the proxy, which is the better question here. + // The public name stays in the MESSAGES, because that is what the developer + // opens; the loopback pair is printed next to it above. + const appProbeUrl = internalUrl(appPort) || appUrl; + const apiProbeUrl = internalUrl(apiPort) || apiUrl; + // Wait for the test App to answer. if (appUrl) { log.info(log.dim(`Waiting for ${appUrl} …`)); - const appReady = await waitForHttp(appUrl, 90_000, isStackServing, () => gone(pids.app)); + const appReady = await waitForHttp(appProbeUrl, 90_000, isStackServing, () => gone(pids.app)); if (!appReady) throw unreachableStackError('App', appUrl, appLogPath); } // Wait for the test API to actually SERVE before handing off to Playwright. @@ -492,7 +502,7 @@ export async function bringUpTestSession( // `ensureApiReachableOrSkip` guard (the API-readiness race). if (apiUrl) { log.info(log.dim(`Waiting for ${apiUrl}/meta …`)); - const apiReady = await waitForHttp(`${apiUrl}/meta`, 120_000, isStackServing, () => gone(pids.api)); + const apiReady = await waitForHttp(`${apiProbeUrl}/meta`, 120_000, isStackServing, () => gone(pids.api)); if (!apiReady) throw unreachableStackError('API', `${apiUrl}/meta`, apiLogPath); }