fix(scm): 🐛 address commit message provider review feedback

Author: Mirrowel

src/i18n/locales/en/common.json

@@ -270,6 +270,9 @@
 		"generationFailed": "Zoo: Failed to generate commit message: {{errorMessage}}",
 		"contextWarnings": "Zoo: Git context warning: {{warnings}}",
 		"generatingFromUnstaged": "Zoo: Generating message using unstaged changes",
+		"confirmUnstaged": "No staged changes found. Generate a commit message from {{count}} unstaged/untracked changes instead?",
+		"confirmUnstagedAction": "Generate from unstaged changes",
+		"useUnstagedConfirm": "No staged changes were found. Generate a commit message from unstaged changes instead?",
 		"activationFailed": "Zoo: Failed to activate message generator: {{error}}",
 		"providerRegistered": "Zoo: Commit message provider registered",
 		"initializing": "Initializing...",

src/package.json

@@ -218,6 +218,7 @@
 			"scm/input": [
 				{
 					"command": "zoo-code.generateCommitMessage",
+					"when": "scmProvider == git",
 					"group": "navigation"
 				}
 			],

src/services/commit-message/CommitMessageProvider.ts

@@ -1,3 +1,4 @@
+import * as path from "path"
 import * as vscode from "vscode"
 import { ProviderSettingsManager } from "../../core/config/ProviderSettingsManager"
 import { t } from "../../i18n"
@@ -8,13 +9,17 @@ import { CommitMessageGenerator } from "./CommitMessageGenerator"
 import { getCommitMessageGitContextSettings, toGitContextCollectorOptions } from "./gitContextSettings"
 
 interface VscGenerationRequest {
+	/** Source control input box that should receive the generated message. */
 	inputBox: { value: string }
+	/** Root URI supplied by VS Code for the source control command invocation. */
 	rootUri?: vscode.Uri
 }
 
