From 94d20020a848a00414ca5b65b2c0339891b979b8 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 18 Jun 2026 23:57:36 +0000 Subject: [PATCH 1/4] refactor(main): extract shared toErrorMessage/describeError + isRecord MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Nine main-process modules each carried a private copy of `toErrorMessage`; eight were byte-identical and one (integration-event-bridge) was a richer variant. Consolidate into: - src/main/errors.ts — `toErrorMessage` (concise) and `describeError` (the rich field-probing fallback, formerly inlined in integration-event-bridge). - src/main/guards.ts — canonical `isRecord` (non-null, non-array). integration-event-bridge now imports `describeError as toErrorMessage` so its call sites are unchanged. Behavior is preserved exactly. Relative value imports of these modules from files in the node:test strip-types runtime graph (burn, integration-event-bridge) use the explicit `.ts` extension with the repo's `@ts-expect-error` idiom; other importers stay extensionless. Verified: npm test (122), vitest (451), typecheck, lint all pass. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01J59WsDm4zNGqhW137jEkaG --- src/main/broker.ts | 6 +-- src/main/burn.ts | 5 +-- src/main/cloud-agent.ts | 4 +- src/main/errors.ts | 57 ++++++++++++++++++++++++++++ src/main/guards.ts | 12 ++++++ src/main/integration-event-bridge.ts | 39 +------------------ src/main/integration-mounts.ts | 4 +- src/main/integration-symlinks.ts | 4 +- src/main/integrations.ts | 4 +- src/main/ipc-handlers.ts | 4 +- src/main/proactive-agent.ts | 4 +- 11 files changed, 80 insertions(+), 63 deletions(-) create mode 100644 src/main/errors.ts create mode 100644 src/main/guards.ts diff --git a/src/main/broker.ts b/src/main/broker.ts index 61e4bdb8..36793bf2 100644 --- a/src/main/broker.ts +++ b/src/main/broker.ts @@ -19,6 +19,7 @@ import { import { AgentRelay, type RelayMessage } from '@agent-relay/sdk' import { getAccessToken, getApiUrl } from './auth' import { assertDirectory } from './path-utils' +import { toErrorMessage } from './errors' import { PtyInputStreamManager } from './pty-input-stream' import { PtyChunkDeduper } from './pty-dedup' import { @@ -825,11 +826,6 @@ async function terminateOwnedBrokerProcess(pid: number | undefined): Promise { return new Promise((resolve) => setTimeout(resolve, ms)) } -function toErrorMessage(error: unknown): string { - return error instanceof Error ? error.message : String(error) -} function normalizeAgentRecord(record: ProactiveAgentRecord | Record): CloudAgentRecord { const fallbackId = typeof record.agentId === 'string' ? record.agentId : '' diff --git a/src/main/errors.ts b/src/main/errors.ts new file mode 100644 index 00000000..240b4782 --- /dev/null +++ b/src/main/errors.ts @@ -0,0 +1,57 @@ +/** + * Shared error-to-string helpers for the main process. + * + * `toErrorMessage` is the concise form used almost everywhere; `describeError` + * is the richer fallback (used for opaque integration/transport errors) that + * digs through common error-shaped fields before giving up. + */ +// @ts-expect-error Node's strip-types test runner requires the explicit .ts extension. +import { isRecord } from './guards.ts' + +/** Concise message: `Error.message`, or `String()` of any other thrown value. */ +export function toErrorMessage(error: unknown): string { + return error instanceof Error ? error.message : String(error) +} + +/** + * Best-effort human-readable description for opaque/transport errors that may + * arrive as plain objects, strings, or structured `{ code, status, ... }` + * payloads rather than `Error` instances. + */ +export function describeError(error: unknown): string { + if (error instanceof Error) { + const message = error.message.trim() + return message || error.name || error.constructor.name || 'Error' + } + if (typeof error === 'string') { + const message = error.trim() + return message || 'empty string error' + } + if (isRecord(error)) { + const record = error as Record + const message = record.message + if (typeof message === 'string' && message.trim()) return message.trim() + const reason = record.reason + if (typeof reason === 'string' && reason.trim()) return reason.trim() + const parts = [ + typeof record.name === 'string' && record.name.trim() ? record.name.trim() : null, + typeof record.type === 'string' && record.type.trim() ? `type=${record.type.trim()}` : null, + typeof record.code === 'string' && record.code.trim() ? `code=${record.code.trim()}` : typeof record.code === 'number' ? `code=${record.code}` : null, + typeof record.status === 'string' && record.status.trim() ? `status=${record.status.trim()}` : typeof record.status === 'number' ? `status=${record.status}` : null, + typeof record.statusCode === 'string' && record.statusCode.trim() ? `statusCode=${record.statusCode.trim()}` : typeof record.statusCode === 'number' ? `statusCode=${record.statusCode}` : null, + typeof record.httpStatus === 'string' && record.httpStatus.trim() ? `httpStatus=${record.httpStatus.trim()}` : typeof record.httpStatus === 'number' ? `httpStatus=${record.httpStatus}` : null + ].filter((entry): entry is string => entry !== null) + if (parts.length > 0) return parts.join(' ') + const constructorName = record.constructor?.name + if (constructorName && constructorName !== 'Object') return constructorName + + try { + const json = JSON.stringify(error) + if (json && json !== '{}') return json + } catch { + // Fall through to String(error). + } + } + const text = String(error).trim() + return text && text !== '[object Object]' ? text : 'unknown error' +} diff --git a/src/main/guards.ts b/src/main/guards.ts new file mode 100644 index 00000000..7c9fc763 --- /dev/null +++ b/src/main/guards.ts @@ -0,0 +1,12 @@ +/** + * Shared runtime type guards for the main process. + * + * Several modules grew private copies of these primitives; keeping a single + * canonical definition here avoids the subtle drift that accumulated (some + * copies treated arrays as records, others did not). + */ + +/** True for non-null, non-array objects — i.e. plain `Record`-like values. */ +export function isRecord(value: unknown): value is Record { + return !!value && typeof value === 'object' && !Array.isArray(value) +} diff --git a/src/main/integration-event-bridge.ts b/src/main/integration-event-bridge.ts index 115f90bc..74bab2df 100644 --- a/src/main/integration-event-bridge.ts +++ b/src/main/integration-event-bridge.ts @@ -1,4 +1,6 @@ import { createHash } from 'node:crypto' +// @ts-expect-error Node's strip-types test runner requires the explicit .ts extension. +import { describeError as toErrorMessage } from './errors.ts' import { existsSync, watch, type FSWatcher } from 'node:fs' import { appendFile, mkdir, readFile, rm, stat } from 'node:fs/promises' import { homedir } from 'node:os' @@ -369,43 +371,6 @@ export function integrationRelayFileSyncOptions( } } -function toErrorMessage(error: unknown): string { - if (error instanceof Error) { - const message = error.message.trim() - return message || error.name || error.constructor.name || 'Error' - } - if (typeof error === 'string') { - const message = error.trim() - return message || 'empty string error' - } - if (isRecord(error)) { - const record = error as Record - const message = record.message - if (typeof message === 'string' && message.trim()) return message.trim() - const reason = record.reason - if (typeof reason === 'string' && reason.trim()) return reason.trim() - const parts = [ - typeof record.name === 'string' && record.name.trim() ? record.name.trim() : null, - typeof record.type === 'string' && record.type.trim() ? `type=${record.type.trim()}` : null, - typeof record.code === 'string' && record.code.trim() ? `code=${record.code.trim()}` : typeof record.code === 'number' ? `code=${record.code}` : null, - typeof record.status === 'string' && record.status.trim() ? `status=${record.status.trim()}` : typeof record.status === 'number' ? `status=${record.status}` : null, - typeof record.statusCode === 'string' && record.statusCode.trim() ? `statusCode=${record.statusCode.trim()}` : typeof record.statusCode === 'number' ? `statusCode=${record.statusCode}` : null, - typeof record.httpStatus === 'string' && record.httpStatus.trim() ? `httpStatus=${record.httpStatus.trim()}` : typeof record.httpStatus === 'number' ? `httpStatus=${record.httpStatus}` : null - ].filter((entry): entry is string => entry !== null) - if (parts.length > 0) return parts.join(' ') - const constructorName = record.constructor?.name - if (constructorName && constructorName !== 'Object') return constructorName - - try { - const json = JSON.stringify(error) - if (json && json !== '{}') return json - } catch { - // Fall through to String(error). - } - } - const text = String(error).trim() - return text && text !== '[object Object]' ? text : 'unknown error' -} function isUnauthorizedError(error: unknown): boolean { if (!error || typeof error !== 'object') return false diff --git a/src/main/integration-mounts.ts b/src/main/integration-mounts.ts index 9b9d0e9c..9c8a87ae 100644 --- a/src/main/integration-mounts.ts +++ b/src/main/integration-mounts.ts @@ -1,4 +1,5 @@ import { chmod, mkdir, readFile, rm } from 'node:fs/promises' +import { toErrorMessage } from './errors' import { homedir } from 'node:os' import { join } from 'node:path' import { @@ -107,9 +108,6 @@ type IntegrationMountSpec = { scopes: string[] } -function toErrorMessage(error: unknown): string { - return error instanceof Error ? error.message : String(error) -} function isAccountWorkspaceRequiredError(error: unknown): boolean { return /account-workspace-required/i.test(toErrorMessage(error)) diff --git a/src/main/integration-symlinks.ts b/src/main/integration-symlinks.ts index 9fce78dc..ceb67fc8 100644 --- a/src/main/integration-symlinks.ts +++ b/src/main/integration-symlinks.ts @@ -3,6 +3,7 @@ import { lstat, mkdir, readFile, readlink, rm, symlink, writeFile } from 'node:f import { dirname, isAbsolute, join, resolve } from 'node:path' import { promisify } from 'node:util' import { integrationMountRootForWorkspace } from './integration-mounts' +import { toErrorMessage } from './errors' const execFileAsync = promisify(execFile) const gitDirCache = new Map>() @@ -15,9 +16,6 @@ export const PROJECT_INTEGRATIONS_LINK_NAME = '.integrations' const GIT_EXCLUDE_MARKER = '# pear: integration mount symlink (auto-managed)' -function toErrorMessage(error: unknown): string { - return error instanceof Error ? error.message : String(error) -} function isFileAlreadyExistsError(error: unknown): boolean { return !!error && diff --git a/src/main/integrations.ts b/src/main/integrations.ts index eb1ad8a5..628b1cb8 100644 --- a/src/main/integrations.ts +++ b/src/main/integrations.ts @@ -1,4 +1,5 @@ import { randomUUID } from 'node:crypto' +import { toErrorMessage } from './errors' import { existsSync, readFileSync } from 'node:fs' import { isAbsolute, relative, resolve } from 'node:path' import { BrowserWindow, shell } from 'electron' @@ -294,9 +295,6 @@ function buildApiUrl(apiUrl: string, path: string): string { return new URL(path.replace(/^\/+/, ''), `${apiUrl}/`).toString() } -function toErrorMessage(error: unknown): string { - return error instanceof Error ? error.message : String(error) -} function isCloudAuthRequiredError(error: unknown): boolean { return /cloud-auth-required/i.test(toErrorMessage(error)) diff --git a/src/main/ipc-handlers.ts b/src/main/ipc-handlers.ts index afd417fb..c377c63f 100644 --- a/src/main/ipc-handlers.ts +++ b/src/main/ipc-handlers.ts @@ -1,4 +1,5 @@ import { app, ipcMain, dialog, BrowserWindow, shell } from 'electron' +import { toErrorMessage } from './errors' import { createHash } from 'crypto' import { existsSync } from 'fs' import { mkdir, readFile, writeFile } from 'fs/promises' @@ -67,9 +68,6 @@ function assertPathWithinProjects(targetPath: string): void { const gitStatusWarnings = new Set() -function toErrorMessage(error: unknown): string { - return error instanceof Error ? error.message : String(error) -} function warnGitStatusOnce(path: string, error: unknown): void { const message = toErrorMessage(error) diff --git a/src/main/proactive-agent.ts b/src/main/proactive-agent.ts index a133ac0f..6fea086f 100644 --- a/src/main/proactive-agent.ts +++ b/src/main/proactive-agent.ts @@ -1,4 +1,5 @@ import { rm } from 'node:fs/promises' +import { toErrorMessage } from './errors' import { buildPersonaSpec, deployBundle, @@ -104,9 +105,6 @@ async function readCloudJson(response: Response): Promise { return response.json().catch(() => null) } -function toErrorMessage(error: unknown): string { - return error instanceof Error ? error.message : String(error) -} function isMissingModuleError(error: unknown): boolean { if (!isRecord(error)) return false From 9ff8d7c514f6622fd539a4cab68d1569782473c2 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 19 Jun 2026 00:01:11 +0000 Subject: [PATCH 2/4] refactor(main): consolidate isRecord into shared guards module Eight modules carried private `isRecord` copies across three slightly different shapes; broker and spawn-coordinator used an array-INCLUDING form while the rest excluded arrays. Replace all with the canonical `isRecord` from src/main/guards.ts (non-null, non-array). The two array-including call sites are pure object-property coercions (`isRecord(x) ? x : {}`) that only read named string/typed properties, which an array yields as undefined either way, so switching to the stricter guard is behavior-preserving. Verified: npm test (122), vitest (451), typecheck, lint all pass. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01J59WsDm4zNGqhW137jEkaG --- src/main/auth.ts | 4 +--- src/main/broker.ts | 4 +--- src/main/integration-event-bridge.ts | 5 ++--- src/main/integrations.ts | 4 +--- src/main/ipc-handlers.ts | 4 +--- src/main/proactive-agent.ts | 4 +--- src/main/spawn-coordinator.ts | 4 +--- src/main/store.ts | 4 +--- 8 files changed, 9 insertions(+), 24 deletions(-) diff --git a/src/main/auth.ts b/src/main/auth.ts index c5054d13..c1f9445b 100644 --- a/src/main/auth.ts +++ b/src/main/auth.ts @@ -1,4 +1,5 @@ import { app, shell, safeStorage } from 'electron' +import { isRecord } from './guards' import { createHash } from 'crypto' import { createServer, type Server } from 'http' import { readFileSync, writeFileSync, mkdirSync, existsSync } from 'fs' @@ -77,9 +78,6 @@ function hasStoredTokens(): boolean { // The cloud API has historically returned the same logical field under several // keys. We tolerate common camelCase/snake_case variants, then validate the // final shape with UserInfoSchema. -function isRecord(value: unknown): value is Record { - return !!value && typeof value === 'object' && !Array.isArray(value) -} function firstString(record: Record | undefined, keys: string[]): string | undefined { if (!record) return undefined diff --git a/src/main/broker.ts b/src/main/broker.ts index 36793bf2..56ed99ce 100644 --- a/src/main/broker.ts +++ b/src/main/broker.ts @@ -20,6 +20,7 @@ import { AgentRelay, type RelayMessage } from '@agent-relay/sdk' import { getAccessToken, getApiUrl } from './auth' import { assertDirectory } from './path-utils' import { toErrorMessage } from './errors' +import { isRecord } from './guards' import { PtyInputStreamManager } from './pty-input-stream' import { PtyChunkDeduper } from './pty-dedup' import { @@ -637,9 +638,6 @@ async function mapWithConcurrency( return results } -function isRecord(value: unknown): value is Record { - return typeof value === 'object' && value !== null -} function brokerEventString(event: BrokerEvent, key: string): string | undefined { const value = (event as unknown as Record)[key] diff --git a/src/main/integration-event-bridge.ts b/src/main/integration-event-bridge.ts index 74bab2df..9731ba38 100644 --- a/src/main/integration-event-bridge.ts +++ b/src/main/integration-event-bridge.ts @@ -1,6 +1,8 @@ import { createHash } from 'node:crypto' // @ts-expect-error Node's strip-types test runner requires the explicit .ts extension. import { describeError as toErrorMessage } from './errors.ts' +// @ts-expect-error Node's strip-types test runner requires the explicit .ts extension. +import { isRecord } from './guards.ts' import { existsSync, watch, type FSWatcher } from 'node:fs' import { appendFile, mkdir, readFile, rm, stat } from 'node:fs/promises' import { homedir } from 'node:os' @@ -440,9 +442,6 @@ function withTimeout(promise: Promise, timeoutMs: number, message: string) }) } -function isRecord(value: unknown): value is Record { - return !!value && typeof value === 'object' && !Array.isArray(value) -} // View any value as a string-keyed record for dynamic field access. Non-record // inputs collapse to an empty record so callers can read optional keys off diff --git a/src/main/integrations.ts b/src/main/integrations.ts index 628b1cb8..e05c2181 100644 --- a/src/main/integrations.ts +++ b/src/main/integrations.ts @@ -1,5 +1,6 @@ import { randomUUID } from 'node:crypto' import { toErrorMessage } from './errors' +import { isRecord } from './guards' import { existsSync, readFileSync } from 'node:fs' import { isAbsolute, relative, resolve } from 'node:path' import { BrowserWindow, shell } from 'electron' @@ -322,9 +323,6 @@ function integrationAuthRecoveryMessage(reason: IntegrationAuthRecoveryState['re return failureClass ? `${reason}:${failureClass}` : reason } -function isRecord(value: unknown): value is Record { - return !!value && typeof value === 'object' && !Array.isArray(value) -} function stringList(value: unknown): string[] { if (!Array.isArray(value)) return [] diff --git a/src/main/ipc-handlers.ts b/src/main/ipc-handlers.ts index c377c63f..ed4f80ff 100644 --- a/src/main/ipc-handlers.ts +++ b/src/main/ipc-handlers.ts @@ -1,5 +1,6 @@ import { app, ipcMain, dialog, BrowserWindow, shell } from 'electron' import { toErrorMessage } from './errors' +import { isRecord } from './guards' import { createHash } from 'crypto' import { existsSync } from 'fs' import { mkdir, readFile, writeFile } from 'fs/promises' @@ -77,9 +78,6 @@ function warnGitStatusOnce(path: string, error: unknown): void { console.warn(`[git] Failed to read status for ${path}: ${message}`) } -function isRecord(value: unknown): value is Record { - return !!value && typeof value === 'object' && !Array.isArray(value) -} function firstString(record: Record, keys: string[]): string | undefined { for (const key of keys) { diff --git a/src/main/proactive-agent.ts b/src/main/proactive-agent.ts index 6fea086f..08cc472a 100644 --- a/src/main/proactive-agent.ts +++ b/src/main/proactive-agent.ts @@ -1,5 +1,6 @@ import { rm } from 'node:fs/promises' import { toErrorMessage } from './errors' +import { isRecord } from './guards' import { buildPersonaSpec, deployBundle, @@ -69,9 +70,6 @@ function cloneRun(run: ProactiveAgentRun): ProactiveAgentRun { return JSON.parse(JSON.stringify(run)) as ProactiveAgentRun } -function isRecord(value: unknown): value is Record { - return !!value && typeof value === 'object' && !Array.isArray(value) -} function cloudPath(path: string, query?: Record): string { const params = new URLSearchParams() diff --git a/src/main/spawn-coordinator.ts b/src/main/spawn-coordinator.ts index b39d5209..293817af 100644 --- a/src/main/spawn-coordinator.ts +++ b/src/main/spawn-coordinator.ts @@ -1,4 +1,5 @@ import type { SpawnPtyInput } from '@agent-relay/harness-driver' +import { isRecord } from './guards' export type BrokerSpawnResult = { name: string @@ -11,9 +12,6 @@ export type PersonaBrokerSpawnResult = BrokerSpawnResult & { workerStreamBaselineCount: number } -function isRecord(value: unknown): value is Record { - return typeof value === 'object' && value !== null -} // Coerce the loosely-typed spawnPty result into a BrokerSpawnResult, filling in // a fallback name/runtime and preferring an explicitly resolved CLI over the diff --git a/src/main/store.ts b/src/main/store.ts index 452dcdbb..8676d750 100644 --- a/src/main/store.ts +++ b/src/main/store.ts @@ -1,4 +1,5 @@ import { app } from 'electron' +import { isRecord } from './guards' import { readFileSync, writeFileSync, renameSync, mkdirSync, existsSync } from 'fs' import { basename, join, resolve } from 'path' import { z } from 'zod' @@ -83,9 +84,6 @@ function defaultRootName(path: string): string { return basename(path) || path } -function isRecord(value: unknown): value is Record { - return typeof value === 'object' && value !== null && !Array.isArray(value) -} // Normalize an untrusted relay-workspace record read from disk or supplied by a // caller into a clean RelayWorkspaceRecord, or undefined when it lacks a usable From 9590786f5982ca5cb8571cecee88dd3239a3dea8 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 19 Jun 2026 00:07:11 +0000 Subject: [PATCH 3/4] refactor(broker): extract binary resolution into broker-binary.ts The broker binary resolution helpers and the legacy `--name` compatibility shim were ~215 lines of stateless infrastructure embedded in the 4282-line broker.ts. Move them to a dedicated src/main/broker-binary.ts: - resolveBundledBrokerBinary / parseBrokerInitCliFlags (publicly tested) - resolveHarnessBrokerBinary (now exported; consumed by BrokerManager) - the private resolve/inspect/shim helpers broker.ts imports resolveHarnessBrokerBinary from the new module and drops the now-unused fs/promises imports (chmod/mkdir/readFile/writeFile). broker.test.ts imports the two tested helpers from broker-binary. Pure behavior-preserving move (verified by autoreview); __dirname semantics are unchanged since both modules bundle into out/main. Verified: npm test (122), vitest (451), typecheck, lint all pass. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01J59WsDm4zNGqhW137jEkaG --- src/main/broker-binary.ts | 235 ++++++++++++++++++++++++++++++++++++++ src/main/broker.test.ts | 6 +- src/main/broker.ts | 219 +---------------------------------- 3 files changed, 241 insertions(+), 219 deletions(-) create mode 100644 src/main/broker-binary.ts diff --git a/src/main/broker-binary.ts b/src/main/broker-binary.ts new file mode 100644 index 00000000..5ac8e1f5 --- /dev/null +++ b/src/main/broker-binary.ts @@ -0,0 +1,235 @@ +/** + * Broker binary resolution and the legacy `--name` compatibility shim. + * + * The v8 harness-driver runtime normally locates the bundled broker via + * `import.meta.url`, but electron-vite bundles the driver into the main + * process (so `import.meta.url` points at `out/main/` rather than + * `node_modules/`). These helpers resolve the correct per-platform binary + * across the packaged/unpacked/local-build layouts and, when the binary only + * understands the older `--name` flag, launch it through a generated shim that + * rewrites `--instance-name`/`--workspace-key` arguments. + * + * Extracted out of broker.ts: this is stateless infrastructure with no + * dependency on BrokerManager. + */ +import { chmod, mkdir, readFile, writeFile } from 'fs/promises' +import { execFile } from 'child_process' +import { join } from 'path' +import { app } from 'electron' +import { canExecute } from './mcp-command' + +function resolveBrokerBinaryOverride(): string | null { + const configured = process.env.AGENT_RELAY_BIN?.trim() + if (!configured) return null + if (canExecute(configured)) return configured + console.warn('[broker] Ignoring AGENT_RELAY_BIN because it is not executable:', configured) + return null +} + +// Resolve the broker binary bundled with the v8 harness-driver runtime. +// The runtime normally resolves this via import.meta.url, but that breaks when +// electron-vite bundles the driver into the main process (import.meta.url points +// to out/main/ instead of node_modules/). +function brokerBinaryNameForPlatform(platform: NodeJS.Platform): string { + return platform === 'win32' ? 'agent-relay-broker.exe' : 'agent-relay-broker' +} + +export function resolveBundledBrokerBinary(baseDir = __dirname, isPackaged = app.isPackaged): string { + const configuredBinary = resolveBrokerBinaryOverride() + if (configuredBinary) { + console.log('[broker] Using configured Agent Relay broker binary:', configuredBinary) + return configuredBinary + } + + const suffix = `${process.platform}-${process.arch}` + const unpackIfPackaged = (binary: string): string => + isPackaged ? binary.replace('app.asar', 'app.asar.unpacked') : binary + + // v8 ships the broker as a per-platform optional package (@agent-relay/broker-*). + const brokerBinaryName = brokerBinaryNameForPlatform(process.platform) + const optionalPackageBinary = join( + baseDir, '..', '..', 'node_modules', '@agent-relay', `broker-${suffix}`, 'bin', + brokerBinaryName + ) + const unpackedOptionalPackageBinary = unpackIfPackaged(optionalPackageBinary) + if (canExecute(unpackedOptionalPackageBinary)) return unpackedOptionalPackageBinary + if (unpackedOptionalPackageBinary !== optionalPackageBinary && canExecute(optionalPackageBinary)) { + return optionalPackageBinary + } + + const localBinary = join(baseDir, '..', '..', '..', 'relay', 'target', 'debug', brokerBinaryName) + if (canExecute(localBinary)) { + console.warn('[broker] Bundled Agent Relay broker binary was not found; falling back to local relay build:', localBinary) + return localBinary + } + + // Backward-compatible fallback for SDK packages that still carry per-platform + // broker binaries directly. + const brokerBinary = join( + baseDir, '..', '..', 'node_modules', '@agent-relay', 'sdk', 'bin', + `agent-relay-broker-${suffix}${process.platform === 'win32' ? '.exe' : ''}` + ) + return unpackIfPackaged(brokerBinary) +} + +interface BrokerInitCliFlags { + supportsInstanceName: boolean + supportsName: boolean + supportsWorkspaceKey: boolean +} + +export function parseBrokerInitCliFlags(helpText: string): BrokerInitCliFlags { + return { + supportsInstanceName: /(?:^|\s)--instance-name(?:\s|[<=[,]|$)/.test(helpText), + supportsName: /(?:^|\s)--name(?:\s|[<=[,]|$)/.test(helpText), + supportsWorkspaceKey: /(?:^|\s)--workspace-key(?:\s|[<=[,]|$)/.test(helpText) + } +} + +function inspectBrokerInitCliFlags(binaryPath: string): Promise { + return new Promise((resolve) => { + execFile(binaryPath, ['init', '--help'], { + encoding: 'utf8', + timeout: 2_000, + windowsHide: true + }, (err, stdout) => { + if (err) { + console.warn(`[broker] Failed to inspect broker init CLI for ${binaryPath}:`, err) + resolve({ + supportsInstanceName: true, + supportsName: true, + supportsWorkspaceKey: true + }) + return + } + resolve(parseBrokerInitCliFlags(stdout)) + }) + }) +} + +function brokerBinaryCompatShimSource(): string { + return `#!/usr/bin/env node +const { spawn } = require('node:child_process') + +const realBinary = process.env.PEAR_AGENT_RELAY_BROKER_BINARY +if (!realBinary) { + console.error('[pear-broker-compat] PEAR_AGENT_RELAY_BROKER_BINARY is required') + process.exit(127) +} + +const supportsInstanceName = process.env.PEAR_AGENT_RELAY_BROKER_SUPPORTS_INSTANCE_NAME === '1' +const supportsWorkspaceKey = process.env.PEAR_AGENT_RELAY_BROKER_SUPPORTS_WORKSPACE_KEY === '1' +const inputArgs = process.argv.slice(2) +const outputArgs = [] + +for (let index = 0; index < inputArgs.length; index += 1) { + const arg = inputArgs[index] + if (!supportsInstanceName && arg === '--instance-name') { + outputArgs.push('--name') + if (index + 1 < inputArgs.length) outputArgs.push(inputArgs[++index]) + continue + } + if (!supportsInstanceName && arg.startsWith('--instance-name=')) { + outputArgs.push('--name=' + arg.slice('--instance-name='.length)) + continue + } + if (!supportsWorkspaceKey && arg === '--workspace-key') { + index += 1 + continue + } + if (!supportsWorkspaceKey && arg.startsWith('--workspace-key=')) { + continue + } + outputArgs.push(arg) +} + +const child = spawn(realBinary, outputArgs, { + cwd: process.cwd(), + env: process.env, + stdio: 'inherit' +}) + +child.on('error', (error) => { + console.error('[pear-broker-compat] failed to launch broker:', error instanceof Error ? error.message : String(error)) + process.exit(127) +}) + +child.on('exit', (code, signal) => { + if (signal) { + try { + process.kill(process.pid, signal) + } catch { + // Re-raising the child's signal failed; exit non-zero so the failure still + // propagates to the parent. Nothing to log from inside this shim process. + process.exit(1) + } + return + } + process.exit(code ?? 0) +}) +` +} + +async function textFileMatches(filePath: string, source: string): Promise { + try { + return await readFile(filePath, 'utf8') === source + } catch { + return false + } +} + +async function ensureBrokerBinaryCompatShim(): Promise { + const shimDir = join(app.getPath('userData'), 'broker-compat') + const shimPath = join(shimDir, process.platform === 'win32' ? 'agent-relay-broker-compat.cmd' : 'agent-relay-broker-compat') + const source = process.platform === 'win32' + ? `@echo off\r\nnode "%~dp0agent-relay-broker-compat.js" %*\r\n` + : brokerBinaryCompatShimSource() + + await mkdir(shimDir, { recursive: true }) + if (!(await textFileMatches(shimPath, source))) { + await writeFile(shimPath, source) + await chmod(shimPath, 0o755) + } + if (process.platform === 'win32') { + const jsPath = join(shimDir, 'agent-relay-broker-compat.js') + const jsSource = brokerBinaryCompatShimSource() + if (!(await textFileMatches(jsPath, jsSource))) { + await writeFile(jsPath, jsSource) + } + } + return shimPath +} + +// `binaryPath` is what Pear launches (possibly the legacy compat shim); +// `realBinaryPath` is always the actual broker binary, for callers that hand +// the binary itself to other processes. +export async function resolveHarnessBrokerBinary(workspaceKey?: string): Promise<{ binaryPath: string; realBinaryPath: string; env: NodeJS.ProcessEnv }> { + const binaryPath = resolveBundledBrokerBinary() + const flags = await inspectBrokerInitCliFlags(binaryPath) + + if (workspaceKey && !flags.supportsWorkspaceKey) { + throw new Error( + `Broker binary does not support --workspace-key: ${binaryPath}. Rebuild or update agent-relay-broker, or unset AGENT_RELAY_WORKSPACE_KEY.` + ) + } + + if (flags.supportsInstanceName) { + return { binaryPath, realBinaryPath: binaryPath, env: {} } + } + + if (!flags.supportsName) { + throw new Error(`Broker binary supports neither --instance-name nor --name: ${binaryPath}`) + } + + const shimPath = await ensureBrokerBinaryCompatShim() + console.warn(`[broker] Broker binary uses legacy --name flag; launching through compatibility shim: ${binaryPath}`) + return { + binaryPath: shimPath, + realBinaryPath: binaryPath, + env: { + PEAR_AGENT_RELAY_BROKER_BINARY: binaryPath, + PEAR_AGENT_RELAY_BROKER_SUPPORTS_INSTANCE_NAME: flags.supportsInstanceName ? '1' : '0', + PEAR_AGENT_RELAY_BROKER_SUPPORTS_WORKSPACE_KEY: flags.supportsWorkspaceKey ? '1' : '0' + } + } +} diff --git a/src/main/broker.test.ts b/src/main/broker.test.ts index 40c46d4b..f4802376 100644 --- a/src/main/broker.test.ts +++ b/src/main/broker.test.ts @@ -193,10 +193,12 @@ vi.mock('./burn', () => ({ import { BrokerManager, isCommandAvailableWithAugmentedPath, + resolveAgentRelayMcpCommand +} from './broker' +import { parseBrokerInitCliFlags, - resolveAgentRelayMcpCommand, resolveBundledBrokerBinary -} from './broker' +} from './broker-binary' import { classifyBrokerEvent, KNOWN_BROKER_EVENT_KINDS diff --git a/src/main/broker.ts b/src/main/broker.ts index 56ed99ce..8b2bd67e 100644 --- a/src/main/broker.ts +++ b/src/main/broker.ts @@ -1,5 +1,5 @@ import { existsSync, readFileSync } from 'fs' -import { chmod, mkdir, readFile, rm, writeFile } from 'fs/promises' +import { rm } from 'fs/promises' import { randomUUID } from 'node:crypto' import { homedir } from 'node:os' import { delimiter, basename, isAbsolute, join } from 'path' @@ -21,6 +21,7 @@ import { getAccessToken, getApiUrl } from './auth' import { assertDirectory } from './path-utils' import { toErrorMessage } from './errors' import { isRecord } from './guards' +import { resolveHarnessBrokerBinary } from './broker-binary' import { PtyInputStreamManager } from './pty-input-stream' import { PtyChunkDeduper } from './pty-dedup' import { @@ -308,222 +309,6 @@ async function findWorkforcePersona( } } -function resolveBrokerBinaryOverride(): string | null { - const configured = process.env.AGENT_RELAY_BIN?.trim() - if (!configured) return null - if (canExecute(configured)) return configured - console.warn('[broker] Ignoring AGENT_RELAY_BIN because it is not executable:', configured) - return null -} - -// Resolve the broker binary bundled with the v8 harness-driver runtime. -// The runtime normally resolves this via import.meta.url, but that breaks when -// electron-vite bundles the driver into the main process (import.meta.url points -// to out/main/ instead of node_modules/). -function brokerBinaryNameForPlatform(platform: NodeJS.Platform): string { - return platform === 'win32' ? 'agent-relay-broker.exe' : 'agent-relay-broker' -} - -export function resolveBundledBrokerBinary(baseDir = __dirname, isPackaged = app.isPackaged): string { - const configuredBinary = resolveBrokerBinaryOverride() - if (configuredBinary) { - console.log('[broker] Using configured Agent Relay broker binary:', configuredBinary) - return configuredBinary - } - - const suffix = `${process.platform}-${process.arch}` - const unpackIfPackaged = (binary: string): string => - isPackaged ? binary.replace('app.asar', 'app.asar.unpacked') : binary - - // v8 ships the broker as a per-platform optional package (@agent-relay/broker-*). - const brokerBinaryName = brokerBinaryNameForPlatform(process.platform) - const optionalPackageBinary = join( - baseDir, '..', '..', 'node_modules', '@agent-relay', `broker-${suffix}`, 'bin', - brokerBinaryName - ) - const unpackedOptionalPackageBinary = unpackIfPackaged(optionalPackageBinary) - if (canExecute(unpackedOptionalPackageBinary)) return unpackedOptionalPackageBinary - if (unpackedOptionalPackageBinary !== optionalPackageBinary && canExecute(optionalPackageBinary)) { - return optionalPackageBinary - } - - const localBinary = join(baseDir, '..', '..', '..', 'relay', 'target', 'debug', brokerBinaryName) - if (canExecute(localBinary)) { - console.warn('[broker] Bundled Agent Relay broker binary was not found; falling back to local relay build:', localBinary) - return localBinary - } - - // Backward-compatible fallback for SDK packages that still carry per-platform - // broker binaries directly. - const brokerBinary = join( - baseDir, '..', '..', 'node_modules', '@agent-relay', 'sdk', 'bin', - `agent-relay-broker-${suffix}${process.platform === 'win32' ? '.exe' : ''}` - ) - return unpackIfPackaged(brokerBinary) -} - -interface BrokerInitCliFlags { - supportsInstanceName: boolean - supportsName: boolean - supportsWorkspaceKey: boolean -} - -export function parseBrokerInitCliFlags(helpText: string): BrokerInitCliFlags { - return { - supportsInstanceName: /(?:^|\s)--instance-name(?:\s|[<=[,]|$)/.test(helpText), - supportsName: /(?:^|\s)--name(?:\s|[<=[,]|$)/.test(helpText), - supportsWorkspaceKey: /(?:^|\s)--workspace-key(?:\s|[<=[,]|$)/.test(helpText) - } -} - -function inspectBrokerInitCliFlags(binaryPath: string): Promise { - return new Promise((resolve) => { - execFile(binaryPath, ['init', '--help'], { - encoding: 'utf8', - timeout: 2_000, - windowsHide: true - }, (err, stdout) => { - if (err) { - console.warn(`[broker] Failed to inspect broker init CLI for ${binaryPath}:`, err) - resolve({ - supportsInstanceName: true, - supportsName: true, - supportsWorkspaceKey: true - }) - return - } - resolve(parseBrokerInitCliFlags(stdout)) - }) - }) -} - -function brokerBinaryCompatShimSource(): string { - return `#!/usr/bin/env node -const { spawn } = require('node:child_process') - -const realBinary = process.env.PEAR_AGENT_RELAY_BROKER_BINARY -if (!realBinary) { - console.error('[pear-broker-compat] PEAR_AGENT_RELAY_BROKER_BINARY is required') - process.exit(127) -} - -const supportsInstanceName = process.env.PEAR_AGENT_RELAY_BROKER_SUPPORTS_INSTANCE_NAME === '1' -const supportsWorkspaceKey = process.env.PEAR_AGENT_RELAY_BROKER_SUPPORTS_WORKSPACE_KEY === '1' -const inputArgs = process.argv.slice(2) -const outputArgs = [] - -for (let index = 0; index < inputArgs.length; index += 1) { - const arg = inputArgs[index] - if (!supportsInstanceName && arg === '--instance-name') { - outputArgs.push('--name') - if (index + 1 < inputArgs.length) outputArgs.push(inputArgs[++index]) - continue - } - if (!supportsInstanceName && arg.startsWith('--instance-name=')) { - outputArgs.push('--name=' + arg.slice('--instance-name='.length)) - continue - } - if (!supportsWorkspaceKey && arg === '--workspace-key') { - index += 1 - continue - } - if (!supportsWorkspaceKey && arg.startsWith('--workspace-key=')) { - continue - } - outputArgs.push(arg) -} - -const child = spawn(realBinary, outputArgs, { - cwd: process.cwd(), - env: process.env, - stdio: 'inherit' -}) - -child.on('error', (error) => { - console.error('[pear-broker-compat] failed to launch broker:', error instanceof Error ? error.message : String(error)) - process.exit(127) -}) - -child.on('exit', (code, signal) => { - if (signal) { - try { - process.kill(process.pid, signal) - } catch { - // Re-raising the child's signal failed; exit non-zero so the failure still - // propagates to the parent. Nothing to log from inside this shim process. - process.exit(1) - } - return - } - process.exit(code ?? 0) -}) -` -} - -async function textFileMatches(filePath: string, source: string): Promise { - try { - return await readFile(filePath, 'utf8') === source - } catch { - return false - } -} - -async function ensureBrokerBinaryCompatShim(): Promise { - const shimDir = join(app.getPath('userData'), 'broker-compat') - const shimPath = join(shimDir, process.platform === 'win32' ? 'agent-relay-broker-compat.cmd' : 'agent-relay-broker-compat') - const source = process.platform === 'win32' - ? `@echo off\r\nnode "%~dp0agent-relay-broker-compat.js" %*\r\n` - : brokerBinaryCompatShimSource() - - await mkdir(shimDir, { recursive: true }) - if (!(await textFileMatches(shimPath, source))) { - await writeFile(shimPath, source) - await chmod(shimPath, 0o755) - } - if (process.platform === 'win32') { - const jsPath = join(shimDir, 'agent-relay-broker-compat.js') - const jsSource = brokerBinaryCompatShimSource() - if (!(await textFileMatches(jsPath, jsSource))) { - await writeFile(jsPath, jsSource) - } - } - return shimPath -} - -// `binaryPath` is what Pear launches (possibly the legacy compat shim); -// `realBinaryPath` is always the actual broker binary, for callers that hand -// the binary itself to other processes. -async function resolveHarnessBrokerBinary(workspaceKey?: string): Promise<{ binaryPath: string; realBinaryPath: string; env: NodeJS.ProcessEnv }> { - const binaryPath = resolveBundledBrokerBinary() - const flags = await inspectBrokerInitCliFlags(binaryPath) - - if (workspaceKey && !flags.supportsWorkspaceKey) { - throw new Error( - `Broker binary does not support --workspace-key: ${binaryPath}. Rebuild or update agent-relay-broker, or unset AGENT_RELAY_WORKSPACE_KEY.` - ) - } - - if (flags.supportsInstanceName) { - return { binaryPath, realBinaryPath: binaryPath, env: {} } - } - - if (!flags.supportsName) { - throw new Error(`Broker binary supports neither --instance-name nor --name: ${binaryPath}`) - } - - const shimPath = await ensureBrokerBinaryCompatShim() - console.warn(`[broker] Broker binary uses legacy --name flag; launching through compatibility shim: ${binaryPath}`) - return { - binaryPath: shimPath, - realBinaryPath: binaryPath, - env: { - PEAR_AGENT_RELAY_BROKER_BINARY: binaryPath, - PEAR_AGENT_RELAY_BROKER_SUPPORTS_INSTANCE_NAME: flags.supportsInstanceName ? '1' : '0', - PEAR_AGENT_RELAY_BROKER_SUPPORTS_WORKSPACE_KEY: flags.supportsWorkspaceKey ? '1' : '0' - } - } -} - type TerminalAttachMode = 'view' | 'drive' | 'passthrough' export interface CloudAgentSandboxHandle { From 92d8b8fca877581f309c8bc26b7b17fe13583848 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 19 Jun 2026 00:10:28 +0000 Subject: [PATCH 4/4] refactor(broker): extract BrokerEvent accessors into broker-event-utils.ts The loose-union `BrokerEvent` accessors and delivery/exit/stream predicates (brokerEventString, isDeliveryEventForMessage, deliveryFailureMessage, isWorkerStreamForAgent, isAgentExitEventForAgent, brokerEventChunk, brokerEventNumber) were module-private helpers in broker.ts. Move them to src/main/broker-event-utils.ts; BrokerManager imports what it needs. The AGENT_EXIT_EVENT_KINDS constant stays private to the new module. Pure behavior-preserving move, depends only on the BrokerEvent type and guards.isRecord. Verified: npm test (122), vitest (451), typecheck, lint all pass. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01J59WsDm4zNGqhW137jEkaG --- src/main/broker-event-utils.ts | 68 ++++++++++++++++++++++++++++++++++ src/main/broker.ts | 66 +++++---------------------------- 2 files changed, 77 insertions(+), 57 deletions(-) create mode 100644 src/main/broker-event-utils.ts diff --git a/src/main/broker-event-utils.ts b/src/main/broker-event-utils.ts new file mode 100644 index 00000000..74aaff16 --- /dev/null +++ b/src/main/broker-event-utils.ts @@ -0,0 +1,68 @@ +/** + * Pure accessors and predicates over `BrokerEvent` payloads. + * + * Broker events arrive as a loose discriminated union; many optional fields + * (`reason`, `lastError`, `chunk`, numeric counters) are not declared on the + * base type, so these helpers read them through a single dynamic accessor and + * centralize the delivery/exit/stream classification used by BrokerManager. + * + * Extracted out of broker.ts: stateless, no dependency on BrokerManager. + */ +import type { BrokerEvent } from '@agent-relay/harness-driver' +import { isRecord } from './guards' + +export function brokerEventString(event: BrokerEvent, key: string): string | undefined { + const value = (event as unknown as Record)[key] + return typeof value === 'string' ? value : undefined +} + +export function isDeliveryEventForMessage( + event: BrokerEvent, + eventId: string, + targets: string[], + allowedKinds: string[] = [ + 'delivery_ack', + 'delivery_verified', + 'delivery_failed', + 'message_delivery_confirmed', + 'message_delivery_failed' + ] +): boolean { + const kind = brokerEventString(event, 'kind') + if (!allowedKinds.includes(kind || '')) return false + if (brokerEventString(event, 'event_id') !== eventId) return false + const name = brokerEventString(event, 'name') + return !name || targets.length === 0 || targets.includes(name) +} + +export function deliveryFailureMessage(event: BrokerEvent): string { + if (!isRecord(event)) return 'Broker delivery failed' + // reason/lastError are not declared on the base BrokerEvent union; read them + // through the same dynamic accessor used for other optional broker fields. + const reason = brokerEventString(event, 'reason') + const lastError = brokerEventString(event, 'lastError') + return reason || lastError || 'Broker delivery failed' +} + +export function isWorkerStreamForAgent(event: BrokerEvent, name: string): boolean { + return brokerEventString(event, 'kind') === 'worker_stream' && brokerEventString(event, 'name') === name +} + +const AGENT_EXIT_EVENT_KINDS = ['agent_exit', 'agent_exited', 'agent_released'] + +export function isAgentExitEventForAgent(event: BrokerEvent, name: string): boolean { + return ( + AGENT_EXIT_EVENT_KINDS.includes(brokerEventString(event, 'kind') || '') && + brokerEventString(event, 'name') === name + ) +} + +export function brokerEventChunk(event: BrokerEvent): string { + const value = (event as unknown as Record).chunk + return typeof value === 'string' ? value : '' +} + +export function brokerEventNumber(event: BrokerEvent, key: string): number | undefined { + const value = (event as unknown as Record)[key] + return typeof value === 'number' && Number.isFinite(value) ? value : undefined +} diff --git a/src/main/broker.ts b/src/main/broker.ts index 8b2bd67e..bf85dbe6 100644 --- a/src/main/broker.ts +++ b/src/main/broker.ts @@ -22,6 +22,15 @@ import { assertDirectory } from './path-utils' import { toErrorMessage } from './errors' import { isRecord } from './guards' import { resolveHarnessBrokerBinary } from './broker-binary' +import { + brokerEventChunk, + brokerEventNumber, + brokerEventString, + deliveryFailureMessage, + isAgentExitEventForAgent, + isDeliveryEventForMessage, + isWorkerStreamForAgent +} from './broker-event-utils' import { PtyInputStreamManager } from './pty-input-stream' import { PtyChunkDeduper } from './pty-dedup' import { @@ -423,63 +432,6 @@ async function mapWithConcurrency( return results } - -function brokerEventString(event: BrokerEvent, key: string): string | undefined { - const value = (event as unknown as Record)[key] - return typeof value === 'string' ? value : undefined -} - -function isDeliveryEventForMessage( - event: BrokerEvent, - eventId: string, - targets: string[], - allowedKinds: string[] = [ - 'delivery_ack', - 'delivery_verified', - 'delivery_failed', - 'message_delivery_confirmed', - 'message_delivery_failed' - ] -): boolean { - const kind = brokerEventString(event, 'kind') - if (!allowedKinds.includes(kind || '')) return false - if (brokerEventString(event, 'event_id') !== eventId) return false - const name = brokerEventString(event, 'name') - return !name || targets.length === 0 || targets.includes(name) -} - -function deliveryFailureMessage(event: BrokerEvent): string { - if (!isRecord(event)) return 'Broker delivery failed' - // reason/lastError are not declared on the base BrokerEvent union; read them - // through the same dynamic accessor used for other optional broker fields. - const reason = brokerEventString(event, 'reason') - const lastError = brokerEventString(event, 'lastError') - return reason || lastError || 'Broker delivery failed' -} - -function isWorkerStreamForAgent(event: BrokerEvent, name: string): boolean { - return brokerEventString(event, 'kind') === 'worker_stream' && brokerEventString(event, 'name') === name -} - -const AGENT_EXIT_EVENT_KINDS = ['agent_exit', 'agent_exited', 'agent_released'] - -function isAgentExitEventForAgent(event: BrokerEvent, name: string): boolean { - return ( - AGENT_EXIT_EVENT_KINDS.includes(brokerEventString(event, 'kind') || '') && - brokerEventString(event, 'name') === name - ) -} - -function brokerEventChunk(event: BrokerEvent): string { - const value = (event as unknown as Record).chunk - return typeof value === 'string' ? value : '' -} - -function brokerEventNumber(event: BrokerEvent, key: string): number | undefined { - const value = (event as unknown as Record)[key] - return typeof value === 'number' && Number.isFinite(value) ? value : undefined -} - function personaHarnessReadyFromOutput(output: string): boolean { return /Sandbox mount ready/i.test(output) }