sdk: fail fast when spawn manager is disabled by willwashburn · Pull Request #387 · AgentWorkforce/relay

willwashburn · 2026-02-08T01:19:12Z

Summary

make RelayClient reject pending spawn/release/send-input/list-workers operations immediately when daemon returns ERROR with "SpawnManager not enabled"
surface server ERROR frames through onError so callers can handle daemon-side failures directly
add SDK client regression test to ensure spawn promise rejects quickly instead of timing out

Why

In standalone mode with spawn manager disabled, client.spawn() currently waits until timeout even though the daemon has already returned an explicit error. This change is non-breaking and improves developer feedback/latency.

Validation

npm --prefix ~/Projects/relay/packages/sdk test
all tests pass (77/77)

devin-ai-integration

Devin Review found 1 potential issue.

View 4 additional findings in Devin Review.

packages/sdk/src/client.ts

Copilot

Pull request overview

Updates the Node SDK RelayClient to fail fast on daemon-side “SpawnManager not enabled” errors and to propagate server ERROR frames to SDK consumers via onError, with added regression tests to prevent spawn calls from timing out unnecessarily in this scenario.

Changes:

Detect “SpawnManager not enabled” daemon ERROR frames and immediately reject pending spawn/release/send-input/list-workers operations.
Invoke RelayClient.onError for server-side ERROR frames.
Add tests verifying spawn() rejects quickly and clears pending spawn/ready bookkeeping when SpawnManager is disabled.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File	Description
`packages/sdk/src/client.ts`	Adds daemon-error detection to fail fast on disabled SpawnManager and surfaces server errors via `onError`; introduces `clearPendingAgentReady()`.
`packages/sdk/src/client.test.ts`	Adds regression tests ensuring spawn rejects promptly and pending state is cleared when SpawnManager is disabled.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-08T02:54:12Z

packages/sdk/src/client.ts

+      this.rejectPendingReleases(err);
+      this.rejectPendingSendInputs(err);
+      this.rejectPendingListWorkers(err);
+      this.clearPendingAgentReady();


clearPendingAgentReady() clears all pendingAgentReady entries when the daemon reports SpawnManager is disabled. This can break unrelated waitForAgentReady() callers by removing their waiter and cancelling its timeout, leaving the returned promise permanently pending. Prefer cleaning up only the waiter(s) associated with the rejected spawn(s), or reject the relevant pendingAgentReady entries with the same error while ensuring any internal, un-awaited ready promises from spawn(waitForReady) won’t produce unhandled rejections (e.g., by handling cleanup in spawn() when the spawn promise rejects).

Suggested change

this.clearPendingAgentReady();

Copilot · 2026-02-08T02:54:12Z

packages/sdk/src/client.ts

+    for (const [agentName, pending] of this.pendingAgentReady.entries()) {
+      clearTimeout(pending.timeoutHandle);
+      this.pendingAgentReady.delete(agentName);
+    }
+  }
+
  private scheduleReconnect(): void {
    this.setState('BACKOFF');


clearPendingAgentReady() deletes entries without calling pending.reject(...). If any consumer is awaiting waitForAgentReady(), this will leave their promise hanging forever (and the timeout has been cleared). Consider removing this helper and using rejectPendingAgentReady(err) (or otherwise settling the promises) so callers deterministically receive an error instead of a never-resolving promise.

Suggested change

for (const [agentName, pending] of this.pendingAgentReady.entries()) {

clearTimeout(pending.timeoutHandle);

this.pendingAgentReady.delete(agentName);

}

}

private scheduleReconnect(): void {

this.setState('BACKOFF');

this.rejectPendingAgentReady(new Error('Pending agent readiness wait was cleared.'));

}

private scheduleReconnect(): void {

this.setState('BACKOFF');

private scheduleReconnect(): void {

this.setState('BACKOFF');

Copilot · 2026-02-08T02:54:13Z

packages/sdk/src/client.ts

+    // Surface server-side ERROR frames to SDK consumers.
+    if (this.onError) {
+      this.onError(new Error(errorMessage));
+    }


onError is now used to surface server ERROR frames, but the callback only receives a generic Error with the message, losing structured context like code, fatal, and the server frame id. To let callers handle daemon-side failures “directly”, consider attaching the ErrorPayload (and possibly the full envelope) via Error’s cause option or a dedicated error class with code/fatal fields.

Copilot · 2026-02-08T02:54:13Z

packages/sdk/src/client.test.ts

+    it('rejects pending spawn when SpawnManager is disabled', async () => {
+      const client = new RelayClient({ reconnect: false, quiet: true });
+      (client as any)._state = 'READY';
+      const sendMock = vi.fn().mockReturnValue(true);
+      (client as any).send = sendMock;
+
+      const spawnPromise = client.spawn(
+        {
+          name: 'Worker',
+          cli: 'codex',
+          task: 'Do work',
+        },
+        10000
+      );
+
+      const errorEnvelope: Envelope<ErrorPayload> = {
+        v: 1,
+        type: 'ERROR',
+        id: 'err-spawn-disabled',
+        ts: Date.now(),
+        payload: {
+          code: 'INTERNAL' as any,
+          message: 'SpawnManager not enabled. Configure spawnManager: true in daemon config.',
+          fatal: false,
+        },
+      };
+
+      (client as any).processFrame(errorEnvelope);
+
+      await expect(spawnPromise).rejects.toThrow('SpawnManager not enabled');
+      expect((client as any).pendingSpawns.size).toBe(0);
+    });


New behavior surfaces server ERROR frames via onError, but there’s no regression test asserting that onError is invoked (and ideally that it exposes enough context to be actionable). Adding a focused test here would help prevent accidental removal/behavior drift of this new public-facing signal.

khaliqgant

Looks good. Devin comments look worth addressing imo

willwashburn · 2026-02-08T20:38:04Z

@copilot open a new pull request to apply changes based on the comments in this thread

Copilot · 2026-02-08T20:38:13Z

@willwashburn I've opened a new pull request, #388, to work on those changes. Once the pull request is ready, I'll request review from you.

…layServerError with structured context, add onError test, handle unhandled rejections Co-authored-by: willwashburn <957608+willwashburn@users.noreply.github.com>

Co-authored-by: willwashburn <957608+willwashburn@users.noreply.github.com>

sdk: properly reject pending agent-ready promises and surface structured server errors

devin-ai-integration

Devin Review found 1 new potential issue.

⚠️ 1 issue in files not directly in the diff

⚠️ RelayServerError not exported from SDK public API, so consumers cannot type-check it (`packages/sdk/src/index.ts:176-184`)

The PR introduces RelayServerError as a structured error that is passed to onError callbacks in handleErrorFrame (packages/sdk/src/client.ts:1894-1901), but the class is never exported from the SDK's public entry point at packages/sdk/src/index.ts:176-184.

Root Cause and Impact

The onError callback (packages/sdk/src/client.ts:305) is typed as (error: Error) => void. When a server ERROR frame arrives, handleErrorFrame now creates a RelayServerError (with .code, .fatal, .envelope properties) and passes it to onError. However, packages/sdk/src/index.ts only re-exports the older error classes:

export {
  RelayError,
  DaemonNotRunningError,
  AgentNotFoundError,
  TimeoutError,
  ConnectionError,
  ChannelNotFoundError,
  SpawnError,
} from './errors.js';

RelayServerError is missing. SDK consumers who want to distinguish server-side errors from transport errors in their onError handler cannot do instanceof RelayServerError because the class is not importable from @agent-relay/sdk or @agent-relay/sdk/errors. They would need to resort to fragile checks like error.name === 'RelayServerError' or duck-typing for code/fatal properties.

Impact: Consumers receiving the new structured error cannot properly type-narrow it, undermining the stated goal of letting "callers handle daemon-side failures directly."

View 7 additional findings in Devin Review.

sdk: fail fast spawn requests on spawn manager errors

c1021c0

devin-ai-integration bot reviewed Feb 8, 2026

View reviewed changes

packages/sdk/src/client.ts Show resolved Hide resolved

sdk: clear pending agent-ready on spawn-manager errors

a78006c

willwashburn requested review from Copilot and khaliqgant February 8, 2026 02:50

Copilot started reviewing on behalf of willwashburn February 8, 2026 02:50 View session

Copilot AI reviewed Feb 8, 2026

View reviewed changes

khaliqgant previously approved these changes Feb 8, 2026

View reviewed changes

Initial plan

1978ece

Copilot AI mentioned this pull request Feb 8, 2026

sdk: properly reject pending agent-ready promises and surface structured server errors #388

Merged

Copilot AI and others added 3 commits February 8, 2026 20:44

Address PR review comments: reject pendingAgentReady properly, add Re…

54cef38

…layServerError with structured context, add onError test, handle unhandled rejections Co-authored-by: willwashburn <957608+willwashburn@users.noreply.github.com>

Add packages/*/package-lock.json to .gitignore and remove tracked files

158dff3

Co-authored-by: willwashburn <957608+willwashburn@users.noreply.github.com>

Merge pull request #388 from AgentWorkforce/copilot/sub-pr-387

0bc1375

sdk: properly reject pending agent-ready promises and surface structured server errors

devin-ai-integration bot reviewed Feb 8, 2026

View reviewed changes

willwashburn dismissed khaliqgant’s stale review via 0bc1375 February 20, 2026 12:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

sdk: fail fast when spawn manager is disabled#387

sdk: fail fast when spawn manager is disabled#387
willwashburn wants to merge 6 commits intomainfrom
fix/sdk-standalone-spawn-defaults

willwashburn commented Feb 8, 2026 •

edited by devin-ai-integration bot

Loading

Uh oh!

devin-ai-integration bot left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 8, 2026

Uh oh!

Copilot AI Feb 8, 2026

Uh oh!

Copilot AI Feb 8, 2026

Uh oh!

Copilot AI Feb 8, 2026

Uh oh!

khaliqgant left a comment

Uh oh!

willwashburn commented Feb 8, 2026

Uh oh!

Copilot AI commented Feb 8, 2026

Uh oh!

devin-ai-integration bot left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Comments

Conversation

willwashburn commented Feb 8, 2026 • edited by devin-ai-integration bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Why

Validation

Uh oh!

devin-ai-integration bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

khaliqgant left a comment

Choose a reason for hiding this comment

Uh oh!

willwashburn commented Feb 8, 2026

Uh oh!

Copilot AI commented Feb 8, 2026

Uh oh!

devin-ai-integration bot left a comment

Choose a reason for hiding this comment

⚠️ RelayServerError not exported from SDK public API, so consumers cannot type-check it (packages/sdk/src/index.ts:176-184)

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

willwashburn commented Feb 8, 2026 •

edited by devin-ai-integration bot

Loading

⚠️ RelayServerError not exported from SDK public API, so consumers cannot type-check it (`packages/sdk/src/index.ts:176-184`)