refactor(main): consolidate shared helpers + decompose broker.ts

[willwashburn]

Summary

Two highest-value, lowest-risk architecture improvements, each behavior-preserving and verified green at every step:

1. Eliminate pervasive duplication — extract cross-cutting helpers into shared modules.
2. Decompose the largest god-file (broker.ts, 4282 lines) by pulling out cohesive, stateless clusters.

No BrokerManager behavior was touched — the stateful PTY/event-duplication paths AGENTS.md warns about are left intact.

Changes

Commit	What
extract shared toErrorMessage/describeError + isRecord	toErrorMessage was duplicated across 9 main-process files (8 byte-identical, 1 richer variant). New src/main/errors.ts (toErrorMessage + describeError) and src/main/guards.ts.
consolidate isRecord into shared guards module	isRecord duplicated 8× across 3 semantic variants, unified on guards.ts. The two array-including call sites are pure object-property coercions → behavior-preserving.
extract binary resolution into broker-binary.ts	~215 lines of stateless broker-binary resolution + the legacy --name compat shim moved out of broker.ts.
extract BrokerEvent accessors into broker-event-utils.ts	The 7 pure BrokerEvent predicate/accessor helpers moved to their own module.

broker.ts: 4282 → 4013 lines.

Import-extension note

Files in the npm test (Node --experimental-strip-types) runtime graph (burn.ts, integration-event-bridge.ts) use explicit ./x.ts value imports with the repo's existing // @ts-expect-error idiom; all other importers stay extensionless. This matches the pre-existing convention in integration-event-bridge.ts.

Verification (run after every step)

• npm test (node:test): 122 pass
• npx vitest run: 451 pass
• npm run typecheck: clean
• npm run lint: clean
• Each step also passed an autoreview pass (no correctness regressions).

🤖 Generated with Claude Code

---

Generated by Claude Code

[gemini-code-assist]

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

[coderabbitai]

📝 Walkthrough

Walkthrough

Four new shared modules are introduced: guards.ts (isRecord), errors.ts (toErrorMessage, describeError), broker-binary.ts (binary resolution and compat shim logic extracted from broker.ts), and broker-event-utils.ts (pure BrokerEvent accessors/predicates). Twelve consumer files drop their duplicate inline helper definitions and import from the new modules.

Changes

Shared Helper Extraction and Broker Module Split

Layer / File(s)	Summary
New shared utility modules src/main/guards.ts, src/main/errors.ts	Creates isRecord type guard and toErrorMessage/describeError error helpers as standalone shared modules.
New broker-binary module src/main/broker-binary.ts	Extracts bundled broker binary lookup, CLI flag inspection, compat shim generation/caching, and resolveHarnessBrokerBinary from broker.ts into a dedicated module.
New broker-event-utils module src/main/broker-event-utils.ts	Adds pure stateless BrokerEvent field accessors, event classifiers, and failure-message helpers as a standalone module.
broker.ts refactored to use new modules src/main/broker.ts, src/main/broker.test.ts	Removes ~282 lines of in-file binary resolution and toErrorMessage, replacing them with imports from the new modules; test imports updated to match.
Consumer files adopt shared helpers src/main/auth.ts, src/main/burn.ts, src/main/cloud-agent.ts, src/main/integration-event-bridge.ts, src/main/integration-mounts.ts, src/main/integration-symlinks.ts, src/main/integrations.ts, src/main/ipc-handlers.ts, src/main/proactive-agent.ts, src/main/spawn-coordinator.ts, src/main/store.ts	Removes duplicated local isRecord and toErrorMessage implementations from each file and adds imports from guards.ts and errors.ts; store.ts also gains normalizeRelayWorkspace using the imported guard.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• AgentWorkforce/pear#185: Directly modifies resolveBundledBrokerBinary and the AGENT_RELAY_BIN override logic that this PR extracts into broker-binary.ts.

Poem

🐇 Helpers once scattered in every file's lair,
Now live in one burrow — guards, errors there.
The broker sheds code like a coat in the spring,
Each module now carries its own little thing.
With isRecord and toErrorMessage shared at last,
The duplicate thicket is firmly in the past! 🌿

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 16.28% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and accurately summarizes the two main objectives: consolidating shared helpers and decomposing broker.ts, which are exactly what the changeset accomplishes.
Description check	✅ Passed	The description provides comprehensive, well-structured documentation of the refactoring with clear sections covering motivation, detailed changes, verification results, and implementation notes.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch claude/architecture-refactor-vzenx8

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

src/main/broker-binary.ts (1)

68-73: ⚡ Quick win

Fail fast if the SDK fallback binary is missing or not executable.

The last fallback is returned unvalidated. If that path is absent/non-executable, failure occurs later with less actionable errors. Check executability here and throw a clear error.

Suggested patch

   const brokerBinary = join(
     baseDir, '..', '..', 'node_modules', '`@agent-relay`', 'sdk', 'bin',
     `agent-relay-broker-${suffix}${process.platform === 'win32' ? '.exe' : ''}`
   )
-  return unpackIfPackaged(brokerBinary)
+  const finalCandidate = unpackIfPackaged(brokerBinary)
+  if (canExecute(finalCandidate)) return finalCandidate
+  if (finalCandidate !== brokerBinary && canExecute(brokerBinary)) return brokerBinary
+  throw new Error(
+    `[broker] Unable to locate an executable Agent Relay broker binary. Last checked: ${finalCandidate}`
+  )
 }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/main/broker-binary.ts` around lines 68 - 73, The function returns the
unpacked brokerBinary path without validating its existence or executability,
causing failures downstream with unclear error messages. After calling
unpackIfPackaged(brokerBinary), add validation to check that the binary file
exists and is executable using appropriate Node.js fs methods. If either
validation fails, throw a descriptive error that includes the full brokerBinary
path and indicates whether the file is missing or not executable, ensuring users
get actionable feedback immediately rather than encountering cryptic errors
later.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/main/broker-binary.ts`:
- Around line 68-73: The function returns the unpacked brokerBinary path without
validating its existence or executability, causing failures downstream with
unclear error messages. After calling unpackIfPackaged(brokerBinary), add
validation to check that the binary file exists and is executable using
appropriate Node.js fs methods. If either validation fails, throw a descriptive
error that includes the full brokerBinary path and indicates whether the file is
missing or not executable, ensuring users get actionable feedback immediately
rather than encountering cryptic errors later.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 569d9278-3603-4fa9-9204-0f861fa247fb

📥 Commits

Reviewing files that changed from the base of the PR and between 9212c69 and 92d8b8f.

📒 Files selected for processing (17)

• src/main/auth.ts
• src/main/broker-binary.ts
• src/main/broker-event-utils.ts
• src/main/broker.test.ts
• src/main/broker.ts
• src/main/burn.ts
• src/main/cloud-agent.ts
• src/main/errors.ts
• src/main/guards.ts
• src/main/integration-event-bridge.ts
• src/main/integration-mounts.ts
• src/main/integration-symlinks.ts
• src/main/integrations.ts
• src/main/ipc-handlers.ts
• src/main/proactive-agent.ts
• src/main/spawn-coordinator.ts
• src/main/store.ts