diff --git a/apps/gittensory-ui/src/lib/selfhost-env-reference.ts b/apps/gittensory-ui/src/lib/selfhost-env-reference.ts index 669c05a41b..dcfb23097a 100644 --- a/apps/gittensory-ui/src/lib/selfhost-env-reference.ts +++ b/apps/gittensory-ui/src/lib/selfhost-env-reference.ts @@ -85,6 +85,10 @@ export const SELFHOST_ENV_REFERENCE_ROWS: SelfHostEnvReferenceRow[] = [ name: "CLAUDE_AI_EFFORT", firstReference: "src/selfhost/ai.ts", }, + { + name: "CLAUDE_AI_FIRST_OUTPUT_TIMEOUT_MS", + firstReference: "src/selfhost/ai.ts", + }, { name: "CLAUDE_AI_MODEL", firstReference: "src/selfhost/ai.ts", @@ -482,6 +486,7 @@ export const SELFHOST_ENV_REFERENCE_MARKDOWN = [ "| `BACKUP_ACKNOWLEDGED` | `src/server.ts` |", "| `BROWSER_WS_ENDPOINT` | `src/selfhost/stubs/puppeteer.ts` |", "| `CLAUDE_AI_EFFORT` | `src/selfhost/ai.ts` |", + "| `CLAUDE_AI_FIRST_OUTPUT_TIMEOUT_MS` | `src/selfhost/ai.ts` |", "| `CLAUDE_AI_MODEL` | `src/selfhost/ai.ts` |", "| `CLAUDE_AI_TIMEOUT_MS` | `src/selfhost/ai.ts` |", "| `CLOUDFLARE_D1_MONITOR_ACCOUNT_ID` | `src/selfhost/d1-size-probe.ts` |", diff --git a/src/env.d.ts b/src/env.d.ts index 2b23e498b1..102700e90e 100644 --- a/src/env.d.ts +++ b/src/env.d.ts @@ -102,9 +102,13 @@ declare global { CLAUDE_AI_MODEL?: string; CLAUDE_AI_EFFORT?: string; CLAUDE_AI_TIMEOUT_MS?: string; + /** Fast-fail deadline for a stalled-no-output claude-code subprocess (#4994) — see resolveClaudeFirstOutputTimeoutMs. */ + CLAUDE_AI_FIRST_OUTPUT_TIMEOUT_MS?: string; CODEX_AI_MODEL?: string; CODEX_AI_EFFORT?: string; CODEX_AI_TIMEOUT_MS?: string; + /** Fast-fail deadline for a stalled-no-output codex subprocess (#codex-first-output-timeout) — see resolveCodexFirstOutputTimeoutMs. */ + CODEX_AI_FIRST_OUTPUT_TIMEOUT_MS?: string; OLLAMA_AI_BASE_URL?: string; OLLAMA_AI_API_KEY?: string; OLLAMA_AI_MODEL?: string; diff --git a/src/selfhost/ai.ts b/src/selfhost/ai.ts index 1d6cfaba42..c1a5826db9 100644 --- a/src/selfhost/ai.ts +++ b/src/selfhost/ai.ts @@ -227,6 +227,19 @@ export function resolveCodexFirstOutputTimeoutMs(env: Record): number { + const raw = Number(firstConfigured(env.CLAUDE_AI_FIRST_OUTPUT_TIMEOUT_MS)); + if (Number.isFinite(raw) && raw > 0) return Math.min(120_000, Math.max(1_000, raw)); + return 30_000; +} + /** Read the per-call repo override matching this provider variant (#3902) -- ollama/openai/openai-compatible * each have their OWN `.gittensory.yml` field, so a bare `options.model`-style single field would collide * across variants sharing this one function. `firstConfigured` gives the repo override priority over the @@ -646,11 +659,12 @@ type SpawnFn = ( input?: string; timeoutMs: number; cwd?: string; - // Optional, generic on SpawnFn (not codex-specific) so any CLI whose real progress lands on STDOUT (not - // stderr banners/logs) could opt in later — but ONLY codex wires it up today (see - // resolveCodexFirstOutputTimeoutMs): Claude Code has no comparable prod-observed dead-air hang, so leaving - // this undefined for that caller keeps its spawn path byte-identical to before this option existed. See the - // stdout-only rationale on the timer construction below — this deadline is cleared by stdout data ONLY. + // Optional, generic on SpawnFn so any CLI whose real progress lands on STDOUT (not stderr banners/logs) can + // opt in. Originally codex-only (resolveCodexFirstOutputTimeoutMs) — claude-code was deliberately left + // unwired on the belief it had no comparable dead-air hang, until GITTENSORY-K/M/8/Z (#4994) proved that + // premise stale (4,030+ subscription_cli_timeout events). Both CLI providers wire this up now + // (resolveCodexFirstOutputTimeoutMs / resolveClaudeFirstOutputTimeoutMs). See the stdout-only rationale on + // the timer construction below — this deadline is cleared by stdout data ONLY. firstOutputTimeoutMs?: number; }, ) => Promise<{ stdout: string; code: number | null; stderr?: string; timedOut?: boolean; stalledNoOutput?: boolean }>; @@ -794,6 +808,10 @@ export function createClaudeCodeAi(parentEnv: Record const claudeModel = resolveModel(configuredClaudeModel(parentEnv, options.claudeModel), model, "claude-sonnet-5"); const effort = resolveEffort(firstConfigured(options.claudeEffort, parentEnv.CLAUDE_AI_EFFORT)); const timeoutMs = resolveClaudeCliTimeoutMs(parentEnv); + // #4994: same clamp reasoning as createCodexAi's identical line — keeps the fast-fail deadline strictly + // below the full timeout even if a low CLAUDE_AI_TIMEOUT_MS override (floor 30_000ms) would otherwise let + // them collide, which would make the "outer" timeout unreachable and defeat having two distinct signals. + const firstOutputTimeoutMs = Math.min(resolveClaudeFirstOutputTimeoutMs(parentEnv), Math.max(1, timeoutMs - 1)); let attempted = false; let stdoutForMetrics = ""; try { @@ -820,12 +838,20 @@ export function createClaudeCodeAi(parentEnv: Record const spawn = spawnImpl ?? (await defaultSpawn()); const args = ["--print", "--output-format", "json", "--model", claudeModel, "--permission-mode", "plan", "--effort", effort, "--disallowedTools", "Bash,Edit,Write,WebFetch,WebSearch"]; attempted = true; - const { stdout, code, stderr, timedOut } = await spawn( + const { stdout, code, stderr, timedOut, stalledNoOutput } = await spawn( "claude", args, - { env, input: prompt, timeoutMs, cwd: await isolatedCliCwd() }, + { env, input: prompt, timeoutMs, firstOutputTimeoutMs, cwd: await isolatedCliCwd() }, ); stdoutForMetrics = stdout; + if (timedOut && stalledNoOutput) { + // Fast-fail path (#4994, GITTENSORY-K/M/8/Z), mirrors createCodexAi's identical stalled-no-output + // branch: killed at firstOutputTimeoutMs, well before the full timeoutMs, because STDOUT produced no + // bytes at all. A distinct error (never reusing `subscription_cli_timeout`) so this fast-fail is + // separately countable in Sentry/logs from a genuine full-timeout where the process was at least + // emitting output before it was killed. + throw new Error("claude_stalled_no_output: no stdout within firstOutputTimeoutMs — claude likely hung"); + } if (timedOut) throw new Error("subscription_cli_timeout"); // Surface the STRUCTURED error envelope FIRST. `claude --output-format json` reports API/auth/model errors in its // stdout JSON ({is_error,api_error_status}) on a NON-ZERO exit too — e.g. an unknown model exits 1 with the 404 diff --git a/test/unit/selfhost-ai.test.ts b/test/unit/selfhost-ai.test.ts index 54013d99f4..9b1115065f 100644 --- a/test/unit/selfhost-ai.test.ts +++ b/test/unit/selfhost-ai.test.ts @@ -2,7 +2,7 @@ import { chmodSync, mkdirSync, mkdtempSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { delimiter, join } from "node:path"; import { afterEach, describe, expect, it, vi } from "vitest"; -import { assertNoLegacySharedAiEnv, buildProvider, claudeErrorStatus, codexErrorFromStdout, createAnthropicAi, createChainAi, createClaudeCodeAi, createCodexAi, createOpenAiCompatibleAi, createSelfHostAi, extractCliText, extractCliUsage, isAiProviderHealthy, markAiProviderUnhealthyAtBoot, resetAiProviderCircuitBreakerForTest, resetAiProviderHealthForTest, resolveAiReviewerPlan, resolveClaudeCliTimeoutMs, resolveCodexAuthPath, resolveCodexCliTimeoutMs, resolveCodexEffort, resolveCodexFirstOutputTimeoutMs, resolveEffort, resolveModel, resolveProviderNames, resolveRequiredCliProviders, resolveSubscriptionCliPath, redactSecrets, routeProviders, shouldMarkAiProviderUnhealthyAtBoot, subscriptionCliEnv, withAdvisoryAiEnv } from "../../src/selfhost/ai"; +import { assertNoLegacySharedAiEnv, buildProvider, claudeErrorStatus, codexErrorFromStdout, createAnthropicAi, createChainAi, createClaudeCodeAi, createCodexAi, createOpenAiCompatibleAi, createSelfHostAi, extractCliText, extractCliUsage, isAiProviderHealthy, markAiProviderUnhealthyAtBoot, resetAiProviderCircuitBreakerForTest, resetAiProviderHealthForTest, resolveAiReviewerPlan, resolveClaudeCliTimeoutMs, resolveClaudeFirstOutputTimeoutMs, resolveCodexAuthPath, resolveCodexCliTimeoutMs, resolveCodexEffort, resolveCodexFirstOutputTimeoutMs, resolveEffort, resolveModel, resolveProviderNames, resolveRequiredCliProviders, resolveSubscriptionCliPath, redactSecrets, routeProviders, shouldMarkAiProviderUnhealthyAtBoot, subscriptionCliEnv, withAdvisoryAiEnv } from "../../src/selfhost/ai"; import { labelSelfHostReviewerModel, labelSelfHostReviewerModels } from "../../src/selfhost/ai-config"; import { renderMetrics, resetMetrics } from "../../src/selfhost/metrics"; @@ -78,6 +78,22 @@ describe("provider-specific CLI timeouts (#selfhost — no shared timeout ambigu // zero/negative also falls back (raw > 0 false branch) expect(resolveCodexFirstOutputTimeoutMs({ CODEX_AI_FIRST_OUTPUT_TIMEOUT_MS: "0" })).toBe(30_000); }); + it("resolveClaudeFirstOutputTimeoutMs defaults to 30s, is independent of effort, and honors + clamps CLAUDE_AI_FIRST_OUTPUT_TIMEOUT_MS (#4994)", () => { + // absent → the 30s default (?? right side) + expect(resolveClaudeFirstOutputTimeoutMs({})).toBe(30_000); + // effort must NOT scale this deadline — a slow COMPLETION is not a slow first byte. + expect(resolveClaudeFirstOutputTimeoutMs({ CLAUDE_AI_EFFORT: "max" })).toBe(30_000); + // present + valid → honored verbatim (?? left side, within bounds) + expect(resolveClaudeFirstOutputTimeoutMs({ CLAUDE_AI_FIRST_OUTPUT_TIMEOUT_MS: "15000" })).toBe(15_000); + // clamped to the 1s floor + expect(resolveClaudeFirstOutputTimeoutMs({ CLAUDE_AI_FIRST_OUTPUT_TIMEOUT_MS: "1" })).toBe(1_000); + // clamped to the 120s ceiling (well under the shortest full timeout, 120_000ms) + expect(resolveClaudeFirstOutputTimeoutMs({ CLAUDE_AI_FIRST_OUTPUT_TIMEOUT_MS: "999999" })).toBe(120_000); + // non-finite/garbage falls back to the default (Number.isFinite false branch) + expect(resolveClaudeFirstOutputTimeoutMs({ CLAUDE_AI_FIRST_OUTPUT_TIMEOUT_MS: "not-a-number" })).toBe(30_000); + // zero/negative also falls back (raw > 0 false branch) + expect(resolveClaudeFirstOutputTimeoutMs({ CLAUDE_AI_FIRST_OUTPUT_TIMEOUT_MS: "0" })).toBe(30_000); + }); }); afterEach(() => { @@ -1348,6 +1364,35 @@ describe("subscription CLI helpers + fail-safe", () => { } }); + // REGRESSION (GITTENSORY-K/M/8/Z, #4994): the real defaultSpawn fast-fail path against a genuinely-hung fake + // `claude` that writes nothing to either stream and never exits — mirrors the identical codex real-subprocess + // test below, proving createClaudeCodeAi's plumbing (not just a stubbed spawn) actually wires + // firstOutputTimeoutMs through to the shared defaultSpawn timer logic. + it("REAL subprocess: a fake claude that never writes to either stream is killed at the fast-fail deadline, not the full timeout", async () => { + const dir = mkdtempSync(join(tmpdir(), "fakecli-")); + const fake = join(dir, "claude"); + writeFileSync(fake, "#!/usr/bin/env node\nprocess.stdin.on('data',()=>{});\nsetInterval(()=>{},1000);\n"); + chmodSync(fake, 0o755); + const origPath = process.env.PATH; + try { + const start = Date.now(); + await expect( + createClaudeCodeAi({ + PATH: `${dir}:${origPath ?? ""}`, + CLAUDE_CODE_OAUTH_TOKEN: "t", + // Full timeout stays large (60s) so a false-pass (hitting the FULL timeout instead of the fast one) + // would make this test hang for a minute rather than silently succeed for the wrong reason. + CLAUDE_AI_TIMEOUT_MS: "60000", + CLAUDE_AI_FIRST_OUTPUT_TIMEOUT_MS: "200", + }).run("sonnet", { prompt: "hello" }), + ).rejects.toThrow(/claude_stalled_no_output/); + // Killed at ~200ms (the fast-fail deadline), nowhere near the 60_000ms full timeout. + expect(Date.now() - start).toBeLessThan(5_000); + } finally { + process.env.PATH = origPath; + } + }, 10_000); + it("drives the REAL subprocess (defaultSpawn) against a fake `codex` on PATH", async () => { const dir = mkdtempSync(join(tmpdir(), "fakecli-")); const fake = join(dir, "codex"); @@ -1540,6 +1585,32 @@ describe("subscription CLI helpers + fail-safe", () => { ); }); + it("REGRESSION (GITTENSORY-K/M/8/Z, #4994): a stalled-no-output timeout is thrown as claude_stalled_no_output, distinct from subscription_cli_timeout, and passes firstOutputTimeoutMs through to spawn", async () => { + let capturedOpts: { timeoutMs: number; firstOutputTimeoutMs?: number } | undefined; + const stalled: StubSpawn = async (_cmd, _args, o) => { + capturedOpts = o; + return { stdout: "", code: null, stderr: "", timedOut: true, stalledNoOutput: true }; + }; + await expect(createClaudeCodeAi({ CLAUDE_CODE_OAUTH_TOKEN: "t" }, stalled).run("m", { prompt: "x" })).rejects.toThrow( + /claude_stalled_no_output/, + ); + // Never the generic message — the whole point is that these two failure modes are separately observable. + await expect(createClaudeCodeAi({ CLAUDE_CODE_OAUTH_TOKEN: "t" }, stalled).run("m", { prompt: "x" })).rejects.not.toThrow( + /^subscription_cli_timeout/, + ); + // The fast-fail deadline defaults to 30s and is strictly less than the (180s-default) full timeout. + expect(capturedOpts?.firstOutputTimeoutMs).toBe(30_000); + expect(capturedOpts?.timeoutMs).toBe(180_000); + expect(capturedOpts?.firstOutputTimeoutMs).toBeLessThan(capturedOpts!.timeoutMs); + }); + + it("a full timeout WITHOUT stalledNoOutput still throws the generic subscription_cli_timeout, not claude_stalled_no_output (some output was produced before the kill)", async () => { + const timedOutWithOutput: StubSpawn = async () => ({ stdout: "partial output before kill", code: null, timedOut: true, stalledNoOutput: false }); + await expect(createClaudeCodeAi({ CLAUDE_CODE_OAUTH_TOKEN: "t" }, timedOutWithOutput).run("m", { prompt: "x" })).rejects.toThrow( + /^subscription_cli_timeout$/, + ); + }); + it("REGRESSION (GITTENSORY-K/GITTENSORY-M): a stalled-no-output timeout is thrown as codex_stalled_no_output, distinct from codex_timeout, and passes firstOutputTimeoutMs through to spawn", async () => { let capturedOpts: { timeoutMs: number; firstOutputTimeoutMs?: number } | undefined; const stalled: StubSpawn = async (_cmd, _args, o) => { @@ -1768,7 +1839,7 @@ describe("subscription CLI helpers + fail-safe", () => { } }); - it("defaultSpawn's spawn-error handler clears whichever timers were actually armed — firstOutputTimer present (codex) vs absent (claude-code)", async () => { + it("defaultSpawn's spawn-error handler clears the firstOutputTimer for both providers (#4994: both now arm one)", async () => { // Explicit env (no ambient CODEX_HOME / GITTENSORY_ENABLE_UNSAFE_CODEX_REVIEWER inherited from the operator's // shell) so this reaches the REAL ENOENT spawn error deterministically, rather than short-circuiting on the // credential-isolation guard the way an ambient CODEX_HOME would. @@ -1778,8 +1849,9 @@ describe("subscription CLI helpers + fail-safe", () => { { prompt: "x" }, ), ).rejects.toThrow(/ENOENT/); - // Claude Code never sets firstOutputTimeoutMs (no comparable prod hang), so this exercises the SAME spawn() - // error path's firstOutputTimer-ABSENT branch — the option is simply never passed for this provider. + // Claude Code now also passes firstOutputTimeoutMs (#4994) — this exercises the SAME spawn() error path's + // firstOutputTimer-PRESENT branch for claude too, proving the error handler clears it cleanly (no leaked + // timer, no unhandled rejection) rather than only ever having been exercised via codex. await expect( createClaudeCodeAi({ PATH: "/nonexistent-gittensory-empty", CLAUDE_CODE_OAUTH_TOKEN: "t" }).run("sonnet", { prompt: "x" }), ).rejects.toThrow(/ENOENT/);