Skip to content

Commit fd94cb6

Browse files
committed
fix: Event ordering
Closes #143 TIL a lot about how events are queued due to "syntactic sugar" that has more effects than I thought! The change is able to get this test passing though!
1 parent 658f76d commit fd94cb6

2 files changed

Lines changed: 356 additions & 183 deletions

File tree

src/acp.test.ts

Lines changed: 163 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -649,6 +649,147 @@ describe("Connection", () => {
649649
expect(events).toEqual(["NewSessionResponse", "SessionNotification"]);
650650
});
651651

652+
it("processes notification after response when both arrive in the same chunk", async () => {
653+
const events: string[] = [];
654+
const {
655+
promise: sessionNotification,
656+
resolve: resolveSessionNotification,
657+
} = Promise.withResolvers<void>();
658+
659+
class TestClient implements Client {
660+
async writeTextFile(
661+
_: WriteTextFileRequest,
662+
): Promise<WriteTextFileResponse> {
663+
return {};
664+
}
665+
async readTextFile(
666+
_: ReadTextFileRequest,
667+
): Promise<ReadTextFileResponse> {
668+
return { content: "test" };
669+
}
670+
async requestPermission(
671+
_: RequestPermissionRequest,
672+
): Promise<RequestPermissionResponse> {
673+
return {
674+
outcome: {
675+
outcome: "selected",
676+
optionId: "allow",
677+
},
678+
};
679+
}
680+
async sessionUpdate(_: SessionNotification): Promise<void> {
681+
events.push("SessionNotification");
682+
resolveSessionNotification();
683+
}
684+
}
685+
686+
const connection = new ClientSideConnection(
687+
() => new TestClient(),
688+
ndJsonStream(clientToAgent.writable, agentToClient.readable),
689+
);
690+
691+
const newSessionResponse = connection
692+
.newSession({ cwd: "/test", mcpServers: [] })
693+
.then((result) => {
694+
events.push("NewSessionResponse");
695+
return result;
696+
});
697+
698+
const requestReader = clientToAgent.readable.getReader();
699+
const { value: requestChunk } = await requestReader.read();
700+
requestReader.releaseLock();
701+
const { id: requestId } = JSON.parse(
702+
new TextDecoder().decode(requestChunk),
703+
);
704+
705+
const sessionId = "test-session";
706+
const writer = agentToClient.writable.getWriter();
707+
await writer.write(
708+
new TextEncoder().encode(
709+
JSON.stringify({
710+
jsonrpc: "2.0",
711+
id: requestId,
712+
result: { sessionId },
713+
}) +
714+
"\n" +
715+
JSON.stringify({
716+
jsonrpc: "2.0",
717+
method: "session/update",
718+
params: {
719+
sessionId,
720+
update: {
721+
sessionUpdate: "available_commands_update",
722+
availableCommands: [],
723+
},
724+
},
725+
}) +
726+
"\n",
727+
),
728+
);
729+
writer.releaseLock();
730+
731+
await newSessionResponse;
732+
await sessionNotification;
733+
734+
expect(events).toEqual(["NewSessionResponse", "SessionNotification"]);
735+
});
736+
737+
it("normalizes null results for known empty object responses", async () => {
738+
class TestClient implements Client {
739+
async writeTextFile(
740+
_: WriteTextFileRequest,
741+
): Promise<WriteTextFileResponse> {
742+
return {};
743+
}
744+
async readTextFile(
745+
_: ReadTextFileRequest,
746+
): Promise<ReadTextFileResponse> {
747+
return { content: "test" };
748+
}
749+
async requestPermission(
750+
_: RequestPermissionRequest,
751+
): Promise<RequestPermissionResponse> {
752+
return {
753+
outcome: {
754+
outcome: "selected",
755+
optionId: "allow",
756+
},
757+
};
758+
}
759+
async sessionUpdate(_: SessionNotification): Promise<void> {}
760+
}
761+
762+
const connection = new ClientSideConnection(
763+
() => new TestClient(),
764+
ndJsonStream(clientToAgent.writable, agentToClient.readable),
765+
);
766+
767+
const authenticateResponse = connection.authenticate({
768+
methodId: "test",
769+
});
770+
771+
const requestReader = clientToAgent.readable.getReader();
772+
const { value: requestChunk } = await requestReader.read();
773+
requestReader.releaseLock();
774+
const { id: requestId } = JSON.parse(
775+
new TextDecoder().decode(requestChunk),
776+
);
777+
778+
const writer = agentToClient.writable.getWriter();
779+
await writer.write(
780+
new TextEncoder().encode(
781+
JSON.stringify({
782+
jsonrpc: "2.0",
783+
id: requestId,
784+
result: null,
785+
}) + "\n",
786+
),
787+
);
788+
writer.releaseLock();
789+
790+
await expect(authenticateResponse).resolves.toEqual({});
791+
});
792+
652793
it("handles initialize method", async () => {
653794
// Create client
654795
class TestClient implements Client {
@@ -1307,6 +1448,28 @@ describe("Connection", () => {
13071448
).rejects.toThrow("ACP connection closed");
13081449
});
13091450

1451+
it("rejects requests issued after the connection closes with a falsy reason", async () => {
1452+
const connection = new ClientSideConnection(() => new MinimalTestClient(), {
1453+
readable: new ReadableStream<AnyMessage>({
1454+
start(controller) {
1455+
controller.error(0);
1456+
},
1457+
}),
1458+
writable: new WritableStream<AnyMessage>({
1459+
async write() {
1460+
// no-op
1461+
},
1462+
}),
1463+
});
1464+
1465+
await connection.closed;
1466+
expect(connection.signal.aborted).toBe(true);
1467+
1468+
await expect(
1469+
connection.newSession({ cwd: "/test", mcpServers: [] }),
1470+
).rejects.toBe(0);
1471+
});
1472+
13101473
it("supports removing signal event listeners", async () => {
13111474
const closeLog: string[] = [];
13121475

0 commit comments

Comments
 (0)