diff --git a/packages/cli/src/acp/acpClient.ts b/packages/cli/src/acp/acpClient.ts index db2d04dab4..ad796e6a9c 100644 --- a/packages/cli/src/acp/acpClient.ts +++ b/packages/cli/src/acp/acpClient.ts @@ -96,6 +96,8 @@ export async function runAcpClient( await connection.closed.finally(runExitCleanup); } +let callIdCounter = 0; + export class GeminiAgent { private sessions: Map = new Map(); private clientCapabilities: acp.ClientCapabilities | undefined; @@ -834,7 +836,7 @@ export class Session { promptId: string, fc: FunctionCall, ): Promise { - const callId = fc.id ?? `${fc.name}-${Date.now()}`; + const callId = fc.id ?? `${fc.name}-${Date.now()}-${++callIdCounter}`; const args = fc.args ?? {}; const startTime = Date.now(); @@ -1295,7 +1297,7 @@ export class Session { include: pathSpecsToRead, }; - const callId = `${readManyFilesTool.name}-${Date.now()}`; + const callId = `${readManyFilesTool.name}-${Date.now()}-${++callIdCounter}`; try { const invocation = readManyFilesTool.build(toolArgs); diff --git a/packages/cli/src/ui/components/MainContent.test.tsx b/packages/cli/src/ui/components/MainContent.test.tsx index e0880e624c..df80d93f1b 100644 --- a/packages/cli/src/ui/components/MainContent.test.tsx +++ b/packages/cli/src/ui/components/MainContent.test.tsx @@ -96,7 +96,7 @@ describe('getToolGroupBorderAppearance', () => { }); it('inspects only the last pending tool_group item if current has no tools', () => { - const item = { type: 'tool_group' as const, tools: [], id: 1 }; + const item = { type: 'tool_group' as const, tools: [], id: -1 }; const pendingItems = [ { type: 'tool_group' as const, @@ -157,7 +157,7 @@ describe('getToolGroupBorderAppearance', () => { confirmationDetails: undefined, } as IndividualToolCallDisplay, ], - id: 1, + id: -1, }; const result = getToolGroupBorderAppearance( item, @@ -186,7 +186,7 @@ describe('getToolGroupBorderAppearance', () => { confirmationDetails: undefined, } as IndividualToolCallDisplay, ], - id: 1, + id: -1, }; const result = getToolGroupBorderAppearance( item, @@ -275,7 +275,7 @@ describe('getToolGroupBorderAppearance', () => { confirmationDetails: undefined, } as IndividualToolCallDisplay, ], - id: 1, + id: -1, }; const result = getToolGroupBorderAppearance( item, @@ -291,7 +291,7 @@ describe('getToolGroupBorderAppearance', () => { }); it('handles empty tools with active shell turn (isCurrentlyInShellTurn)', () => { - const item = { type: 'tool_group' as const, tools: [], id: 1 }; + const item = { type: 'tool_group' as const, tools: [], id: -1 }; // active shell turn const result = getToolGroupBorderAppearance( @@ -690,7 +690,7 @@ describe('MainContent', () => { pendingHistoryItems: [ { type: 'tool_group', - id: 1, + id: -1, tools: [ { callId: 'call_1', diff --git a/packages/cli/src/ui/components/MainContent.tsx b/packages/cli/src/ui/components/MainContent.tsx index d7e04bd351..8e0915a5cc 100644 --- a/packages/cli/src/ui/components/MainContent.tsx +++ b/packages/cli/src/ui/components/MainContent.tsx @@ -126,7 +126,7 @@ export const MainContent = () => { const pendingItems = useMemo( () => ( - + {pendingHistoryItems.map((item, i) => { const prevType = i === 0 @@ -139,12 +139,12 @@ export const MainContent = () => { return ( { ); })} {showConfirmationQueue && confirmingTool && ( - + )} ), diff --git a/packages/cli/src/ui/components/messages/DenseToolMessage.tsx b/packages/cli/src/ui/components/messages/DenseToolMessage.tsx index 2751f4fd2c..359045719e 100644 --- a/packages/cli/src/ui/components/messages/DenseToolMessage.tsx +++ b/packages/cli/src/ui/components/messages/DenseToolMessage.tsx @@ -519,12 +519,12 @@ export const DenseToolMessage: React.FC = (props) => { {description} {summary && ( - + {summary} )} {isAlternateBuffer && diff && ( - + [{isExpanded ? 'Hide Diff' : 'Show Diff'}] diff --git a/packages/cli/src/ui/hooks/useHistoryManager.test.ts b/packages/cli/src/ui/hooks/useHistoryManager.test.ts index 696f9d60c0..b657e5d4ba 100644 --- a/packages/cli/src/ui/hooks/useHistoryManager.test.ts +++ b/packages/cli/src/ui/hooks/useHistoryManager.test.ts @@ -39,6 +39,56 @@ describe('useHistoryManager', () => { expect(result.current.history[0].id).toBeGreaterThanOrEqual(timestamp); }); + it('should generate strictly increasing IDs even if baseTimestamp goes backwards', () => { + const { result } = renderHook(() => useHistory()); + const timestamp = 1000000; + const itemData: Omit = { type: 'info', text: 'First' }; + + let id1!: number; + let id2!: number; + + act(() => { + id1 = result.current.addItem(itemData, timestamp); + // Try to add with a smaller timestamp + id2 = result.current.addItem(itemData, timestamp - 500); + }); + + expect(id1).toBe(timestamp); + expect(id2).toBe(id1 + 1); + expect(result.current.history[1].id).toBe(id2); + }); + + it('should ensure new IDs start after existing IDs when resuming a session', () => { + const initialItems: HistoryItem[] = [ + { id: 5000, type: 'info', text: 'Existing' }, + ]; + const { result } = renderHook(() => useHistory({ initialItems })); + + let newId!: number; + act(() => { + // Try to add with a timestamp smaller than the highest existing ID + newId = result.current.addItem({ type: 'info', text: 'New' }, 2000); + }); + + expect(newId).toBe(5001); + expect(result.current.history[1].id).toBe(5001); + }); + + it('should update lastIdRef when loading new history', () => { + const { result } = renderHook(() => useHistory()); + + act(() => { + result.current.loadHistory([{ id: 8000, type: 'info', text: 'Loaded' }]); + }); + + let newId!: number; + act(() => { + newId = result.current.addItem({ type: 'info', text: 'New' }, 1000); + }); + + expect(newId).toBe(8001); + }); + it('should generate unique IDs for items added with the same base timestamp', () => { const { result } = renderHook(() => useHistory()); const timestamp = Date.now(); @@ -215,8 +265,8 @@ describe('useHistoryManager', () => { const after = Date.now(); expect(result.current.history).toHaveLength(1); - // ID should be >= before + 1 (since counter starts at 0 and increments to 1) - expect(result.current.history[0].id).toBeGreaterThanOrEqual(before + 1); + // ID should be >= before (since baseTimestamp defaults to Date.now()) + expect(result.current.history[0].id).toBeGreaterThanOrEqual(before); expect(result.current.history[0].id).toBeLessThanOrEqual(after + 1); }); diff --git a/packages/cli/src/ui/hooks/useHistoryManager.ts b/packages/cli/src/ui/hooks/useHistoryManager.ts index 93f7f01f28..c6ceabb920 100644 --- a/packages/cli/src/ui/hooks/useHistoryManager.ts +++ b/packages/cli/src/ui/hooks/useHistoryManager.ts @@ -42,16 +42,22 @@ export function useHistory({ initialItems?: HistoryItem[]; } = {}): UseHistoryManagerReturn { const [history, setHistory] = useState(initialItems); - const messageIdCounterRef = useRef(0); + const lastIdRef = useRef( + initialItems.reduce((max, item) => Math.max(max, item.id), 0), + ); - // Generates a unique message ID based on a timestamp and a counter. + // Generates a unique message ID based on a timestamp, ensuring it is always + // greater than any previously assigned ID. const getNextMessageId = useCallback((baseTimestamp: number): number => { - messageIdCounterRef.current += 1; - return baseTimestamp + messageIdCounterRef.current; + const nextId = Math.max(baseTimestamp, lastIdRef.current + 1); + lastIdRef.current = nextId; + return nextId; }, []); const loadHistory = useCallback((newHistory: HistoryItem[]) => { setHistory(newHistory); + const maxId = newHistory.reduce((max, item) => Math.max(max, item.id), 0); + lastIdRef.current = Math.max(lastIdRef.current, maxId); }, []); // Adds a new item to the history state with a unique ID. @@ -153,7 +159,7 @@ export function useHistory({ // Clears the entire history state and resets the ID counter. const clearItems = useCallback(() => { setHistory([]); - messageIdCounterRef.current = 0; + lastIdRef.current = 0; }, []); return useMemo(