Browse Source

fix: cancel auto-approval timeout when user starts typing (#9937)

Co-authored-by: Roo Code <[email protected]>
Co-authored-by: Matt Rubens <[email protected]>
roomote[bot] 1 month ago
parent
commit
d976a9b296

+ 22 - 6
src/core/task/Task.ts

@@ -278,6 +278,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
 	private askResponseText?: string
 	private askResponseText?: string
 	private askResponseImages?: string[]
 	private askResponseImages?: string[]
 	public lastMessageTs?: number
 	public lastMessageTs?: number
+	private autoApprovalTimeoutRef?: NodeJS.Timeout
 
 
 	// Tool Use
 	// Tool Use
 	consecutiveMistakeCount: number = 0
 	consecutiveMistakeCount: number = 0
@@ -1095,12 +1096,13 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
 		} else if (approval.decision === "deny") {
 		} else if (approval.decision === "deny") {
 			this.denyAsk()
 			this.denyAsk()
 		} else if (approval.decision === "timeout") {
 		} else if (approval.decision === "timeout") {
-			timeouts.push(
-				setTimeout(() => {
-					const { askResponse, text, images } = approval.fn()
-					this.handleWebviewAskResponse(askResponse, text, images)
-				}, approval.timeout),
-			)
+			// Store the auto-approval timeout so it can be cancelled if user interacts
+			this.autoApprovalTimeoutRef = setTimeout(() => {
+				const { askResponse, text, images } = approval.fn()
+				this.handleWebviewAskResponse(askResponse, text, images)
+				this.autoApprovalTimeoutRef = undefined
+			}, approval.timeout)
+			timeouts.push(this.autoApprovalTimeoutRef)
 		}
 		}
 
 
 		// The state is mutable if the message is complete and the task will
 		// The state is mutable if the message is complete and the task will
@@ -1209,6 +1211,9 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
 	}
 	}
 
 
 	handleWebviewAskResponse(askResponse: ClineAskResponse, text?: string, images?: string[]) {
 	handleWebviewAskResponse(askResponse: ClineAskResponse, text?: string, images?: string[]) {
+		// Clear any pending auto-approval timeout when user responds
+		this.cancelAutoApprovalTimeout()
+
 		this.askResponse = askResponse
 		this.askResponse = askResponse
 		this.askResponseText = text
 		this.askResponseText = text
 		this.askResponseImages = images
 		this.askResponseImages = images
@@ -1239,6 +1244,17 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
 		}
 		}
 	}
 	}
 
 
+	/**
+	 * Cancel any pending auto-approval timeout.
+	 * Called when user interacts (types, clicks buttons, etc.) to prevent the timeout from firing.
+	 */
+	public cancelAutoApprovalTimeout(): void {
+		if (this.autoApprovalTimeoutRef) {
+			clearTimeout(this.autoApprovalTimeoutRef)
+			this.autoApprovalTimeoutRef = undefined
+		}
+	}
+
 	public approveAsk({ text, images }: { text?: string; images?: string[] } = {}) {
 	public approveAsk({ text, images }: { text?: string; images?: string[] } = {}) {
 		this.handleWebviewAskResponse("yesButtonClicked", text, images)
 		this.handleWebviewAskResponse("yesButtonClicked", text, images)
 	}
 	}

+ 4 - 0
src/core/webview/webviewMessageHandler.ts

