From 4e3ba687a6bd37ba387edfc3db88467e47c7775f Mon Sep 17 00:00:00 2001 From: Brandon Keiji Date: Thu, 22 May 2025 06:00:36 +0000 Subject: [PATCH] fix: forward entire tool call confirmation object through useToolScheduler (#481) --- .../messages/ToolConfirmationMessage.tsx | 9 +---- packages/cli/src/ui/hooks/useToolScheduler.ts | 39 +++++++++---------- packages/server/src/tools/edit.ts | 1 + packages/server/src/tools/shell.ts | 1 + packages/server/src/tools/tools.ts | 14 +++---- packages/server/src/tools/write-file.ts | 1 + .../server/src/utils/nextSpeakerChecker.ts | 2 +- 7 files changed, 30 insertions(+), 37 deletions(-) diff --git a/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx b/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx index 19b1841a..0606856f 100644 --- a/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx +++ b/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx @@ -10,7 +10,6 @@ import { DiffRenderer } from './DiffRenderer.js'; import { Colors } from '../../colors.js'; import { ToolCallConfirmationDetails, - ToolEditConfirmationDetails, ToolConfirmationOutcome, ToolExecuteConfirmationDetails, } from '@gemini-code/server'; @@ -23,12 +22,6 @@ export interface ToolConfirmationMessageProps { confirmationDetails: ToolCallConfirmationDetails; } -function isEditDetails( - props: ToolCallConfirmationDetails, -): props is ToolEditConfirmationDetails { - return (props as ToolEditConfirmationDetails).fileName !== undefined; -} - export const ToolConfirmationMessage: React.FC< ToolConfirmationMessageProps > = ({ confirmationDetails }) => { @@ -49,7 +42,7 @@ export const ToolConfirmationMessage: React.FC< RadioSelectItem >(); - if (isEditDetails(confirmationDetails)) { + if (confirmationDetails.type === 'edit') { // Body content is now the DiffRenderer, passing filename to it // The bordered box is removed from here and handled within DiffRenderer bodyContent = ( diff --git a/packages/cli/src/ui/hooks/useToolScheduler.ts b/packages/cli/src/ui/hooks/useToolScheduler.ts index 2cb27141..fde632df 100644 --- a/packages/cli/src/ui/hooks/useToolScheduler.ts +++ b/packages/cli/src/ui/hooks/useToolScheduler.ts @@ -10,6 +10,7 @@ import { ToolCallResponseInfo, ToolConfirmationOutcome, Tool, + ToolCallConfirmationDetails, } from '@gemini-code/server'; import { Part } from '@google/genai'; import { useCallback, useEffect, useState } from 'react'; @@ -55,7 +56,7 @@ type WaitingToolCall = { status: 'awaiting_approval'; request: ToolCallRequestInfo; tool: Tool; - confirm: (outcome: ToolConfirmationOutcome) => Promise; + confirmationDetails: ToolCallConfirmationDetails; }; export type Status = ToolCall['status']; @@ -119,17 +120,20 @@ export function useToolScheduler( status: 'awaiting_approval', request: r, tool, - confirm: async (outcome) => { - await userApproval.onConfirm(outcome); - setToolCalls( - outcome === ToolConfirmationOutcome.Cancel - ? setStatus( - r.callId, - 'cancelled', - 'User did not allow tool call', - ) - : setStatus(r.callId, 'scheduled'), - ); + confirmationDetails: { + ...userApproval, + onConfirm: async (outcome) => { + await userApproval.onConfirm(outcome); + setToolCalls( + outcome === ToolConfirmationOutcome.Cancel + ? setStatus( + r.callId, + 'cancelled', + 'User did not allow tool call', + ) + : setStatus(r.callId, 'scheduled'), + ); + }, }, }; } @@ -249,7 +253,7 @@ function setStatus( function setStatus( targetCallId: string, status: 'awaiting_approval', - confirm: (t: ToolConfirmationOutcome) => Promise, + confirm: ToolCallConfirmationDetails, ): (t: ToolCall[]) => ToolCall[]; function setStatus( targetCallId: string, @@ -296,9 +300,7 @@ function setStatus( const next: WaitingToolCall = { ...t, status: 'awaiting_approval', - confirm: auxiliaryData as ( - o: ToolConfirmationOutcome, - ) => Promise, + confirmationDetails: auxiliaryData as ToolCallConfirmationDetails, }; return next; } @@ -426,10 +428,7 @@ export function mapToDisplay( description: t.tool.getDescription(t.request.args), resultDisplay: undefined, status: mapStatus(t.status), - confirmationDetails: { - title: t.request.name, - onConfirm: t.confirm, - }, + confirmationDetails: t.confirmationDetails, }; case 'executing': return { diff --git a/packages/server/src/tools/edit.ts b/packages/server/src/tools/edit.ts index 7b327778..9301d5e8 100644 --- a/packages/server/src/tools/edit.ts +++ b/packages/server/src/tools/edit.ts @@ -291,6 +291,7 @@ Expectation for parameters: { context: 3 }, ); const confirmationDetails: ToolEditConfirmationDetails = { + type: 'edit', title: `Confirm Edit: ${shortenPath(makeRelative(params.file_path, this.rootDirectory))}`, fileName, fileDiff, diff --git a/packages/server/src/tools/shell.ts b/packages/server/src/tools/shell.ts index 6cf49a7b..dd8f8639 100644 --- a/packages/server/src/tools/shell.ts +++ b/packages/server/src/tools/shell.ts @@ -107,6 +107,7 @@ export class ShellTool extends BaseTool { return false; // already approved and whitelisted } const confirmationDetails: ToolExecuteConfirmationDetails = { + type: 'exec', title: 'Confirm Shell Command', command: params.command, rootCommand, diff --git a/packages/server/src/tools/tools.ts b/packages/server/src/tools/tools.ts index 58209166..2f5a4095 100644 --- a/packages/server/src/tools/tools.ts +++ b/packages/server/src/tools/tools.ts @@ -171,25 +171,23 @@ export interface FileDiff { fileName: string; } -export interface ToolCallConfirmationDetailsDefault { +export interface ToolEditConfirmationDetails { + type: 'edit'; title: string; onConfirm: (outcome: ToolConfirmationOutcome) => Promise; -} - -export interface ToolEditConfirmationDetails - extends ToolCallConfirmationDetailsDefault { fileName: string; fileDiff: string; } -export interface ToolExecuteConfirmationDetails - extends ToolCallConfirmationDetailsDefault { +export interface ToolExecuteConfirmationDetails { + type: 'exec'; + title: string; + onConfirm: (outcome: ToolConfirmationOutcome) => Promise; command: string; rootCommand: string; } export type ToolCallConfirmationDetails = - | ToolCallConfirmationDetailsDefault | ToolEditConfirmationDetails | ToolExecuteConfirmationDetails; diff --git a/packages/server/src/tools/write-file.ts b/packages/server/src/tools/write-file.ts index c9f95ea9..1b6b6dc8 100644 --- a/packages/server/src/tools/write-file.ts +++ b/packages/server/src/tools/write-file.ts @@ -158,6 +158,7 @@ export class WriteFileTool extends BaseTool { ); const confirmationDetails: ToolEditConfirmationDetails = { + type: 'edit', title: `Confirm Write: ${shortenPath(relativePath)}`, fileName, fileDiff, diff --git a/packages/server/src/utils/nextSpeakerChecker.ts b/packages/server/src/utils/nextSpeakerChecker.ts index a7818713..5eb0c512 100644 --- a/packages/server/src/utils/nextSpeakerChecker.ts +++ b/packages/server/src/utils/nextSpeakerChecker.ts @@ -94,7 +94,7 @@ export async function checkNextSpeaker( return null; } catch (error) { console.warn( - 'Failed to talk to Gemiin endpoint when seeing if conversation should continue.', + 'Failed to talk to Gemini endpoint when seeing if conversation should continue.', error, ); return null;