Skip to content

feat(sdk): typed multi-listener registry replaces on* callback fields#936

Merged
willwashburn merged 3 commits into
mainfrom
feat/sdk-lifecycle-listeners-v7
May 21, 2026
Merged

feat(sdk): typed multi-listener registry replaces on* callback fields#936
willwashburn merged 3 commits into
mainfrom
feat/sdk-lifecycle-listeners-v7

Conversation

@willwashburn
Copy link
Copy Markdown
Member

Summary

  • Breaking: @agent-relay/sdk 7.0 — AgentRelay swaps its 13 single-callback on*: EventHook<T> = null fields for a typed multi-listener registry. relay.onAgentSpawned = handler becomes const off = relay.addListener('agentSpawned', handler) (returning unsubscribe). Multiple consumers can coexist for the same event.
  • New: four call-site hooks at the SDK boundary — beforeAgentSpawn, afterAgentSpawn, beforeAgentRelease, afterAgentRelease. beforeAgentSpawn listeners may return a SpawnPatch (shallow-merged in registration order) to mutate the spawn input before the broker POST. The other three are observe-only.
  • First consumer: a Pear ↔ relay ↔ burn integration that preallocates Claude session ids via --session-id <uuid> from a beforeAgentSpawn handler and stamps the session in burn via the new writeStamp verb (shipped in burn#426, published as @relayburn/sdk@2.9.0). The Pear PR follows once this lands.

Why

The single-callback shape was OK when only the SDK itself wrote to those fields, but as soon as a second consumer wanted in (burn cost-tagging, Pear UI), they had to chain through one slot — fragile and order-dependent. Multi-listener via addListener is the standard Node EventEmitter shape and matches the existing Agent.onOutput() pattern we ship today, so this is unification, not a new concept.

The call-site beforeAgentSpawn hook specifically unlocks integrations that need to intercept the spawn input (not just observe). Burn is the concrete case — preallocating --session-id is the most reliable way to tag a Claude session — but the contract is generic: any consumer can return a SpawnPatch to amend args, channels, task, model, team, or agentToken.

What's in the change

New files

  • packages/sdk/src/event-bus.tsEventBus<E extends EventMap> with addListener (returns unsubscribe), removeListener, emit, listenerCount, listeners() snapshot. Sequential async dispatch; handler errors caught + logged so one bad listener never blocks the chain or the originating operation.
  • packages/sdk/src/lifecycle-hooks.tsAgentRelayEvents map (17 events), SpawnPatch, the four call-site context types, and BeforeAgentSpawnHandler. Type-only imports from relay.ts to avoid runtime cycle.

SDK changes

  • packages/sdk/src/relay.ts: removes the 13 on* fields, adds bus: EventBus<AgentRelayEvents> + thin addListener / removeListener methods. All 13 internal this.onX?.(...) callsites become this.bus.emit('x', ...). JSDoc + Agent exit-event references updated to the new event names.
  • packages/sdk/src/client.ts: AgentRelayClient accepts eventBus? (the facade shares its bus). spawnPty / spawnProvider build a BeforeAgentSpawnContext, fold patches through beforeAgentSpawn handlers to produce resolvedInput, POST that, then emit afterAgentSpawn with result or error plus durationMs. release emits before/after. The pre-existing 30+ line POST body construction is extracted into buildSpawnPtyBody / buildSpawnProviderBody helpers so the resolved input is serialized once.
  • packages/sdk/src/index.ts: re-exports EventBus, the event map, and all call-site context types.

Migration sweep (no behavior change)

  • packages/sdk/src/spawn-from-env.ts, workflows/runner.ts (unsubscribes tracked in unsubRelayListeners and cleared in the existing teardown block), four src/examples/ scripts.
  • scripts/spawn-reviewers.ts, tests/workflows/run-e2e-owner-review.ts.
  • packages/acp-bridge/src/acp-agent.ts, packages/openclaw/src/spawn/process.ts.
  • 14 test files under packages/sdk/src/__tests__/ and packages/sdk/src/workflows/__tests__/. Mocks that exposed onX fields are restructured to expose addListener + a local listener registry (relayListeners Map + emitRelayEvent helper) so tests can fire events at the SUT after capture.

Special-case migration

onChannelSubscribed / onChannelUnsubscribed handler signatures change from positional (agent, channels) to a single { agent, channels } object — documented in the changelog. The unsubscribe-via-addListener returns an unsub function; old relay.onX = null callsites are deleted (they no longer apply).

Docs + changelog

  • web/content/docs/event-handlers.mdx rewritten for the new API with a 6.x → 7.0 migration block. Python examples preserved unchanged.
  • [Unreleased] block in CHANGELOG.md gains a "Breaking Changes" entry and a "Migration Guidance" snippet.
  • packages/sdk/package.json bumps to 7.0.0.

Test plan

  • npx tsc -p tsconfig.json --noEmit in packages/sdk — clean production build
  • npx vitest run src/__tests__/event-bus.test.ts src/__tests__/lifecycle-hooks.test.ts — 21/21 new tests pass (10 EventBus + 11 lifecycle-hook integration via mocked transport)
  • eslint --fix via lint-staged pre-commit — clean
  • Full CI suite

Per-call options unchanged

spawnPersona({ onStart, onSuccess, onError }), onStderr on AgentRelaySpawnOptions, and Agent.onOutput(cb, opts) → unsubscribe are all unchanged — those are scoped per-call APIs (or already multi-listener), not global hooks.

Follow-up: Pear PR

A separate PR in pear consumes:

  • @agent-relay/sdk@^7.0.0 for BeforeAgentSpawnContext + addListener
  • @relayburn/sdk@^2.9.0 for writeStamp (from burn#426)

It registers a beforeAgentSpawn listener on every broker session that preallocates a UUID for Claude spawns, returns a SpawnPatch injecting --session-id <uuid>, and stamps the session with { spawner: 'pear', on_relay: 'true', spawned_by: 'direct' } so the session appears in burn summary --tags spawner=pear. That PR opens once this one is published.

🤖 Generated with Claude Code

Replaces the 13 single-callback `on*: EventHook<T> = null` fields on
`AgentRelay` with a typed multi-listener registry:

  relay.onAgentSpawned = handler;        // 6.x — one consumer wins
  // becomes
  const off = relay.addListener('agentSpawned', handler);  // 7.0

Adds four new call-site hooks that fire at the SDK boundary rather than
from broker events, so handlers can observe (and, for spawn, optionally
mutate) the spawn input before it reaches the broker:

  - `beforeAgentSpawn` — handlers may return a `SpawnPatch` that is
    shallow-merged into the input in registration order. The first
    consumer is a burn integration in Pear that preallocates Claude
    session ids via `--session-id <uuid>` so the resulting session can
    be stamped by exact id rather than via the sidecar manifest path.
  - `afterAgentSpawn` — observe-only; payload includes `resolvedInput`,
    `durationMs`, and either `result` or `error`.
  - `beforeAgentRelease` / `afterAgentRelease` — observe-only spawn-side
    counterparts for the release verb.

Channel subscribe / unsubscribe handler signatures change from
positional `(agent, channels)` args to a single `{ agent, channels }`
object payload for shape consistency with the rest of the registry.

Per-call option callbacks (`onStart` / `onSuccess` / `onError` /
`onStderr` passed as method arguments) and `Agent.onOutput()` are
unchanged — those are scoped local APIs, not global hooks.

Migration sweep covers `spawn-from-env.ts`, `workflows/runner.ts`
(unsubs tracked in `unsubRelayListeners`), `scripts/spawn-reviewers.ts`,
`tests/workflows/run-e2e-owner-review.ts`, fourteen `__tests__` files in
`packages/sdk`, `shadow.ts`, `packages/acp-bridge/src/acp-agent.ts`,
`packages/openclaw/src/spawn/process.ts`, and the four example scripts
in `src/examples`.

New types and runtime exported from `@agent-relay/sdk`:

  - `EventBus<E>`, `EventHandler<Args>`, `EventMap`
  - `AgentRelayEvents` map
  - `BeforeAgentSpawnContext` / `AfterAgentSpawnContext`
  - `BeforeAgentReleaseContext` / `AfterAgentReleaseContext`
  - `BeforeAgentSpawnHandler` / `SpawnPatch`

Tests: `packages/sdk/src/__tests__/event-bus.test.ts` (10 tests covering
add/remove, ordering, async dispatch, error isolation, snapshot safety)
and `packages/sdk/src/__tests__/lifecycle-hooks.test.ts` (11 tests
covering the four call-site hooks, patch-merge semantics, array
replace-vs-spread contract, error-path `afterAgentSpawn` shape, and
`removeListener`). Vitest run passes 21/21.

Docs: rewrites `web/content/docs/event-handlers.mdx` for the new API
and includes a 6.x → 7.0 migration block.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 21, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 7dabfd4f-f996-4c56-85d5-664abd8a2ca4

📥 Commits

Reviewing files that changed from the base of the PR and between 8334a56 and 4936b05.

📒 Files selected for processing (4)
  • CHANGELOG.md
  • packages/sdk/src/__tests__/event-bus.test.ts
  • packages/sdk/src/event-bus.ts
  • scripts/spawn-reviewers.ts

📝 Walkthrough

Walkthrough

This PR migrates the @agent-relay/sdk from single-callback event properties (on* fields) to a typed multi-listener EventBus pattern, adds lifecycle hooks (beforeAgentSpawn, afterAgentSpawn, beforeAgentRelease, afterAgentRelease) with spawn-input mutation support via SpawnPatch, and updates all test suites, examples, and integrations to use the new listener registration API.

Changes

Multi-listener Event System

Layer / File(s) Summary
EventBus implementation and test suite
packages/sdk/src/event-bus.ts, packages/sdk/src/__tests__/event-bus.test.ts
Generic EventBus class parameterized by an EventMap, supporting multi-listener registration via addListener() with unsubscribe closures, idempotent removeListener(), sequential async dispatch with error isolation, and listener snapshotting for concurrent mutation safety. Tests validate listener ordering, unsubscribe correctness, async sequencing, error handling, and variable-arity events.
Lifecycle hooks contract and event payload types
packages/sdk/src/lifecycle-hooks.ts
New file defining SpawnPatch for pre-spawn input mutations, context interfaces for beforeAgentSpawn/afterAgentSpawn and beforeAgentRelease/afterAgentRelease hooks, broker event payloads (AgentIdlePayload, AgentExitRequestedPayload, WorkerOutputPayload), standardized ChannelSubscriptionPayload (object with agent and channels), and AgentRelayEvents typed event map for bus dispatch.
AgentRelay event bus integration and emissions
packages/sdk/src/relay.ts
Transitions from single-callback on* properties to public bus: EventBus<AgentRelayEvents> with addListener()/removeListener() methods; replaces all internal callback invocations with bus.emit() for inbound messages, agent lifecycle, exit requests, worker output, idle, delivery updates, and channel subscription events. JSDoc updated to show new listener registration pattern.
AgentRelayClient lifecycle hooks, spawn patches, and event bus
packages/sdk/src/client.ts
Adds optional eventBus to client/spawn options; exposes public baseUrl and eventBus properties; adds addListener()/removeListener() facade methods; introduces runBeforeSpawn() to iterate and apply beforeAgentSpawn hooks with SpawnPatch merging in registration order; reworks spawn methods to emit afterAgentSpawn on success and failure; release emits beforeAgentRelease/afterAgentRelease with duration tracking.

Test Suite and Consumer Migration

Layer / File(s) Summary
Test mock relay infrastructure and listener registration
packages/sdk/src/__tests__/completion-pipeline.test.ts, packages/sdk/src/__tests__/e2e-owner-review.test.ts, packages/sdk/src/__tests__/resume-fallback.test.ts, packages/sdk/src/__tests__/start-from.test.ts, packages/sdk/src/__tests__/workflow-runner.test.ts, packages/sdk/src/workflows/__tests__/*
Update mock relay instances from on* callback fields to addListener() stubs; introduce mockListeners Map and emitMockEvent() helpers across test suites; mock PTY implementations emit workerOutput events through the listener registry; test setup/teardown clears listener registry between tests.
Test assertions and listener registrations
packages/sdk/src/__tests__/agent-activity.test.ts, packages/sdk/src/__tests__/facade.test.ts, packages/sdk/src/__tests__/orchestration-upgrades.test.ts, packages/sdk/src/__tests__/quickstart.test.ts, packages/sdk/src/__tests__/relay-channel-ops.test.ts
Update test cases to register event listeners via relay.addListener() instead of property assignment; adjust assertions for new payload shapes (e.g., channel events now receive { agent, channels } object instead of positional arguments).
Lifecycle hooks test suite
packages/sdk/src/__tests__/lifecycle-hooks.test.ts
New comprehensive test suite validating beforeAgentSpawn/afterAgentSpawn ordering and patch application, SpawnPatch shallow-merge semantics, error handling during spawn, context field enrichment (spawnerPid, spawnStartTs, baseUrl, kind), release hook timing, and listener removal contract.
WorkflowRunner listener cleanup
packages/sdk/src/workflows/runner.ts
Tracks Relay listeners in unsubRelayListeners array and invokes unsubscribe callbacks during shutdown; replaces all relay.on* assignments with relay.addListener() registration for workerOutput, messageReceived, agentSpawned, agentReleased, agentExited, deliveryUpdate, and agentIdle.
Integration scripts: acp-bridge, openclaw, spawn-from-env, spawn-reviewers
packages/acp-bridge/src/acp-agent.ts, packages/openclaw/src/spawn/process.ts, packages/sdk/src/spawn-from-env.ts, scripts/spawn-reviewers.ts
Switch from callback assignment to addListener() registration for relay event handling; consolidate type imports; apply minor formatting improvements.
Example scripts
packages/sdk/src/examples/demo.ts, packages/sdk/src/examples/quickstart.ts, packages/sdk/src/examples/persona-spawn.ts, packages/sdk/src/examples/ralph-loop.ts, packages/sdk/src/examples/shadow.ts, tests/workflows/run-e2e-owner-review.ts
Update event wiring from relay.on* assignments to relay.addListener() callbacks; ralph-loop also refactors polling logic for verdict detection and adds env-based config; shadow normalizes string literals to single quotes.

Public API and Documentation

Layer / File(s) Summary
Public API re-exports
packages/sdk/src/index.ts
Re-exports EventBus, EventHandler, EventMap from event-bus.js; adds new type exports from lifecycle-hooks.js: SpawnPatch, BeforeAgentSpawnContext, BeforeAgentSpawnHandler, AfterAgentSpawnContext, BeforeAgentReleaseContext, AfterAgentReleaseContext, broker event payloads, and ChannelSubscriptionPayload.
Documentation and migration guidance
CHANGELOG.md, web/content/docs/event-handlers.mdx
CHANGELOG documents breaking changes (on* removal, addListener/removeListener addition, channel payload shape), migration examples, and lifecycle hook behavior. event-handlers.mdx comprehensively documents the new multi-listener API, all event types, lifecycle hooks with code examples, pre-spawn mutation via SpawnPatch, and detailed 6.x-to-7.0 migration guidance.

Sequence Diagram

sequenceDiagram
    participant App
    participant Client as AgentRelayClient
    participant EventBus
    participant Relay as AgentRelay
    participant Broker

    App->>Client: addListener('beforeAgentSpawn', hook)
    Client->>EventBus: addListener hook registered
    App->>Client: spawnPty(name, task, options)
    Client->>EventBus: emit('beforeAgentSpawn', context)
    EventBus->>App: invoke hook, return SpawnPatch
    Client->>Client: merge SpawnPatch into resolvedInput
    Client->>Broker: POST /api/spawn with resolvedInput
    Broker-->>Client: success (Agent)
    Client->>EventBus: emit('afterAgentSpawn', context, result)
    EventBus->>App: invoke afterAgentSpawn listeners
    
    App->>Relay: addListener('agentExited', handler)
    Relay->>EventBus: addListener handler registered
    Broker-->>Relay: agent exited event
    Relay->>EventBus: emit('agentExited', agent)
    EventBus->>App: invoke handler with Agent
    
    App->>Client: release(name, reason)
    Client->>EventBus: emit('beforeAgentRelease', context)
    Client->>Broker: DELETE /api/agents/{name}
    Broker-->>Client: success
    Client->>EventBus: emit('afterAgentRelease', context, durationMs)
    EventBus->>App: invoke afterAgentRelease listeners
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • AgentWorkforce/relay#838: Modifies the spawn implementation in packages/sdk/src/client.ts; main PR adds lifecycle hooks and event-bus plumbing while the retrieved PR adds post-startup broker stdout draining, creating overlapping changes in the same core flow.
  • AgentWorkforce/relay#818: Modifies the public AgentRelay class in packages/sdk/src/relay.ts; main PR refactors the event model to EventBus while the personas PR adds persona-spawn functionality on the same class surface.

Suggested reviewers

  • khaliqgant

🐰 Hops excitedly with clipboard in paw
Event listeners bloom like carrots in spring—
No more single callbacks on the vine!
Hooks, patches, type-safe registry—
A burrow of listeners, all listening fine! 🌾✨

✨ 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 feat/sdk-lifecycle-listeners-v7

function runQualityChecks(): { passed: boolean; output: string } {
try {
const output = execSync(QUALITY_CMD, { encoding: "utf-8", stdio: "pipe" });
const output = execSync(QUALITY_CMD, { encoding: 'utf-8', stdio: 'pipe' });
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8334a56d15

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread packages/sdk/src/event-bus.ts Outdated
Comment on lines +39 to +40
if (set!.size === 0) {
this.handlers.delete(event);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Prevent stale unsubscribe from clearing newer listeners

The unsubscribe closure returned by addListener always runs this.handlers.delete(event) when its captured set is empty, even if that set is no longer the one stored for the event. If a caller unsubscribes, re-subscribes later, and then invokes the old unsubscribe again (double-cleanup is common in teardown/finally paths), this stale closure will delete the new set and silently drop active listeners for that event. Make the delete conditional on identity (e.g., only delete when this.handlers.get(event) === set).

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 potential issue.

View 5 additional findings in Devin Review.

Open in Devin Review

Comment thread scripts/spawn-reviewers.ts Outdated
Comment on lines +23 to +25
relay.addListener('agentExited', (agent: Agent, code?: number) => {
console.log(`[exited] ${agent.name} code=${code}`);
};
});
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 agentExited handler's code parameter is always undefined — exit code never printed

The agentExited event in AgentRelayEvents is typed as [Agent] (single-element tuple at packages/sdk/src/lifecycle-hooks.ts:148), so the handler receives exactly one argument. The handler at line 23 declares a second code?: number parameter that will always be undefined at runtime, causing every log line to print code=undefined instead of the actual exit code. The fix is to read agent.exitCode from the Agent object (as done correctly in packages/openclaw/src/spawn/process.ts:183).

Suggested change
relay.addListener('agentExited', (agent: Agent, code?: number) => {
console.log(`[exited] ${agent.name} code=${code}`);
};
});
relay.addListener('agentExited', (agent: Agent) => {
console.log(`[exited] ${agent.name} code=${agent.exitCode}`);
});
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
web/content/docs/event-handlers.mdx (1)

24-31: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Define a named handler before showing removeListener.

Line 27 uses myHandler without defining it in the snippet, so copy-paste users can’t apply the “or” path. Consider naming the handler up front and using it for both add/remove.

Proposed doc fix
-const off = relay.addListener('messageReceived', (msg) => {
+const myHandler = (msg) => {
   console.log(`${msg.from} -> ${msg.to}: ${msg.text}`);
-});
+};
+const off = relay.addListener('messageReceived', myHandler);
 ...
 relay.removeListener('messageReceived', myHandler);
🤖 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 `@web/content/docs/event-handlers.mdx` around lines 24 - 31, The snippet shows
removeListener('messageReceived', myHandler) but never defines myHandler; update
the example to declare a named handler function (e.g., myHandler) before
registering it so the same function reference can be used for both adding and
removing listeners (use the same named handler when calling
relay.on/relay.addListener and when calling relay.removeListener or off()).
Ensure the example also shows relay = AgentRelay() remains and that off() usage
is demonstrated with the named handler for clarity.
packages/sdk/src/shadow.ts (1)

19-28: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Fix JSDoc example: intercept is called with the wrong arity.

The example passes a third argument (msg) to intercept, but intercept only accepts (from, to). This can mislead SDK users copying the snippet.

Suggested doc fix
-  const copies = shadows.intercept(msg.from, msg.to, msg);
+  const copies = shadows.intercept(msg.from, msg.to);
🤖 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 `@packages/sdk/src/shadow.ts` around lines 19 - 28, The JSDoc example calls
shadows.intercept with three arguments but intercept only accepts (from, to);
update the example to call shadows.intercept(msg.from, msg.to) (remove the third
arg) so it matches the intercept(from, to) signature used later when iterating
over the returned copies; ensure any surrounding comments or variable names
(shadows, intercept, relay.addListener, relay.human) remain consistent with this
corrected call.
🤖 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.

Inline comments:
In `@CHANGELOG.md`:
- Line 109: Edit the CHANGELOG.md entry that reads "Detect opencode api key
completion (`#934`) (`#934`)" and remove the issue/PR references so it becomes
"Detect opencode api key completion" (remove both "(`#934`)" instances); ensure no
other PR/issue tokens remain on that bullet and save the file.
- Line 12: Change the long single CHANGELOG entry for `@agent-relay/sdk` /
`AgentRelay` into 3–5 short, impact-first bullets: one bullet describing the
listener API migration (mention `addListener(event, handler)` /
`removeListener(event, handler)` and that this replaces the old single-callback
`on*` fields like `onMessageReceived`, `onAgentSpawned`, etc., with a short
migration hint `relay.onX = handler;` → `relay.addListener('x', handler);`), one
bullet describing the channel subscribe/unsubscribe payload shape change (note
handlers now receive a single `{ agent, channels }` object instead of `(agent,
channels)`), and one bullet listing the new lifecycle hooks (`beforeAgentSpawn`,
`afterAgentSpawn`, `beforeAgentRelease`, `afterAgentRelease`) with a short note
that `beforeAgentSpawn` listeners can return a `SpawnPatch` to mutate spawn
input; drop implementation backstory and keep each bullet focused on
user-visible impact.

In `@packages/sdk/package.json`:
- Line 3: package.json was bumped to version "7.0.0" but the package-lock.json
was not regenerated, causing npm ci to fail; run npm install (or npm ci) locally
in the packages/sdk directory to regenerate and update package-lock.json so it
matches the new version and dependency tree, then commit the updated
package-lock.json alongside the package.json change (ensure `@agent-relay/sdk`
version entries in package-lock.json reflect 7.0.0).

In `@packages/sdk/src/event-bus.ts`:
- Around line 30-43: The returned off() closure in addListener currently closes
over the local `set` which becomes stale; change the off closure in
`addListener` to re-read the current set from `this.handlers` (e.g. `const cur =
this.handlers.get(event)`) and operate on `cur` (delete the handler and if
`cur.size === 0` delete the key), making the off() idempotent and avoiding
removal of a newly-registered set; update `addListener` to no longer rely on the
captured `set` and add a regression test in `event-bus.test.ts` that
double-calls the original `off()` after re-registering a listener to confirm the
new listener is not dropped.

---

Outside diff comments:
In `@packages/sdk/src/shadow.ts`:
- Around line 19-28: The JSDoc example calls shadows.intercept with three
arguments but intercept only accepts (from, to); update the example to call
shadows.intercept(msg.from, msg.to) (remove the third arg) so it matches the
intercept(from, to) signature used later when iterating over the returned
copies; ensure any surrounding comments or variable names (shadows, intercept,
relay.addListener, relay.human) remain consistent with this corrected call.

In `@web/content/docs/event-handlers.mdx`:
- Around line 24-31: The snippet shows removeListener('messageReceived',
myHandler) but never defines myHandler; update the example to declare a named
handler function (e.g., myHandler) before registering it so the same function
reference can be used for both adding and removing listeners (use the same named
handler when calling relay.on/relay.addListener and when calling
relay.removeListener or off()). Ensure the example also shows relay =
AgentRelay() remains and that off() usage is demonstrated with the named handler
for clarity.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 93ae9994-9400-4413-9ea2-df42ccba93da

📥 Commits

Reviewing files that changed from the base of the PR and between cb8cfc0 and 8334a56.

📒 Files selected for processing (38)
  • CHANGELOG.md
  • packages/acp-bridge/src/acp-agent.ts
  • packages/openclaw/src/spawn/process.ts
  • packages/sdk/package.json
  • packages/sdk/src/__tests__/agent-activity.test.ts
  • packages/sdk/src/__tests__/builder-resume-persistence.test.ts
  • packages/sdk/src/__tests__/completion-pipeline.test.ts
  • packages/sdk/src/__tests__/e2e-owner-review.test.ts
  • packages/sdk/src/__tests__/event-bus.test.ts
  • packages/sdk/src/__tests__/facade.test.ts
  • packages/sdk/src/__tests__/idle-nudge.test.ts
  • packages/sdk/src/__tests__/lifecycle-hooks.test.ts
  • packages/sdk/src/__tests__/orchestration-upgrades.test.ts
  • packages/sdk/src/__tests__/quickstart.test.ts
  • packages/sdk/src/__tests__/relay-channel-ops.test.ts
  • packages/sdk/src/__tests__/resume-fallback.test.ts
  • packages/sdk/src/__tests__/start-from.test.ts
  • packages/sdk/src/__tests__/workflow-runner.test.ts
  • packages/sdk/src/client.ts
  • packages/sdk/src/event-bus.ts
  • packages/sdk/src/examples/demo.ts
  • packages/sdk/src/examples/persona-spawn.ts
  • packages/sdk/src/examples/quickstart.ts
  • packages/sdk/src/examples/ralph-loop.ts
  • packages/sdk/src/index.ts
  • packages/sdk/src/lifecycle-hooks.ts
  • packages/sdk/src/relay.ts
  • packages/sdk/src/shadow.ts
  • packages/sdk/src/spawn-from-env.ts
  • packages/sdk/src/workflows/__tests__/budget-enforcement.test.ts
  • packages/sdk/src/workflows/__tests__/e2e-permissions.test.ts
  • packages/sdk/src/workflows/__tests__/permissions-integration.test.ts
  • packages/sdk/src/workflows/__tests__/verification-custom.test.ts
  • packages/sdk/src/workflows/__tests__/verification-traceback.test.ts
  • packages/sdk/src/workflows/runner.ts
  • scripts/spawn-reviewers.ts
  • tests/workflows/run-e2e-owner-review.ts
  • web/content/docs/event-handlers.mdx

Comment thread CHANGELOG.md Outdated
Comment thread CHANGELOG.md

#### User-Impacting Fixes

- Detect opencode api key completion (#934) (#934)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Remove issue/PR references from this changelog bullet.

Line 109 includes (#934) (#934), which should be removed per changelog style rules.
As per coding guidelines, “Drop issue/PR links… unless that text clearly explains the shipped impact.”

🤖 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 `@CHANGELOG.md` at line 109, Edit the CHANGELOG.md entry that reads "Detect
opencode api key completion (`#934`) (`#934`)" and remove the issue/PR references so
it becomes "Detect opencode api key completion" (remove both "(`#934`)"
instances); ensure no other PR/issue tokens remain on that bullet and save the
file.

Comment thread packages/sdk/package.json Outdated
Comment thread packages/sdk/src/event-bus.ts
The previous commit set @agent-relay/sdk to 7.0.0 in the PR, which
broke CI because the package-lock.json + sibling workspace package.json
files all reference 6.3.3. The publish.yml workflow takes a `version`
input (patch | minor | major | …) on workflow_dispatch and bumps every
package in lockstep, so version bumps belong in the release, not the PR
that ships the breaking change. The release-notes "Breaking Changes"
entry in CHANGELOG already covers the v7-bound migration guidance.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
EventBus.addListener — stale-closure bug (CodeRabbit + Codex P1):
the unsubscribe returned by `addListener` captured `set` by reference.
After unsubscribing, registering a new handler for the same event, and
calling the original `off` again, the closure would observe the stale
(empty) `Set` and call `this.handlers.delete(event)` — silently dropping
every fresh listener for that event.

Fix: re-read the current Set from `this.handlers` inside the closure
and bail out if it isn't the one we originally inserted. `removeListener`
already had this shape; bringing `addListener`'s unsubscribe in line.

Regression test added — covers off()→addListener→off() →
new listener must still be called on emit.

spawn-reviewers script — `agentExited` handler had a stray `code?: number`
second parameter that's always undefined under the new single-arg
listener contract. Read `agent.exitCode` from the Agent payload instead,
matching the pattern in `packages/openclaw/src/spawn/process.ts`.

CHANGELOG — split the single long `@agent-relay/sdk` breaking-change
bullet into three impact-first bullets (listener API migration,
channel-event payload shape change, new lifecycle hooks) per the repo's
changelog-style rule.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

@barryollama barryollama left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review: Approved ✅

This is a well-designed, comprehensive PR that improves the SDK's event system architecture.

Highlights:

  • EventBus implementation is robust with proper stale-closure protection and sequential async dispatch
  • Lifecycle hooks (, etc.) provide clean interception points for integrations
  • SpawnPatch design correctly narrows mutable fields to prevent identity rewriting
  • Test coverage is thorough, including edge cases like self-removal during emit
  • Migration docs are clear with before/after examples

Minor Suggestions (non-blocking):

  1. In line 50, consider adding a brief inline comment explaining why the Set identity check matters - the stale closure protection is subtle
  2. The patch-merging in registration order (later wins) is documented and tested - good

Files Reviewed:

    • Clean generic EventBus implementation
    • Well-designed SpawnPatch contract
    • Proper hook integration in spawn/release paths
    • Clean facade wrapper
    • Comprehensive test coverage
    • Good migration documentation

LGTM - ship it!

Reviewed by Hermes Agent

Copy link
Copy Markdown

@barryollama barryollama left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review: Approved ✅

This is a well-designed, comprehensive PR that improves the SDK's event system architecture.

Highlights:

  • EventBus implementation is robust with proper stale-closure protection (line 50 identity check pattern) and sequential async dispatch
  • Lifecycle hooks (beforeAgentSpawn, etc.) provide clean interception points for integrations like burn cost-tracking
  • SpawnPatch design correctly narrows mutable fields to prevent identity rewriting
  • Test coverage is thorough, including edge cases like self-removal during emit (event-bus.test.ts line 66)
  • Migration docs are clear with before/after examples

Minor Notes:

  • The patch-merging in registration order (later wins) is documented and tested
  • Handler isolation via try/catch prevents cascade failures
  • All 37 files touched are appropriate to the change
  • Channel subscription payload now uses object shape instead of positional args - good API design

One Suggestion (non-blocking):

In event-bus.ts line 50, consider adding a brief inline comment explaining why the Set identity check matters - the stale closure protection is subtle and future readers might miss it without context.

LGTM - ship it!

Reviewed by Hermes Agent

@willwashburn willwashburn merged commit f9b4839 into main May 21, 2026
50 of 51 checks passed
@willwashburn willwashburn deleted the feat/sdk-lifecycle-listeners-v7 branch May 21, 2026 19:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants