Skip to content

Commit 785a691

Browse files
MnigosMnigos
committed
fix(acp): request ids stay within signed 32-bit so Kotlin agents answer
1 parent 00eb8f6 commit 785a691

5 files changed

Lines changed: 123 additions & 4 deletions

File tree

‎packages/effect-acp/src/agent.test.ts‎

Lines changed: 91 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,7 @@ it.effect("effect-acp agent handles core agent requests and outbound client requ
218218
}),
219219
);
220220

221-
it.effect("effect-acp agent uses distinct ids for RPC calls and extension requests", () =>
221+
it.effect("effect-acp agent keeps RPC ids within int32 and apart from extension ids", () =>
222222
Effect.gen(function* () {
223223
const { stdio, input, output } = yield* makeInMemoryStdio();
224224
const scope = yield* Scope.make();
@@ -267,6 +267,11 @@ it.effect("effect-acp agent uses distinct ids for RPC calls and extension reques
267267
: yield* decodedExt(firstOutbound);
268268

269269
assert.notEqual(permissionRequest.id, extRequest.id);
270+
// Keep locally generated ids compatible with SDKs that decode signed int32.
271+
assert.typeOf(permissionRequest.id, "number");
272+
assert.equal(permissionRequest.id, 2 ** 30);
273+
assert.isAtMost(Number(permissionRequest.id), 2 ** 31 - 1);
274+
assert.equal(extRequest.id, 1);
270275

271276
yield* Queue.offer(
272277
input,
@@ -293,6 +298,91 @@ it.effect("effect-acp agent uses distinct ids for RPC calls and extension reques
293298
const permission = yield* Fiber.join(permissionFiber);
294299
assert.equal(permission.outcome.outcome, "selected");
295300
assert.deepEqual(yield* Fiber.join(extFiber), { ok: true });
301+
302+
const nextPermissionFiber = yield* agent.client
303+
.requestPermission(permissionRequest.params)
304+
.pipe(Effect.forkScoped);
305+
const nextPermissionRequest = yield* decodedPermission(yield* Queue.take(output));
306+
assert.equal(nextPermissionRequest.id, 2 ** 30 + 1);
307+
yield* Queue.offer(
308+
input,
309+
yield* encodeJsonl(RequestPermissionResponse, {
310+
jsonrpc: "2.0",
311+
id: nextPermissionRequest.id,
312+
result: { outcome: { outcome: "cancelled" } },
313+
}),
314+
);
315+
assert.equal((yield* Fiber.join(nextPermissionFiber)).outcome.outcome, "cancelled");
296316
}).pipe(Effect.provide(context), Effect.ensuring(Scope.close(scope, Exit.void)));
297317
}),
298318
);
319+
320+
it.effect.each([0, 2 ** 30, 2 ** 32])(
321+
"effect-acp agent answers incoming id %s independently of outbound ids",
322+
(requestId) =>
323+
Effect.gen(function* () {
324+
const { stdio, input, output } = yield* makeInMemoryStdio();
325+
const agent = yield* AcpAgent.make(stdio);
326+
const contexts = yield* Ref.make<Array<AcpProtocol.AcpRequestContext>>([]);
327+
yield* agent.handleInitialize((_request, context) =>
328+
Ref.update(contexts, (current) => [...current, context]).pipe(
329+
Effect.as({
330+
protocolVersion: 2,
331+
capabilities: {},
332+
info: { name: "mock-agent", version: "0.0.0" },
333+
}),
334+
),
335+
);
336+
337+
// Opposite directions may reuse an id even while a request is pending.
338+
const outboundFiber = yield* agent.client
339+
.requestPermission({
340+
sessionId: "session-1",
341+
title: "Allow mock action",
342+
subject: {
343+
type: "tool_call",
344+
toolCall: { toolCallId: "tool-1", title: "Allow mock action" },
345+
},
346+
options: [{ optionId: "allow", name: "Allow", kind: "allow_once" }],
347+
})
348+
.pipe(Effect.forkScoped);
349+
const outbound = yield* decodeRequestPermissionRequest(yield* Queue.take(output));
350+
351+
yield* Queue.offer(
352+
input,
353+
yield* encodeJsonl(InitializeRequest, {
354+
jsonrpc: "2.0",
355+
id: requestId,
356+
method: "initialize",
357+
params: {
358+
protocolVersion: 2,
359+
capabilities: {},
360+
info: { name: "test-client", version: "0.0.0" },
361+
},
362+
headers: [],
363+
}),
364+
);
365+
assert.deepEqual(yield* decodeInitializeResponse(yield* Queue.take(output)), {
366+
jsonrpc: "2.0",
367+
id: requestId,
368+
result: {
369+
protocolVersion: 2,
370+
capabilities: {},
371+
info: { name: "mock-agent", version: "0.0.0" },
372+
},
373+
});
374+
assert.deepEqual(yield* Ref.get(contexts), [
375+
{ requestId: `$t3:jsonrpc:number:${requestId}`, method: "initialize" },
376+
]);
377+
378+
yield* Queue.offer(
379+
input,
380+
yield* encodeJsonl(RequestPermissionResponse, {
381+
jsonrpc: "2.0",
382+
id: outbound.id,
383+
result: { outcome: { outcome: "cancelled" } },
384+
}),
385+
);
386+
assert.equal((yield* Fiber.join(outboundFiber)).outcome.outcome, "cancelled");
387+
}),
388+
);

‎packages/effect-acp/src/agent.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -427,7 +427,7 @@ export const make = Effect.fn("effect-acp/AcpAgent.make")(function* (
427427
Effect.forkScoped,
428428
);
429429

430-
let nextRpcRequestId = 2 ** 32;
430+
let nextRpcRequestId = AcpProtocol.RPC_REQUEST_ID_START;
431431
const rpc = yield* RpcClient.make(AcpRpcs.ClientRpcs, {
432432
generateRequestId: () => RpcMessage.RequestId(nextRpcRequestId++),
433433
}).pipe(Effect.provideService(RpcClient.Protocol, transport.clientProtocol));

‎packages/effect-acp/src/client.test.ts‎

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -743,7 +743,7 @@ it.layer(NodeServices.layer)("effect-acp client", (it) => {
743743
}),
744744
);
745745

746-
it.effect("uses distinct ids for RPC calls and extension requests", () =>
746+
it.effect("keeps RPC ids within int32 and apart from extension ids", () =>
747747
Effect.gen(function* () {
748748
const { stdio, input, output } = yield* makeInMemoryStdio();
749749
const scope = yield* Scope.make();
@@ -784,6 +784,11 @@ it.layer(NodeServices.layer)("effect-acp client", (it) => {
784784
: yield* decodedExt(firstOutbound);
785785

786786
assert.notEqual(initializeRequest.id, extRequest.id);
787+
// Keep locally generated ids compatible with SDKs that decode signed int32.
788+
assert.typeOf(initializeRequest.id, "number");
789+
assert.equal(initializeRequest.id, 2 ** 30);
790+
assert.isAtMost(Number(initializeRequest.id), 2 ** 31 - 1);
791+
assert.equal(extRequest.id, 1);
787792

788793
yield* Queue.offer(
789794
input,
@@ -811,6 +816,23 @@ it.layer(NodeServices.layer)("effect-acp client", (it) => {
811816

812817
yield* Fiber.join(initializeFiber);
813818
assert.deepEqual(yield* Fiber.join(extFiber), { ok: true });
819+
820+
const sessionFiber = yield* acp.agent
821+
.createSession({ cwd: "/tmp", mcpServers: [] })
822+
.pipe(Effect.forkScoped);
823+
const sessionRequest = yield* Schema.decodeEffect(
824+
Schema.fromJsonString(jsonRpcRequest("session/new", AcpSchema.NewSessionRequest)),
825+
)(yield* Queue.take(output));
826+
assert.equal(sessionRequest.id, 2 ** 30 + 1);
827+
yield* Queue.offer(
828+
input,
829+
yield* encodeJsonl(jsonRpcResponse(AcpSchema.NewSessionResponse), {
830+
jsonrpc: "2.0",
831+
id: sessionRequest.id,
832+
result: { sessionId: "session-1" },
833+
}),
834+
);
835+
assert.equal((yield* Fiber.join(sessionFiber)).sessionId, "session-1");
814836
yield* Scope.close(scope, Exit.void);
815837
}),
816838
);

‎packages/effect-acp/src/client.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1105,7 +1105,7 @@ export const make = Effect.fn("effect-acp/AcpClient.make")(function* (
11051105
Effect.forkScoped,
11061106
);
11071107

1108-
let nextRpcRequestId = 2 ** 32;
1108+
let nextRpcRequestId = AcpProtocol.RPC_REQUEST_ID_START;
11091109
const rpc = yield* RpcClient.make(AcpRpcs.CompatAgentRpcs, {
11101110
generateRequestId: () => RpcMessage.RequestId(nextRpcRequestId++),
11111111
}).pipe(Effect.provideService(RpcClient.Protocol, transport.clientProtocol));

‎packages/effect-acp/src/protocol.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,13 @@ const encodeJsonRpcNotification = Schema.encodeUnknownExit(
156156
),
157157
);
158158

159+
/**
160+
* First id for typed Effect RPC requests. Starts within signed int32 for SDKs that reject
161+
* larger numeric ids (e.g. the Kotlin ACP SDK decodes ids as `Int`), while staying far
162+
* above extension ids, which count up from 1.
163+
*/
164+
export const RPC_REQUEST_ID_START = 2 ** 30;
165+
159166
const isEffectRpcRequestId = (requestId: AcpError.AcpRequestId): boolean =>
160167
typeof requestId === "number" && Number.isSafeInteger(requestId);
161168

0 commit comments

Comments
 (0)