diff --git a/src/main/agent-hooks/windows-hook-payload-delivery.test.ts b/src/main/agent-hooks/windows-hook-payload-delivery.test.ts index 79d9f4f60ae..ea71fd13eaf 100644 --- a/src/main/agent-hooks/windows-hook-payload-delivery.test.ts +++ b/src/main/agent-hooks/windows-hook-payload-delivery.test.ts @@ -5,9 +5,9 @@ // through it, because none of them ever piped a payload. This suite pipes one and follows // it to the listener, so a launcher that takes stdin away from the hook fails here (#14818). import { afterEach, describe, expect, it, vi } from 'vitest' -import { spawn } from 'node:child_process' +import { runProcess } from '../../shared/child-process/run-process' import { createServer, type Server } from 'node:http' -import { mkdtempSync, readFileSync, writeFileSync } from 'node:fs' +import { mkdtempSync, writeFileSync } from 'node:fs' import { removeTreeSync } from '../../shared/windows-transient-lock-removal' import { tmpdir } from 'node:os' import { join } from 'node:path' @@ -32,7 +32,9 @@ vi.mock('os', async (importOriginal) => { }) import { ClaudeHookService } from '../claude/hook-service' -import { WINDOWS_CMD_SAFE_PATH } from './installer-utils' +import { MuseHookService } from '../muse/hook-service' +import { getMuseManagedHooksPath } from '../muse/hook-settings' +import { readHooksJson, WINDOWS_CMD_SAFE_PATH } from './installer-utils' import { getConfigPath, getWindowsManagedLifecycleHook } from '../claude/hook-settings' import { findGitBash } from './windows-git-bash-path.test-fixture' @@ -89,38 +91,9 @@ function runHookCommand( args: string[], env: NodeJS.ProcessEnv ): Promise { - return new Promise((resolve, reject) => { - const child = spawn(executable, args, { - stdio: ['pipe', 'pipe', 'pipe'], - windowsHide: true, - env - }) - let stdout = '' - let stderr = '' - let timedOut = false - // Why: Claude Code abandons a hook at 10s, so a launcher that strands the payload reads - // as a timeout to the user; fail the same way instead of hanging the suite. - const timer = setTimeout(() => { - timedOut = true - child.kill('SIGKILL') - }, 15_000) - child.on('error', (error) => { - clearTimeout(timer) - reject(error) - }) - child.stdout.on('data', (chunk: Buffer) => { - stdout += chunk.toString() - }) - child.stderr.on('data', (chunk: Buffer) => { - stderr += chunk.toString() - }) - child.on('close', (exitCode) => { - clearTimeout(timer) - // Why: curl exits once the POST is written; give the listener a beat to finish reading it. - setTimeout(() => resolve({ exitCode, stdout, stderr, timedOut }), 250) - }) - child.stdin.end(Buffer.from(PAYLOAD, 'utf8')) - }) + return runProcess({ program: executable, args, env, input: PAYLOAD, timeoutMs: 15_000 }).then( + (result) => ({ ...result, exitCode: result.code }) + ) } // Why: a developer box may set HKCU\...\Command Processor\AutoRun, which cmd.exe runs before @@ -136,7 +109,7 @@ function hookEnvironment(extra: NodeJS.ProcessEnv): NodeJS.ProcessEnv { const base = Object.fromEntries( Object.entries(process.env).filter(([key]) => !key.startsWith('ORCA_')) ) - return { ...base, ...extra } + return { ...base, ORCA_BACKGROUND_LAUNCH: '1', ...extra } } describe('Windows managed hook launcher', () => { @@ -156,6 +129,7 @@ describe.skipIf(process.platform !== 'win32')('Windows managed hook payload deli let server: Server | null = null afterEach(() => { + vi.unstubAllEnvs() server?.close() server = null homedirMock.mockImplementation(() => process.env.HOME ?? tmpdir()) @@ -165,58 +139,88 @@ describe.skipIf(process.platform !== 'win32')('Windows managed hook payload deli } }) - it('delivers the piped payload to the hook listener through cmd.exe and Git Bash', async () => { - home = mkdtempSync(join(tmpdir(), 'orca-hook-payload-')) - homedirMock.mockReturnValue(home) - seedCmdAutoRunTarget(home) - expect(new ClaudeHookService().install().state).toBe('installed') - - const settings = JSON.parse(readFileSync(getConfigPath(), 'utf8')) as { - hooks: Record - } - // Why: assert nothing about the launcher's shape here — this test's whole value is - // that it fails for any launcher that loses the payload, named conhost or not. - const registeredCommand = settings.hooks.PreToolUse[0].hooks[0].command - // ...with one exception: a cmd-safe profile must reach the script with no interpreter in - // front of it, or #18875's per-event PowerShell start-up has quietly come back. - if (WINDOWS_CMD_SAFE_PATH.test(join(home, '.orca', 'agent-hooks', 'claude-hook.cmd'))) { - expect(registeredCommand).not.toMatch(/powershell|-EncodedCommand/i) + it.each([ + { + name: 'Claude', + install: () => new ClaudeHookService().install(), + configPath: getConfigPath, + script: 'claude-hook.cmd', + stdout: '{}' + }, + { + name: 'Muse', + install: () => new MuseHookService().install(), + configPath: getMuseManagedHooksPath, + script: 'muse-hook.cmd', + stdout: '' } + ])( + '$name delivers the piped payload through cmd.exe and Git Bash', + async (provider) => { + home = mkdtempSync(join(tmpdir(), 'orca-hook-payload-')) + vi.stubEnv('XDG_CONFIG_HOME', undefined) + homedirMock.mockReturnValue(home) + seedCmdAutoRunTarget(home) + expect(provider.install().state).toBe('installed') - const listener = await startHookListener() - server = listener.server - const env = hookEnvironment({ - USERPROFILE: home, - HOME: home, - ORCA_AGENT_HOOK_PORT: String(listener.port), - ORCA_AGENT_HOOK_TOKEN: HOOK_TOKEN, - ORCA_PANE_KEY: PANE_KEY - }) + const settings = readHooksJson(provider.configPath()) + // Why: assert nothing about the launcher's shape here — this test's whole value is + // that it fails for any launcher that loses the payload, named conhost or not. + const registeredCommand = settings?.hooks?.PreToolUse[0]?.hooks?.[0]?.command + if (!registeredCommand) { + throw new Error('Missing managed PreToolUse command') + } + // ...with one exception: a cmd-safe profile must reach the script with no interpreter in + // front of it, or #18875's per-event PowerShell start-up has quietly come back. + if (WINDOWS_CMD_SAFE_PATH.test(join(home, '.orca', 'agent-hooks', provider.script))) { + expect(registeredCommand).not.toMatch(/powershell|-EncodedCommand/i) + } - const shells = [ - { name: 'cmd.exe', executable: 'cmd.exe', args: ['/d', '/c', registeredCommand] }, - { name: 'Git Bash', executable: findGitBash(), args: ['-c', registeredCommand] } - ] - for (const shell of shells) { - const before = listener.posts.length - const result = await runHookCommand(shell.executable, shell.args, env) - expect(result.timedOut, `${shell.name} timed out`).toBe(false) - expect(result.exitCode, `${shell.name} exit code`).toBe(0) + const listener = await startHookListener() + server = listener.server + const env = hookEnvironment({ + USERPROFILE: home, + HOME: home, + ORCA_AGENT_HOOK_PORT: String(listener.port), + ORCA_AGENT_HOOK_TOKEN: HOOK_TOKEN, + ORCA_PANE_KEY: PANE_KEY + }) - // Why: assert delivery before stdout so a launcher that swallows the payload fails - // on the symptom users report, not on some downstream difference in what it printed. - const posts = listener.posts.slice(before) - expect(posts, `${shell.name} posted exactly one hook`).toHaveLength(1) - // Why: byte-exact, not "non-empty" — a pseudoconsole host delivers nothing, and a - // code-page-translating launcher delivers a corrupted body that still looks posted. - expect(posts[0].payload, `${shell.name} payload`).toBe(PAYLOAD) - expect(posts[0].paneKey, `${shell.name} paneKey`).toBe(PANE_KEY) - expect(posts[0].token, `${shell.name} token`).toBe(HOOK_TOKEN) + const shells = [ + { name: 'cmd.exe', executable: 'cmd.exe', args: ['/d', '/c', registeredCommand] }, + { name: 'Git Bash', executable: findGitBash(), args: ['-c', registeredCommand] } + ] + for (const shell of shells) { + const before = listener.posts.length + const result = await runHookCommand(shell.executable, shell.args, env) + expect(result.timedOut, `${shell.name} timed out`).toBe(false) + expect(result.exitCode, `${shell.name} exit code`).toBe(0) - expect(result.stderr, `${shell.name} stderr`).toBe('') - // Why: compat consumers gate the tool call on parseable stdout (#14818). - expect(result.stdout.trim(), `${shell.name} stdout`).toBe('{}') - } - // Why: two shell launches plus a real install can overrun the default under load. - }, 60_000) + // Why: assert delivery before stdout so a launcher that swallows the payload fails + // on the symptom users report, not on some downstream difference in what it printed. + const posts = listener.posts.slice(before) + expect(posts, `${shell.name} posted exactly one hook`).toHaveLength(1) + // Why: byte-exact, not "non-empty" — a pseudoconsole host delivers nothing, and a + // code-page-translating launcher delivers a corrupted body that still looks posted. + expect(posts[0].payload, `${shell.name} payload`).toBe(PAYLOAD) + expect(posts[0].paneKey, `${shell.name} paneKey`).toBe(PANE_KEY) + expect(posts[0].token, `${shell.name} token`).toBe(HOOK_TOKEN) + + expect(result.stderr, `${shell.name} stderr`).toBe('') + // Why: compat consumers gate the tool call on parseable stdout (#14818). + expect(result.stdout.trim(), `${shell.name} stdout`).toBe(provider.stdout) + + const outsideOrca = await runHookCommand( + shell.executable, + shell.args, + hookEnvironment({ USERPROFILE: home, HOME: home }) + ) + expect(outsideOrca.timedOut, `${shell.name} outside Orca timed out`).toBe(false) + expect(outsideOrca.exitCode, `${shell.name} outside Orca exit code`).toBe(0) + expect(listener.posts).toHaveLength(before + 1) + } + // Why: two shell launches plus a real install can overrun the default under load. + }, + 60_000 + ) }) diff --git a/src/main/muse/hook-service.test.ts b/src/main/muse/hook-service.test.ts index af04fc839e1..f25992d3221 100644 --- a/src/main/muse/hook-service.test.ts +++ b/src/main/muse/hook-service.test.ts @@ -1,10 +1,12 @@ -import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' +import { mkdirSync, mkdtempSync, readFileSync, rmSync, statSync, writeFileSync } from 'node:fs' import { tmpdir } from 'node:os' import { join } from 'node:path' -import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { parseMuseSettingsText } from './hook-config-json' import { MuseHookService } from './hook-service' -import { MUSE_HOOK_EVENTS } from './hook-settings' +import { MUSE_HOOK_EVENTS, buildMuseManagedHooksFile, getMuseManagedCommand } from './hook-settings' +import { wrapWindowsHookCommand } from '../agent-hooks/windows-hook-command' +import { createAgentHookMemorySftp } from '../agent-hooks/agent-hook-memory-sftp.test-fixture' // Why: getSharedManagedScriptPath() writes under homedir()/.orca and the // Muse config resolves via XDG_CONFIG_HOME ?? ~/.config/muse. Point HOME @@ -13,9 +15,12 @@ import { MUSE_HOOK_EVENTS } from './hook-settings' let home: string let originalHome: string | undefined let originalXdg: string | undefined +let originalProfile: string | undefined beforeEach(() => { home = mkdtempSync(join(tmpdir(), 'orca-muse-hook-')) + originalProfile = process.env.USERPROFILE + process.env.USERPROFILE = home originalHome = process.env.HOME originalXdg = process.env.XDG_CONFIG_HOME process.env.HOME = home @@ -33,12 +38,24 @@ afterEach(() => { } else { process.env.XDG_CONFIG_HOME = originalXdg } + if (originalProfile === undefined) { + delete process.env.USERPROFILE + } else { + process.env.USERPROFILE = originalProfile + } + vi.restoreAllMocks() rmSync(home, { recursive: true, force: true }) }) const configPath = (): string => join(home, '.config', 'muse', 'settings.json') const managedHooksPath = (): string => join(home, '.orca', 'agent-hooks', 'muse-hooks.json') -const scriptPath = (): string => join(home, '.orca', 'agent-hooks', 'muse-hook.sh') +const scriptPath = (): string => + join( + home, + '.orca', + 'agent-hooks', + process.platform === 'win32' ? 'muse-hook.cmd' : 'muse-hook.sh' + ) describe('MuseHookService', () => { it('reports not_installed before install', () => { @@ -58,14 +75,16 @@ describe('MuseHookService', () => { expect(settings?.managed_hooks_env_vars).toContain('ORCA_PANE_KEY') const managedText = readFileSync(managedHooksPath(), 'utf-8') - expect(managedText).toContain('agent-hooks/muse-hook.sh') + expect(managedText).toContain(process.platform === 'win32' ? 'muse-hook.cmd' : 'muse-hook.sh') expect(MUSE_HOOK_EVENTS.every((event) => managedText.includes(`"${event}"`))).toBe(true) // The managed script must exist and POST to the muse hook endpoint. const script = readFileSync(scriptPath(), 'utf-8') expect(script).toContain('/hook/muse') // Why: payload is piped to curl via stdin so it never lands on the curl // command line (EDR oversized-command-line false positive). - expect(script).toContain('printf \'%s\' "$payload" | curl') + expect(script).toContain( + process.platform === 'win32' ? 'payload@-' : 'printf \'%s\' "$payload" | curl' + ) }) it('keeps user settings when installing, then drops only the pointer on remove', () => { @@ -137,4 +156,57 @@ describe('MuseHookService', () => { expect(() => service.getStatus()).not.toThrow() expect(service.getStatus().state).toBe('partial') }) + + it.skipIf(process.platform !== 'win32')( + 'upgrades encoded registrations once during refresh', + async () => { + const service = new MuseHookService() + service.install() + const settings = readFileSync(configPath(), 'utf8') + writeFileSync( + managedHooksPath(), + buildMuseManagedHooksFile(wrapWindowsHookCommand(scriptPath())) + ) + await service.refreshManagedScripts() + const upgraded = readFileSync(managedHooksPath(), 'utf8') + expect(upgraded).toBe(buildMuseManagedHooksFile(getMuseManagedCommand(scriptPath()))) + expect(upgraded).toContain(getMuseManagedCommand(scriptPath())) + const modified = statSync(managedHooksPath()).mtimeMs + await service.refreshManagedScripts() + expect(readFileSync(managedHooksPath(), 'utf8')).toBe(upgraded) + expect(statSync(managedHooksPath()).mtimeMs).toBe(modified) + expect(readFileSync(configPath(), 'utf8')).toBe(settings) + expect(service.getStatus().state).toBe('installed') + } + ) + + it('refresh never registers absent hooks or replaces a user-owned pointer', async () => { + const service = new MuseHookService() + await service.refreshManagedScripts() + expect(service.getStatus().state).toBe('not_installed') + service.install() + const managed = readFileSync(managedHooksPath(), 'utf8') + writeFileSync(configPath(), JSON.stringify({ managed_hooks_path: '/user/hooks.json' })) + await service.refreshManagedScripts() + expect(readFileSync(managedHooksPath(), 'utf8')).toBe(managed) + expect( + parseMuseSettingsText(readFileSync(configPath(), 'utf8'), 'test')?.managed_hooks_path + ).toBe('/user/hooks.json') + }) + + it('installs POSIX hooks on the SSH host even from a Windows client', async () => { + vi.spyOn(process, 'platform', 'get').mockReturnValue('win32') + const remote = createAgentHookMemorySftp() + const service = new MuseHookService() + expect((await service.installRemote(remote.sftp, '/home/alice smith')).state).toBe('installed') + const remoteHooksPath = '/home/alice smith/.orca/agent-hooks/muse-hooks.json' + const installed = remote.fs.files.get(remoteHooksPath) + expect(installed).toContain('/home/alice smith/.orca/agent-hooks/muse-hook.sh') + expect(installed).not.toMatch(/powershell|EncodedCommand|muse-hook.cmd/) + expect(remote.fs.files.get('/home/alice smith/.orca/agent-hooks/muse-hook.sh')).toContain( + '#!/bin/sh' + ) + await service.installRemote(remote.sftp, '/home/alice smith') + expect(remote.fs.files.get(remoteHooksPath)).toBe(installed) + }) }) diff --git a/src/main/muse/hook-service.ts b/src/main/muse/hook-service.ts index 130e89fe0ca..3b9217ec3fc 100644 --- a/src/main/muse/hook-service.ts +++ b/src/main/muse/hook-service.ts @@ -33,7 +33,8 @@ import { getMuseRemoteManagedCommand, getMuseRemoteManagedHooksPath, MUSE_HOOK_EVENTS, - readManagedMuseHookEvents + readManagedMuseHookEvents, + refreshMuseManagedHooksFile } from './hook-settings' import { MUSE_MANAGED_HOOK_ENV_VARS, @@ -145,7 +146,13 @@ function buildStatus( export class MuseHookService { async refreshManagedScripts(): Promise { - await refreshManagedScriptIfPresent(getMuseManagedScriptPath(), getManagedScript()) + const present = await refreshManagedScriptIfPresent( + getMuseManagedScriptPath(), + getManagedScript() + ) + if (present && process.platform === 'win32') { + await refreshMuseManagedHooksFile() + } } getStatus(): AgentHookInstallStatus { diff --git a/src/main/muse/hook-settings.test.ts b/src/main/muse/hook-settings.test.ts new file mode 100644 index 00000000000..157a5ba28d9 --- /dev/null +++ b/src/main/muse/hook-settings.test.ts @@ -0,0 +1,39 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' +import { wrapWindowsHookCommand } from '../agent-hooks/windows-hook-command' +import { wrapPosixHookCommand } from '../agent-hooks/posix-hook-command' +import { getMuseManagedCommand, getMuseRemoteManagedCommand } from './hook-settings' + +afterEach(() => vi.restoreAllMocks()) + +describe('Muse hook commands', () => { + it('dispatches safe Windows batch paths without PowerShell', () => { + vi.spyOn(process, 'platform', 'get').mockReturnValue('win32') + expect(getMuseManagedCommand('C:\\Users\\alice\\.orca\\agent-hooks\\muse-hook.cmd')).toBe( + 'C:/Users/alice/.orca/agent-hooks/muse-hook.cmd' + ) + }) + + it.each([ + 'C:\\Users\\Alice Smith\\.orca\\agent-hooks\\muse-hook.cmd', + 'C:\\Users\\a&b\\.orca\\agent-hooks\\muse-hook.cmd', + 'C:\\Users\\a%b\\.orca\\agent-hooks\\muse-hook.cmd', + "C:\\Users\\O'Brien\\.orca\\agent-hooks\\muse-hook.cmd", + 'C:\\Users\\日本語\\.orca\\agent-hooks\\muse-hook.cmd', + '\\\\server\\share\\.orca\\agent-hooks\\muse-hook.cmd' + ])('retains the established quoting fallback for %s', (path) => { + vi.spyOn(process, 'platform', 'get').mockReturnValue('win32') + expect(getMuseManagedCommand(path)).toBe(wrapWindowsHookCommand(path)) + }) + + it.each(['darwin', 'linux'] as const)('keeps local %s commands POSIX', (platform) => { + vi.spyOn(process, 'platform', 'get').mockReturnValue(platform) + const path = '/home/alice smith/.orca/agent-hooks/muse-hook.sh' + expect(getMuseManagedCommand(path)).toBe(wrapPosixHookCommand(path)) + }) + + it('keeps SSH commands POSIX when the client runs Windows', () => { + vi.spyOn(process, 'platform', 'get').mockReturnValue('win32') + const path = '/home/alice smith/.orca/agent-hooks/muse-hook.sh' + expect(getMuseRemoteManagedCommand(path)).toBe(wrapPosixHookCommand(path)) + }) +}) diff --git a/src/main/muse/hook-settings.ts b/src/main/muse/hook-settings.ts index 0ef3d5d462b..98d208bd935 100644 --- a/src/main/muse/hook-settings.ts +++ b/src/main/muse/hook-settings.ts @@ -1,5 +1,6 @@ import { homedir } from 'node:os' import { join } from 'node:path' +import { readFile } from 'node:fs/promises' import { buildManagedCommandHook, createManagedCommandMatcher, @@ -9,6 +10,9 @@ import { type HookDefinition } from '../agent-hooks/installer-utils' import { readManagedHookEventsFromJson } from '../agent-hooks/managed-hooks-json-events' +import { wrapWindowsDirectCmdHookCommand } from '../agent-hooks/windows-direct-cmd-hook-command' +import { refreshManagedScriptIfPresent } from '../agent-hooks/managed-hook-script-refresh' +import { parseMuseSettingsText } from './hook-config-json' const MUSE_SCRIPT_BASE = 'muse-hook' @@ -64,7 +68,7 @@ export function getMuseRemoteManagedHooksPath(remoteHome: string): string { export function getMuseManagedCommand(scriptPath: string): string { return process.platform === 'win32' - ? wrapWindowsHookCommand(scriptPath) + ? (wrapWindowsDirectCmdHookCommand(scriptPath) ?? wrapWindowsHookCommand(scriptPath)) : wrapPosixHookCommand(scriptPath) } @@ -83,6 +87,22 @@ export function buildMuseManagedHooksFile(command: string): string { return `${JSON.stringify({ hooks }, null, 2)}\n` } +export async function refreshMuseManagedHooksFile(): Promise { + const managedHooksPath = getMuseManagedHooksPath() + const [settings, existing] = await Promise.all([ + readFile(getMuseConfigPath(), 'utf8').catch(() => null), + readFile(managedHooksPath, 'utf8').catch(() => null) + ]) + const config = settings === null ? null : parseMuseSettingsText(settings, 'Muse settings.json') + if (!existing || config?.managed_hooks_path !== managedHooksPath) { + return + } + const next = buildMuseManagedHooksFile(getMuseManagedCommand(getMuseManagedScriptPath())) + if (existing !== next) { + await refreshManagedScriptIfPresent(managedHooksPath, next) + } +} + export function readManagedMuseHookEvents( parsed: unknown, isManagedCommand: (command: string | undefined) => boolean