Merge pull request #4138 from code-yeongyu/fix/3207-git-master-shell-detect-reland

fix(git-master): use shared shell detection for cross-platform env prefix (fix #3207)
2026-05-18 13:40:33 +09:00
parent fdf7ba2e6f d7caeb0178
commit f0f798d1b8
2 changed files with 198 additions and 7 deletions
@@ -1,7 +1,7 @@
 /// <reference types="bun-types" />

-import { describe, it, expect } from "bun:test"
-import { injectGitMasterConfig } from "./git-master-template-injection"
+import { describe, it, expect, beforeEach, afterEach } from "bun:test"
+import { injectGitMasterConfig, parseBashEnvPrefix, buildShellAwareGitPrefix } from "./git-master-template-injection"

 const SAMPLE_TEMPLATE = [
 	"# Git Master Agent",
@@ -175,3 +175,148 @@ describe("#given idempotency of prefixGitCommandsInBashCodeBlocks", () => {
 		})
 	})
 })
+
+describe("#given parseBashEnvPrefix", () => {
+	describe("#when single VAR=value pair", () => {
+		it("#then parses into a single-entry record", () => {
+			const result = parseBashEnvPrefix("GIT_MASTER=1")
+			expect(result).toEqual({ GIT_MASTER: "1" })
+		})
+	})
+
+	describe("#when multiple VAR=value pairs", () => {
+		it("#then parses all pairs", () => {
+			const result = parseBashEnvPrefix("CI=true DEBIAN_FRONTEND=noninteractive")
+			expect(result).toEqual({ CI: "true", DEBIAN_FRONTEND: "noninteractive" })
+		})
+	})
+
+	describe("#when empty string", () => {
+		it("#then returns empty record", () => {
+			const result = parseBashEnvPrefix("")
+			expect(result).toEqual({})
+		})
+	})
+})
+
+describe("#given buildShellAwareGitPrefix", () => {
+	describe("#when shell type is unix", () => {
+		it("#then returns the bash prefix unchanged", () => {
+			const result = buildShellAwareGitPrefix("GIT_MASTER=1", "unix")
+			expect(result).toBe("GIT_MASTER=1")
+		})
+	})
+
+	describe("#when shell type is powershell", () => {
+		it("#then returns PowerShell $env: syntax", () => {
+			const result = buildShellAwareGitPrefix("GIT_MASTER=1", "powershell")
+			expect(result).toBe("$env:GIT_MASTER='1';")
+		})
+
+		it("#then handles multiple env vars", () => {
+			const result = buildShellAwareGitPrefix("CI=true GIT_MASTER=1", "powershell")
+			expect(result).toBe("$env:CI='true'; $env:GIT_MASTER='1';")
+		})
+	})
+
+	describe("#when shell type is cmd", () => {
+		it("#then returns cmd set syntax", () => {
+			const result = buildShellAwareGitPrefix("GIT_MASTER=1", "cmd")
+			expect(result).toBe('set GIT_MASTER="1" &&')
+		})
+
+		it("#then handles multiple env vars", () => {
+			const result = buildShellAwareGitPrefix("CI=true GIT_MASTER=1", "cmd")
+			expect(result).toBe('set CI="true" && set GIT_MASTER="1" &&')
+		})
+	})
+
+	describe("#when prefix is empty", () => {
+		it("#then returns empty string", () => {
+			const result = buildShellAwareGitPrefix("", "powershell")
+			expect(result).toBe("")
+		})
+	})
+})
+
+describe("#given PowerShell shell detection in injectGitMasterConfig", () => {
+	let originalEnv: Record<string, string | undefined>
+	let originalPlatform: NodeJS.Platform
+
+	beforeEach(() => {
+		originalPlatform = process.platform
+		originalEnv = {
+			SHELL: process.env.SHELL,
+			PSModulePath: process.env.PSModulePath,
+			MSYSTEM: process.env.MSYSTEM,
+		}
+	})
+
+	afterEach(() => {
+		Object.defineProperty(process, "platform", { value: originalPlatform })
+		for (const [key, value] of Object.entries(originalEnv)) {
+			if (value !== undefined) {
+				process.env[key] = value
+			} else {
+				delete process.env[key]
+			}
+		}
+	})
+
+	describe("#when shell is PowerShell (PSModulePath set, no SHELL)", () => {
+		it("#then emits $env: prefix syntax in pwsh code block", () => {
+			delete process.env.SHELL
+			delete process.env.MSYSTEM
+			process.env.PSModulePath = "C:\\Program Files\\PowerShell\\Modules"
+			Object.defineProperty(process, "platform", { value: "win32" })
+
+			const result = injectGitMasterConfig(SAMPLE_TEMPLATE, {
+				commit_footer: false,
+				include_co_authored_by: false,
+				git_env_prefix: "GIT_MASTER=1",
+			})
+
+			expect(result).toContain("$env:GIT_MASTER='1'; git status")
+			expect(result).toContain("```pwsh")
+			expect(result).not.toContain("```bash\n$env:")
+		})
+
+		it("#then does NOT prefix bash code blocks with PowerShell syntax", () => {
+			delete process.env.SHELL
+			delete process.env.MSYSTEM
+			process.env.PSModulePath = "C:\\Program Files\\PowerShell\\Modules"
+			Object.defineProperty(process, "platform", { value: "win32" })
+
+			const result = injectGitMasterConfig(SAMPLE_TEMPLATE, {
+				commit_footer: false,
+				include_co_authored_by: false,
+				git_env_prefix: "GIT_MASTER=1",
+			})
+
+			const bashBlockMatch = result.match(/```bash\r?\n([\s\S]*?)```/g)
+			if (bashBlockMatch) {
+				for (const block of bashBlockMatch) {
+					expect(block).not.toContain("$env:")
+				}
+			}
+		})
+	})
+
+	describe("#when shell is Git Bash on Windows (SHELL env set)", () => {
+		it("#then keeps unix-style prefix", () => {
+			process.env.SHELL = "C:\\Program Files\\Git\\bin\\bash.exe"
+			process.env.PSModulePath = "C:\\Program Files\\PowerShell\\Modules"
+			Object.defineProperty(process, "platform", { value: "win32" })
+
+			const result = injectGitMasterConfig(SAMPLE_TEMPLATE, {
+				commit_footer: false,
+				include_co_authored_by: false,
+				git_env_prefix: "GIT_MASTER=1",
+			})
+
+			expect(result).toContain("GIT_MASTER=1 git status")
+			expect(result).toContain("```bash")
+			expect(result).not.toContain("$env:")
+		})
+	})
+})
@@ -1,18 +1,60 @@
 import { assertValidGitEnvPrefix, type GitMasterConfig } from "../../config/schema"