+/** Registers and handles the VS Code command that writes AI commit messages into SCM input. */
 export class CommitMessageProvider implements vscode.Disposable {
 	private generator: CommitMessageGenerator
 
+	/** Creates the provider and wires it to the extension settings store. */
 	constructor(
 		private context: vscode.ExtensionContext,
 		private outputChannel: vscode.OutputChannel,
@@ -24,6 +29,7 @@ export class CommitMessageProvider implements vscode.Disposable {
 		this.generator = new CommitMessageGenerator(providerSettingsManager)
 	}
 
+	/** Registers the generate commit message command with VS Code. */
 	public async activate(): Promise<void> {
 		this.outputChannel.appendLine(t("common:commitMessage.activated"))
 
@@ -36,6 +42,7 @@ export class CommitMessageProvider implements vscode.Disposable {
 		this.context.subscriptions.push(...disposables)
 	}
 
+	/** Handles the command invocation from VS Code's SCM UI. */
 	private async handleVSCodeCommand(vsRequest?: VscGenerationRequest): Promise<void> {
 		try {
 			const workspacePath = this.determineWorkspacePath(vsRequest?.rootUri)
@@ -72,7 +79,6 @@ export class CommitMessageProvider implements vscode.Disposable {
 							vscode.window.showInformationMessage(t("common:commitMessage.noChanges"))
 							return
 						}
-
 						reportProgress(25, t("common:commitMessage.foundChanges", { count: resolution.changes.length }))
 
 						if (!resolution.usedStaged) {
@@ -94,10 +100,14 @@ export class CommitMessageProvider implements vscode.Disposable {
 						}
 
 						reportProgress(70, t("common:commitMessage.generating"))
+						const gitContext = this.appendExistingCommitMessageDraft(
+							gitContextResult.context,
+							targetRepository.inputBox.value,
+						)
 						const message = await this.generator.generateMessage({
 							workspacePath,
 							selectedFiles: resolution.files,
-							gitContext: gitContextResult.context,
+							gitContext,
 							onProgress: (update) => {
 								if (update.percentage !== undefined) {
 									reportProgress(70 + update.percentage * 0.25, update.message)
@@ -118,6 +128,7 @@ export class CommitMessageProvider implements vscode.Disposable {
 		}
 	}
 
+	/** Resolves staged changes, asking before falling back to unstaged worktree changes. */
 	private async resolveCommitChanges(gitCollector: GitContextCollector): Promise<{
 		changes: GitChange[]
 		files: string[]
@@ -127,6 +138,15 @@ export class CommitMessageProvider implements vscode.Disposable {
 		let usedStaged = true
 
 		if (changes.length === 0) {
+			const useUnstaged = await this.confirmUnstagedGeneration()
+			if (!useUnstaged) {
+				return {
+					changes: [],
+					files: [],
+					usedStaged,
+				}
+			}
+
 			changes = await gitCollector.gatherChanges({ staged: false })
 			usedStaged = false
 		}
@@ -138,6 +158,7 @@ export class CommitMessageProvider implements vscode.Disposable {
 		}
 	}
 
+	/** Finds the Git repository that owns the requested workspace path. */
 	private async determineTargetRepository(workspacePath: string): Promise<VscGenerationRequest | null> {
 		try {
 			const gitExtension = vscode.extensions.getExtension("vscode.git")
@@ -154,18 +175,31 @@ export class CommitMessageProvider implements vscode.Disposable {
 				return null
 			}
 
-			for (const repo of gitApi.repositories ?? []) {
-				if (repo.rootUri && workspacePath.startsWith(repo.rootUri.fsPath)) {
-					return repo
-				}
+			const repositories = gitApi.repositories ?? []
+			const matchingRepositories = repositories
+				.filter((repo: VscGenerationRequest) =>
+					repo.rootUri ? isPathWithinRepository(workspacePath, repo.rootUri.fsPath) : false,
+				)
+				.sort(
+					(a: VscGenerationRequest, b: VscGenerationRequest) =>
+						(b.rootUri?.fsPath.length ?? 0) - (a.rootUri?.fsPath.length ?? 0),
+				)
+
+			if (matchingRepositories.length > 0) {
+				return matchingRepositories[0]
+			}
+
+			if (repositories.length === 1) {
+				return repositories[0]
 			}
 
-			return gitApi.repositories[0] ?? null
+			return null
 		} catch (error) {
 			return null
 		}
 	}
 
+	/** Derives the workspace path from the SCM resource or active workspace. */
 	private determineWorkspacePath(resourceUri?: vscode.Uri): string {
 		if (resourceUri) {
 			return resourceUri.fsPath
@@ -179,5 +213,41 @@ export class CommitMessageProvider implements vscode.Disposable {
 		throw new Error("Could not determine workspace path")
 	}
 
+	/** Adds an existing commit input draft to the model context so the next message can improve it. */
+	private appendExistingCommitMessageDraft(gitContext: string, existingDraft: string): string {
+		const normalizedDraft = existingDraft.trim()
+		if (!normalizedDraft) {
+			return gitContext
+		}
+
+		return `${gitContext}
+
+## Existing Commit Message Draft
+The Git commit input already contains this draft. Use it as guidance and generate the best final commit message for the changes. You may improve, replace, or preserve parts of it as appropriate.
+
+\`\`\`
+${normalizedDraft}
+\`\`\``
+	}
+
+	/** Confirms whether unstaged changes may be gathered when there are no staged changes. */
+	private async confirmUnstagedGeneration(): Promise<boolean> {
+		const confirmAction = t("common:commitMessage.confirmUnstagedAction")
+		const choice = await vscode.window.showWarningMessage(
+			t("common:commitMessage.useUnstagedConfirm"),
+			{ modal: true },
+			confirmAction,
+		)
+
+		return choice === confirmAction
+	}
+
+	/** Keeps provider cleanup compatible with VS Code disposable registration. */
 	public dispose(): void {}
 }
+
+/** Returns true when the target path is the repository root or is contained by it. */
+export function isPathWithinRepository(targetPath: string, repositoryPath: string): boolean {
+	const relativePath = path.relative(path.resolve(repositoryPath), path.resolve(targetPath))
+	return relativePath === "" || (!!relativePath && !relativePath.startsWith("..") && !path.isAbsolute(relativePath))
+}

src/services/commit-message/__tests__/CommitMessageProvider.spec.ts

@@ -1,10 +1,25 @@
 import * as path from "path"
+import * as vscode from "vscode"
 
-import { isPathWithinRepository } from "../CommitMessageProvider"
+import { CommitMessageProvider, isPathWithinRepository } from "../CommitMessageProvider"
 
-vi.mock("vscode", () => ({}))
+vi.mock("vscode", () => ({
+	window: {
+		showWarningMessage: vi.fn(),
+	},
+}))
 
 describe("CommitMessageProvider", () => {
+	const createProvider = () =>
+		new CommitMessageProvider(
+			{} as vscode.ExtensionContext,
+			{ appendLine: vi.fn() } as unknown as vscode.OutputChannel,
+		)
+
+	beforeEach(() => {
+		vi.clearAllMocks()
+	})
+
 	it("matches repository roots by path containment instead of string prefix", () => {
 		const root = path.parse(process.cwd()).root
 		const repositoryPath = path.join(root, "work", "app")
@@ -13,4 +28,61 @@ describe("CommitMessageProvider", () => {
 		expect(isPathWithinRepository(repositoryPath, repositoryPath)).toBe(true)
 		expect(isPathWithinRepository(path.join(root, "work", "application"), repositoryPath)).toBe(false)
 	})
+
+	it("adds existing commit input to the generation context", () => {
+		const provider = createProvider()
+		const gitContext = "diff --git a/src/file.ts b/src/file.ts"
+
+		const contextWithDraft = (provider as any).appendExistingCommitMessageDraft(gitContext, "existing message")
+
+		expect(contextWithDraft).toContain(gitContext)
+		expect(contextWithDraft).toContain("## Existing Commit Message Draft")
+		expect(contextWithDraft).toContain("existing message")
+	})
+
+	it("does not add empty commit input to the generation context", () => {
+		const provider = createProvider()
+		const gitContext = "diff --git a/src/file.ts b/src/file.ts"
+
+		expect((provider as any).appendExistingCommitMessageDraft(gitContext, "   ")).toBe(gitContext)
+	})
+
+	it("asks before falling back to unstaged changes", async () => {
+		vi.mocked(vscode.window.showWarningMessage).mockResolvedValue("commitMessage.confirmUnstagedAction" as never)
+		const provider = createProvider()
+		const gitCollector = {
+			gatherChanges: vi
+				.fn()
+				.mockResolvedValueOnce([])
+				.mockResolvedValueOnce([{ filePath: "src/file.ts" }]),
+		}
+
+		const resolution = await (provider as any).resolveCommitChanges(gitCollector)
+
+		expect(vscode.window.showWarningMessage).toHaveBeenCalledWith(
+			"commitMessage.useUnstagedConfirm",
+			{ modal: true },
+			"commitMessage.confirmUnstagedAction",
+		)
+		expect(gitCollector.gatherChanges).toHaveBeenNthCalledWith(1, { staged: true })
+		expect(gitCollector.gatherChanges).toHaveBeenNthCalledWith(2, { staged: false })
+		expect(resolution).toEqual({
+			changes: [{ filePath: "src/file.ts" }],
+			files: ["src/file.ts"],
+			usedStaged: false,
+		})
+	})
+
+	it("does not read unstaged changes when fallback is declined", async () => {
+		vi.mocked(vscode.window.showWarningMessage).mockResolvedValue(undefined)
+		const provider = createProvider()
+		const gitCollector = {
+			gatherChanges: vi.fn().mockResolvedValueOnce([]),
+		}
+
+		const resolution = await (provider as any).resolveCommitChanges(gitCollector)
+
+		expect(gitCollector.gatherChanges).toHaveBeenCalledTimes(1)
+		expect(resolution).toEqual({ changes: [], files: [], usedStaged: true })
+	})
 })

src/services/commit-message/gitContextSettings.ts

@@ -3,6 +3,7 @@ import { defaultCommitMessageGitContextSettings, type CommitMessageGitContextSet
 import { ContextProxy } from "../../core/config/ContextProxy"
 import type { GitContextCollectorOptions } from "../git-context"
 
+/** Reads and normalizes the persisted Git context settings for commit message generation. */
 export function getCommitMessageGitContextSettings(): Required<CommitMessageGitContextSettings> {
 	const rawSettings = ContextProxy.instance.getValue("commitMessageGitContext") as
 		| CommitMessageGitContextSettings
@@ -38,6 +39,7 @@ export function normalizeCommitMessageGitContextSettings(
 	}
 }
 
+/** Converts commit-message settings into options consumed by the Git context collector. */
 export function toGitContextCollectorOptions(
 	staged: boolean,
 	settings: Required<CommitMessageGitContextSettings>,

src/services/commit-message/index.ts

@@ -2,6 +2,7 @@ import * as vscode from "vscode"
 import { CommitMessageProvider } from "./CommitMessageProvider"
 import { t } from "../../i18n"
 
+/** Registers the commit message provider and reports activation failures to the output channel. */
 export function registerCommitMessageProvider(
 	context: vscode.ExtensionContext,
 	outputChannel: vscode.OutputChannel,