Skip to content

Commit 1d00058

Browse files
committed
fix(acp): avoid deadlock on permission requests during long POST
1 parent 7f9460b commit 1d00058

File tree

2 files changed

+130
-15
lines changed

2 files changed

+130
-15
lines changed

sdks/acp-http-client/src/index.ts

Lines changed: 90 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -376,32 +376,61 @@ class StreamableHttpAcpTransport {
376376
});
377377

378378
const url = this.buildUrl(this.bootstrapQueryIfNeeded());
379-
const response = await this.fetcher(url, {
379+
const responsePromise = this.fetcher(url, {
380380
method: "POST",
381381
headers,
382382
body: JSON.stringify(message),
383383
});
384384

385385
this.postedOnce = true;
386+
this.ensureSseLoop();
386387

387-
if (!response.ok) {
388-
throw new AcpHttpError(response.status, await readProblem(response), response);
389-
}
388+
const consumeResponse = async (): Promise<void> => {
389+
const response = await responsePromise;
390390

391-
this.ensureSseLoop();
391+
if (!response.ok) {
392+
throw new AcpHttpError(response.status, await readProblem(response), response);
393+
}
392394

393-
if (response.status === 200) {
394-
const text = await response.text();
395-
if (text.trim()) {
396-
const envelope = JSON.parse(text) as AnyMessage;
397-
this.pushInbound(envelope);
395+
if (response.status === 200) {
396+
const text = await response.text();
397+
if (text.trim()) {
398+
const envelope = JSON.parse(text) as AnyMessage;
399+
this.pushInbound(envelope);
400+
}
401+
} else {
402+
// Drain response body so the underlying connection is released back to
403+
// the pool. Without this, Node.js undici keeps the socket occupied and
404+
// may stall subsequent requests to the same origin.
405+
await response.text().catch(() => {});
398406
}
399-
} else {
400-
// Drain response body so the underlying connection is released back to
401-
// the pool. Without this, Node.js undici keeps the socket occupied and
402-
// may stall subsequent requests to the same origin.
403-
await response.text().catch(() => {});
407+
};
408+
409+
// Don't block subsequent writes (e.g. permission replies) behind long
410+
// running prompt turns; prompt completions arrive via SSE.
411+
if (isRequestMessage(message)) {
412+
consumeResponse().catch((error) => {
413+
this.handleDetachedRequestError(message, error);
414+
});
415+
return;
404416
}
417+
418+
await consumeResponse();
419+
}
420+
421+
private handleDetachedRequestError(message: AnyMessage, error: unknown): void {
422+
const id = requestIdFromMessage(message);
423+
if (id === undefined) {
424+
this.failReadable(error);
425+
return;
426+
}
427+
428+
const rpcError = toRpcError(error);
429+
this.pushInbound({
430+
jsonrpc: "2.0",
431+
id,
432+
error: rpcError,
433+
} as AnyMessage);
405434
}
406435

407436
private ensureSseLoop(): void {
@@ -681,5 +710,51 @@ function buildQueryParams(source: Record<string, QueryValue>): URLSearchParams {
681710
return params;
682711
}
683712

713+
function isRecord(value: unknown): value is Record<string, unknown> {
714+
return typeof value === "object" && value !== null;
715+
}
716+
717+
function isRequestMessage(message: AnyMessage): boolean {
718+
if (!isRecord(message)) {
719+
return false;
720+
}
721+
const record = message as Record<string, unknown>;
722+
const method = record["method"];
723+
const id = record["id"];
724+
return typeof method === "string" && id !== undefined;
725+
}
726+
727+
function requestIdFromMessage(message: AnyMessage): number | string | null | undefined {
728+
if (!isRecord(message) || !Object.prototype.hasOwnProperty.call(message, "id")) {
729+
return undefined;
730+
}
731+
const record = message as Record<string, unknown>;
732+
const id = record["id"];
733+
if (typeof id === "string" || typeof id === "number" || id === null) {
734+
return id;
735+
}
736+
return undefined;
737+
}
738+
739+
function toRpcError(error: unknown): RpcErrorResponse {
740+
if (error instanceof AcpHttpError) {
741+
return {
742+
code: -32003,
743+
message: error.problem?.title ?? `HTTP ${error.status}`,
744+
data: error.problem ?? { status: error.status },
745+
};
746+
}
747+
if (error instanceof Error) {
748+
return {
749+
code: -32603,
750+
message: error.message,
751+
};
752+
}
753+
return {
754+
code: -32603,
755+
message: String(error),
756+
};
757+
}
758+
684759
export type * from "@agentclientprotocol/sdk";
685760
export { PROTOCOL_VERSION } from "@agentclientprotocol/sdk";

sdks/typescript/src/client.ts

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,11 @@ import {
77
type CancelNotification,
88
type NewSessionRequest,
99
type NewSessionResponse,
10+
type PermissionOption,
1011
type PromptRequest,
1112
type PromptResponse,
13+
type RequestPermissionRequest,
14+
type RequestPermissionResponse,
1215
type SessionNotification,
1316
type SetSessionConfigOptionRequest,
1417
type SetSessionModeRequest,
@@ -227,6 +230,9 @@ export class LiveAcpConnection {
227230
bootstrapQuery: { agent: options.agent },
228231
},
229232
client: {
233+
requestPermission: async (request: RequestPermissionRequest): Promise<RequestPermissionResponse> => {
234+
return autoSelectPermissionResponse(request);
235+
},
230236
sessionUpdate: async (_notification: SessionNotification) => {
231237
// Session updates are observed via envelope persistence.
232238
},
@@ -1011,6 +1017,40 @@ function normalizeSessionInit(
10111017
};
10121018
}
10131019

1020+
function autoSelectPermissionResponse(
1021+
request: RequestPermissionRequest,
1022+
): RequestPermissionResponse {
1023+
const chosen = selectPermissionOption(request.options ?? []);
1024+
if (!chosen) {
1025+
return {
1026+
outcome: {
1027+
outcome: "cancelled",
1028+
},
1029+
};
1030+
}
1031+
1032+
return {
1033+
outcome: {
1034+
outcome: "selected",
1035+
optionId: chosen.optionId,
1036+
},
1037+
};
1038+
}
1039+
1040+
function selectPermissionOption(options: PermissionOption[]): PermissionOption | null {
1041+
const allowOnce = options.find((option) => option.kind === "allow_once");
1042+
if (allowOnce) {
1043+
return allowOnce;
1044+
}
1045+
1046+
const allowAlways = options.find((option) => option.kind === "allow_always");
1047+
if (allowAlways) {
1048+
return allowAlways;
1049+
}
1050+
1051+
return null;
1052+
}
1053+
10141054
function mapSessionParams(params: Record<string, unknown>, agentSessionId: string): Record<string, unknown> {
10151055
return {
10161056
...params,

0 commit comments

Comments
 (0)