Created 2 new modules; integration-testing and testing

[cmoulliard]

What changed

• Add 2 new maven modules to the project:
  • integration-testing that a user can use with JBang to perform some checks: session ,etc
  • testing which provides Test fixtures: MockAcpAgent (pipe-based mock) and AcpAgentProcess (real-agent JUnit 5 extension)

Interested to review: @myfear ?

Fix: #4

[myfear]

On it. Give me a little!

[myfear]

Charles, I read the changes, but I haven't checked it out and tested it. So this is only from the diff.

The direction looks like what we talked about. AcpTransport lets the client talk without starting a process, MockAcpAgent answers on a pipe so you can check the JSON-RPC messages, and AcpAgentProcess is the JUnit piece for a real agent when the binary is there.

A few things I saw while reading. AcpAgentProcess builds the client in beforeAll, and then each test calls connect(), which starts a new process every time. A second test can leave the first agent running.

The JBang scripts are a separate runner, and they don't use either of those fixtures. The check named initialize still sends a prompt and waits for the turn to finish, so a slow answer is a failed handshake, right? If a mock handler throws, nothing is sent back and the client just waits for the timeout. The permission test only watches the callback and doesn't show that the reply went back on the pipe.

Add two helpers on MockAcpAgent. The README already shows sendNotification with a nested Map.of, and that is the example people will copy. Keep sendNotification and sendRequest for the odd cases.

sessionUpdate can take the session id, the update type, and the record. ContentChunk has no sessionUpdate field, and the client uses that field to choose the record type, so the helper has to put it on the JSON before the line goes on the pipe. requestPermission can take the RequestPermissionRequest and return the client's reply. The test then waits on that future, and the JSON-RPC id stays inside the fixture.

agent.sessionUpdate("s1", "agent_message_chunk",
        new ContentChunk(Map.of("type", "text", "text", "Hello from agent")));
JsonNode reply = agent.requestPermission(permissionRequest).get(2, TimeUnit.SECONDS);

Hope this makes sense.

[aureamunoz]

+1 for renaming using only 'test'.

[aureamunoz]

transport is not closed in happy path, isn't it?

[cmoulliard]

Good catch. I reviewed the logic: 7c7e465

[cmoulliard]

Hope this makes sense.

@myfear

I addressed your remark part of this commit: b9ce916

1. Fix process leak in AcpAgentProcess (test/.../AcpAgentProcess.java)

• Added AfterEachCallback so the extension now shuts down the transport (and its subprocess) after every test, then rebuilds a fresh transport+client for the next test.
• Extracted buildTransportAndClient() and shutdownTransport() private methods, shared by the lifecycle callbacks.
• Before: a second test calling connect() started a new agent process without killing the first one.
• After: each test gets a clean transport; no leaked processes.

2. Fix handler exceptions swallowed silently (test/.../MockAcpAgent.java)

• Handler invocations (handler.apply(params)) are now wrapped in a try/catch that sends a JSON-RPC error response (-32603) back to the client instead of silently dropping the exception.
• Before: a throwing handler left the client waiting until timeout.
• After: the client gets an immediate error response with the exception message.

3. Add sessionUpdate() helper (test/.../MockAcpAgent.java)

• New method: sessionUpdate(String sessionId, String updateType, Object record)
• Serializes the record to JSON, injects the sessionUpdate discriminator field, wraps it in a session/update notification, and sends it.
• Eliminates the nested Map.of boilerplate from tests and README examples.

4. Add requestPermission() helper (test/.../MockAcpAgent.java)

• New method: requestPermission(RequestPermissionRequest) returns CompletableFuture<JsonNode>
• Manages the JSON-RPC id internally (auto-incrementing counter starting at 1000).
• The read loop now routes response messages (id + result/error, no method) to pending agent request futures.
• Tests can now agent.requestPermission(req).get(2, SECONDS) and assert on the actual reply that came back on the pipe.

5. Updated tests (test/.../MockAcpAgentTest.java)

• agentSendsSessionUpdateNotification: uses agent.sessionUpdate(...) with a typed ContentChunk instead of raw Map.of.
• agentSendsPermissionRequest: uses agent.requestPermission(...), awaits the reply future, and asserts the response content (outcome.optionId == "opt-allow") — verifying the reply actually went back on the pipe.

[xstefank]

ok with me, just a small comment

[xstefank]

maybe should default to defaultMapper?

[xstefank]

or maybe a mistake with defaultMapper method?

[cmoulliard]

Yes, it makes sense to have both as they serve different purposes.

StdioAcpClientTransport has two constructors:

// Uses the shared default mapper
public StdioAcpClientTransport(AgentParameters params)

// Uses a custom mapper (e.g. for testing or special serialization)
public StdioAcpClientTransport(AgentParameters params, ObjectMapper mapper)

getMapper() returns whichever mapper was configured for that specific transport instance — it could be the default or a custom one. Callers like AcpAsyncClient use transport.getMapper() to serialize requests with the same mapper the transport uses internally.

AcpTransport.defaultMapper() is just the static factory for the shared singleton — it's the default value, not the only possible value.

If we remove getMapper(), there'd be no way for consumers to get the mapper from a transport that was constructed with a custom one.

Make sense ? @xstefank