mirror of
https://github.com/stablyai/orca.git
synced 2026-09-28 08:02:43 +00:00
fix(agent-hooks): only redirect the Codex hook layout when the home really moved
`installRemote` treats an explicit `codexHomeDir` as a redirected runtime home: it moves the hook script under that home, switches the command wrapper to the readable form and prepends rather than appends the managed hook group. Passing it `~/.codex` for every host applied that WSL-shaped contract to plain SSH hosts too, relocating the script from `~/.orca/agent-hooks/codex-hook.sh` to `~/.codex/.orca/agent-hooks/codex-hook.sh` for users who have no wrapper at all. `resolveRelayRedirectedCodexHome` now answers `null` for the ordinary home, and the installer only receives `codexHomeDir` when Codex really did name somewhere else. Verified against the Linux SSH host: an unwrapped host reproduces the `origin/main` layout exactly, and a wrapped one installs under the reported home.
This commit is contained in:
@@ -17,7 +17,7 @@ const { execFile } = await import('node:child_process')
|
||||
const execFileMock = vi.mocked(execFile)
|
||||
const { execFile: actualExecFile } =
|
||||
await vi.importActual<typeof NodeChildProcess>('node:child_process')
|
||||
const { installManagedHooks, resolveRelayCodexHome, resolveRelayGrokHome } =
|
||||
const { installManagedHooks, resolveRelayGrokHome, resolveRelayRedirectedCodexHome } =
|
||||
await import('./managed-hook-runtime')
|
||||
|
||||
type ExecFileCallback = (error: Error | null, result?: { stdout: string; stderr: string }) => void
|
||||
@@ -160,21 +160,40 @@ describe.runIf(process.platform !== 'win32')('installManagedHooks', () => {
|
||||
})
|
||||
})
|
||||
|
||||
describe.runIf(process.platform !== 'win32')('resolveRelayCodexHome', () => {
|
||||
describe.runIf(process.platform !== 'win32')('resolveRelayRedirectedCodexHome', () => {
|
||||
const CODEX_ONLY = ['codex'] as const
|
||||
|
||||
it('uses the CODEX_HOME the host s own Codex reports', async () => {
|
||||
it('reports the CODEX_HOME the host s own Codex names', async () => {
|
||||
// #19598: a launcher wrapper exports CODEX_HOME inside the script, so only
|
||||
// Codex itself knows. The login-shell environment never sees it.
|
||||
await expect(
|
||||
resolveRelayCodexHome('/home/orca', CODEX_ONLY, undefined, async () => '/home/orca/.codex-x/')
|
||||
resolveRelayRedirectedCodexHome(
|
||||
'/home/orca',
|
||||
CODEX_ONLY,
|
||||
undefined,
|
||||
async () => '/home/orca/.codex-x/'
|
||||
)
|
||||
).resolves.toBe('/home/orca/.codex-x')
|
||||
})
|
||||
|
||||
it('falls back to ~/.codex when Codex does not answer', async () => {
|
||||
// Why null rather than the path: an explicit codexHomeDir switches
|
||||
// installRemote to its redirected-runtime contract (script location, command
|
||||
// wrapper, hook order). An unwrapped host must keep the incumbent layout.
|
||||
it('reports no redirect when Codex names the ordinary home', async () => {
|
||||
await expect(
|
||||
resolveRelayCodexHome('/home/orca', CODEX_ONLY, undefined, async () => null)
|
||||
).resolves.toBe('/home/orca/.codex')
|
||||
resolveRelayRedirectedCodexHome(
|
||||
'/home/orca/',
|
||||
CODEX_ONLY,
|
||||
undefined,
|
||||
async () => '/home/orca/.codex'
|
||||
)
|
||||
).resolves.toBeNull()
|
||||
})
|
||||
|
||||
it('reports no redirect when Codex does not answer', async () => {
|
||||
await expect(
|
||||
resolveRelayRedirectedCodexHome('/home/orca', CODEX_ONLY, undefined, async () => null)
|
||||
).resolves.toBeNull()
|
||||
})
|
||||
|
||||
it.each([
|
||||
@@ -182,18 +201,18 @@ describe.runIf(process.platform !== 'win32')('resolveRelayCodexHome', () => {
|
||||
['Windows', 'C:\\Users\\bob\\.codex'],
|
||||
['control-character', '/home/orca/.codex\u0007'],
|
||||
['empty', ' ']
|
||||
])('falls back when the reported home is %s', async (_label, reported) => {
|
||||
])('reports no redirect when the reported home is %s', async (_label, reported) => {
|
||||
await expect(
|
||||
resolveRelayCodexHome('/home/orca', CODEX_ONLY, undefined, async () => reported)
|
||||
).resolves.toBe('/home/orca/.codex')
|
||||
resolveRelayRedirectedCodexHome('/home/orca', CODEX_ONLY, undefined, async () => reported)
|
||||
).resolves.toBeNull()
|
||||
})
|
||||
|
||||
it('does not probe for a host with no detected Codex', async () => {
|
||||
const probe = vi.fn()
|
||||
|
||||
await expect(resolveRelayCodexHome('/home/orca', ['claude'], undefined, probe)).resolves.toBe(
|
||||
'/home/orca/.codex'
|
||||
)
|
||||
await expect(
|
||||
resolveRelayRedirectedCodexHome('/home/orca', ['claude'], undefined, probe)
|
||||
).resolves.toBeNull()
|
||||
expect(probe).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
@@ -235,7 +254,9 @@ describe.runIf(process.platform !== 'win32')(
|
||||
})
|
||||
})
|
||||
|
||||
it('still installs into ~/.codex when Codex reports the default home', async () => {
|
||||
// The redirected-home contract moves the hook script under CODEX_HOME and
|
||||
// rewrites the command; an unwrapped host must keep the incumbent layout.
|
||||
it('leaves an unwrapped host on its incumbent ~/.codex layout', async () => {
|
||||
const home = await createTempHome()
|
||||
await stubWrappedCodex(home, join(home, '.codex'))
|
||||
|
||||
@@ -245,8 +266,23 @@ describe.runIf(process.platform !== 'win32')(
|
||||
})
|
||||
|
||||
await expect(readFile(join(home, '.codex', 'hooks.json'), 'utf8')).resolves.toContain(
|
||||
'codex-hook.sh'
|
||||
join(home, '.orca', 'agent-hooks', 'codex-hook.sh')
|
||||
)
|
||||
await expect(
|
||||
readFile(join(home, '.codex', '.orca', 'agent-hooks', 'codex-hook.sh'), 'utf8')
|
||||
).rejects.toMatchObject({ code: 'ENOENT' })
|
||||
})
|
||||
|
||||
it('moves the hook script under a redirected home, as the runtime installer does', async () => {
|
||||
const home = await createTempHome()
|
||||
const codexHome = join(home, '.codex-openai')
|
||||
await stubWrappedCodex(home, codexHome)
|
||||
|
||||
await installManagedHooks({ agents: ['codex'] })
|
||||
|
||||
await expect(
|
||||
readFile(join(codexHome, '.orca', 'agent-hooks', 'codex-hook.sh'), 'utf8')
|
||||
).resolves.toContain('#!/bin/sh')
|
||||
})
|
||||
}
|
||||
)
|
||||
|
||||
@@ -80,26 +80,30 @@ export async function resolveRelayGrokHome(home: string, signal?: AbortSignal):
|
||||
}
|
||||
|
||||
/**
|
||||
* Where this host's Codex actually keeps its config.
|
||||
* The Codex home this host redirects to, or `null` for the ordinary `~/.codex`.
|
||||
*
|
||||
* `printenv CODEX_HOME` is not enough: a launcher wrapper exports it inside the
|
||||
* script and `exec`s the real binary, so it never reaches the parent shell
|
||||
* (#19598). Codex's own app-server handshake reports the value the wrapper set,
|
||||
* which is the only authority on the question. Anything else — no Codex on
|
||||
* PATH, a CLI too old for the handshake, a timeout, a non-POSIX answer — falls
|
||||
* back to `~/.codex`, the incumbent behaviour.
|
||||
* which is the only authority on the question. No Codex on PATH, a CLI too old
|
||||
* for the handshake, a timeout, an abort, or a non-POSIX answer all read as
|
||||
* "not redirected".
|
||||
*
|
||||
* Why `null` rather than the default path: `installRemote` treats an explicit
|
||||
* `codexHomeDir` as a redirected runtime home and moves the hook script under
|
||||
* it, switches the command wrapper and reorders the hook groups. Handing it
|
||||
* `~/.codex` would apply that contract to every unwrapped host as well.
|
||||
*/
|
||||
export async function resolveRelayCodexHome(
|
||||
export async function resolveRelayRedirectedCodexHome(
|
||||
home: string,
|
||||
agents: readonly AgentHookTarget[],
|
||||
signal?: AbortSignal,
|
||||
probe: typeof probeCodexHomeViaAppServer = probeCodexHomeViaAppServer
|
||||
): Promise<string> {
|
||||
const fallback = defaultAgentHome(home, '.codex')
|
||||
): Promise<string | null> {
|
||||
// Why: only a positively detected Codex pays for the probe; other hosts must
|
||||
// not start an app-server for a CLI the user never installed.
|
||||
if (!agents.includes('codex')) {
|
||||
return fallback
|
||||
return null
|
||||
}
|
||||
const { shell, flag } = loginShellInvocation()
|
||||
const reported = await probe({
|
||||
@@ -107,7 +111,8 @@ export async function resolveRelayCodexHome(
|
||||
loginShellFlag: flag,
|
||||
...(signal ? { signal } : {})
|
||||
})
|
||||
return (reported === null ? null : normalizePosixAgentHome(reported.trim())) ?? fallback
|
||||
const codexHome = reported === null ? null : normalizePosixAgentHome(reported.trim())
|
||||
return !codexHome || codexHome === defaultAgentHome(home, '.codex') ? null : codexHome
|
||||
}
|
||||
|
||||
export async function installManagedHooks(options?: {
|
||||
@@ -124,7 +129,7 @@ export async function installManagedHooks(options?: {
|
||||
const home = homedir()
|
||||
const grokHomeDir = await resolveRelayGrokHome(home, options?.signal)
|
||||
options?.signal?.throwIfAborted()
|
||||
const codexHomeDir = await resolveRelayCodexHome(home, agents, options?.signal)
|
||||
const codexHomeDir = await resolveRelayRedirectedCodexHome(home, agents, options?.signal)
|
||||
options?.signal?.throwIfAborted()
|
||||
const hostIdentity = scopeManagedHookHostIdentity(
|
||||
await readManagedHookHostIdentity(),
|
||||
@@ -138,7 +143,7 @@ export async function installManagedHooks(options?: {
|
||||
createManagedHookLocalFilesystem(),
|
||||
home,
|
||||
{
|
||||
codexHomeDir,
|
||||
...(codexHomeDir ? { codexHomeDir } : {}),
|
||||
grokHomeDir,
|
||||
signal: options?.signal,
|
||||
agents
|
||||
|
||||
Reference in New Issue
Block a user