From 8f7885fe3bc8e0dfa5dd235d403d5c4063c5039d Mon Sep 17 00:00:00 2001 From: Herbicide7665 Date: Wed, 21 Oct 2026 16:54:46 +0800 Subject: [PATCH 1/2] fix(setup): apply TESTSPRITE_REQUEST_TIMEOUT_MS to the pre-write key verification ping The --from-env key-verification ping built its HttpClient from the --request-timeout flag only, so a TESTSPRITE_REQUEST_TIMEOUT_MS deadline was silently ignored and the 120s default applied. Resolve the timeout through resolveRequestTimeoutMs (flag > env > default, same clamping), matching every other command. Fixes #470. --- src/commands/auth.test.ts | 98 ++++++++++++++++++++++++++++++++++++++- src/commands/auth.ts | 6 ++- 2 files changed, 101 insertions(+), 3 deletions(-) diff --git a/src/commands/auth.test.ts b/src/commands/auth.test.ts index e2efd79e..234f5559 100644 --- a/src/commands/auth.test.ts +++ b/src/commands/auth.test.ts @@ -191,7 +191,7 @@ describe('runConfigure', () => { const signal = init?.signal; const timeout = setTimeout(() => { reject(new Error('requestTimeoutMs was not applied to the validation ping')); - }, 50); + }, 1800); signal?.addEventListener( 'abort', () => { @@ -211,7 +211,7 @@ describe('runConfigure', () => { output: 'text', debug: false, fromEnv: true, - requestTimeoutMs: 1, + requestTimeoutMs: 1000, }, { ...deps, @@ -226,6 +226,100 @@ describe('runConfigure', () => { expect(readProfile('default', { path: credentialsPath })).toBeUndefined(); }); + it('applies TESTSPRITE_REQUEST_TIMEOUT_MS to the pre-write validation ping when no flag is set', async () => { + const { deps } = makeCapture(); + let sawAbort = false; + const fetchImpl = vi.fn( + async (_input: string | URL | Request, init?: RequestInit) => + new Promise((_resolve, reject) => { + const signal = init?.signal; + const timeout = setTimeout(() => { + reject(new Error('TESTSPRITE_REQUEST_TIMEOUT_MS was not applied to the validation ping')); + }, 2500); + signal?.addEventListener( + 'abort', + () => { + sawAbort = true; + clearTimeout(timeout); + reject(new DOMException('The operation timed out.', 'TimeoutError')); + }, + { once: true }, + ); + }), + ) as unknown as typeof fetch; + + await expect( + runConfigure( + { + profile: 'default', + output: 'text', + debug: false, + fromEnv: true, + }, + { + ...deps, + env: { + TESTSPRITE_API_KEY: 'sk-user-min', + TESTSPRITE_REQUEST_TIMEOUT_MS: '1000', + }, + credentialsPath, + fetchImpl, + }, + ), + ).rejects.toBeInstanceOf(CLIError); + + expect(sawAbort).toBe(true); + expect(readProfile('default', { path: credentialsPath })).toBeUndefined(); + }); + + it('the request-timeout flag wins over TESTSPRITE_REQUEST_TIMEOUT_MS for the validation ping', async () => { + const { deps } = makeCapture(); + let sawAbort = false; + const fetchImpl = vi.fn( + async (_input: string | URL | Request, init?: RequestInit) => + new Promise((_resolve, reject) => { + const signal = init?.signal; + // If the env var (600000ms) won over the flag (1000ms), the abort + // would land far past this watchdog and the test fails fast. + const timeout = setTimeout(() => { + reject(new Error('flag did not win over TESTSPRITE_REQUEST_TIMEOUT_MS')); + }, 2500); + signal?.addEventListener( + 'abort', + () => { + sawAbort = true; + clearTimeout(timeout); + reject(new DOMException('The operation timed out.', 'TimeoutError')); + }, + { once: true }, + ); + }), + ) as unknown as typeof fetch; + + await expect( + runConfigure( + { + profile: 'default', + output: 'text', + debug: false, + fromEnv: true, + requestTimeoutMs: 1000, + }, + { + ...deps, + env: { + TESTSPRITE_API_KEY: 'sk-user-min', + TESTSPRITE_REQUEST_TIMEOUT_MS: '600000', + }, + credentialsPath, + fetchImpl, + }, + ), + ).rejects.toBeInstanceOf(CLIError); + + expect(sawAbort).toBe(true); + }); + it('throws VALIDATION_ERROR when --from-env is set but key is missing', async () => { const { deps } = makeCapture(); await expect( diff --git a/src/commands/auth.ts b/src/commands/auth.ts index 49968585..829e612b 100644 --- a/src/commands/auth.ts +++ b/src/commands/auth.ts @@ -6,6 +6,7 @@ import { emitDryRunBanner, makeHttpClient, parseRequestTimeoutFlag, + resolveRequestTimeoutMs, type CommonOptions as FactoryCommonOptions, } from '../lib/client-factory.js'; import { ApiError, CLIError, InterruptError } from '../lib/errors.js'; @@ -283,7 +284,10 @@ export async function runConfigure( baseUrl: facadeBaseUrl(apiUrl), apiKey, fetchImpl: deps.fetchImpl, - requestTimeoutMs: opts.requestTimeoutMs, + // Same precedence as every other command's client (flag > env > default, + // same clamping): `setup --from-env` previously read the flag only, so a + // TESTSPRITE_REQUEST_TIMEOUT_MS deadline was silently ignored here. + requestTimeoutMs: resolveRequestTimeoutMs(opts, env), shutdownSignal: deps.shutdownSignal ?? globalShutdown.signal, shutdown: deps.shutdown ?? globalShutdown, }); From 7f3d872d7876a72fe0475ef8bb4d2296acb4615a Mon Sep 17 00:00:00 2001 From: Herbicide7665 Date: Thu, 22 Oct 2026 09:56:23 +0800 Subject: [PATCH 2/2] test(setup): cover default and clamped timeouts through the pre-write validation ping CodeRabbit review follow-up: the changed tests covered env handling and flag precedence but not the default timeout or clamping through runConfigure. Adds a fake-timer test for the 120s default and a real-timer test for the sub-minimum env value clamping up to 1s. --- src/commands/auth.test.ts | 79 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 79 insertions(+) diff --git a/src/commands/auth.test.ts b/src/commands/auth.test.ts index 234f5559..efe802b3 100644 --- a/src/commands/auth.test.ts +++ b/src/commands/auth.test.ts @@ -272,6 +272,85 @@ describe('runConfigure', () => { expect(readProfile('default', { path: credentialsPath })).toBeUndefined(); }); + it('uses the default timeout for the pre-write validation ping when nothing is configured', async () => { + vi.useFakeTimers(); + try { + const { deps } = makeCapture(); + let sawAbort = false; + const fetchImpl = vi.fn( + async (_input: string | URL | Request, init?: RequestInit) => + new Promise((_resolve, reject) => { + const signal = init?.signal; + signal?.addEventListener( + 'abort', + () => { + sawAbort = true; + reject(signal.reason); + }, + { once: true }, + ); + }), + ) as unknown as typeof fetch; + + const pending = runConfigure( + { profile: 'default', output: 'text', debug: false, fromEnv: true }, + { + ...deps, + env: { TESTSPRITE_API_KEY: 'sk-user-min' }, + credentialsPath, + fetchImpl, + }, + ); + const assertion = expect(pending).rejects.toBeInstanceOf(CLIError); + await vi.advanceTimersByTimeAsync(120_000 + 1); + await assertion; + expect(sawAbort).toBe(true); + expect(readProfile('default', { path: credentialsPath })).toBeUndefined(); + } finally { + vi.useRealTimers(); + } + }); + + it('clamps a sub-minimum TESTSPRITE_REQUEST_TIMEOUT_MS up to 1s for the validation ping', async () => { + const { deps } = makeCapture(); + let sawAbort = false; + const fetchImpl = vi.fn( + async (_input: string | URL | Request, init?: RequestInit) => + new Promise((_resolve, reject) => { + const signal = init?.signal; + const timeout = setTimeout(() => { + reject(new Error('clamped TESTSPRITE_REQUEST_TIMEOUT_MS was not applied')); + }, 2500); + signal?.addEventListener( + 'abort', + () => { + sawAbort = true; + clearTimeout(timeout); + reject(new DOMException('The operation timed out.', 'TimeoutError')); + }, + { once: true }, + ); + }), + ) as unknown as typeof fetch; + + await expect( + runConfigure( + { profile: 'default', output: 'text', debug: false, fromEnv: true }, + { + ...deps, + env: { + TESTSPRITE_API_KEY: 'sk-user-min', + TESTSPRITE_REQUEST_TIMEOUT_MS: '50', + }, + credentialsPath, + fetchImpl, + }, + ), + ).rejects.toBeInstanceOf(CLIError); + + expect(sawAbort).toBe(true); + }); + it('the request-timeout flag wins over TESTSPRITE_REQUEST_TIMEOUT_MS for the validation ping', async () => { const { deps } = makeCapture(); let sawAbort = false;