fix(sdk): address PR #936 review feedback

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>

Author: willwashburn

CHANGELOG.md

@@ -9,7 +9,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Breaking Changes
 
-- `@agent-relay/sdk` `AgentRelay` now exposes a typed multi-listener registry — `addListener(event, handler) → unsubscribe` / `removeListener(event, handler)` — replacing the 13 single-callback `on*` fields (`onMessageReceived`, `onMessageSent`, `onAgentSpawned`, `onAgentReleased`, `onAgentExited`, `onAgentReady`, `onWorkerOutput`, `onDeliveryUpdate`, `onAgentExitRequested`, `onAgentIdle`, `onAgentActivityChanged`, `onChannelSubscribed`, `onChannelUnsubscribed`). Channel subscribe / unsubscribe handler args change from `(agent, channels)` to a single `{ agent, channels }` object payload. Migration: `relay.onX = handler;` → `relay.addListener('x', handler);`. New `beforeAgentSpawn` / `afterAgentSpawn` / `beforeAgentRelease` / `afterAgentRelease` call-site hooks fire at the SDK boundary; `beforeAgentSpawn` listeners can return a `SpawnPatch` to mutate the spawn input before POST (e.g. for burn-style session-id preallocation).
+- `@agent-relay/sdk`: `AgentRelay` events move to a multi-listener registry. Use `relay.addListener('x', handler)` / `removeListener` in place of `relay.onX = handler` — the 13 `on*` fields (`onMessageReceived`, `onMessageSent`, `onAgentSpawned`, `onAgentReleased`, `onAgentExited`, `onAgentReady`, `onWorkerOutput`, `onDeliveryUpdate`, `onAgentExitRequested`, `onAgentIdle`, `onAgentActivityChanged`, `onChannelSubscribed`, `onChannelUnsubscribed`) are removed.
+- `@agent-relay/sdk`: `channelSubscribed` / `channelUnsubscribed` handlers receive a single `{ agent, channels }` object instead of positional `(agent, channels)` args.
+- `@agent-relay/sdk`: new `beforeAgentSpawn` / `afterAgentSpawn` / `beforeAgentRelease` / `afterAgentRelease` call-site hooks. `beforeAgentSpawn` listeners may return a `SpawnPatch` (shallow-merged in registration order) to mutate the spawn input before the broker POST.
 - Broker/SDK wire protocol is now version 2 for delivery terminal events and lifecycle event shape changes.
 - `relay.spawn({ task })` now returns `success: false` and terminates the agent when task delivery fails after retries.
 - `agent-relay send` now uses the orchestrator identity by default so `agent-relay replies <worker>` can correlate worker DMs.

packages/sdk/src/__tests__/event-bus.test.ts

@@ -52,6 +52,22 @@ describe('EventBus', () => {
     expect(bus.listenerCount('ping')).toBe(0);
   });
 
+  it('returned unsubscribe is idempotent and does not clobber later registrations', async () => {
+    const bus = new EventBus<Events>();
+    const h1 = vi.fn();
+    const h2 = vi.fn();
+    const off = bus.addListener('ping', h1);
+    off();
+    bus.addListener('ping', h2);
+    // The second call to the original unsubscribe must NOT wipe the
+    // freshly-registered h2 — a stale-closure bug would do exactly that.
+    off();
+    expect(bus.listenerCount('ping')).toBe(1);
+    await bus.emit('ping', { id: 1 });
+    expect(h1).not.toHaveBeenCalled();
+    expect(h2).toHaveBeenCalledTimes(1);
+  });
+
   it('drops the event-name entry when the last listener is removed', () => {
     const bus = new EventBus<Events>();
     const a = vi.fn();

packages/sdk/src/event-bus.ts

@@ -35,8 +35,15 @@ export class EventBus<E extends EventMap> {
     }
     set.add(handler as EventHandler<readonly unknown[]>);
     return () => {
-      set!.delete(handler as EventHandler<readonly unknown[]>);
-      if (set!.size === 0) {
+      // Re-read the current Set from the map so a stale closure can't blow
+      // away the new Set if the caller unsubscribes, re-registers a fresh
+      // handler under the same event, then calls the original `off` again.
+      // Without this identity check the double-`off()` would silently drop
+      // every listener for the event.
+      const current = this.handlers.get(event);
+      if (current !== set) return;
+      current.delete(handler as EventHandler<readonly unknown[]>);
+      if (current.size === 0) {
         this.handlers.delete(event);
       }
     };

scripts/spawn-reviewers.ts

@@ -20,8 +20,8 @@ async function main() {
   relay.addListener('agentSpawned', (agent: Agent) => {
     console.log(`[spawned] ${agent.name}`);
   });
-  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 ?? 'none'}`);
   });
 
   const human = relay.human({ name: 'Human' });