Skip to content

Commit fbf3a52

Browse files
committed
Merge main into PR 7 for review
2 parents 3985c82 + 5e1225b commit fbf3a52

22 files changed

Lines changed: 960 additions & 542 deletions

File tree

‎.github/workflows/ci.yml‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,3 +38,23 @@ jobs:
3838
name: test-results
3939
path: '**/target/surefire-reports/'
4040
retention-days: 7
41+
42+
# Issue #14 only reproduced when the JVM was starved of cores: the agent's lock release
43+
# and the client's next prompt race on one CPU. Pin the contention tests to one core so
44+
# the window stays open in CI.
45+
contention:
46+
runs-on: ubuntu-latest
47+
timeout-minutes: 20
48+
49+
steps:
50+
- uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4
51+
52+
- name: Set up JDK 17
53+
uses: actions/setup-java@cf277c60eb25467037889841efdb72551f06f6c3 # v4
54+
with:
55+
java-version: '17'
56+
distribution: 'temurin'
57+
cache: maven
58+
59+
- name: Contention tests pinned to one CPU
60+
run: taskset -c 0 ./mvnw -B -pl acp-core -am test -Dtest='MultiPromptSyncSessionTest,AcpAgentSessionPromptLockTest' -Dsurefire.failIfNoSpecifiedTests=false

‎AGENTS.md‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
# ACP Java SDK Agent Instructions
2+
3+
This public repository owns code, tests, Maven builds, releases, shipped contracts, and public
4+
documentation. Private planning and control state are authoritative in
5+
`/home/mark/projects/acp-java-steward`; read its `BINDING.md` before planning or executing work.
6+
7+
This repository belongs to the `agentclientprotocol` organization and is maintained, not owned,
8+
from that steward. Repository settings, secrets, branch protection and organization membership are
9+
the organization's; do not plan work that assumes them.
10+
11+
Use `./mvnw`, never `mvn`. The normal gate is `./mvnw clean verify`, which runs the unit tests and
12+
the `*IT` integration classes through Failsafe. Before 0.15.0 the integration tests never ran, so
13+
"tests pass" on older history did not include them.
14+
15+
Releases happen only by dispatching the organization's `release.yml` workflow; a local
16+
`-Prelease deploy` is not a path, because the local publishing token is not entitled to the
17+
`com.agentclientprotocol` namespace. The workflow's release commit bumps the six `pom.xml` files and
18+
tags; it does not touch `CHANGELOG.md`. Keep the changelog current by hand, in the same change
19+
that earns the entry.
20+
21+
The wire format is the contract. Polymorphic types write their discriminator exactly once
22+
(`Access.WRITE_ONLY` on the record component, `visible = true` on the type info); a duplicate JSON
23+
key passes Jackson-based peers and breaks strict ones. Unstable protocol methods carry
24+
`@UnstableAcpApi`; a method without it is a stable promise.
25+
26+
Client-session notifications are delivered in order through a single sink drained by `concatMap`,
27+
and `closeGracefully()` waits for that drain, bounded by the request timeout, while `close()`
28+
interrupts. Sync clients always have async handlers, because every sync consumer is wrapped in
29+
`subscribeOn`. Keep both properties when touching `AcpClientSession` or a transport, and add the
30+
test that would have caught their loss.
31+
32+
Fixtures and reproductions come from the protocol's own schema or a real peer, not from memory of
33+
what the schema said. The canonical spec is `agentclientprotocol/agent-client-protocol`; check the
34+
released schema version before adding or changing a method.
35+
36+
The project is Apache-2.0 with the verbatim license text; `LICENSE` and `NOTICE` ship in every
37+
artifact. Commit messages contain no AI attribution.
38+
39+
Do not copy private planning, current-action, roadmap, checkpoint, or dirty-tree state into public
40+
files. The ignored `plans/` tree in a checkout is transition material, not public authority.

‎CHANGELOG.md‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,61 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
99

1010
### Fixed
1111

