From 473e0af6a548807eedf9e156e624943c38e2ae63 Mon Sep 17 00:00:00 2001 From: 1220moritz Date: Mon, 2 Nov 2026 21:40:29 +0100 Subject: [PATCH 01/10] feat(scm): add generate commit message action in source control input box --- packages/types/src/vscode.ts | 2 + .../__tests__/registerCommands.spec.ts | 30 ++ src/activate/registerCommands.ts | 9 + src/package.json | 12 + src/package.nls.ca.json | 1 + src/package.nls.de.json | 1 + src/package.nls.es.json | 1 + src/package.nls.fr.json | 1 + src/package.nls.hi.json | 1 + src/package.nls.id.json | 1 + src/package.nls.it.json | 1 + src/package.nls.ja.json | 1 + src/package.nls.json | 1 + src/package.nls.ko.json | 1 + src/package.nls.nl.json | 1 + src/package.nls.pl.json | 1 + src/package.nls.pt-BR.json | 1 + src/package.nls.ru.json | 1 + src/package.nls.tr.json | 1 + src/package.nls.vi.json | 1 + src/package.nls.zh-CN.json | 1 + src/package.nls.zh-TW.json | 1 + .../__tests__/commitMessageGenerator.spec.ts | 385 ++++++++++++++++++ src/services/git/commitMessageGenerator.ts | 228 +++++++++++ 24 files changed, 684 insertions(+) create mode 100644 src/services/git/__tests__/commitMessageGenerator.spec.ts create mode 100644 src/services/git/commitMessageGenerator.ts diff --git a/packages/types/src/vscode.ts b/packages/types/src/vscode.ts index 811ceda165..fbb343ce82 100644 --- a/packages/types/src/vscode.ts +++ b/packages/types/src/vscode.ts @@ -48,6 +48,8 @@ export const commandIds = [ "retryWebviewRecovery", "toggleAutoApprove", + "generateGitCommitMessage", + "showRipgrepDiagnostic", "createDiagnosticsReport", ] as const diff --git a/src/activate/__tests__/registerCommands.spec.ts b/src/activate/__tests__/registerCommands.spec.ts index c59933a7c1..c68be5a052 100644 --- a/src/activate/__tests__/registerCommands.spec.ts +++ b/src/activate/__tests__/registerCommands.spec.ts @@ -100,6 +100,10 @@ vi.mock("../../services/diagnostics", () => ({ createDiagnosticsReport: vi.fn().mockResolvedValue(undefined), })) +vi.mock("../../services/git/commitMessageGenerator", () => ({ + generateCommitMessage: vi.fn().mockResolvedValue(undefined), +})) + describe("getVisibleProviderOrLog", () => { let mockOutputChannel: vscode.OutputChannel @@ -282,6 +286,32 @@ describe("registerCommands handlers", () => { }) }) + it("generateGitCommitMessage calls generateCommitMessage with visible provider and context", async () => { + const { generateCommitMessage } = await import("../../services/git/commitMessageGenerator") + const mockScmContext = { rootUri: { fsPath: "/mock/repo" } } + + await handlers["zoo-code.generateGitCommitMessage"](mockScmContext) + + expect(generateCommitMessage).toHaveBeenCalledWith({ + provider: mockVisibleProviderInstance, + outputChannel: mockOutputChannel, + context: mockScmContext, + }) + }) + + it("generateGitCommitMessage falls back to default provider when no visible provider", async () => { + const { generateCommitMessage } = await import("../../services/git/commitMessageGenerator") + ;(ClineProvider.getVisibleInstance as Mock).mockReturnValue(undefined) + + await handlers["zoo-code.generateGitCommitMessage"]() + + expect(generateCommitMessage).toHaveBeenCalledWith({ + provider: mockProvider, + outputChannel: mockOutputChannel, + context: undefined, + }) + }) + it("settingsButtonClicked posts both settingsButtonClicked and didBecomeVisible actions", () => { handlers["zoo-code.settingsButtonClicked"]() diff --git a/src/activate/registerCommands.ts b/src/activate/registerCommands.ts index f0f4747eee..2e483c4de0 100644 --- a/src/activate/registerCommands.ts +++ b/src/activate/registerCommands.ts @@ -14,6 +14,7 @@ import { importSettingsWithFeedback } from "../core/config/importExport" import { MdmService } from "../services/mdm/MdmService" import { registerRipgrepDiagnosticCommand } from "../services/ripgrep/diagnostic" import { createDiagnosticsReport } from "../services/diagnostics" +import { generateCommitMessage } from "../services/git/commitMessageGenerator" /** * Helper to get the visible ClineProvider instance or log if not found. @@ -225,6 +226,14 @@ const getCommandsMap = ({ outputChannel.appendLine(`[toggleAutoApprove] postMessageToWebview failed: ${error}`) } }, + generateGitCommitMessage: async (context?: unknown) => { + const visibleProvider = ClineProvider.getVisibleInstance() ?? provider + await generateCommitMessage({ + provider: visibleProvider, + outputChannel, + context, + }) + }, }) type OpenClineInNewTabOptions = { diff --git a/src/package.json b/src/package.json index 6c59828b1d..9e8cab6b22 100644 --- a/src/package.json +++ b/src/package.json @@ -179,9 +179,21 @@ "command": "zoo-code.toggleAutoApprove", "title": "%command.toggleAutoApprove.title%", "category": "%configuration.title%" + }, + { + "command": "zoo-code.generateGitCommitMessage", + "title": "%command.generateGitCommitMessage.title%", + "icon": "$(sparkle)" } ], "menus": { + "scm/inputBox": [ + { + "command": "zoo-code.generateGitCommitMessage", + "group": "inline", + "when": "scmRepository" + } + ], "editor/context": [ { "submenu": "zoo-code.contextMenu", diff --git a/src/package.nls.ca.json b/src/package.nls.ca.json index d6b40efba3..a144328090 100644 --- a/src/package.nls.ca.json +++ b/src/package.nls.ca.json @@ -18,6 +18,7 @@ "command.showRipgrepDiagnostic.title": "Mostra el diagnòstic de Ripgrep", "command.createDiagnosticsReport.title": "Crea un informe de diagnòstic", "command.toggleAutoApprove.title": "Alternar Auto-Aprovació", + "command.generateGitCommitMessage.title": "Generar Missatge de Commit", "views.activitybar.title": "Zoo Code", "views.contextMenu.label": "Zoo Code", "views.terminalMenu.label": "Zoo Code", diff --git a/src/package.nls.de.json b/src/package.nls.de.json index a8bbadbf15..c8f048d935 100644 --- a/src/package.nls.de.json +++ b/src/package.nls.de.json @@ -18,6 +18,7 @@ "command.showRipgrepDiagnostic.title": "Ripgrep-Diagnose anzeigen", "command.createDiagnosticsReport.title": "Diagnosebericht erstellen", "command.toggleAutoApprove.title": "Auto-Genehmigung Umschalten", + "command.generateGitCommitMessage.title": "Commit-Nachricht generieren", "views.activitybar.title": "Zoo Code", "views.contextMenu.label": "Zoo Code", "views.terminalMenu.label": "Zoo Code", diff --git a/src/package.nls.es.json b/src/package.nls.es.json index 26d9f87e9d..b0a59a5163 100644 --- a/src/package.nls.es.json +++ b/src/package.nls.es.json @@ -18,6 +18,7 @@ "command.showRipgrepDiagnostic.title": "Mostrar diagnóstico de Ripgrep", "command.createDiagnosticsReport.title": "Crear informe de diagnóstico", "command.toggleAutoApprove.title": "Alternar Auto-Aprobación", + "command.generateGitCommitMessage.title": "Generar mensaje de confirmación", "views.activitybar.title": "Zoo Code", "views.contextMenu.label": "Zoo Code", "views.terminalMenu.label": "Zoo Code", diff --git a/src/package.nls.fr.json b/src/package.nls.fr.json index 364a8878c4..feb40a8211 100644 --- a/src/package.nls.fr.json +++ b/src/package.nls.fr.json @@ -18,6 +18,7 @@ "command.showRipgrepDiagnostic.title": "Afficher le diagnostic Ripgrep", "command.createDiagnosticsReport.title": "Créer un rapport de diagnostic", "command.toggleAutoApprove.title": "Basculer Auto-Approbation", + "command.generateGitCommitMessage.title": "Générer un message de commit", "views.activitybar.title": "Zoo Code", "views.contextMenu.label": "Zoo Code", "views.terminalMenu.label": "Zoo Code", diff --git a/src/package.nls.hi.json b/src/package.nls.hi.json index db70bc8417..8b2eb5f1f1 100644 --- a/src/package.nls.hi.json +++ b/src/package.nls.hi.json @@ -18,6 +18,7 @@ "command.showRipgrepDiagnostic.title": "Ripgrep डायग्नोस्टिक दिखाएं", "command.createDiagnosticsReport.title": "डायग्नोस्टिक रिपोर्ट बनाएं", "command.toggleAutoApprove.title": "ऑटो-अनुमोदन टॉगल करें", + "command.generateGitCommitMessage.title": "कमिट संदेश बनाएं", "views.activitybar.title": "Zoo Code", "views.contextMenu.label": "Zoo Code", "views.terminalMenu.label": "Zoo Code", diff --git a/src/package.nls.id.json b/src/package.nls.id.json index b7e150ee53..2298fc3f24 100644 --- a/src/package.nls.id.json +++ b/src/package.nls.id.json @@ -27,6 +27,7 @@ "command.showRipgrepDiagnostic.title": "Tampilkan Diagnostik Ripgrep", "command.createDiagnosticsReport.title": "Buat Laporan Diagnostik", "command.toggleAutoApprove.title": "Alihkan Persetujuan Otomatis", + "command.generateGitCommitMessage.title": "Buat Pesan Commit", "configuration.title": "Zoo Code", "commands.allowedCommands.description": "Perintah yang dapat dijalankan secara otomatis ketika 'Selalu setujui operasi eksekusi' diaktifkan", "commands.deniedCommands.description": "Awalan perintah yang akan otomatis ditolak tanpa meminta persetujuan. Jika terjadi konflik dengan perintah yang diizinkan, pencocokan awalan terpanjang akan diprioritaskan. Tambahkan * untuk menolak semua perintah.", diff --git a/src/package.nls.it.json b/src/package.nls.it.json index 6d50b96bc4..deb1575a6f 100644 --- a/src/package.nls.it.json +++ b/src/package.nls.it.json @@ -18,6 +18,7 @@ "command.showRipgrepDiagnostic.title": "Mostra diagnostica Ripgrep", "command.createDiagnosticsReport.title": "Crea rapporto di diagnostica", "command.toggleAutoApprove.title": "Attiva/Disattiva Auto-Approvazione", + "command.generateGitCommitMessage.title": "Genera Messaggio di Commit", "views.activitybar.title": "Zoo Code", "views.contextMenu.label": "Zoo Code", "views.terminalMenu.label": "Zoo Code", diff --git a/src/package.nls.ja.json b/src/package.nls.ja.json index 22f4f0dfce..5b8797ccff 100644 --- a/src/package.nls.ja.json +++ b/src/package.nls.ja.json @@ -27,6 +27,7 @@ "command.showRipgrepDiagnostic.title": "Ripgrep 診断を表示", "command.createDiagnosticsReport.title": "診断レポートを作成", "command.toggleAutoApprove.title": "自動承認を切替", + "command.generateGitCommitMessage.title": "コミットメッセージの生成", "configuration.title": "Zoo Code", "commands.allowedCommands.description": "'常に実行操作を承認する'が有効な場合に自動実行できるコマンド", "commands.deniedCommands.description": "承認を求めずに自動的に拒否されるコマンドプレフィックス。許可されたコマンドとの競合がある場合、最長プレフィックスマッチが優先されます。すべてのコマンドを拒否するには * を追加してください。", diff --git a/src/package.nls.json b/src/package.nls.json index d1f9afbc39..1f0818516c 100644 --- a/src/package.nls.json +++ b/src/package.nls.json @@ -27,6 +27,7 @@ "command.showRipgrepDiagnostic.title": "Show Ripgrep Diagnostic", "command.createDiagnosticsReport.title": "Create Diagnostics Report", "command.toggleAutoApprove.title": "Toggle Auto-Approve", + "command.generateGitCommitMessage.title": "Generate Commit Message", "configuration.title": "Zoo Code", "commands.allowedCommands.description": "Commands that can be auto-executed when 'Always approve execute operations' is enabled", "commands.deniedCommands.description": "Command prefixes that will be automatically denied without asking for approval. In case of conflicts with allowed commands, the longest prefix match takes precedence. Add * to deny all commands.", diff --git a/src/package.nls.ko.json b/src/package.nls.ko.json index 48d8695a1d..f33415292b 100644 --- a/src/package.nls.ko.json +++ b/src/package.nls.ko.json @@ -18,6 +18,7 @@ "command.showRipgrepDiagnostic.title": "Ripgrep 진단 표시", "command.createDiagnosticsReport.title": "진단 보고서 만들기", "command.toggleAutoApprove.title": "자동 승인 전환", + "command.generateGitCommitMessage.title": "커밋 메시지 생성", "views.activitybar.title": "Zoo Code", "views.contextMenu.label": "Zoo Code", "views.terminalMenu.label": "Zoo Code", diff --git a/src/package.nls.nl.json b/src/package.nls.nl.json index 81113c4b5a..c77b273630 100644 --- a/src/package.nls.nl.json +++ b/src/package.nls.nl.json @@ -27,6 +27,7 @@ "command.showRipgrepDiagnostic.title": "Ripgrep-diagnose weergeven", "command.createDiagnosticsReport.title": "Diagnoserapport maken", "command.toggleAutoApprove.title": "Auto-Goedkeuring Schakelen", + "command.generateGitCommitMessage.title": "Commitbericht genereren", "configuration.title": "Zoo Code", "commands.allowedCommands.description": "Commando's die automatisch kunnen worden uitgevoerd wanneer 'Altijd goedkeuren uitvoerbewerkingen' is ingeschakeld", "commands.deniedCommands.description": "Commando-prefixen die automatisch worden geweigerd zonder om goedkeuring te vragen. Bij conflicten met toegestane commando's heeft de langste prefix-match voorrang. Voeg * toe om alle commando's te weigeren.", diff --git a/src/package.nls.pl.json b/src/package.nls.pl.json index 267ddff277..860bac18ed 100644 --- a/src/package.nls.pl.json +++ b/src/package.nls.pl.json @@ -18,6 +18,7 @@ "command.showRipgrepDiagnostic.title": "Pokaż diagnostykę Ripgrep", "command.createDiagnosticsReport.title": "Utwórz raport diagnostyczny", "command.toggleAutoApprove.title": "Przełącz Auto-Zatwierdzanie", + "command.generateGitCommitMessage.title": "Wygeneruj komunikat commita", "views.activitybar.title": "Zoo Code", "views.contextMenu.label": "Zoo Code", "views.terminalMenu.label": "Zoo Code", diff --git a/src/package.nls.pt-BR.json b/src/package.nls.pt-BR.json index 853832b699..0de9a3f51c 100644 --- a/src/package.nls.pt-BR.json +++ b/src/package.nls.pt-BR.json @@ -18,6 +18,7 @@ "command.showRipgrepDiagnostic.title": "Mostrar diagnóstico do Ripgrep", "command.createDiagnosticsReport.title": "Criar relatório de diagnóstico", "command.toggleAutoApprove.title": "Alternar Auto-Aprovação", + "command.generateGitCommitMessage.title": "Gerar mensagem de commit", "views.activitybar.title": "Zoo Code", "views.contextMenu.label": "Zoo Code", "views.terminalMenu.label": "Zoo Code", diff --git a/src/package.nls.ru.json b/src/package.nls.ru.json index d46ea14245..cdfd27bfbd 100644 --- a/src/package.nls.ru.json +++ b/src/package.nls.ru.json @@ -27,6 +27,7 @@ "command.showRipgrepDiagnostic.title": "Показать диагностику Ripgrep", "command.createDiagnosticsReport.title": "Создать диагностический отчёт", "command.toggleAutoApprove.title": "Переключить Авто-Подтверждение", + "command.generateGitCommitMessage.title": "Сгенерировать сообщение коммита", "configuration.title": "Zoo Code", "commands.allowedCommands.description": "Команды, которые могут быть автоматически выполнены, когда включена опция 'Всегда подтверждать операции выполнения'", "commands.deniedCommands.description": "Префиксы команд, которые будут автоматически отклонены без запроса подтверждения. В случае конфликтов с разрешенными командами приоритет имеет самое длинное совпадение префикса. Добавьте * чтобы отклонить все команды.", diff --git a/src/package.nls.tr.json b/src/package.nls.tr.json index cbc81d959f..8f7955b582 100644 --- a/src/package.nls.tr.json +++ b/src/package.nls.tr.json @@ -18,6 +18,7 @@ "command.showRipgrepDiagnostic.title": "Ripgrep Tanılamasını Göster", "command.createDiagnosticsReport.title": "Tanılama Raporu Oluştur", "command.toggleAutoApprove.title": "Otomatik Onayı Değiştir", + "command.generateGitCommitMessage.title": "Commit Mesajı Oluştur", "views.activitybar.title": "Zoo Code", "views.contextMenu.label": "Zoo Code", "views.terminalMenu.label": "Zoo Code", diff --git a/src/package.nls.vi.json b/src/package.nls.vi.json index 1ee4eabd70..0b346dee69 100644 --- a/src/package.nls.vi.json +++ b/src/package.nls.vi.json @@ -18,6 +18,7 @@ "command.showRipgrepDiagnostic.title": "Hiển thị chẩn đoán Ripgrep", "command.createDiagnosticsReport.title": "Tạo báo cáo chẩn đoán", "command.toggleAutoApprove.title": "Bật/Tắt Tự Động Phê Duyệt", + "command.generateGitCommitMessage.title": "Tạo thông điệp commit", "views.activitybar.title": "Zoo Code", "views.contextMenu.label": "Zoo Code", "views.terminalMenu.label": "Zoo Code", diff --git a/src/package.nls.zh-CN.json b/src/package.nls.zh-CN.json index 85dafc3bc7..eb7745b3bb 100644 --- a/src/package.nls.zh-CN.json +++ b/src/package.nls.zh-CN.json @@ -18,6 +18,7 @@ "command.showRipgrepDiagnostic.title": "显示 Ripgrep 诊断", "command.createDiagnosticsReport.title": "创建诊断报告", "command.toggleAutoApprove.title": "切换自动批准", + "command.generateGitCommitMessage.title": "生成提交信息", "views.activitybar.title": "Zoo Code", "views.contextMenu.label": "Zoo Code", "views.terminalMenu.label": "Zoo Code", diff --git a/src/package.nls.zh-TW.json b/src/package.nls.zh-TW.json index cda075e306..22c66d8052 100644 --- a/src/package.nls.zh-TW.json +++ b/src/package.nls.zh-TW.json @@ -18,6 +18,7 @@ "command.showRipgrepDiagnostic.title": "顯示 Ripgrep 診斷", "command.createDiagnosticsReport.title": "建立診斷報告", "command.toggleAutoApprove.title": "切換自動批准", + "command.generateGitCommitMessage.title": "產生提交訊息", "views.activitybar.title": "Zoo Code", "views.contextMenu.label": "Zoo Code", "views.terminalMenu.label": "Zoo Code", diff --git a/src/services/git/__tests__/commitMessageGenerator.spec.ts b/src/services/git/__tests__/commitMessageGenerator.spec.ts new file mode 100644 index 0000000000..5d2a4ff08d --- /dev/null +++ b/src/services/git/__tests__/commitMessageGenerator.spec.ts @@ -0,0 +1,385 @@ +import * as vscode from "vscode" +import type { Mock } from "vitest" +import { providerIdentifiers } from "@roo-code/types" + +import { + buildCommitMessagePrompt, + cleanCommitMessage, + generateCommitMessage, + getGitApi, + getRepositoryDiff, + resolveGitRepository, + type GitAPI, + type GitRepository, +} from "../commitMessageGenerator" +import { ClineProvider } from "../../../core/webview/ClineProvider" +import { singleCompletionHandler } from "../../../utils/single-completion-handler" +import { makeUri } from "../../../test-utils/vscode" + +vi.mock("vscode", () => ({ + ProgressLocation: { + Notification: 15, + }, + extensions: { + getExtension: vi.fn(), + }, + window: { + activeTextEditor: undefined, + showErrorMessage: vi.fn(), + showInformationMessage: vi.fn(), + withProgress: vi.fn(), + }, +})) + +vi.mock("../../../utils/single-completion-handler", () => ({ + singleCompletionHandler: vi.fn(), +})) + +describe("commitMessageGenerator", () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + describe("cleanCommitMessage", () => { + it("removes markdown code fences and backticks", () => { + const input = "```markdown\nfeat(auth): add login flow\n```" + expect(cleanCommitMessage(input)).toBe("feat(auth): add login flow") + }) + + it("removes surrounding quotes", () => { + expect(cleanCommitMessage('"feat: simple commit"')).toBe("feat: simple commit") + expect(cleanCommitMessage("'feat: simple commit'")).toBe("feat: simple commit") + }) + + it("trims whitespace", () => { + expect(cleanCommitMessage(" fix: bug \n")).toBe("fix: bug") + }) + }) + + describe("buildCommitMessagePrompt", () => { + it("includes diff and instructions", () => { + const prompt = buildCommitMessagePrompt("diff --git a/file b/file\n+test") + expect(prompt).toContain("diff --git a/file b/file\n+test") + expect(prompt).toContain("Conventional Commits") + }) + }) + + describe("getGitApi", () => { + it("returns undefined if git extension not found", async () => { + ;(vscode.extensions.getExtension as Mock).mockReturnValue(undefined) + const api = await getGitApi() + expect(api).toBeUndefined() + }) + + it("activates git extension if not active and returns API", async () => { + const mockApi = { repositories: [] } + const mockExt = { + isActive: false, + activate: vi.fn().mockResolvedValue(undefined), + exports: { + getAPI: vi.fn().mockReturnValue(mockApi), + }, + } + ;(vscode.extensions.getExtension as Mock).mockReturnValue(mockExt) + + const api = await getGitApi() + expect(mockExt.activate).toHaveBeenCalled() + expect(mockExt.exports.getAPI).toHaveBeenCalledWith(1) + expect(api).toBe(mockApi) + }) + }) + + describe("resolveGitRepository", () => { + it("returns undefined if repositories list is empty", () => { + const git: GitAPI = { repositories: [] } + expect(resolveGitRepository(git)).toBeUndefined() + }) + + it("resolves from repository object context directly", () => { + const repo: GitRepository = { + rootUri: makeUri("/workspace/repo"), + inputBox: { value: "" }, + diff: vi.fn(), + } + const git: GitAPI = { repositories: [repo] } + + expect(resolveGitRepository(git, repo)).toBe(repo) + }) + + it("resolves from scm context with rootUri", () => { + const repo1: GitRepository = { + rootUri: makeUri("/workspace/repo1"), + inputBox: { value: "" }, + diff: vi.fn(), + } + const repo2: GitRepository = { + rootUri: makeUri("/workspace/repo2"), + inputBox: { value: "" }, + diff: vi.fn(), + } + const git: GitAPI = { repositories: [repo1, repo2] } + + expect(resolveGitRepository(git, { rootUri: makeUri("/workspace/repo2") })).toBe(repo2) + }) + + it("resolves from scm context with inputBox", () => { + const inputBox1 = { value: "" } + const inputBox2 = { value: "" } + const repo1: GitRepository = { + rootUri: makeUri("/workspace/repo1"), + inputBox: inputBox1, + diff: vi.fn(), + } + const repo2: GitRepository = { + rootUri: makeUri("/workspace/repo2"), + inputBox: inputBox2, + diff: vi.fn(), + } + const git: GitAPI = { repositories: [repo1, repo2] } + + expect(resolveGitRepository(git, { inputBox: inputBox2 })).toBe(repo2) + }) + + it("falls back to first repository when only one exists", () => { + const repo: GitRepository = { + rootUri: makeUri("/workspace/repo"), + inputBox: { value: "" }, + diff: vi.fn(), + } + const git: GitAPI = { repositories: [repo] } + + expect(resolveGitRepository(git)).toBe(repo) + }) + + it("matches active editor URI if multiple repositories exist", () => { + const repo1: GitRepository = { + rootUri: makeUri("/workspace/repo1"), + inputBox: { value: "" }, + diff: vi.fn(), + } + const repo2: GitRepository = { + rootUri: makeUri("/workspace/repo2"), + inputBox: { value: "" }, + diff: vi.fn(), + } + const git: GitAPI = { repositories: [repo1, repo2] } + + const mutableWindow = vscode.window as unknown as { activeTextEditor?: vscode.TextEditor } + mutableWindow.activeTextEditor = { + document: { + uri: makeUri("/workspace/repo2/src/file.ts"), + }, + } as unknown as vscode.TextEditor // Structural test double providing document.uri + + expect(resolveGitRepository(git)).toBe(repo2) + }) + }) + + describe("getRepositoryDiff", () => { + it("prioritizes staged diff", async () => { + const repo: GitRepository = { + rootUri: makeUri("/workspace/repo"), + inputBox: { value: "" }, + diff: vi.fn().mockImplementation((cached?: boolean) => { + if (cached) return Promise.resolve("staged diff") + return Promise.resolve("working diff") + }), + } + + const diff = await getRepositoryDiff(repo) + expect(diff).toBe("staged diff") + expect(repo.diff).toHaveBeenCalledWith(true) + }) + + it("falls back to working tree diff if staged diff is empty", async () => { + const repo: GitRepository = { + rootUri: makeUri("/workspace/repo"), + inputBox: { value: "" }, + diff: vi.fn().mockImplementation((cached?: boolean) => { + if (cached) return Promise.resolve("") + return Promise.resolve("working diff") + }), + } + + const diff = await getRepositoryDiff(repo) + expect(diff).toBe("working diff") + expect(repo.diff).toHaveBeenCalledWith(true) + expect(repo.diff).toHaveBeenCalledWith(false) + }) + + it("returns undefined if both staged and working diffs are empty", async () => { + const repo: GitRepository = { + rootUri: makeUri("/workspace/repo"), + inputBox: { value: "" }, + diff: vi.fn().mockResolvedValue(""), + } + + const diff = await getRepositoryDiff(repo) + expect(diff).toBeUndefined() + }) + }) + + describe("generateCommitMessage", () => { + let mockProvider: ClineProvider + let mockOutputChannel: vscode.OutputChannel + let mockRepo: GitRepository + + beforeEach(() => { + mockOutputChannel = { + appendLine: vi.fn(), + append: vi.fn(), + clear: vi.fn(), + hide: vi.fn(), + name: "test", + replace: vi.fn(), + show: vi.fn(), + dispose: vi.fn(), + } + + mockRepo = { + rootUri: makeUri("/workspace/repo"), + inputBox: { value: "" }, + diff: vi.fn().mockResolvedValue("diff content"), + } + + // Structural test double providing getState for generateCommitMessage + mockProvider = { + getState: vi.fn().mockResolvedValue({ + apiConfiguration: { + apiProvider: providerIdentifiers.anthropic, + apiKey: "test-key", + }, + }), + } as unknown as ClineProvider + + const mockGitApi: GitAPI = { + repositories: [mockRepo], + } + ;(vscode.extensions.getExtension as Mock).mockReturnValue({ + isActive: true, + exports: { + getAPI: () => mockGitApi, + }, + }) + }) + + it("shows error if git extension is unavailable", async () => { + ;(vscode.extensions.getExtension as Mock).mockReturnValue(undefined) + + await generateCommitMessage({ + provider: mockProvider, + outputChannel: mockOutputChannel, + }) + + expect(vscode.window.showErrorMessage).toHaveBeenCalledWith("Git extension not found or not enabled.") + }) + + it("shows error if no git repository found", async () => { + ;(vscode.extensions.getExtension as Mock).mockReturnValue({ + isActive: true, + exports: { + getAPI: () => ({ repositories: [] }), + }, + }) + + await generateCommitMessage({ + provider: mockProvider, + outputChannel: mockOutputChannel, + }) + + expect(vscode.window.showErrorMessage).toHaveBeenCalledWith("No Git repository found.") + }) + + it("shows info if no diff is found", async () => { + mockRepo.diff = vi.fn().mockResolvedValue("") + + await generateCommitMessage({ + provider: mockProvider, + outputChannel: mockOutputChannel, + }) + + expect(vscode.window.showInformationMessage).toHaveBeenCalledWith( + "No staged or working directory changes found to generate commit message.", + ) + }) + + it("shows error if no valid API provider is configured", async () => { + mockProvider.getState = vi.fn().mockResolvedValue({ + apiConfiguration: {}, + }) + + await generateCommitMessage({ + provider: mockProvider, + outputChannel: mockOutputChannel, + }) + + expect(vscode.window.showErrorMessage).toHaveBeenCalledWith( + "No valid API provider configured. Please configure an API provider in Zoo Code settings.", + ) + }) + + it("generates commit message and assigns it to inputBox.value", async () => { + ;(singleCompletionHandler as Mock).mockResolvedValue("feat(core): implement git commit message") + ;(vscode.window.withProgress as Mock).mockImplementation((_opts, callback) => { + const progress = { report: vi.fn() } + const token = { + isCancellationRequested: false, + onCancellationRequested: vi.fn(), + } + return callback(progress, token) + }) + + await generateCommitMessage({ + provider: mockProvider, + outputChannel: mockOutputChannel, + }) + + expect(singleCompletionHandler).toHaveBeenCalled() + expect(mockRepo.inputBox.value).toBe("feat(core): implement git commit message") + }) + + it("does not update inputBox if cancelled", async () => { + ;(singleCompletionHandler as Mock).mockResolvedValue("feat: should be cancelled") + ;(vscode.window.withProgress as Mock).mockImplementation((_opts, callback) => { + const progress = { report: vi.fn() } + let cancellationHandler: () => void = () => {} + let isCancelled = false + const token = { + get isCancellationRequested() { + return isCancelled + }, + onCancellationRequested: vi.fn((fn: () => void) => { + cancellationHandler = fn + return { dispose: vi.fn() } + }), + } + cancellationHandler() + isCancelled = true + return callback(progress, token) + }) + + await generateCommitMessage({ + provider: mockProvider, + outputChannel: mockOutputChannel, + }) + + expect(mockRepo.inputBox.value).toBe("") + }) + + it("handles errors gracefully and shows error message", async () => { + ;(vscode.window.withProgress as Mock).mockRejectedValue(new Error("Network error")) + + await generateCommitMessage({ + provider: mockProvider, + outputChannel: mockOutputChannel, + }) + + expect(mockOutputChannel.appendLine).toHaveBeenCalledWith( + expect.stringContaining("[generateGitCommitMessage] failed: Network error"), + ) + expect(vscode.window.showErrorMessage).toHaveBeenCalledWith( + "Failed to generate commit message: Network error", + ) + }) + }) +}) diff --git a/src/services/git/commitMessageGenerator.ts b/src/services/git/commitMessageGenerator.ts new file mode 100644 index 0000000000..03f425fee1 --- /dev/null +++ b/src/services/git/commitMessageGenerator.ts @@ -0,0 +1,228 @@ +import * as vscode from "vscode" +import type { ProviderSettings } from "@roo-code/types" + +import { ClineProvider } from "../../core/webview/ClineProvider" +import { truncateOutput } from "../../integrations/misc/extract-text" +import { singleCompletionHandler } from "../../utils/single-completion-handler" + +export interface GitRepository { + readonly rootUri: vscode.Uri + readonly inputBox: { + value: string + } + diff(cached?: boolean): Promise +} + +export interface GitAPI { + readonly repositories: readonly GitRepository[] +} + +export interface GitExtension { + readonly enabled?: boolean + getAPI(version: 1): GitAPI +} + +export interface GenerateCommitMessageOptions { + provider: ClineProvider + outputChannel?: vscode.OutputChannel + context?: unknown +} + +export const COMMIT_MESSAGE_SYSTEM_PROMPT = `You are an expert developer specializing in Git commit messages. +Generate a concise, meaningful Git commit message for the provided diff following the Conventional Commits specification (v1.0.0). + +Guidelines: +1. Format: (): + [optional body] + [optional footer(s)] +2. Common types: + - feat: A new feature + - fix: A bug fix + - docs: Documentation changes + - style: Formatting or code style changes (no logic change) + - refactor: Refactoring code without changing behavior + - perf: Performance improvements + - test: Adding or updating tests + - build: Changes affecting build systems or dependencies + - ci: CI/CD configuration changes + - chore: Other maintenance changes +3. Use imperative, present tense ("add" not "added", "fix" not "fixes"). +4. Keep the subject line within 72 characters and do not end with a period. +5. If the changes warrant explanation, include a blank line followed by a concise body or bullet points detailing what changed and why. +6. Return ONLY the commit message. Do NOT include markdown code blocks, quotes, or any introductory or concluding text.` + +/** + * Accesses the VS Code Git extension API. + */ +export async function getGitApi(): Promise { + const gitExtension = vscode.extensions.getExtension("vscode.git") + if (!gitExtension) { + return undefined + } + if (!gitExtension.isActive) { + await gitExtension.activate() + } + return gitExtension.exports?.getAPI(1) +} + +/** + * Resolves the appropriate Git repository given an optional SCM invocation context. + */ +export function resolveGitRepository(git: GitAPI, context?: unknown): GitRepository | undefined { + if (!git.repositories || git.repositories.length === 0) { + return undefined + } + + if (context && typeof context === "object") { + const candidate = context as Partial + if (typeof candidate.diff === "function" && candidate.inputBox && typeof candidate.inputBox === "object") { + return candidate as GitRepository + } + + const sc = context as { rootUri?: vscode.Uri; inputBox?: unknown } + if (sc.rootUri) { + const found = git.repositories.find((r) => r.rootUri.toString() === sc.rootUri?.toString()) + if (found) { + return found + } + } + + if (sc.inputBox) { + const found = git.repositories.find((r) => r.inputBox === sc.inputBox) + if (found) { + return found + } + } + } + + if (git.repositories.length === 1) { + return git.repositories[0] + } + + const activeDocUri = vscode.window.activeTextEditor?.document.uri + if (activeDocUri) { + const found = git.repositories.find((r) => activeDocUri.fsPath.startsWith(r.rootUri.fsPath)) + if (found) { + return found + } + } + + return git.repositories[0] +} + +/** + * Retrieves diff from the repository, prioritizing staged changes and falling back to working tree changes. + */ +export async function getRepositoryDiff(repository: GitRepository): Promise { + let diff = "" + try { + diff = await repository.diff(true) + } catch { + // Staged diff failed, continue to working tree diff + } + + if (!diff || !diff.trim()) { + try { + diff = await repository.diff(false) + } catch { + // Working tree diff failed + } + } + + const trimmed = diff ? diff.trim() : "" + return trimmed.length > 0 ? trimmed : undefined +} + +/** + * Cleans the commit message output from LLM completions. + */ +export function cleanCommitMessage(message: string): string { + let cleaned = message.trim() + if (cleaned.startsWith("```")) { + cleaned = cleaned.replace(/^```[a-zA-Z]*\r?\n?/, "") + cleaned = cleaned.replace(/\r?\n?```$/, "") + cleaned = cleaned.trim() + } + if ((cleaned.startsWith('"') && cleaned.endsWith('"')) || (cleaned.startsWith("'") && cleaned.endsWith("'"))) { + cleaned = cleaned.slice(1, -1).trim() + } + return cleaned +} + +/** + * Builds the full prompt to generate a commit message from a git diff. + */ +export function buildCommitMessagePrompt(diff: string): string { + const truncatedDiff = truncateOutput(diff, 1000, 50000) + return `${COMMIT_MESSAGE_SYSTEM_PROMPT}\n\nDiff:\n\`\`\`diff\n${truncatedDiff}\n\`\`\`\n\nCommit Message:` +} + +/** + * Generates a commit message using the active LLM provider and populates the SCM input box. + */ +export async function generateCommitMessage(options: GenerateCommitMessageOptions): Promise { + const { provider, outputChannel, context } = options + + try { + const git = await getGitApi() + if (!git) { + vscode.window.showErrorMessage("Git extension not found or not enabled.") + return + } + + const repository = resolveGitRepository(git, context) + if (!repository) { + vscode.window.showErrorMessage("No Git repository found.") + return + } + + const diff = await getRepositoryDiff(repository) + if (!diff) { + vscode.window.showInformationMessage("No staged or working directory changes found to generate commit message.") + return + } + + const { apiConfiguration } = await provider.getState() + if (!apiConfiguration || !apiConfiguration.apiProvider) { + vscode.window.showErrorMessage( + "No valid API provider configured. Please configure an API provider in Zoo Code settings.", + ) + return + } + + await vscode.window.withProgress( + { + location: vscode.ProgressLocation.Notification, + title: "Generating commit message...", + cancellable: true, + }, + async (_progress, token) => { + const abortController = new AbortController() + token.onCancellationRequested(() => { + abortController.abort() + }) + + const prompt = buildCommitMessagePrompt(diff) + const rawMessage = await singleCompletionHandler(apiConfiguration as ProviderSettings, prompt, { + abortSignal: abortController.signal, + }) + + if (token.isCancellationRequested || abortController.signal.aborted) { + return + } + + const commitMessage = cleanCommitMessage(rawMessage) + if (commitMessage) { + repository.inputBox.value = commitMessage + } + }, + ) + } catch (error) { + if (error instanceof Error && (error.name === "AbortError" || error.message.includes("aborted"))) { + return + } + const errorMessage = error instanceof Error ? error.message : String(error) + outputChannel?.appendLine(`[generateGitCommitMessage] failed: ${errorMessage}`) + vscode.window.showErrorMessage(`Failed to generate commit message: ${errorMessage}`) + } +} From ce2e38cce2ab4c201c3e5e9591800335d87c9832 Mon Sep 17 00:00:00 2001 From: 1220moritz Date: Mon, 2 Nov 2026 22:35:46 +0100 Subject: [PATCH 02/10] fix(scm): disambiguate commit message action icon and title for multi-extension environments - Update command title to "Generate Commit Message (Zoo Code)" across all locale files to prevent collisions with other extensions like GitHub Copilot in the SCM dropdown - Change button icon from `$(sparkle)` to `$(wand)` to give Zoo Code a visually distinct button in the Source Control input box - Add contribSourceControlInputBoxMenu proposed API declaration --- .vscode/launch.json | 10 +++++++--- src/package.json | 5 ++++- src/package.nls.ca.json | 2 +- src/package.nls.de.json | 2 +- src/package.nls.es.json | 2 +- src/package.nls.fr.json | 2 +- src/package.nls.hi.json | 2 +- src/package.nls.id.json | 2 +- src/package.nls.it.json | 2 +- src/package.nls.ja.json | 2 +- src/package.nls.json | 2 +- src/package.nls.ko.json | 2 +- src/package.nls.nl.json | 2 +- src/package.nls.pl.json | 2 +- src/package.nls.pt-BR.json | 2 +- src/package.nls.ru.json | 2 +- src/package.nls.tr.json | 2 +- src/package.nls.vi.json | 2 +- src/package.nls.zh-CN.json | 2 +- src/package.nls.zh-TW.json | 2 +- src/services/git/commitMessageGenerator.ts | 4 +++- 21 files changed, 32 insertions(+), 23 deletions(-) diff --git a/.vscode/launch.json b/.vscode/launch.json index 902b251dae..c37f4766ee 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -10,13 +10,17 @@ "type": "extensionHost", "request": "launch", "runtimeExecutable": "${execPath}", - "args": ["--extensionDevelopmentPath=${workspaceFolder}/src", "--disable-extensions"], + "args": [ + "--extensionDevelopmentPath=${workspaceFolder}/src", + "--enable-proposed-api=ZooCodeOrganization.zoo-code", + "--disable-extensions" + ], "sourceMaps": true, "outFiles": ["${workspaceFolder}/src/dist/**/*.js"], - "preLaunchTask": "${defaultBuildTask}", "env": { "NODE_ENV": "development", - "VSCODE_DEBUG_MODE": "true" + "VSCODE_DEBUG_MODE": "true", + "ROO_CODE_THEME_FIXTURE_PROBE": "1" }, "resolveSourceMapLocations": ["${workspaceFolder}/**", "!**/node_modules/**"], "presentation": { diff --git a/src/package.json b/src/package.json index 9e8cab6b22..53435f004b 100644 --- a/src/package.json +++ b/src/package.json @@ -21,6 +21,9 @@ "url": "https://github.com/Zoo-Code-Org/Zoo-Code" }, "homepage": "https://zoocode.dev", + "enabledApiProposals": [ + "contribSourceControlInputBoxMenu" + ], "categories": [ "AI", "Chat", @@ -183,7 +186,7 @@ { "command": "zoo-code.generateGitCommitMessage", "title": "%command.generateGitCommitMessage.title%", - "icon": "$(sparkle)" + "icon": "$(wand)" } ], "menus": { diff --git a/src/package.nls.ca.json b/src/package.nls.ca.json index a144328090..231c0b2e55 100644 --- a/src/package.nls.ca.json +++ b/src/package.nls.ca.json @@ -18,7 +18,7 @@ "command.showRipgrepDiagnostic.title": "Mostra el diagnòstic de Ripgrep", "command.createDiagnosticsReport.title": "Crea un informe de diagnòstic", "command.toggleAutoApprove.title": "Alternar Auto-Aprovació", - "command.generateGitCommitMessage.title": "Generar Missatge de Commit", + "command.generateGitCommitMessage.title": "Generar Missatge de Commit (Zoo Code)", "views.activitybar.title": "Zoo Code", "views.contextMenu.label": "Zoo Code", "views.terminalMenu.label": "Zoo Code", diff --git a/src/package.nls.de.json b/src/package.nls.de.json index c8f048d935..05a57bdf5b 100644 --- a/src/package.nls.de.json +++ b/src/package.nls.de.json @@ -18,7 +18,7 @@ "command.showRipgrepDiagnostic.title": "Ripgrep-Diagnose anzeigen", "command.createDiagnosticsReport.title": "Diagnosebericht erstellen", "command.toggleAutoApprove.title": "Auto-Genehmigung Umschalten", - "command.generateGitCommitMessage.title": "Commit-Nachricht generieren", + "command.generateGitCommitMessage.title": "Commit-Nachricht generieren (Zoo Code)", "views.activitybar.title": "Zoo Code", "views.contextMenu.label": "Zoo Code", "views.terminalMenu.label": "Zoo Code", diff --git a/src/package.nls.es.json b/src/package.nls.es.json index b0a59a5163..175a53e9be 100644 --- a/src/package.nls.es.json +++ b/src/package.nls.es.json @@ -18,7 +18,7 @@ "command.showRipgrepDiagnostic.title": "Mostrar diagnóstico de Ripgrep", "command.createDiagnosticsReport.title": "Crear informe de diagnóstico", "command.toggleAutoApprove.title": "Alternar Auto-Aprobación", - "command.generateGitCommitMessage.title": "Generar mensaje de confirmación", + "command.generateGitCommitMessage.title": "Generar mensaje de confirmación (Zoo Code)", "views.activitybar.title": "Zoo Code", "views.contextMenu.label": "Zoo Code", "views.terminalMenu.label": "Zoo Code", diff --git a/src/package.nls.fr.json b/src/package.nls.fr.json index feb40a8211..9e71eb9a94 100644 --- a/src/package.nls.fr.json +++ b/src/package.nls.fr.json @@ -18,7 +18,7 @@ "command.showRipgrepDiagnostic.title": "Afficher le diagnostic Ripgrep", "command.createDiagnosticsReport.title": "Créer un rapport de diagnostic", "command.toggleAutoApprove.title": "Basculer Auto-Approbation", - "command.generateGitCommitMessage.title": "Générer un message de commit", + "command.generateGitCommitMessage.title": "Générer un message de commit (Zoo Code)", "views.activitybar.title": "Zoo Code", "views.contextMenu.label": "Zoo Code", "views.terminalMenu.label": "Zoo Code", diff --git a/src/package.nls.hi.json b/src/package.nls.hi.json index 8b2eb5f1f1..62a352cd81 100644 --- a/src/package.nls.hi.json +++ b/src/package.nls.hi.json @@ -18,7 +18,7 @@ "command.showRipgrepDiagnostic.title": "Ripgrep डायग्नोस्टिक दिखाएं", "command.createDiagnosticsReport.title": "डायग्नोस्टिक रिपोर्ट बनाएं", "command.toggleAutoApprove.title": "ऑटो-अनुमोदन टॉगल करें", - "command.generateGitCommitMessage.title": "कमिट संदेश बनाएं", + "command.generateGitCommitMessage.title": "कमिट संदेश बनाएं (Zoo Code)", "views.activitybar.title": "Zoo Code", "views.contextMenu.label": "Zoo Code", "views.terminalMenu.label": "Zoo Code", diff --git a/src/package.nls.id.json b/src/package.nls.id.json index 2298fc3f24..31ac13fcc6 100644 --- a/src/package.nls.id.json +++ b/src/package.nls.id.json @@ -27,7 +27,7 @@ "command.showRipgrepDiagnostic.title": "Tampilkan Diagnostik Ripgrep", "command.createDiagnosticsReport.title": "Buat Laporan Diagnostik", "command.toggleAutoApprove.title": "Alihkan Persetujuan Otomatis", - "command.generateGitCommitMessage.title": "Buat Pesan Commit", + "command.generateGitCommitMessage.title": "Buat Pesan Commit (Zoo Code)", "configuration.title": "Zoo Code", "commands.allowedCommands.description": "Perintah yang dapat dijalankan secara otomatis ketika 'Selalu setujui operasi eksekusi' diaktifkan", "commands.deniedCommands.description": "Awalan perintah yang akan otomatis ditolak tanpa meminta persetujuan. Jika terjadi konflik dengan perintah yang diizinkan, pencocokan awalan terpanjang akan diprioritaskan. Tambahkan * untuk menolak semua perintah.", diff --git a/src/package.nls.it.json b/src/package.nls.it.json index deb1575a6f..ef3a0ff2dc 100644 --- a/src/package.nls.it.json +++ b/src/package.nls.it.json @@ -18,7 +18,7 @@ "command.showRipgrepDiagnostic.title": "Mostra diagnostica Ripgrep", "command.createDiagnosticsReport.title": "Crea rapporto di diagnostica", "command.toggleAutoApprove.title": "Attiva/Disattiva Auto-Approvazione", - "command.generateGitCommitMessage.title": "Genera Messaggio di Commit", + "command.generateGitCommitMessage.title": "Genera Messaggio di Commit (Zoo Code)", "views.activitybar.title": "Zoo Code", "views.contextMenu.label": "Zoo Code", "views.terminalMenu.label": "Zoo Code", diff --git a/src/package.nls.ja.json b/src/package.nls.ja.json index 5b8797ccff..d2ce85c395 100644 --- a/src/package.nls.ja.json +++ b/src/package.nls.ja.json @@ -27,7 +27,7 @@ "command.showRipgrepDiagnostic.title": "Ripgrep 診断を表示", "command.createDiagnosticsReport.title": "診断レポートを作成", "command.toggleAutoApprove.title": "自動承認を切替", - "command.generateGitCommitMessage.title": "コミットメッセージの生成", + "command.generateGitCommitMessage.title": "コミットメッセージの生成 (Zoo Code)", "configuration.title": "Zoo Code", "commands.allowedCommands.description": "'常に実行操作を承認する'が有効な場合に自動実行できるコマンド", "commands.deniedCommands.description": "承認を求めずに自動的に拒否されるコマンドプレフィックス。許可されたコマンドとの競合がある場合、最長プレフィックスマッチが優先されます。すべてのコマンドを拒否するには * を追加してください。", diff --git a/src/package.nls.json b/src/package.nls.json index 1f0818516c..cc0c7fd454 100644 --- a/src/package.nls.json +++ b/src/package.nls.json @@ -27,7 +27,7 @@ "command.showRipgrepDiagnostic.title": "Show Ripgrep Diagnostic", "command.createDiagnosticsReport.title": "Create Diagnostics Report", "command.toggleAutoApprove.title": "Toggle Auto-Approve", - "command.generateGitCommitMessage.title": "Generate Commit Message", + "command.generateGitCommitMessage.title": "Generate Commit Message (Zoo Code)", "configuration.title": "Zoo Code", "commands.allowedCommands.description": "Commands that can be auto-executed when 'Always approve execute operations' is enabled", "commands.deniedCommands.description": "Command prefixes that will be automatically denied without asking for approval. In case of conflicts with allowed commands, the longest prefix match takes precedence. Add * to deny all commands.", diff --git a/src/package.nls.ko.json b/src/package.nls.ko.json index f33415292b..9397f4d752 100644 --- a/src/package.nls.ko.json +++ b/src/package.nls.ko.json @@ -18,7 +18,7 @@ "command.showRipgrepDiagnostic.title": "Ripgrep 진단 표시", "command.createDiagnosticsReport.title": "진단 보고서 만들기", "command.toggleAutoApprove.title": "자동 승인 전환", - "command.generateGitCommitMessage.title": "커밋 메시지 생성", + "command.generateGitCommitMessage.title": "커밋 메시지 생성 (Zoo Code)", "views.activitybar.title": "Zoo Code", "views.contextMenu.label": "Zoo Code", "views.terminalMenu.label": "Zoo Code", diff --git a/src/package.nls.nl.json b/src/package.nls.nl.json index c77b273630..c2c9699ce5 100644 --- a/src/package.nls.nl.json +++ b/src/package.nls.nl.json @@ -27,7 +27,7 @@ "command.showRipgrepDiagnostic.title": "Ripgrep-diagnose weergeven", "command.createDiagnosticsReport.title": "Diagnoserapport maken", "command.toggleAutoApprove.title": "Auto-Goedkeuring Schakelen", - "command.generateGitCommitMessage.title": "Commitbericht genereren", + "command.generateGitCommitMessage.title": "Commitbericht genereren (Zoo Code)", "configuration.title": "Zoo Code", "commands.allowedCommands.description": "Commando's die automatisch kunnen worden uitgevoerd wanneer 'Altijd goedkeuren uitvoerbewerkingen' is ingeschakeld", "commands.deniedCommands.description": "Commando-prefixen die automatisch worden geweigerd zonder om goedkeuring te vragen. Bij conflicten met toegestane commando's heeft de langste prefix-match voorrang. Voeg * toe om alle commando's te weigeren.", diff --git a/src/package.nls.pl.json b/src/package.nls.pl.json index 860bac18ed..b4d0c54539 100644 --- a/src/package.nls.pl.json +++ b/src/package.nls.pl.json @@ -18,7 +18,7 @@ "command.showRipgrepDiagnostic.title": "Pokaż diagnostykę Ripgrep", "command.createDiagnosticsReport.title": "Utwórz raport diagnostyczny", "command.toggleAutoApprove.title": "Przełącz Auto-Zatwierdzanie", - "command.generateGitCommitMessage.title": "Wygeneruj komunikat commita", + "command.generateGitCommitMessage.title": "Wygeneruj komunikat commita (Zoo Code)", "views.activitybar.title": "Zoo Code", "views.contextMenu.label": "Zoo Code", "views.terminalMenu.label": "Zoo Code", diff --git a/src/package.nls.pt-BR.json b/src/package.nls.pt-BR.json index 0de9a3f51c..7ec732d98c 100644 --- a/src/package.nls.pt-BR.json +++ b/src/package.nls.pt-BR.json @@ -18,7 +18,7 @@ "command.showRipgrepDiagnostic.title": "Mostrar diagnóstico do Ripgrep", "command.createDiagnosticsReport.title": "Criar relatório de diagnóstico", "command.toggleAutoApprove.title": "Alternar Auto-Aprovação", - "command.generateGitCommitMessage.title": "Gerar mensagem de commit", + "command.generateGitCommitMessage.title": "Gerar mensagem de commit (Zoo Code)", "views.activitybar.title": "Zoo Code", "views.contextMenu.label": "Zoo Code", "views.terminalMenu.label": "Zoo Code", diff --git a/src/package.nls.ru.json b/src/package.nls.ru.json index cdfd27bfbd..3ad4f5ada0 100644 --- a/src/package.nls.ru.json +++ b/src/package.nls.ru.json @@ -27,7 +27,7 @@ "command.showRipgrepDiagnostic.title": "Показать диагностику Ripgrep", "command.createDiagnosticsReport.title": "Создать диагностический отчёт", "command.toggleAutoApprove.title": "Переключить Авто-Подтверждение", - "command.generateGitCommitMessage.title": "Сгенерировать сообщение коммита", + "command.generateGitCommitMessage.title": "Сгенерировать сообщение коммита (Zoo Code)", "configuration.title": "Zoo Code", "commands.allowedCommands.description": "Команды, которые могут быть автоматически выполнены, когда включена опция 'Всегда подтверждать операции выполнения'", "commands.deniedCommands.description": "Префиксы команд, которые будут автоматически отклонены без запроса подтверждения. В случае конфликтов с разрешенными командами приоритет имеет самое длинное совпадение префикса. Добавьте * чтобы отклонить все команды.", diff --git a/src/package.nls.tr.json b/src/package.nls.tr.json index 8f7955b582..799f8c0d2c 100644 --- a/src/package.nls.tr.json +++ b/src/package.nls.tr.json @@ -18,7 +18,7 @@ "command.showRipgrepDiagnostic.title": "Ripgrep Tanılamasını Göster", "command.createDiagnosticsReport.title": "Tanılama Raporu Oluştur", "command.toggleAutoApprove.title": "Otomatik Onayı Değiştir", - "command.generateGitCommitMessage.title": "Commit Mesajı Oluştur", + "command.generateGitCommitMessage.title": "Commit Mesajı Oluştur (Zoo Code)", "views.activitybar.title": "Zoo Code", "views.contextMenu.label": "Zoo Code", "views.terminalMenu.label": "Zoo Code", diff --git a/src/package.nls.vi.json b/src/package.nls.vi.json index 0b346dee69..36d255e048 100644 --- a/src/package.nls.vi.json +++ b/src/package.nls.vi.json @@ -18,7 +18,7 @@ "command.showRipgrepDiagnostic.title": "Hiển thị chẩn đoán Ripgrep", "command.createDiagnosticsReport.title": "Tạo báo cáo chẩn đoán", "command.toggleAutoApprove.title": "Bật/Tắt Tự Động Phê Duyệt", - "command.generateGitCommitMessage.title": "Tạo thông điệp commit", + "command.generateGitCommitMessage.title": "Tạo thông điệp commit (Zoo Code)", "views.activitybar.title": "Zoo Code", "views.contextMenu.label": "Zoo Code", "views.terminalMenu.label": "Zoo Code", diff --git a/src/package.nls.zh-CN.json b/src/package.nls.zh-CN.json index eb7745b3bb..5005fd0b5d 100644 --- a/src/package.nls.zh-CN.json +++ b/src/package.nls.zh-CN.json @@ -18,7 +18,7 @@ "command.showRipgrepDiagnostic.title": "显示 Ripgrep 诊断", "command.createDiagnosticsReport.title": "创建诊断报告", "command.toggleAutoApprove.title": "切换自动批准", - "command.generateGitCommitMessage.title": "生成提交信息", + "command.generateGitCommitMessage.title": "生成提交信息 (Zoo Code)", "views.activitybar.title": "Zoo Code", "views.contextMenu.label": "Zoo Code", "views.terminalMenu.label": "Zoo Code", diff --git a/src/package.nls.zh-TW.json b/src/package.nls.zh-TW.json index 22c66d8052..1b4a7ec91b 100644 --- a/src/package.nls.zh-TW.json +++ b/src/package.nls.zh-TW.json @@ -18,7 +18,7 @@ "command.showRipgrepDiagnostic.title": "顯示 Ripgrep 診斷", "command.createDiagnosticsReport.title": "建立診斷報告", "command.toggleAutoApprove.title": "切換自動批准", - "command.generateGitCommitMessage.title": "產生提交訊息", + "command.generateGitCommitMessage.title": "產生提交訊息 (Zoo Code)", "views.activitybar.title": "Zoo Code", "views.contextMenu.label": "Zoo Code", "views.terminalMenu.label": "Zoo Code", diff --git a/src/services/git/commitMessageGenerator.ts b/src/services/git/commitMessageGenerator.ts index 03f425fee1..5aa847e4d7 100644 --- a/src/services/git/commitMessageGenerator.ts +++ b/src/services/git/commitMessageGenerator.ts @@ -178,7 +178,9 @@ export async function generateCommitMessage(options: GenerateCommitMessageOption const diff = await getRepositoryDiff(repository) if (!diff) { - vscode.window.showInformationMessage("No staged or working directory changes found to generate commit message.") + vscode.window.showInformationMessage( + "No staged or working directory changes found to generate commit message.", + ) return } From c03fcdd416333ade2ebae5092fd9014446f39da4 Mon Sep 17 00:00:00 2001 From: 1220moritz Date: Tue, 3 Nov 2026 00:14:33 +0100 Subject: [PATCH 03/10] fix(scm): address CodeRabbit review findings for generate commit message feature - Restore preLaunchTask and remove ROO_CODE_THEME_FIXTURE_PROBE from launch.json - Remove enabledApiProposals and tighten SCM menu when clause to scmProvider == git - Add redactSensitiveInformation() to sanitize diffs before LLM transmission - Support bare vscode.Uri context via duck-typing (no instanceof) for SCM input box - Fix path segment boundary check (path.sep) and longest-root match for active editor - Wrap singleCompletionHandler in try/finally to dispose cancellationListener - Show warning when generated commit message is empty after cleaning - Surface untracked files as fallback when both staged and working diffs are empty - Add GitRepositoryChange/State interfaces to support untracked file detection - Add tests: redaction, bare Uri resolution, untracked fallback, abort wiring --- .vscode/launch.json | 10 +- src/package.json | 5 +- .../__tests__/commitMessageGenerator.spec.ts | 343 +++++++++++++++++- src/services/git/commitMessageGenerator.ts | 106 +++++- 4 files changed, 434 insertions(+), 30 deletions(-) diff --git a/.vscode/launch.json b/.vscode/launch.json index c37f4766ee..902b251dae 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -10,17 +10,13 @@ "type": "extensionHost", "request": "launch", "runtimeExecutable": "${execPath}", - "args": [ - "--extensionDevelopmentPath=${workspaceFolder}/src", - "--enable-proposed-api=ZooCodeOrganization.zoo-code", - "--disable-extensions" - ], + "args": ["--extensionDevelopmentPath=${workspaceFolder}/src", "--disable-extensions"], "sourceMaps": true, "outFiles": ["${workspaceFolder}/src/dist/**/*.js"], + "preLaunchTask": "${defaultBuildTask}", "env": { "NODE_ENV": "development", - "VSCODE_DEBUG_MODE": "true", - "ROO_CODE_THEME_FIXTURE_PROBE": "1" + "VSCODE_DEBUG_MODE": "true" }, "resolveSourceMapLocations": ["${workspaceFolder}/**", "!**/node_modules/**"], "presentation": { diff --git a/src/package.json b/src/package.json index 53435f004b..321859b1ce 100644 --- a/src/package.json +++ b/src/package.json @@ -21,9 +21,6 @@ "url": "https://github.com/Zoo-Code-Org/Zoo-Code" }, "homepage": "https://zoocode.dev", - "enabledApiProposals": [ - "contribSourceControlInputBoxMenu" - ], "categories": [ "AI", "Chat", @@ -194,7 +191,7 @@ { "command": "zoo-code.generateGitCommitMessage", "group": "inline", - "when": "scmRepository" + "when": "scmRepository && scmProvider == git" } ], "editor/context": [ diff --git a/src/services/git/__tests__/commitMessageGenerator.spec.ts b/src/services/git/__tests__/commitMessageGenerator.spec.ts index 5d2a4ff08d..aa8a9fe85a 100644 --- a/src/services/git/__tests__/commitMessageGenerator.spec.ts +++ b/src/services/git/__tests__/commitMessageGenerator.spec.ts @@ -8,6 +8,7 @@ import { generateCommitMessage, getGitApi, getRepositoryDiff, + redactSensitiveInformation, resolveGitRepository, type GitAPI, type GitRepository, @@ -27,8 +28,13 @@ vi.mock("vscode", () => ({ activeTextEditor: undefined, showErrorMessage: vi.fn(), showInformationMessage: vi.fn(), + showWarningMessage: vi.fn(), withProgress: vi.fn(), }, + Uri: { + file: (path: string) => makeUri(path), + parse: (uri: string) => makeUri(uri), + }, })) vi.mock("../../../utils/single-completion-handler", () => ({ @@ -38,6 +44,7 @@ vi.mock("../../../utils/single-completion-handler", () => ({ describe("commitMessageGenerator", () => { beforeEach(() => { vi.clearAllMocks() + ;(vscode.window as unknown as { activeTextEditor?: vscode.TextEditor }).activeTextEditor = undefined }) describe("cleanCommitMessage", () => { @@ -56,12 +63,39 @@ describe("commitMessageGenerator", () => { }) }) + describe("redactSensitiveInformation", () => { + it("redacts private keys", () => { + const content = + "-----BEGIN RSA PRIVATE KEY-----\nMIIEowIBAAKCAQEA0...\n-----END RSA PRIVATE KEY-----\nother text" + expect(redactSensitiveInformation(content)).toBe("[REDACTED_SECRET]\nother text") + }) + + it("redacts api keys and secrets", () => { + const content = 'apiKey: "super-secret-key-12345"\npassword=mysecretpassword123\nother_config=123' + const redacted = redactSensitiveInformation(content) + expect(redacted).toContain('apiKey: "[REDACTED]"') + expect(redacted).toContain("password=[REDACTED]") + expect(redacted).toContain("other_config=123") + }) + + it("leaves normal diff content unchanged", () => { + const content = "diff --git a/src/app.ts b/src/app.ts\n+console.log('hello world')" + expect(redactSensitiveInformation(content)).toBe(content) + }) + }) + describe("buildCommitMessagePrompt", () => { it("includes diff and instructions", () => { const prompt = buildCommitMessagePrompt("diff --git a/file b/file\n+test") expect(prompt).toContain("diff --git a/file b/file\n+test") expect(prompt).toContain("Conventional Commits") }) + + it("redacts sensitive information in the prompt", () => { + const prompt = buildCommitMessagePrompt("apiKey: 'secret_token_12345678'") + expect(prompt).not.toContain("secret_token_12345678") + expect(prompt).toContain("[REDACTED]") + }) }) describe("getGitApi", () => { @@ -106,6 +140,22 @@ describe("commitMessageGenerator", () => { expect(resolveGitRepository(git, repo)).toBe(repo) }) + it("resolves from bare vscode.Uri context", () => { + const repo1: GitRepository = { + rootUri: makeUri("/workspace/repo1"), + inputBox: { value: "" }, + diff: vi.fn(), + } + const repo2: GitRepository = { + rootUri: makeUri("/workspace/repo2"), + inputBox: { value: "" }, + diff: vi.fn(), + } + const git: GitAPI = { repositories: [repo1, repo2] } + + expect(resolveGitRepository(git, makeUri("/workspace/repo2"))).toBe(repo2) + }) + it("resolves from scm context with rootUri", () => { const repo1: GitRepository = { rootUri: makeUri("/workspace/repo1"), @@ -173,6 +223,123 @@ describe("commitMessageGenerator", () => { expect(resolveGitRepository(git)).toBe(repo2) }) + + it("falls through if bare vscode.Uri context does not match any repository", () => { + const repo1: GitRepository = { + rootUri: makeUri("/workspace/repo1"), + inputBox: { value: "" }, + diff: vi.fn(), + } + const repo2: GitRepository = { + rootUri: makeUri("/workspace/repo2"), + inputBox: { value: "" }, + diff: vi.fn(), + } + const git: GitAPI = { repositories: [repo1, repo2] } + + expect(resolveGitRepository(git, makeUri("/workspace/unknown"))).toBe(repo1) + }) + + it("falls through if scm context rootUri does not match any repository", () => { + const repo1: GitRepository = { + rootUri: makeUri("/workspace/repo1"), + inputBox: { value: "" }, + diff: vi.fn(), + } + const repo2: GitRepository = { + rootUri: makeUri("/workspace/repo2"), + inputBox: { value: "" }, + diff: vi.fn(), + } + const git: GitAPI = { repositories: [repo1, repo2] } + + expect(resolveGitRepository(git, { rootUri: makeUri("/workspace/unknown") })).toBe(repo1) + }) + + it("falls through if scm context inputBox does not match any repository", () => { + const repo1: GitRepository = { + rootUri: makeUri("/workspace/repo1"), + inputBox: { value: "" }, + diff: vi.fn(), + } + const repo2: GitRepository = { + rootUri: makeUri("/workspace/repo2"), + inputBox: { value: "" }, + diff: vi.fn(), + } + const git: GitAPI = { repositories: [repo1, repo2] } + + expect(resolveGitRepository(git, { inputBox: { value: "other" } })).toBe(repo1) + }) + + it("falls back to first repository when active editor does not match any repository", () => { + const repo1: GitRepository = { + rootUri: makeUri("/workspace/repo1"), + inputBox: { value: "" }, + diff: vi.fn(), + } + const repo2: GitRepository = { + rootUri: makeUri("/workspace/repo2"), + inputBox: { value: "" }, + diff: vi.fn(), + } + const git: GitAPI = { repositories: [repo1, repo2] } + + const mutableWindow = vscode.window as unknown as { activeTextEditor?: vscode.TextEditor } + mutableWindow.activeTextEditor = { + document: { + uri: makeUri("/workspace/other/file.ts"), + }, + } as unknown as vscode.TextEditor // Structural test double providing document.uri + + expect(resolveGitRepository(git)).toBe(repo1) + }) + + it("distinguishes repository roots on path segment boundaries", () => { + const repo1: GitRepository = { + rootUri: makeUri("/workspace/repo"), + inputBox: { value: "" }, + diff: vi.fn(), + } + const repo2: GitRepository = { + rootUri: makeUri("/workspace/repo-other"), + inputBox: { value: "" }, + diff: vi.fn(), + } + const git: GitAPI = { repositories: [repo1, repo2] } + + const mutableWindow = vscode.window as unknown as { activeTextEditor?: vscode.TextEditor } + mutableWindow.activeTextEditor = { + document: { + uri: makeUri("/workspace/repo-other/file.ts"), + }, + } as unknown as vscode.TextEditor + + expect(resolveGitRepository(git)).toBe(repo2) + }) + + it("matches longest root when repositories are nested", () => { + const parentRepo: GitRepository = { + rootUri: makeUri("/workspace/monorepo"), + inputBox: { value: "" }, + diff: vi.fn(), + } + const childRepo: GitRepository = { + rootUri: makeUri("/workspace/monorepo/packages/nested"), + inputBox: { value: "" }, + diff: vi.fn(), + } + const git: GitAPI = { repositories: [parentRepo, childRepo] } + + const mutableWindow = vscode.window as unknown as { activeTextEditor?: vscode.TextEditor } + mutableWindow.activeTextEditor = { + document: { + uri: makeUri("/workspace/monorepo/packages/nested/src/index.ts"), + }, + } as unknown as vscode.TextEditor + + expect(resolveGitRepository(git)).toBe(childRepo) + }) }) describe("getRepositoryDiff", () => { @@ -207,6 +374,22 @@ describe("commitMessageGenerator", () => { expect(repo.diff).toHaveBeenCalledWith(false) }) + it("falls back to working tree diff if staged diff throws", async () => { + const repo: GitRepository = { + rootUri: makeUri("/workspace/repo"), + inputBox: { value: "" }, + diff: vi.fn().mockImplementation((cached?: boolean) => { + if (cached) return Promise.reject(new Error("Git diff staged failed")) + return Promise.resolve("working diff fallback") + }), + } + + const diff = await getRepositoryDiff(repo) + expect(diff).toBe("working diff fallback") + expect(repo.diff).toHaveBeenCalledWith(true) + expect(repo.diff).toHaveBeenCalledWith(false) + }) + it("returns undefined if both staged and working diffs are empty", async () => { const repo: GitRepository = { rootUri: makeUri("/workspace/repo"), @@ -217,6 +400,55 @@ describe("commitMessageGenerator", () => { const diff = await getRepositoryDiff(repo) expect(diff).toBeUndefined() }) + + it("returns undefined if working diff throws after empty staged diff", async () => { + const repo: GitRepository = { + rootUri: makeUri("/workspace/repo"), + inputBox: { value: "" }, + diff: vi.fn().mockImplementation((cached?: boolean) => { + if (cached) return Promise.resolve("") + return Promise.reject(new Error("Git working tree diff failed")) + }), + } + + const diff = await getRepositoryDiff(repo) + expect(diff).toBeUndefined() + }) + + it("surfaces untracked files when both diffs are empty", async () => { + const repo: GitRepository = { + rootUri: makeUri("/workspace/repo"), + inputBox: { value: "" }, + diff: vi.fn().mockResolvedValue(""), + state: { + workingTreeChanges: [ + { uri: makeUri("/workspace/repo/new-file.ts"), status: 7 }, // UNTRACKED + { uri: makeUri("/workspace/repo/modified.ts"), status: 5 }, // MODIFIED + ], + }, + } + + const diff = await getRepositoryDiff(repo) + expect(diff).toContain("Untracked files:") + expect(diff).toContain("new-file.ts") + expect(diff).not.toContain("modified.ts") + }) + + it("returns undefined when both diffs are empty and no untracked files exist", async () => { + const repo: GitRepository = { + rootUri: makeUri("/workspace/repo"), + inputBox: { value: "" }, + diff: vi.fn().mockResolvedValue(""), + state: { + workingTreeChanges: [ + { uri: makeUri("/workspace/repo/modified.ts"), status: 5 }, // MODIFIED (not untracked) + ], + }, + } + + const diff = await getRepositoryDiff(repo) + expect(diff).toBeUndefined() + }) }) describe("generateCommitMessage", () => { @@ -324,7 +556,7 @@ describe("commitMessageGenerator", () => { const progress = { report: vi.fn() } const token = { isCancellationRequested: false, - onCancellationRequested: vi.fn(), + onCancellationRequested: vi.fn().mockReturnValue({ dispose: vi.fn() }), } return callback(progress, token) }) @@ -366,6 +598,115 @@ describe("commitMessageGenerator", () => { expect(mockRepo.inputBox.value).toBe("") }) + it("aborts when token triggers cancellation callback", async () => { + let cancellationHandler: (() => void) | undefined + ;(vscode.window.withProgress as Mock).mockImplementation((_opts, callback) => { + const progress = { report: vi.fn() } + const token = { + isCancellationRequested: false, + onCancellationRequested: vi.fn((fn: () => void) => { + cancellationHandler = fn + return { dispose: vi.fn() } + }), + } + return callback(progress, token) + }) + ;(singleCompletionHandler as Mock).mockImplementation(async (_config, _prompt, options) => { + cancellationHandler?.() + expect(options.abortSignal.aborted).toBe(true) + return "" + }) + + await generateCommitMessage({ + provider: mockProvider, + outputChannel: mockOutputChannel, + }) + + expect(mockRepo.inputBox.value).toBe("") + }) + + it("aborts the in-flight provider request when cancellation is requested", async () => { + let cancelHandler: () => void = () => {} + let capturedSignal: AbortSignal | undefined + ;(singleCompletionHandler as Mock).mockImplementation((_config, _prompt, options) => { + capturedSignal = options.abortSignal as AbortSignal + cancelHandler() + return Promise.resolve("feat: should be cancelled") + }) + ;(vscode.window.withProgress as Mock).mockImplementation((_opts, callback) => { + const progress = { report: vi.fn() } + const token = { + isCancellationRequested: false, + onCancellationRequested: vi.fn((fn: () => void) => { + cancelHandler = fn + return { dispose: vi.fn() } + }), + } + return callback(progress, token) + }) + + await generateCommitMessage({ + provider: mockProvider, + outputChannel: mockOutputChannel, + }) + + expect(capturedSignal?.aborted).toBe(true) + expect(mockRepo.inputBox.value).toBe("") + }) + + it("does not update inputBox and shows warning if generated message is empty", async () => { + mockRepo.inputBox.value = "preserved value" + const mockDisposable = { dispose: vi.fn() } + ;(singleCompletionHandler as Mock).mockResolvedValue(" ") + ;(vscode.window.withProgress as Mock).mockImplementation((_opts, callback) => { + const progress = { report: vi.fn() } + const token = { + isCancellationRequested: false, + onCancellationRequested: vi.fn().mockReturnValue(mockDisposable), + } + return callback(progress, token) + }) + + await generateCommitMessage({ + provider: mockProvider, + outputChannel: mockOutputChannel, + }) + + expect(mockRepo.inputBox.value).toBe("preserved value") + expect(mockDisposable.dispose).toHaveBeenCalled() + expect(vscode.window.showWarningMessage).toHaveBeenCalledWith("No commit message generated.") + }) + + it("ignores AbortError without showing an error message", async () => { + const abortError = new Error("This operation was aborted") + abortError.name = "AbortError" + ;(vscode.window.withProgress as Mock).mockRejectedValue(abortError) + + await generateCommitMessage({ + provider: mockProvider, + outputChannel: mockOutputChannel, + }) + + expect(vscode.window.showErrorMessage).not.toHaveBeenCalled() + expect(mockOutputChannel.appendLine).not.toHaveBeenCalled() + }) + + it("handles non-Error objects gracefully and shows error message", async () => { + ;(vscode.window.withProgress as Mock).mockRejectedValue("String-based error") + + await generateCommitMessage({ + provider: mockProvider, + outputChannel: mockOutputChannel, + }) + + expect(mockOutputChannel.appendLine).toHaveBeenCalledWith( + expect.stringContaining("[generateGitCommitMessage] failed: String-based error"), + ) + expect(vscode.window.showErrorMessage).toHaveBeenCalledWith( + "Failed to generate commit message: String-based error", + ) + }) + it("handles errors gracefully and shows error message", async () => { ;(vscode.window.withProgress as Mock).mockRejectedValue(new Error("Network error")) diff --git a/src/services/git/commitMessageGenerator.ts b/src/services/git/commitMessageGenerator.ts index 5aa847e4d7..659c6f3c02 100644 --- a/src/services/git/commitMessageGenerator.ts +++ b/src/services/git/commitMessageGenerator.ts @@ -1,3 +1,4 @@ +import path from "path" import * as vscode from "vscode" import type { ProviderSettings } from "@roo-code/types" @@ -5,11 +6,21 @@ import { ClineProvider } from "../../core/webview/ClineProvider" import { truncateOutput } from "../../integrations/misc/extract-text" import { singleCompletionHandler } from "../../utils/single-completion-handler" +export interface GitRepositoryChange { + readonly uri: vscode.Uri + readonly status: number +} + +export interface GitRepositoryState { + readonly workingTreeChanges?: ReadonlyArray +} + export interface GitRepository { readonly rootUri: vscode.Uri readonly inputBox: { value: string } + readonly state?: GitRepositoryState diff(cached?: boolean): Promise } @@ -49,7 +60,29 @@ Guidelines: 3. Use imperative, present tense ("add" not "added", "fix" not "fixes"). 4. Keep the subject line within 72 characters and do not end with a period. 5. If the changes warrant explanation, include a blank line followed by a concise body or bullet points detailing what changed and why. -6. Return ONLY the commit message. Do NOT include markdown code blocks, quotes, or any introductory or concluding text.` +6. Return ONLY the commit message. Do NOT include markdown code blocks, quotes, or any introductory or concluding text. +7. Never output secrets, passwords, or personal identifiable information in commit messages.` + +const SENSITIVE_PATTERNS = [ + /-----BEGIN [A-Z ]*PRIVATE KEY-----[\s\S]*?-----END [A-Z ]*PRIVATE KEY-----/g, + /(?:api[_-]?key|auth[_-]?token|bearer|secret|password)[\s]*[=:]\s*['"]?([a-zA-Z0-9_\-\.]{8,})['"]?/gi, +] + +/** + * Redacts secrets, private keys, and credential tokens from content before sending to an LLM. + */ +export function redactSensitiveInformation(content: string): string { + let redacted = content + for (const pattern of SENSITIVE_PATTERNS) { + redacted = redacted.replace(pattern, (match, captured) => { + if (captured) { + return match.replace(captured, "[REDACTED]") + } + return "[REDACTED_SECRET]" + }) + } + return redacted +} /** * Accesses the VS Code Git extension API. @@ -74,6 +107,17 @@ export function resolveGitRepository(git: GitAPI, context?: unknown): GitReposit } if (context && typeof context === "object") { + // Bare vscode.Uri or URI-like object passed from scm/inputBox menu + if ("scheme" in context && "fsPath" in context) { + const uri = context as vscode.Uri + const found = git.repositories.find( + (r) => r.rootUri.toString() === uri.toString() || r.rootUri.fsPath === uri.fsPath, + ) + if (found) { + return found + } + } + const candidate = context as Partial if (typeof candidate.diff === "function" && candidate.inputBox && typeof candidate.inputBox === "object") { return candidate as GitRepository @@ -81,7 +125,9 @@ export function resolveGitRepository(git: GitAPI, context?: unknown): GitReposit const sc = context as { rootUri?: vscode.Uri; inputBox?: unknown } if (sc.rootUri) { - const found = git.repositories.find((r) => r.rootUri.toString() === sc.rootUri?.toString()) + const found = git.repositories.find( + (r) => r.rootUri.toString() === sc.rootUri?.toString() || r.rootUri.fsPath === sc.rootUri?.fsPath, + ) if (found) { return found } @@ -101,9 +147,13 @@ export function resolveGitRepository(git: GitAPI, context?: unknown): GitReposit const activeDocUri = vscode.window.activeTextEditor?.document.uri if (activeDocUri) { - const found = git.repositories.find((r) => activeDocUri.fsPath.startsWith(r.rootUri.fsPath)) - if (found) { - return found + const matchingRepos = git.repositories.filter((r) => { + const root = r.rootUri.fsPath + return activeDocUri.fsPath === root || activeDocUri.fsPath.startsWith(root + path.sep) + }) + if (matchingRepos.length > 0) { + matchingRepos.sort((a, b) => b.rootUri.fsPath.length - a.rootUri.fsPath.length) + return matchingRepos[0] } } @@ -130,7 +180,20 @@ export async function getRepositoryDiff(repository: GitRepository): Promise 0 ? trimmed : undefined + if (trimmed.length > 0) { + return trimmed + } + + // `git diff` omits untracked files; surface them as a plain listing so the + // LLM can still generate an "initial commit" or "add files" message. + const UNTRACKED_STATUS = 7 // vscode.git Status.UNTRACKED + const untracked = repository.state?.workingTreeChanges?.filter((c) => c.status === UNTRACKED_STATUS) + if (untracked && untracked.length > 0) { + const fileList = untracked.map((c) => c.uri.fsPath).join("\n") + return `Untracked files:\n${fileList}` + } + + return undefined } /** @@ -153,7 +216,8 @@ export function cleanCommitMessage(message: string): string { * Builds the full prompt to generate a commit message from a git diff. */ export function buildCommitMessagePrompt(diff: string): string { - const truncatedDiff = truncateOutput(diff, 1000, 50000) + const sanitizedDiff = redactSensitiveInformation(diff) + const truncatedDiff = truncateOutput(sanitizedDiff, 1000, 50000) return `${COMMIT_MESSAGE_SYSTEM_PROMPT}\n\nDiff:\n\`\`\`diff\n${truncatedDiff}\n\`\`\`\n\nCommit Message:` } @@ -200,22 +264,28 @@ export async function generateCommitMessage(options: GenerateCommitMessageOption }, async (_progress, token) => { const abortController = new AbortController() - token.onCancellationRequested(() => { + const cancellationListener = token.onCancellationRequested(() => { abortController.abort() }) - const prompt = buildCommitMessagePrompt(diff) - const rawMessage = await singleCompletionHandler(apiConfiguration as ProviderSettings, prompt, { - abortSignal: abortController.signal, - }) + try { + const prompt = buildCommitMessagePrompt(diff) + const rawMessage = await singleCompletionHandler(apiConfiguration as ProviderSettings, prompt, { + abortSignal: abortController.signal, + }) - if (token.isCancellationRequested || abortController.signal.aborted) { - return - } + if (token.isCancellationRequested || abortController.signal.aborted) { + return + } - const commitMessage = cleanCommitMessage(rawMessage) - if (commitMessage) { - repository.inputBox.value = commitMessage + const commitMessage = cleanCommitMessage(rawMessage) + if (commitMessage) { + repository.inputBox.value = commitMessage + } else { + vscode.window.showWarningMessage("No commit message generated.") + } + } finally { + cancellationListener?.dispose() } }, ) From fef189b35f72ba6eda6aaea2b5e8a4741a44e497 Mon Sep 17 00:00:00 2001 From: 1220moritz Date: Tue, 3 Nov 2026 00:25:50 +0100 Subject: [PATCH 04/10] fix: address CodeRabbit review comments on commit message generator - Forward abortSignal through completePrompt in openai.ts to the underlying HTTP client so cancellation propagates to in-flight OpenAI API requests - Move withProgress earlier in generateCommitMessage so the progress indicator and cancellation token are active during getRepositoryDiff and provider.getState(), not just during the LLM call - Add default withProgress mock in spec beforeEach so tests that check early-return paths (no diff, no provider) work without per-test setup - Fix pre-existing no-useless-escape lint error in SENSITIVE_PATTERNS regex (unnecessary backslash before dot inside character class) --- src/api/providers/openai.ts | 10 +++--- .../__tests__/commitMessageGenerator.spec.ts | 11 ++++++ src/services/git/commitMessageGenerator.ts | 34 +++++++++---------- 3 files changed, 33 insertions(+), 22 deletions(-) diff --git a/src/api/providers/openai.ts b/src/api/providers/openai.ts index 98ea18df8a..221bfc250f 100644 --- a/src/api/providers/openai.ts +++ b/src/api/providers/openai.ts @@ -354,7 +354,7 @@ export class OpenAiHandler extends BaseProvider implements SingleCompletionHandl } } - async completePrompt(prompt: string, _options?: CompletePromptOptions): Promise { + async completePrompt(prompt: string, options?: CompletePromptOptions): Promise { try { const isAzureAiInference = this._isAzureAiInference(this.options.openAiBaseUrl) const model = this.getModel() @@ -372,10 +372,10 @@ export class OpenAiHandler extends BaseProvider implements SingleCompletionHandl let response try { - response = await this.client.chat.completions.create( - requestOptions, - isAzureAiInference ? { path: OPENAI_AZURE_AI_INFERENCE_PATH } : {}, - ) + response = await this.client.chat.completions.create(requestOptions, { + ...(isAzureAiInference ? { path: OPENAI_AZURE_AI_INFERENCE_PATH } : {}), + ...(options?.abortSignal ? { signal: options.abortSignal } : {}), + }) } catch (error) { throw handleOpenAIError(error, this.providerName) } diff --git a/src/services/git/__tests__/commitMessageGenerator.spec.ts b/src/services/git/__tests__/commitMessageGenerator.spec.ts index aa8a9fe85a..309c64ee68 100644 --- a/src/services/git/__tests__/commitMessageGenerator.spec.ts +++ b/src/services/git/__tests__/commitMessageGenerator.spec.ts @@ -493,6 +493,17 @@ describe("commitMessageGenerator", () => { getAPI: () => mockGitApi, }, }) + + // Default withProgress mock: runs the callback immediately with a non-cancelled token. + // Individual tests may override this for cancellation or error scenarios. + ;(vscode.window.withProgress as Mock).mockImplementation((_opts, callback) => { + const progress = { report: vi.fn() } + const token = { + isCancellationRequested: false, + onCancellationRequested: vi.fn().mockReturnValue({ dispose: vi.fn() }), + } + return callback(progress, token) + }) }) it("shows error if git extension is unavailable", async () => { diff --git a/src/services/git/commitMessageGenerator.ts b/src/services/git/commitMessageGenerator.ts index 659c6f3c02..d0fe0c51b4 100644 --- a/src/services/git/commitMessageGenerator.ts +++ b/src/services/git/commitMessageGenerator.ts @@ -65,7 +65,7 @@ Guidelines: const SENSITIVE_PATTERNS = [ /-----BEGIN [A-Z ]*PRIVATE KEY-----[\s\S]*?-----END [A-Z ]*PRIVATE KEY-----/g, - /(?:api[_-]?key|auth[_-]?token|bearer|secret|password)[\s]*[=:]\s*['"]?([a-zA-Z0-9_\-\.]{8,})['"]?/gi, + /(?:api[_-]?key|auth[_-]?token|bearer|secret|password)[\s]*[=:]\s*['"]?([a-zA-Z0-9_\-.]{8,})['"]?/gi, ] /** @@ -240,22 +240,6 @@ export async function generateCommitMessage(options: GenerateCommitMessageOption return } - const diff = await getRepositoryDiff(repository) - if (!diff) { - vscode.window.showInformationMessage( - "No staged or working directory changes found to generate commit message.", - ) - return - } - - const { apiConfiguration } = await provider.getState() - if (!apiConfiguration || !apiConfiguration.apiProvider) { - vscode.window.showErrorMessage( - "No valid API provider configured. Please configure an API provider in Zoo Code settings.", - ) - return - } - await vscode.window.withProgress( { location: vscode.ProgressLocation.Notification, @@ -269,6 +253,22 @@ export async function generateCommitMessage(options: GenerateCommitMessageOption }) try { + const diff = await getRepositoryDiff(repository) + if (!diff) { + vscode.window.showInformationMessage( + "No staged or working directory changes found to generate commit message.", + ) + return + } + + const { apiConfiguration } = await provider.getState() + if (!apiConfiguration || !apiConfiguration.apiProvider) { + vscode.window.showErrorMessage( + "No valid API provider configured. Please configure an API provider in Zoo Code settings.", + ) + return + } + const prompt = buildCommitMessagePrompt(diff) const rawMessage = await singleCompletionHandler(apiConfiguration as ProviderSettings, prompt, { abortSignal: abortController.signal, From b1d43b354b9ec7dbc7820d54325e4b363b3d95ba Mon Sep 17 00:00:00 2001 From: 1220moritz Date: Tue, 3 Nov 2026 00:36:34 +0100 Subject: [PATCH 05/10] fix(scm): use explicit separator literals for cross-platform path matching Replace path.sep with both '/' and '\' literals when checking whether an active editor URI lives under a repository root. path.sep is '\' on Windows but VS Code fsPath values on Windows are still POSIX-style '/' strings, so startsWith(root + path.sep) never matched and tests that depend on active-editor resolution were selecting the wrong repository. Also removes the now-unused 'path' import. --- src/services/git/commitMessageGenerator.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/services/git/commitMessageGenerator.ts b/src/services/git/commitMessageGenerator.ts index d0fe0c51b4..17602b526a 100644 --- a/src/services/git/commitMessageGenerator.ts +++ b/src/services/git/commitMessageGenerator.ts @@ -1,4 +1,3 @@ -import path from "path" import * as vscode from "vscode" import type { ProviderSettings } from "@roo-code/types" @@ -149,7 +148,11 @@ export function resolveGitRepository(git: GitAPI, context?: unknown): GitReposit if (activeDocUri) { const matchingRepos = git.repositories.filter((r) => { const root = r.rootUri.fsPath - return activeDocUri.fsPath === root || activeDocUri.fsPath.startsWith(root + path.sep) + return ( + activeDocUri.fsPath === root || + activeDocUri.fsPath.startsWith(root + "/") || + activeDocUri.fsPath.startsWith(root + "\\") + ) }) if (matchingRepos.length > 0) { matchingRepos.sort((a, b) => b.rootUri.fsPath.length - a.rootUri.fsPath.length) From 2c957dbe27bfdd772a064ccf410107619d07be80 Mon Sep 17 00:00:00 2001 From: 1220moritz Date: Tue, 3 Nov 2026 12:47:33 +0100 Subject: [PATCH 06/10] fix(scm): address CodeRabbit review round 2 - Add Bearer token redaction pattern to SENSITIVE_PATTERNS - Fix SCM context fallthrough: return undefined instead of falling back to active editor/first repo when explicit context doesn't match - Add token.isCancellationRequested guards before provider.getState() and before singleCompletionHandler call - Add multi-untracked files test mixed with tracked changes - Add cancellation-before-LLM test (cancelled after getState resolves) - Rename three 'falls through' tests to 'returns undefined' + toBeUndefined() - Fix zoo/no-raw-provider-identifiers: use providerIdentifiers.openai --- .../__tests__/commitMessageGenerator.spec.ts | 69 +++++++++++++++++-- src/services/git/commitMessageGenerator.ts | 29 ++++---- 2 files changed, 79 insertions(+), 19 deletions(-) diff --git a/src/services/git/__tests__/commitMessageGenerator.spec.ts b/src/services/git/__tests__/commitMessageGenerator.spec.ts index 309c64ee68..1996c8213a 100644 --- a/src/services/git/__tests__/commitMessageGenerator.spec.ts +++ b/src/services/git/__tests__/commitMessageGenerator.spec.ts @@ -78,6 +78,15 @@ describe("commitMessageGenerator", () => { expect(redacted).toContain("other_config=123") }) + it("redacts Bearer tokens in Authorization headers", () => { + const content = + "+Authorization: Bearer eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9.payload.signature\n+other: value" + const redacted = redactSensitiveInformation(content) + expect(redacted).not.toContain("eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9.payload.signature") + expect(redacted).toContain("[REDACTED]") + expect(redacted).toContain("+other: value") + }) + it("leaves normal diff content unchanged", () => { const content = "diff --git a/src/app.ts b/src/app.ts\n+console.log('hello world')" expect(redactSensitiveInformation(content)).toBe(content) @@ -224,7 +233,7 @@ describe("commitMessageGenerator", () => { expect(resolveGitRepository(git)).toBe(repo2) }) - it("falls through if bare vscode.Uri context does not match any repository", () => { + it("returns undefined if bare vscode.Uri context does not match any repository", () => { const repo1: GitRepository = { rootUri: makeUri("/workspace/repo1"), inputBox: { value: "" }, @@ -237,10 +246,10 @@ describe("commitMessageGenerator", () => { } const git: GitAPI = { repositories: [repo1, repo2] } - expect(resolveGitRepository(git, makeUri("/workspace/unknown"))).toBe(repo1) + expect(resolveGitRepository(git, makeUri("/workspace/unknown"))).toBeUndefined() }) - it("falls through if scm context rootUri does not match any repository", () => { + it("returns undefined if scm context rootUri does not match any repository", () => { const repo1: GitRepository = { rootUri: makeUri("/workspace/repo1"), inputBox: { value: "" }, @@ -253,10 +262,10 @@ describe("commitMessageGenerator", () => { } const git: GitAPI = { repositories: [repo1, repo2] } - expect(resolveGitRepository(git, { rootUri: makeUri("/workspace/unknown") })).toBe(repo1) + expect(resolveGitRepository(git, { rootUri: makeUri("/workspace/unknown") })).toBeUndefined() }) - it("falls through if scm context inputBox does not match any repository", () => { + it("returns undefined if scm context inputBox does not match any repository", () => { const repo1: GitRepository = { rootUri: makeUri("/workspace/repo1"), inputBox: { value: "" }, @@ -269,7 +278,7 @@ describe("commitMessageGenerator", () => { } const git: GitAPI = { repositories: [repo1, repo2] } - expect(resolveGitRepository(git, { inputBox: { value: "other" } })).toBe(repo1) + expect(resolveGitRepository(git, { inputBox: { value: "other" } })).toBeUndefined() }) it("falls back to first repository when active editor does not match any repository", () => { @@ -434,6 +443,27 @@ describe("commitMessageGenerator", () => { expect(diff).not.toContain("modified.ts") }) + it("surfaces all untracked files when mixed with tracked changes", async () => { + const repo: GitRepository = { + rootUri: makeUri("/workspace/repo"), + inputBox: { value: "" }, + diff: vi.fn().mockResolvedValue(""), + state: { + workingTreeChanges: [ + { uri: makeUri("/workspace/repo/new-a.ts"), status: 7 }, // UNTRACKED + { uri: makeUri("/workspace/repo/new-b.ts"), status: 7 }, // UNTRACKED + { uri: makeUri("/workspace/repo/modified.ts"), status: 5 }, // MODIFIED (not untracked) + ], + }, + } + + const diff = await getRepositoryDiff(repo) + expect(diff).toContain("Untracked files:") + expect(diff).toContain("new-a.ts") + expect(diff).toContain("new-b.ts") + expect(diff).not.toContain("modified.ts") + }) + it("returns undefined when both diffs are empty and no untracked files exist", async () => { const repo: GitRepository = { rootUri: makeUri("/workspace/repo"), @@ -609,6 +639,33 @@ describe("commitMessageGenerator", () => { expect(mockRepo.inputBox.value).toBe("") }) + it("skips LLM call when cancelled between diff fetch and provider state read", async () => { + let isCancelled = false + ;(vscode.window.withProgress as Mock).mockImplementation((_opts, callback) => { + const progress = { report: vi.fn() } + const token = { + get isCancellationRequested() { + return isCancelled + }, + onCancellationRequested: vi.fn().mockReturnValue({ dispose: vi.fn() }), + } + return callback(progress, token) + }) + // Set cancelled = true inside getState so the guard after getState fires + mockProvider.getState = vi.fn().mockImplementation(async () => { + isCancelled = true + return { apiConfiguration: { apiProvider: providerIdentifiers.openai, apiKey: "sk-test" } } + }) + + await generateCommitMessage({ + provider: mockProvider, + outputChannel: mockOutputChannel, + }) + + expect(singleCompletionHandler).not.toHaveBeenCalled() + expect(mockRepo.inputBox.value).toBe("") + }) + it("aborts when token triggers cancellation callback", async () => { let cancellationHandler: (() => void) | undefined ;(vscode.window.withProgress as Mock).mockImplementation((_opts, callback) => { diff --git a/src/services/git/commitMessageGenerator.ts b/src/services/git/commitMessageGenerator.ts index 17602b526a..a1b6edc12d 100644 --- a/src/services/git/commitMessageGenerator.ts +++ b/src/services/git/commitMessageGenerator.ts @@ -65,6 +65,7 @@ Guidelines: const SENSITIVE_PATTERNS = [ /-----BEGIN [A-Z ]*PRIVATE KEY-----[\s\S]*?-----END [A-Z ]*PRIVATE KEY-----/g, /(?:api[_-]?key|auth[_-]?token|bearer|secret|password)[\s]*[=:]\s*['"]?([a-zA-Z0-9_\-.]{8,})['"]?/gi, + /\bBearer\s+([a-zA-Z0-9_\-.~+/]{8,}=*)/gi, ] /** @@ -106,15 +107,13 @@ export function resolveGitRepository(git: GitAPI, context?: unknown): GitReposit } if (context && typeof context === "object") { - // Bare vscode.Uri or URI-like object passed from scm/inputBox menu + // Bare vscode.Uri or URI-like object passed from scm/inputBox menu. + // Return undefined (not first repo) when it doesn't match any repository. if ("scheme" in context && "fsPath" in context) { const uri = context as vscode.Uri - const found = git.repositories.find( + return git.repositories.find( (r) => r.rootUri.toString() === uri.toString() || r.rootUri.fsPath === uri.fsPath, ) - if (found) { - return found - } } const candidate = context as Partial @@ -123,20 +122,16 @@ export function resolveGitRepository(git: GitAPI, context?: unknown): GitReposit } const sc = context as { rootUri?: vscode.Uri; inputBox?: unknown } + // When an explicit SCM rootUri or inputBox is provided but matches no repository, + // return undefined rather than silently falling through to a different repository. if (sc.rootUri) { - const found = git.repositories.find( + return git.repositories.find( (r) => r.rootUri.toString() === sc.rootUri?.toString() || r.rootUri.fsPath === sc.rootUri?.fsPath, ) - if (found) { - return found - } } if (sc.inputBox) { - const found = git.repositories.find((r) => r.inputBox === sc.inputBox) - if (found) { - return found - } + return git.repositories.find((r) => r.inputBox === sc.inputBox) } } @@ -264,6 +259,10 @@ export async function generateCommitMessage(options: GenerateCommitMessageOption return } + if (token.isCancellationRequested) { + return + } + const { apiConfiguration } = await provider.getState() if (!apiConfiguration || !apiConfiguration.apiProvider) { vscode.window.showErrorMessage( @@ -272,6 +271,10 @@ export async function generateCommitMessage(options: GenerateCommitMessageOption return } + if (token.isCancellationRequested) { + return + } + const prompt = buildCommitMessagePrompt(diff) const rawMessage = await singleCompletionHandler(apiConfiguration as ProviderSettings, prompt, { abortSignal: abortController.signal, From 0b58879174ec54dc8689fdfdfdccf10c2ae84500 Mon Sep 17 00:00:00 2001 From: 1220moritz Date: Tue, 3 Nov 2026 13:33:53 +0100 Subject: [PATCH 07/10] style(scm): run prettier on commitMessageGenerator.ts --- src/services/git/commitMessageGenerator.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/services/git/commitMessageGenerator.ts b/src/services/git/commitMessageGenerator.ts index a1b6edc12d..b09e28cc75 100644 --- a/src/services/git/commitMessageGenerator.ts +++ b/src/services/git/commitMessageGenerator.ts @@ -262,7 +262,7 @@ export async function generateCommitMessage(options: GenerateCommitMessageOption if (token.isCancellationRequested) { return } - + const { apiConfiguration } = await provider.getState() if (!apiConfiguration || !apiConfiguration.apiProvider) { vscode.window.showErrorMessage( @@ -274,7 +274,7 @@ export async function generateCommitMessage(options: GenerateCommitMessageOption if (token.isCancellationRequested) { return } - + const prompt = buildCommitMessagePrompt(diff) const rawMessage = await singleCompletionHandler(apiConfiguration as ProviderSettings, prompt, { abortSignal: abortController.signal, From 9c6b0aef90498e847973a8b94308077b8202edab Mon Sep 17 00:00:00 2001 From: 1220moritz Date: Tue, 10 Nov 2026 20:06:41 +0100 Subject: [PATCH 08/10] fix(scm): address CodeRabbit review round 3 for commit message generation - Move the action from the proposed scm/inputBox menu to the stable scm/title menu. VS Code ignores scm/inputBox contributions unless the extension enables the contribSourceControlInputBoxMenu proposal, and vsce publish rejects that proposal. - Accept a command context only when it matches an entry in git.repositories. Never return the context object or call its methods. - Let staged and working-tree diff failures reach the error handler instead of reporting "no changes". - Expand redaction: JSON/YAML keys, prefixed env keys, short values, Authorization headers, bare token formats, URL credentials and email addresses. Keep the key name and replace only the value. - List untracked files relative to the repository root. - Race the diff, provider state and completion against cancellation so the progress notification closes at once. - Skip the write when a newer request for the same repository exists or when the user edited the commit box during generation. - Warn when the diff was shortened before generation. - Remove the OpenAI completePrompt abort forwarding. Open PRs #1311 and #1545 own abort support for the OpenAI-family providers. Refs #2301 Co-Authored-By: Claude Opus 5.5 --- src/api/providers/openai.ts | 10 +- src/package.json | 6 +- .../__tests__/commitMessageGenerator.spec.ts | 464 +++++++++++++++++- src/services/git/commitMessageGenerator.ts | 271 +++++++--- 4 files changed, 648 insertions(+), 103 deletions(-) diff --git a/src/api/providers/openai.ts b/src/api/providers/openai.ts index 221bfc250f..98ea18df8a 100644 --- a/src/api/providers/openai.ts +++ b/src/api/providers/openai.ts @@ -354,7 +354,7 @@ export class OpenAiHandler extends BaseProvider implements SingleCompletionHandl } } - async completePrompt(prompt: string, options?: CompletePromptOptions): Promise { + async completePrompt(prompt: string, _options?: CompletePromptOptions): Promise { try { const isAzureAiInference = this._isAzureAiInference(this.options.openAiBaseUrl) const model = this.getModel() @@ -372,10 +372,10 @@ export class OpenAiHandler extends BaseProvider implements SingleCompletionHandl let response try { - response = await this.client.chat.completions.create(requestOptions, { - ...(isAzureAiInference ? { path: OPENAI_AZURE_AI_INFERENCE_PATH } : {}), - ...(options?.abortSignal ? { signal: options.abortSignal } : {}), - }) + response = await this.client.chat.completions.create( + requestOptions, + isAzureAiInference ? { path: OPENAI_AZURE_AI_INFERENCE_PATH } : {}, + ) } catch (error) { throw handleOpenAIError(error, this.providerName) } diff --git a/src/package.json b/src/package.json index 321859b1ce..3b3b4406eb 100644 --- a/src/package.json +++ b/src/package.json @@ -187,11 +187,11 @@ } ], "menus": { - "scm/inputBox": [ + "scm/title": [ { "command": "zoo-code.generateGitCommitMessage", - "group": "inline", - "when": "scmRepository && scmProvider == git" + "group": "navigation", + "when": "scmProvider == git" } ], "editor/context": [ diff --git a/src/services/git/__tests__/commitMessageGenerator.spec.ts b/src/services/git/__tests__/commitMessageGenerator.spec.ts index 1996c8213a..a65940fef1 100644 --- a/src/services/git/__tests__/commitMessageGenerator.spec.ts +++ b/src/services/git/__tests__/commitMessageGenerator.spec.ts @@ -5,9 +5,15 @@ import { providerIdentifiers } from "@roo-code/types" import { buildCommitMessagePrompt, cleanCommitMessage, + COMMIT_MESSAGE_SYSTEM_PROMPT, + DIFF_CHARACTER_LIMIT, + DIFF_LINE_LIMIT, + DIFF_TRUNCATED_MESSAGE, + DRAFT_CHANGED_MESSAGE, generateCommitMessage, getGitApi, getRepositoryDiff, + NO_CHANGES_MESSAGE, redactSensitiveInformation, resolveGitRepository, type GitAPI, @@ -41,6 +47,52 @@ vi.mock("../../../utils/single-completion-handler", () => ({ singleCompletionHandler: vi.fn(), })) +function makeRepo(root: string, overrides: Partial = {}): GitRepository { + return { + rootUri: makeUri(root), + inputBox: { value: "" }, + diff: vi.fn().mockResolvedValue("diff content"), + ...overrides, + } +} + +function deferred() { + let resolve: (value: T) => void = () => {} + const promise = new Promise((r) => { + resolve = r + }) + return { promise, resolve } +} + +/** + * Installs a withProgress mock whose token can be cancelled from the test. Every registered + * cancellation listener runs on cancel(), like a real vscode.CancellationTokenSource. + */ +function useCancellableProgress() { + let cancelled = false + const listeners: Array<() => void> = [] + const disposable = { dispose: vi.fn() } + ;(vscode.window.withProgress as Mock).mockImplementation((_opts, callback) => { + const token = { + get isCancellationRequested() { + return cancelled + }, + onCancellationRequested: vi.fn((fn: () => void) => { + listeners.push(fn) + return disposable + }), + } + return callback({ report: vi.fn() }, token) + }) + return { + disposable, + cancel: () => { + cancelled = true + listeners.forEach((fn) => fn()) + }, + } +} + describe("commitMessageGenerator", () => { beforeEach(() => { vi.clearAllMocks() @@ -87,6 +139,77 @@ describe("commitMessageGenerator", () => { expect(redacted).toContain("+other: value") }) + it("redacts secret_key values", () => { + const content = "secret_key=supersecret12345\nother=unchanged" + const redacted = redactSensitiveInformation(content) + expect(redacted).not.toContain("supersecret12345") + expect(redacted).toContain("[REDACTED]") + expect(redacted).toContain("other=unchanged") + }) + + it("redacts passwords containing special characters like @", () => { + const content = "password=pass@word123\nother=unchanged" + const redacted = redactSensitiveInformation(content) + expect(redacted).not.toContain("pass@word123") + expect(redacted).toContain("[REDACTED]") + expect(redacted).toContain("other=unchanged") + }) + + it.each([ + ["JSON key", '+ "apiKey": "sk-live-123456789",', '+ "apiKey": "[REDACTED]",', "sk-live-123456789"], + ["YAML key", "+ client_secret: hunter2", "+ client_secret: [REDACTED]", "hunter2"], + ["env var with prefix", "+OPENAI_API_KEY=abc", "+OPENAI_API_KEY=[REDACTED]", "abc"], + ["short password", "+DB_PASSWORD=a", "+DB_PASSWORD=[REDACTED]", "=a"], + ["camelCase key", '+ openAiApiKey: "abc123",', '+ openAiApiKey: "[REDACTED]",', "abc123"], + ["bare bearer token", "+curl -H x Bearer abc123def456", "+curl -H x Bearer [REDACTED]", "abc123def456"], + ["access token", "+access_token: 'tok_1'", "+access_token: '[REDACTED]'", "tok_1"], + [ + "Basic authorization header", + "+Authorization: Basic dXNlcjpwYXNzd29yZA==", + "+Authorization: Basic [REDACTED]", + "dXNlcjpwYXNzd29yZA", + ], + [ + "JSON authorization header", + '+"Authorization": "Bearer abc.def.ghi"', + '+"Authorization": "Bearer [REDACTED]"', + "abc.def.ghi", + ], + ["raw authorization header", "+authorization: rawtoken", "+authorization: [REDACTED]", "rawtoken"], + [ + "URL credentials", + "+url = postgres://admin:s3cr3t@db.internal:5432/app", + "+url = postgres://admin:[REDACTED]@db.internal:5432/app", + "s3cr3t", + ], + ["email address", '+ "author": "alice@example.com",', '+ "author": "[REDACTED_EMAIL]",', "alice@"], + ])("redacts %s", (_label, content, expected, secret) => { + const redacted = redactSensitiveInformation(content) + expect(redacted).toBe(expected) + expect(redacted).not.toContain(secret) + }) + + it.each([ + ["OpenAI key", "sk-proj-abcdefghijklmnopqrstuvwx"], + ["GitHub token", "ghp_abcdefghijklmnopqrstuvwxyz0123456789"], + ["AWS access key id", "AKIAABCDEFGHIJKLMNOP"], + ["Slack token", "xoxb-1234567890-abcdef"], + ["JWT", "eyJhbGciOiJIUzI1NiJ9.eyJzdWIiOiIxMjM0NTY3ODkwIn0.dozjgNryP4J3jVmNHl0w5N"], + ])("redacts a bare %s", (_label, token) => { + const redacted = redactSensitiveInformation(`+const value = callWith(${token})`) + expect(redacted).toBe("+const value = callWith([REDACTED_SECRET])") + }) + + it.each([ + ["comparisons", "+if (password === confirmation) {"], + ["arrow functions", "+const check = (password) => verify(password)"], + ["prose mentioning auth schemes", "+// Adds basic functionality for Bearer authentication"], + ["scoped package names", '+import { x } from "@roo-code/types"'], + ["versioned package specs", "+npx react@18.3.1"], + ])("leaves %s unchanged", (_label, content) => { + expect(redactSensitiveInformation(content)).toBe(content) + }) + it("leaves normal diff content unchanged", () => { const content = "diff --git a/src/app.ts b/src/app.ts\n+console.log('hello world')" expect(redactSensitiveInformation(content)).toBe(content) @@ -95,16 +218,32 @@ describe("commitMessageGenerator", () => { describe("buildCommitMessagePrompt", () => { it("includes diff and instructions", () => { - const prompt = buildCommitMessagePrompt("diff --git a/file b/file\n+test") + const { prompt, truncated } = buildCommitMessagePrompt("diff --git a/file b/file\n+test") expect(prompt).toContain("diff --git a/file b/file\n+test") expect(prompt).toContain("Conventional Commits") + expect(truncated).toBe(false) }) it("redacts sensitive information in the prompt", () => { - const prompt = buildCommitMessagePrompt("apiKey: 'secret_token_12345678'") + const { prompt } = buildCommitMessagePrompt("apiKey: 'secret_token_12345678'") expect(prompt).not.toContain("secret_token_12345678") expect(prompt).toContain("[REDACTED]") }) + + it.each([ + ["character", "x".repeat(DIFF_CHARACTER_LIMIT + 1)], + ["line", Array.from({ length: DIFF_LINE_LIMIT + 1 }, (_, i) => `+line ${i}`).join("\n")], + ])("reports truncation when the diff exceeds the %s limit", (_label, diff) => { + const { prompt, truncated } = buildCommitMessagePrompt(diff) + expect(truncated).toBe(true) + expect(prompt).toContain(COMMIT_MESSAGE_SYSTEM_PROMPT) + expect(prompt).toMatch(/\[\.\.\.\d+ (?:characters|lines) omitted\.\.\.\]/) + }) + + it("does not report truncation at exactly the character limit", () => { + const { truncated } = buildCommitMessagePrompt("x".repeat(DIFF_CHARACTER_LIMIT)) + expect(truncated).toBe(false) + }) }) describe("getGitApi", () => { @@ -149,6 +288,43 @@ describe("commitMessageGenerator", () => { expect(resolveGitRepository(git, repo)).toBe(repo) }) + it("resolves a SourceControl context from the scm/title menu by rootUri", () => { + const repo1 = makeRepo("/workspace/repo1") + const repo2 = makeRepo("/workspace/repo2") + const git: GitAPI = { repositories: [repo1, repo2] } + const sourceControl = { id: "git", rootUri: makeUri("/workspace/repo2"), inputBox: { value: "" } } + + expect(resolveGitRepository(git, sourceControl)).toBe(repo2) + }) + + it("rejects a repository-shaped context that is not a known repository", () => { + const known = makeRepo("/workspace/repo1") + const other = makeRepo("/workspace/repo2") + const git: GitAPI = { repositories: [known, other] } + const fake = makeRepo("/workspace/attacker") + + expect(resolveGitRepository(git, fake)).toBeUndefined() + expect(fake.diff).not.toHaveBeenCalled() + }) + + it("returns the known repository, not the context object, when a fake context reuses its rootUri", () => { + const known = makeRepo("/workspace/repo") + const git: GitAPI = { repositories: [known, makeRepo("/workspace/other")] } + const fake = makeRepo("/workspace/repo") + + const resolved = resolveGitRepository(git, fake) + expect(resolved).toBe(known) + expect(resolved).not.toBe(fake) + }) + + it("rejects a fake context with a matching-looking inputBox", () => { + const git: GitAPI = { repositories: [makeRepo("/workspace/repo1"), makeRepo("/workspace/repo2")] } + const fake = { inputBox: { value: "" }, diff: vi.fn() } + + expect(resolveGitRepository(git, fake)).toBeUndefined() + expect(fake.diff).not.toHaveBeenCalled() + }) + it("resolves from bare vscode.Uri context", () => { const repo1: GitRepository = { rootUri: makeUri("/workspace/repo1"), @@ -383,20 +559,28 @@ describe("commitMessageGenerator", () => { expect(repo.diff).toHaveBeenCalledWith(false) }) - it("falls back to working tree diff if staged diff throws", async () => { - const repo: GitRepository = { - rootUri: makeUri("/workspace/repo"), - inputBox: { value: "" }, + it("rejects without reading the working tree when the staged diff fails", async () => { + const repo = makeRepo("/workspace/repo", { diff: vi.fn().mockImplementation((cached?: boolean) => { if (cached) return Promise.reject(new Error("Git diff staged failed")) return Promise.resolve("working diff fallback") }), - } + }) - const diff = await getRepositoryDiff(repo) - expect(diff).toBe("working diff fallback") - expect(repo.diff).toHaveBeenCalledWith(true) - expect(repo.diff).toHaveBeenCalledWith(false) + await expect(getRepositoryDiff(repo)).rejects.toThrow("Git diff staged failed") + expect(repo.diff).not.toHaveBeenCalledWith(false) + }) + + it("rejects with the staged error when both staged and working tree diffs fail", async () => { + const repo = makeRepo("/workspace/repo", { + diff: vi + .fn() + .mockImplementation((cached?: boolean) => + Promise.reject(new Error(cached ? "staged failed" : "working failed")), + ), + }) + + await expect(getRepositoryDiff(repo)).rejects.toThrow("staged failed") }) it("returns undefined if both staged and working diffs are empty", async () => { @@ -410,18 +594,28 @@ describe("commitMessageGenerator", () => { expect(diff).toBeUndefined() }) - it("returns undefined if working diff throws after empty staged diff", async () => { - const repo: GitRepository = { - rootUri: makeUri("/workspace/repo"), - inputBox: { value: "" }, + it("rejects if working diff fails after empty staged diff", async () => { + const repo = makeRepo("/workspace/repo", { diff: vi.fn().mockImplementation((cached?: boolean) => { if (cached) return Promise.resolve("") return Promise.reject(new Error("Git working tree diff failed")) }), - } + }) + + await expect(getRepositoryDiff(repo)).rejects.toThrow("Git working tree diff failed") + }) + + it("lists untracked files relative to the repository root", async () => { + const repo = makeRepo("/home/alice/workspace/repo", { + diff: vi.fn().mockResolvedValue(""), + state: { + workingTreeChanges: [{ uri: makeUri("/home/alice/workspace/repo/src/new-file.ts"), status: 7 }], + }, + }) const diff = await getRepositoryDiff(repo) - expect(diff).toBeUndefined() + // path.relative uses the platform separator. + expect(diff).toMatch(/^Untracked files:\nsrc[\\/]new-file\.ts$/) }) it("surfaces untracked files when both diffs are empty", async () => { @@ -571,9 +765,195 @@ describe("commitMessageGenerator", () => { outputChannel: mockOutputChannel, }) - expect(vscode.window.showInformationMessage).toHaveBeenCalledWith( - "No staged or working directory changes found to generate commit message.", + expect(vscode.window.showInformationMessage).toHaveBeenCalledWith(NO_CHANGES_MESSAGE) + }) + + it("reports a Git diff failure as an error instead of 'no changes'", async () => { + mockRepo.diff = vi.fn().mockRejectedValue(new Error("fatal: bad index file")) + + await generateCommitMessage({ + provider: mockProvider, + outputChannel: mockOutputChannel, + }) + + expect(vscode.window.showInformationMessage).not.toHaveBeenCalledWith(NO_CHANGES_MESSAGE) + expect(vscode.window.showErrorMessage).toHaveBeenCalledWith( + "Failed to generate commit message: fatal: bad index file", + ) + expect(singleCompletionHandler).not.toHaveBeenCalled() + }) + + it("never reads from or writes to a command context that is not a known repository", async () => { + const fake = makeRepo("/workspace/attacker") + + await generateCommitMessage({ + provider: mockProvider, + outputChannel: mockOutputChannel, + context: fake, + }) + + expect(fake.diff).not.toHaveBeenCalled() + expect(singleCompletionHandler).not.toHaveBeenCalled() + expect(fake.inputBox.value).toBe("") + expect(vscode.window.showErrorMessage).toHaveBeenCalledWith("No Git repository found.") + }) + + it("generates for the repository named by an scm/title SourceControl context", async () => { + ;(singleCompletionHandler as Mock).mockResolvedValue("fix: second repo") + const second = makeRepo("/workspace/second") + ;(vscode.extensions.getExtension as Mock).mockReturnValue({ + isActive: true, + exports: { getAPI: () => ({ repositories: [mockRepo, second] }) }, + }) + + await generateCommitMessage({ + provider: mockProvider, + outputChannel: mockOutputChannel, + context: { rootUri: makeUri("/workspace/second"), inputBox: { value: "" } }, + }) + + expect(second.inputBox.value).toBe("fix: second repo") + expect(mockRepo.inputBox.value).toBe("") + }) + + it("warns after writing when the diff was truncated", async () => { + mockRepo.diff = vi.fn().mockResolvedValue("x".repeat(DIFF_CHARACTER_LIMIT + 1)) + ;(singleCompletionHandler as Mock).mockResolvedValue("feat: large change") + + await generateCommitMessage({ + provider: mockProvider, + outputChannel: mockOutputChannel, + }) + + expect(mockRepo.inputBox.value).toBe("feat: large change") + expect(vscode.window.showWarningMessage).toHaveBeenCalledWith(DIFF_TRUNCATED_MESSAGE) + }) + + it("does not warn about truncation for a diff within the limits", async () => { + ;(singleCompletionHandler as Mock).mockResolvedValue("feat: small change") + + await generateCommitMessage({ + provider: mockProvider, + outputChannel: mockOutputChannel, + }) + + expect(mockRepo.inputBox.value).toBe("feat: small change") + expect(vscode.window.showWarningMessage).not.toHaveBeenCalled() + }) + + it("keeps text the user typed while the request was pending", async () => { + mockRepo.inputBox.value = "draft" + ;(singleCompletionHandler as Mock).mockImplementation(async () => { + mockRepo.inputBox.value = "typed by user" + return "feat: generated" + }) + + await generateCommitMessage({ + provider: mockProvider, + outputChannel: mockOutputChannel, + }) + + expect(mockRepo.inputBox.value).toBe("typed by user") + expect(vscode.window.showInformationMessage).toHaveBeenCalledWith(DRAFT_CHANGED_MESSAGE) + }) + + it("replaces an unchanged draft with the generated message", async () => { + mockRepo.inputBox.value = "draft" + ;(singleCompletionHandler as Mock).mockResolvedValue("feat: generated") + + await generateCommitMessage({ + provider: mockProvider, + outputChannel: mockOutputChannel, + }) + + expect(mockRepo.inputBox.value).toBe("feat: generated") + expect(vscode.window.showInformationMessage).not.toHaveBeenCalledWith(DRAFT_CHANGED_MESSAGE) + }) + + it("discards an older result that finishes after a newer request for the same repository", async () => { + const first = deferred() + const second = deferred() + ;(singleCompletionHandler as Mock) + .mockImplementationOnce(() => first.promise) + .mockImplementationOnce(() => second.promise) + + const older = generateCommitMessage({ provider: mockProvider, outputChannel: mockOutputChannel }) + const newer = generateCommitMessage({ provider: mockProvider, outputChannel: mockOutputChannel }) + await vi.waitFor(() => expect(singleCompletionHandler).toHaveBeenCalledTimes(2)) + + second.resolve("feat: newer") + await newer + first.resolve("feat: older") + await older + + expect(mockRepo.inputBox.value).toBe("feat: newer") + expect(vscode.window.showInformationMessage).not.toHaveBeenCalledWith(DRAFT_CHANGED_MESSAGE) + }) + + it("keeps the latest request of each repository independent", async () => { + const otherRepo = makeRepo("/workspace/other") + ;(vscode.extensions.getExtension as Mock).mockReturnValue({ + isActive: true, + exports: { getAPI: () => ({ repositories: [mockRepo, otherRepo] }) }, + }) + const first = deferred() + const second = deferred() + ;(singleCompletionHandler as Mock) + .mockImplementationOnce(() => first.promise) + .mockImplementationOnce(() => second.promise) + + const forMockRepo = generateCommitMessage({ provider: mockProvider, context: mockRepo }) + const forOtherRepo = generateCommitMessage({ provider: mockProvider, context: otherRepo }) + await vi.waitFor(() => expect(singleCompletionHandler).toHaveBeenCalledTimes(2)) + + second.resolve("feat: other") + await forOtherRepo + first.resolve("feat: mock") + await forMockRepo + + expect(mockRepo.inputBox.value).toBe("feat: mock") + expect(otherRepo.inputBox.value).toBe("feat: other") + }) + + it("closes the progress immediately when cancelled while the LLM call is pending", async () => { + const { cancel, disposable } = useCancellableProgress() + let capturedSignal: AbortSignal | undefined + // The provider ignores the abort signal and never settles. + ;(singleCompletionHandler as Mock).mockImplementation((_config, _prompt, options) => { + capturedSignal = options.abortSignal as AbortSignal + return new Promise(() => {}) + }) + + const run = generateCommitMessage({ provider: mockProvider, outputChannel: mockOutputChannel }) + await vi.waitFor(() => expect(singleCompletionHandler).toHaveBeenCalled()) + cancel() + await run + + expect(capturedSignal?.aborted).toBe(true) + expect(disposable.dispose).toHaveBeenCalled() + expect(mockRepo.inputBox.value).toBe("") + expect(vscode.window.showErrorMessage).not.toHaveBeenCalled() + }) + + it("does not report a provider rejection that arrives after cancellation", async () => { + const { cancel } = useCancellableProgress() + let rejectCompletion: (error: Error) => void = () => {} + ;(singleCompletionHandler as Mock).mockImplementation( + () => + new Promise((_resolve, reject) => { + rejectCompletion = reject + }), ) + + const run = generateCommitMessage({ provider: mockProvider, outputChannel: mockOutputChannel }) + await vi.waitFor(() => expect(singleCompletionHandler).toHaveBeenCalled()) + cancel() + await run + rejectCompletion(new Error("late provider failure")) + await Promise.resolve() + + expect(vscode.window.showErrorMessage).not.toHaveBeenCalled() + expect(mockRepo.inputBox.value).toBe("") }) it("shows error if no valid API provider is configured", async () => { @@ -639,7 +1019,41 @@ describe("commitMessageGenerator", () => { expect(mockRepo.inputBox.value).toBe("") }) - it("skips LLM call when cancelled between diff fetch and provider state read", async () => { + it("skips provider and LLM call when cancelled while diff is pending", async () => { + let isCancelled = false + const cancellationHandlers: Array<() => void> = [] + // diff never resolves on its own — simulates a slow/stuck git process + mockRepo.diff = vi.fn().mockImplementation(() => new Promise(() => {})) + ;(vscode.window.withProgress as Mock).mockImplementation((_opts, callback) => { + const progress = { report: vi.fn() } + const token = { + get isCancellationRequested() { + return isCancelled + }, + onCancellationRequested: vi.fn((fn: () => void) => { + cancellationHandlers.push(fn) + return { dispose: vi.fn() } + }), + } + // Trigger cancellation after the race has started (next microtask) + void Promise.resolve().then(() => { + isCancelled = true + cancellationHandlers.forEach((fn) => fn()) + }) + return callback(progress, token) + }) + + await generateCommitMessage({ + provider: mockProvider, + outputChannel: mockOutputChannel, + }) + + expect(mockProvider.getState).not.toHaveBeenCalled() + expect(singleCompletionHandler).not.toHaveBeenCalled() + expect(mockRepo.inputBox.value).toBe("") + }) + + it("skips LLM call when cancelled after provider state resolves", async () => { let isCancelled = false ;(vscode.window.withProgress as Mock).mockImplementation((_opts, callback) => { const progress = { report: vi.fn() } @@ -694,11 +1108,14 @@ describe("commitMessageGenerator", () => { }) it("aborts the in-flight provider request when cancellation is requested", async () => { - let cancelHandler: () => void = () => {} + // Collect ALL registered cancellation handlers so both abortController.abort() + // and the cancelPromise resolver are called when the test triggers cancellation. + const registeredHandlers: Array<() => void> = [] + let triggerCancellation: () => void = () => {} let capturedSignal: AbortSignal | undefined ;(singleCompletionHandler as Mock).mockImplementation((_config, _prompt, options) => { capturedSignal = options.abortSignal as AbortSignal - cancelHandler() + triggerCancellation() return Promise.resolve("feat: should be cancelled") }) ;(vscode.window.withProgress as Mock).mockImplementation((_opts, callback) => { @@ -706,7 +1123,8 @@ describe("commitMessageGenerator", () => { const token = { isCancellationRequested: false, onCancellationRequested: vi.fn((fn: () => void) => { - cancelHandler = fn + registeredHandlers.push(fn) + triggerCancellation = () => registeredHandlers.forEach((h) => h()) return { dispose: vi.fn() } }), } diff --git a/src/services/git/commitMessageGenerator.ts b/src/services/git/commitMessageGenerator.ts index b09e28cc75..b82458c4d9 100644 --- a/src/services/git/commitMessageGenerator.ts +++ b/src/services/git/commitMessageGenerator.ts @@ -1,3 +1,4 @@ +import * as path from "path" import * as vscode from "vscode" import type { ProviderSettings } from "@roo-code/types" @@ -38,6 +39,11 @@ export interface GenerateCommitMessageOptions { context?: unknown } +export interface CommitMessagePrompt { + prompt: string + truncated: boolean +} + export const COMMIT_MESSAGE_SYSTEM_PROMPT = `You are an expert developer specializing in Git commit messages. Generate a concise, meaningful Git commit message for the provided diff following the Conventional Commits specification (v1.0.0). @@ -62,24 +68,88 @@ Guidelines: 6. Return ONLY the commit message. Do NOT include markdown code blocks, quotes, or any introductory or concluding text. 7. Never output secrets, passwords, or personal identifiable information in commit messages.` -const SENSITIVE_PATTERNS = [ - /-----BEGIN [A-Z ]*PRIVATE KEY-----[\s\S]*?-----END [A-Z ]*PRIVATE KEY-----/g, - /(?:api[_-]?key|auth[_-]?token|bearer|secret|password)[\s]*[=:]\s*['"]?([a-zA-Z0-9_\-.]{8,})['"]?/gi, - /\bBearer\s+([a-zA-Z0-9_\-.~+/]{8,}=*)/gi, +export const DIFF_LINE_LIMIT = 1000 +export const DIFF_CHARACTER_LIMIT = 50_000 + +export const NO_CHANGES_MESSAGE = "No staged or working directory changes found to generate commit message." +export const DRAFT_CHANGED_MESSAGE = + "The commit message box changed while Zoo Code generated a message. Your text was kept. Run the command again to replace it." +export const DIFF_TRUNCATED_MESSAGE = + "The diff was too large and Zoo Code shortened it before generation. Make sure the commit message covers all changes." + +// vscode.git Status.UNTRACKED +const UNTRACKED_STATUS = 7 + +const CREDENTIAL_KEYWORDS = [ + "api[_-]?key", + "access[_-]?key(?:[_-]?id)?", + "access[_-]?token", + "refresh[_-]?token", + "auth[_-]?token", + "client[_-]?secret", + "private[_-]?key", + "secret(?:[_-]?(?:access[_-]?)?key)?", + "passw(?:or)?d", + "pwd", + "credentials?", +].join("|") + +/** + * Each entry replaces one class of sensitive value. Patterns with a `prefix` capture group keep the + * prefix (key name, header scheme) and replace only the value, so the model still sees which setting + * changed. The order matters: header schemes run before the generic key/value pattern, and URL + * credentials run before the email pattern. + */ +const REDACTION_RULES: ReadonlyArray<{ pattern: RegExp; replacement: string }> = [ + // PEM private key blocks + { + pattern: /-----BEGIN [A-Z ]*PRIVATE KEY-----[\s\S]*?-----END [A-Z ]*PRIVATE KEY-----/g, + replacement: "[REDACTED_SECRET]", + }, + // Authorization header values, with or without a scheme ("Authorization: Basic dXNlcjpwYXNz") + { + pattern: + /(?\bauthorization["']?\s*[:=](?![=>])\s*["']?(?:(?:Bearer|Basic|Digest|Token)\s+)?)[^\s"',;]+/gi, + replacement: "$[REDACTED]", + }, + // Bearer tokens outside an Authorization header. The token must contain a digit so prose such as + // "Bearer authentication" is not redacted. + { + pattern: /(?\b[Bb]earer\s+)(?=[A-Za-z_\-.~+/]*\d)[A-Za-z0-9_\-.~+/]{8,}=*/g, + replacement: "$[REDACTED]", + }, + // key = value, key: value, "key": "value" (env files, YAML, JSON, code). + // `(?![=>])` skips comparisons (`password === x`) and arrow functions. + { + pattern: new RegExp(`(?(?:${CREDENTIAL_KEYWORDS})["']?\\s*[:=](?![=>])\\s*["']?)[^\\s"',;]+`, "gi"), + replacement: "$[REDACTED]", + }, + // Well-known token formats that can appear without a key name + { + pattern: + /\b(?:sk-(?:proj-|live-|test-|ant-)?[A-Za-z0-9_-]{16,}|gh[pousr]_[A-Za-z0-9]{20,}|github_pat_[A-Za-z0-9_]{20,}|xox[abprs]-[A-Za-z0-9-]{10,}|AKIA[0-9A-Z]{16}|AIza[0-9A-Za-z_-]{30,}|eyJ[A-Za-z0-9_-]{8,}\.[A-Za-z0-9_-]{8,}\.[A-Za-z0-9_-]{8,})/g, + replacement: "[REDACTED_SECRET]", + }, + // Credentials embedded in URLs: scheme://user:password@host + { + pattern: /(?:\/\/[^\s:@/]+:)[^\s@/]+(?=@)/g, + replacement: "$[REDACTED]", + }, + // Email addresses + { + pattern: /[A-Za-z0-9._%+-]+@[A-Za-z0-9-]+(?:\.[A-Za-z0-9-]+)*\.[A-Za-z]{2,}\b/g, + replacement: "[REDACTED_EMAIL]", + }, ] /** - * Redacts secrets, private keys, and credential tokens from content before sending to an LLM. + * Redacts secrets, private keys, credential tokens, and email addresses from content before it is + * sent to an LLM. */ export function redactSensitiveInformation(content: string): string { let redacted = content - for (const pattern of SENSITIVE_PATTERNS) { - redacted = redacted.replace(pattern, (match, captured) => { - if (captured) { - return match.replace(captured, "[REDACTED]") - } - return "[REDACTED_SECRET]" - }) + for (const { pattern, replacement } of REDACTION_RULES) { + redacted = redacted.replace(pattern, replacement) } return redacted } @@ -98,8 +168,25 @@ export async function getGitApi(): Promise { return gitExtension.exports?.getAPI(1) } +function isUriLike(value: unknown): value is vscode.Uri { + return ( + typeof value === "object" && + value !== null && + typeof (value as { scheme?: unknown }).scheme === "string" && + typeof (value as { fsPath?: unknown }).fsPath === "string" + ) +} + +function findRepositoryByRoot(git: GitAPI, uri: vscode.Uri): GitRepository | undefined { + const uriString = uri.toString() + return git.repositories.find((r) => r.rootUri.toString() === uriString || r.rootUri.fsPath === uri.fsPath) +} + /** - * Resolves the appropriate Git repository given an optional SCM invocation context. + * Resolves the Git repository for an optional SCM command context. + * + * The command is public, so the context is untrusted. The result is always an entry of + * `git.repositories`. The function never returns the context object and never calls its methods. */ export function resolveGitRepository(git: GitAPI, context?: unknown): GitRepository | undefined { if (!git.repositories || git.repositories.length === 0) { @@ -107,27 +194,21 @@ export function resolveGitRepository(git: GitAPI, context?: unknown): GitReposit } if (context && typeof context === "object") { - // Bare vscode.Uri or URI-like object passed from scm/inputBox menu. - // Return undefined (not first repo) when it doesn't match any repository. - if ("scheme" in context && "fsPath" in context) { - const uri = context as vscode.Uri - return git.repositories.find( - (r) => r.rootUri.toString() === uri.toString() || r.rootUri.fsPath === uri.fsPath, - ) + const known = git.repositories.find((r) => r === context) + if (known) { + return known } - const candidate = context as Partial - if (typeof candidate.diff === "function" && candidate.inputBox && typeof candidate.inputBox === "object") { - return candidate as GitRepository + // A bare Uri, or a SourceControl from the scm/title menu (rootUri + inputBox). + // When an explicit context matches no repository, return undefined instead of + // silently picking a different repository. + if (isUriLike(context)) { + return findRepositoryByRoot(git, context) } - const sc = context as { rootUri?: vscode.Uri; inputBox?: unknown } - // When an explicit SCM rootUri or inputBox is provided but matches no repository, - // return undefined rather than silently falling through to a different repository. - if (sc.rootUri) { - return git.repositories.find( - (r) => r.rootUri.toString() === sc.rootUri?.toString() || r.rootUri.fsPath === sc.rootUri?.fsPath, - ) + const sc = context as { rootUri?: unknown; inputBox?: unknown } + if (isUriLike(sc.rootUri)) { + return findRepositoryByRoot(git, sc.rootUri) } if (sc.inputBox) { @@ -159,35 +240,30 @@ export function resolveGitRepository(git: GitAPI, context?: unknown): GitReposit } /** - * Retrieves diff from the repository, prioritizing staged changes and falling back to working tree changes. + * Retrieves the diff from the repository. Staged changes come first. Working tree changes are used + * only when nothing is staged, and untracked files only when both diffs are empty. + * + * A failed `git diff` rejects. It is not reported as "no changes", because a failed staged read + * would otherwise make the message describe unstaged changes that the next commit does not include. */ export async function getRepositoryDiff(repository: GitRepository): Promise { - let diff = "" - try { - diff = await repository.diff(true) - } catch { - // Staged diff failed, continue to working tree diff - } - - if (!diff || !diff.trim()) { - try { - diff = await repository.diff(false) - } catch { - // Working tree diff failed - } + const staged = (await repository.diff(true))?.trim() + if (staged) { + return staged } - const trimmed = diff ? diff.trim() : "" - if (trimmed.length > 0) { - return trimmed + const workingTree = (await repository.diff(false))?.trim() + if (workingTree) { + return workingTree } // `git diff` omits untracked files; surface them as a plain listing so the // LLM can still generate an "initial commit" or "add files" message. - const UNTRACKED_STATUS = 7 // vscode.git Status.UNTRACKED + // Paths are relative to the repository root so absolute local paths are not sent. const untracked = repository.state?.workingTreeChanges?.filter((c) => c.status === UNTRACKED_STATUS) if (untracked && untracked.length > 0) { - const fileList = untracked.map((c) => c.uri.fsPath).join("\n") + const rootPath = repository.rootUri.fsPath + const fileList = untracked.map((c) => path.relative(rootPath, c.uri.fsPath) || c.uri.fsPath).join("\n") return `Untracked files:\n${fileList}` } @@ -212,18 +288,29 @@ export function cleanCommitMessage(message: string): string { /** * Builds the full prompt to generate a commit message from a git diff. + * `truncated` is true when the diff exceeded the line or character limit and was shortened. */ -export function buildCommitMessagePrompt(diff: string): string { +export function buildCommitMessagePrompt(diff: string): CommitMessagePrompt { const sanitizedDiff = redactSensitiveInformation(diff) - const truncatedDiff = truncateOutput(sanitizedDiff, 1000, 50000) - return `${COMMIT_MESSAGE_SYSTEM_PROMPT}\n\nDiff:\n\`\`\`diff\n${truncatedDiff}\n\`\`\`\n\nCommit Message:` + const truncatedDiff = truncateOutput(sanitizedDiff, DIFF_LINE_LIMIT, DIFF_CHARACTER_LIMIT) + return { + prompt: `${COMMIT_MESSAGE_SYSTEM_PROMPT}\n\nDiff:\n\`\`\`diff\n${truncatedDiff}\n\`\`\`\n\nCommit Message:`, + truncated: truncatedDiff !== sanitizedDiff, + } } +// Invocation order for the stale-write check. Numbers are assigned before the first await, so they +// follow call order. The latest number is tracked per repository so that a request for one +// repository never discards the result for another. +let invocationCounter = 0 +const latestInvocationByRepository = new WeakMap() + /** * Generates a commit message using the active LLM provider and populates the SCM input box. */ export async function generateCommitMessage(options: GenerateCommitMessageOptions): Promise { const { provider, outputChannel, context } = options + const invocationId = ++invocationCounter try { const git = await getGitApi() @@ -238,6 +325,11 @@ export async function generateCommitMessage(options: GenerateCommitMessageOption return } + if (invocationId > (latestInvocationByRepository.get(repository) ?? 0)) { + latestInvocationByRepository.set(repository, invocationId) + } + const draft = repository.inputBox.value + await vscode.window.withProgress( { location: vscode.ProgressLocation.Notification, @@ -245,25 +337,48 @@ export async function generateCommitMessage(options: GenerateCommitMessageOption cancellable: true, }, async (_progress, token) => { + // One listener aborts the provider request and ends every pending wait below, so the + // notification closes as soon as the user cancels, even when Git or the provider + // ignores the abort signal. const abortController = new AbortController() - const cancellationListener = token.onCancellationRequested(() => { - abortController.abort() + let resolveCancelled: () => void = () => {} + const cancelled = new Promise((resolve) => { + resolveCancelled = () => resolve(undefined) }) + const cancel = () => { + abortController.abort() + resolveCancelled() + } + const cancellationListener = token.onCancellationRequested(cancel) + if (token.isCancellationRequested) { + cancel() + } + const isCancelled = () => token.isCancellationRequested || abortController.signal.aborted + const untilCancelled = (work: Promise): Promise => { + // A rejection after cancellation has no reader; keep it from becoming unhandled. + void work.catch(() => undefined) + return Promise.race([work, cancelled]) + } try { - const diff = await getRepositoryDiff(repository) - if (!diff) { - vscode.window.showInformationMessage( - "No staged or working directory changes found to generate commit message.", - ) + if (isCancelled()) { return } - if (token.isCancellationRequested) { + const diff = await untilCancelled(getRepositoryDiff(repository)) + if (isCancelled()) { + return + } + if (!diff) { + vscode.window.showInformationMessage(NO_CHANGES_MESSAGE) return } - const { apiConfiguration } = await provider.getState() + const state = await untilCancelled(provider.getState()) + if (isCancelled()) { + return + } + const apiConfiguration = state?.apiConfiguration if (!apiConfiguration || !apiConfiguration.apiProvider) { vscode.window.showErrorMessage( "No valid API provider configured. Please configure an API provider in Zoo Code settings.", @@ -271,27 +386,39 @@ export async function generateCommitMessage(options: GenerateCommitMessageOption return } - if (token.isCancellationRequested) { + const { prompt, truncated } = buildCommitMessagePrompt(diff) + const rawMessage = await untilCancelled( + singleCompletionHandler(apiConfiguration as ProviderSettings, prompt, { + abortSignal: abortController.signal, + }), + ) + if (isCancelled() || rawMessage === undefined) { return } - const prompt = buildCommitMessagePrompt(diff) - const rawMessage = await singleCompletionHandler(apiConfiguration as ProviderSettings, prompt, { - abortSignal: abortController.signal, - }) - - if (token.isCancellationRequested || abortController.signal.aborted) { + // A newer request for this repository owns the input box now. + if (latestInvocationByRepository.get(repository) !== invocationId) { return } const commitMessage = cleanCommitMessage(rawMessage) - if (commitMessage) { - repository.inputBox.value = commitMessage - } else { + if (!commitMessage) { vscode.window.showWarningMessage("No commit message generated.") + return + } + + // Do not overwrite text the user typed while the request was pending. + if (repository.inputBox.value !== draft) { + vscode.window.showInformationMessage(DRAFT_CHANGED_MESSAGE) + return + } + + repository.inputBox.value = commitMessage + if (truncated) { + vscode.window.showWarningMessage(DIFF_TRUNCATED_MESSAGE) } } finally { - cancellationListener?.dispose() + cancellationListener.dispose() } }, ) From 7a778b35976fdfe19176b346cd1fd656654700dc Mon Sep 17 00:00:00 2001 From: 1220moritz Date: Tue, 10 Nov 2026 20:11:34 +0100 Subject: [PATCH 09/10] fix(scm): redact the full quoted credential value The key/value and Authorization redaction stopped at "," and ";", so `password: "pass;word123"` kept ";word123" in the prompt. A quoted value now runs to its matching closing quote, with backslash escapes. An unquoted value runs to the next whitespace or quote. The quotes stay in the output, and empty quotes are left unchanged. Refs #2301 Co-Authored-By: Claude Opus 5.5 --- .../__tests__/commitMessageGenerator.spec.ts | 23 ++++++++++++ src/services/git/commitMessageGenerator.ts | 35 +++++++++++++------ 2 files changed, 48 insertions(+), 10 deletions(-) diff --git a/src/services/git/__tests__/commitMessageGenerator.spec.ts b/src/services/git/__tests__/commitMessageGenerator.spec.ts index a65940fef1..3a178caf11 100644 --- a/src/services/git/__tests__/commitMessageGenerator.spec.ts +++ b/src/services/git/__tests__/commitMessageGenerator.spec.ts @@ -183,6 +183,28 @@ describe("commitMessageGenerator", () => { "s3cr3t", ], ["email address", '+ "author": "alice@example.com",', '+ "author": "[REDACTED_EMAIL]",', "alice@"], + [ + "a double-quoted value with a semicolon", + '+password: "pass;word123"', + '+password: "[REDACTED]"', + "word123", + ], + [ + "a JSON value with commas and semicolons", + '+ "api_key": "a,b;c-123",', + '+ "api_key": "[REDACTED]",', + "b;c-123", + ], + ["a single-quoted value with spaces", "+secret = 'two words;x'", "+secret = '[REDACTED]'", "words"], + ["a backtick value", "+const pwd = `p;w`", "+const pwd = `[REDACTED]`", "p;w"], + ["a quoted value with an escaped quote", String.raw`+password: "a\";b"`, '+password: "[REDACTED]"', ";b"], + ["an unterminated quoted value", '+password: "abc;def', "+password: [REDACTED]", "abc"], + [ + "a quoted Authorization value with a semicolon", + '+"Authorization": "Basic ab;cd=="', + '+"Authorization": "Basic [REDACTED]"', + ";cd", + ], ])("redacts %s", (_label, content, expected, secret) => { const redacted = redactSensitiveInformation(content) expect(redacted).toBe(expected) @@ -206,6 +228,7 @@ describe("commitMessageGenerator", () => { ["prose mentioning auth schemes", "+// Adds basic functionality for Bearer authentication"], ["scoped package names", '+import { x } from "@roo-code/types"'], ["versioned package specs", "+npx react@18.3.1"], + ["empty quoted credentials", '+ password: "",'], ])("leaves %s unchanged", (_label, content) => { expect(redactSensitiveInformation(content)).toBe(content) }) diff --git a/src/services/git/commitMessageGenerator.ts b/src/services/git/commitMessageGenerator.ts index b82458c4d9..0b0281c0db 100644 --- a/src/services/git/commitMessageGenerator.ts +++ b/src/services/git/commitMessageGenerator.ts @@ -94,11 +94,22 @@ const CREDENTIAL_KEYWORDS = [ "credentials?", ].join("|") +// `\x60` is a backtick. An assignment operator not followed by `=` or `>` skips comparisons +// (`password === x`) and arrow functions. +const ASSIGNMENT = String.raw`["'\x60]?\s*[:=](?![=>])\s*` +// A quoted value runs to its closing quote, so `;` or `,` inside the quotes is redacted too. +// Backslash escapes are honored. Empty quotes are left alone. +const QUOTED_VALUE = String.raw`(?["'\x60])(?:\\.|(?!\k)[^\\\n])+\k` +// An unquoted (or unterminated) value runs to the next whitespace or quote. +const UNQUOTED_VALUE = String.raw`["'\x60]?[^\s"'\x60]+` +const AUTH_SCHEME = String.raw`(?:Bearer|Basic|Digest|Token)\s+` + /** * Each entry replaces one class of sensitive value. Patterns with a `prefix` capture group keep the - * prefix (key name, header scheme) and replace only the value, so the model still sees which setting - * changed. The order matters: header schemes run before the generic key/value pattern, and URL - * credentials run before the email pattern. + * prefix (key name, header scheme, quotes) and replace only the value, so the model still sees which + * setting changed. The order matters: header schemes run before the generic key/value pattern, and + * URL credentials run before the email pattern. A named group that did not take part in the match is + * replaced with an empty string. */ const REDACTION_RULES: ReadonlyArray<{ pattern: RegExp; replacement: string }> = [ // PEM private key blocks @@ -108,9 +119,11 @@ const REDACTION_RULES: ReadonlyArray<{ pattern: RegExp; replacement: string }> = }, // Authorization header values, with or without a scheme ("Authorization: Basic dXNlcjpwYXNz") { - pattern: - /(?\bauthorization["']?\s*[:=](?![=>])\s*["']?(?:(?:Bearer|Basic|Digest|Token)\s+)?)[^\s"',;]+/gi, - replacement: "$[REDACTED]", + pattern: new RegExp( + String.raw`(?\bauthorization${ASSIGNMENT})(?:(?["'\x60])(?${AUTH_SCHEME})?(?:\\.|(?!\k)[^\\\n])+\k|(?${AUTH_SCHEME})?${UNQUOTED_VALUE})`, + "gi", + ), + replacement: "$$$$[REDACTED]$", }, // Bearer tokens outside an Authorization header. The token must contain a digit so prose such as // "Bearer authentication" is not redacted. @@ -118,11 +131,13 @@ const REDACTION_RULES: ReadonlyArray<{ pattern: RegExp; replacement: string }> = pattern: /(?\b[Bb]earer\s+)(?=[A-Za-z_\-.~+/]*\d)[A-Za-z0-9_\-.~+/]{8,}=*/g, replacement: "$[REDACTED]", }, - // key = value, key: value, "key": "value" (env files, YAML, JSON, code). - // `(?![=>])` skips comparisons (`password === x`) and arrow functions. + // key = value, key: value, "key": "value" (env files, YAML, JSON, code) { - pattern: new RegExp(`(?(?:${CREDENTIAL_KEYWORDS})["']?\\s*[:=](?![=>])\\s*["']?)[^\\s"',;]+`, "gi"), - replacement: "$[REDACTED]", + pattern: new RegExp( + String.raw`(?(?:${CREDENTIAL_KEYWORDS})${ASSIGNMENT})(?:${QUOTED_VALUE}|${UNQUOTED_VALUE})`, + "gi", + ), + replacement: "$$[REDACTED]$", }, // Well-known token formats that can appear without a key name { From d5928a85f266e1ec13f8d1ba1bb85e1b45bdd1ac Mon Sep 17 00:00:00 2001 From: 1220moritz Date: Tue, 10 Nov 2026 21:35:29 +0100 Subject: [PATCH 10/10] fix(scm): address CodeRabbit review round 4 for commit message generation - Redact email addresses with a linear scan that starts at each "@". The unanchored regex rescanned long runs without "@" from every position, which is quadratic on a large diff. - Read untracked files from state.untrackedChanges as well as workingTreeChanges, for git.untrackedChanges = "separate". De-duplicate by path. - Stop suppressing errors by message text. Only a rejection caused by the user's cancel ends quietly. Provider timeouts and other errors that mention "aborted" are now reported. - Remove a test whose in-mock assertion could not fail the test. Refs #2301 Co-Authored-By: Claude Opus 5.5 --- .../__tests__/commitMessageGenerator.spec.ts | 127 +++++++++++++----- src/services/git/commitMessageGenerator.ts | 75 ++++++++--- 2 files changed, 152 insertions(+), 50 deletions(-) diff --git a/src/services/git/__tests__/commitMessageGenerator.spec.ts b/src/services/git/__tests__/commitMessageGenerator.spec.ts index 3a178caf11..5aa8d89153 100644 --- a/src/services/git/__tests__/commitMessageGenerator.spec.ts +++ b/src/services/git/__tests__/commitMessageGenerator.spec.ts @@ -237,6 +237,26 @@ describe("commitMessageGenerator", () => { const content = "diff --git a/src/app.ts b/src/app.ts\n+console.log('hello world')" expect(redactSensitiveInformation(content)).toBe(content) }) + + it.each([ + ["back-to-back addresses", "a@b.co+c@d.co", "[REDACTED_EMAIL][REDACTED_EMAIL]"], + ["addresses in a list", "to: x@y.io, z@w.dev", "to: [REDACTED_EMAIL], [REDACTED_EMAIL]"], + ["an address after a bare @", "@ then bob@example.org", "@ then [REDACTED_EMAIL]"], + ["an address next to a decorator", "@Input() admin@corp.example", "@Input() [REDACTED_EMAIL]"], + ])("redacts %s", (_label, content, expected) => { + expect(redactSensitiveInformation(content)).toBe(expected) + }) + + it.each([ + ["a long run without @", "a".repeat(200_000)], + ["many @ signs with no domain", "a@".repeat(100_000)], + ["many local-part characters before one @", `${"a.".repeat(100_000)}@`], + ])("redacts %s in linear time", (_label, content) => { + // A quadratic scan of 200k characters takes far longer than this budget. + const startedAt = performance.now() + expect(redactSensitiveInformation(content)).toBe(content) + expect(performance.now() - startedAt).toBeLessThan(1000) + }) }) describe("buildCommitMessagePrompt", () => { @@ -641,6 +661,45 @@ describe("commitMessageGenerator", () => { expect(diff).toMatch(/^Untracked files:\nsrc[\\/]new-file\.ts$/) }) + it("lists untracked files from the separate untrackedChanges group", async () => { + const repo = makeRepo("/workspace/repo", { + diff: vi.fn().mockResolvedValue(""), + state: { + workingTreeChanges: [], + untrackedChanges: [{ uri: makeUri("/workspace/repo/brand-new.ts"), status: 7 }], + }, + }) + + expect(await getRepositoryDiff(repo)).toBe("Untracked files:\nbrand-new.ts") + }) + + it("de-duplicates a file listed in both untracked locations", async () => { + const repo = makeRepo("/workspace/repo", { + diff: vi.fn().mockResolvedValue(""), + state: { + workingTreeChanges: [ + { uri: makeUri("/workspace/repo/a.ts"), status: 7 }, + { uri: makeUri("/workspace/repo/modified.ts"), status: 5 }, + ], + untrackedChanges: [ + { uri: makeUri("/workspace/repo/a.ts"), status: 7 }, + { uri: makeUri("/workspace/repo/b.ts"), status: 7 }, + ], + }, + }) + + expect(await getRepositoryDiff(repo)).toBe("Untracked files:\na.ts\nb.ts") + }) + + it("returns undefined when both untracked locations are empty", async () => { + const repo = makeRepo("/workspace/repo", { + diff: vi.fn().mockResolvedValue(""), + state: { workingTreeChanges: [], untrackedChanges: [] }, + }) + + expect(await getRepositoryDiff(repo)).toBeUndefined() + }) + it("surfaces untracked files when both diffs are empty", async () => { const repo: GitRepository = { rootUri: makeUri("/workspace/repo"), @@ -1103,33 +1162,6 @@ describe("commitMessageGenerator", () => { expect(mockRepo.inputBox.value).toBe("") }) - it("aborts when token triggers cancellation callback", async () => { - let cancellationHandler: (() => void) | undefined - ;(vscode.window.withProgress as Mock).mockImplementation((_opts, callback) => { - const progress = { report: vi.fn() } - const token = { - isCancellationRequested: false, - onCancellationRequested: vi.fn((fn: () => void) => { - cancellationHandler = fn - return { dispose: vi.fn() } - }), - } - return callback(progress, token) - }) - ;(singleCompletionHandler as Mock).mockImplementation(async (_config, _prompt, options) => { - cancellationHandler?.() - expect(options.abortSignal.aborted).toBe(true) - return "" - }) - - await generateCommitMessage({ - provider: mockProvider, - outputChannel: mockOutputChannel, - }) - - expect(mockRepo.inputBox.value).toBe("") - }) - it("aborts the in-flight provider request when cancellation is requested", async () => { // Collect ALL registered cancellation handlers so both abortController.abort() // and the cancelPromise resolver are called when the test triggers cancellation. @@ -1186,18 +1218,43 @@ describe("commitMessageGenerator", () => { expect(vscode.window.showWarningMessage).toHaveBeenCalledWith("No commit message generated.") }) - it("ignores AbortError without showing an error message", async () => { - const abortError = new Error("This operation was aborted") - abortError.name = "AbortError" - ;(vscode.window.withProgress as Mock).mockRejectedValue(abortError) + it("stays quiet when the provider rejects synchronously on the user's cancel", async () => { + const { cancel } = useCancellableProgress() + ;(singleCompletionHandler as Mock).mockImplementation( + (_config, _prompt, options: { abortSignal: AbortSignal }) => + new Promise((_resolve, reject) => { + options.abortSignal.addEventListener("abort", () => { + const abortError = new Error("This operation was aborted") + abortError.name = "AbortError" + reject(abortError) + }) + }), + ) - await generateCommitMessage({ - provider: mockProvider, - outputChannel: mockOutputChannel, - }) + const run = generateCommitMessage({ provider: mockProvider, outputChannel: mockOutputChannel }) + await vi.waitFor(() => expect(singleCompletionHandler).toHaveBeenCalled()) + cancel() + await run expect(vscode.window.showErrorMessage).not.toHaveBeenCalled() expect(mockOutputChannel.appendLine).not.toHaveBeenCalled() + expect(mockRepo.inputBox.value).toBe("") + }) + + it.each([ + ["a message that mentions 'aborted'", "request aborted by server", "Error"], + ["a provider timeout", "The operation was aborted due to timeout", "TimeoutError"], + ["an AbortError the user did not cause", "This operation was aborted", "AbortError"], + ])("reports %s when the user did not cancel", async (_label, message, name) => { + const error = new Error(message) + error.name = name + ;(singleCompletionHandler as Mock).mockRejectedValue(error) + + await generateCommitMessage({ provider: mockProvider, outputChannel: mockOutputChannel }) + + expect(vscode.window.showErrorMessage).toHaveBeenCalledWith(`Failed to generate commit message: ${message}`) + expect(mockOutputChannel.appendLine).toHaveBeenCalledWith(`[generateGitCommitMessage] failed: ${message}`) + expect(mockRepo.inputBox.value).toBe("") }) it("handles non-Error objects gracefully and shows error message", async () => { diff --git a/src/services/git/commitMessageGenerator.ts b/src/services/git/commitMessageGenerator.ts index 0b0281c0db..0af021a431 100644 --- a/src/services/git/commitMessageGenerator.ts +++ b/src/services/git/commitMessageGenerator.ts @@ -13,6 +13,7 @@ export interface GitRepositoryChange { export interface GitRepositoryState { readonly workingTreeChanges?: ReadonlyArray + readonly untrackedChanges?: ReadonlyArray } export interface GitRepository { @@ -150,13 +151,42 @@ const REDACTION_RULES: ReadonlyArray<{ pattern: RegExp; replacement: string }> = pattern: /(?:\/\/[^\s:@/]+:)[^\s@/]+(?=@)/g, replacement: "$[REDACTED]", }, - // Email addresses - { - pattern: /[A-Za-z0-9._%+-]+@[A-Za-z0-9-]+(?:\.[A-Za-z0-9-]+)*\.[A-Za-z]{2,}\b/g, - replacement: "[REDACTED_EMAIL]", - }, ] +const EMAIL_LOCAL_PART_CHAR = /[A-Za-z0-9._%+-]/ +const EMAIL_DOMAIN = /[A-Za-z0-9-]+(?:\.[A-Za-z0-9-]+)*\.[A-Za-z]{2,}\b/y + +/** + * Replaces email addresses with [REDACTED_EMAIL] in linear time. + * + * An unanchored `local@domain` regex rescans a long run of local-part characters from every start + * position when the run has no `@`, which is quadratic on a large diff. This scan starts at each `@` + * instead. The local part is read backwards and stops at the end of the previous replacement or at + * the previous `@`, so every character is visited a constant number of times. Back-to-back addresses + * such as `a@b.co+c@d.co` are both replaced. + */ +function redactEmailAddresses(content: string): string { + let result = "" + let copiedUpTo = 0 + let at = content.indexOf("@") + while (at !== -1) { + let start = at + while (start > copiedUpTo && EMAIL_LOCAL_PART_CHAR.test(content[start - 1])) { + start-- + } + EMAIL_DOMAIN.lastIndex = at + 1 + const domain = start < at ? EMAIL_DOMAIN.exec(content) : null + if (domain) { + result += content.slice(copiedUpTo, start) + "[REDACTED_EMAIL]" + copiedUpTo = at + 1 + domain[0].length + at = content.indexOf("@", copiedUpTo) + } else { + at = content.indexOf("@", at + 1) + } + } + return result + content.slice(copiedUpTo) +} + /** * Redacts secrets, private keys, credential tokens, and email addresses from content before it is * sent to an LLM. @@ -166,7 +196,8 @@ export function redactSensitiveInformation(content: string): string { for (const { pattern, replacement } of REDACTION_RULES) { redacted = redacted.replace(pattern, replacement) } - return redacted + // Runs after the URL credential rule so `user:password@host` is handled there first. + return redactEmailAddresses(redacted) } /** @@ -274,11 +305,19 @@ export async function getRepositoryDiff(repository: GitRepository): Promise c.status === UNTRACKED_STATUS) - if (untracked && untracked.length > 0) { + const state = repository.state + const untrackedPaths = new Set( + [ + ...(state?.workingTreeChanges ?? []).filter((c) => c.status === UNTRACKED_STATUS), + ...(state?.untrackedChanges ?? []), + ].map((c) => c.uri.fsPath), + ) + if (untrackedPaths.size > 0) { const rootPath = repository.rootUri.fsPath - const fileList = untracked.map((c) => path.relative(rootPath, c.uri.fsPath) || c.uri.fsPath).join("\n") + const fileList = [...untrackedPaths].map((p) => path.relative(rootPath, p) || p).join("\n") return `Untracked files:\n${fileList}` } @@ -370,9 +409,16 @@ export async function generateCommitMessage(options: GenerateCommitMessageOption } const isCancelled = () => token.isCancellationRequested || abortController.signal.aborted const untilCancelled = (work: Promise): Promise => { - // A rejection after cancellation has no reader; keep it from becoming unhandled. - void work.catch(() => undefined) - return Promise.race([work, cancelled]) + // A rejection caused by the user's cancel (for example, a provider that rejects + // synchronously on abort and wins the race) ends the step quietly. Any other + // rejection reaches the outer catch and is reported. + const guarded = work.catch((error: unknown) => { + if (isCancelled()) { + return undefined + } + throw error + }) + return Promise.race([guarded, cancelled]) } try { @@ -438,9 +484,8 @@ export async function generateCommitMessage(options: GenerateCommitMessageOption }, ) } catch (error) { - if (error instanceof Error && (error.name === "AbortError" || error.message.includes("aborted"))) { - return - } + // The cancel path is handled inside the progress callback, so every error here is a real + // failure, including provider timeouts and errors whose text mentions "aborted". const errorMessage = error instanceof Error ? error.message : String(error) outputChannel?.appendLine(`[generateGitCommitMessage] failed: ${errorMessage}`) vscode.window.showErrorMessage(`Failed to generate commit message: ${errorMessage}`)