Skip to content

Commit a6982ab

Browse files
hannesrudolphclaude
andcommitted
Refactor workflow detection logic and add comprehensive test coverage
- Extract duplicated automated workflow detection into shared utility - Add 23 comprehensive unit tests covering all edge cases and scenarios - Add detailed JSDoc documentation with examples and issue references - Improve code maintainability and eliminate duplication Addresses PR review feedback for issue #4574. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
1 parent 1096793 commit a6982ab

File tree

4 files changed

+449
-29
lines changed

4 files changed

+449
-29
lines changed

src/core/mentions/index.ts

Lines changed: 5 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import { getCommitInfo, getWorkingState } from "../../utils/git"
1010
import { getWorkspacePath } from "../../utils/path"
1111

1212
import { openFile } from "../../integrations/misc/open-file"
13-
import { ClineProvider } from "../webview/ClineProvider"
13+
import { isInAutomatedWorkflowFromVisibleProvider } from "../../utils/workflow-detection"
1414
import { extractTextFromFile } from "../../integrations/misc/extract-text"
1515
import { diagnosticsToProblemsString } from "../../integrations/diagnostics"
1616

@@ -37,21 +37,10 @@ export async function openMention(mention?: string): Promise<void> {
3737
if (mention.endsWith("/")) {
3838
vscode.commands.executeCommand("revealInExplorer", vscode.Uri.file(absPath))
3939
} else {
40-
// Check if we're in an automated workflow when opening file mentions
41-
const visibleProvider = ClineProvider.getVisibleInstance()
42-
const currentCline = visibleProvider?.getCurrentCline()
43-
const autoApprovalEnabled = visibleProvider?.contextProxy.getValue("autoApprovalEnabled")
44-
45-
// Detect automated workflow: AI is actively processing, streaming, or auto-approval is enabled
46-
// and there's an active task that hasn't completed reading/processing
47-
const isInAutomatedWorkflow =
48-
currentCline &&
49-
(currentCline.isStreaming ||
50-
currentCline.isWaitingForFirstChunk ||
51-
(autoApprovalEnabled && !currentCline.didCompleteReadingStream) ||
52-
(autoApprovalEnabled && currentCline.presentAssistantMessageLocked))
53-
54-
openFile(absPath, { preserveFocus: !!isInAutomatedWorkflow })
40+
// Check if we're in an automated workflow to preserve chat focus during AI processing
41+
const shouldPreserveFocus = isInAutomatedWorkflowFromVisibleProvider()
42+
43+
openFile(absPath, { preserveFocus: shouldPreserveFocus })
5544
}
5645
} else if (mention === "problems") {
5746
vscode.commands.executeCommand("workbench.actions.view.problems")

src/core/webview/webviewMessageHandler.ts

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ import { getModels, flushModels } from "../../api/providers/fetchers/modelCache"
3939
import { GetModelsOptions } from "../../shared/api"
4040
import { generateSystemPrompt } from "./generateSystemPrompt"
4141
import { getCommand } from "../../utils/commands"
42+
import { isInAutomatedWorkflowFromProvider } from "../../utils/workflow-detection"
4243

4344
const ALLOWED_VSCODE_SETTINGS = new Set(["terminal.integrated.inheritEnv"])
4445

@@ -426,22 +427,12 @@ export const webviewMessageHandler = async (
426427
openImage(message.text!)
427428
break
428429
case "openFile":
429-
// Check if we're in an automated workflow (AI is streaming/processing)
430-
const currentCline = provider.getCurrentCline()
431-
const autoApprovalEnabled = getGlobalState("autoApprovalEnabled")
432-
433-
// Detect automated workflow: AI is actively processing, streaming, or auto-approval is enabled
434-
// and there's an active task that hasn't completed reading/processing
435-
const isInAutomatedWorkflow =
436-
currentCline &&
437-
(currentCline.isStreaming ||
438-
currentCline.isWaitingForFirstChunk ||
439-
(autoApprovalEnabled && !currentCline.didCompleteReadingStream) ||
440-
(autoApprovalEnabled && currentCline.presentAssistantMessageLocked))
430+
// Check if we're in an automated workflow to preserve chat focus during AI processing
431+
const shouldPreserveFocus = isInAutomatedWorkflowFromProvider(provider)
441432

442433
openFile(message.text!, {
443434
...(message.values as { create?: boolean; content?: string; line?: number }),
444-
preserveFocus: !!isInAutomatedWorkflow,
435+
preserveFocus: shouldPreserveFocus,
445436
})
446437
break
447438
case "openMention":
Lines changed: 322 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,322 @@
1+
// Mock ClineProvider first
2+
const mockGetVisibleInstance = jest.fn()
3+
jest.mock("../../core/webview/ClineProvider", () => ({
4+
ClineProvider: {
5+
getVisibleInstance: mockGetVisibleInstance,
6+
},
7+
}))
8+
9+
import {
10+
isInAutomatedWorkflow,
11+
isInAutomatedWorkflowFromProvider,
12+
isInAutomatedWorkflowFromVisibleProvider,
13+
} from "../workflow-detection"
14+
import type { Task } from "../../core/task/Task"
15+
import type { ClineProvider } from "../../core/webview/ClineProvider"
16+
17+
// Mock ClineProvider
18+
const mockProvider = {
19+
getCurrentCline: jest.fn(),
20+
contextProxy: {
21+
getValue: jest.fn(),
22+
},
23+
} as unknown as ClineProvider
24+
25+
const mockVisibleProvider = {
26+
getCurrentCline: jest.fn(),
27+
contextProxy: {
28+
getValue: jest.fn(),
29+
},
30+
} as unknown as ClineProvider
31+
32+
describe("workflow-detection", () => {
33+
beforeEach(() => {
34+
jest.clearAllMocks()
35+
})
36+
37+
describe("isInAutomatedWorkflow", () => {
38+
it("should return false when currentTask is null", () => {
39+
expect(isInAutomatedWorkflow(null, false)).toBe(false)
40+
expect(isInAutomatedWorkflow(null, true)).toBe(false)
41+
})
42+
43+
it("should return false when currentTask is undefined", () => {
44+
expect(isInAutomatedWorkflow(undefined, false)).toBe(false)
45+
expect(isInAutomatedWorkflow(undefined, true)).toBe(false)
46+
})
47+
48+
it("should return true when task is streaming", () => {
49+
const streamingTask = {
50+
isStreaming: true,
51+
isWaitingForFirstChunk: false,
52+
didCompleteReadingStream: true,
53+
presentAssistantMessageLocked: false,
54+
} as Task
55+
56+
expect(isInAutomatedWorkflow(streamingTask, false)).toBe(true)
57+
expect(isInAutomatedWorkflow(streamingTask, true)).toBe(true)
58+
})
59+
60+
it("should return true when task is waiting for first chunk", () => {
61+
const waitingTask = {
62+
isStreaming: false,
63+
isWaitingForFirstChunk: true,
64+
didCompleteReadingStream: true,
65+
presentAssistantMessageLocked: false,
66+
} as Task
67+
68+
expect(isInAutomatedWorkflow(waitingTask, false)).toBe(true)
69+
expect(isInAutomatedWorkflow(waitingTask, true)).toBe(true)
70+
})
71+
72+
it("should return true when auto-approval is enabled and stream not completed", () => {
73+
const incompleteStreamTask = {
74+
isStreaming: false,
75+
isWaitingForFirstChunk: false,
76+
didCompleteReadingStream: false,
77+
presentAssistantMessageLocked: false,
78+
} as Task
79+
80+
expect(isInAutomatedWorkflow(incompleteStreamTask, false)).toBe(false)
81+
expect(isInAutomatedWorkflow(incompleteStreamTask, true)).toBe(true)
82+
})
83+
84+
it("should return true when auto-approval is enabled and assistant message is locked", () => {
85+
const lockedMessageTask = {
86+
isStreaming: false,
87+
isWaitingForFirstChunk: false,
88+
didCompleteReadingStream: true,
89+
presentAssistantMessageLocked: true,
90+
} as Task
91+
92+
expect(isInAutomatedWorkflow(lockedMessageTask, false)).toBe(false)
93+
expect(isInAutomatedWorkflow(lockedMessageTask, true)).toBe(true)
94+
})
95+
96+
it("should return false when task is idle and auto-approval is disabled", () => {
97+
const idleTask = {
98+
isStreaming: false,
99+
isWaitingForFirstChunk: false,
100+
didCompleteReadingStream: true,
101+
presentAssistantMessageLocked: false,
102+
} as Task
103+
104+
expect(isInAutomatedWorkflow(idleTask, false)).toBe(false)
105+
})
106+
107+
it("should return false when task is idle even with auto-approval enabled", () => {
108+
const idleTask = {
109+
isStreaming: false,
110+
isWaitingForFirstChunk: false,
111+
didCompleteReadingStream: true,
112+
presentAssistantMessageLocked: false,
113+
} as Task
114+
115+
expect(isInAutomatedWorkflow(idleTask, true)).toBe(false)
116+
})
117+
118+
it("should return true for multiple simultaneous conditions", () => {
119+
const busyTask = {
120+
isStreaming: true,
121+
isWaitingForFirstChunk: true,
122+
didCompleteReadingStream: false,
123+
presentAssistantMessageLocked: true,
124+
} as Task
125+
126+
expect(isInAutomatedWorkflow(busyTask, false)).toBe(true)
127+
expect(isInAutomatedWorkflow(busyTask, true)).toBe(true)
128+
})
129+
130+
it("should handle tasks with missing properties gracefully", () => {
131+
const partialTask = {} as Task
132+
133+
expect(isInAutomatedWorkflow(partialTask, false)).toBe(false)
134+
expect(isInAutomatedWorkflow(partialTask, true)).toBe(false)
135+
})
136+
})
137+
138+
describe("isInAutomatedWorkflowFromProvider", () => {
139+
it("should return false when no current task", () => {
140+
mockProvider.getCurrentCline = jest.fn().mockReturnValue(null)
141+
mockProvider.contextProxy.getValue = jest.fn().mockReturnValue(false)
142+
143+
expect(isInAutomatedWorkflowFromProvider(mockProvider)).toBe(false)
144+
})
145+
146+
it("should return true when task is streaming", () => {
147+
const streamingTask = {
148+
isStreaming: true,
149+
isWaitingForFirstChunk: false,
150+
didCompleteReadingStream: true,
151+
presentAssistantMessageLocked: false,
152+
} as Task
153+
154+
mockProvider.getCurrentCline = jest.fn().mockReturnValue(streamingTask)
155+
mockProvider.contextProxy.getValue = jest.fn().mockReturnValue(false)
156+
157+
expect(isInAutomatedWorkflowFromProvider(mockProvider)).toBe(true)
158+
})
159+
160+
it("should return true when auto-approval is enabled and conditions are met", () => {
161+
const incompleteTask = {
162+
isStreaming: false,
163+
isWaitingForFirstChunk: false,
164+
didCompleteReadingStream: false,
165+
presentAssistantMessageLocked: false,
166+
} as Task
167+
168+
mockProvider.getCurrentCline = jest.fn().mockReturnValue(incompleteTask)
169+
mockProvider.contextProxy.getValue = jest.fn().mockReturnValue(true)
170+
171+
expect(isInAutomatedWorkflowFromProvider(mockProvider)).toBe(true)
172+
})
173+
174+
it("should return false when task is idle and auto-approval is disabled", () => {
175+
const idleTask = {
176+
isStreaming: false,
177+
isWaitingForFirstChunk: false,
178+
didCompleteReadingStream: true,
179+
presentAssistantMessageLocked: false,
180+
} as Task
181+
182+
mockProvider.getCurrentCline = jest.fn().mockReturnValue(idleTask)
183+
mockProvider.contextProxy.getValue = jest.fn().mockReturnValue(false)
184+
185+
expect(isInAutomatedWorkflowFromProvider(mockProvider)).toBe(false)
186+
})
187+
188+
it("should handle undefined values from contextProxy", () => {
189+
const streamingTask = {
190+
isStreaming: true,
191+
isWaitingForFirstChunk: false,
192+
didCompleteReadingStream: true,
193+
presentAssistantMessageLocked: false,
194+
} as Task
195+
196+
mockProvider.getCurrentCline = jest.fn().mockReturnValue(streamingTask)
197+
mockProvider.contextProxy.getValue = jest.fn().mockReturnValue(undefined)
198+
199+
// Should still return true because isStreaming is true, regardless of autoApprovalEnabled value
200+
expect(isInAutomatedWorkflowFromProvider(mockProvider)).toBe(true)
201+
})
202+
})
203+
204+
describe("isInAutomatedWorkflowFromVisibleProvider", () => {
205+
it("should return false when no visible provider", () => {
206+
mockGetVisibleInstance.mockReturnValue(null)
207+
208+
expect(isInAutomatedWorkflowFromVisibleProvider()).toBe(false)
209+
})
210+
211+
it("should return false when visible provider has no current task", () => {
212+
mockVisibleProvider.getCurrentCline = jest.fn().mockReturnValue(null)
213+
mockVisibleProvider.contextProxy.getValue = jest.fn().mockReturnValue(false)
214+
mockGetVisibleInstance.mockReturnValue(mockVisibleProvider)
215+
216+
expect(isInAutomatedWorkflowFromVisibleProvider()).toBe(false)
217+
})
218+
219+
it("should return true when visible provider has streaming task", () => {
220+
const streamingTask = {
221+
isStreaming: true,
222+
isWaitingForFirstChunk: false,
223+
didCompleteReadingStream: true,
224+
presentAssistantMessageLocked: false,
225+
} as Task
226+
227+
mockVisibleProvider.getCurrentCline = jest.fn().mockReturnValue(streamingTask)
228+
mockVisibleProvider.contextProxy.getValue = jest.fn().mockReturnValue(false)
229+
mockGetVisibleInstance.mockReturnValue(mockVisibleProvider)
230+
231+
expect(isInAutomatedWorkflowFromVisibleProvider()).toBe(true)
232+
})
233+
234+
it("should return true when visible provider has auto-approval enabled with incomplete task", () => {
235+
const incompleteTask = {
236+
isStreaming: false,
237+
isWaitingForFirstChunk: false,
238+
didCompleteReadingStream: false,
239+
presentAssistantMessageLocked: false,
240+
} as Task
241+
242+
mockVisibleProvider.getCurrentCline = jest.fn().mockReturnValue(incompleteTask)
243+
mockVisibleProvider.contextProxy.getValue = jest.fn().mockReturnValue(true)
244+
mockGetVisibleInstance.mockReturnValue(mockVisibleProvider)
245+
246+
expect(isInAutomatedWorkflowFromVisibleProvider()).toBe(true)
247+
})
248+
249+
it("should return false when visible provider has idle task and no auto-approval", () => {
250+
const idleTask = {
251+
isStreaming: false,
252+
isWaitingForFirstChunk: false,
253+
didCompleteReadingStream: true,
254+
presentAssistantMessageLocked: false,
255+
} as Task
256+
257+
mockVisibleProvider.getCurrentCline = jest.fn().mockReturnValue(idleTask)
258+
mockVisibleProvider.contextProxy.getValue = jest.fn().mockReturnValue(false)
259+
mockGetVisibleInstance.mockReturnValue(mockVisibleProvider)
260+
261+
expect(isInAutomatedWorkflowFromVisibleProvider()).toBe(false)
262+
})
263+
})
264+
265+
describe("edge cases and type safety", () => {
266+
it("should handle boolean coercion correctly", () => {
267+
// Test that the double negation (!!) in the original function works as expected
268+
const truthyTask = {
269+
isStreaming: true,
270+
} as Task
271+
272+
expect(isInAutomatedWorkflow(truthyTask, false)).toBe(true)
273+
274+
// Test falsy values
275+
const falsyTask = {
276+
isStreaming: false,
277+
isWaitingForFirstChunk: false,
278+
didCompleteReadingStream: true,
279+
presentAssistantMessageLocked: false,
280+
} as Task
281+
282+
expect(isInAutomatedWorkflow(falsyTask, false)).toBe(false)
283+
})
284+
285+
it("should handle provider with undefined getCurrentCline", () => {
286+
const providerWithUndefinedTask = {
287+
getCurrentCline: jest.fn().mockReturnValue(undefined),
288+
contextProxy: {
289+
getValue: jest.fn().mockReturnValue(false),
290+
},
291+
} as unknown as ClineProvider
292+
293+
expect(isInAutomatedWorkflowFromProvider(providerWithUndefinedTask)).toBe(false)
294+
})
295+
296+
it("should handle all combinations of auto-approval conditions", () => {
297+
// Test matrix of auto-approval conditions
298+
const testCases = [
299+
// [didCompleteReadingStream, presentAssistantMessageLocked, autoApprovalEnabled, expected]
300+
[true, false, false, false],
301+
[true, false, true, false],
302+
[false, false, false, false],
303+
[false, false, true, true], // incomplete stream with auto-approval
304+
[true, true, false, false],
305+
[true, true, true, true], // locked message with auto-approval
306+
[false, true, false, false],
307+
[false, true, true, true], // both incomplete stream and locked message
308+
]
309+
310+
testCases.forEach(([didComplete, isLocked, autoApproval, expected]) => {
311+
const task = {
312+
isStreaming: false,
313+
isWaitingForFirstChunk: false,
314+
didCompleteReadingStream: didComplete,
315+
presentAssistantMessageLocked: isLocked,
316+
} as Task
317+
318+
expect(isInAutomatedWorkflow(task, autoApproval as boolean)).toBe(expected as boolean)
319+
})
320+
})
321+
})
322+
})

0 commit comments

Comments
 (0)