12+
- **A second client on an already-connected transport now fails at construction.** A transport
13+
instance carries exactly one session. `StdioAcpClientTransport.connect()` had no once-only guard
14+
(the WebSocket and agent transports did), so a second `AcpClient.async(transport)` or
15+
`AcpClient.sync(transport)` on the same instance subscribed the transport's unicast inbound sink a
16+
second time — logged and dropped as `Sinks.many().unicast() sinks only allow a single
17+
Subscriber` — and then started a second agent process, whose owner could never receive a
18+
response: its first request timed out after the full `requestTimeout`. That is the Spring Boot
19+
autoconfiguration failure reported in June 2026 (both an `AcpAsyncClient` and an `AcpSyncClient`
20+
bean built from one transport bean); it never depended on the Reactor or Boot version. Now the
21+
stdio transport refuses a second `connect()`, and `AcpClientSession` / `AcpAgentSession` surface a
22+
connect or start failure instead of dropping it: construction throws `IllegalStateException` when
23+
the transport refuses synchronously, and every later request fails immediately with the cause.
24+
25+
- **Second prompt on a session could be rejected or hang under CPU contention (#14).** The agent
26+
released its single-turn prompt lock in `doFinally`, *after* the response had already reached the
27+
client. On a starved machine (`taskset -c 0`, busy CI runners) the client's next prompt arrived
28+
before that release and was rejected with `-32000 There is already an active prompt execution`;
29+
the rejection's emission then collided with the still-running response emission on the same sink
30+
(`FAIL_NON_SERIALIZED`), which the shipped transports silently dropped and the in-memory test
31+
transport turned into a fatal error for the agent, so `AcpSyncClient.prompt` waited out the full
32+
`requestTimeout`. The lock is now released before the response is published, on the success and
33+
error paths alike, and a handler that throws before returning its `Mono` no longer holds the lock
34+
forever. Response emission in `StdioAcpAgentTransport`, `WebSocketAcpClientTransport`,
35+
`WebSocketAcpAgentTransport` and `InMemoryTransportPair` is serialised the way `sendMessage`
36+
already was, and a failed emission in the in-memory pair no longer terminates the agent. The
37+
reporter's repro is now a test, and CI runs the contention tests pinned to one CPU. Reported by
38+
@krickert.
39+
- The agent session now reads `sessionId` from typed `PromptRequest`/`CancelNotification` params as
40+
well as from maps, so in-process transports get the right lock owner in logs and cancel matching.
41+
42+
### Changed
43+
44+
- **Unknown-field policy moved from the schema to the mapper (#10).** Every schema record carried
45+
`@JsonIgnoreProperties(ignoreUnknown = true)`, which made the SDK tolerate fields it does not know
46+
(deliberate: the spec adds fields between releases and a newer agent must keep working) but also
47+
defeated any consumer's strict `ObjectMapper`, since a class-level annotation wins over
48+
`FAIL_ON_UNKNOWN_PROPERTIES`. The annotations are gone. The default mapper,
49+
`JacksonAcpJsonMapper.defaultObjectMapper()`, is lenient and logs each ignored property at DEBUG
50+
so spec drift is observable; a consumer who passes a strict mapper to `JacksonAcpJsonMapper` now
51+
gets strict behaviour. **If you construct `JacksonAcpJsonMapper` with a bare `new ObjectMapper()`,
52+
you now get Jackson's default, which fails on unknown fields**: start from
53+
`defaultObjectMapper()` instead. Unknown fields are not routed into `_meta`, which has its own
54+
spec-defined meaning. Reported by @KallivdH.
55+
56+
### Added
57+
58+
- `AcpSyncClient(AcpAsyncClient)` is public: the supported way to have both APIs over one session.
59+
60+
## [0.17.0] - 2026-08-28
61+
62+
Wire-format correction. No public API change: every constructor and accessor is unchanged, and the
63+
ACP surface is identical to 0.16.1.
64+
65+
### Fixed
66+
1267
- **Duplicate discriminator on the wire.** Every polymorphic type declared its discriminator twice —
1368
once as Jackson's `@JsonTypeInfo` property on the interface and again as an explicit
1469
`@JsonProperty` record component — so both wrote it. `new TextContent("hello")` serialised to
@@ -19,6 +74,23 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1974
and is still populated on deserialization — previously it deserialised to `null`. 27 records
2075
across 6 hierarchies. No API change: every constructor and accessor is unchanged.
2176

77+
## [0.16.1] - 2026-08-24
78+
79+
Release-tooling patch. No protocol, dependency-closure or public API changes beyond a SLF4J patch
80+
bump: the ACP surface is identical to 0.16.0.
81+
82+
### Build
83+
84+
- **Consumer-scoped SBOMs.** Every module artifact now publishes a CycloneDX 1.6 SBOM (classifier
85+
`cyclonedx`) rooted at that module and covering its compile and runtime scope only, so a
86+
consumer's dependency closure can be checked against what the artifact actually ships.
87+
- The release workflow commits the release version bump before tagging, so each `vX.Y.Z` tag now
88+
points at a commit whose POMs carry that version.
89+
90+
### Changed
91+
92+
- SLF4J 2.0.17 → **2.0.18**.
93+
2294
## [0.16.0] - 2026-08-24
2395

2496
Maintenance dependency and release-tooling refresh. No protocol or public API changes: the ACP
@@ -204,5 +276,7 @@ Protocol currency: catching up to ACP spec v0.13.6 (June 2026). Supersedes the n
204276
- SLF4J 2.0.16
205277

206278
[0.9.0]: https://github.com/agentclientprotocol/java-sdk/releases/tag/v0.9.0
279+
[0.17.0]: https://github.com/agentclientprotocol/java-sdk/releases/tag/v0.17.0
280+
[0.16.1]: https://github.com/agentclientprotocol/java-sdk/releases/tag/v0.16.1
207281
[0.16.0]: https://github.com/agentclientprotocol/java-sdk/releases/tag/v0.16.0
208282
[0.15.0]: https://github.com/agentclientprotocol/java-sdk/releases/tag/v0.15.0

‎README.md‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ Three API styles for building agents:
3030
<dependency>
3131
<groupId>com.agentclientprotocol</groupId>
3232
<artifactId>acp-core</artifactId>
33-
<version>0.16.0</version>
33+
<version>0.17.0</version>
3434
</dependency>
3535
```
3636

@@ -39,7 +39,7 @@ For annotation-based agent development:
3939
<dependency>
4040
<groupId>com.agentclientprotocol</groupId>
4141
<artifactId>acp-agent-support</artifactId>
42-
<version>0.16.0</version>
42+
<version>0.17.0</version>
4343
</dependency>
4444
```
4545

@@ -48,7 +48,7 @@ For WebSocket server support (agents accepting WebSocket connections):
4848
<dependency>
4949
<groupId>com.agentclientprotocol</groupId>
5050
<artifactId>acp-websocket-jetty</artifactId>
51-
<version>0.16.0</version>
51+
<version>0.17.0</version>
5252
</dependency>
5353
```
5454

@@ -476,7 +476,18 @@ If you need a stable target, pin to an exact version.
476476

477477
## Releases
478478

479-
### 0.16.0 (Current — [Maven Central](https://central.sonatype.com/artifact/com.agentclientprotocol/acp-core))
479+
### 0.17.0 (Current — [Maven Central](https://central.sonatype.com/artifact/com.agentclientprotocol/acp-core))
480+
481+
Wire-format correction with no API change: every polymorphic discriminator (`type`, `sessionUpdate`,
482+
`outcome`) was written twice, which Jackson-based peers tolerated and strict parsers rejected. It is
483+
now written exactly once and still populated on deserialization.
484+
485+
### 0.16.1 ([Maven Central](https://central.sonatype.com/artifact/com.agentclientprotocol/acp-core))
486+
487+
Release-tooling patch preserving the 0.16 protocol and public API surface: every artifact ships a
488+
consumer-scoped CycloneDX SBOM, and release tags point at commits carrying the released version.
489+
490+
### 0.16.0 ([Maven Central](https://central.sonatype.com/artifact/com.agentclientprotocol/acp-core))
480491

481492
Maintenance release preserving the 0.15 protocol and public API surface while advancing Reactor to
482493
3.8.7, Jackson to 2.22.2, Jetty to 12.1.12, and the compatible stable test and release toolchain.

‎acp-core/src/main/java/com/agentclientprotocol/sdk/agent/transport/StdioAcpAgentTransport.java‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,15 @@ private void handleIncomingMessages(Function<Mono<JSONRPCMessage>, Mono<JSONRPCM
158158
.flatMap(message -> Mono.just(message).transform(handler))
159159
.doOnNext(response -> {
160160
if (response != null) {
161-
this.outboundSink.tryEmitNext(response);
161+
// Responses are emitted from the inbound thread while sendMessage emits
162+
// from user threads on the same sink; both must go through the serialising
163+
// busy-loop or a collision drops the response (FAIL_NON_SERIALIZED, #14).
164+
try {
165+
this.outboundSink.emitNext(response, Sinks.EmitFailureHandler.busyLooping(Duration.ofMillis(100)));
166+
}
167+
catch (Sinks.EmissionException e) {
168+
logger.error("Dropped response {}: {}", response, e.getReason());
169+
}
162170
}
163171
})
164172
.doOnTerminate(() -> {

‎acp-core/src/main/java/com/agentclientprotocol/sdk/client/AcpSyncClient.java‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,11 +76,18 @@ public class AcpSyncClient implements AutoCloseable {
7676
private final AcpAsyncClient delegate;
7777

7878
/**
79-
* Creates a new AcpSyncClient with the given async delegate.
79+
* Creates a synchronous facade over an existing asynchronous client.
80+
*
81+
* <p>
82+
* Both clients share the one session and the one transport connection behind
83+
* {@code delegate}: use this when an application needs both APIs, because a transport
84+
* instance carries exactly one session, and building a second client on an
85+
* already-connected transport fails. Closing either client closes the shared session.
86+
* </p>
8087
* @param delegate the asynchronous client on top of which this synchronous client
8188
* provides a blocking API
8289
*/
83-
AcpSyncClient(AcpAsyncClient delegate) {
90+
public AcpSyncClient(AcpAsyncClient delegate) {
8491
Assert.notNull(delegate, "Delegate must not be null");
8592
this.delegate = delegate;
8693
}

‎acp-core/src/main/java/com/agentclientprotocol/sdk/client/transport/StdioAcpClientTransport.java‎

Lines changed: 46 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import java.util.ArrayList;
1313
import java.util.List;
1414
import java.util.concurrent.Executors;
15+
import java.util.concurrent.atomic.AtomicBoolean;
1516
import java.util.function.Consumer;
1617
import java.util.function.Function;
1718

@@ -76,6 +77,13 @@ public class StdioAcpClientTransport implements AcpClientTransport {
7677

7778
private volatile boolean isClosing = false;
7879

80+
/**
81+
* A transport instance carries exactly one session. {@link #connect} subscribes the
82+
* unicast inbound and outbound sinks and starts the agent process; a second call would
83+
* subscribe them again and start a second process, so it is refused up front.
84+
*/
85+
private final AtomicBoolean isConnected = new AtomicBoolean(false);
86+
7987
// visible for tests
8088
private Consumer<String> stdErrorHandler = error -> logger.info("STDERR Message received: {}", error);
8189

@@ -135,39 +143,49 @@ public StdioAcpClientTransport(AgentParameters params, AcpJsonMapper jsonMapper)
135143
*/
136144
@Override
137145
public Mono<Void> connect(Function<Mono<JSONRPCMessage>, Mono<JSONRPCMessage>> handler) {
138-
return Mono.<Void>fromRunnable(() -> {
139-
logger.info("ACP agent starting.");
140-
handleIncomingMessages(handler);
141-
handleIncomingErrors();
146+
// The guard runs at subscribe time, not at assembly, so a connect publisher that is
147+
// subscribed twice is refused the second time and one that is never subscribed does
148+
// not consume the connection.
149+
return Mono.defer(() -> {
150+
if (!isConnected.compareAndSet(false, true)) {
151+
return Mono.error(new IllegalStateException("StdioAcpClientTransport is already connected. "
152+
+ "A transport instance carries exactly one session and cannot be reused: build one client per "
153+
+ "transport, or share one AcpAsyncClient by wrapping it with new AcpSyncClient(asyncClient)."));
154+
}
155+
return Mono.<Void>fromRunnable(() -> {
156+
logger.info("ACP agent starting.");
157+
handleIncomingMessages(handler);
158+
handleIncomingErrors();
142159

143-
// Prepare command and environment
144-
List<String> fullCommand = new ArrayList<>();
145-
fullCommand.add(params.getCommand());
146-
fullCommand.addAll(params.getArgs());
160+
// Prepare command and environment
161+
List<String> fullCommand = new ArrayList<>();
162+
fullCommand.add(params.getCommand());
163+
fullCommand.addAll(params.getArgs());
147164

148-
ProcessBuilder processBuilder = this.getProcessBuilder();
149-
processBuilder.command(fullCommand);
150-
processBuilder.environment().putAll(params.getEnv());
165+
ProcessBuilder processBuilder = this.getProcessBuilder();
166+
processBuilder.command(fullCommand);
167+
processBuilder.environment().putAll(params.getEnv());
151168

152-
// Start the process
153-
try {
154-
this.process = processBuilder.start();
155-
}
156-
catch (IOException e) {
157-
throw new RuntimeException("Failed to start process with command: " + fullCommand, e);
158-
}
169+
// Start the process
170+
try {
171+
this.process = processBuilder.start();
172+
}
173+
catch (IOException e) {
174+
throw new RuntimeException("Failed to start process with command: " + fullCommand, e);
175+
}
159176

160-
// Validate process streams
161-
if (this.process.getInputStream() == null || process.getOutputStream() == null) {
162-
this.process.destroy();
163-
throw new RuntimeException("Process input or output stream is null");
164-
}
177+
// Validate process streams
178+
if (this.process.getInputStream() == null || process.getOutputStream() == null) {
179+
this.process.destroy();
180+
throw new RuntimeException("Process input or output stream is null");
181+
}
165182

166-
// Start threads
167-
startInboundProcessing();
168-
startOutboundProcessing();
169-
startErrorProcessing();
170-
logger.info("ACP agent started");
183+
// Start threads
184+
startInboundProcessing();
185+
startOutboundProcessing();
186+
startErrorProcessing();
187+
logger.info("ACP agent started");
188+
});
171189
});
172190
}
173191

‎acp-core/src/main/java/com/agentclientprotocol/sdk/client/transport/WebSocketAcpClientTransport.java‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,15 @@ private void handleIncomingMessages(Function<Mono<JSONRPCMessage>, Mono<JSONRPCM
167167
.flatMap(message -> Mono.just(message).transform(handler))
168168
.doOnNext(response -> {
169169
if (response != null) {
170-
this.outboundSink.tryEmitNext(response);
170+
// Responses are emitted from the inbound thread while sendMessage emits
171+
// from user threads on the same sink; both must go through the serialising
172+
// busy-loop or a collision drops the response (FAIL_NON_SERIALIZED, #14).
173+
try {
174+
this.outboundSink.emitNext(response, Sinks.EmitFailureHandler.busyLooping(Duration.ofMillis(100)));
175+
}
176+
catch (Sinks.EmissionException e) {
177+
logger.error("Dropped response {}: {}", response, e.getReason());
178+
}
171179
}
172180
})
173181
.doOnTerminate(() -> {

0 commit comments

Comments
 (0)