Skip to content

Commit 9af3f86

Browse files
fix(cli): wait for the startup chat before an OpenTUI turn sends (#11042) (#11046)
* fix(cli): wait for the startup chat before an OpenTUI turn sends (#11042) The `E2E Interactive - OpenTUI renderer (bun)` leg goes red on main with no commit to blame. It failed at 0dd5bf2 and at 39a84c9 — the rewind classifier refactor this issue names — and passed at 74fe3a6, which carries that same refactor; 56f75ad failed run 33834473606 and passed run 33836390526 on an identical SHA. The job uploads no artifact and its log is auth-gated, so the failing test is not readable from the run. Reproduced locally instead, under CI's own bun pin and its exact command: four interactive specs fail all three vitest attempts (exit 1, 460s) while the same four pass under ink on the same machine in 34.8s. The rendered screen names the cause — `Chat not initialized` sitting where the first prompt should have opened a turn — and the scripted model server records zero requests, so the prompt never became a turn at all. OpenTUI mounts its composer and, from that mount effect, loads the command registry, which calls `config.initialize()`. `Config.initialize()` sets its own `initialized` guard before the work runs and reaches `llmClient.initialize()` -> `startChat()` only near the end, so a prompt submitted inside that window makes the turn's own `initialize()` throw "already initialized" — which the turn's catch reads as "already done" and proceeds. The client has no chat yet, the send dies in `getChat()`, and the prompt is dropped. ink cannot reach this state: #11000 gates `isInputActive` on `isConfigInitialized`, so its composer accepts nothing until initialization completes, and OpenTUI has no equivalent gate. `OpenTuiSlashDispatcher.ensureCommandsLoaded` already self-heals the same window for the registry with a bounded poll; the turn path never got one. Wait for the chat the in-flight initialization creates, on the same budget, so a config that never finishes still reports its own error instead of hanging the prompt. Starting the chat directly is not the fix: `LlmClient.initialize()` guards only on an already-built chat, so a second caller inside the window would build a second one. Witness: a turn whose `initialize()` throws "already initialized" against a client that reports no chat and throws from the send until one appears. With the wait removed the case fails `Error: Chat not initialized`; restored, the send runs once and the file's 45 cases pass. Measured: the leg now exits 0 with 10 files passed and 1 skipped (19 tests), against 4 failed files before. * fix(cli): hold the OpenTUI turn until the startup chat has tools (#11042) Chat existence is not readiness: startChat() assigns the chat and only then awaits the SessionStart hook, its context and setTools(), so a wait polling isInitialized() released the session's first prompt before the tool declarations existed and the turn reached the model with no tools. Release on the generation config's tools instead — setTools() is the flight's last stage and always writes them — and give the wait's deadline branch its own fake-timer witness. * fix(cli): settle the startup-chat wait on abort and on expiry (#11042) The wait this branch added is the first multi-second window between an OpenTUI submit and its send, and it consulted neither the turn's abort signal nor what its own expiry means. An Esc pressed inside it was ignored for the whole budget, and nothing between the loop and sendMessageStream re-checks the signal for text-only input — applyPromptVisionBridge returns on !hasImageParts before its abort check, and @-expansion is skipped for non-@ text — so a prompt the user had already cancelled still fired its UserPromptSubmit hooks and pushed a history entry. Poll the signal in the loop condition and throw once it aborts: the throw is what routes the turn through runTurn's catch, which settles on abort.signal.aborted, where a clean generator return would have fired onComplete instead. Expiry also fell through to a send whenever the flight had assigned the chat but not reached setTools(), so a SessionStart hook slower than the budget produced a turn answered with zero tool declarations and nothing on screen to say so. Fail that case with a named error, gated on isInitialized() so a config that never created a chat still surfaces the client's own "Chat not initialized". One existing test changed shape, not assertion: "skips steering when the turn is aborted" pre-aborted its controller before calling the generator, a state the production caller cannot reach and which now settles before the send. It aborts inside the fake stream instead, where a real Esc arrives. Measured on the pre-round source, neither version witnesses the tool-response boundary guard — both pass with `|| abort.aborted` deleted — so no coverage moved. --------- Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
1 parent f747999 commit 9af3f86

2 files changed

Lines changed: 237 additions & 4 deletions

File tree

packages/cli/src/ui/opentui/live-session.test.ts

Lines changed: 198 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import {
2929
nextApprovalMode,
3030
resetPromptCountForTesting,
3131
selectAutoApprovals,
32+
STARTUP_CHAT_WAIT_MS,
3233
type WaitingCallInfo,
3334
} from './live-session.js';
3435
import type { OpenTuiStreamEvent } from './event-adapter.js';
@@ -188,10 +189,19 @@ vi.mock('../hooks/atCommandProcessor.js', () => ({
188189
function createFakeConfig(
189190
sendMessageStream: (...args: unknown[]) => unknown,
190191
bridgeModel?: VisionBridgeModelSelection,
192+
isInitialized: () => boolean = () => true,
191193
) {
192194
return {
193195
initialize: vi.fn(async () => {}),
194-
getGeminiClient: () => ({ sendMessageStream }),
196+
getGeminiClient: () => ({
197+
sendMessageStream,
198+
isInitialized,
199+
// A chat the startup flight has already completed: `setTools()` ran, so
200+
// its declarations are in the generation config the send reads.
201+
getChat: () => ({
202+
getGenerationConfig: () => ({ tools: [{ functionDeclarations: [] }] }),
203+
}),
204+
}),
195205
getSessionId: () => 'session-1',
196206
getModel: () => 'test-model',
197207
getMaxSessionTurns: () => 10,
@@ -276,6 +286,185 @@ describe('livePromptEvents', () => {
276286
expect(options).toEqual({ type: SendMessageType.UserQuery });
277287
});
278288

289+
it('waits for the chat an in-flight startup initialization creates', async () => {
290+
// The boot-time command-registry load owns the initialize() flight, so the
291+
// turn's own call throws "already initialized" and its catch proceeds
292+
// while startChat() has not run yet. The real client then throws from
293+
// getChat(); the stand-in throws the same way, so the wait is what keeps
294+
// the turn alive.
295+
let chatReady = false;
296+
const sendMessageStream = vi.fn(function* () {
297+
if (!chatReady) throw new Error('Chat not initialized');
298+
yield { type: 'finished', value: {} };
299+
});
300+
const config = {
301+
...createFakeConfig(sendMessageStream, undefined, () => chatReady),
302+
initialize: vi.fn(async () => {
303+
throw new Error('Config was already initialized');
304+
}),
305+
} as unknown as Config;
306+
setTimeout(() => {
307+
chatReady = true;
308+
}, 250);
309+
310+
await drain(livePromptEvents(config, 'hello'));
311+
312+
expect(sendMessageStream).toHaveBeenCalledTimes(1);
313+
});
314+
315+
it('holds the first send until the startup chat has tool declarations', async () => {
316+
// `startChat()` assigns the chat (client.ts:2261) and only then awaits the
317+
// SessionStart hook, the session-start context and `setTools()`. A wait
318+
// that releases on chat existence alone sends the first prompt of the
319+
// session with no tool declarations at all.
320+
let chatExists = false;
321+
let tools: unknown[] | undefined;
322+
let toolsAtSend: unknown[] | undefined;
323+
const sendMessageStream = vi.fn(function* () {
324+
toolsAtSend = tools;
325+
yield { type: 'finished', value: {} };
326+
});
327+
const config = {
328+
...createFakeConfig(sendMessageStream),
329+
initialize: vi.fn(async () => {
330+
throw new Error('Config was already initialized');
331+
}),
332+
getGeminiClient: () => ({
333+
sendMessageStream,
334+
isInitialized: () => chatExists,
335+
getChat: () => ({ getGenerationConfig: () => ({ tools }) }),
336+
}),
337+
} as unknown as Config;
338+
339+
vi.useFakeTimers();
340+
try {
341+
chatExists = true;
342+
const pending = drain(livePromptEvents(config, 'hello'));
343+
// The chat exists while the startup flight is still inside setTools().
344+
await vi.advanceTimersByTimeAsync(1_000);
345+
expect(sendMessageStream).not.toHaveBeenCalled();
346+
tools = [{ functionDeclarations: [] }];
347+
await vi.advanceTimersByTimeAsync(1_000);
348+
await pending;
349+
} finally {
350+
vi.useRealTimers();
351+
}
352+
353+
expect(sendMessageStream).toHaveBeenCalledTimes(1);
354+
// A literal, not the mutable `tools`, so a send that observed no tools
355+
// cannot pass by comparing two undefined values.
356+
expect(toolsAtSend).toEqual([{ functionDeclarations: [] }]);
357+
});
358+
359+
it('reports the client error once the startup chat wait is spent', async () => {
360+
// The bound is what keeps a config that never finishes initializing from
361+
// polling forever: the turn falls through to the send and surfaces the
362+
// client's own error instead of hanging the prompt.
363+
const sendMessageStream = vi.fn(function* () {
364+
throw new Error('Chat not initialized');
365+
yield { type: 'finished', value: {} };
366+
});
367+
const config = {
368+
...createFakeConfig(sendMessageStream),
369+
initialize: vi.fn(async () => {
370+
throw new Error('Config was already initialized');
371+
}),
372+
getGeminiClient: () => ({
373+
sendMessageStream,
374+
isInitialized: () => false,
375+
getChat: () => {
376+
throw new Error('Chat not initialized');
377+
},
378+
}),
379+
} as unknown as Config;
380+
381+
vi.useFakeTimers();
382+
try {
383+
const pending = drain(livePromptEvents(config, 'hello'));
384+
// The wait expires deep inside the virtual-time run, so keep the
385+
// rejection handled until the assertion below can claim it.
386+
void pending.catch(() => {});
387+
await vi.advanceTimersByTimeAsync(STARTUP_CHAT_WAIT_MS + 1_000);
388+
await expect(pending).rejects.toThrow('Chat not initialized');
389+
} finally {
390+
vi.useRealTimers();
391+
}
392+
393+
expect(sendMessageStream).toHaveBeenCalledTimes(1);
394+
});
395+
396+
it('settles an Esc pressed during the startup wait without sending', async () => {
397+
// Releasing the wait is not enough: a text-only prompt has no abort gate
398+
// between the loop and `sendMessageStream`, so falling through would fire
399+
// the UserPromptSubmit hooks and push history for a cancelled prompt.
400+
const controller = new AbortController();
401+
const sendMessageStream = vi.fn(function* () {
402+
yield { type: 'finished', value: {} };
403+
});
404+
// A startup flight that never creates the chat, so the wait is still
405+
// running when the abort lands.
406+
const config = createFakeConfig(sendMessageStream, undefined, () => false);
407+
408+
vi.useFakeTimers();
409+
try {
410+
const pending = drain(
411+
livePromptEvents(config, 'hello', controller.signal),
412+
);
413+
let settled = false;
414+
const markSettled = () => {
415+
settled = true;
416+
};
417+
void pending.then(markSettled, markSettled);
418+
await vi.advanceTimersByTimeAsync(500);
419+
controller.abort();
420+
// Poll ticks, not the budget: the abort must release the wait at once.
421+
await vi.advanceTimersByTimeAsync(500);
422+
expect(settled).toBe(true);
423+
await expect(pending).rejects.toThrow();
424+
} finally {
425+
vi.useRealTimers();
426+
}
427+
428+
expect(sendMessageStream).not.toHaveBeenCalled();
429+
});
430+
431+
it('fails the turn when the startup wait expires on a chat with no tools', async () => {
432+
// `startChat()` assigns the chat before the SessionStart hook and
433+
// `setTools()`, so a flight still inside that gap — or one that died in
434+
// it — leaves a chat that is not ready: a send against it declares zero
435+
// tools, silently.
436+
const sendMessageStream = vi.fn(function* () {
437+
yield { type: 'finished', value: {} };
438+
});
439+
const config = {
440+
...createFakeConfig(sendMessageStream),
441+
initialize: vi.fn(async () => {
442+
throw new Error('Config was already initialized');
443+
}),
444+
getGeminiClient: () => ({
445+
sendMessageStream,
446+
isInitialized: () => true,
447+
getChat: () => ({ getGenerationConfig: () => ({ tools: undefined }) }),
448+
}),
449+
} as unknown as Config;
450+
451+
vi.useFakeTimers();
452+
try {
453+
const pending = drain(livePromptEvents(config, 'hello'));
454+
// The wait expires deep inside the virtual-time run, so keep the
455+
// rejection handled until the assertion below can claim it.
456+
void pending.catch(() => {});
457+
await vi.advanceTimersByTimeAsync(STARTUP_CHAT_WAIT_MS + 1_000);
458+
await expect(pending).rejects.toThrow(
459+
`Timed out after ${STARTUP_CHAT_WAIT_MS}ms`,
460+
);
461+
} finally {
462+
vi.useRealTimers();
463+
}
464+
465+
expect(sendMessageStream).not.toHaveBeenCalled();
466+
});
467+
279468
it('uses the ink promptId format and increments promptCount per turn', async () => {
280469
const sendMessageStream = vi.fn(function* () {});
281470
const config = createFakeConfig(sendMessageStream);
@@ -516,14 +705,19 @@ describe('livePromptEvents', () => {
516705

517706
it('skips steering when the turn is aborted', async () => {
518707
const drainSteering = vi.fn(() => ['never']);
519-
const sendMessageStream = oneToolBatchStream({
708+
const controller = new AbortController();
709+
const batch = oneToolBatchStream({
520710
callId: 't1',
521711
name: 'test_tool',
522712
args: {},
523713
});
714+
// Esc reaches the generator mid-turn, not before it: the abort lands on
715+
// the tool-response boundary, which is where a real cancel arrives.
716+
const sendMessageStream = vi.fn(() => {
717+
controller.abort();
718+
return batch();
719+
});
524720
const config = createFakeConfig(sendMessageStream);
525-
const controller = new AbortController();
526-
controller.abort();
527721

528722
await drain(
529723
livePromptEvents(config, 'start', controller.signal, { drainSteering }),

packages/cli/src/ui/opentui/live-session.ts

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -544,6 +544,12 @@ async function resolveSteeredPromptParts(
544544
return { parts, events, restore: [] };
545545
}
546546

547+
// How long a turn waits for a startup initialization that is already in
548+
// flight to create the chat. Bounded so a config that never finishes
549+
// initializing still reports its own error instead of hanging the prompt.
550+
export const STARTUP_CHAT_WAIT_MS = 15_000;
551+
const STARTUP_CHAT_POLL_MS = 100;
552+
547553
/**
548554
* Sends one user prompt through the real client and yields neutral events.
549555
* The caller (backend) drains this into the streaming model.
@@ -565,6 +571,39 @@ export async function* livePromptEvents(
565571
/* already initialized by command loading / startup */
566572
}
567573
const client = config.getGeminiClient();
574+
// `Config.initialize()` flips its own guard before the work runs, so the
575+
// boot-time command-registry load owning that flight makes the call above
576+
// throw while `startChat()` has not run yet. Sending then dies in
577+
// `getChat()` and the prompt is dropped as "Chat not initialized". The
578+
// registry self-heal in commands-dispatch bounds the same window; this
579+
// waits it out so the turn starts against a real chat.
580+
//
581+
// Chat existence alone is not readiness: `startChat()` assigns the chat and
582+
// only then awaits the SessionStart hook, its context and `setTools()`, so
583+
// releasing on existence sends the session's first prompt with no tool
584+
// declarations. `setTools()` is the flight's last stage and always writes
585+
// `tools`, which makes its presence the marker that the chat is complete.
586+
const chatDeadline = Date.now() + STARTUP_CHAT_WAIT_MS;
587+
const chatReady = () =>
588+
client.isInitialized() &&
589+
client.getChat().getGenerationConfig().tools !== undefined;
590+
while (!chatReady() && Date.now() < chatDeadline && !signal?.aborted) {
591+
await new Promise((resolve) => setTimeout(resolve, STARTUP_CHAT_POLL_MS));
592+
}
593+
// An abort before the send must reach runTurn's catch, not the send path:
594+
// nothing between here and `sendMessageStream` re-checks the signal for
595+
// text-only input, so a cancelled prompt would still fire its
596+
// UserPromptSubmit hooks and leave an entry in the session history.
597+
signal?.throwIfAborted();
598+
// Expiry with a chat assigned but not ready — the flight is still inside,
599+
// or died inside, the hook or `setTools()` stage — must not fall through:
600+
// the send would answer the whole turn with zero tool declarations and no
601+
// error. Gated so the no-chat branch still surfaces the client's own error.
602+
if (client.isInitialized() && !chatReady()) {
603+
throw new Error(
604+
`Timed out after ${STARTUP_CHAT_WAIT_MS}ms waiting for the startup chat to become ready`,
605+
);
606+
}
568607
const promptId = options?.promptId ?? nextLivePromptId(config);
569608
const abort = signal ?? new AbortController().signal;
570609
// Read per boundary rather than once: the vision bridge can pick a full-turn

0 commit comments

Comments
 (0)