mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-08-20 22:25:30 +00:00
fix(core): clear tool display list before awaiting completion callback
The TUI completion callback commits the finalized tool_group to history and then awaits the tool-result continuation, which since #9121 spans the entire next model turn. The display-list clear was chained after that callback in the finally block, so the completed group stayed in the live pending list - pinned at the bottom of the virtualized list - until the next tool call arrived or the loop ended (#9420, regression in v0.21.13; v0.21.12's fire-and-forget submission cleared same-frame). Notify observers that the display list is empty immediately before invoking the completion callback (no await in between, so the clear and the history commit land in the same React render); the finally-block notify remains as the error-path fallback. Adds a regression test that fails on main.
This commit is contained in:
parent
3e99b71669
commit
ced8552c09
2 changed files with 67 additions and 0 deletions
|
|
@ -1198,6 +1198,64 @@ describe('CoreToolScheduler', () => {
|
|||
expect(callerArgs.file_path).toBe('/tmp/my\\ docs/a.txt');
|
||||
});
|
||||
|
||||
it('clears the display list before awaiting the completion callback (#9420)', async () => {
|
||||
// Regression: since v0.21.13 (#9121) the TUI's completion callback awaits
|
||||
// the whole next model turn, so chaining the display-list clear after it
|
||||
// pinned the just-completed tool group at the bottom of the virtualized
|
||||
// list until the next tool call arrived. The clear must not depend on how
|
||||
// long onAllToolCallsComplete takes.
|
||||
const readExecute = vi.fn().mockResolvedValue({
|
||||
llmContent: 'read',
|
||||
returnDisplay: 'read',
|
||||
});
|
||||
let releaseCompletion: () => void = () => {};
|
||||
const onAllToolCallsComplete = vi.fn(
|
||||
() =>
|
||||
new Promise<void>((resolve) => {
|
||||
releaseCompletion = resolve;
|
||||
}),
|
||||
);
|
||||
const onToolCallsUpdate = vi.fn();
|
||||
const { scheduler } = createSchedulerForLegacyToolTests({
|
||||
toolsByName: new Map([
|
||||
[
|
||||
ToolNames.READ_FILE,
|
||||
new MockTool({ name: ToolNames.READ_FILE, execute: readExecute }),
|
||||
],
|
||||
]),
|
||||
onAllToolCallsComplete,
|
||||
onToolCallsUpdate,
|
||||
});
|
||||
|
||||
await scheduler.schedule(
|
||||
[
|
||||
{
|
||||
callId: 'clear-timing-call',
|
||||
name: ToolNames.READ_FILE,
|
||||
args: { file_path: 'a.txt' },
|
||||
isClientInitiated: false,
|
||||
prompt_id: 'prompt-clear-timing',
|
||||
},
|
||||
],
|
||||
new AbortController().signal,
|
||||
);
|
||||
|
||||
// The completion callback was invoked but is still pending: observers
|
||||
// must already have seen the emptied display list at this point.
|
||||
await vi.waitFor(() => {
|
||||
expect(onAllToolCallsComplete).toHaveBeenCalledOnce();
|
||||
});
|
||||
expect(
|
||||
onToolCallsUpdate.mock.calls.some(([calls]) => calls.length === 0),
|
||||
).toBe(true);
|
||||
|
||||
releaseCompletion();
|
||||
// The finally-block notify still fires after the callback resolves.
|
||||
await vi.waitFor(() => {
|
||||
expect(onToolCallsUpdate.mock.calls.at(-1)?.[0]).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
it('marks the budget-exempt plan reminder unchanged in the scheduler pass', async () => {
|
||||
boundaryDiagnosticsEnabled.value = true;
|
||||
const reminder = getPlanModeSystemReminder(false);
|
||||
|
|
|
|||
|
|
@ -6057,6 +6057,15 @@ export class CoreToolScheduler {
|
|||
|
||||
this.recordToolResults(completedCalls);
|
||||
|
||||
// Notify observers that the display list is empty before awaiting the
|
||||
// completion callback: the TUI commits the finalized tool_group to
|
||||
// history inside that callback, which may await the entire next model
|
||||
// turn (#9121). Deferring this notify to the finally block pinned the
|
||||
// completed group at the bottom of the virtualized list until the
|
||||
// next tool call arrived (#9420). Placed immediately before the
|
||||
// callback (no await in between) so the clear and the history commit
|
||||
// land in the same React render.
|
||||
this.notifyToolCallsUpdate();
|
||||
if (this.onAllToolCallsComplete) {
|
||||
await this.onAllToolCallsComplete(completedCalls);
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue