diff --git a/src/core/Cline.ts b/src/core/Cline.ts index 788b5fddb2..1fde70977e 100644 --- a/src/core/Cline.ts +++ b/src/core/Cline.ts @@ -51,7 +51,7 @@ import { detectCodeOmission } from "../integrations/editor/detect-omission" import { BrowserSession } from "../services/browser/BrowserSession" import { OpenRouterHandler } from "../api/providers/openrouter" import { McpHub } from "../services/mcp/McpHub" -import { SlackNotifier } from "../services/slack" +import { setSlackEnabled, setWebhookUrl, notifyTaskComplete, notifyUserInputNeeded, notifyTaskFailed, notifyCommandExecution } from "../services/slack" const cwd = vscode.workspace.workspaceFolders?.map((folder) => folder.uri.fsPath).at(0) ?? path.join(os.homedir(), "Desktop") // may or may not exist but fs checking existence would immediately ask for permission which would be bad UX, need to come up with a better solution @@ -67,7 +67,6 @@ export class Cline { private terminalManager: TerminalManager private urlContentFetcher: UrlContentFetcher private browserSession: BrowserSession - private slackNotifier?: SlackNotifier private didEditFile: boolean = false customInstructions?: string diffStrategy?: DiffStrategy @@ -98,21 +97,20 @@ export class Cline { private didAlreadyUseTool = false private didCompleteReadingStream = false - private async notifySlack(type: 'complete' | 'input' | 'fail', message: string) { - if (!this.slackNotifier) { - return; - } - + private async notifySlack(type: 'complete' | 'input' | 'fail' | 'command', message: string) { try { switch (type) { case 'complete': - await this.slackNotifier.notifyTaskComplete(message); + await notifyTaskComplete(message); break; case 'input': - await this.slackNotifier.notifyUserInputNeeded(message); + await notifyUserInputNeeded(message); break; case 'fail': - await this.slackNotifier.notifyTaskFailed(message); + await notifyTaskFailed(message); + break; + case 'command': + await notifyCommandExecution(message); break; } } catch (error) { @@ -131,37 +129,27 @@ export class Cline { historyItem?: HistoryItem | undefined, slackConfig?: { enabled: boolean; webhookUrl: string } ) { - // Set taskId first - if (historyItem) { - this.taskId = historyItem.id - } else if (task || images) { - this.taskId = Date.now().toString() - } else { - throw new Error("Either historyItem or task/images must be provided") - } - this.providerRef = new WeakRef(provider) this.api = buildApiHandler(apiConfiguration) this.terminalManager = new TerminalManager() this.urlContentFetcher = new UrlContentFetcher(provider.context) this.browserSession = new BrowserSession(provider.context) - // Initialize Slack notifier + // Initialize Slack settings + setSlackEnabled(slackConfig?.enabled ?? false) + setWebhookUrl(slackConfig?.webhookUrl ?? '') + + // Send initialization notification if enabled if (slackConfig?.enabled && slackConfig?.webhookUrl) { - try { - this.slackNotifier = new SlackNotifier(slackConfig); - - // Send initialization notification - const initMessage = task - ? `🚀 New task started: ${task}` - : historyItem - ? `📝 Resuming task: ${historyItem.task}` - : "🔄 Roo Cline initialized"; - - this.notifySlack('input', initMessage).catch(error => { - }); - } catch (error) { - } + const initMessage = task + ? `🚀 New task started: ${task}` + : historyItem + ? `📝 Resuming task: ${historyItem.task}` + : "🔄 Roo Cline initialized"; + + this.notifySlack('input', initMessage).catch(error => { + console.error('Failed to send initialization notification:', error); + }); } this.diffViewProvider = new DiffViewProvider(cwd) this.customInstructions = customInstructions @@ -170,9 +158,13 @@ export class Cline { this.diffStrategy = getDiffStrategy(this.api.getModel().id, fuzzyMatchThreshold ?? 1.0) } if (historyItem) { + this.taskId = historyItem.id this.resumeTaskFromHistory() } else if (task || images) { + this.taskId = Date.now().toString() this.startTask(task, images) + } else { + throw new Error("Either historyItem or task/images must be provided") } } @@ -214,6 +206,7 @@ export class Cline { await fs.writeFile(filePath, JSON.stringify(this.apiConversationHistory)) } catch (error) { // in the off chance this fails, we don't want to stop the task + console.error("Failed to save api conversation history", error) } } @@ -279,9 +272,14 @@ export class Cline { text?: string, partial?: boolean, ): Promise<{ response: ClineAskResponse; text?: string; images?: string[] }> { - // Send notification for user input needed - if (type === "followup" && text && !partial) { - await this.notifySlack('input', text) + // Send notifications for user input needed or command requests + if (!partial && text) { + if (type === "followup") { + await this.notifySlack('input', text) + } else if (type === "command") { + // Notify when Cline asks to run a command (when Run Command button appears) + await this.notifySlack('command', text) + } } // If this Cline instance was aborted by the provider, then the only thing keeping us alive is a promise still running in the background, in which case we don't want to send its result to the webview as it is attached to a new instance of Cline now. So we can safely ignore the result of any active promises, and this class will be deallocated. (Although we set Cline = undefined in provider, that simply removes the reference to this instance, but the instance is still alive until this promise resolves or rejects.) @@ -364,13 +362,10 @@ export class Cline { } await pWaitFor(() => this.askResponse !== undefined || this.lastMessageTs !== askTs, { interval: 100 }) - if (this.lastMessageTs !== askTs) { throw new Error("Current ask promise was ignored") // could happen if we send multiple asks in a row i.e. with command_output. It's important that when we know an ask could fail, it is handled gracefully } - const result = { response: this.askResponse!, text: this.askResponseText, images: this.askResponseImages } - this.askResponse = undefined this.askResponseText = undefined this.askResponseImages = undefined @@ -945,8 +940,6 @@ export class Cline { } const block = cloneDeep(this.assistantMessageContent[this.currentStreamingContentIndex]) // need to create copy bc while stream is updating the array, it could be updating the reference block properties too - - switch (block.type) { case "text": { if (this.didRejectTool || this.didAlreadyUseTool) { @@ -1763,7 +1756,6 @@ export class Cline { break } this.consecutiveMistakeCount = 0 - const didApprove = await askApproval("command", command) if (!didApprove) { break @@ -2043,11 +2035,7 @@ export class Cline { await this.say("completion_result", result, undefined, false) } - // complete command message - const didApprove = await askApproval("command", command) - if (!didApprove) { - break - } + // Execute command from attempt_completion const [userRejected, execCommandResult] = await this.executeCommandTool(command!) if (userRejected) { this.didRejectTool = true @@ -2066,27 +2054,11 @@ export class Cline { timestamp: new Date().toISOString() }); } - - console.log("About to call notifySlack with type 'complete'", { - resultLength: result?.length, - taskId: this.taskId, - hasSlackNotifier: !!this.slackNotifier, - slackConfig: this.slackNotifier?.config, - timestamp: new Date().toISOString() - }); - const completionMessage = result ? `✅ Task completed successfully!\n\nResult:\n${result}` : "✅ Task completed successfully!"; - // Make sure to await the notification await this.notifySlack('complete', completionMessage); - - console.log("Successfully sent completion notification to Slack", { - taskId: this.taskId, - messageLength: completionMessage.length, - timestamp: new Date().toISOString() - }); } catch (error) { console.error("Error during notifySlack call:", { errorMessage: error instanceof Error ? error.message : 'Unknown error', @@ -2097,22 +2069,10 @@ export class Cline { } })(); } - // we already sent completion_result says, an empty string asks relinquishes control over button and field - let askResponse; - try { - askResponse = await this.ask("completion_result", "", false); - } catch (error) { - console.error("Error during ask call:", { - error, - stack: error instanceof Error ? error.stack : 'No stack trace', - taskId: this.taskId - }); - throw error; - } - const { response, text, images } = askResponse; + const { response, text, images } = await this.ask("completion_result", "", false); if (response === "yesButtonClicked") { - pushToolResult(""); + pushToolResult(""); //signals to recursive loop to stop (for now this never happens since yesButtonClicked will trigger a new task) break; } await this.say("user_feedback", text ?? "", images) diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 584de2b4aa..72bf72560d 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -1398,12 +1398,12 @@ export class ClineProvider implements vscode.WebviewViewProvider { const fileExists = await fileExistsAtPath(apiConversationHistoryFilePath) if (fileExists) { const apiConversationHistory = JSON.parse(await fs.readFile(apiConversationHistoryFilePath, "utf8")) - return { - historyItem, - taskDirPath, - apiConversationHistoryFilePath, - uiMessagesFilePath, - apiConversationHistory, + return { + historyItem, + taskDirPath, + apiConversationHistoryFilePath, + uiMessagesFilePath, + apiConversationHistory, } } } @@ -1436,7 +1436,7 @@ export class ClineProvider implements vscode.WebviewViewProvider { await this.deleteTaskFromState(id) - // Delete the task files + // Delete the task files const apiConversationHistoryFileExists = await fileExistsAtPath(apiConversationHistoryFilePath) if (apiConversationHistoryFileExists) { await fs.unlink(apiConversationHistoryFilePath) @@ -1453,12 +1453,12 @@ export class ClineProvider implements vscode.WebviewViewProvider { } async deleteTaskFromState(id: string) { - // Remove the task from history + // Remove the task from history const taskHistory = ((await this.getGlobalState("taskHistory")) as HistoryItem[]) || [] const updatedTaskHistory = taskHistory.filter((task) => task.id !== id) await this.updateGlobalState("taskHistory", updatedTaskHistory) - // Notify the webview that the task has been deleted + // Notify the webview that the task has been deleted await this.postStateToWebview() } @@ -1541,7 +1541,7 @@ export class ClineProvider implements vscode.WebviewViewProvider { async clearTask() { this.cline?.abortTask() - this.cline = undefined // removes reference to it, so once promises end it will be garbage collected + this.cline = undefined // removes reference to it, so once promises end it will be garbage collected } // Caching mechanism to keep track of webview messages + API conversation history per provider instance @@ -1904,25 +1904,25 @@ export class ClineProvider implements vscode.WebviewViewProvider { for (const key of this.context.globalState.keys()) { await this.context.globalState.update(key, undefined) } - const secretKeys: SecretKey[] = [ - "apiKey", - "glamaApiKey", - "openRouterApiKey", - "awsAccessKey", - "awsSecretKey", - "awsSessionToken", - "openAiApiKey", - "geminiApiKey", - "openAiNativeApiKey", - "deepSeekApiKey", + const secretKeys: SecretKey[] = [ + "apiKey", + "glamaApiKey", + "openRouterApiKey", + "awsAccessKey", + "awsSecretKey", + "awsSessionToken", + "openAiApiKey", + "geminiApiKey", + "openAiNativeApiKey", + "deepSeekApiKey", ] for (const key of secretKeys) { await this.storeSecret(key, undefined) } - if (this.cline) { + if (this.cline) { this.cline.abortTask() this.cline = undefined - } + } vscode.window.showInformationMessage("State reset") await this.postStateToWebview() await this.postMessageToWebview({ type: "action", action: "chatButtonClicked" }) diff --git a/src/services/slack/index.ts b/src/services/slack/index.ts index f7a5e696f0..e1471e5990 100644 --- a/src/services/slack/index.ts +++ b/src/services/slack/index.ts @@ -1,94 +1,83 @@ import * as vscode from 'vscode' -export interface SlackConfig { - webhookUrl: string - enabled: boolean +let isSlackEnabled = false +let webhookUrl = '' + +/** + * Set slack notification configuration + * @param enabled boolean + */ +export const setSlackEnabled = (enabled: boolean): void => { + isSlackEnabled = enabled } -export class SlackNotifier { - public readonly config: SlackConfig +/** + * Set slack webhook URL + * @param url string + */ +export const setWebhookUrl = (url: string): void => { + webhookUrl = url +} - constructor(config: SlackConfig) { - console.log("Creating new SlackNotifier instance with config:", { - enabled: config.enabled, - hasWebhookUrl: !!config.webhookUrl, - webhookUrlLength: config.webhookUrl?.length - }); - this.config = config; - } - - private async sendMessage(text: string): Promise { - console.log("SlackNotifier.sendMessage called with:", { - text, - config: { - enabled: this.config.enabled, - hasWebhookUrl: !!this.config.webhookUrl, - webhookUrlLength: this.config.webhookUrl?.length - } - }); - - if (!this.config.enabled) { - console.log("Slack notifications are disabled in config"); - return; - } - - if (!this.config.webhookUrl) { - console.log("No Slack webhook URL configured in config"); - return; +/** + * Send a slack message + * @param text string + * @return Promise + */ +export const sendSlackMessage = async (text: string): Promise => { + try { + if (!isSlackEnabled) { + return } - try { - console.log("Preparing Slack webhook request..."); - const body = JSON.stringify({ text }); - console.log("Request body prepared:", { bodyLength: body.length }); - - console.log("Sending request to Slack webhook..."); - const response = await fetch(this.config.webhookUrl, { - method: 'POST', - headers: { - 'Content-Type': 'application/json' - }, - body: JSON.stringify({ text }) - }) - - console.log("Received response from Slack webhook:", { - status: response.status, - statusText: response.statusText, - ok: response.ok - }); - - if (!response.ok) { - const responseText = await response.text(); - console.error("Slack API error response:", { - status: response.status, - statusText: response.statusText, - responseText, - webhookUrlLength: this.config.webhookUrl.length, - messageLength: text.length - }); - throw new Error(`Failed to send Slack message: ${response.statusText} (${response.status})`); - } - - console.log("Successfully sent Slack message:", { - messageLength: text.length, - timestamp: new Date().toISOString() - }); - } catch (error) { - console.error('Error sending Slack notification:', error); - vscode.window.showErrorMessage(`Failed to send Slack notification: ${error instanceof Error ? error.message : 'Unknown error'}`); - // Don't throw - we don't want Slack errors to interrupt the main flow + if (!webhookUrl) { + return } - } - async notifyTaskComplete(taskDescription: string): Promise { - await this.sendMessage(`✅ Task Complete: ${taskDescription}`) - } + const response = await fetch(webhookUrl, { + method: 'POST', + headers: { + 'Content-Type': 'application/json' + }, + body: JSON.stringify({ text }) + }) - async notifyUserInputNeeded(question: string): Promise { - await this.sendMessage(`❓ User Input Received: ${question}`) + if (!response.ok) { + throw new Error(`Failed to send Slack message: ${response.statusText} (${response.status})`) + } + } catch (error: any) { + vscode.window.showErrorMessage(`Failed to send Slack notification: ${error.message}`) } +} - async notifyTaskFailed(error: string): Promise { - await this.sendMessage(`❌ Task Failed: ${error}`) - } +/** + * Notify task completion via Slack + * @param taskDescription string + */ +export const notifyTaskComplete = async (taskDescription: string): Promise => { + await sendSlackMessage(`✅ Task Complete: ${taskDescription}`) +} + +/** + * Notify user input needed via Slack + * @param question string + */ +export const notifyUserInputNeeded = async (question: string): Promise => { + await sendSlackMessage(`❓ User Input Received: ${question}`) +} + +/** + * Notify task failure via Slack + * @param error string + */ +export const notifyTaskFailed = async (error: string): Promise => { + await sendSlackMessage(`❌ Task Failed: ${error}`) +} + +/** + * Notify command execution request via Slack + * @param command string + */ +export const notifyCommandExecution = async (command: string): Promise => { + await sendSlackMessage(`🔧 Command Requested: ${command}`) } \ No newline at end of file diff --git a/src/test/slack.test.ts b/src/test/slack.test.ts index dac6a5217e..990cf50acb 100644 --- a/src/test/slack.test.ts +++ b/src/test/slack.test.ts @@ -1,24 +1,23 @@ -import { SlackNotifier } from '../services/slack' +import { setSlackEnabled, setWebhookUrl, sendSlackMessage, notifyTaskComplete, notifyUserInputNeeded, notifyTaskFailed, notifyCommandExecution } from '../services/slack' -describe('SlackNotifier', () => { - let slackNotifier: SlackNotifier +describe('Slack Notifications', () => { let mockFetch: jest.Mock beforeEach(() => { mockFetch = jest.fn() global.fetch = mockFetch - slackNotifier = new SlackNotifier({ - webhookUrl: 'https://hooks.slack.com/services/test', - enabled: true - }) + setWebhookUrl('https://hooks.slack.com/services/test') + setSlackEnabled(true) }) afterEach(() => { jest.resetAllMocks() + setSlackEnabled(false) + setWebhookUrl('') }) it('should send task completion notification', async () => { - await slackNotifier.notifyTaskComplete('Task completed successfully') + await notifyTaskComplete('Task completed successfully') expect(mockFetch).toHaveBeenCalledWith( 'https://hooks.slack.com/services/test', expect.objectContaining({ @@ -30,7 +29,7 @@ describe('SlackNotifier', () => { }) it('should send user input needed notification', async () => { - await slackNotifier.notifyUserInputNeeded('What is your preference?') + await notifyUserInputNeeded('What is your preference?') expect(mockFetch).toHaveBeenCalledWith( 'https://hooks.slack.com/services/test', expect.objectContaining({ @@ -42,7 +41,7 @@ describe('SlackNotifier', () => { }) it('should send task failed notification', async () => { - await slackNotifier.notifyTaskFailed('Error occurred') + await notifyTaskFailed('Error occurred') expect(mockFetch).toHaveBeenCalledWith( 'https://hooks.slack.com/services/test', expect.objectContaining({ @@ -54,14 +53,11 @@ describe('SlackNotifier', () => { }) it('should not send notifications when disabled', async () => { - slackNotifier = new SlackNotifier({ - webhookUrl: 'https://hooks.slack.com/services/test', - enabled: false - }) + setSlackEnabled(false) - await slackNotifier.notifyTaskComplete('Task completed') - await slackNotifier.notifyUserInputNeeded('Input needed') - await slackNotifier.notifyTaskFailed('Task failed') + await notifyTaskComplete('Task completed') + await notifyUserInputNeeded('Input needed') + await notifyTaskFailed('Task failed') expect(mockFetch).not.toHaveBeenCalled() }) @@ -70,8 +66,26 @@ describe('SlackNotifier', () => { mockFetch.mockRejectedValue(new Error('Network error')) // These should not throw errors - await expect(slackNotifier.notifyTaskComplete('Task completed')).resolves.not.toThrow() - await expect(slackNotifier.notifyUserInputNeeded('Input needed')).resolves.not.toThrow() - await expect(slackNotifier.notifyTaskFailed('Task failed')).resolves.not.toThrow() + await expect(notifyTaskComplete('Task completed')).resolves.not.toThrow() + await expect(notifyUserInputNeeded('Input needed')).resolves.not.toThrow() + await expect(notifyTaskFailed('Task failed')).resolves.not.toThrow() + }) + + it('should not send message when webhook URL is not set', async () => { + setWebhookUrl('') + await sendSlackMessage('Test message') + expect(mockFetch).not.toHaveBeenCalled() + }) + + it('should send command execution notification', async () => { + await notifyCommandExecution('npm install') + expect(mockFetch).toHaveBeenCalledWith( + 'https://hooks.slack.com/services/test', + expect.objectContaining({ + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ text: '🔧 Command Requested: npm install' }) + }) + ) }) }) \ No newline at end of file diff --git a/webview-ui/src/components/chat/ChatView.tsx b/webview-ui/src/components/chat/ChatView.tsx index a20cd4acb5..2c8ea33885 100644 --- a/webview-ui/src/components/chat/ChatView.tsx +++ b/webview-ui/src/components/chat/ChatView.tsx @@ -327,10 +327,8 @@ const ChatView = ({ isHidden, showAnnouncement, hideAnnouncement, showHistoryVie vscode.postMessage({ type: "askResponse", askResponse: "yesButtonClicked" }) break case "completion_result": - // First send the completion approval, then start new task - vscode.postMessage({ type: "askResponse", askResponse: "yesButtonClicked" }); // Increase delay to ensure notification is processed - break; case "resume_completed_task": + vscode.postMessage({ type: "askResponse", askResponse: "yesButtonClicked" }) startNewTask() break }