From e13f3cbaf331d553e2bafcbc176e40ab62db862b Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Mon, 14 Sep 2026 18:19:55 -0700 Subject: [PATCH] feat(agent-hooks): scope integrations and bound runners --- AGENT_STATUS_C4_RESULT.md | 50 +++++ .../hook-script-outside-orca.test.ts | 11 +- src/main/agent-hooks/hook-stdin-contract.ts | 11 + .../managed-agent-hook-controls.test.ts | 18 ++ .../managed-agent-hook-controls.ts | 77 ++++--- .../managed-agent-hook-registry.ts | 201 ++++++++++++------ .../managed-hook-stdin-lifecycle.test.ts | 9 +- .../server-hook-http-ingest.test.ts | 25 +++ .../agent-hooks/server/server-lifecycle.ts | 13 +- src/main/codex/codex-hook-script.test.ts | 60 ++++++ src/main/codex/codex-hook-script.ts | 10 +- src/main/hermes/hermes-config-yaml.ts | 52 ++++- src/main/hermes/hermes-home-filesystem.ts | 51 ++++- src/main/hermes/hook-service.test.ts | 44 +++- src/main/hermes/hook-service.ts | 44 ++-- src/main/ipc/pty/host-env/assembly.ts | 11 +- .../ipc/pty/host-env/pi-agent-scope.test.ts | 69 ++++++ src/main/ipc/pty/host-env/pi-agent.ts | 61 +++++- src/relay/agent-hook-server.test.ts | 26 +++ src/relay/agent-hook-server.ts | 43 ++-- src/relay/agent-hook-spool-ingest.ts | 33 +++ src/relay/plugin-overlay-env.test.ts | 28 ++- src/relay/plugin-overlay-env.ts | 65 +++++- src/relay/relay-agent-hook-runtime.ts | 9 +- .../agent-hook-listener-transport.test.ts | 4 +- .../agent-hook-listener/request-body.ts | 43 +++- 26 files changed, 906 insertions(+), 162 deletions(-) create mode 100644 AGENT_STATUS_C4_RESULT.md create mode 100644 src/main/codex/codex-hook-script.test.ts create mode 100644 src/main/ipc/pty/host-env/pi-agent-scope.test.ts create mode 100644 src/relay/agent-hook-spool-ingest.ts diff --git a/AGENT_STATUS_C4_RESULT.md b/AGENT_STATUS_C4_RESULT.md new file mode 100644 index 00000000000..7df89e93339 --- /dev/null +++ b/AGENT_STATUS_C4_RESULT.md @@ -0,0 +1,50 @@ +# C4 integration implementation result + +## Scope delivered + +This branch establishes the first C4 integration slice on the execution host: + +- Added one `ManagedAgentIntegration` descriptor per existing managed vendor. Install, refresh, remove, async-remove, and status projections now derive from that descriptor, while legacy tuple exports remain compatible for existing callers. The lifecycle loop consumes descriptors, so operation lists cannot silently diverge. +- Made Hermes lifecycle operations profile-aware. Explicit `--profile`, `-p`, and `--profile=` launch arguments take precedence over `active_profile`; profile names are validated before path construction. Install/status/remove use the selected profile home, and YAML updates preserve existing top-level and nested comments while retaining atomic unchanged-write behavior. +- Added a bounded Codex JSON stdin runner for POSIX launchers. It returns after a complete JSON value even when the caller keeps stdin open, never falls back to an unbounded `cat`, and emits neutral `{}` output on fail-open paths on POSIX and Windows. +- Classified oversized listener payloads with `AgentHookRequestTooLargeError`. Main and relay listeners return JSON HTTP 413 with the fixed one-megabyte limit, pause the request before responding, and do not forward the rejected event. +- Resolved OMP profile/config roots before `PI_CODING_AGENT_DIR` exists, validated profile names, and propagated effective XDG data/state/cache roots from shell startup or inherited process environment into local PTY and relay-spawned environments. + +## Evidence + +- Focused C4 integration run: 10 files, 97 passed, 2 skipped. +- Broader integration run: 13 files, 185 passed, 4 skipped. +- Additional hook/transport run: 6 files, 105 passed, 7 skipped. +- Final profile/source-scope run: 2 files, 9 passed. +- `pnpm tc:node` passed. +- `pnpm run check:code-quality:changed` passed with zero new findings, including type-aware, React Doctor, and casting-safety checks. +- `pnpm exec oxfmt --check` passed for all changed/new implementation and test files. +- `git diff --check` passed. + +Representative coverage includes managed lifecycle projection, stale-script refresh, profile selection and comment preservation, open-stdin Codex execution, outside-Orca fail-open output, oversized relay requests, listener size bounds, OMP shell/XDG resolution, and existing Windows/WSL hook command contracts. + +## Cases not closed by this branch + +These remain intentionally unresolved and are not claimed as fixed: + +- OpenCode V1/V2 loader migration and a verified current vendor adapter were not changed; Auggie has no verified vendor API and remains unimplemented. +- Overlay-only agents (`opencode`, `mimo-code`, `pi`, `omp`, and `prime-agent`) are not yet represented in the managed installer/status registry. Their per-launch overlay paths remain outside the descriptor lifecycle. +- Artifact version markers, loader acceptance evidence, round-trip delivery health, and a general integration-health state model were not added. +- Claude running-session account ownership/hot switching remains pending the execution binding contract. This branch does not select credentials through trust state, restart a live process, or claim a credential change succeeded. +- Remote Hermes profile launch plumbing needs an execution-host launch context from the remote PTY path; the local/profile filesystem resolver is implemented and tested. +- Direct executable-argv/fish reproductions for STA-5230 and STA-3936, and historical Kimi/OMP/Prime behavior, need additional vendor/runtime evidence. +- Rerouted completion, readiness, interaction, and publication cases remain with their owning batches. + +## Required sibling integration + +- C5 must publish the run/attachment binding contract so integration scope and account roots can be resolved per execution without a second identity or reservation mechanism. +- C10 must consume that binding for launch membership/adoption; C4’s descriptors must not become a competing launch registry. +- C1 provides the canonical turn reducer; C2 consumes it for provider recovery. C4 adapters must publish normalized provider facts into that reducer rather than adjudicating completion independently. +- C3 owns readiness and prompt-delivery evidence; C6 owns exact-attachment execution evidence; C7 owns host composition, remote publication, and replica cutover. Remote status must not be inferred from this branch’s local filesystem reads. + +## Judgments + +- **Architecture fit:** The descriptor projection removes the fragmented local lifecycle ownership and keeps vendor-specific operations behind one host-side contract. Profile and environment resolution now follow launch scope. The full host registry, overlay integration, artifact provenance, and account-binding architecture are still pending the contracts above. +- **Functional correctness:** The implemented slices are covered by executable tests and the stated validation gates. They handle bounded input, fail-open output, profile-safe paths, comment-preserving updates, and local/relay OMP environment propagation. Unsupported vendor/API and account-transition behavior remains unverified. +- **Private precedent limitations:** The implementation matches the useful mechanism of a single descriptor-driven lifecycle and scoped, atomic materialization. It does not yet match a complete verified-integration health model, vendor-owned transport migration, or host-authoritative remote registry; those are recorded as deviations rather than inferred from passing tests. +- **Validation:** All listed checks passed on the current branch. Electron/mobile UI validation was not applicable; no app window was launched. diff --git a/src/main/agent-hooks/hook-script-outside-orca.test.ts b/src/main/agent-hooks/hook-script-outside-orca.test.ts index 6e9a27f6acc..9d8be12b380 100644 --- a/src/main/agent-hooks/hook-script-outside-orca.test.ts +++ b/src/main/agent-hooks/hook-script-outside-orca.test.ts @@ -7,7 +7,8 @@ import { _internals as codexInternals } from '../codex/hook-service' import { buildPosixHookSpoolLines } from './hook-stdin-contract' /** Managed hooks are installed into the user's agent config, so they also run when the - * agent is launched from a plain terminal. There they must be inert and silent. */ + * agent is launched from a plain terminal. There they must be inert and return the + * vendor-required neutral JSON response without creating Orca state. */ function runHook(dir: string, extraEnv: NodeJS.ProcessEnv = {}) { const script = join(dir, 'codex-hook.sh') writeFileSync(script, codexInternals.getManagedScript('posix')) @@ -31,7 +32,7 @@ describe('managed hook outside an Orca terminal', () => { const dir = mkdtempSync(join(tmpdir(), 'orca-outside-')) const res = runHook(dir) expect(res.status).toBe(0) - expect(res.stdout).toBe('') + expect(res.stdout).toBe('{}\n') expect(res.stderr).toBe('') expect(readdirSync(dir)).toEqual(['codex-hook.sh']) }) @@ -40,7 +41,7 @@ describe('managed hook outside an Orca terminal', () => { const dir = mkdtempSync(join(tmpdir(), 'orca-outside-partial-')) const res = runHook(dir, { ORCA_PANE_KEY: 'tab:0', ORCA_TAB_ID: 'tab' }) expect(res.status).toBe(0) - expect(res.stdout).toBe('') + expect(res.stdout).toBe('{}\n') expect(res.stderr).toBe('') expect(readdirSync(dir)).toEqual(['codex-hook.sh']) }) @@ -52,7 +53,7 @@ describe('managed hook outside an Orca terminal', () => { ORCA_PANE_KEY: 'tab:0' }) expect(res.status).toBe(0) - expect(res.stdout).toBe('') + expect(res.stdout).toBe('{}\n') expect(res.stderr).toBe('') // a stale env var must not create a spool tree for an Orca that is not installed here expect(readdirSync(dir)).toEqual(['codex-hook.sh']) @@ -64,7 +65,7 @@ describe('managed hook outside an Orca terminal', () => { writeFileSync(endpoint, 'ORCA_AGENT_HOOK_PORT=9\nORCA_AGENT_HOOK_TOKEN=stale\n') const res = runHook(dir, { ORCA_AGENT_HOOK_ENDPOINT: endpoint }) expect(res.status).toBe(0) - expect(res.stdout).toBe('') + expect(res.stdout).toBe('{}\n') expect(res.stderr).toBe('') expect(readdirSync(dir).sort()).toEqual(['codex-hook.sh', 'endpoint.env']) }) diff --git a/src/main/agent-hooks/hook-stdin-contract.ts b/src/main/agent-hooks/hook-stdin-contract.ts index eeec395b578..7be5b8e5879 100644 --- a/src/main/agent-hooks/hook-stdin-contract.ts +++ b/src/main/agent-hooks/hook-stdin-contract.ts @@ -89,6 +89,12 @@ const POSIX_HOOK_JSON_STDIN_HOME_GUARD = '{ [ -d "${HOME:-}" ] || unset HOME; }' // that only ships `python` does not drop straight to the `cat` hang. export const POSIX_HOOK_JSON_STDIN_READER = `${POSIX_HOOK_JSON_STDIN_HOME_GUARD}; ${jsonStdinInterpreter('python3')} || ${jsonStdinInterpreter('python')} || ${POSIX_HOOK_STDIN_READER}` +// Hook callers that keep stdin open (notably Codex's macOS bridge) must never +// fall back to an unbounded `cat`. Missing Python is a bounded no-op: the +// vendor can continue without an Orca observation, while its TUI remains +// responsive and the event can still be recovered by the provider adapter. +export const POSIX_HOOK_BOUNDED_JSON_STDIN_READER = `${POSIX_HOOK_JSON_STDIN_HOME_GUARD}; ${jsonStdinInterpreter('python3')} || ${jsonStdinInterpreter('python')} || { exit 0; }` + /** Optional reader override for an agent whose caller keeps stdin open after the payload. * `prelude` must be emitted before the capture line; keep them together. */ export type PosixHookStdinReader = { @@ -101,6 +107,11 @@ export const POSIX_HOOK_JSON_STDIN: PosixHookStdinReader = { prelude: POSIX_HOOK_JSON_STDIN_PRELUDE } +export const POSIX_HOOK_BOUNDED_JSON_STDIN: PosixHookStdinReader = { + reader: POSIX_HOOK_BOUNDED_JSON_STDIN_READER, + prelude: POSIX_HOOK_JSON_STDIN_PRELUDE +} + // Why: every POSIX hook must own stdin before any no-op exit; sharing this // prelude prevents agent templates from inventing different drain semantics. export function buildPosixHookPayloadCapture( diff --git a/src/main/agent-hooks/managed-agent-hook-controls.test.ts b/src/main/agent-hooks/managed-agent-hook-controls.test.ts index 625cc06f505..b69a787266a 100644 --- a/src/main/agent-hooks/managed-agent-hook-controls.test.ts +++ b/src/main/agent-hooks/managed-agent-hook-controls.test.ts @@ -24,6 +24,24 @@ vi.mock('../claude/claude-session-end-hook-capability', () => ({ })) vi.mock('./managed-agent-hook-registry', () => ({ + MANAGED_AGENT_INTEGRATIONS: [ + { + agent: 'claude', + install: mocks.installClaude, + refreshManagedScripts: mocks.refreshClaude, + remove: mocks.removeClaude, + removeAsync: mocks.removeClaudeAsync, + getStatus: mocks.statusClaude + }, + { + agent: 'codex', + install: mocks.installCodex, + refreshManagedScripts: mocks.refreshCodex, + remove: mocks.removeCodex, + removeAsync: mocks.removeCodexAsync, + getStatus: mocks.statusCodex + } + ], MANAGED_AGENT_HOOK_INSTALLERS: [ ['claude', mocks.installClaude], ['codex', mocks.installCodex] diff --git a/src/main/agent-hooks/managed-agent-hook-controls.ts b/src/main/agent-hooks/managed-agent-hook-controls.ts index 7edfd94c7c8..ac08ab7a133 100644 --- a/src/main/agent-hooks/managed-agent-hook-controls.ts +++ b/src/main/agent-hooks/managed-agent-hook-controls.ts @@ -13,7 +13,8 @@ import { MANAGED_AGENT_HOOK_REMOVERS, MANAGED_AGENT_HOOK_SCRIPT_REFRESHERS, MANAGED_AGENT_HOOK_STATUS_READERS, - type ManagedAgentHookInstaller, + MANAGED_AGENT_INTEGRATIONS, + type ManagedAgentIntegration, type ManagedAgentHookInstallOptions } from './managed-agent-hook-registry' @@ -103,20 +104,42 @@ function skippedStatus( } } -function selectedInstallers(options: InstallOptions): readonly ManagedAgentHookInstaller[] { +function managedIntegrations(): readonly ManagedAgentIntegration[] { + if (Array.isArray(MANAGED_AGENT_INTEGRATIONS)) { + return MANAGED_AGENT_INTEGRATIONS + } + + // Compatibility for embedders that mocked the tuple projections before the + // descriptor registry existed. Production always takes the branch above. + const refreshers = new Map(MANAGED_AGENT_HOOK_SCRIPT_REFRESHERS) + const removers = new Map(MANAGED_AGENT_HOOK_REMOVERS) + const asyncRemovers = new Map(MANAGED_AGENT_HOOK_ASYNC_REMOVERS) + const readers = new Map(MANAGED_AGENT_HOOK_STATUS_READERS) + return MANAGED_AGENT_HOOK_INSTALLERS.map(([agent, install]) => ({ + agent, + install, + refreshManagedScripts: refreshers.get(agent), + remove: removers.get(agent) ?? (() => errorStatus(agent, 'remove is unavailable')), + removeAsync: asyncRemovers.get(agent), + getStatus: readers.get(agent) ?? (() => errorStatus(agent, 'status is unavailable')) + })) +} + +function selectedIntegrations(options: InstallOptions): readonly ManagedAgentIntegration[] { + const integrations = managedIntegrations() if (!options.agents) { - return MANAGED_AGENT_HOOK_INSTALLERS + return integrations } const allowed = new Set(options.agents) - return MANAGED_AGENT_HOOK_INSTALLERS.filter(([agent]) => allowed.has(agent)) + return integrations.filter(({ agent }) => allowed.has(agent)) } async function runInstaller( - entry: ManagedAgentHookInstaller, + entry: ManagedAgentIntegration, onInstallError: InstallOptions['onInstallError'], options: ManagedAgentHookInstallOptions ): Promise { - const [agent, install] = entry + const { agent, install } = entry try { return await install(options) } catch (error) { @@ -136,8 +159,8 @@ async function runInstaller( // current before any gating; creating new ones remains install()'s presence-gated job. async function refreshExistingManagedScripts(options: InstallOptions): Promise { const allowed = options.agents ? new Set(options.agents) : null - for (const [agent, refresh] of MANAGED_AGENT_HOOK_SCRIPT_REFRESHERS) { - if (allowed !== null && !allowed.has(agent)) { + for (const { agent, refreshManagedScripts: refresh } of managedIntegrations()) { + if (!refresh || (allowed !== null && !allowed.has(agent))) { continue } try { @@ -153,10 +176,10 @@ export async function installManagedAgentHooks( options: InstallOptions = {} ): Promise { await refreshExistingManagedScripts(options) - const installers = selectedInstallers(options) + const installers = selectedIntegrations(options) const disabled = new Set(normalizeDisabledTuiAgents(settings?.disabledTuiAgents)) - const enabledInstallers = installers.filter(([agent]) => !disabled.has(agent)) - const targets = enabledInstallers.flatMap(([agent]) => { + const enabledInstallers = installers.filter(({ agent }) => !disabled.has(agent)) + const targets = enabledInstallers.flatMap(({ agent }) => { const target = getManagedAgentHookTarget(agent) return target ? [target] : [] }) @@ -167,7 +190,7 @@ export async function installManagedAgentHooks( }) } catch (error) { const detail = error instanceof Error ? error.message : String(error) - return installers.map(([agent]) => + return installers.map(({ agent }) => disabled.has(agent) ? skippedStatus(agent, 'agent_disabled', 'Agent is disabled in Settings.') : skippedStatus(agent, 'cli_presence_unknown', detail) @@ -176,7 +199,7 @@ export async function installManagedAgentHooks( const results: AgentHookInstallStatus[] = [] for (const entry of installers) { - const [agent] = entry + const { agent } = entry if (disabled.has(agent)) { results.push(skippedStatus(agent, 'agent_disabled', 'Agent is disabled in Settings.')) continue @@ -221,7 +244,7 @@ export async function removeManagedAgentHooks( ): Promise { const allowed = options.agents ? new Set(options.agents) : null const results: AgentHookInstallStatus[] = [] - for (const [agent, remove] of MANAGED_AGENT_HOOK_REMOVERS) { + for (const { agent, remove } of managedIntegrations()) { if (allowed !== null && !allowed.has(agent)) { continue } @@ -239,20 +262,26 @@ export async function removeManagedAgentHooksAsync( ): Promise { const allowed = options.agents ? new Set(options.agents) : null return await Promise.all( - MANAGED_AGENT_HOOK_ASYNC_REMOVERS.filter( - ([agent]) => allowed === null || allowed.has(agent) - ).map(async ([agent, remove]) => { - try { - return await remove() - } catch (error) { - return errorStatus(agent, error) - } - }) + managedIntegrations() + .filter( + ({ agent, removeAsync }) => + removeAsync !== undefined && (allowed === null || allowed.has(agent)) + ) + .map(async ({ agent, removeAsync }) => { + if (!removeAsync) { + return errorStatus(agent, 'remove is unavailable') + } + try { + return await removeAsync() + } catch (error) { + return errorStatus(agent, error) + } + }) ) } export function getManagedAgentHookStatuses(): AgentHookInstallStatus[] { - return MANAGED_AGENT_HOOK_STATUS_READERS.map(([agent, getStatus]) => { + return managedIntegrations().map(({ agent, getStatus }) => { try { return getStatus() } catch (error) { diff --git a/src/main/agent-hooks/managed-agent-hook-registry.ts b/src/main/agent-hooks/managed-agent-hook-registry.ts index 49fdcadda42..49b586a0854 100644 --- a/src/main/agent-hooks/managed-agent-hook-registry.ts +++ b/src/main/agent-hooks/managed-agent-hook-registry.ts @@ -36,78 +36,151 @@ export type ManagedAgentHookAsyncRemover = readonly [ ] export type ManagedAgentHookStatusReader = readonly [HookInstallAgent, () => AgentHookInstallStatus] -export const MANAGED_AGENT_HOOK_INSTALLERS: readonly ManagedAgentHookInstaller[] = [ - ['claude', (options) => claudeHookService.install({ claudeVersion: options?.cliVersion })], - ['openclaude', () => openClaudeHookService.install()], - ['codex', () => codexHookService.install()], - ['gemini', () => geminiHookService.install()], - ['antigravity', () => antigravityHookService.install()], - ['amp', () => ampHookService.install()], - ['cursor', () => cursorHookService.install()], - ['droid', () => droidHookService.install()], - ['command-code', () => commandCodeHookService.install()], - ['grok', (options) => grokHookService.install(options)], - ['copilot', () => copilotHookService.install()], - ['hermes', () => hermesHookService.install()], - ['devin', () => devinHookService.install()], - ['kimi', () => kimiHookService.install()] +/** + * The complete lifecycle for one vendor integration. The tuple exports below + * remain as a compatibility projection for older callers, but new lifecycle + * code should consume this descriptor so install, refresh, remove and status + * cannot silently drift into different vendor lists. + */ +export type ManagedAgentIntegration = { + readonly agent: HookInstallAgent + readonly install: ( + options?: ManagedAgentHookInstallOptions + ) => AgentHookInstallStatus | Promise + readonly refreshManagedScripts?: () => Promise + readonly remove: () => AgentHookInstallStatus | Promise + readonly removeAsync?: () => Promise + readonly getStatus: () => AgentHookInstallStatus +} + +export const MANAGED_AGENT_INTEGRATIONS: readonly ManagedAgentIntegration[] = [ + { + agent: 'claude', + install: (options) => claudeHookService.install({ claudeVersion: options?.cliVersion }), + refreshManagedScripts: () => claudeHookService.refreshManagedScripts(), + remove: () => claudeHookService.remove(), + getStatus: () => claudeHookService.getStatus() + }, + { + agent: 'openclaude', + install: () => openClaudeHookService.install(), + refreshManagedScripts: () => openClaudeHookService.refreshManagedScripts(), + remove: () => openClaudeHookService.remove(), + getStatus: () => openClaudeHookService.getStatus() + }, + { + agent: 'codex', + install: () => codexHookService.install(), + refreshManagedScripts: () => codexHookService.refreshManagedScripts(), + remove: () => codexHookService.remove(), + getStatus: () => codexHookService.getStatus() + }, + { + agent: 'gemini', + install: () => geminiHookService.install(), + refreshManagedScripts: () => geminiHookService.refreshManagedScripts(), + remove: () => geminiHookService.remove(), + getStatus: () => geminiHookService.getStatus() + }, + { + agent: 'antigravity', + install: () => antigravityHookService.install(), + refreshManagedScripts: () => antigravityHookService.refreshManagedScripts(), + remove: () => antigravityHookService.remove(), + getStatus: () => antigravityHookService.getStatus() + }, + { + agent: 'amp', + install: () => ampHookService.install(), + remove: () => ampHookService.remove(), + getStatus: () => ampHookService.getStatus() + }, + { + agent: 'cursor', + install: () => cursorHookService.install(), + refreshManagedScripts: () => cursorHookService.refreshManagedScripts(), + remove: () => cursorHookService.remove(), + getStatus: () => cursorHookService.getStatus() + }, + { + agent: 'droid', + install: () => droidHookService.install(), + refreshManagedScripts: () => droidHookService.refreshManagedScripts(), + remove: () => droidHookService.remove(), + getStatus: () => droidHookService.getStatus() + }, + { + agent: 'command-code', + install: () => commandCodeHookService.install(), + refreshManagedScripts: () => commandCodeHookService.refreshManagedScripts(), + remove: () => commandCodeHookService.remove(), + getStatus: () => commandCodeHookService.getStatus() + }, + { + agent: 'grok', + install: (options) => grokHookService.install(options), + refreshManagedScripts: () => grokHookService.refreshManagedScripts(), + remove: () => grokHookService.remove(), + removeAsync: () => grokHookService.removeAsync(), + getStatus: () => grokHookService.getStatus() + }, + { + agent: 'copilot', + install: () => copilotHookService.install(), + refreshManagedScripts: () => copilotHookService.refreshManagedScripts(), + remove: () => copilotHookService.remove(), + getStatus: () => copilotHookService.getStatus() + }, + { + agent: 'hermes', + install: () => hermesHookService.install(), + remove: () => hermesHookService.remove(), + getStatus: () => hermesHookService.getStatus() + }, + { + agent: 'devin', + install: () => devinHookService.install(), + refreshManagedScripts: () => devinHookService.refreshManagedScripts(), + remove: () => devinHookService.remove(), + getStatus: () => devinHookService.getStatus() + }, + { + agent: 'kimi', + install: () => kimiHookService.install(), + refreshManagedScripts: () => kimiHookService.refreshManagedScripts(), + remove: () => kimiHookService.remove(), + getStatus: () => kimiHookService.getStatus() + } ] +// Compatibility projections for the existing IPC and remote installer tests. +// They are derived from the descriptor and therefore cannot acquire a vendor +// independently of the lifecycle entry above. +export const MANAGED_AGENT_HOOK_INSTALLERS: readonly ManagedAgentHookInstaller[] = + MANAGED_AGENT_INTEGRATIONS.map((integration) => [integration.agent, integration.install] as const) + // Why: covers the shared launcher/statusline scripts under ~/.orca/agent-hooks — the files a // user-wide agent config keeps invoking after the CLI falls off PATH. Amp and Hermes write // provider-native plugin code into their own config dirs with their own install lifecycles, // not shared launchers, so they are deliberately absent. Enforced by the coverage test in // managed-hook-script-refresh.test.ts: a new installer that writes a launcher without adding // a refresher here fails that test. -export const MANAGED_AGENT_HOOK_SCRIPT_REFRESHERS: readonly ManagedAgentHookScriptRefresher[] = [ - ['claude', () => claudeHookService.refreshManagedScripts()], - ['openclaude', () => openClaudeHookService.refreshManagedScripts()], - ['codex', () => codexHookService.refreshManagedScripts()], - ['gemini', () => geminiHookService.refreshManagedScripts()], - ['antigravity', () => antigravityHookService.refreshManagedScripts()], - ['cursor', () => cursorHookService.refreshManagedScripts()], - ['droid', () => droidHookService.refreshManagedScripts()], - ['command-code', () => commandCodeHookService.refreshManagedScripts()], - ['grok', () => grokHookService.refreshManagedScripts()], - ['copilot', () => copilotHookService.refreshManagedScripts()], - ['devin', () => devinHookService.refreshManagedScripts()], - ['kimi', () => kimiHookService.refreshManagedScripts()] -] +export const MANAGED_AGENT_HOOK_SCRIPT_REFRESHERS: readonly ManagedAgentHookScriptRefresher[] = + MANAGED_AGENT_INTEGRATIONS.flatMap((integration) => + integration.refreshManagedScripts + ? ([[integration.agent, integration.refreshManagedScripts]] as const) + : [] + ) -export const MANAGED_AGENT_HOOK_REMOVERS: readonly ManagedAgentHookRemover[] = [ - ['claude', () => claudeHookService.remove()], - ['openclaude', () => openClaudeHookService.remove()], - ['codex', () => codexHookService.remove()], - ['gemini', () => geminiHookService.remove()], - ['antigravity', () => antigravityHookService.remove()], - ['amp', () => ampHookService.remove()], - ['cursor', () => cursorHookService.remove()], - ['droid', () => droidHookService.remove()], - ['command-code', () => commandCodeHookService.remove()], - ['grok', () => grokHookService.remove()], - ['copilot', () => copilotHookService.remove()], - ['hermes', () => hermesHookService.remove()], - ['devin', () => devinHookService.remove()], - ['kimi', () => kimiHookService.remove()] -] +export const MANAGED_AGENT_HOOK_REMOVERS: readonly ManagedAgentHookRemover[] = + MANAGED_AGENT_INTEGRATIONS.map((integration) => [integration.agent, integration.remove] as const) -export const MANAGED_AGENT_HOOK_ASYNC_REMOVERS: readonly ManagedAgentHookAsyncRemover[] = [ - ['grok', () => grokHookService.removeAsync()] -] +export const MANAGED_AGENT_HOOK_ASYNC_REMOVERS: readonly ManagedAgentHookAsyncRemover[] = + MANAGED_AGENT_INTEGRATIONS.flatMap((integration) => + integration.removeAsync ? ([[integration.agent, integration.removeAsync]] as const) : [] + ) -export const MANAGED_AGENT_HOOK_STATUS_READERS: readonly ManagedAgentHookStatusReader[] = [ - ['claude', () => claudeHookService.getStatus()], - ['openclaude', () => openClaudeHookService.getStatus()], - ['codex', () => codexHookService.getStatus()], - ['gemini', () => geminiHookService.getStatus()], - ['antigravity', () => antigravityHookService.getStatus()], - ['amp', () => ampHookService.getStatus()], - ['cursor', () => cursorHookService.getStatus()], - ['droid', () => droidHookService.getStatus()], - ['grok', () => grokHookService.getStatus()], - ['command-code', () => commandCodeHookService.getStatus()], - ['copilot', () => copilotHookService.getStatus()], - ['hermes', () => hermesHookService.getStatus()], - ['devin', () => devinHookService.getStatus()], - ['kimi', () => kimiHookService.getStatus()] -] +export const MANAGED_AGENT_HOOK_STATUS_READERS: readonly ManagedAgentHookStatusReader[] = + MANAGED_AGENT_INTEGRATIONS.map( + (integration) => [integration.agent, integration.getStatus] as const + ) diff --git a/src/main/agent-hooks/managed-hook-stdin-lifecycle.test.ts b/src/main/agent-hooks/managed-hook-stdin-lifecycle.test.ts index cd6c336e751..97f59dbbfa3 100644 --- a/src/main/agent-hooks/managed-hook-stdin-lifecycle.test.ts +++ b/src/main/agent-hooks/managed-hook-stdin-lifecycle.test.ts @@ -66,6 +66,7 @@ import { wrapPosixHookCommand, wrapWindowsHookCommand } from './installer-utils' import { POSIX_HOOK_JSON_STDIN_PRELUDE, POSIX_HOOK_JSON_STDIN_READER, + POSIX_HOOK_BOUNDED_JSON_STDIN_READER, POSIX_HOOK_STDIN_READER, WINDOWS_POWERSHELL_HOOK_ENVIRONMENT_GUARD } from './hook-stdin-contract' @@ -565,14 +566,18 @@ describe.skipIf(process.platform === 'win32')('managed hook stdin lifecycle', () for (const [agent, script] of scripts) { const captureIndex = Math.max( script.indexOf(`payload=$(${POSIX_HOOK_STDIN_READER})`), - script.indexOf(`payload=$(${POSIX_HOOK_JSON_STDIN_READER})`) + script.indexOf(`payload=$(${POSIX_HOOK_JSON_STDIN_READER})`), + script.indexOf(`payload=$(${POSIX_HOOK_BOUNDED_JSON_STDIN_READER})`) ) const firstExitIndex = script.indexOf('exit 0') expect(captureIndex, `${agent} payload capture`).toBeGreaterThanOrEqual(0) expect(firstExitIndex, `${agent} first success exit`).toBeGreaterThan(captureIndex) // Why: the JSON reader dereferences a variable the prelude sets, so a script // that carries the reader must carry its prelude above the capture line. - if (script.includes(POSIX_HOOK_JSON_STDIN_READER)) { + if ( + script.includes(POSIX_HOOK_JSON_STDIN_READER) || + script.includes(POSIX_HOOK_BOUNDED_JSON_STDIN_READER) + ) { const prelude = POSIX_HOOK_JSON_STDIN_PRELUDE.join('\n') expect(script.indexOf(prelude), `${agent} JSON reader prelude`).toBeGreaterThanOrEqual(0) expect(script.indexOf(prelude), `${agent} prelude before capture`).toBeLessThan( diff --git a/src/main/agent-hooks/server-hook-http-ingest.test.ts b/src/main/agent-hooks/server-hook-http-ingest.test.ts index f89dcebd005..2ab49635252 100644 --- a/src/main/agent-hooks/server-hook-http-ingest.test.ts +++ b/src/main/agent-hooks/server-hook-http-ingest.test.ts @@ -2,6 +2,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { AgentHookServer, _internals } from './server' import { AGENT_STATUS_MAX_FIELD_LENGTH } from '../../shared/agent-status-types' import { makePaneKey } from '../../shared/stable-pane-id' +import { HOOK_REQUEST_MAX_BYTES } from '../../shared/agent-hook-listener/request-body' import { buildBody, PANE, LEAF_2, LEAF_3 } from './server.test-fixtures' const { getCohortAtEmitMock, trackMock } = vi.hoisted(() => ({ @@ -44,6 +45,30 @@ async function postClaudeHook( } describe('AgentHookServer listener replay', () => { + it('classifies oversized authenticated hook bodies with 413', async () => { + const server = new AgentHookServer() + await server.start({ env: 'production' }) + try { + const env = server.buildPtyEnv() + const response = await fetch(`http://127.0.0.1:${env.ORCA_AGENT_HOOK_PORT}/hook/claude`, { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + 'X-Orca-Agent-Hook-Token': env.ORCA_AGENT_HOOK_TOKEN + }, + body: JSON.stringify({ value: 'x'.repeat(HOOK_REQUEST_MAX_BYTES + 1) }) + }) + + expect(response.status).toBe(413) + await expect(response.json()).resolves.toEqual({ + error: 'hook_request_too_large', + maxBytes: HOOK_REQUEST_MAX_BYTES + }) + } finally { + server.stop() + } + }) + it('accepts raw JSON hook bodies with base64 metadata headers', async () => { const server = new AgentHookServer() await server.start({ env: 'production' }) diff --git a/src/main/agent-hooks/server/server-lifecycle.ts b/src/main/agent-hooks/server/server-lifecycle.ts index e7f68829f3b..4babd5133c5 100644 --- a/src/main/agent-hooks/server/server-lifecycle.ts +++ b/src/main/agent-hooks/server/server-lifecycle.ts @@ -6,7 +6,11 @@ import { parseClaudeStatusLineBody } from '../../../shared/claude-statusline-rate-limits' import { mergeAgentHookRequestHeaders } from '../../../shared/agent-hook-listener/hook-envelope' -import { readRequestBody } from '../../../shared/agent-hook-listener/request-body' +import { + isAgentHookRequestTooLargeError, + readRequestBody, + respondWithAgentHookRequestTooLarge +} from '../../../shared/agent-hook-listener/request-body' import { resolveHookSource } from '../../../shared/agent-hook-listener/source-routing' import { HOOK_REQUEST_SLOWLORIS_MS } from '../../../shared/agent-hook-listener/listener-limits' import { isHookRequestTruncatedError } from '../../../shared/agent-hook-transport-interference' @@ -122,6 +126,13 @@ export abstract class AgentHookServerLifecycle extends AgentHookServerRuntimeEnv res.writeHead(204) res.end() } catch (error) { + if (isAgentHookRequestTooLargeError(error)) { + // Return an explicit bounded-transport classification while keeping + // the hook fail-open for the agent. Destroy only after the response + // is flushed so callers can observe 413 instead of ECONNRESET. + respondWithAgentHookRequestTooLarge(res, req) + return + } // Why (#11217): an authenticated POST whose body dies short of its own Content-Length was cut // by something on the loopback path, not by a bad payload. Fail open as before, but count it — // this is the one failure mode that silently stops status for every runtime at once. diff --git a/src/main/codex/codex-hook-script.test.ts b/src/main/codex/codex-hook-script.test.ts new file mode 100644 index 00000000000..66dfba40142 --- /dev/null +++ b/src/main/codex/codex-hook-script.test.ts @@ -0,0 +1,60 @@ +import { chmodSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { spawn } from 'node:child_process' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { describe, expect, it } from 'vitest' +import { getManagedScript } from './codex-hook-script' + +describe('Codex managed hook runner', () => { + it('uses a bounded reader and emits neutral JSON when Orca is unavailable', () => { + const script = getManagedScript('posix') + expect(script).toContain('orca_hook_json_stdin_py') + expect(script).toContain('python3') + expect(script).not.toContain('|| { command -p cat') + expect(script).toContain("printf '{}\\n'") + }) + + it('returns after a complete payload even when the caller keeps stdin open', async () => { + const dir = mkdtempSync(join(tmpdir(), 'orca-codex-hook-script-')) + const scriptPath = join(dir, 'codex-hook.sh') + writeFileSync(scriptPath, getManagedScript('posix'), 'utf8') + chmodSync(scriptPath, 0o755) + + try { + const result = await new Promise<{ status: number | null; stdout: string }>( + (resolve, reject) => { + const child = spawn('/bin/sh', [scriptPath], { + env: Object.fromEntries( + Object.entries(process.env).filter(([key]) => !key.startsWith('ORCA_')) + ), + stdio: ['pipe', 'pipe', 'ignore'] + }) + let stdout = '' + child.stdout.setEncoding('utf8') + child.stdout.on('data', (chunk: string) => { + stdout += chunk + }) + const timer = setTimeout(() => { + child.kill('SIGKILL') + reject(new Error('Codex hook did not return with an open stdin')) + }, 4_000) + child.on('error', (error) => { + clearTimeout(timer) + reject(error) + }) + child.on('close', (status) => { + clearTimeout(timer) + resolve({ status, stdout }) + }) + child.stdin.write('{"hook_event_name":"Stop","cwd":"/tmp"}') + // Intentionally leave stdin open: Codex's bridge keeps the pipe alive. + } + ) + + expect(result.status).toBe(0) + expect(result.stdout).toBe('{}\n') + } finally { + rmSync(dir, { recursive: true, force: true }) + } + }, 8_000) +}) diff --git a/src/main/codex/codex-hook-script.ts b/src/main/codex/codex-hook-script.ts index 7bbf6f7bab7..9abe66bb9df 100644 --- a/src/main/codex/codex-hook-script.ts +++ b/src/main/codex/codex-hook-script.ts @@ -3,7 +3,8 @@ import { buildPosixHookPayloadCapture, buildPosixHookSpoolLines, buildWindowsHookEnvironmentGuardLines, - buildWindowsHookStdinDrainEpilogue + buildWindowsHookStdinDrainEpilogue, + POSIX_HOOK_BOUNDED_JSON_STDIN } from '../agent-hooks/hook-stdin-contract' import { buildWindowsAgentHookCurlPostCommand } from '../agent-hooks/installer-utils' @@ -14,6 +15,7 @@ export function getManagedScript(target: 'local' | 'posix' = 'local'): string { 'setlocal', // Why: the endpoint file holds this install's live port/token; sourcing it lets a surviving PTY reach the current server (see claude/hook-service.ts). 'if defined ORCA_AGENT_HOOK_ENDPOINT if exist "%ORCA_AGENT_HOOK_ENDPOINT%" call "%ORCA_AGENT_HOOK_ENDPOINT%" 2>nul', + 'echo {}', ...buildWindowsHookEnvironmentGuardLines(), buildWindowsAgentHookCurlPostCommand('codex'), 'exit /b 0', @@ -24,7 +26,7 @@ export function getManagedScript(target: 'local' | 'posix' = 'local'): string { return [ '#!/bin/sh', - ...buildPosixHookPayloadCapture(), + ...buildPosixHookPayloadCapture('empty-object', POSIX_HOOK_BOUNDED_JSON_STDIN), ...buildPosixHookSpoolLines('codex'), // Why: sourcing refreshes PORT/TOKEN/ENV/VERSION from the current Orca so a surviving PTY keeps reporting after a restart (see claude/hook-service.ts). 'load_hook_endpoint() {', @@ -55,6 +57,7 @@ export function getManagedScript(target: 'local' | 'posix' = 'local'): string { 'fi', 'if [ -z "$ORCA_AGENT_HOOK_PORT" ] || [ -z "$ORCA_AGENT_HOOK_TOKEN" ] || [ -z "$ORCA_PANE_KEY" ]; then', ' spool_hook_event', + " printf '{}\\n'", ' exit 0', 'fi', 'post_codex_hook() {', @@ -72,18 +75,21 @@ export function getManagedScript(target: 'local' | 'posix' = 'local'): string { ' grep -qiE "microsoft|wsl" /proc/sys/kernel/osrelease /proc/version 2>/dev/null', '}', 'if post_codex_hook curl >/dev/null 2>&1; then', + " printf '{}\\n'", ' exit 0', 'fi', 'if is_wsl_runtime; then', ' windows_curl=$(command -v curl.exe 2>/dev/null || true)', ' if [ -n "$windows_curl" ] && [ -x "$windows_curl" ]; then', ' if post_codex_hook "$windows_curl" 3 5 >/dev/null 2>&1; then', + " printf '{}\\n'", ' exit 0', ' fi', ' # post_codex_hook "$windows_curl" 3 5 >/dev/null 2>&1 || true', ' fi', 'fi', 'spool_hook_event', + "printf '{}\\n'", 'exit 0', '' ].join('\n') diff --git a/src/main/hermes/hermes-config-yaml.ts b/src/main/hermes/hermes-config-yaml.ts index 9641359c3dd..012a3465b0d 100644 --- a/src/main/hermes/hermes-config-yaml.ts +++ b/src/main/hermes/hermes-config-yaml.ts @@ -1,10 +1,12 @@ -import { parse, stringify } from 'yaml' +import { parse, parseDocument, stringify, YAMLMap } from 'yaml' import { HERMES_PLUGIN_NAME } from './hermes-managed-plugin-source' export type HermesConfig = Record -export type ConfigParseResult = { ok: true; config: HermesConfig } | { ok: false; detail: string } +export type ConfigParseResult = + | { ok: true; config: HermesConfig; source?: string } + | { ok: false; detail: string } function isRecord(value: unknown): value is Record { return typeof value === 'object' && value !== null && !Array.isArray(value) @@ -41,7 +43,49 @@ export function parseHermesConfig(content: string | null): ConfigParseResult { } } -export function serializeHermesConfig(config: HermesConfig): string { +export function serializeHermesConfig(config: HermesConfig, source?: string): string { + if (source !== undefined && source.trim().length > 0) { + try { + const document = parseDocument(source) + if (document.errors.length === 0) { + const original = document.toJS() + if (isRecord(original)) { + for (const [key, value] of Object.entries(config)) { + // Mutate an existing mapping in place so comments attached to + // nested pairs survive the managed update. + if (key === 'plugins' && isRecord(value)) { + const pluginsNode = document.get(key, true) + if (pluginsNode instanceof YAMLMap) { + for (const [pluginKey, pluginValue] of Object.entries(value)) { + pluginsNode.set(pluginKey, pluginValue) + } + const pluginKeysToRemove = pluginsNode.items + .map((pair) => pair.key) + .filter( + (pairKey): pairKey is string => + typeof pairKey === 'string' && !(pairKey in value) + ) + for (const pairKey of pluginKeysToRemove) { + pluginsNode.delete(pairKey) + } + continue + } + } + document.set(key, value) + } + for (const key of Object.keys(original)) { + if (!(key in config)) { + document.delete(key) + } + } + return document.toString() + } + } + } catch { + // Fall back to a canonical serialization below. Parsing already + // succeeded for the normal write path, so this is defensive only. + } + } return `${stringify(config, { lineWidth: 0 }).trimEnd()}\n` } @@ -85,7 +129,7 @@ export function updateConfigContent( if (!parsed.ok) { return { content: null, detail: parsed.detail } } - return { content: serializeHermesConfig(updater(parsed.config)) } + return { content: serializeHermesConfig(updater(parsed.config), content ?? undefined) } } export function getConfigEnablement(config: HermesConfig): { diff --git a/src/main/hermes/hermes-home-filesystem.ts b/src/main/hermes/hermes-home-filesystem.ts index 7cd7a48a863..e9e6ff3ccfe 100644 --- a/src/main/hermes/hermes-home-filesystem.ts +++ b/src/main/hermes/hermes-home-filesystem.ts @@ -25,12 +25,47 @@ export function getHermesHome(env: NodeJS.ProcessEnv = process.env): string { return explicit ? explicit : join(homedir(), '.hermes') } -export function getConfigPath(): string { - return join(getHermesHome(), 'config.yaml') +function isSafeProfileName(value: string): boolean { + return /^[A-Za-z0-9][A-Za-z0-9._-]{0,127}$/.test(value) } -export function getPluginDir(): string { - return join(getHermesHome(), 'plugins', HERMES_PLUGIN_NAME) +function profileFromCommand(command: string | undefined): string | undefined { + if (!command) { + return undefined + } + const match = command.match(/(?:^|\s)(?:--profile|-p)(?:=|\s+)(?:"([^"]+)"|'([^']+)'|([^\s]+))/) + const profile = match?.[1] ?? match?.[2] ?? match?.[3] + return profile && isSafeProfileName(profile) ? profile : undefined +} + +function activeHermesProfile(home: string): string | undefined { + try { + const profile = readFileSync(join(home, 'active_profile'), 'utf8').trim() + return profile && isSafeProfileName(profile) ? profile : undefined + } catch { + return undefined + } +} + +/** Resolve the profile Hermes will actually read for one launch. */ +export function resolveHermesHomeForLaunch( + env: NodeJS.ProcessEnv = process.env, + launchCommand?: string +): string { + const root = getHermesHome(env) + const profile = profileFromCommand(launchCommand) ?? activeHermesProfile(root) + if (!profile || profile === 'default') { + return root + } + return join(root, 'profiles', profile) +} + +export function getConfigPath(home = getHermesHome()): string { + return join(home, 'config.yaml') +} + +export function getPluginDir(home = getHermesHome()): string { + return join(home, 'plugins', HERMES_PLUGIN_NAME) } function getManifestPath(pluginDir = getPluginDir()): string { @@ -45,13 +80,15 @@ export function readConfigFile(configPath: string): ConfigParseResult { if (!existsSync(configPath)) { return { ok: true, config: {} } } - return parseHermesConfig(readFileSync(configPath, 'utf-8')) + const source = readFileSync(configPath, 'utf-8') + const parsed = parseHermesConfig(source) + return parsed.ok ? { ...parsed, source } : parsed } -export function writeConfigFile(configPath: string, config: HermesConfig): void { +export function writeConfigFile(configPath: string, config: HermesConfig, source?: string): void { const dir = dirname(configPath) mkdirSync(dir, { recursive: true }) - const serialized = serializeHermesConfig(config) + const serialized = serializeHermesConfig(config, source) if (existsSync(configPath)) { try { if (readFileSync(configPath, 'utf-8') === serialized) { diff --git a/src/main/hermes/hook-service.test.ts b/src/main/hermes/hook-service.test.ts index 4180caf616e..db9a99932d3 100644 --- a/src/main/hermes/hook-service.test.ts +++ b/src/main/hermes/hook-service.test.ts @@ -1,6 +1,6 @@ import { createServer } from 'node:http' import { execFile, execFileSync, spawnSync } from 'node:child_process' -import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' +import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' import { tmpdir } from 'node:os' import { join } from 'node:path' import { afterEach, beforeEach, describe, expect, it } from 'vitest' @@ -8,6 +8,7 @@ import { parse } from 'yaml' import { makePaneKey } from '../../shared/stable-pane-id' import { HermesHookService, _internals } from './hook-service' +import { resolveHermesHomeForLaunch } from './hermes-home-filesystem' const PANE_KEY = makePaneKey('tab-1', '11111111-1111-4111-8111-111111111111') @@ -30,6 +31,47 @@ describe('HermesHookService', () => { rmSync(homeDir, { recursive: true, force: true }) }) + it.each([ + ['--profile coder', 'coder'], + ['-p coder', 'coder'], + ['--profile=coder', 'coder'], + ['hermes --continue --profile "review-team"', 'review-team'] + ])('resolves an explicit Hermes profile from the launch command (%s)', (command, profile) => { + expect(resolveHermesHomeForLaunch({ HERMES_HOME: homeDir }, command)).toBe( + join(homeDir, 'profiles', profile) + ) + }) + + it('uses the active profile when a launch has no explicit profile', () => { + writeFileSync(join(homeDir, 'active_profile'), 'coder\n', 'utf8') + expect(resolveHermesHomeForLaunch({ HERMES_HOME: homeDir }, 'hermes --tui')).toBe( + join(homeDir, 'profiles', 'coder') + ) + }) + + it('installs into the selected profile and preserves user comments', () => { + const profileHome = join(homeDir, 'profiles', 'coder') + const configPath = join(profileHome, 'config.yaml') + mkdirSync(profileHome, { recursive: true }) + writeFileSync( + configPath, + '# keep this profile comment\nmodel: test-model\nplugins:\n # keep plugin notes\n enabled: []\n', + 'utf8' + ) + + const status = new HermesHookService().install({ + env: { HERMES_HOME: homeDir }, + launchCommand: 'hermes --profile coder --tui' + }) + + expect(status.state).toBe('installed') + const updated = readFileSync(configPath, 'utf8') + expect(updated).toContain('# keep this profile comment') + expect(updated).toContain('# keep plugin notes') + expect(updated).toContain(_internals.HERMES_PLUGIN_NAME) + expect(existsSync(join(homeDir, 'config.yaml'))).toBe(false) + }) + it('installs the managed Hermes plugin and enables it in config.yaml', () => { const status = new HermesHookService().install() diff --git a/src/main/hermes/hook-service.ts b/src/main/hermes/hook-service.ts index 6b1b141f721..3020b7c9afd 100644 --- a/src/main/hermes/hook-service.ts +++ b/src/main/hermes/hook-service.ts @@ -23,6 +23,7 @@ import { writeConfigFile, writePluginFiles } from './hermes-home-filesystem' +import { resolveHermesHomeForLaunch } from './hermes-home-filesystem' import { HERMES_EVENTS, HERMES_PLUGIN_NAME, @@ -30,8 +31,12 @@ import { getPluginManifest } from './hermes-managed-plugin-source' -function buildStatus(configPath: string, config: HermesConfig): AgentHookInstallStatus { - const pluginFiles = getPluginFilesState() +function buildStatus( + configPath: string, + config: HermesConfig, + home: string +): AgentHookInstallStatus { + const pluginFiles = getPluginFilesState(getPluginDir(home)) const enablement = getConfigEnablement(config) const details = [ pluginFiles.detail, @@ -68,37 +73,39 @@ function stripTrailingSlash(path: string): string { } export class HermesHookService { - getStatus(): AgentHookInstallStatus { - const configPath = getConfigPath() + getStatus(options?: { env?: NodeJS.ProcessEnv; launchCommand?: string }): AgentHookInstallStatus { + const home = resolveHermesHomeForLaunch(options?.env, options?.launchCommand) + const configPath = getConfigPath(home) const parsed = readConfigFile(configPath) if (!parsed.ok) { return { agent: 'hermes', state: 'error', configPath, - managedHooksPresent: getPluginFilesState().managed, + managedHooksPresent: getPluginFilesState(getPluginDir(home)).managed, detail: `Could not parse Hermes config.yaml: ${parsed.detail}` } } - return buildStatus(configPath, parsed.config) + return buildStatus(configPath, parsed.config, home) } - install(): AgentHookInstallStatus { - const configPath = getConfigPath() + install(options?: { env?: NodeJS.ProcessEnv; launchCommand?: string }): AgentHookInstallStatus { + const home = resolveHermesHomeForLaunch(options?.env, options?.launchCommand) + const configPath = getConfigPath(home) const parsed = readConfigFile(configPath) if (!parsed.ok) { return { agent: 'hermes', state: 'error', configPath, - managedHooksPresent: getPluginFilesState().managed, + managedHooksPresent: getPluginFilesState(getPluginDir(home)).managed, detail: `Could not parse Hermes config.yaml: ${parsed.detail}` } } - writePluginFiles() - writeConfigFile(configPath, enablePlugin(parsed.config)) - return this.getStatus() + writePluginFiles(getPluginDir(home)) + writeConfigFile(configPath, enablePlugin(parsed.config), parsed.source) + return this.getStatus(options) } async installRemote(sftp: SFTPWrapper, remoteHome: string): Promise { @@ -138,24 +145,25 @@ export class HermesHookService { } } - remove(): AgentHookInstallStatus { - const configPath = getConfigPath() + remove(options?: { env?: NodeJS.ProcessEnv; launchCommand?: string }): AgentHookInstallStatus { + const home = resolveHermesHomeForLaunch(options?.env, options?.launchCommand) + const configPath = getConfigPath(home) const parsed = readConfigFile(configPath) if (!parsed.ok) { return { agent: 'hermes', state: 'error', configPath, - managedHooksPresent: getPluginFilesState().managed, + managedHooksPresent: getPluginFilesState(getPluginDir(home)).managed, detail: `Could not parse Hermes config.yaml: ${parsed.detail}` } } - const pluginDir = getPluginDir() + const pluginDir = getPluginDir(home) if (getPluginFilesState(pluginDir).managed) { rmSync(pluginDir, { recursive: true, force: true }) } - writeConfigFile(configPath, disablePlugin(parsed.config)) - return this.getStatus() + writeConfigFile(configPath, disablePlugin(parsed.config), parsed.source) + return this.getStatus(options) } } diff --git a/src/main/ipc/pty/host-env/assembly.ts b/src/main/ipc/pty/host-env/assembly.ts index 3c65796b250..fe90bab2269 100644 --- a/src/main/ipc/pty/host-env/assembly.ts +++ b/src/main/ipc/pty/host-env/assembly.ts @@ -19,6 +19,7 @@ import { stripInheritedOrcaCodexHomeOverride } from './codex-home' import { clearPiAgentShadowEnv, exposePiManagedExtensionEnv, + inheritOmpXdgEnvironment, isMimoLaunchCommand, resolveMimocodeSourceHome, resolveOpenCodeSourceConfigDir, @@ -54,6 +55,14 @@ export function buildPtyHostEnv( const hasLaunchCommand = typeof launchCommandHint === 'string' && launchCommandHint.trim().length > 0 + if (piAgentKind === 'omp' || !hasLaunchCommand) { + // OMP uses XDG data/state/cache roots for daemon-owned fragments. Shell + // startup exports are not present in a direct daemon spawn, so carry the + // effective values into this PTY rather than silently falling back to + // ~/.omp. + inheritOmpXdgEnvironment(baseEnv) + } + // Why: unattended agents must fail instead of looping on OS credential prompts; user terminals keep normal Git behavior. applyTerminalGitCredentialPromptGuard(baseEnv, { launchCommand: launchCommandHint, @@ -66,7 +75,7 @@ export function buildPtyHostEnv( const preexistingPiAgentDir = resolvePiAgentSourceDir(baseEnv, 'pi') const preexistingOmpAgentDir = piAgentKind === 'omp' - ? resolvePiAgentSourceDir(baseEnv, 'omp') + ? resolvePiAgentSourceDir(baseEnv, 'omp', launchCommandHint) : resolveScopedPiAgentSourceDir(baseEnv, 'omp') const preexistingPrimeAgentDir = piAgentKind === 'prime-agent' diff --git a/src/main/ipc/pty/host-env/pi-agent-scope.test.ts b/src/main/ipc/pty/host-env/pi-agent-scope.test.ts new file mode 100644 index 00000000000..838573c1d31 --- /dev/null +++ b/src/main/ipc/pty/host-env/pi-agent-scope.test.ts @@ -0,0 +1,69 @@ +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { __resetShellStartupEnvCache } from '../../../pty/shell-startup-env' +import { inheritOmpXdgEnvironment, resolvePiAgentSourceDir } from './pi-agent' + +describe('OMP launch scope', () => { + let homeDir: string + + beforeEach(() => { + homeDir = mkdtempSync(join(tmpdir(), 'orca-omp-scope-')) + __resetShellStartupEnvCache() + }) + + afterEach(() => { + rmSync(homeDir, { recursive: true, force: true }) + __resetShellStartupEnvCache() + }) + + it('resolves the configured OMP profile before PI_CODING_AGENT_DIR exists', () => { + const configDir = join(homeDir, 'omp-config') + expect( + resolvePiAgentSourceDir( + { HOME: homeDir, PI_CONFIG_DIR: configDir }, + 'omp', + 'omp --profile review-team' + ) + ).toBe(join(configDir, 'profiles', 'review-team', 'agent')) + }) + + it('keeps an explicit non-Orca PI_CODING_AGENT_DIR for the default profile', () => { + expect( + resolvePiAgentSourceDir( + { + HOME: homeDir, + PI_CONFIG_DIR: join(homeDir, 'config'), + PI_CODING_AGENT_DIR: join(homeDir, 'custom-agent') + }, + 'omp', + 'omp' + ) + ).toBe(join(homeDir, 'custom-agent')) + }) + + it('copies XDG data roots exported by the launching shell', () => { + const xdgDataHome = join(homeDir, 'xdg-data') + const xdgStateHome = join(homeDir, 'xdg-state') + const xdgCacheHome = join(homeDir, 'xdg-cache') + mkdirSync(xdgDataHome) + writeFileSync( + join(homeDir, '.zshrc'), + [ + `export XDG_DATA_HOME="$HOME/${xdgDataHome.slice(homeDir.length + 1)}"`, + `export XDG_STATE_HOME="$HOME/${xdgStateHome.slice(homeDir.length + 1)}"`, + `export XDG_CACHE_HOME="$HOME/${xdgCacheHome.slice(homeDir.length + 1)}"` + ].join('\n') + ) + + const env: Record = { HOME: homeDir, SHELL: '/bin/zsh' } + inheritOmpXdgEnvironment(env) + + expect(env).toMatchObject({ + XDG_DATA_HOME: xdgDataHome, + XDG_STATE_HOME: xdgStateHome, + XDG_CACHE_HOME: xdgCacheHome + }) + }) +}) diff --git a/src/main/ipc/pty/host-env/pi-agent.ts b/src/main/ipc/pty/host-env/pi-agent.ts index 545d04d063d..cdc90990401 100644 --- a/src/main/ipc/pty/host-env/pi-agent.ts +++ b/src/main/ipc/pty/host-env/pi-agent.ts @@ -7,6 +7,8 @@ import { SOURCE_AGENT_DIR_ENV_BY_KIND, type PiAgentKind } from '../../../../shared/pi-agent-kind' +import { join } from 'node:path' +import { homedir } from 'node:os' import { readSessionShellStartupEnvVar } from '../../../pty/shell-startup-env' import { AGENT_HOOK_RUNTIME_ENV_KEYS, CLAUDE_CHILD_SESSION_STAMP_ENV_KEYS } from './spawn-env-keys' @@ -19,11 +21,30 @@ export function readEnvWithProcessFallback( export function resolvePiAgentSourceDir( baseEnv: Record, - kind: PiAgentKind + kind: PiAgentKind, + launchCommand?: string ): string | undefined { const sourceKey = SOURCE_AGENT_DIR_ENV_BY_KIND[kind] const primaryKey = PRIMARY_AGENT_DIR_ENV_BY_KIND[kind] + const ompProfile = + kind === 'omp' + ? [ + readOmpProfileFromCommand(launchCommand), + readEnvWithProcessFallback(baseEnv, 'OMP_PROFILE'), + readEnvWithProcessFallback(baseEnv, 'PI_PROFILE') + ].find((candidate) => candidate !== undefined && isSafeOmpProfile(candidate)) + : undefined + + if (kind === 'omp' && ompProfile) { + const configuredRoot = + readEnvWithProcessFallback(baseEnv, 'PI_CONFIG_DIR') ?? + readSessionShellStartupEnvVar('PI_CONFIG_DIR', baseEnv) + const configDir = + configuredRoot ?? join(readEnvWithProcessFallback(baseEnv, 'HOME') ?? homedir(), '.omp') + return join(configDir, 'profiles', ompProfile, 'agent') + } + const sourceDir = readEnvWithProcessFallback(baseEnv, sourceKey) if (sourceDir) { return sourceDir @@ -47,9 +68,47 @@ export function resolvePiAgentSourceDir( return publicDir } + // OMP keeps its configurable root in PI_CONFIG_DIR; PI_CODING_AGENT_DIR is + // only populated after OMP has booted. Resolve the root before launch so the + // managed extension is materialized in the same profile the binary will use. + if (kind === 'omp') { + const configuredRoot = + readEnvWithProcessFallback(baseEnv, 'PI_CONFIG_DIR') ?? + readSessionShellStartupEnvVar('PI_CONFIG_DIR', baseEnv) + if (configuredRoot) { + return join(configuredRoot, 'agent') + } + if (launchCommand?.trim()) { + return join(readEnvWithProcessFallback(baseEnv, 'HOME') ?? homedir(), '.omp', 'agent') + } + } + return readSessionShellStartupEnvVar(primaryKey, baseEnv) } +function readOmpProfileFromCommand(command: string | undefined): string | undefined { + const match = command?.match(/(?:^|\s)--profile(?:=|\s+)(?:"([^"]+)"|'([^']+)'|([^\s]+))/) + const profile = match?.[1] ?? match?.[2] ?? match?.[3] + return profile && isSafeOmpProfile(profile) ? profile : undefined +} + +function isSafeOmpProfile(value: string): boolean { + return /^[A-Za-z0-9][A-Za-z0-9._-]{0,63}$/.test(value) +} + +/** Copy XDG roots discovered from the user's actual shell into daemon spawns. */ +export function inheritOmpXdgEnvironment(baseEnv: Record): void { + for (const name of ['XDG_DATA_HOME', 'XDG_STATE_HOME', 'XDG_CACHE_HOME'] as const) { + if (baseEnv[name] !== undefined) { + continue + } + const value = readSessionShellStartupEnvVar(name, baseEnv) ?? process.env[name] + if (value) { + baseEnv[name] = value + } + } +} + export function resolveScopedPiAgentSourceDir( baseEnv: Record, kind: PiAgentKind diff --git a/src/relay/agent-hook-server.test.ts b/src/relay/agent-hook-server.test.ts index c3a99b8a20c..25342f9b1ef 100644 --- a/src/relay/agent-hook-server.test.ts +++ b/src/relay/agent-hook-server.test.ts @@ -349,6 +349,32 @@ describe('RelayAgentHookServer', () => { } }) + it('classifies oversized authenticated hook bodies with 413', async () => { + const forward = vi.fn() + const server = new RelayAgentHookServer({ endpointDir: dir, forward }) + await server.start() + try { + const { port, token } = server.getCoordinates() + const response = await fetch(`http://127.0.0.1:${port}/hook/claude`, { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + 'X-Orca-Agent-Hook-Token': token + }, + body: JSON.stringify({ value: 'x'.repeat(HOOK_REQUEST_MAX_BYTES + 1) }) + }) + + expect(response.status).toBe(413) + await expect(response.json()).resolves.toEqual({ + error: 'hook_request_too_large', + maxBytes: HOOK_REQUEST_MAX_BYTES + }) + expect(forward).not.toHaveBeenCalled() + } finally { + server.stop() + } + }) + it('replays cached payloads on demand', async () => { const forward = vi.fn<(envelope: AgentHookRelayEnvelope) => void>() const server = new RelayAgentHookServer({ endpointDir: dir, forward }) diff --git a/src/relay/agent-hook-server.ts b/src/relay/agent-hook-server.ts index 378f81cf022..e97f19e1a09 100644 --- a/src/relay/agent-hook-server.ts +++ b/src/relay/agent-hook-server.ts @@ -20,7 +20,11 @@ import { import { HOOK_REQUEST_SLOWLORIS_MS } from '../shared/agent-hook-listener/listener-limits' import { normalizeHookPayload } from '../shared/agent-hook-listener' import { mergeAgentHookRequestHeaders } from '../shared/agent-hook-listener/hook-envelope' -import { readRequestBody } from '../shared/agent-hook-listener/request-body' +import { + isAgentHookRequestTooLargeError, + readRequestBody, + respondWithAgentHookRequestTooLarge +} from '../shared/agent-hook-listener/request-body' import { resolveHookSource } from '../shared/agent-hook-listener/source-routing' import type { AgentHookEventPayload } from '../shared/agent-hook-listener/listener-event' import { @@ -29,16 +33,12 @@ import { isHookRequestTruncatedError } from '../shared/agent-hook-transport-interference' import { - isAgentHookSource, REMOTE_AGENT_HOOK_ENV, type AgentHookRelayEnvelope, type AgentHookSource } from '../shared/agent-hook-relay' -import { - buildSpoolHookBody, - drainAgentHookSpool, - type SpoolRecord -} from '../shared/agent-hook-spool' +import { drainAgentHookSpool } from '../shared/agent-hook-spool' +import { ingestRelayAgentHookSpoolRecord } from './agent-hook-spool-ingest' import { buildRelayHookPtyEnv, defaultEndpointDir } from './agent-hook-endpoint-coordinates' import { buildRelayHookEnvelope, hookBodyEnv, hookBodyVersion } from './agent-hook-envelope-build' import { AgentHookResultRetryScheduler } from './agent-hook-result-retry-scheduler' @@ -123,7 +123,14 @@ export class RelayAgentHookServer { drainAgentHookSpool({ endpointDir: this.endpointDir, getPersistedLaunchTokenHash: () => undefined, - ingest: (record) => this.ingestSpoolRecord(record) + ingest: (record) => + ingestRelayAgentHookSpoolRecord( + record, + this.state, + this.env, + (event, source, env, version, options) => + this.applyEvent(event, source, env, version, options) + ) }) } catch (err) { // Why: a downstream relay failure must not prevent the loopback listener from starting; @@ -289,6 +296,10 @@ export class RelayAgentHookServer { res.writeHead(204) res.end() } catch (err) { + if (isAgentHookRequestTooLargeError(err)) { + respondWithAgentHookRequestTooLarge(res, req) + return + } // Why (#11217): a remote host can run the same IDS; count truncations here so a blocked SSH // relay reports the cause instead of an anonymous "hook request failed". if (isHookRequestTruncatedError(err) && !destroyedBySlowlorisCap) { @@ -335,20 +346,4 @@ export class RelayAgentHookServer { this.lastEnvelopeMetaByPaneKey.set(event.paneKey, { source, env, version }) this.forward(buildRelayHookEnvelope(event, source, env, version, options)) } - - private ingestSpoolRecord(record: SpoolRecord): void { - if (!isAgentHookSource(record.source)) { - return - } - const body = buildSpoolHookBody(record) - const event = normalizeHookPayload(this.state, record.source, body, this.env, { - deferCompactOwnershipToClient: true - }) - if (!event) { - return - } - this.applyEvent(event, record.source, hookBodyEnv(body), hookBodyVersion(body), { - isReplay: true - }) - } } diff --git a/src/relay/agent-hook-spool-ingest.ts b/src/relay/agent-hook-spool-ingest.ts new file mode 100644 index 00000000000..7f6d526c24b --- /dev/null +++ b/src/relay/agent-hook-spool-ingest.ts @@ -0,0 +1,33 @@ +import type { AgentHookEventPayload } from '../shared/agent-hook-listener/listener-event' +import { isAgentHookSource, type AgentHookSource } from '../shared/agent-hook-relay' +import { buildSpoolHookBody, type SpoolRecord } from '../shared/agent-hook-spool' +import type { HookListenerState } from '../shared/agent-hook-listener/listener-state' +import { normalizeHookPayload } from '../shared/agent-hook-listener' +import { hookBodyEnv, hookBodyVersion } from './agent-hook-envelope-build' + +type ApplyRelayEvent = ( + event: AgentHookEventPayload, + source: AgentHookSource, + env?: string, + version?: string, + options?: { isReplay?: boolean } +) => void + +export function ingestRelayAgentHookSpoolRecord( + record: SpoolRecord, + state: HookListenerState, + env: string, + applyEvent: ApplyRelayEvent +): void { + if (!isAgentHookSource(record.source)) { + return + } + const body = buildSpoolHookBody(record) + const event = normalizeHookPayload(state, record.source, body, env, { + deferCompactOwnershipToClient: true + }) + if (!event) { + return + } + applyEvent(event, record.source, hookBodyEnv(body), hookBodyVersion(body), { isReplay: true }) +} diff --git a/src/relay/plugin-overlay-env.test.ts b/src/relay/plugin-overlay-env.test.ts index 7746307cf47..a7cd1bd9a27 100644 --- a/src/relay/plugin-overlay-env.test.ts +++ b/src/relay/plugin-overlay-env.test.ts @@ -3,7 +3,11 @@ import { tmpdir } from 'node:os' import { join } from 'node:path' import { afterEach, beforeEach, describe, expect, it } from 'vitest' import { __resetShellStartupEnvCache } from '../main/pty/shell-startup-env' -import { resolveOpenCodeSourceConfigDir, resolvePiSourceAgentDir } from './plugin-overlay-env' +import { + inheritOmpXdgEnvironment, + resolveOpenCodeSourceConfigDir, + resolvePiSourceAgentDir +} from './plugin-overlay-env' describe('plugin overlay env source resolution', () => { let homeDir: string @@ -44,6 +48,28 @@ describe('plugin overlay env source resolution', () => { } ) + it.skipIf(process.platform === 'win32')( + 'resolves OMP profile and XDG roots from the remote shell', + () => { + const configDir = join(homeDir, 'omp-config') + const dataDir = join(homeDir, 'xdg-data') + mkdirSync(dataDir, { recursive: true }) + writeFileSync( + join(homeDir, '.zshrc'), + [ + `export PI_CONFIG_DIR="$HOME/${configDir.slice(homeDir.length + 1)}"`, + `export XDG_DATA_HOME="$HOME/${dataDir.slice(homeDir.length + 1)}"` + ].join('\n') + ) + + const env: Record = { HOME: homeDir, SHELL: '/bin/zsh' } + expect(resolvePiSourceAgentDir(env, '/bin/zsh', 'omp', 'omp --profile review')).toBe( + join(configDir, 'profiles', 'review', 'agent') + ) + expect(inheritOmpXdgEnvironment(env, '/bin/zsh')).toEqual({ XDG_DATA_HOME: dataDir }) + } + ) + it.skipIf(process.platform === 'win32')( 'discovers overlay sources from a custom zsh ZDOTDIR', () => { diff --git a/src/relay/plugin-overlay-env.ts b/src/relay/plugin-overlay-env.ts index 1db43f4717e..529e6749e14 100644 --- a/src/relay/plugin-overlay-env.ts +++ b/src/relay/plugin-overlay-env.ts @@ -1,4 +1,6 @@ import { readSessionShellStartupEnvVar } from '../main/pty/shell-startup-env' +import { join } from 'node:path' +import { homedir } from 'node:os' import { PRIMARY_AGENT_DIR_ENV_BY_KIND, SOURCE_AGENT_DIR_ENV_BY_KIND, @@ -33,11 +35,28 @@ export function resolveOpenCodeSourceConfigDir( export function resolvePiSourceAgentDir( env: Record, shell: string | undefined, - kind: PiAgentKind + kind: PiAgentKind, + launchCommand?: string ): string | undefined { const sourceKey = SOURCE_AGENT_DIR_ENV_BY_KIND[kind] const primaryKey = PRIMARY_AGENT_DIR_ENV_BY_KIND[kind] + const ompProfile = + kind === 'omp' + ? [readOmpProfileFromCommand(launchCommand), env.OMP_PROFILE, env.PI_PROFILE].find( + (candidate) => candidate !== undefined && isSafeOmpProfile(candidate) + ) + : undefined + + if (kind === 'omp' && ompProfile) { + const configuredRoot = firstNonEmpty( + env.PI_CONFIG_DIR, + readStartupEnv('PI_CONFIG_DIR', env, shell) + ) + const configDir = configuredRoot ?? join(env.HOME ?? process.env.HOME ?? homedir(), '.omp') + return join(configDir, 'profiles', ompProfile, 'agent') + } + const sourceDir = firstNonEmpty(env[sourceKey]) if (sourceDir) { return sourceDir @@ -65,5 +84,49 @@ export function resolvePiSourceAgentDir( ) { return env[primaryKey] } + + // OMP resolves its agent directory from PI_CONFIG_DIR before it populates + // PI_CODING_AGENT_DIR. Resolve the launch profile up front so the relay + // materializes the extension where the remote OMP process will load it. + if (kind === 'omp') { + const configuredRoot = firstNonEmpty( + env.PI_CONFIG_DIR, + readStartupEnv('PI_CONFIG_DIR', env, shell) + ) + if (configuredRoot) { + return join(configuredRoot, 'agent') + } + if (launchCommand?.trim()) { + return join(env.HOME ?? process.env.HOME ?? homedir(), '.omp', 'agent') + } + } return undefined } + +function readOmpProfileFromCommand(command: string | undefined): string | undefined { + const match = command?.match(/(?:^|\s)--profile(?:=|\s+)(?:"([^"]+)"|'([^']+)'|([^\s]+))/) + const profile = match?.[1] ?? match?.[2] ?? match?.[3] + return profile && isSafeOmpProfile(profile) ? profile : undefined +} + +function isSafeOmpProfile(value: string): boolean { + return /^[A-Za-z0-9][A-Za-z0-9._-]{0,63}$/.test(value) +} + +/** Carry shell-selected XDG roots into relay-spawned daemon children. */ +export function inheritOmpXdgEnvironment( + env: Record, + shell: string | undefined +): Record { + const next: Record = {} + for (const name of ['XDG_DATA_HOME', 'XDG_STATE_HOME', 'XDG_CACHE_HOME'] as const) { + if (env[name] !== undefined) { + continue + } + const value = readStartupEnv(name, env, shell) ?? process.env[name] + if (value) { + next[name] = value + } + } + return next +} diff --git a/src/relay/relay-agent-hook-runtime.ts b/src/relay/relay-agent-hook-runtime.ts index 0ca7111ae6b..7515d88e393 100644 --- a/src/relay/relay-agent-hook-runtime.ts +++ b/src/relay/relay-agent-hook-runtime.ts @@ -9,7 +9,11 @@ import { } from '../shared/agent-hook-relay' import { publishAgentHookEnvelope } from './agent-hook-envelope-publication' import { assertPluginSourceUnderByteCap } from './plugin-source-limit' -import { resolveOpenCodeSourceConfigDir, resolvePiSourceAgentDir } from './plugin-overlay-env' +import { + inheritOmpXdgEnvironment, + resolveOpenCodeSourceConfigDir, + resolvePiSourceAgentDir +} from './plugin-overlay-env' import { detectExplicitPiAgentKindFromCommand, isPiCompatibleAgentType @@ -110,9 +114,10 @@ export class RelayAgentHookRuntime { } } if (kind === 'omp' || !hasLaunchCommand) { + Object.assign(env, inheritOmpXdgEnvironment(context.env, context.shell)) const sourceDir = kind === 'omp' - ? resolvePiSourceAgentDir(context.env, context.shell, 'omp') + ? resolvePiSourceAgentDir(context.env, context.shell, 'omp', launchCommandHint) : context.env.ORCA_OMP_SOURCE_AGENT_DIR const result = this.pluginOverlay.materializePi(overlayId, sourceDir, 'omp', { materializeDefaultHome: explicitKind === 'omp' diff --git a/src/shared/agent-hook-listener-transport.test.ts b/src/shared/agent-hook-listener-transport.test.ts index f0a86986414..b686fcf5fc1 100644 --- a/src/shared/agent-hook-listener-transport.test.ts +++ b/src/shared/agent-hook-listener-transport.test.ts @@ -23,12 +23,14 @@ import { clearGrokSessionPathLookupCacheForTests } from './grok-session-paths' type FakeIncomingMessage = EventEmitter & { headers: IncomingHttpHeaders destroy: ReturnType + pause: ReturnType } function createReadableRequest(headers: IncomingHttpHeaders = {}): FakeIncomingMessage { const req = new EventEmitter() as FakeIncomingMessage req.headers = headers req.destroy = vi.fn(() => req.emit('close')) + req.pause = vi.fn() return req } @@ -144,7 +146,7 @@ describe('shared agent-hook-listener', () => { req.emit('data', Buffer.alloc(HOOK_REQUEST_MAX_BYTES + 1)) await expect(body).rejects.toThrow('payload too large') - expect(req.destroy).toHaveBeenCalledTimes(1) + expect(req.pause).toHaveBeenCalledTimes(1) expectRequestParserListenersReleased(req) }) diff --git a/src/shared/agent-hook-listener/request-body.ts b/src/shared/agent-hook-listener/request-body.ts index b453c7c1f7e..c26579abeb6 100644 --- a/src/shared/agent-hook-listener/request-body.ts +++ b/src/shared/agent-hook-listener/request-body.ts @@ -1,4 +1,4 @@ -import type { IncomingMessage } from 'node:http' +import type { IncomingMessage, ServerResponse } from 'node:http' import { classifyTruncatedHookRequest } from '../agent-hook-transport-interference' import { assertJsonTextStructureWithinLimits } from '../json-text-structure-limit' @@ -11,6 +11,35 @@ const AGENT_HOOK_JSON_STRUCTURE_LIMITS = { nestingDepth: 64 } as const +/** The peer sent more than the listener can safely retain. */ +export class AgentHookRequestTooLargeError extends Error { + readonly code = 'HOOK_REQUEST_TOO_LARGE' + readonly maxBytes = HOOK_REQUEST_MAX_BYTES + + constructor(readonly receivedBytes: number) { + super(`payload too large (${receivedBytes} bytes; maximum ${HOOK_REQUEST_MAX_BYTES})`) + this.name = 'AgentHookRequestTooLargeError' + } +} + +export function isAgentHookRequestTooLargeError( + error: unknown +): error is AgentHookRequestTooLargeError { + return error instanceof AgentHookRequestTooLargeError +} + +/** Send the bounded transport classification before closing the unread request. */ +export function respondWithAgentHookRequestTooLarge( + res: ServerResponse, + req: IncomingMessage +): void { + res.writeHead(413, { 'content-type': 'application/json' }) + res.end( + JSON.stringify({ error: 'hook_request_too_large', maxBytes: HOOK_REQUEST_MAX_BYTES }), + () => req.destroy() + ) +} + export function parseAgentHookJson(content: string): unknown { // Why: Cursor on Windows writes UTF-8-with-BOM to the hook's stdin and `JSON.parse` rejects U+FEFF, // so the whole event was dropped. Strip exactly one leading BOM — not a trim — to keep every other @@ -63,8 +92,16 @@ export function readRequestBody(req: IncomingMessage): Promise { // Why: bound by bytes (not UTF-16 units) and stop accumulating after rejection so a client can't push memory past the cap. const nextByteLength = byteLength + chunk.length if (nextByteLength > HOOK_REQUEST_MAX_BYTES) { - settleReject(new Error('payload too large')) - req.destroy() + settleReject(new AgentHookRequestTooLargeError(nextByteLength)) + // Pause first so the caller can send a bounded 413 response. The + // response completion callback destroys the unread request/socket. + // The fallback keeps test doubles and unusual stream implementations + // from retaining an unbounded body when pause is unavailable. + if (typeof req.pause === 'function') { + req.pause() + } else { + req.destroy() + } return } if (retained.length < nextByteLength) {