Skip to content
39 changes: 37 additions & 2 deletions packages/cli/src/nonInteractiveCliCommands.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,19 @@
* SPDX-License-Identifier: Apache-2.0
*/

import { describe, it, expect, vi, beforeEach } from 'vitest';
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
import {
getAvailableCommands,
handleSlashCommand,
} from './nonInteractiveCliCommands.js';
import type { Config } from '@qwen-code/qwen-code-core';
import {
__resetActiveGoalStoreForTests,
type Config,
} from '@qwen-code/qwen-code-core';
import type { LoadedSettings } from './config/settings.js';
import { CommandKind, type ExecutionMode } from './ui/commands/types.js';
import { filterCommandsForMode } from './services/commandUtils.js';
import { goalCommand } from './ui/commands/goalCommand.js';

// Mock the CommandService
const mockGetCommands = vi.hoisted(() => vi.fn());
Expand All @@ -32,6 +36,7 @@ describe('handleSlashCommand', () => {

beforeEach(() => {
vi.clearAllMocks();
__resetActiveGoalStoreForTests();
// getCommandsForMode applies real mode filtering on top of getCommands()
mockGetCommandsForMode.mockImplementation((mode: ExecutionMode) =>
filterCommandsForMode(mockGetCommands(), mode),
Expand All @@ -55,6 +60,12 @@ describe('handleSlashCommand', () => {
getFolderTrustFeature: vi.fn().mockReturnValue(false),
getFolderTrust: vi.fn().mockReturnValue(false),
getProjectRoot: vi.fn().mockReturnValue('/test/project'),
isTrustedFolder: vi.fn().mockReturnValue(true),
getDisableAllHooks: vi.fn().mockReturnValue(false),
getHookSystem: vi.fn().mockReturnValue({
addFunctionHook: vi.fn().mockReturnValue('goal-hook-id'),
removeFunctionHook: vi.fn().mockReturnValue(true),
}),
setModelInvocableCommandsProvider: vi.fn(),
setModelInvocableCommandsExecutor: vi.fn(),
getDisabledSlashCommands: vi.fn().mockReturnValue([]),
Expand All @@ -71,6 +82,10 @@ describe('handleSlashCommand', () => {
abortController = new AbortController();
});

afterEach(() => {
__resetActiveGoalStoreForTests();
});

it('should return no_command for non-slash input', async () => {
const result = await handleSlashCommand(
'regular text',
Expand Down Expand Up @@ -199,6 +214,26 @@ describe('handleSlashCommand', () => {
}
});

it('should execute /goal in non-interactive mode as a submit_prompt command', async () => {
mockGetCommands.mockReturnValue([goalCommand]);

const result = await handleSlashCommand(
'/goal write a hello world script',
abortController,
mockConfig,
mockSettings,
);

expect(result.type).toBe('submit_prompt');
if (result.type === 'submit_prompt') {
expect(result.content).toEqual([
expect.objectContaining({
text: expect.stringContaining('write a hello world script'),
}),
]);
}
});

it('should execute FILE commands in any mode without explicit supportedModes', async () => {
const mockFileCommand = {
name: 'custom',
Expand Down
7 changes: 5 additions & 2 deletions packages/cli/src/ui/commands/goalCommand.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,11 @@ describe('goalCommand', () => {
beforeEach(() => __resetActiveGoalStoreForTests());
afterEach(() => __resetActiveGoalStoreForTests());

it('is currently limited to interactive mode', () => {
expect(goalCommand.supportedModes).toEqual(['interactive']);
it('is available in interactive and non-interactive modes', () => {
expect(goalCommand.supportedModes).toEqual([
'interactive',
'non_interactive',
]);
});

it('rejects when config is missing', async () => {
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/src/ui/commands/goalCommand.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ export const goalCommand: SlashCommand = {
},
argumentHint: '[<condition> | clear]',
kind: CommandKind.BUILT_IN,
supportedModes: ['interactive'] as const,
supportedModes: ['interactive', 'non_interactive'] as const,
action: async (
context: CommandContext,
args: string,
Expand Down
82 changes: 82 additions & 0 deletions packages/cli/src/ui/hooks/useGeminiStream.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,9 @@ const mockParseAndFormatApiError = vi.hoisted(() =>
);
const mockLogApiCancel = vi.hoisted(() => vi.fn());
const mockGetActiveGoal = vi.hoisted(() => vi.fn());
const mockActiveGoalEquals = vi.hoisted(() => vi.fn());
const mockSetActiveGoal = vi.hoisted(() => vi.fn());
const mockClearActiveGoal = vi.hoisted(() => vi.fn());

vi.mock('@qwen-code/qwen-code-core', async (importOriginal) => {
const actualCoreModule = (await importOriginal()) as any;
Expand All @@ -90,6 +93,9 @@ vi.mock('@qwen-code/qwen-code-core', async (importOriginal) => {
parseAndFormatApiError: mockParseAndFormatApiError,
logApiCancel: mockLogApiCancel,
getActiveGoal: mockGetActiveGoal,
activeGoalEquals: mockActiveGoalEquals,
setActiveGoal: mockSetActiveGoal,
clearActiveGoal: mockClearActiveGoal,
};
});

Expand Down Expand Up @@ -153,6 +159,7 @@ describe('useGeminiStream', () => {
beforeEach(() => {
vi.clearAllMocks(); // Clear mocks before each test
mockGetActiveGoal.mockReturnValue(undefined);
mockActiveGoalEquals.mockReturnValue(false);
vi.mocked(findLastSafeSplitPoint).mockImplementation(
(s: string) => s.length,
);
Expand Down Expand Up @@ -4638,6 +4645,81 @@ describe('useGeminiStream', () => {
});

describe('StopHookLoop Event', () => {
it('syncs active_goal events into the active goal store', async () => {
const activeGoal = {
condition: 'finish the refactor',
iterations: 1,
setAt: 123,
tokensAtStart: 456,
hookId: 'goal-hook-id',
lastReason: 'still missing verification',
};
mockSendMessageStream.mockReturnValue(
(async function* () {
yield {
type: ServerGeminiEventType.ActiveGoal,
value: activeGoal,
};
yield {
type: ServerGeminiEventType.ActiveGoal,
value: null,
};
})(),
);
mockGetActiveGoal
.mockReturnValueOnce(undefined)
.mockReturnValueOnce(activeGoal);
mockActiveGoalEquals.mockReturnValue(false);

const { result } = renderTestHook();

await act(async () => {
await result.current.submitQuery('continue goal');
});

expect(mockSetActiveGoal).toHaveBeenCalledWith(
'test-session-id',
activeGoal,
);
expect(mockClearActiveGoal).toHaveBeenCalledWith('test-session-id');
});

it('skips redundant active_goal store updates', async () => {
const activeGoal = {
condition: 'finish the refactor',
iterations: 1,
setAt: 123,
tokensAtStart: 456,
hookId: 'goal-hook-id',
lastReason: 'still missing verification',
};
mockSendMessageStream.mockReturnValue(
(async function* () {
yield {
type: ServerGeminiEventType.ActiveGoal,
value: activeGoal,
};
yield {
type: ServerGeminiEventType.ActiveGoal,
value: null,
};
})(),
);
mockGetActiveGoal
.mockReturnValueOnce(activeGoal)
.mockReturnValueOnce(undefined);
mockActiveGoalEquals.mockReturnValue(true);

const { result } = renderTestHook();

await act(async () => {
await result.current.submitQuery('continue goal');
});

expect(mockSetActiveGoal).not.toHaveBeenCalled();
expect(mockClearActiveGoal).not.toHaveBeenCalled();
});

it('should handle StopHookLoop event and add stop hook loop history item', async () => {
mockSendMessageStream.mockReturnValue(
(async function* () {
Expand Down
27 changes: 27 additions & 0 deletions packages/cli/src/ui/hooks/useGeminiStream.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import type {
ToolCallRequestInfo,
GeminiErrorEventValue,
StopFailureErrorType,
ActiveGoal,
} from '@qwen-code/qwen-code-core';
import {
GeminiEventType as ServerGeminiEventType,
Expand All @@ -52,6 +53,9 @@ import {
getUnsupportedImageFormatWarning,
generateToolUseSummary,
getActiveGoal,
activeGoalEquals,
setActiveGoal,
clearActiveGoal,
} from '@qwen-code/qwen-code-core';
import { type Part, type PartListUnion, FinishReason } from '@google/genai';
import type {
Expand Down Expand Up @@ -1339,6 +1343,25 @@ export const useGeminiStream = (
[addItem, config, pendingHistoryItemRef, setPendingHistoryItem],
);

const handleActiveGoalEvent = useCallback(
(activeGoal: ActiveGoal | null) => {
const sessionId = config.getSessionId();
const currentActiveGoal = getActiveGoal(sessionId);
if (activeGoal) {
if (activeGoalEquals(currentActiveGoal, activeGoal)) {
return;
}
setActiveGoal(sessionId, activeGoal);
return;
}
if (!currentActiveGoal) {
return;
}
clearActiveGoal(sessionId);
},
[config],
);

const processGeminiStreamEvents = useCallback(
async (
stream: AsyncIterable<GeminiEvent>,
Expand Down Expand Up @@ -1573,6 +1596,9 @@ export const useGeminiStream = (
flushBufferedStreamEvents();
handleStopHookLoopEvent(event.value, userMessageTimestamp);
break;
case ServerGeminiEventType.ActiveGoal:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Critical] ActiveGoal case is the sole exception among 22+ event handlers that skips flushBufferedStreamEvents(). Every other case (Content, Thought, Citation, HookSystemMessage, StopHookLoop, etc.) calls flush before processing.

When ActiveGoal arrives after content events still in the debounce buffer, the goal-state mutation happens before preceding content is committed to history. Today this is mostly latent (the handler only writes to an in-memory store), but any future change making handleActiveGoalEvent produce a history item or trigger a UI update will immediately cause ordering bugs.

Suggested change
case ServerGeminiEventType.ActiveGoal:
case ServerGeminiEventType.ActiveGoal:
flushBufferedStreamEvents();
handleActiveGoalEvent(event.value);
break;

— qwen-latest-series-invite-beta-v28 via Qwen Code /review

handleActiveGoalEvent(event.value);
break;
default: {
// enforces exhaustive switch-case
const unreachable: never = event;
Expand Down Expand Up @@ -1609,6 +1635,7 @@ export const useGeminiStream = (
setPendingHistoryItem,
handleUserPromptSubmitBlockedEvent,
handleStopHookLoopEvent,
handleActiveGoalEvent,
addItem,
dualOutput,
],
Expand Down
Loading
Loading