mirror of
https://github.com/wu736139669/hapi.git
synced 2026-08-05 06:24:37 +00:00
fix(cli): preserve codex config args on Windows
This commit is contained in:
@@ -1,5 +1,20 @@
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import { filterResumeSubcommand } from './codexLocal';
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
|
||||
const { spawnWithTerminalGuardMock } = vi.hoisted(() => ({
|
||||
spawnWithTerminalGuardMock: vi.fn(async (_options: unknown) => {})
|
||||
}));
|
||||
|
||||
vi.mock('@/utils/spawnWithTerminalGuard', () => ({
|
||||
spawnWithTerminalGuard: spawnWithTerminalGuardMock
|
||||
}));
|
||||
|
||||
vi.mock('@/ui/logger', () => ({
|
||||
logger: {
|
||||
debug: vi.fn()
|
||||
}
|
||||
}));
|
||||
|
||||
import { codexLocal, filterResumeSubcommand } from './codexLocal';
|
||||
|
||||
describe('filterResumeSubcommand', () => {
|
||||
it('returns empty array unchanged', () => {
|
||||
@@ -24,13 +39,57 @@ describe('filterResumeSubcommand', () => {
|
||||
});
|
||||
|
||||
it('does not filter resume when it appears as flag value', () => {
|
||||
// --name resume should pass through (resume is value, not subcommand)
|
||||
expect(filterResumeSubcommand(['--name', 'resume'])).toEqual(['--name', 'resume']);
|
||||
});
|
||||
|
||||
it('does not filter resume in middle of args', () => {
|
||||
// If resume appears after flags, it's not the subcommand position
|
||||
expect(filterResumeSubcommand(['--model', 'gpt-4', 'resume', '123']))
|
||||
.toEqual(['--model', 'gpt-4', 'resume', '123']);
|
||||
});
|
||||
});
|
||||
|
||||
describe('codexLocal', () => {
|
||||
beforeEach(() => {
|
||||
spawnWithTerminalGuardMock.mockClear();
|
||||
});
|
||||
|
||||
it('launches codex without shell so Windows keeps -c config values as argv elements', async () => {
|
||||
const controller = new AbortController();
|
||||
|
||||
await codexLocal({
|
||||
abort: controller.signal,
|
||||
sessionId: null,
|
||||
path: 'C:\\workspace\\project',
|
||||
onSessionFound: vi.fn(),
|
||||
mcpServers: {
|
||||
hapi: {
|
||||
command: 'C:\\Users\\test\\AppData\\Local\\hapi.exe',
|
||||
args: ['mcp', '--url', 'http://127.0.0.1:63995/']
|
||||
}
|
||||
},
|
||||
sessionHook: {
|
||||
port: 63996,
|
||||
token: 'secret-token'
|
||||
}
|
||||
});
|
||||
|
||||
expect(spawnWithTerminalGuardMock).toHaveBeenCalledOnce();
|
||||
const spawnOptions = spawnWithTerminalGuardMock.mock.calls[0][0] as {
|
||||
command: string;
|
||||
cwd: string;
|
||||
args: string[];
|
||||
shell?: unknown;
|
||||
};
|
||||
expect(spawnOptions).toEqual(expect.objectContaining({
|
||||
command: 'codex',
|
||||
cwd: 'C:\\workspace\\project'
|
||||
}));
|
||||
expect(spawnOptions).not.toHaveProperty('shell');
|
||||
|
||||
const args = spawnOptions.args;
|
||||
const hookArg = args.find((arg) => arg.startsWith('hooks.SessionStart='));
|
||||
expect(hookArg).toBeDefined();
|
||||
expect(hookArg).toContain('{ hooks = [{ type = "command", command = "');
|
||||
expect(args).toContain("mcp_servers.hapi.args=['mcp','--url','http://127.0.0.1:63995/']");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -95,7 +95,6 @@ export async function codexLocal(opts: {
|
||||
spawnName: 'codex',
|
||||
installHint: 'Codex CLI',
|
||||
includeCause: true,
|
||||
logExit: true,
|
||||
shell: process.platform === 'win32'
|
||||
logExit: true
|
||||
});
|
||||
}
|
||||
|
||||
@@ -1,11 +1,13 @@
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
const { execFileSyncMock } = vi.hoisted(() => ({
|
||||
execFileSyncMock: vi.fn()
|
||||
const { spawnSyncMock } = vi.hoisted(() => ({
|
||||
spawnSyncMock: vi.fn()
|
||||
}))
|
||||
|
||||
vi.mock('node:child_process', () => ({
|
||||
execFileSync: execFileSyncMock
|
||||
vi.mock('cross-spawn', () => ({
|
||||
default: {
|
||||
sync: spawnSyncMock
|
||||
}
|
||||
}))
|
||||
|
||||
import {
|
||||
@@ -17,7 +19,7 @@ import {
|
||||
|
||||
describe('codexVersion', () => {
|
||||
beforeEach(() => {
|
||||
execFileSyncMock.mockReset()
|
||||
spawnSyncMock.mockReset()
|
||||
})
|
||||
|
||||
describe('parseCodexVersion', () => {
|
||||
@@ -47,16 +49,24 @@ describe('codexVersion', () => {
|
||||
|
||||
describe('assertCodexLocalSupported', () => {
|
||||
it('passes when codex is new enough', () => {
|
||||
execFileSyncMock.mockReturnValueOnce('codex-cli 0.124.0\n')
|
||||
spawnSyncMock.mockReturnValueOnce({
|
||||
status: 0,
|
||||
stdout: 'codex-cli 0.124.0\n',
|
||||
stderr: ''
|
||||
})
|
||||
|
||||
expect(() => assertCodexLocalSupported()).not.toThrow()
|
||||
expect(execFileSyncMock).toHaveBeenCalledWith('codex', ['--version'], expect.objectContaining({
|
||||
expect(spawnSyncMock).toHaveBeenCalledWith('codex', ['--version'], expect.objectContaining({
|
||||
encoding: 'utf8'
|
||||
}))
|
||||
})
|
||||
|
||||
it('fails when codex is too old', () => {
|
||||
execFileSyncMock.mockReturnValueOnce('codex-cli 0.123.9\n')
|
||||
spawnSyncMock.mockReturnValueOnce({
|
||||
status: 0,
|
||||
stdout: 'codex-cli 0.123.9\n',
|
||||
stderr: ''
|
||||
})
|
||||
|
||||
expect(() => assertCodexLocalSupported()).toThrow(
|
||||
'Codex CLI 0.124.0+ is required for hapi codex local mode because HAPI depends on stable hooks. Detected: 0.123.9. Please upgrade Codex and retry.'
|
||||
@@ -64,7 +74,11 @@ describe('codexVersion', () => {
|
||||
})
|
||||
|
||||
it('fails when the version output cannot be parsed', () => {
|
||||
execFileSyncMock.mockReturnValueOnce('codex-cli version unknown\n')
|
||||
spawnSyncMock.mockReturnValueOnce({
|
||||
status: 0,
|
||||
stdout: 'codex-cli version unknown\n',
|
||||
stderr: ''
|
||||
})
|
||||
|
||||
expect(() => assertCodexLocalSupported()).toThrow(
|
||||
'Could not determine Codex CLI version. Codex CLI 0.124.0+ is required for hapi codex local mode because HAPI depends on stable hooks. Please upgrade Codex and retry.'
|
||||
@@ -74,13 +88,28 @@ describe('codexVersion', () => {
|
||||
it('fails when codex is not available on PATH', () => {
|
||||
const error = new Error('spawnSync codex ENOENT') as NodeJS.ErrnoException
|
||||
error.code = 'ENOENT'
|
||||
execFileSyncMock.mockImplementationOnce(() => {
|
||||
throw error
|
||||
spawnSyncMock.mockReturnValueOnce({
|
||||
status: null,
|
||||
stdout: '',
|
||||
stderr: '',
|
||||
error
|
||||
})
|
||||
|
||||
expect(() => assertCodexLocalSupported()).toThrow(
|
||||
'Codex CLI 0.124.0+ is required for hapi codex local mode because HAPI depends on stable hooks. Codex was not found on PATH. Please install or upgrade Codex and retry.'
|
||||
)
|
||||
})
|
||||
|
||||
it('fails when codex version exits unsuccessfully', () => {
|
||||
spawnSyncMock.mockReturnValueOnce({
|
||||
status: 1,
|
||||
stdout: '',
|
||||
stderr: 'codex failed'
|
||||
})
|
||||
|
||||
expect(() => assertCodexLocalSupported()).toThrow(
|
||||
'Could not determine Codex CLI version. codex failed Codex CLI 0.124.0+ is required for hapi codex local mode because HAPI depends on stable hooks. Please upgrade Codex and retry.'
|
||||
)
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { execFileSync } from 'node:child_process'
|
||||
import spawn from 'cross-spawn'
|
||||
import { withBunRuntimeEnv } from '@/utils/bunRuntime'
|
||||
|
||||
export const MIN_CODEX_HOOKS_VERSION = '0.124.0'
|
||||
@@ -54,30 +54,39 @@ export function isCodexVersionAtLeast(version: string, minimum: string): boolean
|
||||
export function assertCodexLocalSupported(): void {
|
||||
let output: string
|
||||
|
||||
try {
|
||||
output = execFileSync('codex', ['--version'], {
|
||||
encoding: 'utf8',
|
||||
env: withBunRuntimeEnv(),
|
||||
shell: process.platform === 'win32'
|
||||
}).trim()
|
||||
} catch (error) {
|
||||
const maybeError = error as NodeJS.ErrnoException
|
||||
const result = spawn.sync('codex', ['--version'], {
|
||||
encoding: 'utf8',
|
||||
env: withBunRuntimeEnv()
|
||||
})
|
||||
|
||||
if (result.error) {
|
||||
const maybeError = result.error as NodeJS.ErrnoException
|
||||
const message = maybeError?.message ? ` ${maybeError.message}` : ''
|
||||
|
||||
if (maybeError?.code === 'ENOENT') {
|
||||
throw new Error(
|
||||
`${getLocalModeRequirementMessage()} Codex was not found on PATH. Please install or upgrade Codex and retry.`,
|
||||
{ cause: error }
|
||||
{ cause: result.error }
|
||||
)
|
||||
}
|
||||
|
||||
throw new Error(
|
||||
`Could not determine Codex CLI version.${message} ` +
|
||||
`${getLocalModeRequirementMessage()} Please upgrade Codex and retry.`,
|
||||
{ cause: error }
|
||||
{ cause: result.error }
|
||||
)
|
||||
}
|
||||
|
||||
if (result.status !== 0) {
|
||||
const detail = result.stderr ? ` ${result.stderr.trim()}` : ''
|
||||
throw new Error(
|
||||
`Could not determine Codex CLI version.${detail} ` +
|
||||
`${getLocalModeRequirementMessage()} Please upgrade Codex and retry.`
|
||||
)
|
||||
}
|
||||
|
||||
output = result.stdout.trim()
|
||||
|
||||
const version = parseCodexVersion(output)
|
||||
if (!version) {
|
||||
throw new Error(
|
||||
|
||||
@@ -4,15 +4,17 @@ import { EventEmitter } from 'node:events';
|
||||
// Create a fake child process emitter for each test
|
||||
let childEmitter: EventEmitter & { exitCode: number | null; killed: boolean; pid: number };
|
||||
|
||||
vi.mock('node:child_process', () => ({
|
||||
spawn: vi.fn(() => {
|
||||
childEmitter = Object.assign(new EventEmitter(), {
|
||||
exitCode: null,
|
||||
killed: false,
|
||||
pid: 12345,
|
||||
});
|
||||
return childEmitter;
|
||||
}),
|
||||
const spawnMock = vi.hoisted(() => vi.fn(() => {
|
||||
childEmitter = Object.assign(new EventEmitter(), {
|
||||
exitCode: null,
|
||||
killed: false,
|
||||
pid: 12345,
|
||||
});
|
||||
return childEmitter;
|
||||
}));
|
||||
|
||||
vi.mock('cross-spawn', () => ({
|
||||
default: spawnMock
|
||||
}));
|
||||
|
||||
vi.mock('@/ui/logger', () => ({
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import { spawn, type SpawnOptions, type StdioOptions } from 'node:child_process';
|
||||
import type { SpawnOptions, StdioOptions } from 'node:child_process';
|
||||
import spawn from 'cross-spawn';
|
||||
import { logger } from '@/ui/logger';
|
||||
import { killProcessByChildProcess } from '@/utils/process';
|
||||
|
||||
@@ -149,4 +150,3 @@ export async function spawnWithAbort(options: SpawnWithAbortOptions): Promise<vo
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user