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

This commit is contained in:
孔祥俊
2026-03-27 11:11:58 +08:00
parent 1c9f4148d0
commit 661737b95a
4 changed files with 156 additions and 3 deletions

View File

@@ -138,6 +138,8 @@ bunx oh-my-openagent run [prompt]
| `--on-complete <action>` | Action on completion |
| `--verbose` | Enable verbose logging |
`--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

View File

@@ -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,

View File

@@ -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))

View File

@@ -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,