Skip to content

Commit 0e4e3fc

Browse files
committed
fix: await streamable listener shutdown
1 parent 2c77d58 commit 0e4e3fc

2 files changed

Lines changed: 169 additions & 21 deletions

File tree

acp-streamable-http-jetty/src/main/java/com/agentclientprotocol/sdk/agent/transport/StreamableHttpAcpAgentTransport.java

Lines changed: 43 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -259,27 +259,36 @@ public int getPort() {
259259
* @return a mono that completes when shutdown finishes
260260
*/
261261
public Mono<Void> closeGracefully() {
262-
return Mono.fromRunnable(() -> {
262+
return Mono.defer(() -> {
263263
if (!closing.compareAndSet(false, true)) {
264-
return;
264+
return Mono.<Void>empty();
265265
}
266-
connections.values().forEach(ConnectionState::close);
266+
List<Mono<Void>> connectionClosures = new ArrayList<>();
267+
connections.values().forEach(connection -> connectionClosures.add(connection.closeGracefully()));
267268
connections.clear();
268-
webSocketConnections.values().forEach(WebSocketConnectionState::close);
269+
webSocketConnections.values().forEach(connection -> connectionClosures.add(connection.closeGracefully()));
269270
webSocketConnections.clear();
270-
Server currentServer = this.server;
271-
if (currentServer != null) {
272-
try {
273-
currentServer.stop();
274-
}
275-
catch (Exception e) {
276-
throw new AcpConnectionException("Failed to stop Streamable HTTP listener", e);
277-
}
278-
}
279-
terminationSink.tryEmitValue(null);
271+
272+
return Mono.whenDelayError(connectionClosures)
273+
.then(Mono.<Void>fromRunnable(this::stopServer))
274+
.doOnSuccess(ignored -> {
275+
terminationSink.tryEmitValue(null);
276+
});
280277
});
281278
}
282279

280+
private void stopServer() {
281+
Server currentServer = this.server;
282+
if (currentServer != null) {
283+
try {
284+
currentServer.stop();
285+
}
286+
catch (Exception e) {
287+
throw new AcpConnectionException("Failed to stop Streamable HTTP listener", e);
288+
}
289+
}
290+
}
291+
283292
/**
284293
* Returns a mono that completes once the listener terminates.
285294
* @return termination mono
@@ -550,11 +559,15 @@ void openStream(HttpServletRequest request, HttpServletResponse response, String
550559
stream.subscribe(asyncContext, response);
551560
}
552561

553-
void close() {
562+
Mono<Void> closeGracefully() {
554563
connections.remove(id, this);
555564
connectionStream.close();
556565
sessionStreams.values().forEach(OutboundStream::close);
557-
connection.closeGracefully().subscribe(v -> {
566+
return connection.closeGracefully();
567+
}
568+
569+
void close() {
570+
closeGracefully().subscribe(v -> {
558571
}, error -> logger.warn("Error closing Streamable HTTP ACP connection {}", id, error));
559572
}
560573

@@ -950,21 +963,30 @@ void sendToClient(JSONRPCMessage message) {
950963
}
951964
}
952965

953-
void close() {
954-
close(StatusCode.NORMAL, "server closing");
966+
Mono<Void> closeGracefully() {
967+
return closeGracefully(StatusCode.NORMAL, "server closing");
955968
}
956969

957-
void close(int statusCode, String reason) {
970+
private Mono<Void> closeGracefully(int statusCode, String reason) {
958971
if (!closed.compareAndSet(false, true)) {
959-
return;
972+
return Mono.empty();
960973
}
961974
outboundSender.close();
962975
webSocketConnections.remove(id, this);
963976
Session currentSession = this.session;
964977
if (currentSession != null && currentSession.isOpen()) {
965978
currentSession.close(statusCode, reason, Callback.NOOP);
966979
}
967-
remoteConnection.closeGracefully().subscribe(v -> {
980+
return remoteConnection.closeGracefully();
981+
}
982+
983+
void close() {
984+
closeGracefully().subscribe(v -> {
985+
}, error -> logger.warn("Error closing Streamable ACP WebSocket connection {}", id, error));
986+
}
987+
988+
void close(int statusCode, String reason) {
989+
closeGracefully(statusCode, reason).subscribe(v -> {
968990
}, error -> logger.warn("Error closing Streamable ACP WebSocket connection {}", id, error));
969991
}
970992

acp-streamable-http-jetty/src/test/java/com/agentclientprotocol/sdk/agent/transport/StreamableHttpAcpAgentTransportWebSocketIntegrationTest.java

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@
3232

3333
import com.agentclientprotocol.sdk.agent.AcpAgent;
3434
import com.agentclientprotocol.sdk.agent.AcpAgentFactory;
35+
import com.agentclientprotocol.sdk.agent.AcpAsyncAgent;
36+
import com.agentclientprotocol.sdk.capabilities.NegotiatedCapabilities;
3537
import com.agentclientprotocol.sdk.client.AcpAsyncClient;
3638
import com.agentclientprotocol.sdk.client.AcpClient;
3739
import com.agentclientprotocol.sdk.client.transport.WebSocketAcpClientTransport;
@@ -41,6 +43,7 @@
4143
import org.junit.jupiter.api.Test;
4244
import reactor.core.publisher.Flux;
4345
import reactor.core.publisher.Mono;
46+
import reactor.core.publisher.Sinks;
4447
import reactor.core.scheduler.Schedulers;
4548

4649
import static org.assertj.core.api.Assertions.assertThat;
@@ -325,6 +328,30 @@ void rejectsDuplicateInitializeWithoutForwardingItToTheAgent() throws Exception
325328
}
326329
}
327330

331+
@Test
332+
void listenerShutdownWaitsForWebSocketAgentShutdown() throws Exception {
333+
Sinks.One<Void> allowAgentShutdown = Sinks.one();
334+
CountDownLatch agentShutdownStarted = new CountDownLatch(1);
335+
AcpAgentFactory agentFactory = transport -> new BlockingCloseAgent(allowAgentShutdown, agentShutdownStarted);
336+
337+
try (FixtureServer server = FixtureServer.start(agentFactory)) {
338+
MessageRecordingListener listener = new MessageRecordingListener();
339+
HttpClient.newHttpClient()
340+
.newWebSocketBuilder()
341+
.connectTimeout(TIMEOUT)
342+
.buildAsync(server.endpoint(), listener)
343+
.get(TIMEOUT.toMillis(), TimeUnit.MILLISECONDS);
344+
assertThat(listener.openLatch.await(TIMEOUT.toMillis(), TimeUnit.MILLISECONDS)).isTrue();
345+
346+
CompletableFuture<Void> shutdown = server.transport().closeGracefully().toFuture();
347+
assertThat(agentShutdownStarted.await(TIMEOUT.toMillis(), TimeUnit.MILLISECONDS)).isTrue();
348+
assertThat(shutdown).isNotDone();
349+
350+
allowAgentShutdown.tryEmitEmpty();
351+
shutdown.get(TIMEOUT.toMillis(), TimeUnit.MILLISECONDS);
352+
}
353+
}
354+
328355
private static AcpAgentFactory simpleAgentFactory() {
329356
AtomicInteger sessionCounter = new AtomicInteger();
330357
return AcpAgentFactory.async(transport -> AcpAgent.async(transport)
@@ -462,4 +489,103 @@ public CompletionStage<?> onText(WebSocket webSocket, CharSequence data, boolean
462489

463490
}
464491

492+
private static final class BlockingCloseAgent implements AcpAsyncAgent {
493+
494+
private final Sinks.One<Void> allowShutdown;
495+
496+
private final CountDownLatch shutdownStarted;
497+
498+
BlockingCloseAgent(Sinks.One<Void> allowShutdown, CountDownLatch shutdownStarted) {
499+
this.allowShutdown = allowShutdown;
500+
this.shutdownStarted = shutdownStarted;
501+
}
502+
503+
@Override
504+
public Mono<Void> start() {
505+
return Mono.empty();
506+
}
507+
508+
@Override
509+
public Mono<Void> awaitTermination() {
510+
return Mono.never();
511+
}
512+
513+
@Override
514+
public NegotiatedCapabilities getClientCapabilities() {
515+
return null;
516+
}
517+
518+
@Override
519+
public Mono<Void> sendSessionUpdate(String sessionId, AcpSchema.SessionUpdate update) {
520+
return unsupported();
521+
}
522+
523+
@Override
524+
public Mono<AcpSchema.RequestPermissionResponse> requestPermission(AcpSchema.RequestPermissionRequest request) {
525+
return unsupported();
526+
}
527+
528+
@Override
529+
public Mono<AcpSchema.ReadTextFileResponse> readTextFile(AcpSchema.ReadTextFileRequest request) {
530+
return unsupported();
531+
}
532+
533+
@Override
534+
public Mono<AcpSchema.WriteTextFileResponse> writeTextFile(AcpSchema.WriteTextFileRequest request) {
535+
return unsupported();
536+
}
537+
538+
@Override
539+
public Mono<AcpSchema.CreateTerminalResponse> createTerminal(AcpSchema.CreateTerminalRequest request) {
540+
return unsupported();
541+
}
542+
543+
@Override
544+
public Mono<AcpSchema.TerminalOutputResponse> getTerminalOutput(AcpSchema.TerminalOutputRequest request) {
545+
return unsupported();
546+
}
547+
548+
@Override
549+
public Mono<AcpSchema.ReleaseTerminalResponse> releaseTerminal(AcpSchema.ReleaseTerminalRequest request) {
550+
return unsupported();
551+
}
552+
553+
@Override
554+
public Mono<AcpSchema.WaitForTerminalExitResponse> waitForTerminalExit(AcpSchema.WaitForTerminalExitRequest request) {
555+
return unsupported();
556+
}
557+
558+
@Override
559+
public Mono<AcpSchema.KillTerminalCommandResponse> killTerminal(AcpSchema.KillTerminalCommandRequest request) {
560+
return unsupported();
561+
}
562+
563+
@Override
564+
public Mono<AcpSchema.CreateElicitationResponse> createElicitation(AcpSchema.CreateElicitationRequest request) {
565+
return unsupported();
566+
}
567+
568+
@Override
569+
public Mono<Void> completeElicitation(AcpSchema.CompleteElicitationNotification notification) {
570+
return unsupported();
571+
}
572+
573+
@Override
574+
public Mono<Void> closeGracefully() {
575+
return Mono.defer(() -> {
576+
shutdownStarted.countDown();
577+
return allowShutdown.asMono();
578+
});
579+
}
580+
581+
@Override
582+
public void close() {
583+
}
584+
585+
private static <T> Mono<T> unsupported() {
586+
return Mono.error(new UnsupportedOperationException());
587+
}
588+
589+
}
590+
465591
}

0 commit comments

Comments
 (0)