diff --git a/src/main/agent-hooks/server.test.ts b/src/main/agent-hooks/server.test.ts index 704acb9a8..d12c73754 100644 --- a/src/main/agent-hooks/server.test.ts +++ b/src/main/agent-hooks/server.test.ts @@ -1,6 +1,16 @@ /* eslint-disable max-lines -- Why: this suite exercises the full hook HTTP surface (Claude/Codex/Gemini parsing, transcript chunked scan, paneKey dispatch) and keeping the scenarios co-located avoids fixture drift across files. */ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import { mkdtempSync, rmSync, writeFileSync } from 'fs' +import { execFileSync } from 'child_process' +import { + existsSync, + mkdirSync, + mkdtempSync, + readFileSync, + rmSync, + statSync, + utimesSync, + writeFileSync +} from 'fs' import { tmpdir } from 'os' import { join } from 'path' import { AgentHookServer, _internals } from './server' @@ -829,3 +839,202 @@ describe('Cursor hook normalization', () => { expect(result).toBeNull() }) }) + +describe('Endpoint file lifecycle', () => { + let userDataPath: string + + beforeEach(() => { + userDataPath = mkdtempSync(join(tmpdir(), 'orca-endpoint-')) + }) + + afterEach(() => { + rmSync(userDataPath, { recursive: true, force: true }) + }) + + it('writes the endpoint file with the expected shell-sourceable shape', async () => { + const server = new AgentHookServer() + await server.start({ env: 'development', userDataPath }) + try { + const filePath = server.endpointFilePath + expect(filePath).toBeTruthy() + expect(existsSync(filePath!)).toBe(true) + const contents = readFileSync(filePath!, 'utf8') + const expectedPort = server.buildPtyEnv().ORCA_AGENT_HOOK_PORT + const expectedToken = server.buildPtyEnv().ORCA_AGENT_HOOK_TOKEN + const prefix = process.platform === 'win32' ? 'set ' : '' + expect(contents).toContain(`${prefix}ORCA_AGENT_HOOK_PORT=${expectedPort}`) + expect(contents).toContain(`${prefix}ORCA_AGENT_HOOK_TOKEN=${expectedToken}`) + expect(contents).toContain(`${prefix}ORCA_AGENT_HOOK_ENV=development`) + expect(contents).toContain(`${prefix}ORCA_AGENT_HOOK_VERSION=1`) + } finally { + server.stop() + } + }) + + it('writes the endpoint file with owner-only permissions on POSIX', async () => { + if (process.platform === 'win32') { + return + } + const server = new AgentHookServer() + await server.start({ env: 'production', userDataPath }) + try { + const filePath = server.endpointFilePath! + // Why: mask off type/setuid bits so we assert only the rwx octet that + // writeFileSync(mode:0o600) sets. A leaky umask at dir-create time can + // leave group/other bits on the *parent* dir but not on the file itself. + const mode = statSync(filePath).mode & 0o777 + expect(mode).toBe(0o600) + } finally { + server.stop() + } + }) + + it('rewrites the endpoint file with a new port after restart on the same path', async () => { + const server = new AgentHookServer() + await server.start({ env: 'production', userDataPath }) + const firstPath = server.endpointFilePath + const firstToken = server.buildPtyEnv().ORCA_AGENT_HOOK_TOKEN + server.stop() + + await server.start({ env: 'production', userDataPath }) + try { + const secondPath = server.endpointFilePath + const secondPort = server.buildPtyEnv().ORCA_AGENT_HOOK_PORT + const secondToken = server.buildPtyEnv().ORCA_AGENT_HOOK_TOKEN + // Path is stable (so PTYs stamped before restart can still find the file) + expect(secondPath).toBe(firstPath) + // But contents are refreshed with the new token (and port) — that is the + // whole point of the design: survivors reading a stale-env file reach the + // live server. Why token-first: the token is randomUUID()-minted per + // start(), so it is guaranteed to differ across restarts. The port comes + // from listen(0) and the kernel can legitimately reassign the same + // ephemeral port, so asserting port-inequality would be a latent flake. + expect(secondToken).toBeTruthy() + expect(secondToken).not.toBe(firstToken) + const contents = readFileSync(secondPath!, 'utf8') + // Why: token-based content check is the rewrite signal. A strict + // "contents does NOT contain firstPort" assertion would flake on the + // (rare but legitimate) case where listen(0) reuses the same ephemeral + // port across restarts. The token is randomUUID() and cannot collide. + expect(contents).toContain(`ORCA_AGENT_HOOK_PORT=${secondPort}`) + expect(contents).toContain(`ORCA_AGENT_HOOK_TOKEN=${secondToken}`) + expect(contents).not.toContain(`ORCA_AGENT_HOOK_TOKEN=${firstToken}`) + } finally { + server.stop() + } + }) + + it('leaves the endpoint file in place on stop()', async () => { + // Why: stop() deliberately does NOT unlink the endpoint file. A stale file + // points at a dead port — the fail-open path (hook POSTs silently fail, + // same as pre-endpoint-file). Unlinking would introduce a TOCTOU race with a + // concurrent Orca instance sharing userData that could rewrite the file + // between our token check and unlink. The next successful start() + // overwrites the file atomically; tmp-file orphan hygiene is handled by + // the sweep inside writeEndpointFile(). + const server = new AgentHookServer() + await server.start({ env: 'production', userDataPath }) + const filePath = server.endpointFilePath! + expect(existsSync(filePath)).toBe(true) + server.stop() + expect(existsSync(filePath)).toBe(true) + }) + + it('buildPtyEnv includes ORCA_AGENT_HOOK_ENDPOINT when the server is running', async () => { + const server = new AgentHookServer() + await server.start({ env: 'production', userDataPath }) + try { + const env = server.buildPtyEnv() + expect(env.ORCA_AGENT_HOOK_ENDPOINT).toBe(server.endpointFilePath) + } finally { + server.stop() + } + }) + + it('buildPtyEnv omits ORCA_AGENT_HOOK_ENDPOINT when no userDataPath was provided', async () => { + // Why: the endpoint file is opt-in via start({ userDataPath }). In tests + // and in the packaged main-process path where userData is unset for any + // reason, hooks should fall back to the v1 behavior (no ENDPOINT key). + const server = new AgentHookServer() + await server.start({ env: 'production' }) + try { + const env = server.buildPtyEnv() + expect(env.ORCA_AGENT_HOOK_ENDPOINT).toBeUndefined() + expect(env.ORCA_AGENT_HOOK_PORT).toBeTruthy() + expect(env.ORCA_AGENT_HOOK_TOKEN).toBeTruthy() + } finally { + server.stop() + } + }) + + it('buildPtyEnv returns empty when the server is not running', () => { + const server = new AgentHookServer() + expect(server.buildPtyEnv()).toEqual({}) + }) + + it('sweeps stale .endpoint-*.tmp orphans older than 5 minutes on start', async () => { + // Why: writeEndpointFile() writes to a unique tmp path then renames. A crash + // between write and rename leaves an orphan tmp; the sweep inside + // writeEndpointFile() must drop ones older than 5 min without touching + // fresh ones (a concurrent writer's in-flight tmp). + const dir = join(userDataPath, 'agent-hooks') + mkdirSync(dir, { recursive: true }) + const staleTmp = join(dir, '.endpoint-999-stale.tmp') + const freshTmp = join(dir, '.endpoint-999-fresh.tmp') + writeFileSync(staleTmp, 'stale') + writeFileSync(freshTmp, 'fresh') + const sixMinAgo = (Date.now() - 6 * 60 * 1000) / 1000 + utimesSync(staleTmp, sixMinAgo, sixMinAgo) + + const server = new AgentHookServer() + await server.start({ env: 'production', userDataPath }) + try { + expect(existsSync(staleTmp)).toBe(false) + expect(existsSync(freshTmp)).toBe(true) + } finally { + server.stop() + } + }) + + it('refuses to write the endpoint file when a value contains shell metacharacters', async () => { + // Why: every value written is sourced as shell. The isShellSafeEndpointValue + // allowlist must reject a metacharacter-bearing value so a future caller + // cannot command-inject via the sourced file. `env` is the only caller- + // provided field we can easily poison from a test — feed it a semicolon + // and assert the file is not written and buildPtyEnv() omits the ENDPOINT + // key (gated on endpointFileWritten). + const server = new AgentHookServer() + await server.start({ env: 'bad;value', userDataPath }) + try { + expect(existsSync(server.endpointFilePath!)).toBe(false) + expect(server.buildPtyEnv().ORCA_AGENT_HOOK_ENDPOINT).toBeUndefined() + // PORT/TOKEN still flow via PTY env — fail-open to v1 behavior. + expect(server.buildPtyEnv().ORCA_AGENT_HOOK_PORT).toBeTruthy() + expect(server.buildPtyEnv().ORCA_AGENT_HOOK_TOKEN).toBeTruthy() + } finally { + server.stop() + } + }) + + it('endpoint file contents are re-parseable by /bin/sh', async () => { + if (process.platform === 'win32') { + return + } + const server = new AgentHookServer() + await server.start({ env: 'production', userDataPath }) + try { + const filePath = server.endpointFilePath! + const expectedPort = server.buildPtyEnv().ORCA_AGENT_HOOK_PORT + // Why: sources the file in a subshell and echoes the resulting env var, + // exactly as the managed hook script does at runtime. If the file shape + // ever drifts from `KEY=VALUE` (e.g. someone adds shell metacharacters + // without quoting), this test catches it before users do. + const out = execFileSync('/bin/sh', ['-c', `. "${filePath}" && echo "$ORCA_AGENT_HOOK_PORT"`]) + .toString() + .trim() + expect(out).toBe(expectedPort) + } finally { + server.stop() + } + }) +}) diff --git a/src/main/agent-hooks/server.ts b/src/main/agent-hooks/server.ts index 5a63f701f..aee9c83e7 100644 --- a/src/main/agent-hooks/server.ts +++ b/src/main/agent-hooks/server.ts @@ -1,7 +1,19 @@ /* eslint-disable max-lines -- Why: the hook server owns the full HTTP ingest surface (routing, body parsing, per-CLI normalization, transcript scan, pane dispatch) in one place so the contract with Claude/Codex/Gemini hooks stays consistent and doesn't drift across files. */ import { createServer, type IncomingMessage, type ServerResponse } from 'http' import { randomUUID } from 'crypto' -import { closeSync, openSync, readSync, statSync } from 'fs' +import { + chmodSync, + closeSync, + mkdirSync, + openSync, + readdirSync, + readSync, + renameSync, + statSync, + unlinkSync, + writeFileSync +} from 'fs' +import { join } from 'path' import { parseAgentStatusPayload, type ParsedAgentStatusPayload @@ -1038,6 +1050,29 @@ function normalizeHookPayload( return payload ? { paneKey, tabId, worktreeId, payload } : null } +// Why: the endpoint file lives under userData so each Orca install (dev vs. +// packaged) has its own path and the two cannot clobber each other. Using a +// per-platform extension (`.env` on POSIX, `.cmd` on Windows) lets the hook +// scripts source the file with their platform-native syntax (`.` on POSIX, +// `call` on Windows); the OpenCode plugin's regex accepts both shapes so no +// platform detection is needed inside the plugin source either. +function getEndpointFileName(): string { + return process.platform === 'win32' ? 'endpoint.cmd' : 'endpoint.env' +} + +// Why: every value in the endpoint file is sourced as shell. Reject any +// value that contains shell/cmd metacharacters so a future field whose +// value is not shell-safe-by-construction cannot command-inject via the +// sourced file. Keep to a conservative allowlist of common printable +// chars plus hyphen/dot/slash/colon/underscore — sufficient for ports, +// UUIDs, version strings, and env names. +// Rejects empty values (`+` quantifier) as defense-in-depth for future +// callers — an empty sourced `KEY=` would silently clear the env var in +// the sourcing shell, masking whatever legitimate value was previously set. +function isShellSafeEndpointValue(value: string): boolean { + return /^[A-Za-z0-9._:/-]+$/.test(value) +} + export class AgentHookServer { private server: ReturnType | null = null private port = 0 @@ -1047,6 +1082,21 @@ export class AgentHookServer { // caller's knowledge of whether this is a packaged build. private env = 'production' private onAgentStatus: ((payload: AgentHookEventPayload) => void) | null = null + // Why: directory that holds the on-disk endpoint file. Set via start()'s + // `userDataPath` option so the class has no direct Electron dependency + // (keeps it mockable in the vitest node environment). When unset, we skip + // the endpoint-file write entirely — hooks still work via PTY env, just + // without survive-a-restart semantics. + private endpointDir: string | null = null + private endpointFilePathCache: string | null = null + // Why: tracks whether writeEndpointFile() succeeded for the *current* + // start(). Without this flag, buildPtyEnv() would expose + // ORCA_AGENT_HOOK_ENDPOINT pointing at a path that may hold stale + // coordinates from a prior crashed instance — hook scripts would source + // those stale coords and silently post to a dead server. Gating the + // ENDPOINT env var on a successful write preserves the + // fail-open-to-fresh-env guarantee. + private endpointFileWritten = false setListener(listener: ((payload: AgentHookEventPayload) => void) | null): void { this.onAgentStatus = listener @@ -1064,7 +1114,7 @@ export class AgentHookServer { } } - async start(options?: { env?: string }): Promise { + async start(options?: { env?: string; userDataPath?: string }): Promise { if (this.server) { return } @@ -1072,7 +1122,12 @@ export class AgentHookServer { if (options?.env) { this.env = options.env } + if (options?.userDataPath) { + this.endpointDir = join(options.userDataPath, 'agent-hooks') + this.endpointFilePathCache = join(this.endpointDir, getEndpointFileName()) + } this.token = randomUUID() + this.endpointFileWritten = false this.server = createServer(async (req: IncomingMessage, res: ServerResponse) => { if (req.method !== 'POST') { res.writeHead(404) @@ -1153,6 +1208,11 @@ export class AgentHookServer { if (address && typeof address === 'object') { this.port = address.port } + // Why: the endpoint file is the core of the survives-Orca-restart + // design. Write it *after* we have a concrete port — hooks that source + // the file must see a usable coordinate set, not a stale one left over + // from a previous process (e.g. one that crashed before getting here). + this.writeEndpointFile() resolve() } this.server!.once('error', onStartupError) @@ -1167,6 +1227,17 @@ export class AgentHookServer { this.token = '' this.env = 'production' this.onAgentStatus = null + // Why: intentionally do NOT delete the endpoint file on stop(). A stale + // file points at a dead port, which matches the fail-open policy (hook + // POSTs silently fail → same as pre-endpoint-file behavior). Attempting to unlink + // introduces a TOCTOU race: a concurrent Orca instance sharing userData + // could rewrite the file between our token check and unlink, and we'd + // delete their live endpoint file. The next successful start() overwrites + // the file atomically; the tmp-file sweep inside writeEndpointFile() + // handles orphan hygiene. + this.endpointDir = null + this.endpointFilePathCache = null + this.endpointFileWritten = false // Why: drop all per-pane cache entries on shutdown so a subsequent start() // in the same process (e.g. during tests or a settings-driven restart) // does not inherit stale prompt/tool state from the previous run. @@ -1194,12 +1265,153 @@ export class AgentHookServer { return {} } - return { + // Why: ORCA_AGENT_HOOK_ENDPOINT is the key that lets a surviving PTY reach + // the *current* Orca after a restart. The other four variables are retained + // for back-compat so pre-endpoint-file hook scripts (which do not know to + // source the endpoint file) continue to work on freshly spawned PTYs, and + // so the current script can fall through to env if the file is + // missing/unreadable for any reason. + const env: Record = { ORCA_AGENT_HOOK_PORT: String(this.port), ORCA_AGENT_HOOK_TOKEN: this.token, ORCA_AGENT_HOOK_ENV: this.env, ORCA_AGENT_HOOK_VERSION: ORCA_HOOK_PROTOCOL_VERSION } + if (this.endpointFileWritten && this.endpointFilePathCache) { + env.ORCA_AGENT_HOOK_ENDPOINT = this.endpointFilePathCache + } + return env + } + + // Why: exposed as a read-only getter so tests (and any future main-process + // caller that needs the path for diagnostics) do not have to reconstruct + // the path convention. + get endpointFilePath(): string | null { + return this.endpointFilePathCache + } + + // Why: writes the four coordinates atomically via a tmp-then-rename so a + // hook reading concurrently either sees the old file or the new one, never + // a half-written one. Fail-open: on EACCES / ENOSPC / etc. we log and move + // on — start() remains usable via PTY env for freshly-spawned PTYs. Only + // survivors lose the endpoint-file path, matching the hook-payload + // fail-open policy already enforced on the receiving end. + private writeEndpointFile(): void { + if (!this.endpointDir || !this.endpointFilePathCache) { + return + } + // Why: defensive reset — buildPtyEnv() must not see a stale `true` from + // a previous start() if this write fails before reaching the success + // assignment below. + this.endpointFileWritten = false + const finalPath = this.endpointFilePathCache + // Why: unique-per-call tmp name (mirrors persistence.ts / installer-utils.ts); prevents cross-process collision if two writers race on the same endpoint dir. + const tmpPath = join(this.endpointDir, `.endpoint-${process.pid}-${randomUUID()}.tmp`) + const prefix = process.platform === 'win32' ? 'set ' : '' + // Why: every value written here is sourced as shell (`. "$file"` on + // POSIX, `call "%file%"` on Windows) — the file format IS shell, not + // key=value data. The current four inputs are shell-safe by + // construction: PORT is a number from listen(), TOKEN is randomUUID() + // output (hex + dashes only), VERSION is a compile-time string + // constant, and ENV is a fixed 'production' / 'development' literal + // passed from index.ts. Any future change that relaxes these + // invariants (user-supplied env name, persisted token, arbitrary + // free-form field) MUST add escaping or a safe-character validator + // before the write — otherwise a value like `foo&malicious` on Windows + // would command-inject via `call`, and a newline in any value would + // corrupt the POSIX sourceable output. The isShellSafeEndpointValue + // check below enforces this contract at runtime. + const valuesToWrite: [string, string][] = [ + ['ORCA_AGENT_HOOK_PORT', String(this.port)], + ['ORCA_AGENT_HOOK_TOKEN', this.token], + ['ORCA_AGENT_HOOK_ENV', this.env], + ['ORCA_AGENT_HOOK_VERSION', ORCA_HOOK_PROTOCOL_VERSION] + ] + for (const [key, value] of valuesToWrite) { + if (!isShellSafeEndpointValue(value)) { + console.error( + `[agent-hooks] refusing to write endpoint file: ${key} contains ` + + 'characters unsafe for shell sourcing. Falling back to PTY env.' + ) + return + } + } + const lines = [...valuesToWrite.map(([key, value]) => `${prefix}${key}=${value}`), ''] + let tmpWritten = false + try { + // Why: mode 0o700 — match the file's owner-only policy so the + // agent-hooks/ directory itself does not leak the existence of this + // Orca install (or the presence of the endpoint file) to other local + // users on a multi-user POSIX host. Default umask would otherwise + // leave the dir at 0o755 even though the file inside is 0o600. + mkdirSync(this.endpointDir, { recursive: true, mode: 0o700 }) + if (process.platform !== 'win32') { + // Why: mkdirSync's `mode` only applies when the dir is newly created — + // a pre-existing agent-hooks/ dir (from an earlier build or user + // intervention) keeps its original permissions. Re-chmod on every + // start() so the directory matches the 0600 file inside it. POSIX + // only; chmod semantics differ on Windows and the filesystem-level + // ACL model makes this check meaningless there. + try { + chmodSync(this.endpointDir, 0o700) + } catch { + // Why: best-effort — a chmod failure (exotic fs, read-only mount) + // must not block the endpoint-file write itself. + } + } + // Why: a crash between writeFileSync and renameSync leaves stale + // `.endpoint--.tmp` in this directory. Sweep older-than-5-min + // orphans so the dir does not grow unboundedly. Fresh tmps are left + // alone so a legitimate concurrent instance is not disturbed. + try { + const entries = readdirSync(this.endpointDir) + const cutoff = Date.now() - 5 * 60 * 1000 + for (const entry of entries) { + if (!entry.startsWith('.endpoint-') || !entry.endsWith('.tmp')) { + continue + } + const entryPath = join(this.endpointDir, entry) + try { + if (statSync(entryPath).mtimeMs < cutoff) { + unlinkSync(entryPath) + } + } catch { + // best-effort sweep + } + } + } catch { + // readdirSync can fail on exotic filesystems; never block the write + } + // Why: 0o600 — the token is a loopback bearer credential and must not + // be readable by other local users. Parity with PTY env exposure via + // /proc//environ (owner-only on modern Linux). + // Why: `.cmd` files require CRLF for consistent `set` parsing across + // Windows versions — LF-only terminators are silently mis-parsed by + // some cmd.exe versions, which would break hook coord refresh (exactly + // the bug this file exists to fix). POSIX stays LF. + const separator = process.platform === 'win32' ? '\r\n' : '\n' + writeFileSync(tmpPath, lines.join(separator), { mode: 0o600 }) + tmpWritten = true + renameSync(tmpPath, finalPath) + this.endpointFileWritten = true + } catch (err) { + console.error('[agent-hooks] failed to write endpoint file:', err) + // Why: clean up tmp; never nuke the prior finalPath when we cannot + // guarantee we have replaced it. Stale finalPath → dead port → silent + // fail on hook POST matches the fail-open policy documented on the + // receiver side. Destroying the prior file would strand surviving PTYs + // that *could* have continued to fail silently against a dead port — + // strictly worse than leaving the prior coords in place until the next + // successful start() overwrites them. + if (tmpWritten) { + try { + unlinkSync(tmpPath) + } catch { + // Why: tmp may already be gone (rename partially succeeded, or an + // external process cleaned it). Nothing to do. + } + } + } } } diff --git a/src/main/claude/hook-service.ts b/src/main/claude/hook-service.ts index 6d66aa5f0..587967ec6 100644 --- a/src/main/claude/hook-service.ts +++ b/src/main/claude/hook-service.ts @@ -56,6 +56,13 @@ function getManagedScript(): string { return [ '@echo off', 'setlocal', + // Why: the endpoint file holds the *live* port/token for this Orca + // install. A PTY that survived an Orca restart has stale PORT/TOKEN + // baked into its env from the old instance — loading `endpoint.cmd` + // (`set KEY=VALUE` lines) via `call` refreshes them so the hook + // reaches the current server. Falls through to PTY env if the file + // is missing (first run / pre-endpoint-file / running outside Orca). + 'if defined ORCA_AGENT_HOOK_ENDPOINT if exist "%ORCA_AGENT_HOOK_ENDPOINT%" call "%ORCA_AGENT_HOOK_ENDPOINT%" 2>nul', 'if "%ORCA_AGENT_HOOK_PORT%"=="" exit /b 0', 'if "%ORCA_AGENT_HOOK_TOKEN%"=="" exit /b 0', 'if "%ORCA_PANE_KEY%"=="" exit /b 0', @@ -67,6 +74,23 @@ function getManagedScript(): string { return [ '#!/bin/sh', + // Why: the endpoint file holds the *live* port/token for this Orca + // install. PTYs that survive an Orca restart have stale PORT/TOKEN + // baked into their env from the old instance — sourcing the file here + // lets us reach the new server. Falls back to PTY env if the file is + // missing (first-run / pre-endpoint-file scripts / running outside Orca). + // Why: suppress stderr on the `.` builtin. A TOCTOU race (endpoint unlinked + // between the `[ -r ]` test and the source) or a malformed line (e.g. CRLF + // bled in from a cross-platform userData copy) would otherwise print a + // parse error that agent transcripts could surface. Stale coords → dead + // port → silent-fail is the documented fail-open path anyway — the env-var + // guards below handle the empty PORT/TOKEN case — so swallowing the noise + // here is strictly better than leaking shell errors into the hook output. + // `|| :` defends against an eventual `set -e` in an outer script context + // (not present today) aborting the hook on a parse error. + 'if [ -n "$ORCA_AGENT_HOOK_ENDPOINT" ] && [ -r "$ORCA_AGENT_HOOK_ENDPOINT" ]; then', + ' . "$ORCA_AGENT_HOOK_ENDPOINT" 2>/dev/null || :', + 'fi', 'if [ -z "$ORCA_AGENT_HOOK_PORT" ] || [ -z "$ORCA_AGENT_HOOK_TOKEN" ] || [ -z "$ORCA_PANE_KEY" ]; then', ' exit 0', 'fi', diff --git a/src/main/codex/hook-service.ts b/src/main/codex/hook-service.ts index d476814c9..d57555c06 100644 --- a/src/main/codex/hook-service.ts +++ b/src/main/codex/hook-service.ts @@ -47,6 +47,11 @@ function getManagedScript(): string { return [ '@echo off', 'setlocal', + // Why: see claude/hook-service.ts for rationale. The endpoint file holds + // the live port/token for this Orca install; sourcing it here lets a + // surviving PTY reach the current server even though its env points at + // the prior Orca's coordinates. + 'if defined ORCA_AGENT_HOOK_ENDPOINT if exist "%ORCA_AGENT_HOOK_ENDPOINT%" call "%ORCA_AGENT_HOOK_ENDPOINT%" 2>nul', 'if "%ORCA_AGENT_HOOK_PORT%"=="" exit /b 0', 'if "%ORCA_AGENT_HOOK_TOKEN%"=="" exit /b 0', 'if "%ORCA_PANE_KEY%"=="" exit /b 0', @@ -58,6 +63,12 @@ function getManagedScript(): string { return [ '#!/bin/sh', + // Why: see claude/hook-service.ts for rationale. Sourcing refreshes + // PORT/TOKEN/ENV/VERSION from the current Orca so a surviving PTY keeps + // reporting after a restart. + 'if [ -n "$ORCA_AGENT_HOOK_ENDPOINT" ] && [ -r "$ORCA_AGENT_HOOK_ENDPOINT" ]; then', + ' . "$ORCA_AGENT_HOOK_ENDPOINT" 2>/dev/null || :', + 'fi', 'if [ -z "$ORCA_AGENT_HOOK_PORT" ] || [ -z "$ORCA_AGENT_HOOK_TOKEN" ] || [ -z "$ORCA_PANE_KEY" ]; then', ' exit 0', 'fi', diff --git a/src/main/cursor/hook-service.ts b/src/main/cursor/hook-service.ts index 93b46fa3b..ed7d479c7 100644 --- a/src/main/cursor/hook-service.ts +++ b/src/main/cursor/hook-service.ts @@ -59,6 +59,11 @@ function getManagedScript(): string { return [ '@echo off', 'setlocal', + // Why: see claude/hook-service.ts for rationale. The endpoint file holds + // the live port/token for this Orca install; sourcing it here lets a + // surviving PTY reach the current server even though its env points at + // the prior Orca's coordinates. + 'if defined ORCA_AGENT_HOOK_ENDPOINT if exist "%ORCA_AGENT_HOOK_ENDPOINT%" call "%ORCA_AGENT_HOOK_ENDPOINT%" 2>nul', 'if "%ORCA_AGENT_HOOK_PORT%"=="" exit /b 0', 'if "%ORCA_AGENT_HOOK_TOKEN%"=="" exit /b 0', 'if "%ORCA_PANE_KEY%"=="" exit /b 0', @@ -70,6 +75,12 @@ function getManagedScript(): string { return [ '#!/bin/sh', + // Why: see claude/hook-service.ts for rationale. Sourcing refreshes + // PORT/TOKEN/ENV/VERSION from the current Orca so a surviving PTY keeps + // reporting after a restart. + 'if [ -n "$ORCA_AGENT_HOOK_ENDPOINT" ] && [ -r "$ORCA_AGENT_HOOK_ENDPOINT" ]; then', + ' . "$ORCA_AGENT_HOOK_ENDPOINT" 2>/dev/null || :', + 'fi', 'if [ -z "$ORCA_AGENT_HOOK_PORT" ] || [ -z "$ORCA_AGENT_HOOK_TOKEN" ] || [ -z "$ORCA_PANE_KEY" ]; then', ' exit 0', 'fi', diff --git a/src/main/gemini/hook-service.ts b/src/main/gemini/hook-service.ts index d613080fc..4239460bd 100644 --- a/src/main/gemini/hook-service.ts +++ b/src/main/gemini/hook-service.ts @@ -49,6 +49,11 @@ function getManagedScript(): string { // to return. Emit `{}` first so the agent never stalls parsing our // output, even if the env-var guards below cause an early exit. 'echo {}', + // Why: see claude/hook-service.ts for rationale. The endpoint file holds + // the live port/token for this Orca install; sourcing it here lets a + // surviving PTY reach the current server even though its env points at + // the prior Orca's coordinates. + 'if defined ORCA_AGENT_HOOK_ENDPOINT if exist "%ORCA_AGENT_HOOK_ENDPOINT%" call "%ORCA_AGENT_HOOK_ENDPOINT%" 2>nul', 'if "%ORCA_AGENT_HOOK_PORT%"=="" exit /b 0', 'if "%ORCA_AGENT_HOOK_TOKEN%"=="" exit /b 0', 'if "%ORCA_PANE_KEY%"=="" exit /b 0', @@ -64,6 +69,12 @@ function getManagedScript(): string { // to return. Emit `{}` first so the agent never stalls parsing our output, // even if the env-var guards below cause an early exit. 'printf "{}\\n"', + // Why: see claude/hook-service.ts for rationale. Sourcing refreshes + // PORT/TOKEN/ENV/VERSION from the current Orca so a surviving PTY keeps + // reporting after a restart. + 'if [ -n "$ORCA_AGENT_HOOK_ENDPOINT" ] && [ -r "$ORCA_AGENT_HOOK_ENDPOINT" ]; then', + ' . "$ORCA_AGENT_HOOK_ENDPOINT" 2>/dev/null || :', + 'fi', 'if [ -z "$ORCA_AGENT_HOOK_PORT" ] || [ -z "$ORCA_AGENT_HOOK_TOKEN" ] || [ -z "$ORCA_PANE_KEY" ]; then', ' exit 0', 'fi', diff --git a/src/main/index.ts b/src/main/index.ts index f7a0dbd2e..6cbc363dc 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -424,14 +424,21 @@ app.whenReady().then(async () => { } setAppRuntimeFlags({ daemonEnabledAtStartup: daemonStarted }) - // Why: the hook server also runs unconditionally so cursor-agent panes can - // reach it. Claude/Codex/Gemini hook scripts stay uninstalled while + // Why: the hook server runs unconditionally so cursor-agent panes can reach + // it. Claude/Codex/Gemini hook scripts stay uninstalled while // AGENT_DASHBOARD_ENABLED is false, so only cursor events flow in. PTY // spawn env reads ORCA_AGENT_HOOK_* from the live server state, so the // server must start before the window opens — otherwise restored terminals // race ahead without the env on first launch. try { - await agentHookServer.start({ env: app.isPackaged ? 'production' : 'development' }) + await agentHookServer.start({ + env: app.isPackaged ? 'production' : 'development', + // Why: passing the userData path lets the server write its endpoint + // file (PORT/TOKEN/ENV/VERSION) to a stable location. Hook scripts + // source that file at invocation time so they reach the current Orca + // even when the PTY's env was frozen under a prior instance. + userDataPath: app.getPath('userData') + }) } catch (error) { // Why: Claude/Codex/Gemini/OpenCode/Cursor hook callbacks are sidebar // enrichment only. Orca must still boot even if the local loopback diff --git a/src/main/opencode/hook-service.test.ts b/src/main/opencode/hook-service.test.ts index 941a181a4..b1ed29622 100644 --- a/src/main/opencode/hook-service.test.ts +++ b/src/main/opencode/hook-service.test.ts @@ -34,6 +34,63 @@ describe('OpenCode hook plugin source', () => { expect(source).toContain('export const OrcaOpenCodeStatusPlugin = async (_ctx) => {') expect(source).toContain('const client = _ctx?.client;') }) + + it('resolves hook coords from the endpoint file before falling back to process.env', () => { + // Why: a long-running OpenCode session was fork()ed with the prior Orca's + // PORT/TOKEN frozen into process.env. The plugin must prefer the on-disk + // endpoint file (rewritten on every Orca start()) over env, otherwise it + // keeps posting to a dead port after an Orca restart. + const source = _internals.getOpenCodePluginSource() + + expect(source).toContain('function readEndpointFile()') + expect(source).toContain('process.env.ORCA_AGENT_HOOK_ENDPOINT') + // Parser accepts both `KEY=VALUE` (Unix) and `set KEY=VALUE` (Windows): + expect(source).toContain('/^(?:set\\s+)?([A-Z0-9_]+)=(.*)$/') + expect(source).toContain('function resolveHookCoords()') + // File takes precedence over env — the whole point of v2: + expect(source).toContain( + 'port: fileEnv.ORCA_AGENT_HOOK_PORT || process.env.ORCA_AGENT_HOOK_PORT' + ) + expect(source).toContain( + 'token: fileEnv.ORCA_AGENT_HOOK_TOKEN || process.env.ORCA_AGENT_HOOK_TOKEN' + ) + // post() uses the resolved coords, not a cached-at-startup url: + expect(source).toContain('const coords = resolveHookCoords();') + expect(source).toContain('`http://127.0.0.1:${coords.port}/hook/opencode`') + expect(source).toContain('"X-Orca-Agent-Hook-Token": coords.token') + }) + + it('caches the parsed endpoint file on mtime+size+inode to skip re-reads per post', () => { + // Why: message.part.updated fires many times per second during a streaming + // assistant reply. Each post() calls resolveHookCoords() which reads the + // endpoint file — without the cache we'd readFileSync + parse on every + // streamed Part. The cache key combines mtime + size + inode so renameSync + // (writeEndpointFile's atomic swap) invalidates the cache via the ino + // change even when mtime resolution is coarse and size happens to match. + const source = _internals.getOpenCodePluginSource() + + expect(source).toContain('let cachedEndpointKey = "";') + expect(source).toContain('let cachedEndpointValues = null;') + expect(source).toContain('const stat = fs.statSync(path);') + expect(source).toContain('const cacheKey = stat.mtimeMs + ":" + stat.size + ":" + stat.ino;') + expect(source).toContain('if (cacheKey === cachedEndpointKey && cachedEndpointValues) {') + expect(source).toContain('return cachedEndpointValues;') + // Stat failure must invalidate the cache, not lock in stale values: + expect(source).toContain('cachedEndpointKey = "";') + expect(source).toContain('cachedEndpointValues = null;') + }) + + it('guards endpoint-file parse warnings with a process-lifetime latch', () => { + // Why: ENOENT is the normal pre-install case and must stay silent, but a + // malformed/unreadable file (EACCES, EIO, parse error) would otherwise + // spam stderr once per hook post. The latch keeps the warning to once per + // OpenCode process — mirrors server.ts's warnedVersions/warnedEnvs intent. + const source = _internals.getOpenCodePluginSource() + + expect(source).toContain('let warnedBadEndpoint = false;') + expect(source).toContain('err.code !== "ENOENT"') + expect(source).toContain('warnedBadEndpoint = true;') + }) }) describe('OpenCode id safety guard', () => { diff --git a/src/main/opencode/hook-service.ts b/src/main/opencode/hook-service.ts index dca80d6c2..5990fa667 100644 --- a/src/main/opencode/hook-service.ts +++ b/src/main/opencode/hook-service.ts @@ -1,3 +1,7 @@ +/* eslint-disable max-lines -- Why: this file contains a multi-line inline + JS plugin source emitted into OpenCode's plugins directory as a single + file; splitting the plugin source across TS modules would obscure the + runtime artifact and scatter tightly coupled string-template logic. */ import { app } from 'electron' import { join } from 'path' import { mkdirSync, writeFileSync, rmSync } from 'fs' @@ -44,9 +48,90 @@ function getOpenCodePluginSource(): string { // mapping is done plugin-side (SessionBusy / SessionIdle / PermissionRequest) // so the server-side normalizer can keep its one-event-per-case switch shape. return [ - 'function getHookUrl() {', - ' const port = process.env.ORCA_AGENT_HOOK_PORT;', - ' return port ? `http://127.0.0.1:${port}/hook/opencode` : null;', + '// Why: process-lifetime guard so a recurring parse error on a malformed', + "// endpoint file does not spam OpenCode's stderr once per hook post.", + '// This guard lives inside the plugin source because the plugin runs in', + "// OpenCode's Node process (not Orca's) and has no access to server.ts's", + '// equivalent warnedVersions / warnedEnvs Sets.', + 'let warnedBadEndpoint = false;', + '', + '// Why: message.part.updated can fire many times per second during a', + '// streaming assistant reply, and each post() calls resolveHookCoords()', + '// which reads the endpoint file. The file only changes on Orca restart', + '// (rare), so a stat+mtime check is substantially cheaper than a full', + '// readFileSync+parse on every streamed part. On stat error we fall', + '// through to parse so the fail-open behavior is preserved.', + 'let cachedEndpointKey = "";', + 'let cachedEndpointValues = null;', + '', + 'function readEndpointFile() {', + ' const path = process.env.ORCA_AGENT_HOOK_ENDPOINT;', + ' if (!path) return null;', + ' try {', + ' const fs = require("fs");', + ' try {', + ' const stat = fs.statSync(path);', + ' // Why: cache key combines mtime + size + inode. renameSync (used by', + ' // writeEndpointFile on the Orca side) allocates a fresh inode on', + ' // POSIX and a new Windows file ID on NTFS, so ino changes on every', + ' // legitimate rewrite even when mtimeMs resolution is coarse and size', + ' // happens to match.', + ' const cacheKey = stat.mtimeMs + ":" + stat.size + ":" + stat.ino;', + ' if (cacheKey === cachedEndpointKey && cachedEndpointValues) {', + ' return cachedEndpointValues;', + ' }', + ' const contents = fs.readFileSync(path, "utf8");', + ' const out = {};', + ' for (const line of contents.split(/\\r?\\n/)) {', + ' // Why: Windows endpoint.cmd uses `set KEY=VALUE`; Unix endpoint.env', + ' // uses `KEY=VALUE`. Making `set ` optional lets the same parser', + ' // handle both without platform detection in the plugin. Allow', + ' // digits in the key for forward-compat with future ORCA_AGENT_HOOK_*', + ' // names that may contain numerics, and strip a trailing CR so', + ' // mixed-EOL files with lone `\\r` do not leak CR into the value.', + ' const m = line.match(/^(?:set\\s+)?([A-Z0-9_]+)=(.*)$/);', + ' if (m) out[m[1]] = m[2].replace(/\\r$/, "");', + ' }', + ' cachedEndpointKey = cacheKey;', + ' cachedEndpointValues = out;', + ' return out;', + ' } catch (ioErr) {', + ' // Why: any stat or read failure (file yanked mid-read, permission', + ' // race, unlink between stat and readFileSync) must invalidate the', + ' // cache so a transient failure does not lock in a stale parse for', + ' // the remaining process lifetime; rethrow to the outer catch.', + ' cachedEndpointKey = "";', + ' cachedEndpointValues = null;', + ' throw ioErr;', + ' }', + ' } catch (err) {', + ' // Why: warn once per process if the file exists but is unreadable or', + ' // malformed — a persistent, silently-swallowed parse error would', + ' // otherwise leave the plugin falling back to stale process.env on', + ' // every post with no signal. ENOENT / missing env var is the normal', + ' // pre-install case; stay silent for it.', + ' if (err && err.code !== "ENOENT" && !warnedBadEndpoint) {', + ' warnedBadEndpoint = true;', + ' console.warn("[orca-hook] failed to parse endpoint file:", err.message);', + ' }', + ' return null;', + ' }', + '}', + '', + 'function resolveHookCoords() {', + ' // Why: prefer the on-disk endpoint file over process.env because env was', + ' // frozen when OpenCode was fork()ed — stale after an Orca restart. The', + ' // file is rewritten on every Orca start(), so sourcing it per post lets', + ' // a long-running OpenCode session reach the current server. Falls back', + ' // to process.env when the file is absent (first-run / pre-endpoint-file / Orca', + ' // never started writing the file).', + ' const fileEnv = readEndpointFile() || {};', + ' return {', + ' port: fileEnv.ORCA_AGENT_HOOK_PORT || process.env.ORCA_AGENT_HOOK_PORT,', + ' token: fileEnv.ORCA_AGENT_HOOK_TOKEN || process.env.ORCA_AGENT_HOOK_TOKEN,', + ' env: fileEnv.ORCA_AGENT_HOOK_ENV || process.env.ORCA_AGENT_HOOK_ENV || "",', + ' version: fileEnv.ORCA_AGENT_HOOK_VERSION || process.env.ORCA_AGENT_HOOK_VERSION || "",', + ' };', '}', '', 'function getStatusType(event) {', @@ -98,16 +183,20 @@ function getOpenCodePluginSource(): string { '}', '', 'async function post(hookEventName, extraProperties) {', - ' const url = getHookUrl();', - ' const token = process.env.ORCA_AGENT_HOOK_TOKEN;', + ' // Why: resolve coords per post — the endpoint file may have been', + ' // rewritten by a newer Orca since the last call. Pane/tab/worktree IDs', + ' // stay on process.env because they are per-PTY (stable for the life of', + ' // the OpenCode process), not per-Orca-instance.', + ' const coords = resolveHookCoords();', ' const paneKey = process.env.ORCA_PANE_KEY;', - ' if (!url || !token || !paneKey) return;', + ' if (!coords.port || !coords.token || !paneKey) return;', + ' const url = `http://127.0.0.1:${coords.port}/hook/opencode`;', ' const body = JSON.stringify({', ' paneKey,', ' tabId: process.env.ORCA_TAB_ID || "",', ' worktreeId: process.env.ORCA_WORKTREE_ID || "",', - ' env: process.env.ORCA_AGENT_HOOK_ENV || "",', - ' version: process.env.ORCA_AGENT_HOOK_VERSION || "",', + ' env: coords.env,', + ' version: coords.version,', ' payload: { hook_event_name: hookEventName, ...(extraProperties || {}) },', ' });', ' try {', @@ -115,7 +204,7 @@ function getOpenCodePluginSource(): string { ' method: "POST",', ' headers: {', ' "Content-Type": "application/json",', - ' "X-Orca-Agent-Hook-Token": token,', + ' "X-Orca-Agent-Hook-Token": coords.token,', ' },', ' body,', ' });', diff --git a/src/shared/agent-hook-types.ts b/src/shared/agent-hook-types.ts index 70a72f276..1b87f1532 100644 --- a/src/shared/agent-hook-types.ts +++ b/src/shared/agent-hook-types.ts @@ -1,8 +1,8 @@ -// Why: these IPC payload shapes live here but have no renderer/main consumer -// in this PR. They are pulled in by the main-process hook server (Claude / -// Codex / Gemini native hook integrations) that lands in the follow-up PR. -// Keeping the types shared up-front avoids a churn PR that renames or splits -// them once the hook server imports them. +// Why: shared agent-hook IPC payload shapes and the managed-script protocol +// version constant. Consumed by both the main-process hook server (src/main/ +// agent-hooks/server.ts) and each per-agent hook service (claude/codex/ +// gemini/cursor/hook-service.ts). Lives in `shared/` to keep a single +// source of truth for the version string and status contract. export type AgentHookTarget = 'claude' | 'codex' | 'gemini' | 'cursor' @@ -19,5 +19,10 @@ export type AgentHookInstallStatus = { // Why: bumped whenever the managed script's request shape changes. The // receiver logs a warning when it sees a request from a different version so a // stale script installed by an older app build is diagnosable instead of -// silently producing partial payloads. +// silently producing partial payloads. Still at v1 because the endpoint-file +// rollout is additive — pre-endpoint-file scripts still post the same JSON +// body shape, and no caller was ever shipped on v2 (the Claude/Codex/Gemini +// install path has been gated behind AGENT_DASHBOARD_ENABLED=false, and the +// Cursor/OpenCode scripts reroll on every Orca launch so no in-wild fleet +// exists to distinguish from). Reserve the next bump for a real wire change. export const ORCA_HOOK_PROTOCOL_VERSION = '1' as const