mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-05-29 15:40:10 +00:00
fix: address PR feedback and background task lifecycle bugs
Addresses high-priority feedback from the automated code review tool regarding the logic error and inconsistency in `useAgentStream.ts` and `useExecutionLifecycle.ts`. - `useAgentStream.ts`: Fixed `schedule_tool` fall-through by returning early if the command type is not `submit_prompt`. Prevented history duplication by only adding items not already handled by the slash command processor. Ensured the original raw string is logged. - `useExecutionLifecycle.ts`: Removed `isActive` guards from the unmount cleanup effect and the `ExecutionLifecycleService.onBackground` listener to prevent background tasks from being incorrectly unsubscribed or missed when switching between Gemini and Agent streams. - Updated `useAgentStream.test.tsx` to reflect the fixed history logic.
This commit is contained in:
@@ -124,13 +124,6 @@ describe('useAgentStream', () => {
|
||||
expect(mockAgentProtocol.send).toHaveBeenCalledWith({
|
||||
message: { content: [{ type: 'text', text: 'modified prompt' }] },
|
||||
});
|
||||
expect(mockAddItem).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
type: MessageType.USER,
|
||||
text: 'modified prompt',
|
||||
}),
|
||||
expect.any(Number),
|
||||
);
|
||||
});
|
||||
|
||||
it('should update streamingState based on agent_start and agent_end events', async () => {
|
||||
|
||||
@@ -374,30 +374,44 @@ export const useAgentStream = ({
|
||||
let localQuery: PartListUnion = query;
|
||||
|
||||
if (!options?.isContinuation) {
|
||||
let shouldAddToHistory = true;
|
||||
if (typeof localQuery === 'string') {
|
||||
const trimmedQuery = localQuery.trim();
|
||||
void logger?.logMessage(MessageSenderType.USER, trimmedQuery);
|
||||
|
||||
if (isSlashCommand(trimmedQuery)) {
|
||||
const slashResult = await handleSlashCommand(trimmedQuery);
|
||||
if (slashResult) {
|
||||
if (slashResult.type === 'handled') {
|
||||
return;
|
||||
}
|
||||
if (slashResult.type === 'submit_prompt') {
|
||||
localQuery = slashResult.content;
|
||||
} else if (slashResult.type === 'schedule_tool') {
|
||||
addItem(
|
||||
{
|
||||
type: MessageType.ERROR,
|
||||
text: `The /${slashResult.toolName} command is not yet supported in Agent mode.`,
|
||||
},
|
||||
timestamp,
|
||||
);
|
||||
return;
|
||||
} else {
|
||||
// 'handled' or other types that don't need LLM submission
|
||||
shouldAddToHistory = false;
|
||||
}
|
||||
// schedule_tool is not yet supported in useAgentStream (mirrors handleAtCommand lack of support here)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
const queryText =
|
||||
typeof localQuery === 'string'
|
||||
? localQuery
|
||||
: partToString(localQuery);
|
||||
if (shouldAddToHistory) {
|
||||
const queryText =
|
||||
typeof localQuery === 'string'
|
||||
? localQuery
|
||||
: partToString(localQuery);
|
||||
|
||||
addItem({ type: MessageType.USER, text: queryText }, timestamp);
|
||||
void logger?.logMessage(MessageSenderType.USER, queryText);
|
||||
addItem({ type: MessageType.USER, text: queryText }, timestamp);
|
||||
if (typeof localQuery !== 'string') {
|
||||
void logger?.logMessage(MessageSenderType.USER, queryText);
|
||||
}
|
||||
}
|
||||
startNewPrompt();
|
||||
}
|
||||
|
||||
|
||||
@@ -151,14 +151,13 @@ export const useExecutionLifecycle = (
|
||||
|
||||
useEffect(
|
||||
() => () => {
|
||||
if (!isActive) return;
|
||||
// Unsubscribe from all background task events on unmount
|
||||
for (const unsubscribe of m.subscriptions.values()) {
|
||||
unsubscribe();
|
||||
}
|
||||
m.subscriptions.clear();
|
||||
},
|
||||
[m, isActive],
|
||||
[m],
|
||||
);
|
||||
|
||||
const toggleBackgroundTasks = useCallback(() => {
|
||||
@@ -330,7 +329,6 @@ export const useExecutionLifecycle = (
|
||||
// ExecutionLifecycleService.createExecution() or attachExecution()
|
||||
// automatically gets Ctrl+B support — no UI changes needed per tool.
|
||||
useEffect(() => {
|
||||
if (!isActive) return;
|
||||
const listener = (info: {
|
||||
executionId: number;
|
||||
label: string;
|
||||
@@ -352,7 +350,7 @@ export const useExecutionLifecycle = (
|
||||
return () => {
|
||||
ExecutionLifecycleService.offBackground(listener);
|
||||
};
|
||||
}, [registerBackgroundTask, m, isActive]);
|
||||
}, [registerBackgroundTask, m]);
|
||||
|
||||
const handleShellCommand = useCallback(
|
||||
(rawQuery: PartListUnion, abortSignal: AbortSignal): boolean => {
|
||||
|
||||
Reference in New Issue
Block a user