@@ -1107,6 +1107,10 @@ export const webviewMessageHandler = async (
 		case "cancelTask":
 		case "cancelTask":
 			await provider.cancelTask()
 			await provider.cancelTask()
 			break
 			break
+		case "cancelAutoApproval":
+			// Cancel any pending auto-approval timeout for the current task
+			provider.getCurrentTask()?.cancelAutoApprovalTimeout()
+			break
 		case "killBrowserSession":
 		case "killBrowserSession":
 			{
 			{
 				const task = provider.getCurrentTask()
 				const task = provider.getCurrentTask()

+ 1 - 0
src/shared/WebviewMessage.ts

@@ -68,6 +68,7 @@ export interface WebviewMessage {
 		| "openFile"
 		| "openFile"
 		| "openMention"
 		| "openMention"
 		| "cancelTask"
 		| "cancelTask"
+		| "cancelAutoApproval"
 		| "updateVSCodeSetting"
 		| "updateVSCodeSetting"
 		| "getVSCodeSetting"
 		| "getVSCodeSetting"
 		| "vsCodeSetting"
 		| "vsCodeSetting"

+ 3 - 0
webview-ui/src/components/chat/ChatRow.tsx

@@ -109,6 +109,7 @@ interface ChatRowProps {
 	onBatchFileResponse?: (response: { [key: string]: boolean }) => void
 	onBatchFileResponse?: (response: { [key: string]: boolean }) => void
 	onFollowUpUnmount?: () => void
 	onFollowUpUnmount?: () => void
 	isFollowUpAnswered?: boolean
 	isFollowUpAnswered?: boolean
+	isFollowUpAutoApprovalPaused?: boolean
 	editable?: boolean
 	editable?: boolean
 	hasCheckpoint?: boolean
 	hasCheckpoint?: boolean
 }
 }
@@ -162,6 +163,7 @@ export const ChatRowContent = ({
 	onFollowUpUnmount,
 	onFollowUpUnmount,
 	onBatchFileResponse,
 	onBatchFileResponse,
 	isFollowUpAnswered,
 	isFollowUpAnswered,
+	isFollowUpAutoApprovalPaused,
 }: ChatRowContentProps) => {
 }: ChatRowContentProps) => {
 	const { t, i18n } = useTranslation()
 	const { t, i18n } = useTranslation()
 
 
@@ -1544,6 +1546,7 @@ export const ChatRowContent = ({
 									ts={message?.ts}
 									ts={message?.ts}
 									onCancelAutoApproval={onFollowUpUnmount}
 									onCancelAutoApproval={onFollowUpUnmount}
 									isAnswered={isFollowUpAnswered}
 									isAnswered={isFollowUpAnswered}
+									isFollowUpAutoApprovalPaused={isFollowUpAutoApprovalPaused}
 								/>
 								/>
 							</div>
 							</div>
 						</>
 						</>

+ 16 - 0
webview-ui/src/components/chat/ChatView.tsx

@@ -189,6 +189,20 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
 		inputValueRef.current = inputValue
 		inputValueRef.current = inputValue
 	}, [inputValue])
 	}, [inputValue])
 
 
+	// Compute whether auto-approval is paused (user is typing in a followup)
+	const isFollowUpAutoApprovalPaused = useMemo(() => {
+		return !!(inputValue && inputValue.trim().length > 0 && clineAsk === "followup")
+	}, [inputValue, clineAsk])
+
+	// Cancel auto-approval timeout when user starts typing
+	useEffect(() => {
+		// Only send cancel if there's actual input (user is typing)
+		// and we have a pending follow-up question
+		if (isFollowUpAutoApprovalPaused) {
+			vscode.postMessage({ type: "cancelAutoApproval" })
+		}
+	}, [isFollowUpAutoApprovalPaused])
+
 	useEffect(() => {
 	useEffect(() => {
 		isMountedRef.current = true
 		isMountedRef.current = true
 		return () => {
 		return () => {
@@ -1272,6 +1286,7 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
 					onSuggestionClick={handleSuggestionClickInRow} // This was already stabilized
 					onSuggestionClick={handleSuggestionClickInRow} // This was already stabilized
 					onBatchFileResponse={handleBatchFileResponse}
 					onBatchFileResponse={handleBatchFileResponse}
 					isFollowUpAnswered={messageOrGroup.isAnswered === true || messageOrGroup.ts === currentFollowUpTs}
 					isFollowUpAnswered={messageOrGroup.isAnswered === true || messageOrGroup.ts === currentFollowUpTs}
+					isFollowUpAutoApprovalPaused={isFollowUpAutoApprovalPaused}
 					editable={
 					editable={
 						messageOrGroup.type === "ask" &&
 						messageOrGroup.type === "ask" &&
 						messageOrGroup.ask === "tool" &&
 						messageOrGroup.ask === "tool" &&
@@ -1304,6 +1319,7 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
 			handleSuggestionClickInRow,
 			handleSuggestionClickInRow,
 			handleBatchFileResponse,
 			handleBatchFileResponse,
 			currentFollowUpTs,
 			currentFollowUpTs,
+			isFollowUpAutoApprovalPaused,
 			alwaysAllowUpdateTodoList,
 			alwaysAllowUpdateTodoList,
 			enableButtons,
 			enableButtons,
 			primaryButtonText,
 			primaryButtonText,

+ 6 - 2
webview-ui/src/components/chat/FollowUpSuggest.tsx

@@ -17,6 +17,7 @@ interface FollowUpSuggestProps {
 	ts: number
 	ts: number
 	onCancelAutoApproval?: () => void
 	onCancelAutoApproval?: () => void
 	isAnswered?: boolean
 	isAnswered?: boolean
+	isFollowUpAutoApprovalPaused?: boolean
 }
 }
 
 
 export const FollowUpSuggest = ({
 export const FollowUpSuggest = ({
@@ -25,6 +26,7 @@ export const FollowUpSuggest = ({
 	ts = 1,
 	ts = 1,
 	onCancelAutoApproval,
 	onCancelAutoApproval,
 	isAnswered = false,
 	isAnswered = false,
+	isFollowUpAutoApprovalPaused = false,
 }: FollowUpSuggestProps) => {
 }: FollowUpSuggestProps) => {
 	const { autoApprovalEnabled, alwaysAllowFollowupQuestions, followupAutoApproveTimeoutMs } = useExtensionState()
 	const { autoApprovalEnabled, alwaysAllowFollowupQuestions, followupAutoApproveTimeoutMs } = useExtensionState()
 	const [countdown, setCountdown] = useState<number | null>(null)
 	const [countdown, setCountdown] = useState<number | null>(null)
@@ -34,13 +36,14 @@ export const FollowUpSuggest = ({
 	// Start countdown timer when auto-approval is enabled for follow-up questions
 	// Start countdown timer when auto-approval is enabled for follow-up questions
 	useEffect(() => {
 	useEffect(() => {
 		// Only start countdown if auto-approval is enabled for follow-up questions and no suggestion has been selected
 		// Only start countdown if auto-approval is enabled for follow-up questions and no suggestion has been selected
-		// Also stop countdown if the question has been answered
+		// Also stop countdown if the question has been answered or auto-approval is paused (user is typing)
 		if (
 		if (
 			autoApprovalEnabled &&
 			autoApprovalEnabled &&
 			alwaysAllowFollowupQuestions &&
 			alwaysAllowFollowupQuestions &&
 			suggestions.length > 0 &&
 			suggestions.length > 0 &&
 			!suggestionSelected &&
 			!suggestionSelected &&
-			!isAnswered
+			!isAnswered &&
+			!isFollowUpAutoApprovalPaused
 		) {
 		) {
 			// Start with the configured timeout in seconds
 			// Start with the configured timeout in seconds
 			const timeoutMs =
 			const timeoutMs =
@@ -80,6 +83,7 @@ export const FollowUpSuggest = ({
 		suggestionSelected,
 		suggestionSelected,
 		onCancelAutoApproval,
 		onCancelAutoApproval,
 		isAnswered,
 		isAnswered,
+		isFollowUpAutoApprovalPaused,
 	])
 	])
 	const handleSuggestionClick = useCallback(
 	const handleSuggestionClick = useCallback(
 		(suggestion: SuggestionItem, event: React.MouseEvent) => {
 		(suggestion: SuggestionItem, event: React.MouseEvent) => {

+ 180 - 0
webview-ui/src/components/chat/__tests__/FollowUpSuggest.spec.tsx

@@ -412,4 +412,184 @@ describe("FollowUpSuggest", () => {
 		// onSuggestionClick should NOT have been called (component doesn't auto-select)
 		// onSuggestionClick should NOT have been called (component doesn't auto-select)
 		expect(mockOnSuggestionClick).not.toHaveBeenCalled()
 		expect(mockOnSuggestionClick).not.toHaveBeenCalled()
 	})
 	})
+
+	describe("isFollowUpAutoApprovalPaused prop", () => {
+		it("should not display countdown timer when isFollowUpAutoApprovalPaused is true", () => {
+			renderWithTestProviders(
+				<FollowUpSuggest
+					suggestions={mockSuggestions}
+					onSuggestionClick={mockOnSuggestionClick}
+					ts={123}
+					onCancelAutoApproval={mockOnCancelAutoApproval}
+					isFollowUpAutoApprovalPaused={true}
+				/>,
+				defaultTestState,
+			)
+
+			// Should not show countdown when user is typing
+			expect(screen.queryByText(/\d+s/)).not.toBeInTheDocument()
+		})
+
+		it("should stop countdown when user starts typing (isFollowUpAutoApprovalPaused becomes true)", () => {
+			const { rerender } = renderWithTestProviders(
+				<FollowUpSuggest
+					suggestions={mockSuggestions}
+					onSuggestionClick={mockOnSuggestionClick}
+					ts={123}
+					onCancelAutoApproval={mockOnCancelAutoApproval}
+					isFollowUpAutoApprovalPaused={false}
+				/>,
+				defaultTestState,
+			)
+
+			// Initially should show countdown
+			expect(screen.getByText(/3s/)).toBeInTheDocument()
+
+			// Simulate user starting to type by setting isFollowUpAutoApprovalPaused to true
+			rerender(
+				<TestExtensionStateProvider value={defaultTestState}>
+					<TooltipProvider>
+						<FollowUpSuggest
+							suggestions={mockSuggestions}
+							onSuggestionClick={mockOnSuggestionClick}
+							ts={123}
+							onCancelAutoApproval={mockOnCancelAutoApproval}
+							isFollowUpAutoApprovalPaused={true}
+						/>
+					</TooltipProvider>
+				</TestExtensionStateProvider>,
+			)
+
+			// Countdown should be hidden immediately when user starts typing
+			expect(screen.queryByText(/\d+s/)).not.toBeInTheDocument()
+
+			// Advance timer to ensure countdown doesn't continue
+			vi.advanceTimersByTime(5000)
+
+			// onSuggestionClick should not have been called
+			expect(mockOnSuggestionClick).not.toHaveBeenCalled()
+
+			// Countdown should still not be visible
+			expect(screen.queryByText(/\d+s/)).not.toBeInTheDocument()
+		})
+
+		it("should resume countdown when user clears input (isFollowUpAutoApprovalPaused becomes false)", async () => {
+			const { rerender } = renderWithTestProviders(
+				<FollowUpSuggest
+					suggestions={mockSuggestions}
+					onSuggestionClick={mockOnSuggestionClick}
+					ts={123}
+					onCancelAutoApproval={mockOnCancelAutoApproval}
+					isFollowUpAutoApprovalPaused={true}
+				/>,
+				defaultTestState,
+			)
+
+			// Should not show countdown when paused
+			expect(screen.queryByText(/\d+s/)).not.toBeInTheDocument()
+
+			// Simulate user clearing input by setting isFollowUpAutoApprovalPaused to false
+			rerender(
+				<TestExtensionStateProvider value={defaultTestState}>
+					<TooltipProvider>
+						<FollowUpSuggest
+							suggestions={mockSuggestions}
+							onSuggestionClick={mockOnSuggestionClick}
+							ts={123}
+							onCancelAutoApproval={mockOnCancelAutoApproval}
+							isFollowUpAutoApprovalPaused={false}
+						/>
+					</TooltipProvider>
+				</TestExtensionStateProvider>,
+			)
+
+			// Countdown should resume from the full timeout
+			expect(screen.getByText(/3s/)).toBeInTheDocument()
+		})
+
+		it("should not show countdown when both isAnswered and isFollowUpAutoApprovalPaused are true", () => {
+			renderWithTestProviders(
+				<FollowUpSuggest
+					suggestions={mockSuggestions}
+					onSuggestionClick={mockOnSuggestionClick}
+					ts={123}
+					onCancelAutoApproval={mockOnCancelAutoApproval}
+					isAnswered={true}
+					isFollowUpAutoApprovalPaused={true}
+				/>,
+				defaultTestState,
+			)
+
+			// Should not show countdown
+			expect(screen.queryByText(/\d+s/)).not.toBeInTheDocument()
+		})
+
+		it("should handle pause during countdown progress", async () => {
+			const { rerender } = renderWithTestProviders(
+				<FollowUpSuggest
+					suggestions={mockSuggestions}
+					onSuggestionClick={mockOnSuggestionClick}
+					ts={123}
+					onCancelAutoApproval={mockOnCancelAutoApproval}
+					isFollowUpAutoApprovalPaused={false}
+				/>,
+				defaultTestState,
+			)
+
+			// Initially should show 3s
+			expect(screen.getByText(/3s/)).toBeInTheDocument()
+
+			// Advance timer by 1 second
+			await act(async () => {
+				vi.advanceTimersByTime(1000)
+			})
+
+			// Should show 2s
+			expect(screen.getByText(/2s/)).toBeInTheDocument()
+
+			// User starts typing (pause)
+			rerender(
+				<TestExtensionStateProvider value={defaultTestState}>
+					<TooltipProvider>
+						<FollowUpSuggest
+							suggestions={mockSuggestions}
+							onSuggestionClick={mockOnSuggestionClick}
+							ts={123}
+							onCancelAutoApproval={mockOnCancelAutoApproval}
+							isFollowUpAutoApprovalPaused={true}
+						/>
+					</TooltipProvider>
+				</TestExtensionStateProvider>,
+			)
+
+			// Countdown should be hidden
+			expect(screen.queryByText(/\d+s/)).not.toBeInTheDocument()
+
+			// Advance timer while paused
+			await act(async () => {
+				vi.advanceTimersByTime(2000)
+			})
+
+			// Countdown should still be hidden
+			expect(screen.queryByText(/\d+s/)).not.toBeInTheDocument()
+
+			// User clears input (unpause) - countdown should restart from full duration
+			rerender(
+				<TestExtensionStateProvider value={defaultTestState}>
+					<TooltipProvider>
+						<FollowUpSuggest
+							suggestions={mockSuggestions}
+							onSuggestionClick={mockOnSuggestionClick}
+							ts={123}
+							onCancelAutoApproval={mockOnCancelAutoApproval}
+							isFollowUpAutoApprovalPaused={false}
+						/>
+					</TooltipProvider>
+				</TestExtensionStateProvider>,
+			)
+
+			// Countdown should restart from full timeout (3s)
+			expect(screen.getByText(/3s/)).toBeInTheDocument()
+		})
+	})
 })
 })