From 6d036aed4118232993718afcd6cc0defc00d944c Mon Sep 17 00:00:00 2001 From: ChiGao Date: Thu, 27 Aug 2026 07:19:19 +0000 Subject: [PATCH] fix(cli): bound the goal-runtime startup wait and skip the no-op Bun memory relaunch (#10128) * fix(cli): bound the goal-runtime startup wait so lease contention cannot hang the TUI * fix(cli): skip the no-op memory-flag relaunch under Bun Bun accepts --max-old-space-size but ignores it (its heap limit adapts dynamically), so the autoConfigureMemory relaunch only wastes a process hop under Bun. * fix(cli): surface goal-runtime degraded startup and fix Bun comment framing - Route the goal-runtime startup timeout through mergeStartupWarnings so users see why goal features are degraded without QWEN_CODE_DEBUG=1. - Reword the Bun memory-flag comment: the one-process relaunch is unconditional, so the guard only stops forwarding a no-op V8 flag into the relaunch/sandbox child. --- packages/cli/src/gemini.tsx | 9 ++++ packages/cli/src/ui/AppContainer.tsx | 30 ++++++++++- .../cli/src/ui/utils/goal-runtime.test.ts | 54 +++++++++++++++++-- packages/cli/src/ui/utils/goal-runtime.ts | 48 +++++++++++++++-- 4 files changed, 130 insertions(+), 11 deletions(-) diff --git a/packages/cli/src/gemini.tsx b/packages/cli/src/gemini.tsx index d0204370920..2f626d39e8c 100644 --- a/packages/cli/src/gemini.tsx +++ b/packages/cli/src/gemini.tsx @@ -146,6 +146,15 @@ export function validateDnsResolutionOrder( } function getNodeMemoryArgs(isDebugMode: boolean): string[] { + // Bun accepts --max-old-space-size but it is a no-op (Bun's heap limit + // starts small and adapts dynamically instead of honouring the flag). + // The one-process relaunch below happens unconditionally, so under Bun + // this only stops forwarding a flag that does nothing into the + // relaunch/sandbox child. + if ('bun' in process.versions) { + return []; + } + const totalMemoryMB = os.totalmem() / (1024 * 1024); const heapStats = v8.getHeapStatistics(); const currentMaxOldSpaceSizeMb = Math.floor( diff --git a/packages/cli/src/ui/AppContainer.tsx b/packages/cli/src/ui/AppContainer.tsx index a77bebbcde7..40cd2ad261d 100644 --- a/packages/cli/src/ui/AppContainer.tsx +++ b/packages/cli/src/ui/AppContainer.tsx @@ -269,6 +269,10 @@ import { import { MAIN_CONTENT_HEIGHT_RESERVATION } from './utils/layoutUtils.js'; const CTRL_EXIT_PROMPT_DURATION_MS = 1000; +// Startup gate for the goal runtime: under session-writer lease contention +// getGoalRuntimeReady() can stay pending forever; bound it so the command +// registry still loads and the TUI stays usable (see waitForGoalRuntime). +const GOAL_RUNTIME_STARTUP_TIMEOUT_MS = 5_000; const debugLogger = createDebugLogger('APP_CONTAINER'); export function isRenderModeToggleKey(key: Key): boolean { @@ -1011,9 +1015,31 @@ export const AppContainer = (props: AppContainerProps) => { // handled by the global catch. profileCheckpoint('config_initialize_start'); await config.initialize(); - await waitForGoalRuntime(config); + // Bound the goal-runtime gate: under session-writer lease contention + // (a crashed/sibling process holding the lease) getGoalRuntimeReady() + // never settles, which used to hang startup here and leave the command + // registry empty — every slash command, even /quit, came back + // "Unknown command". After the timeout we proceed with goal features + // degraded rather than an unusable TUI. + const goalRuntimeReady = await waitForGoalRuntime(config, { + timeoutMs: GOAL_RUNTIME_STARTUP_TIMEOUT_MS, + }); + if (!goalRuntimeReady) { + debugLogger.warn( + `Goal runtime did not settle within ${GOAL_RUNTIME_STARTUP_TIMEOUT_MS}ms ` + + '(session writer lease contention?); continuing with goal features degraded.', + ); + } setStartupWarnings((currentWarnings) => - mergeStartupWarnings(currentWarnings, config.getWarnings()), + mergeStartupWarnings( + currentWarnings, + goalRuntimeReady + ? config.getWarnings() + : [ + ...config.getWarnings(), + `Goal features are degraded: the goal runtime did not settle within ${GOAL_RUNTIME_STARTUP_TIMEOUT_MS}ms at startup.`, + ], + ), ); profileCheckpoint('config_initialize_end'); setConfigInitialized(true); diff --git a/packages/cli/src/ui/utils/goal-runtime.test.ts b/packages/cli/src/ui/utils/goal-runtime.test.ts index cd47a9eea81..affa44ea4ee 100644 --- a/packages/cli/src/ui/utils/goal-runtime.test.ts +++ b/packages/cli/src/ui/utils/goal-runtime.test.ts @@ -4,22 +4,27 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { describe, expect, it, vi } from 'vitest'; -import { GoalPersistenceUnavailableError } from '@qwen-code/qwen-code-core'; +import { describe, expect, it, vi, afterEach } from 'vitest'; +import { + GoalPersistenceUnavailableError, + type GoalRuntime, +} from '@qwen-code/qwen-code-core'; import { shouldDisplayGoalStateCause, waitForGoalRuntime, } from './goal-runtime.js'; describe('waitForGoalRuntime', () => { + afterEach(() => vi.useRealTimers()); + it('allows Goal-less sessions when persistence is disabled', async () => { const getGoalRuntimeReady = vi .fn() .mockRejectedValue(new GoalPersistenceUnavailableError()); - await expect( - waitForGoalRuntime({ getGoalRuntimeReady }), - ).resolves.toBeUndefined(); + await expect(waitForGoalRuntime({ getGoalRuntimeReady })).resolves.toBe( + true, + ); expect(getGoalRuntimeReady).toHaveBeenCalledTimes(1); }); @@ -32,6 +37,45 @@ describe('waitForGoalRuntime', () => { ); }); + it('resolves true once the runtime settles within the timeout', async () => { + const getGoalRuntimeReady = vi.fn().mockResolvedValue({}); + await expect( + waitForGoalRuntime({ getGoalRuntimeReady }, { timeoutMs: 100 }), + ).resolves.toBe(true); + }); + + it('proceeds (false) instead of hanging when the runtime never settles', async () => { + vi.useFakeTimers(); + // A promise that never resolves — the lease-contention hang. + const getGoalRuntimeReady = vi.fn(() => new Promise(() => {})); + const pending = waitForGoalRuntime( + { getGoalRuntimeReady }, + { timeoutMs: 50 }, + ); + await vi.advanceTimersByTimeAsync(60); + await expect(pending).resolves.toBe(false); + }); + + it('a late rejection after timeout does not become unhandled', async () => { + vi.useFakeTimers(); + let reject!: (err: Error) => void; + const getGoalRuntimeReady = vi.fn( + () => + new Promise((_resolve, rej) => { + reject = rej; + }), + ); + const pending = waitForGoalRuntime( + { getGoalRuntimeReady }, + { timeoutMs: 10 }, + ); + await vi.advanceTimersByTimeAsync(20); + await expect(pending).resolves.toBe(false); + // Reject after the timeout won; the race must have a handler attached. + reject(new Error('late')); + await vi.advanceTimersByTimeAsync(10); + }); + it('keeps turn and verifier bookkeeping out of scrollback', () => { expect(shouldDisplayGoalStateCause('turn_finished')).toBe(false); expect(shouldDisplayGoalStateCause('checkpoint')).toBe(false); diff --git a/packages/cli/src/ui/utils/goal-runtime.ts b/packages/cli/src/ui/utils/goal-runtime.ts index 2918dab843d..dc8064e1665 100644 --- a/packages/cli/src/ui/utils/goal-runtime.ts +++ b/packages/cli/src/ui/utils/goal-runtime.ts @@ -35,12 +35,52 @@ export function shouldDisplayGoalStateCause(cause: GoalStateCause): boolean { } } +/** + * Awaits the goal runtime becoming ready. + * + * `getGoalRuntimeReady()` can stay pending indefinitely when the session + * writer lease is contended (a crashed/sibling process holding the lease), + * which previously hung the ink startup gate forever — the command registry + * was never populated and EVERY slash command, including `/quit`, reported + * "Unknown command". `timeoutMs` bounds that wait: after the timeout the + * gate proceeds so the UI stays usable (goal features degrade rather than + * blocking the whole CLI). Pass no timeout for the original unbounded + * semantics. + * + * Resolves `true` when the runtime settled (or persistence is unavailable, + * which is treated as settled), `false` when the timeout fired first. + */ export async function waitForGoalRuntime( config: Pick, -): Promise { + options: { timeoutMs?: number } = {}, +): Promise { + const ready = config.getGoalRuntimeReady(); + const awaitReady = async (): Promise => { + try { + await ready; + } catch (error) { + if (!(error instanceof GoalPersistenceUnavailableError)) throw error; + } + }; + + const { timeoutMs } = options; + if (timeoutMs == null || timeoutMs <= 0) { + await awaitReady(); + return true; + } + + let timer: NodeJS.Timeout | undefined; + const timeout = new Promise<'timeout'>((resolve) => { + timer = setTimeout(() => resolve('timeout'), timeoutMs); + // The timeout must not keep the process alive on its own. + timer.unref?.(); + }); try { - await config.getGoalRuntimeReady(); - } catch (error) { - if (!(error instanceof GoalPersistenceUnavailableError)) throw error; + // Promise.race keeps a handler on the losing promise too, so a late + // rejection of the goal-runtime promise cannot become unhandled. + const winner = await Promise.race([awaitReady(), timeout]); + return winner !== 'timeout'; + } finally { + if (timer) clearTimeout(timer); } }