Merge pull request #3437 from code-yeongyu/fix/bug-batch-2

fix: Git Bash shell detection, legacy agent name resolution, backup spam

src/shared/migration/config-migration.test.ts

@@ -119,3 +119,52 @@ describe("migrateConfigFile sidecar write ordering", () => {
     expect(statSync(getSidecarPath(configPath)).isDirectory()).toBe(true)
   })
 })
+
+describe("migrateConfigFile backup skipping", () => {
+  test("skips backup when file content is identical after migration", () => {
+    // given - config with legacy key that migrates to same on-disk content
+    const workdir = createWorkdir()
+    const configPath = join(workdir, "oh-my-opencode.json")
+    const migratedContent = {
+      disabled_hooks: ["comment-checker"],
+    }
+
+    // Write the already-migrated content to disk
+    writeFileSync(configPath, JSON.stringify(migratedContent, null, 2) + "\n")
+
+    // rawConfig still has the legacy hook that will be removed
+    const rawConfig: Record<string, unknown> = {
+      disabled_hooks: ["gpt-permission-continuation", "comment-checker"],
+    }
+
+    // when
+    migrateConfigFile(configPath, rawConfig)
+
+    // then - no backup file should be created since file content is unchanged
+    const files = require("fs").readdirSync(workdir) as string[]
+    const backupFiles = files.filter((f: string) => f.includes(".bak."))
+    expect(backupFiles.length).toBe(0)
+  })
+
+  test("creates backup when file content actually changes", () => {
+    // given - config with model that needs migration
+    const workdir = createWorkdir()
+    const configPath = join(workdir, "oh-my-opencode.json")
+    const rawConfig = {
+      agents: {
+        prometheus: { model: "anthropic/claude-opus-4-5" },
+      },
+    }
+
+    writeFileSync(configPath, JSON.stringify(rawConfig, null, 2) + "\n")
+
+    // when
+    const needsWrite = migrateConfigFile(configPath, rawConfig as Record<string, unknown>)
+
+    // then - backup should be created since content changed
+    expect(needsWrite).toBe(true)
+    const files = require("fs").readdirSync(workdir) as string[]
+    const backupFiles = files.filter((f: string) => f.includes(".bak."))
+    expect(backupFiles.length).toBe(1)
+  })
+})

src/shared/migration/config-migration.ts

@@ -143,20 +143,36 @@ export function migrateConfigFile(
   }
 
   if (needsWrite) {
+    let finalConfig = JSON.parse(JSON.stringify(copy)) as Record<string, unknown>
+    const newContent = JSON.stringify(finalConfig, null, 2) + "\n"
+
+    // Compare with existing file content to skip backup when unchanged.
+    // The config may still need an in-memory migration even if the file
+    // content is identical (e.g. removing a deleted hook from disabled_hooks
+    // results in content that was already written by a prior migration).
+    let existingContent: string | undefined
+    try {
+      existingContent = fs.readFileSync(configPath, "utf-8")
+    } catch {
+      // File may not exist yet
+    }
+    const contentChanged = existingContent !== newContent
+
     const timestamp = new Date().toISOString().replace(/[:.]/g, "-")
     const backupPath = `${configPath}.bak.${timestamp}`
     let backupSucceeded = false
-    try {
-      fs.copyFileSync(configPath, backupPath)
-      backupSucceeded = true
-    } catch {
-      backupSucceeded = false
+    if (contentChanged) {
+      try {
+        fs.copyFileSync(configPath, backupPath)
+        backupSucceeded = true
+      } catch {
+        backupSucceeded = false
+      }
     }
 
     let writeSucceeded = false
-    let finalConfig = JSON.parse(JSON.stringify(copy)) as Record<string, unknown>
     try {
-      writeFileAtomically(configPath, JSON.stringify(finalConfig, null, 2) + "\n")
+      writeFileAtomically(configPath, newContent)
       writeSucceeded = true
     } catch (err) {
       log(`Failed to write migrated config to ${configPath}:`, err)

src/shared/shell-env.test.ts

@@ -10,6 +10,7 @@ describe("shell-env", () => {
     originalEnv = {
       SHELL: process.env.SHELL,
       PSModulePath: process.env.PSModulePath,
+      MSYSTEM: process.env.MSYSTEM,
     }
   })
 
@@ -47,6 +48,7 @@ describe("shell-env", () => {
 
     test("#given PSModulePath is set without SHELL #when detectShellType is called #then returns powershell", () => {
       delete process.env.SHELL
+      delete process.env.MSYSTEM
       process.env.PSModulePath = "C:\\Program Files\\PowerShell\\Modules"
       Object.defineProperty(process, "platform", { value: "win32" })
 
@@ -58,6 +60,7 @@ describe("shell-env", () => {
     test("#given Windows platform without PSModulePath #when detectShellType is called #then returns cmd", () => {
       delete process.env.PSModulePath
       delete process.env.SHELL
+      delete process.env.MSYSTEM
       Object.defineProperty(process, "platform", { value: "win32" })
 
       const result = detectShellType()
@@ -68,6 +71,7 @@ describe("shell-env", () => {
     test("#given non-Windows platform without SHELL env var #when detectShellType is called #then returns unix", () => {
       delete process.env.PSModulePath
       delete process.env.SHELL
+      delete process.env.MSYSTEM
       Object.defineProperty(process, "platform", { value: "linux" })
 
       const result = detectShellType()
@@ -94,6 +98,28 @@ describe("shell-env", () => {
 
       expect(result).toBe("unix")
     })
+
+    test("#given MSYSTEM set on Windows without SHELL #when detectShellType is called #then returns unix", () => {
+      delete process.env.SHELL
+      process.env.MSYSTEM = "MINGW64"
+      process.env.PSModulePath = "C:\\Program Files\\PowerShell\\Modules"
+      Object.defineProperty(process, "platform", { value: "win32" })
+
+      const result = detectShellType()
+
+      expect(result).toBe("unix")
+    })
+
+    test("#given MSYSTEM set to MSYS without SHELL #when detectShellType is called #then returns unix", () => {
+      delete process.env.SHELL
+      process.env.MSYSTEM = "MSYS"
+      process.env.PSModulePath = "C:\\Program Files\\PowerShell\\Modules"
+      Object.defineProperty(process, "platform", { value: "win32" })
+
+      const result = detectShellType()
+
+      expect(result).toBe("unix")
+    })
   })
 
   describe("shellEscape", () => {

src/shared/shell-env.ts

@@ -21,6 +21,13 @@ export function detectShellType(): ShellType {
     return "unix"
   }
 
+  // Git Bash on Windows sets MSYSTEM (e.g. "MINGW64", "MINGW32", "MSYS")
+  // even when SHELL is not set. Detect this before PSModulePath which is
+  // always present on Windows regardless of the active shell.
+  if (process.env.MSYSTEM) {
+    return "unix"
+  }
+
   if (process.env.PSModulePath) {
     return "powershell"
   }