-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Add submitUserMessage to Task #6895
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -740,6 +740,32 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike { | |||||||||||||||
| this.askResponseImages = images | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| public submitUserMessage(text: string, images?: string[]): void { | ||||||||||||||||
| try { | ||||||||||||||||
| const trimmed = (text ?? "").trim() | ||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider simplifying the null handling here. You're using
Suggested change
This would be more consistent with how you handle the images parameter. |
||||||||||||||||
| const imgs = images ?? [] | ||||||||||||||||
|
|
||||||||||||||||
| if (!trimmed && imgs.length === 0) { | ||||||||||||||||
| return | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| const provider = this.providerRef.deref() | ||||||||||||||||
| if (!provider) { | ||||||||||||||||
| console.error("[Task#submitUserMessage] Provider reference lost") | ||||||||||||||||
| return | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| void provider.postMessageToWebview({ | ||||||||||||||||
| type: "invoke", | ||||||||||||||||
| invoke: "sendMessage", | ||||||||||||||||
| text: trimmed, | ||||||||||||||||
| images: imgs, | ||||||||||||||||
| }) | ||||||||||||||||
| } catch (error) { | ||||||||||||||||
| console.error("[Task#submitUserMessage] Failed to submit user message:", error) | ||||||||||||||||
| } | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| async handleTerminalOperation(terminalOperation: "continue" | "abort") { | ||||||||||||||||
| if (terminalOperation === "continue") { | ||||||||||||||||
| this.terminalProcess?.continue() | ||||||||||||||||
|
|
||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1493,5 +1493,125 @@ describe("Cline", () => { | |
| expect(noModelTask.apiConfiguration.apiProvider).toBe("openai") | ||
| }) | ||
| }) | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good test coverage! Consider adding one more test case for completeness - testing the scenario where only images are provided without text: it("should handle images-only messages", async () => {
const task = new Task({
provider: mockProvider,
apiConfiguration: mockApiConfig,
task: "initial task",
startTask: false,
})
// Call with images but no text
task.submitUserMessage("", ["image1.png", "image2.png"])
// Should not call postMessageToWebview since text is empty
expect(mockProvider.postMessageToWebview).not.toHaveBeenCalled()
})This would ensure the edge case is explicitly tested, even though the current implementation handles it correctly. |
||
| describe("submitUserMessage", () => { | ||
| it("should always route through webview sendMessage invoke", async () => { | ||
| const task = new Task({ | ||
| provider: mockProvider, | ||
| apiConfiguration: mockApiConfig, | ||
| task: "initial task", | ||
| startTask: false, | ||
| }) | ||
|
|
||
| // Set up some existing messages to simulate an ongoing conversation | ||
| task.clineMessages = [ | ||
| { | ||
| ts: Date.now(), | ||
| type: "say", | ||
| say: "text", | ||
| text: "Initial message", | ||
| }, | ||
| ] | ||
|
|
||
| // Call submitUserMessage | ||
| task.submitUserMessage("test message", ["image1.png"]) | ||
|
|
||
| // Verify postMessageToWebview was called with sendMessage invoke | ||
| expect(mockProvider.postMessageToWebview).toHaveBeenCalledWith({ | ||
| type: "invoke", | ||
| invoke: "sendMessage", | ||
| text: "test message", | ||
| images: ["image1.png"], | ||
| }) | ||
| }) | ||
|
|
||
| it("should handle empty messages gracefully", async () => { | ||
| const task = new Task({ | ||
| provider: mockProvider, | ||
| apiConfiguration: mockApiConfig, | ||
| task: "initial task", | ||
| startTask: false, | ||
| }) | ||
|
|
||
| // Call with empty text and no images | ||
| task.submitUserMessage("", []) | ||
|
|
||
| // Should not call postMessageToWebview for empty messages | ||
| expect(mockProvider.postMessageToWebview).not.toHaveBeenCalled() | ||
|
|
||
| // Call with whitespace only | ||
| task.submitUserMessage(" ", []) | ||
| expect(mockProvider.postMessageToWebview).not.toHaveBeenCalled() | ||
| }) | ||
|
|
||
| it("should route through webview for both new and existing tasks", async () => { | ||
| const task = new Task({ | ||
| provider: mockProvider, | ||
| apiConfiguration: mockApiConfig, | ||
| task: "initial task", | ||
| startTask: false, | ||
| }) | ||
|
|
||
| // Test with no messages (new task scenario) | ||
| task.clineMessages = [] | ||
| task.submitUserMessage("new task", ["image1.png"]) | ||
|
|
||
| expect(mockProvider.postMessageToWebview).toHaveBeenCalledWith({ | ||
| type: "invoke", | ||
| invoke: "sendMessage", | ||
| text: "new task", | ||
| images: ["image1.png"], | ||
| }) | ||
|
|
||
| // Clear mock | ||
| mockProvider.postMessageToWebview.mockClear() | ||
|
|
||
| // Test with existing messages (ongoing task scenario) | ||
| task.clineMessages = [ | ||
| { | ||
| ts: Date.now(), | ||
| type: "say", | ||
| say: "text", | ||
| text: "Initial message", | ||
| }, | ||
| ] | ||
| task.submitUserMessage("follow-up message", ["image2.png"]) | ||
|
|
||
| expect(mockProvider.postMessageToWebview).toHaveBeenCalledWith({ | ||
| type: "invoke", | ||
| invoke: "sendMessage", | ||
| text: "follow-up message", | ||
| images: ["image2.png"], | ||
| }) | ||
| }) | ||
|
|
||
| it("should handle undefined provider gracefully", async () => { | ||
| const task = new Task({ | ||
| provider: mockProvider, | ||
| apiConfiguration: mockApiConfig, | ||
| task: "initial task", | ||
| startTask: false, | ||
| }) | ||
|
|
||
| // Simulate weakref returning undefined | ||
| Object.defineProperty(task, "providerRef", { | ||
| value: { deref: () => undefined }, | ||
| writable: false, | ||
| configurable: true, | ||
| }) | ||
|
|
||
| // Spy on console.error to verify error is logged | ||
| const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => {}) | ||
|
|
||
| // Should log error but not throw | ||
| task.submitUserMessage("test message") | ||
|
|
||
| expect(consoleErrorSpy).toHaveBeenCalledWith("[Task#submitUserMessage] Provider reference lost") | ||
| expect(mockProvider.postMessageToWebview).not.toHaveBeenCalled() | ||
|
|
||
| // Restore console.error | ||
| consoleErrorSpy.mockRestore() | ||
| }) | ||
| }) | ||
| }) | ||
| }) | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding JSDoc documentation for this public method to maintain consistency with other public methods and improve IDE tooltips: