Skip to content

Commit 966a475

Browse files
willwashburnclaude
andcommitted
Address review: validate acceptance, sync handlers, spawn input type
- All three clients hard-fail registration when the reply omits a well-formed accepted_capabilities list (a missing/corrupt list means acceptance can't be confirmed); a malformed acceptance entry counts as rejected (fail-safe). Swift previously dropped malformed entries and could register on a corrupt reply. - Python handlers may be sync or async (awaited only when awaitable), matching the TypeScript client. - Swift spawnAgent takes [String: JSONValue] so callers can't build a node.spawn frame the engine's object-typed input schema rejects. - Strengthen the reconnect test to complete re-registration after the drop. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 2b825ec commit 966a475

6 files changed

Lines changed: 158 additions & 26 deletions

File tree

packages/sdk-python/src/relay_sdk/node.py

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
import asyncio
1616
import contextlib
17+
import inspect
1718
import json
1819
import logging
1920
import random
@@ -130,7 +131,8 @@ class _RegisteredCapability:
130131
handler: "NodeCapabilityHandler"
131132

132133

133-
NodeCapabilityHandler = Callable[[Any, "NodeHandlerContext"], Awaitable[Any]]
134+
# A handler may be async (returning an awaitable) or a plain sync callable.
135+
NodeCapabilityHandler = Callable[[Any, "NodeHandlerContext"], Awaitable[Any] | Any]
134136
NodeCapabilitySpec = NodeCapabilityHandler | Mapping[str, Any]
135137

136138

@@ -469,11 +471,21 @@ async def _send_register(self) -> None:
469471
frame["machine_id"] = self._machine_id
470472

471473
data = await self._request(req_id, frame)
472-
accepted = (data or {}).get("accepted_capabilities") or []
473-
rejected = [c for c in accepted if not c.get("accepted")]
474+
# The reply must carry a well-formed acceptance list (the engine always
475+
# does). A missing/corrupt list means acceptance can't be confirmed, so
476+
# fail registration rather than assume success; an entry missing
477+
# `accepted` counts as rejected (fail-safe).
478+
accepted = (data or {}).get("accepted_capabilities")
479+
if not isinstance(accepted, list):
480+
raise NodeRegistrationError(
481+
"Registration reply is missing accepted_capabilities", "invalid_register_reply"
482+
)
483+
rejected = [c for c in accepted if not (isinstance(c, dict) and c.get("accepted") is True)]
474484
if rejected:
475485
detail = ", ".join(
476-
f"{c.get('name')}" + (f" ({c['reason']})" if c.get("reason") else "") for c in rejected
486+
f"{c.get('name') if isinstance(c, dict) else '<unknown>'}"
487+
+ (f" ({c['reason']})" if isinstance(c, dict) and c.get("reason") else "")
488+
for c in rejected
477489
)
478490
raise NodeRegistrationError(f"Capabilities rejected: {detail}", "capabilities_rejected")
479491

@@ -540,7 +552,9 @@ async def _dispatch_invoke(self, invocation_id: str, action: str, input_: Any) -
540552
return
541553
ctx = NodeHandlerContext(self, invocation_id)
542554
try:
543-
output = await cap.handler(input_, ctx)
555+
# Support both async and plain (sync) handlers.
556+
result = cap.handler(input_, ctx)
557+
output = await result if inspect.isawaitable(result) else result
544558
except Exception as exc: # a handler throw becomes an error result; never dropped
545559
await self._send_frame(
546560
{"type": "action.result", "invocation_id": invocation_id, "error": str(exc)}

packages/sdk-python/tests/test_node.py

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,45 @@ async def handler(input, ctx):
230230
await asyncio.wait_for(task, timeout=1.0)
231231

232232

233+
@pytest.mark.asyncio
234+
async def test_invoke_supports_a_sync_handler():
235+
server = FakeNodeServer()
236+
node = NodeProvider(**base_kwargs(server))
237+
238+
@node.capability("run-etl")
239+
def handler(input, ctx): # a plain (non-async) handler
240+
return {"echoed": input}
241+
242+
task = asyncio.create_task(node.serve())
243+
conn = await server.next_connection()
244+
conn.push(accept_all(conn.last_register()))
245+
await node.wait_registered()
246+
247+
conn.push({"v": 1, "type": "action.invoke", "invocation_id": "inv-sync", "action": "run-etl", "input": {"rows": 2}})
248+
await wait_until(lambda: len(conn.sent_of_type("action.result")) == 1)
249+
result = conn.sent_of_type("action.result")[-1]
250+
assert result["output"] == {"echoed": {"rows": 2}}
251+
252+
await node.stop()
253+
await asyncio.wait_for(task, timeout=1.0)
254+
255+
256+
@pytest.mark.asyncio
257+
async def test_registration_missing_accepted_capabilities_raises():
258+
server = FakeNodeServer()
259+
node = NodeProvider(**base_kwargs(server))
260+
261+
task = asyncio.create_task(node.serve())
262+
conn = await server.next_connection()
263+
register = conn.last_register()
264+
# A reply that confirms nothing about capability acceptance is not success.
265+
conn.push({"v": 1, "id": register["id"], "type": "reply", "ok": True, "data": {"provider": register["provider"]}})
266+
with pytest.raises(NodeRegistrationError):
267+
await asyncio.wait_for(node.wait_registered(), timeout=1.0)
268+
with pytest.raises(NodeRegistrationError):
269+
await asyncio.wait_for(task, timeout=1.0)
270+
271+
233272
@pytest.mark.asyncio
234273
async def test_handler_throw_becomes_error_result_never_dropped():
235274
server = FakeNodeServer()

packages/sdk-swift/Sources/Relaycast/NodeProvider.swift

Lines changed: 27 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -54,13 +54,13 @@ public struct NodeHandlerContext: Sendable {
5454
public let invocationID: String
5555

5656
private let sendMessageImpl: @Sendable (String, String, String, NodeMessageMode?, [String: JSONValue]?) async throws -> JSONValue
57-
private let spawnAgentImpl: @Sendable (JSONValue) async throws -> JSONValue
57+
private let spawnAgentImpl: @Sendable ([String: JSONValue]) async throws -> JSONValue
5858

5959
init(
6060
node: NodeInfo,
6161
invocationID: String,
6262
sendMessageImpl: @escaping @Sendable (String, String, String, NodeMessageMode?, [String: JSONValue]?) async throws -> JSONValue,
63-
spawnAgentImpl: @escaping @Sendable (JSONValue) async throws -> JSONValue
63+
spawnAgentImpl: @escaping @Sendable ([String: JSONValue]) async throws -> JSONValue
6464
) {
6565
self.node = node
6666
self.invocationID = invocationID
@@ -83,9 +83,10 @@ public struct NodeHandlerContext: Sendable {
8383

8484
/// Spawn an agent through the node's capacity executor. Capacity-direct: it
8585
/// never re-enters action dispatch, so a `spawn:<harness>` shadow handler
86-
/// that delegates cannot recurse into itself. Resolves with the spawn
87-
/// placement.
88-
public func spawnAgent(_ input: JSONValue) async throws -> JSONValue {
86+
/// that delegates cannot recurse into itself. The spawn spec is a JSON
87+
/// object (the engine's `node.spawn` input is object-typed). Resolves with
88+
/// the spawn placement.
89+
public func spawnAgent(_ input: [String: JSONValue]) async throws -> JSONValue {
8990
try await spawnAgentImpl(input)
9091
}
9192
}
@@ -109,25 +110,33 @@ public struct NodeRegisterReplyData: Equatable, Sendable {
109110
public let raw: JSONValue
110111

111112
static func parse(_ json: JSONValue) throws -> NodeRegisterReplyData {
113+
// A malformed reply is a deterministic registration failure (thrown as a
114+
// NodeRegistrationError so it hard-fails rather than being retried as a
115+
// transient drop).
112116
guard case .object(let obj) = json,
113117
case .object(let providerObj)? = obj["provider"],
114118
case .string(let name)? = providerObj["name"],
115119
case .string(let instanceID)? = providerObj["instance_id"]
116120
else {
117-
throw NodeProviderError.invalidRegisterReply
121+
throw NodeRegistrationError(code: "invalid_register_reply", message: "node.register reply is malformed")
118122
}
119123

120-
var acceptedCapabilities: [NodeCapabilityAcceptance] = []
121-
if case .array(let items)? = obj["accepted_capabilities"] {
122-
acceptedCapabilities = items.compactMap { item -> NodeCapabilityAcceptance? in
123-
guard case .object(let capObj) = item,
124-
case .string(let capName)? = capObj["name"],
125-
case .bool(let accepted)? = capObj["accepted"]
126-
else { return nil }
127-
let kind: String = { if case .string(let k)? = capObj["kind"] { return k } else { return "action" } }()
128-
let reason: String? = { if case .string(let r)? = capObj["reason"] { return r } else { return nil } }()
129-
return NodeCapabilityAcceptance(name: capName, kind: kind, accepted: accepted, reason: reason)
124+
// The reply must carry a well-formed acceptance list (the engine always
125+
// does). A missing/corrupt list means acceptance can't be confirmed, so
126+
// fail registration rather than assume success.
127+
guard case .array(let items)? = obj["accepted_capabilities"] else {
128+
throw NodeRegistrationError(code: "invalid_register_reply", message: "node.register reply is missing accepted_capabilities")
129+
}
130+
let acceptedCapabilities: [NodeCapabilityAcceptance] = items.map { item -> NodeCapabilityAcceptance in
131+
// A malformed entry can't be confirmed accepted — count it as
132+
// rejected (fail-safe) rather than dropping it.
133+
guard case .object(let capObj) = item, case .string(let capName)? = capObj["name"] else {
134+
return NodeCapabilityAcceptance(name: "<unknown>", kind: "action", accepted: false, reason: "malformed acceptance entry")
130135
}
136+
let accepted: Bool = { if case .bool(let a)? = capObj["accepted"] { return a } else { return false } }()
137+
let kind: String = { if case .string(let k)? = capObj["kind"] { return k } else { return "action" } }()
138+
let reason: String? = { if case .string(let r)? = capObj["reason"] { return r } else { return nil } }()
139+
return NodeCapabilityAcceptance(name: capName, kind: kind, accepted: accepted, reason: reason)
131140
}
132141

133142
return NodeRegisterReplyData(providerName: name, instanceID: instanceID, acceptedCapabilities: acceptedCapabilities, raw: json)
@@ -682,12 +691,12 @@ public actor NodeProvider {
682691
return try await request(payload)
683692
}
684693

685-
private func spawnAgentFrame(_ input: JSONValue) async throws -> JSONValue {
694+
private func spawnAgentFrame(_ input: [String: JSONValue]) async throws -> JSONValue {
686695
// Capacity-direct: the engine always targets this connection's own node,
687696
// so the frame carries no node target.
688697
try await request([
689698
"type": .string("node.spawn"),
690-
"input": input
699+
"input": .object(input)
691700
])
692701
}
693702

packages/sdk-swift/Tests/RelaycastTests/NodeProviderTests.swift

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,47 @@ final class NodeProviderTests: XCTestCase {
216216
}
217217
}
218218

219+
func testHardFailsRegistrationWhenReplyOmitsAcceptedCapabilities() async throws {
220+
let factory = FakeTransportFactory()
221+
let node = NodeProvider(
222+
nodeToken: baseOptions.nodeToken,
223+
nodeID: baseOptions.nodeID,
224+
nodeName: baseOptions.nodeName,
225+
provider: (name: "py", instanceID: nil),
226+
transport: { factory.make() }
227+
)
228+
await node.capability("run-etl") { input, _ in input }
229+
230+
let serveTask = Task { try await node.serve() }
231+
232+
await waitUntil { factory.instances.count == 1 }
233+
let transport = factory.instances[0]
234+
await waitUntil { await !transport.sentOfType("node.register").isEmpty }
235+
let register = await transport.lastRegister()!
236+
237+
// A reply that confirms nothing about capability acceptance is not success.
238+
await transport.emit([
239+
"id": register["id"] ?? .null,
240+
"type": .string("reply"),
241+
"ok": .bool(true),
242+
"data": .object(["provider": register["provider"] ?? .object([:])])
243+
])
244+
245+
do {
246+
_ = try await node.waitRegistered()
247+
XCTFail("expected waitRegistered() to throw")
248+
} catch let error as NodeRegistrationError {
249+
XCTAssertEqual(error.code, "invalid_register_reply")
250+
}
251+
252+
do {
253+
try await serveTask.value
254+
XCTFail("expected serve() to throw")
255+
} catch {
256+
// expected
257+
}
258+
}
259+
219260
func testHardFailsRegistrationOnEngineErrorFrame() async throws {
220261
let factory = FakeTransportFactory()
221262
let node = NodeProvider(
@@ -450,7 +491,7 @@ final class NodeProviderTests: XCTestCase {
450491
transport: { factory.make() }
451492
)
452493
await node.capability("spawn:claude") { _, ctx in
453-
try await ctx.spawnAgent(.object(["cli": .string("claude"), "name": .string("worker-1")]))
494+
try await ctx.spawnAgent(["cli": .string("claude"), "name": .string("worker-1")])
454495
}
455496

456497
let serveTask = Task { try await node.serve() }

packages/sdk-typescript/src/__tests__/node-provider.test.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -238,6 +238,23 @@ describe('NodeProviderClient', () => {
238238
const secondInstance = (secondRegister.provider as { instance_id: string }).instance_id;
239239
expect(secondRegister).toMatchObject({ name: 'alpha', provider: { name: 'py' } });
240240
expect(secondInstance).not.toBe(firstInstance);
241+
242+
// The reconnected registration completes its lifecycle: accepting it brings
243+
// the provider back to connected.
244+
second.emit(acceptAll(secondRegister));
245+
await vi.waitFor(() => expect(node.connected).toBe(true));
246+
});
247+
248+
it('hard-fails registration when the reply omits accepted_capabilities', async () => {
249+
const node = new NodeProviderClient({ ...baseOptions, capabilities: { 'run-etl': async () => 'ok' } });
250+
node.serve().catch(() => {});
251+
const sock = newSocket();
252+
sock.open();
253+
const register = sock.lastRegister();
254+
// A reply that confirms nothing about capability acceptance must not be
255+
// treated as success.
256+
sock.emit({ v: 1, id: register.id, type: 'reply', ok: true, data: { provider: register.provider } });
257+
await expect(node.whenRegistered()).rejects.toMatchObject({ code: 'invalid_register_reply' });
241258
});
242259

243260
it('reconnects after a transport drop during the register handshake', async () => {

packages/sdk-typescript/src/node-provider.ts

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -350,9 +350,21 @@ export class NodeProviderClient {
350350
}
351351

352352
private onRegistered(data: NodeRegisterReplyData): void {
353-
const rejected = (data.accepted_capabilities ?? []).filter((c: FleetCapabilityAcceptance) => !c.accepted);
353+
// The reply must carry a well-formed acceptance list (the engine always
354+
// does). A missing/corrupt list means acceptance can't be confirmed, so
355+
// fail registration rather than assume success; an entry missing `accepted`
356+
// counts as rejected (fail-safe).
357+
const list = data.accepted_capabilities;
358+
if (!Array.isArray(list)) {
359+
this.onRegisterFailed(new NodeRegistrationError(
360+
'Registration reply is missing accepted_capabilities',
361+
'invalid_register_reply',
362+
));
363+
return;
364+
}
365+
const rejected = list.filter((c: FleetCapabilityAcceptance) => !c || c.accepted !== true);
354366
if (rejected.length > 0) {
355-
const detail = rejected.map((c) => `${c.name}${c.reason ? ` (${c.reason})` : ''}`).join(', ');
367+
const detail = rejected.map((c) => `${c?.name ?? '<unknown>'}${c?.reason ? ` (${c.reason})` : ''}`).join(', ');
356368
this.onRegisterFailed(new NodeRegistrationError(`Capabilities rejected: ${detail}`, 'capabilities_rejected'));
357369
return;
358370
}

0 commit comments

Comments
 (0)