fix(preflight): bound DNS lookup by timeout and shutdown

DOCUMENTATION.md

@@ -788,7 +788,7 @@ Batch `--report` flags apply only to `test run --all --wait` (and batch `test re
 
 `--target-url` and `--local` can supply an ad-hoc origin while `--env` supplies its sign-in configuration and run attribution. When no saved origin matches and no `--env` is selected, a supporting backend clones the project default's sign-in configuration into a temporary environment. Explicit `--local` can use a public environment's sign-in configuration on a new backend; an older backend's refusal is rendered verbatim and the tunnel is closed. Unknown environment names remain validation errors with the available names. `--env` is also accepted by `--all` and `test rerun`.
 
-**Reachability preflight (refuse before charge).** For a public `--target-url`, the CLI probes **before dispatching** (and before anything is billed): a DNS resolve plus a lightweight HTTP request. Loopback tunnel targets use a TCP probe of the stored host and port instead. A confirmed-dead target — DNS `NXDOMAIN`, connection refused, or a `502`/`503`/`504` gateway error (the signature of a tunnel that has gone away) — is refused with a validation error (exit 5) instead of dispatching a run that can only fail against a URL nobody is serving. A resolved address that lands in private/loopback/link-local space is always refused (the hostname passed the literal check but actually points somewhere unreachable from the runner). Ambiguous signals — a timeout, a TLS error, an odd status — only produce a stderr warning and never block; behind a configured HTTP(S) proxy, a local DNS failure is also downgraded to a warning, since resolution really happens at the proxy. `--skip-preflight` (on `test run`, `test create`, and `test create-batch`) skips these probes; metadata reads needed to resolve an environment still occur. Note: for a backend test the probe is a heuristic (the test's own base URL is baked into its code) — reach for `--skip-preflight` if a refusal surprises you there.
+**Reachability preflight (refuse before charge).** For a public `--target-url`, the CLI probes **before dispatching** (and before anything is billed): a DNS resolve plus a lightweight HTTP request, sharing an 8-second budget. A stalled DNS lookup produces an advisory when that budget expires; interruption stops waiting immediately without dispatching a run. Loopback tunnel targets use a TCP probe of the stored host and port instead. A confirmed-dead target — DNS `NXDOMAIN`, connection refused, or a `502`/`503`/`504` gateway error (the signature of a tunnel that has gone away) — is refused with a validation error (exit 5) instead of dispatching a run that can only fail against a URL nobody is serving. A resolved address that lands in private/loopback/link-local space is always refused (the hostname passed the literal check but actually points somewhere unreachable from the runner). Ambiguous signals — a timeout, a TLS error, an odd status — only produce a stderr warning and never block; behind a configured HTTP(S) proxy, a local DNS failure is also downgraded to a warning, since resolution really happens at the proxy. `--skip-preflight` (on `test run`, `test create`, and `test create-batch`) skips these probes; metadata reads needed to resolve an environment still occur. Note: for a backend test the probe is a heuristic (the test's own base URL is baked into its code) — reach for `--skip-preflight` if a refusal surprises you there.
 
 The `[advisory]` about `--target-url` on V3-routed accounts is now **response-driven**: the CLI reads the run's actual trigger response rather than guessing from account flags, so it fires only when the override genuinely did not take effect (newer backends apply `--target-url` to fresh frontend runs on V3; older ones ignore it and the advisory says so). The CLI auto-mints an idempotency key (printed to stderr under `--output json`, `--verbose`, or `--debug`); pass `--idempotency-key <uuid>` to control it explicitly.
 

src/commands/test.run.spec.ts

@@ -5,6 +5,7 @@
  * sleep injection is wired through `TestDeps.sleep` to avoid real delays.
  */
 
+import * as dns from 'node:dns';
 import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs';
 import { tmpdir } from 'node:os';
 import { join } from 'node:path';
@@ -7103,6 +7104,82 @@ describe('runTestRun — --target-url reachability preflight wiring', () => {
   // DNS lookups, no network flakiness.
   const LITERAL_IP_TARGET = 'http://203.0.113.10';
 
+  it('interrupting stalled DNS rejects before any probe HTTP request or trigger POST', async () => {
+    const { credentialsPath } = makeCreds();
+    const shutdown = new ShutdownController();
+    const fetchImpl = vi.fn();
+    const stderr = vi.fn();
+    const rejected = vi.fn();
+    vi.useFakeTimers();
+    const lookup = vi.spyOn(dns.promises, 'lookup').mockImplementation(() => new Promise(() => {}));
+    try {
+      const pending = runTestRun(
+        {
+          profile: 'default',
+          output: 'json',
+          debug: false,
+          dryRun: false,
+          testId: 'test_xyz',
+          wait: false,
+          timeoutSeconds: 60,
+          targetUrl: 'https://preview.example.test',
+        },
+        { credentialsPath, fetchImpl, shutdown, stdout: () => {}, stderr },
+      ).catch(rejected);
+      await vi.advanceTimersByTimeAsync(0);
+      expect(lookup).toHaveBeenCalledWith('preview.example.test', { all: true });
+      shutdown.interrupt('SIGINT');
+      await vi.advanceTimersByTimeAsync(0);
+      expect(rejected).toHaveBeenCalledWith(shutdown.signal.reason);
+      await pending;
+      expect(fetchImpl).not.toHaveBeenCalled();
+      expect(stderr.mock.calls.some(([line]) => line.includes('[advisory]'))).toBe(false);
+      expect(shutdown.hasCriticalOperations).toBe(false);
+      expect(vi.getTimerCount()).toBe(0);
+    } finally {
+      lookup.mockRestore();
+      vi.useRealTimers();
+    }
+  });
+
+  it('a stalled DNS timeout advises and still dispatches the run', async () => {
+    const { credentialsPath } = makeCreds();
+    const targetUrl = 'https://preview.example.test';
+    const fetchImpl = vi.fn(makeFetch(() => ({ body: { ...TRIGGER_RESP, targetUrl } })));
+    const stderr = vi.fn();
+    const settled = vi.fn();
+    vi.useFakeTimers();
+    const lookup = vi.spyOn(dns.promises, 'lookup').mockImplementation(() => new Promise(() => {}));
+    try {
+      const pending = runTestRun(
+        {
+          profile: 'default',
+          output: 'json',
+          debug: false,
+          testId: 'test_xyz',
+          wait: false,
+          timeoutSeconds: 60,
+          targetUrl,
+        },
+        { credentialsPath, fetchImpl, stdout: () => {}, stderr },
+      ).then(settled);
+      await vi.advanceTimersByTimeAsync(7_999);
+      expect(fetchImpl).not.toHaveBeenCalled();
+      await vi.advanceTimersByTimeAsync(1);
+      expect(settled).toHaveBeenCalledWith(expect.objectContaining({ runId: 'run_abc' }));
+      await pending;
+      expect(fetchImpl).toHaveBeenCalledOnce();
+      expect(fetchImpl).toHaveBeenCalledWith(
+        expect.stringContaining('/tests/test_xyz/runs'),
+        expect.objectContaining({ method: 'POST' }),
+      );
+      expect(stderr).toHaveBeenCalledWith(expect.stringMatching(/\[advisory\].*DNS.*8s/));
+    } finally {
+      lookup.mockRestore();
+      vi.useRealTimers();
+    }
+  });
+
   it('a gateway-error response (502) refuses before the trigger POST — exit 5, no run row', async () => {
     const { credentialsPath } = makeCreds();
     const postedUrls: string[] = [];

src/lib/target-url-preflight-lifetime.test.ts

@@ -0,0 +1,237 @@
+import { getEventListeners } from 'node:events';
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
+import { InterruptError } from './errors.js';
+import { ShutdownController } from './interrupt.js';
+import { probeTargetUrl } from './target-url-preflight.js';
+
+/** Lets tests settle DNS after a chosen deadline or interruption. */
+function deferred<T>() {
+  let resolve!: (value: T) => void;
+  let reject!: (reason: unknown) => void;
+  const promise = new Promise<T>((res, rej) => {
+    resolve = res;
+    reject = rej;
+  });
+  return { promise, resolve, reject };
+}
+
+describe('target URL preflight lifetime', () => {
+  beforeEach(() => vi.useFakeTimers());
+  afterEach(() => {
+    vi.useRealTimers();
+    vi.restoreAllMocks();
+  });
+
+  it.each([false, true])(
+    'bounds a stalled DNS lookup to 8s (proxy active: %s)',
+    async proxyActive => {
+      const shutdown = new ShutdownController();
+      const dnsLookup = vi.fn(() => new Promise<unknown>(() => {}));
+      const fetchImpl = vi.fn();
+      const settled = vi.fn();
+      const pending = probeTargetUrl('https://example.test', {
+        dnsLookup,
+        fetchImpl,
+        proxyActive,
+        shutdown,
+        shutdownSignal: shutdown.signal,
+      }).then(settled);
+
+      await vi.advanceTimersByTimeAsync(7_999);
+      expect(dnsLookup).toHaveBeenCalledOnce();
+      expect(settled).not.toHaveBeenCalled();
+      const trackedDuringDns = shutdown.hasCriticalOperations;
+
+      await vi.advanceTimersByTimeAsync(1);
+      expect(settled).toHaveBeenCalledWith({
+        verdict: 'warn',
+        reason: expect.stringMatching(/DNS.*8s/),
+      });
+      await pending;
+      expect(trackedDuringDns).toBe(true);
+      expect(fetchImpl).not.toHaveBeenCalled();
+      expect(shutdown.hasCriticalOperations).toBe(false);
+      expect(vi.getTimerCount()).toBe(0);
+    },
+  );
+
+  it.each(['SIGINT', 'SIGTERM'] as const)(
+    'propagates %s immediately while DNS is pending',
+    async signal => {
+      const shutdown = new ShutdownController();
+      const fetchImpl = vi.fn();
+      const rejected = vi.fn();
+      const pending = probeTargetUrl('https://example.test', {
+        dnsLookup: () => new Promise(() => {}),
+        fetchImpl,
+        proxyActive: false,
+        shutdown,
+        shutdownSignal: shutdown.signal,
+      }).catch(rejected);
+
+      await vi.advanceTimersByTimeAsync(0);
+      const trackedDuringDns = shutdown.hasCriticalOperations;
+      shutdown.interrupt(signal);
+      await vi.advanceTimersByTimeAsync(0);
+      expect(rejected).toHaveBeenCalledWith(shutdown.signal.reason);
+      await pending;
+      expect(trackedDuringDns).toBe(true);
+      expect(fetchImpl).not.toHaveBeenCalled();
+      expect(shutdown.hasCriticalOperations).toBe(false);
+      expect(vi.getTimerCount()).toBe(0);
+    },
+  );
+
+  it.each(['https://example.test', 'https://93.184.216.34', 'https://[2606:4700::1111]'])(
+    'does no DNS or HTTP work for an already-aborted signal (%s)',
+    async url => {
+      const controller = new AbortController();
+      const reason = new InterruptError('SIGINT');
+      controller.abort(reason);
+      const dnsLookup = vi.fn().mockResolvedValue({ address: '93.184.216.34' });
+      const fetchImpl = vi.fn().mockResolvedValue(new Response(null));
+      await expect(
+        probeTargetUrl(url, { dnsLookup, fetchImpl, shutdownSignal: controller.signal }),
+      ).rejects.toBe(reason);
+      expect(dnsLookup).not.toHaveBeenCalled();
+      expect(fetchImpl).not.toHaveBeenCalled();
+      expect(vi.getTimerCount()).toBe(0);
+    },
+  );
+
+  it('propagates an InterruptError from the DNS hook without an advisory', async () => {
+    const reason = new InterruptError('SIGINT');
+    const fetchImpl = vi.fn();
+    await expect(
+      probeTargetUrl('https://example.test', {
+        dnsLookup: () => Promise.reject(reason),
+        fetchImpl,
+      }),
+    ).rejects.toBe(reason);
+    expect(fetchImpl).not.toHaveBeenCalled();
+    expect(vi.getTimerCount()).toBe(0);
+  });
+
+  it('checks interruption again when DNS resolves in the same turn', async () => {
+    const lookup = deferred<unknown>();
+    const shutdown = new ShutdownController();
+    const fetchImpl = vi.fn();
+    const rejected = vi.fn();
+    const pending = probeTargetUrl('https://example.test', {
+      dnsLookup: () => lookup.promise,
+      fetchImpl,
+      shutdownSignal: shutdown.signal,
+    }).catch(rejected);
+    await vi.advanceTimersByTimeAsync(0);
+    lookup.resolve({ address: '93.184.216.34' });
+    shutdown.interrupt('SIGINT');
+    await vi.advanceTimersByTimeAsync(0);
+    expect(rejected).toHaveBeenCalledWith(shutdown.signal.reason);
+    await pending;
+    expect(fetchImpl).not.toHaveBeenCalled();
+    expect(vi.getTimerCount()).toBe(0);
+  });
+
+  it.each([
+    ['timeout', 'resolve'],
+    ['timeout', 'reject'],
+    ['interrupt', 'resolve'],
+    ['interrupt', 'reject'],
+  ] as const)('ignores a late DNS %s/%s without starting HTTP', async (stop, completion) => {
+    const lookup = deferred<unknown>();
+    const shutdown = new ShutdownController();
+    const fetchImpl = vi.fn();
+    const resolved = vi.fn();
+    const rejected = vi.fn();
+    const pending = probeTargetUrl('https://example.test', {
+      dnsLookup: () => lookup.promise,
+      fetchImpl,
+      shutdown,
+      shutdownSignal: shutdown.signal,
+    }).then(resolved, rejected);
+
+    await vi.advanceTimersByTimeAsync(0);
+    if (stop === 'timeout') await vi.advanceTimersByTimeAsync(8_000);
+    else shutdown.interrupt('SIGINT');
+    await vi.advanceTimersByTimeAsync(0);
+    expect(resolved.mock.calls.length + rejected.mock.calls.length).toBe(1);
+    await pending;
+
+    if (completion === 'resolve') lookup.resolve({ address: '93.184.216.34' });
+    else lookup.reject(Object.assign(new Error('late DNS failure'), { code: 'ENOTFOUND' }));
+    // A late rejection is also observed by Vitest's unhandled-rejection guard.
+    await vi.advanceTimersByTimeAsync(8_000);
+    expect(fetchImpl).not.toHaveBeenCalled();
+    expect(resolved.mock.calls.length + rejected.mock.calls.length).toBe(1);
+    expect(shutdown.hasCriticalOperations).toBe(false);
+    expect(vi.getTimerCount()).toBe(0);
+  });
+
+  it('shares one 8s budget between DNS and HTTP', async () => {
+    const lookup = deferred<unknown>();
+    let requestSignal: AbortSignal | undefined;
+    const fetchImpl = vi.fn((_input: unknown, init?: RequestInit) => {
+      requestSignal = init?.signal ?? undefined;
+      return new Promise<Response>((_resolve, reject) => {
+        requestSignal?.addEventListener('abort', () => reject(requestSignal?.reason), {
+          once: true,
+        });
+      });
+    });
+    const settled = vi.fn();
+    const pending = probeTargetUrl('https://example.test', {
+      dnsLookup: () => lookup.promise,
+      fetchImpl,
+    }).then(settled);
+
+    await vi.advanceTimersByTimeAsync(6_000);
+    lookup.resolve({ address: '93.184.216.34' });
+    await vi.advanceTimersByTimeAsync(0);
+    expect(fetchImpl).toHaveBeenCalledOnce();
+    expect(requestSignal?.aborted).toBe(false);
+    await vi.advanceTimersByTimeAsync(1_999);
+    expect(settled).not.toHaveBeenCalled();
+    await vi.advanceTimersByTimeAsync(1);
+    expect(requestSignal?.aborted).toBe(true);
+    expect(settled).toHaveBeenCalledWith({
+      verdict: 'warn',
+      reason: expect.stringContaining('8s'),
+    });
+    await pending;
+    expect(vi.getTimerCount()).toBe(0);
+  });
+
+  it.each(['success', 'DNS refusal', 'DNS advisory', 'HTTP refusal'])(
+    'cleans up its timer and DNS abort listener after %s',
+    async outcome => {
+      const shutdown = new ShutdownController();
+      const addListener = vi.spyOn(AbortSignal.prototype, 'addEventListener');
+      const dnsLookup = vi.fn(async () => {
+        if (outcome.startsWith('DNS')) {
+          throw Object.assign(new Error('DNS failure'), {
+            code: outcome === 'DNS refusal' ? 'ENOTFOUND' : 'EAI_AGAIN',
+          });
+        }
+        return { address: '93.184.216.34' };
+      });
+      const result = await probeTargetUrl('https://example.test', {
+        dnsLookup,
+        fetchImpl: vi
+          .fn()
+          .mockResolvedValue(
+            new Response(null, { status: outcome === 'HTTP refusal' ? 503 : 200 }),
+          ),
+        shutdown,
+        shutdownSignal: shutdown.signal,
+      });
+      expect(result.verdict).toBe(
+        outcome === 'success' ? 'ok' : outcome === 'DNS advisory' ? 'warn' : 'refuse',
+      );
+      expect(vi.getTimerCount()).toBe(0);
+      expect(shutdown.hasCriticalOperations).toBe(false);
+      for (const signal of addListener.mock.instances) {
+        expect(getEventListeners(signal as AbortSignal, 'abort')).toHaveLength(0);
+      }
+    },
+  );
+});