fix(app-config): refuse an LLM ceiling that would become a zero timeout

LLM_TIMEOUT_MS was validated as "a positive number", which lets two shapes
through that then degrade into an immediate abort rather than a long one.
A fractional value passes the check and floors to 0 in derive, which makes
the dispatcher fall back to undici's default and setTimeout(abort, 0) kill
every model call on the next tick. Anything past 2^31-1 does the same,
because Node clamps the delay to 1 ms. Both now refuse at boot, which is
what the wiki row already promised.

Also pins the seam the three deleted constants used to occupy: the clients'
abort deadline has to come from the configured ceiling. Nothing else in the
suite would have noticed a literal coming back.
This commit is contained in:
Maurice
2026-09-04 18:48:48 +02:00
committed by Maurice
parent 798859517f
commit c1d0435480
3 changed files with 50 additions and 1 deletions
+5 -1
View File
@@ -112,7 +112,11 @@ export const envSchema = z.object({
// non-http(s) entries today — left unvalidated to keep that behavior. // non-http(s) entries today — left unvalidated to keep that behavior.
OVERPASS_URL: anyString, OVERPASS_URL: anyString,
OVERPASS_TIMEOUT_MS: positiveNumber, OVERPASS_TIMEOUT_MS: positiveNumber,
LLM_TIMEOUT_MS: positiveNumber, // Whole milliseconds, and inside setTimeout's 32-bit range. A fractional value
// would floor to 0 and abort every model call on the next tick; anything past
// 2^31-1 makes Node clamp the delay to 1 ms and do the same. Both refuse at
// boot rather than degrading into a timeout of zero.
LLM_TIMEOUT_MS: integer(1, 2_147_483_647, 'must be a whole number of milliseconds between 1 and 2147483647'),
KITINERARY_EXTRACTOR_PATH: anyString, KITINERARY_EXTRACTOR_PATH: anyString,
// The OS search path. Not configuration anybody sets for TREK — it is here so // The OS search path. Not configuration anybody sets for TREK — it is here so
// the kitinerary probe can resolve its binary to an absolute path itself // the kitinerary probe can resolve its binary to an absolute path itself
@@ -34,6 +34,21 @@ describe('validateEnvAtBoot', () => {
).not.toThrow(); ).not.toThrow();
}); });
it('refuses an LLM ceiling that would silently become a zero timeout', () => {
// Both shapes pass a plain "positive number" check and then degrade into an
// immediate abort: a fractional value floors to 0, and anything past the
// 32-bit range makes Node clamp the setTimeout delay to 1 ms. The wiki
// promises invalid values abort startup, so they have to refuse here.
expect(() => validateEnvAtBoot({ LLM_TIMEOUT_MS: '0.5' })).toThrow();
expect(() => validateEnvAtBoot({ LLM_TIMEOUT_MS: '900000.5' })).toThrow();
expect(() => validateEnvAtBoot({ LLM_TIMEOUT_MS: '2147483648' })).toThrow();
expect(() => validateEnvAtBoot({ LLM_TIMEOUT_MS: '0' })).toThrow();
expect(() => validateEnvAtBoot({ LLM_TIMEOUT_MS: '-1' })).toThrow();
expect(() => validateEnvAtBoot({ LLM_TIMEOUT_MS: 'abc' })).toThrow();
expect(() => validateEnvAtBoot({ LLM_TIMEOUT_MS: '900000' })).not.toThrow();
expect(() => validateEnvAtBoot({ LLM_TIMEOUT_MS: '' })).not.toThrow();
});
it('treats blank values as unset (defaults apply, no error)', () => { it('treats blank values as unset (defaults apply, no error)', () => {
expect(() => validateEnvAtBoot({ DEMO_MODE: '', PORT: ' ', TZ: '' })).not.toThrow(); expect(() => validateEnvAtBoot({ DEMO_MODE: '', PORT: ' ', TZ: '' })).not.toThrow();
}); });
@@ -10,6 +10,7 @@ vi.mock('../../../../src/utils/ssrfGuard', () => ({ safeFetchLlm: safeFetchLlmMo
import { OpenAiCompatibleClient } from '../../../../src/nest/llm-parse/clients/openai-compatible.client'; import { OpenAiCompatibleClient } from '../../../../src/nest/llm-parse/clients/openai-compatible.client';
import { AnthropicClient } from '../../../../src/nest/llm-parse/clients/anthropic.client'; import { AnthropicClient } from '../../../../src/nest/llm-parse/clients/anthropic.client';
import type { LlmExtractionInput } from '../../../../src/nest/llm-parse/llm-provider.interface'; import type { LlmExtractionInput } from '../../../../src/nest/llm-parse/llm-provider.interface';
import { readEnv } from '../../../../src/app-config';
const baseInput: LlmExtractionInput = { const baseInput: LlmExtractionInput = {
prompt: 'system', prompt: 'system',
@@ -257,3 +258,32 @@ describe('AnthropicClient', () => {
expect(blocks.some((b: any) => b.type === 'document' && b.source.type === 'base64')).toBe(true); expect(blocks.some((b: any) => b.type === 'document' && b.source.type === 'base64')).toBe(true);
}); });
}); });
/**
* The three per-client constants are gone; what replaced them is a config read.
* Nothing else in the suite would notice if one of them came back as a literal,
* so pin the seam itself: the abort deadline has to come from LLM_TIMEOUT_MS.
*/
describe('the abort deadline comes from the configured ceiling (#2230)', () => {
const setTimeoutSpy = () => vi.spyOn(globalThis, 'setTimeout');
beforeEach(() => vi.restoreAllMocks());
it('AnthropicClient arms its AbortController with the configured value', async () => {
const spy = setTimeoutSpy();
mockFetch(() => jsonResponse({ content: [{ type: 'tool_use', name: 'emit_reservations', input: { reservations: [] } }] }));
await new AnthropicClient().extract(baseInput);
expect(spy).toHaveBeenCalledWith(expect.any(Function), readEnv().integrations.llmTimeoutMs);
});
it('OpenAiCompatibleClient arms its AbortController with the configured value', async () => {
const spy = setTimeoutSpy();
mockFetch(() => jsonResponse({ choices: [{ message: { content: '{"reservations":[]}' } }] }));
await new OpenAiCompatibleClient().extract({ ...baseInput, baseUrl: 'http://ollama.local:11434/v1' });
expect(spy).toHaveBeenCalledWith(expect.any(Function), readEnv().integrations.llmTimeoutMs);
});
});