Skip to content

Commit c58b895

Browse files
authored
feat(followup): add timeout management for follow-up auto-approval (#654)
1 parent 0e76e48 commit c58b895

File tree

7 files changed

+118
-86
lines changed

7 files changed

+118
-86
lines changed

.kilocode/mcp.json

Lines changed: 0 additions & 28 deletions
This file was deleted.

src/core/auto-approval/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ export type CheckAutoApprovalResult =
3838
| {
3939
decision: "timeout"
4040
timeout: number
41+
askType?: string
4142
fn: () => { askResponse: ClineAskResponse; text?: string; images?: string[] }
4243
}
4344

@@ -73,6 +74,7 @@ export async function checkAutoApproval({
7374
return {
7475
decision: "timeout",
7576
timeout: state.followupAutoApproveTimeoutMs,
77+
askType: ask,
7678
fn: () => ({ askResponse: "messageResponse", text: suggestion.answer }),
7779
}
7880
} else {

src/core/task/Task.ts

Lines changed: 72 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,9 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
161161

162162
todoList?: TodoItem[]
163163

164+
private readonly timeoutMap = new Map<number, NodeJS.Timeout>()
165+
private nextTimeoutId = 0
166+
164167
readonly rootTask: Task | undefined = undefined
165168
readonly parentTask: Task | undefined = undefined
166169
readonly taskNumber: number
@@ -855,8 +858,6 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
855858
await this.addToClineMessages({ ts: askTs, type: "ask", ask: type, text, isProtected })
856859
}
857860

858-
let timeouts: NodeJS.Timeout[] = []
859-
860861
// Automatically approve if the ask according to the user's settings.
861862
const provider = this.providerRef.deref()
862863
const state = provider ? await provider.getState() : undefined
@@ -867,12 +868,22 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
867868
} else if (approval.decision === "deny") {
868869
this.denyAsk()
869870
} else if (approval.decision === "timeout") {
870-
timeouts.push(
871-
setTimeout(() => {
872-
const { askResponse, text, images } = approval.fn()
873-
this.handleWebviewAskResponse(askResponse, text, images)
874-
}, approval.timeout),
875-
)
871+
const timeoutId = this.nextTimeoutId++
872+
873+
const timer = setTimeout(() => {
874+
const { askResponse, text, images } = approval.fn()
875+
this.handleWebviewAskResponse(askResponse, text, images)
876+
this.timeoutMap.delete(timeoutId)
877+
}, approval.timeout)
878+
879+
this.timeoutMap.set(timeoutId, timer)
880+
881+
if (approval.askType === "followup") {
882+
provider?.postMessageToWebview({
883+
type: "zgsmFollowupClearTimeout",
884+
value: timeoutId,
885+
})
886+
}
876887
}
877888

878889
// The state is mutable if the message is complete and the task will
@@ -891,47 +902,53 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
891902
const statusMutationTimeout = 2_000
892903

893904
if (isInteractiveAsk(type)) {
894-
timeouts.push(
895-
setTimeout(() => {
896-
const message = this.findMessageByTimestamp(askTs)
905+
const timeoutId = this.nextTimeoutId++
906+
const timer = setTimeout(() => {
907+
const message = this.findMessageByTimestamp(askTs)
908+
909+
if (message) {
910+
this.interactiveAsk = message
911+
this.emit(RooCodeEventName.TaskInteractive, this.taskId)
912+
provider?.postMessageToWebview({ type: "interactionRequired" })
913+
}
914+
this.timeoutMap.delete(timeoutId)
915+
}, statusMutationTimeout)
897916

898-
if (message) {
899-
this.interactiveAsk = message
900-
this.emit(RooCodeEventName.TaskInteractive, this.taskId)
901-
provider?.postMessageToWebview({ type: "interactionRequired" })
902-
}
903-
}, statusMutationTimeout),
904-
)
917+
this.timeoutMap.set(timeoutId, timer)
905918
} else if (isResumableAsk(type)) {
906-
timeouts.push(
907-
setTimeout(() => {
908-
const message = this.findMessageByTimestamp(askTs)
919+
const timeoutId = this.nextTimeoutId++
920+
const timer = setTimeout(() => {
921+
const message = this.findMessageByTimestamp(askTs)
909922

910-
if (message) {
911-
this.resumableAsk = message
912-
this.emit(RooCodeEventName.TaskResumable, this.taskId)
913-
}
914-
}, statusMutationTimeout),
915-
)
923+
if (message) {
924+
this.resumableAsk = message
925+
this.emit(RooCodeEventName.TaskResumable, this.taskId)
926+
}
927+
this.timeoutMap.delete(timeoutId)
928+
}, statusMutationTimeout)
929+
930+
this.timeoutMap.set(timeoutId, timer)
916931
} else if (isIdleAsk(type)) {
917-
timeouts.push(
918-
setTimeout(() => {
919-
const message = this.findMessageByTimestamp(askTs)
932+
const timeoutId = this.nextTimeoutId++
933+
const timer = setTimeout(() => {
934+
const message = this.findMessageByTimestamp(askTs)
920935

921-
if (message) {
922-
this.idleAsk = message
923-
this.emit(RooCodeEventName.TaskIdle, this.taskId)
924-
}
925-
}, statusMutationTimeout),
926-
)
936+
if (message) {
937+
this.idleAsk = message
938+
this.emit(RooCodeEventName.TaskIdle, this.taskId)
939+
}
940+
this.timeoutMap.delete(timeoutId)
941+
}, statusMutationTimeout)
942+
943+
this.timeoutMap.set(timeoutId, timer)
927944
}
928945
} else if (isMessageQueued) {
929946
console.log(`Task#ask: will process message queue -> type: ${type}`)
930947

931948
const message = this.messageQueueService.dequeueMessage()
932949

933950
if (message) {
934-
// Check if this is a tool approval ask that needs to be handled.
951+
// Check if this is a tool approval ask Pthat needs to be handled.
935952
if (
936953
type === "tool" ||
937954
type === "command" ||
@@ -971,7 +988,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
971988
this.askResponseImages = undefined
972989

973990
// Cancel the timeouts if they are still running.
974-
timeouts.forEach((timeout) => clearTimeout(timeout))
991+
this.clearAllAutoApprovalTimeouts()
975992

976993
// Switch back to an active state.
977994
if (this.idleAsk || this.resumableAsk || this.interactiveAsk) {
@@ -985,6 +1002,23 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
9851002
return result
9861003
}
9871004

1005+
public clearAutoApprovalTimeout(timeoutId: number): boolean {
1006+
const timer = this.timeoutMap.get(timeoutId)
1007+
if (timer) {
1008+
clearTimeout(timer)
1009+
this.timeoutMap.delete(timeoutId)
1010+
return true
1011+
}
1012+
return false
1013+
}
1014+
1015+
public clearAllAutoApprovalTimeouts(): void {
1016+
for (const [timeoutId, timer] of this.timeoutMap) {
1017+
clearTimeout(timer)
1018+
}
1019+
this.timeoutMap.clear()
1020+
}
1021+
9881022
public setMessageResponse(text: string, images?: string[]) {
9891023
this.handleWebviewAskResponse("messageResponse", text, images)
9901024
}

src/core/webview/webviewMessageHandler.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3116,6 +3116,16 @@ export const webviewMessageHandler = async (
31163116
}
31173117
break
31183118
}
3119+
case "zgsmFollowupClearTimeout": {
3120+
const currentTask = provider?.getCurrentTask()
3121+
if (currentTask && typeof message.value === "number") {
3122+
const cleared = currentTask.clearAutoApprovalTimeout(message.value)
3123+
if (!cleared) {
3124+
provider?.log(`Failed to clear timeout with ID: ${message.value}`, "info")
3125+
}
3126+
}
3127+
break
3128+
}
31193129
case "zgsmRebuildCodebaseIndex": {
31203130
try {
31213131
const { apiConfiguration } = await provider.getState()

src/shared/ExtensionMessage.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@ export interface ExtensionMessage {
8989
| "zgsmInviteCode"
9090
| "zgsmNotices"
9191
| "settingsUpdated"
92+
| "zgsmFollowupClearTimeout"
9293
// zgsm
9394
| "ollamaModels"
9495
| "lmStudioModels"

src/shared/WebviewMessage.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,7 @@ export interface WebviewMessage {
159159
| "fetchZgsmQuotaInfo"
160160
| "fetchZgsmInviteCode"
161161
| "fixCodebase"
162+
| "zgsmFollowupClearTimeout"
162163
// zgsm
163164
| "shareTaskSuccess"
164165
| "exportMode"

webview-ui/src/components/chat/ChatView.tsx

Lines changed: 32 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -183,8 +183,9 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
183183
ttl: 1000 * 60 * 5,
184184
}),
185185
)
186-
const autoApproveTimeoutRef = useRef<NodeJS.Timeout | null>(null)
187-
const userRespondedRef = useRef<boolean>(false)
186+
// const autoApproveTimeoutRef = useRef<NodeJS.Timeout | null>(null)
187+
const followUpAutoApproveTimeoutRef = useRef<number | undefined>()
188+
// const userRespondedRef = useRef<boolean>(false)
188189
const [currentFollowUpTs, setCurrentFollowUpTs] = useState<number | null>(null)
189190

190191
const clineAskRef = useRef(clineAsk)
@@ -265,7 +266,7 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
265266
switch (lastMessage.type) {
266267
case "ask":
267268
// Reset user response flag when a new ask arrives to allow auto-approval
268-
userRespondedRef.current = false
269+
// userRespondedRef.current = false
269270
const isPartial = lastMessage.partial === true
270271
switch (lastMessage.ask) {
271272
case "api_req_failed":
@@ -436,12 +437,12 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
436437
// Note: sendingDisabled is not reset here as it's managed by message effects
437438

438439
// Clear any pending auto-approval timeout from previous task
439-
if (autoApproveTimeoutRef.current) {
440-
clearTimeout(autoApproveTimeoutRef.current)
441-
autoApproveTimeoutRef.current = null
442-
}
440+
// if (autoApproveTimeoutRef.current) {
441+
// clearTimeout(autoApproveTimeoutRef.current)
442+
// autoApproveTimeoutRef.current = null
443+
// }
443444
// Reset user response flag for new task
444-
userRespondedRef.current = false
445+
// userRespondedRef.current = false
445446
}, [task?.ts])
446447

447448
useEffect(() => {
@@ -530,12 +531,12 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
530531

531532
const handleChatReset = useCallback(() => {
532533
// Clear any pending auto-approval timeout
533-
if (autoApproveTimeoutRef.current) {
534-
clearTimeout(autoApproveTimeoutRef.current)
535-
autoApproveTimeoutRef.current = null
536-
}
534+
// if (autoApproveTimeoutRef.current) {
535+
// clearTimeout(autoApproveTimeoutRef.current)
536+
// autoApproveTimeoutRef.current = null
537+
// }
537538
// Reset user response flag for new message
538-
userRespondedRef.current = false
539+
// userRespondedRef.current = false
539540

540541
// Only reset message-specific state, preserving mode.
541542
setInputValue("")
@@ -578,7 +579,7 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
578579
}
579580

580581
// Mark that user has responded - this prevents any pending auto-approvals.
581-
userRespondedRef.current = true
582+
// userRespondedRef.current = true
582583

583584
if (messagesRef.current.length === 0) {
584585
vscode.postMessage({ type: "newTask", text, images, values: { chatType } })
@@ -664,7 +665,7 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
664665
const handlePrimaryButtonClick = useCallback(
665666
(text?: string, images?: string[]) => {
666667
// Mark that user has responded
667-
userRespondedRef.current = true
668+
// userRespondedRef.current = true
668669

669670
const trimmedInput = text?.trim()
670671

@@ -714,7 +715,7 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
714715
const handleSecondaryButtonClick = useCallback(
715716
(text?: string, images?: string[]) => {
716717
// Mark that user has responded
717-
userRespondedRef.current = true
718+
// userRespondedRef.current = true
718719

719720
const trimmedInput = text?.trim()
720721

@@ -772,6 +773,10 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
772773
const message: ExtensionMessage = e.data
773774

774775
switch (message.type) {
776+
case "zgsmFollowupClearTimeout": {
777+
followUpAutoApproveTimeoutRef.current = message.value
778+
break
779+
}
775780
case "action":
776781
switch (message.action!) {
777782
case "didBecomeVisible":
@@ -1223,9 +1228,9 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
12231228
const handleSuggestionClickInRow = useCallback(
12241229
(suggestion: SuggestionItem, event?: React.MouseEvent) => {
12251230
// Mark that user has responded if this is a manual click (not auto-approval)
1226-
if (event) {
1227-
userRespondedRef.current = true
1228-
}
1231+
// if (event) {
1232+
// userRespondedRef.current = true
1233+
// }
12291234

12301235
// Mark the current follow-up question as answered when a suggestion is clicked
12311236
if (clineAsk === "followup" && !event?.shiftKey) {
@@ -1267,7 +1272,14 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
12671272
// Handler for when FollowUpSuggest component unmounts
12681273
const handleFollowUpUnmount = useCallback(() => {
12691274
// Mark that user has responded
1270-
userRespondedRef.current = true
1275+
// userRespondedRef.current = true
1276+
if (Number.isInteger(followUpAutoApproveTimeoutRef.current)) {
1277+
vscode.postMessage({
1278+
type: "zgsmFollowupClearTimeout",
1279+
value: followUpAutoApproveTimeoutRef.current,
1280+
})
1281+
}
1282+
followUpAutoApproveTimeoutRef.current = undefined
12711283
}, [])
12721284
const shouldHighlight = useCallback(
12731285
(messageOrGroup?: ClineMessage, searchResults: SearchResult[] = [], showSearch?: boolean) => {

0 commit comments

Comments
 (0)