+import { detectShellType, buildEnvPrefix, type ShellType } from "../../shared/shell-env"

 const BASH_CODE_BLOCK_PATTERN = /```bash\r?\n([\s\S]*?)```/g
 const LEADING_GIT_COMMAND_PATTERN = /^([ \t]*(?:[A-Za-z_][A-Za-z0-9_]*=[^ \t]+\s+)*)git(?=[ \t]|$)/gm
 const INLINE_GIT_COMMAND_PATTERN = /([;&|()][ \t]*)git(?=[ \t]|$)/g

+/**
+ * Parse a bash-format env prefix string ("VAR=value VAR2=value2") into a Record.
+ * Only handles simple KEY=VALUE pairs (no quoting needed since assertValidGitEnvPrefix
+ * already validates the format is shell-safe alphanumeric assignments).
+ */
+export function parseBashEnvPrefix(prefix: string): Record<string, string> {
+	const result: Record<string, string> = {}
+	const pairs = prefix.trim().split(/\s+/)
+	for (const pair of pairs) {
+		const eqIndex = pair.indexOf("=")
+		if (eqIndex === -1) continue
+		const key = pair.slice(0, eqIndex)
+		const value = pair.slice(eqIndex + 1)
+		result[key] = value
+	}
+	return result
+}
+
+/**
+ * Build the shell-aware command prefix for git commands.
+ * Uses the shared shell detection and env prefix builder to emit correct syntax
+ * for PowerShell ($env:VAR='value';), cmd (set VAR="value" &&), or unix (VAR=value).
+ *
+ * For unix shells, we use the inline VAR=value prefix style (not export) to match
+ * the original behavior where the env var applies only to the immediately following command.
+ */
+export function buildShellAwareGitPrefix(bashPrefix: string, shellType?: ShellType): string {
+	if (!bashPrefix) return ""
+	const resolvedShellType = shellType ?? detectShellType()
+	if (resolvedShellType === "unix" || resolvedShellType === "csh") {
+		return bashPrefix
+	}
+	const envRecord = parseBashEnvPrefix(bashPrefix)
+	return buildEnvPrefix(envRecord, resolvedShellType)
+}
+
 export function injectGitMasterConfig(template: string, config?: GitMasterConfig): string {
 	const commitFooter = config?.commit_footer ?? true
 	const includeCoAuthoredBy = config?.include_co_authored_by ?? true
 	const gitEnvPrefix = assertValidGitEnvPrefix(config?.git_env_prefix ?? "GIT_MASTER=1")

-	let result = gitEnvPrefix ? injectGitEnvPrefix(template, gitEnvPrefix) : template
+	const shellType = detectShellType()
+	const shellPrefix = gitEnvPrefix ? buildShellAwareGitPrefix(gitEnvPrefix, shellType) : ""
+	const codeBlockLang = shellType === "powershell" ? "pwsh" : "bash"
+	const skipBashBlockPrefixing = shellType === "powershell" || shellType === "cmd"
+
+	let result = gitEnvPrefix ? injectGitEnvPrefix(template, shellPrefix, codeBlockLang) : template

 	if (commitFooter || includeCoAuthoredBy) {
-		const injection = buildCommitFooterInjection(commitFooter, includeCoAuthoredBy, gitEnvPrefix)
+		const injection = buildCommitFooterInjection(commitFooter, includeCoAuthoredBy, shellPrefix)
 		const insertionPoint = result.indexOf("```\n</execution>")

 		result =
@@ -25,10 +67,14 @@ export function injectGitMasterConfig(template: string, config?: GitMasterConfig
 				: result + "\n\n" + injection
 	}

-	return gitEnvPrefix ? prefixGitCommandsInBashCodeBlocks(result, gitEnvPrefix) : result
+	if (gitEnvPrefix && !skipBashBlockPrefixing) {
+		result = prefixGitCommandsInBashCodeBlocks(result, shellPrefix)
+	}
+
+	return result
 }

-function injectGitEnvPrefix(template: string, prefix: string): string {
+function injectGitEnvPrefix(template: string, prefix: string, codeBlockLang: string): string {
 	const envPrefixSection = [
 		"## GIT COMMAND PREFIX (MANDATORY)",
 		"",
@@ -37,7 +83,7 @@ function injectGitEnvPrefix(template: string, prefix: string): string {
 		"",
 		"This allows custom git hooks to detect when git-master skill is active.",
 		"",
-		"```bash",
+		`\`\`\`${codeBlockLang}`,
 		`${prefix} git status`,
 		`${prefix} git add <files>`,
 		`${prefix} git commit -m "message"`,