diff --git a/core/index.d.ts b/core/index.d.ts index 6192666503f..3e546f6cdb4 100644 --- a/core/index.d.ts +++ b/core/index.d.ts @@ -1496,6 +1496,7 @@ export interface ApplyState { fileContent?: string; originalFileContent?: string; toolCallId?: string; + accepted?: boolean; autoFormattingDiff?: string; } diff --git a/extensions/vscode/src/diff/processDiff.ts b/extensions/vscode/src/diff/processDiff.ts index cbff6f4a363..5821c56c33a 100644 --- a/extensions/vscode/src/diff/processDiff.ts +++ b/extensions/vscode/src/diff/processDiff.ts @@ -90,6 +90,7 @@ export async function processDiff( status: "closed", numDiffs: 0, toolCallId, + accepted: action === "accept", autoFormattingDiff, // Include autoformatting diff }); } else { diff --git a/extensions/vscode/src/diff/vertical/handler.ts b/extensions/vscode/src/diff/vertical/handler.ts index 4420af5ec84..e42126307e1 100644 --- a/extensions/vscode/src/diff/vertical/handler.ts +++ b/extensions/vscode/src/diff/vertical/handler.ts @@ -20,6 +20,7 @@ export interface VerticalDiffHandlerOptions { status?: ApplyState["status"], numDiffs?: ApplyState["numDiffs"], fileContent?: ApplyState["fileContent"], + accepted?: ApplyState["accepted"], ) => void; streamId?: string; } @@ -145,6 +146,7 @@ export class VerticalDiffHandler implements vscode.Disposable { "closed", this.editorToVerticalDiffCodeLens.get(this.fileUri)?.length ?? 0, this.editor.document.getText(), + accept, ); this.cancelled = true; @@ -279,6 +281,7 @@ export class VerticalDiffHandler implements vscode.Disposable { status, numDiffs, this.editor.document.getText(), + status === "closed" ? accept : undefined, ); } } diff --git a/extensions/vscode/src/diff/vertical/manager.ts b/extensions/vscode/src/diff/vertical/manager.ts index 059efe0aee4..e28727c8bfb 100644 --- a/extensions/vscode/src/diff/vertical/manager.ts +++ b/extensions/vscode/src/diff/vertical/manager.ts @@ -196,7 +196,7 @@ export class VerticalDiffManager { ); if (blocks.length === 1) { - this.clearForfileUri(fileUri, true); + this.clearForfileUri(fileUri, accept); } else { // Re-enable listener for user changes to file this.enableDocumentChangeListener(); @@ -239,7 +239,7 @@ export class VerticalDiffManager { endLine, { instant, - onStatusUpdate: (status, numDiffs, fileContent) => + onStatusUpdate: (status, numDiffs, fileContent, accepted) => void this.webviewProtocol.request("updateApplyState", { streamId, status, @@ -247,6 +247,7 @@ export class VerticalDiffManager { fileContent, filepath: fileUri, toolCallId, + accepted, }), streamId, }, @@ -318,7 +319,7 @@ export class VerticalDiffManager { editor.document.lineCount - 1, { instant: true, - onStatusUpdate: (status, numDiffs, fileContent) => + onStatusUpdate: (status, numDiffs, fileContent, accepted) => void this.webviewProtocol.request("updateApplyState", { streamId, status, @@ -326,6 +327,7 @@ export class VerticalDiffManager { fileContent, filepath: fileUri, toolCallId, + accepted, }), streamId, }, @@ -446,7 +448,7 @@ export class VerticalDiffManager { { instant: isFastApplyModel(llm), input, - onStatusUpdate: (status, numDiffs, fileContent) => + onStatusUpdate: (status, numDiffs, fileContent, accepted) => streamId && void this.webviewProtocol.request("updateApplyState", { streamId, @@ -455,6 +457,7 @@ export class VerticalDiffManager { fileContent, filepath: fileUri, toolCallId, + accepted, }), streamId, }, diff --git a/gui/src/redux/slices/sessionSlice.ts b/gui/src/redux/slices/sessionSlice.ts index 1a0db2cc441..d68b9c15ffa 100644 --- a/gui/src/redux/slices/sessionSlice.ts +++ b/gui/src/redux/slices/sessionSlice.ts @@ -821,6 +821,9 @@ export const sessionSlice = createSlice({ applyState.fileContent = payload.fileContent ?? applyState.fileContent; applyState.originalFileContent = payload.originalFileContent ?? applyState.originalFileContent; + applyState.accepted = payload.accepted ?? applyState.accepted; + applyState.autoFormattingDiff = + payload.autoFormattingDiff ?? applyState.autoFormattingDiff; } if (payload.status === "done") { diff --git a/gui/src/redux/thunks/handleApplyStateUpdate.test.ts b/gui/src/redux/thunks/handleApplyStateUpdate.test.ts index 2e426b7d7c1..e388cec94ce 100644 --- a/gui/src/redux/thunks/handleApplyStateUpdate.test.ts +++ b/gui/src/redux/thunks/handleApplyStateUpdate.test.ts @@ -264,6 +264,62 @@ describe("handleApplyStateUpdate", () => { expect(streamResponseAfterToolCall).not.toHaveBeenCalled(); }); + it("should not mark a rejected closed apply state as accepted", async () => { + const toolCallState: ToolCallState = { + toolCallId: "test-tool-call", + status: "done", + ...UNUSED_TOOL_CALL_PARAMS, + }; + const newApplyState = { streamId: "chat-stream" }; + + vi.mocked(findToolCallById).mockReturnValue(toolCallState); + mockGetState.mockReturnValue({ + session: { + history: [], + codeBlockApplyStates: { + states: [newApplyState], + }, + }, + config: { config: {} }, + }); + + const applyState: ApplyState = { + streamId: "chat-stream", + toolCallId: "test-tool-call", + status: "closed", + filepath: "test.txt", + numDiffs: 0, + accepted: false, + }; + + const thunk = handleApplyStateUpdate(applyState); + await thunk(mockDispatch, mockGetState, mockExtra); + + expect(logToolUsage).toHaveBeenCalledWith( + toolCallState, + false, + true, + mockExtra.ideMessenger, + ); + expect(logAgentModeEditOutcome).toHaveBeenCalledWith( + [], + {}, + toolCallState, + newApplyState, + false, + mockExtra.ideMessenger, + ); + expect(acceptToolCall).not.toHaveBeenCalled(); + expect(updateToolCallOutput).not.toHaveBeenCalledWith( + expect.objectContaining({ + contextItems: expect.arrayContaining([ + expect.objectContaining({ name: "Edit Success" }), + ]), + }), + ); + expect(streamResponseAfterToolCall).not.toHaveBeenCalled(); + }); + it("should handle errored tool call closure", async () => { const toolCallState: ToolCallState = { toolCallId: "test-tool-call", diff --git a/gui/src/redux/thunks/handleApplyStateUpdate.ts b/gui/src/redux/thunks/handleApplyStateUpdate.ts index a220af51f63..3ec3a39a01e 100644 --- a/gui/src/redux/thunks/handleApplyStateUpdate.ts +++ b/gui/src/redux/thunks/handleApplyStateUpdate.ts @@ -63,7 +63,11 @@ export const handleApplyStateUpdate = createAsyncThunk< if (applyState.status === "closed") { if (toolCallState) { - const accepted = toolCallState.status !== "canceled"; + // Closing a diff does not always mean the user accepted it. + // Only report edit success when the close event confirms acceptance. + const accepted = + toolCallState.status !== "canceled" && + applyState.accepted !== false; logToolUsage(toolCallState, accepted, true, extra.ideMessenger);