Merge pull request #909 from RooVetGit/cte/delete-local-git-config-if-necessary

Delete local git config when appropriate

Author: mrubens

src/services/checkpoints/CheckpointService.ts

@@ -50,6 +50,9 @@ export type CheckpointServiceOptions = {
  */
 
 export class CheckpointService {
+	private static readonly USER_NAME = "Roo Code"
+	private static readonly USER_EMAIL = "[email protected]"
+
 	private _currentCheckpoint?: string
 
 	public get currentCheckpoint() {
@@ -273,6 +276,7 @@ export class CheckpointService {
 		log(
 			`[CheckpointService] taskId = ${taskId}, baseDir = ${baseDir}, currentBranch = ${currentBranch}, currentSha = ${currentSha}, hiddenBranch = ${hiddenBranch}`,
 		)
+
 		return new CheckpointService(taskId, git, baseDir, currentBranch, currentSha, hiddenBranch, log)
 	}
 
@@ -284,16 +288,33 @@ export class CheckpointService {
 			log(`[initRepo] Initialized new Git repository at ${baseDir}`)
 		}
 
-		// Only set user config if not already configured
-		const userName = await git.getConfig("user.name")
-		const userEmail = await git.getConfig("user.email")
+		const globalUserName = await git.getConfig("user.name", "global")
+		const localUserName = await git.getConfig("user.name", "local")
+		const userName = localUserName.value || globalUserName.value
+
+		const globalUserEmail = await git.getConfig("user.email", "global")
+		const localUserEmail = await git.getConfig("user.email", "local")
+		const userEmail = localUserEmail.value || globalUserEmail.value
+
+		// Prior versions of this service indiscriminately set the local user
+		// config, and it should not override the global config. To address
+		// this we remove the local user config if it matches the default
+		// user name and email and there's a global config.
+		if (globalUserName.value && localUserName.value === CheckpointService.USER_NAME) {
+			await git.raw(["config", "--unset", "--local", "user.name"])
+		}
+
+		if (globalUserEmail.value && localUserEmail.value === CheckpointService.USER_EMAIL) {
+			await git.raw(["config", "--unset", "--local", "user.email"])
+		}
 
-		if (!userName.value) {
-			await git.addConfig("user.name", "Roo Code")
+		// Only set user config if not already configured.
+		if (!userName) {
+			await git.addConfig("user.name", CheckpointService.USER_NAME)
 		}
 
-		if (!userEmail.value) {
-			await git.addConfig("user.email", "[email protected]")
+		if (!userEmail) {
+			await git.addConfig("user.email", CheckpointService.USER_EMAIL)
 		}
 
 		if (!isExistingRepo) {

src/services/checkpoints/__tests__/CheckpointService.test.ts

@@ -4,7 +4,7 @@ import fs from "fs/promises"
 import path from "path"
 import os from "os"
 
-import { simpleGit, SimpleGit } from "simple-git"
+import { simpleGit, SimpleGit, SimpleGitTaskCallback } from "simple-git"
 
 import { CheckpointService } from "../CheckpointService"
 
@@ -367,5 +367,47 @@ describe("CheckpointService", () => {
 
 			await fs.rm(baseDir, { recursive: true, force: true })
 		})
+
+		it("removes local git config if it matches default and global exists", async () => {
+			const baseDir = path.join(os.tmpdir(), `checkpoint-service-test-config2-${Date.now()}`)
+			const repo = await initRepo({ baseDir })
+			const newGit = repo.git
+
+			const originalGetConfig = newGit.getConfig.bind(newGit)
+
+			jest.spyOn(newGit, "getConfig").mockImplementation(
+				(
+					key: string,
+					scope?: "system" | "global" | "local" | "worktree",
+					callback?: SimpleGitTaskCallback<string>,
+				) => {
+					if (scope === "global") {
+						if (key === "user.email") {
+							return Promise.resolve({ value: "[email protected]" }) as any
+						}
+						if (key === "user.name") {
+							return Promise.resolve({ value: "Global User" }) as any
+						}
+					}
+
+					return originalGetConfig(key, scope, callback)
+				},
+			)
+
+			await CheckpointService.create({ taskId, git: newGit, baseDir, log: () => {} })
+
+			// Verify local config was removed and global config is used.
+			const localName = await newGit.getConfig("user.name", "local")
+			const localEmail = await newGit.getConfig("user.email", "local")
+			const globalName = await newGit.getConfig("user.name", "global")
+			const globalEmail = await newGit.getConfig("user.email", "global")
+
+			expect(localName.value).toBeNull() // Local config should be removed.
+			expect(localEmail.value).toBeNull()
+			expect(globalName.value).toBe("Global User") // Global config should remain.
+			expect(globalEmail.value).toBe("[email protected]")
+
+			await fs.rm(baseDir, { recursive: true, force: true })
+		})
 	})
 })