Merge pull request #2871 from Jholly2008/kkk/fix-on-complete-hook-shell

fix(cli): respect platform shell for --on-complete

docs/reference/cli.md

@@ -186,6 +186,8 @@ Show version information.
 bunx oh-my-opencode version
 ```
 
+`--on-complete` runs through your current shell when possible: `sh` on Unix shells, `pwsh` for PowerShell on non-Windows, `powershell.exe` for PowerShell on Windows, and `cmd.exe` as the Windows fallback.
+
 ---
 
 ## mcp oauth

src/cli/run/integration.test.ts

@@ -159,8 +159,15 @@ describe("integration: --session-id", () => {
 
 describe("integration: --on-complete", () => {
   let spawnSpy: ReturnType<typeof spyOn>
+  let originalPlatform: NodeJS.Platform
+  let originalEnv: Record<string, string | undefined>
 
   beforeEach(() => {
+    originalPlatform = process.platform
+    originalEnv = {
+      SHELL: process.env.SHELL,
+      PSModulePath: process.env.PSModulePath,
+    }
     spyOn(console, "error").mockImplementation(() => {})
     spawnSpy = spyOn(spawnWithWindowsHideModule, "spawnWithWindowsHide").mockReturnValue({
       exited: Promise.resolve(0),
@@ -172,11 +179,22 @@ describe("integration: --on-complete", () => {
   })
 
   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]
+      }
+    }
     spawnSpy.mockRestore()
   })
 
   it("passes all 4 env vars as strings to spawned process", async () => {
     // given
+    Object.defineProperty(process, "platform", { value: "linux" })
+    process.env.SHELL = "/bin/bash"
+    delete process.env.PSModulePath
     spawnSpy.mockClear()
 
     // when
@@ -206,8 +224,15 @@ describe("integration: option combinations", () => {
   let mockStdout: MockWriteStream
   let mockStderr: MockWriteStream
   let spawnSpy: ReturnType<typeof spyOn>
+  let originalPlatform: NodeJS.Platform
+  let originalEnv: Record<string, string | undefined>
 
   beforeEach(() => {
+    originalPlatform = process.platform
+    originalEnv = {
+      SHELL: process.env.SHELL,
+      PSModulePath: process.env.PSModulePath,
+    }
     spyOn(console, "log").mockImplementation(() => {})
     spyOn(console, "error").mockImplementation(() => {})
     mockStdout = createMockWriteStream()
@@ -222,11 +247,22 @@ describe("integration: option combinations", () => {
   })
 
   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]
+      }
+    }
     spawnSpy?.mockRestore?.()
   })
 
   it("json output and on-complete hook can both execute", async () => {
     // given - json manager active + on-complete hook ready
+    Object.defineProperty(process, "platform", { value: "linux" })
+    process.env.SHELL = "/bin/bash"
+    delete process.env.PSModulePath
     const result: RunResult = {
       sessionId: "session-123",
       success: true,

src/cli/run/on-complete-hook.test.ts

@@ -4,6 +4,9 @@ import * as loggerModule from "../../shared/logger"
 import { executeOnCompleteHook } from "./on-complete-hook"
 
 describe("executeOnCompleteHook", () => {
+  let originalPlatform: NodeJS.Platform
+  let originalEnv: Record<string, string | undefined>
+
   function createStream(text: string): ReadableStream<Uint8Array> | undefined {
     if (text.length === 0) {
       return undefined
@@ -31,15 +34,32 @@ describe("executeOnCompleteHook", () => {
   let logSpy: ReturnType<typeof spyOn<typeof loggerModule, "log">>
 
   beforeEach(() => {
+    originalPlatform = process.platform
+    originalEnv = {
+      SHELL: process.env.SHELL,
+      PSModulePath: process.env.PSModulePath,
+      ComSpec: process.env.ComSpec,
+    }
     logSpy = spyOn(loggerModule, "log").mockImplementation(() => {})
   })
 
   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]
+      }
+    }
     logSpy.mockRestore()
   })
 
-  it("executes command with correct env vars", async () => {
+  it("uses sh on unix shells and passes correct env vars", async () => {
     // given
+    Object.defineProperty(process, "platform", { value: "linux" })
+    process.env.SHELL = "/bin/bash"
+    delete process.env.PSModulePath
     const spawnSpy = spyOn(spawnWithWindowsHideModule, "spawnWithWindowsHide").mockReturnValue(createProc(0))
 
     try {
@@ -68,6 +88,82 @@ describe("executeOnCompleteHook", () => {
     }
   })
 
+  it("uses powershell when PowerShell is detected on Windows", async () => {
+    // given
+    Object.defineProperty(process, "platform", { value: "win32" })
+    process.env.PSModulePath = "C:\\Program Files\\PowerShell\\Modules"
+    delete process.env.SHELL
+    const spawnSpy = spyOn(spawnWithWindowsHideModule, "spawnWithWindowsHide").mockReturnValue(createProc(0))
+
+    try {
+      // when
+      await executeOnCompleteHook({
+        command: "Write-Host done",
+        sessionId: "session-123",
+        exitCode: 0,
+        durationMs: 5000,
+        messageCount: 10,
+      })
+
+      // then
+      const [args] = spawnSpy.mock.calls[0] as Parameters<typeof spawnWithWindowsHideModule.spawnWithWindowsHide>
+      expect(args).toEqual(["powershell.exe", "-NoProfile", "-Command", "Write-Host done"])
+    } finally {
+      spawnSpy.mockRestore()
+    }
+  })
+
+  it("uses pwsh when PowerShell is detected on non-Windows platforms", async () => {
+    // given
+    Object.defineProperty(process, "platform", { value: "linux" })
+    process.env.PSModulePath = "/usr/local/share/powershell/Modules"
+    delete process.env.SHELL
+    const spawnSpy = spyOn(spawnWithWindowsHideModule, "spawnWithWindowsHide").mockReturnValue(createProc(0))
+
+    try {
+      // when
+      await executeOnCompleteHook({
+        command: "Write-Host done",
+        sessionId: "session-123",
+        exitCode: 0,
+        durationMs: 5000,
+        messageCount: 10,
+      })
+
+      // then
+      const [args] = spawnSpy.mock.calls[0] as Parameters<typeof spawnWithWindowsHideModule.spawnWithWindowsHide>
+      expect(args).toEqual(["pwsh", "-NoProfile", "-Command", "Write-Host done"])
+    } finally {
+      spawnSpy.mockRestore()
+    }
+  })
+
+  it("falls back to cmd.exe on Windows when PowerShell is not detected", async () => {
+    // given
+    Object.defineProperty(process, "platform", { value: "win32" })
+    delete process.env.PSModulePath
+    delete process.env.SHELL
+    process.env.ComSpec = "C:\\Windows\\System32\\cmd.exe"
+    const spawnSpy = spyOn(spawnWithWindowsHideModule, "spawnWithWindowsHide").mockReturnValue(createProc(0))
+
+    try {
+      // when
+      await executeOnCompleteHook({
+        command: "echo done",
+        sessionId: "session-123",
+        exitCode: 0,
+        durationMs: 5000,
+        messageCount: 10,
+      })
+
+      // then
+      const [args] = spawnSpy.mock.calls[0] as Parameters<typeof spawnWithWindowsHideModule.spawnWithWindowsHide>
+      expect(args).toEqual(["C:\\Windows\\System32\\cmd.exe", "/d", "/s", "/c", "echo done"])
+    } finally {
+      spawnSpy.mockRestore()
+    }
+  })
+
   it("env var values are strings", async () => {
     // given
     const spawnSpy = spyOn(spawnWithWindowsHideModule, "spawnWithWindowsHide").mockReturnValue(createProc(0))

src/cli/run/on-complete-hook.ts

@@ -1,5 +1,5 @@
 import { spawnWithWindowsHide } from "../../shared/spawn-with-windows-hide"
-import { log } from "../../shared"
+import { detectShellType, log } from "../../shared"
 
 async function readOutput(
   stream: ReadableStream<Uint8Array> | undefined,
@@ -20,6 +20,24 @@ async function readOutput(
   }
 }
 
+function resolveHookShellCommand(command: string): string[] {
+  const shellType = detectShellType()
+
+  switch (shellType) {
+    case "powershell": {
+      const powershellExecutable = process.platform === "win32" ? "powershell.exe" : "pwsh"
+      return [powershellExecutable, "-NoProfile", "-Command", command]
+    }
+    case "cmd":
+      return [process.env.ComSpec || "cmd.exe", "/d", "/s", "/c", command]
+    case "csh":
+      return ["csh", "-c", command]
+    case "unix":
+    default:
+      return ["sh", "-c", command]
+  }
+}
+
 export async function executeOnCompleteHook(options: {
   command: string
   sessionId: string
@@ -37,7 +55,8 @@ export async function executeOnCompleteHook(options: {
   log("Running on-complete hook", { command: trimmedCommand })
 
   try {
-    const proc = spawnWithWindowsHide(["sh", "-c", trimmedCommand], {
+    const shellCommand = resolveHookShellCommand(trimmedCommand)
+    const proc = spawnWithWindowsHide(shellCommand, {
       env: {
         ...process.env,
         SESSION_ID: sessionId,