Skip to content

Commit 93b5019

Browse files
author
Mark Pollack
committed
Discover an annotated agent's handlers across its class hierarchy
A framework proxy of the agent bean (Spring CGLIB, Quarkus ArC, Micronaut AOP) is an unannotated subclass and was rejected, and handlers on a superclass were never found. @AcpAgent may now be on a superclass, handlers are collected up to Object and invoked on the given instance, an annotated override is one handler, and bridge and synthetic methods are skipped.
1 parent c677c9e commit 93b5019

5 files changed

Lines changed: 312 additions & 19 deletions

File tree

‎CHANGELOG.md‎

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

546546
### Fixed
547547

548+
- **Annotated agents work as framework-proxied beans and with inherited handlers.**
549+
`AcpAgentSupport` read `@AcpAgent` and the handler methods from the bean's own class only. A
550+
proxy that Spring (CGLIB), Quarkus (ArC) or Micronaut (AOP) generates for the bean is an
551+
unannotated subclass, so `create(proxy)` failed with "Class must be annotated with @AcpAgent",
552+
and handlers declared on a superclass were never found (the client got `-32601`). Discovery
553+
now walks the class hierarchy: `@AcpAgent` may be on a superclass, handlers are collected
554+
from every class up to `Object` and invoked on the instance given (through the proxy, so its
555+
interceptors run), an annotated override is one handler, and bridge and synthetic methods are
556+
skipped. `agentInfo.name` defaults to the annotated class's simple name, not the proxy's.
557+
548558
- **An annotated agent advertises what its annotations declare.** Without an `@Initialize` method,
549559
`AcpAgentSupport` answered `initialize` with `InitializeResponse.ok()`: `loadSession: false`, no
550560
session, auth or provider capabilities, no `authMethods` and no `agentInfo`. An agent with

‎acp-agent-support/README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ This module transitively includes `acp-annotations` and `acp-core`.
5050

5151
| Annotation | Description |
5252
|------------|-------------|
53-
| `@AcpAgent` | Marks a class as an ACP agent. Required on all agent classes. Its attributes are advertised in the `initialize` response: `name`, `version` and `title` as `agentInfo` (the name defaults to the class's simple name, the version to the jar manifest's `Implementation-Version`, else `"unknown"`), `authMethods` (`@AuthMethod`s) as `authMethods`, and `mcpHttp`/`mcpSse` as `mcpCapabilities`. |
53+
| `@AcpAgent` | Marks a class as an ACP agent. Required on the agent class or a superclass of it: handlers are discovered on the class and its superclasses, so a framework proxy of the bean (Spring CGLIB, Quarkus ArC, Micronaut AOP) is accepted and invoked through the proxy. Its attributes are advertised in the `initialize` response: `name`, `version` and `title` as `agentInfo` (the name defaults to the class's simple name, the version to the jar manifest's `Implementation-Version`, else `"unknown"`), `authMethods` (`@AuthMethod`s) as `authMethods`, and `mcpHttp`/`mcpSse` as `mcpCapabilities`. |
5454
| `@AuthMethod` | One authentication method inside `@AcpAgent(authMethods = ...)`: `id`, `name`, `description`, and `type` `AGENT` (served by `@Authenticate`) or `TERMINAL` (the client reruns the agent program with `args` and `env`). |
5555

5656
### Handler Methods

‎acp-agent-support/src/main/java/com/agentclientprotocol/sdk/agent/support/AcpAgentSupport.java‎

Lines changed: 78 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,11 @@
1212
import java.util.Arrays;
1313
import java.util.Collections;
1414
import java.util.HashMap;
15+
import java.util.HashSet;
1516
import java.util.LinkedHashMap;
1617
import java.util.List;
1718
import java.util.Map;
19+
import java.util.Set;
1820
import java.util.UUID;
1921
import java.util.function.Consumer;
2022
import java.util.function.Supplier;
@@ -673,27 +675,88 @@ private Definition definition() {
673675
List.copyOf(interceptors), requestTimeout, cancelGracePeriod, maxPromptDuration);
674676
}
675677

678+
/**
679+
* Discovers the handler methods of {@code agentClass} and its superclasses, and binds
680+
* them to the one instance the factory gives. The {@code @AcpAgent} annotation may be on
681+
* a superclass, as it is for a proxy a framework generates (Spring CGLIB, Quarkus ArC,
682+
* Micronaut AOP), whose subclass carries no annotations: handlers are then found on the
683+
* user's class and invoked on the proxy, so its interceptors run. A method overridden in
684+
* a subclass is one handler, invoked virtually, and bridge and synthetic methods are not
685+
* handlers.
686+
*/
676687
private void discoverHandlers(Class<?> agentClass, Supplier<Object> instanceFactory) {
677-
if (!agentClass.isAnnotationPresent(com.agentclientprotocol.sdk.annotation.AcpAgent.class)) {
678-
throw new IllegalArgumentException("Class must be annotated with @AcpAgent: " + agentClass.getName());
688+
Class<?> annotatedClass = annotatedAgentClass(agentClass);
689+
if (annotatedClass == null) {
690+
throw new IllegalArgumentException("Class must be annotated with @AcpAgent: " + agentClass.getName()
691+
+ " (neither it nor a superclass is)");
679692
}
680-
AgentAdvertisement.validate(agentClass);
681-
agentClasses.add(agentClass);
693+
AgentAdvertisement.validate(annotatedClass);
694+
agentClasses.add(annotatedClass);
682695
// One instance for every handler and request: handlers share the agent's state
683696
Object agentInstance = instanceFactory.get();
684-
for (Method method : agentClass.getDeclaredMethods()) {
685-
HANDLER_ANNOTATIONS.forEach((annotation, acpMethod) -> {
686-
if (method.isAnnotationPresent(annotation)) {
687-
handlers.put(acpMethod, new AcpHandlerMethod(agentInstance, method, acpMethod));
688-
log.debug("Discovered @{} handler: {}", annotation.getSimpleName(), method.getName());
689-
}
690-
});
691-
String extensionMethod = extensionMethod(method);
692-
if (extensionMethod != null) {
693-
handlers.put(extensionMethod, new AcpHandlerMethod(agentInstance, method, extensionMethod));
694-
log.debug("Discovered extension handler for {}: {}", extensionMethod, method.getName());
697+
Set<String> seen = new HashSet<>();
698+
for (Class<?> type = agentClass; type != null && type != Object.class; type = type.getSuperclass()) {
699+
discoverDeclaredHandlers(type, agentInstance, seen);
700+
}
701+
}
702+
703+
/**
704+
* Registers the handlers {@code type} declares, other than those an annotated override
705+
* below it already registered ({@code seen}, the signatures registered so far). An
706+
* unannotated override, such as a proxy's, leaves the annotations to the method it
707+
* overrides. A generic override's bridge stands for the erased method it overrides.
708+
*/
709+
private void discoverDeclaredHandlers(Class<?> type, Object agentInstance, Set<String> seen) {
710+
Set<String> registeredHere = new HashSet<>();
711+
for (Method method : type.getDeclaredMethods()) {
712+
if (!method.isBridge() && !method.isSynthetic() && !seen.contains(signature(method))
713+
&& discoverHandler(method, agentInstance)) {
714+
registeredHere.add(method.getName());
715+
seen.add(signature(method));
716+
}
717+
}
718+
for (Method method : type.getDeclaredMethods()) {
719+
if (method.isBridge() && registeredHere.contains(method.getName())) {
720+
seen.add(signature(method));
721+
}
722+
}
723+
}
724+
725+
/**
726+
* Registers {@code method} for each handler annotation it carries.
727+
* @return whether it carries any
728+
*/
729+
private boolean discoverHandler(Method method, Object agentInstance) {
730+
boolean handler = false;
731+
for (Map.Entry<Class<? extends Annotation>, String> entry : HANDLER_ANNOTATIONS.entrySet()) {
732+
if (method.isAnnotationPresent(entry.getKey())) {
733+
handlers.put(entry.getValue(), new AcpHandlerMethod(agentInstance, method, entry.getValue()));
734+
log.debug("Discovered @{} handler: {}", entry.getKey().getSimpleName(), method.getName());
735+
handler = true;
736+
}
737+
}
738+
String extensionMethod = extensionMethod(method);
739+
if (extensionMethod != null) {
740+
handlers.put(extensionMethod, new AcpHandlerMethod(agentInstance, method, extensionMethod));
741+
log.debug("Discovered extension handler for {}: {}", extensionMethod, method.getName());
742+
handler = true;
743+
}
744+
return handler;
745+
}
746+
747+
/** A method's name and parameter types, which an override shares. */
748+
private static String signature(Method method) {
749+
return method.getName() + Arrays.toString(method.getParameterTypes());
750+
}
751+
752+
/** The first class from {@code type} up that is annotated {@code @AcpAgent}, or null. */
753+
private static @Nullable Class<?> annotatedAgentClass(Class<?> type) {
754+
for (Class<?> candidate = type; candidate != null; candidate = candidate.getSuperclass()) {
755+
if (candidate.isAnnotationPresent(com.agentclientprotocol.sdk.annotation.AcpAgent.class)) {
756+
return candidate;
695757
}
696758
}
759+
return null;
697760
}
698761

699762
/**
Lines changed: 216 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,216 @@
1+
/*
2+
* Copyright 2026 the original author or authors.
3+
*/
4+
5+
package com.agentclientprotocol.sdk.agent.support;
6+
7+
import java.time.Duration;
8+
import java.util.List;
9+
import java.util.concurrent.atomic.AtomicInteger;
10+
import java.util.function.Function;
11+
12+
import com.agentclientprotocol.sdk.annotation.AcpAgent;
13+
import com.agentclientprotocol.sdk.annotation.LoadSession;
14+
import com.agentclientprotocol.sdk.annotation.NewSession;
15+
import com.agentclientprotocol.sdk.annotation.Prompt;
16+
import com.agentclientprotocol.sdk.client.AcpAsyncClient;
17+
import com.agentclientprotocol.sdk.client.AcpClient;
18+
import com.agentclientprotocol.sdk.spec.AcpSchema;
19+
import com.agentclientprotocol.sdk.spec.AcpSchema.InitializeResponse;
20+
import com.agentclientprotocol.sdk.spec.AcpSchema.NewSessionResponse;
21+
import com.agentclientprotocol.sdk.spec.AcpSchema.PromptRequest;
22+
import com.agentclientprotocol.sdk.spec.AcpSchema.PromptResponse;
23+
import com.agentclientprotocol.sdk.test.InMemoryTransportPair;
24+
import org.junit.jupiter.api.Test;
25+
26+
import static org.assertj.core.api.Assertions.assertThat;
27+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
28+
29+
/**
30+
* Handler discovery walks the class hierarchy: a bean that a framework proxies (Spring CGLIB,
31+
* Quarkus ArC or Micronaut AOP generate a subclass that carries no annotations) is still found,
32+
* and invoked through the proxy, so its interceptors run; handlers declared on a superclass are
33+
* found; an override is one handler, not two; and the bridge method a generic override compiles
34+
* to is not a handler of its own.
35+
*/
36+
class AgentDiscoveryTest {
37+
38+
private static final Duration TIMEOUT = Duration.ofSeconds(5);
39+
40+
@AcpAgent(name = "proxied", version = "1")
41+
static class ProxiedAgent {
42+
43+
@NewSession
44+
public NewSessionResponse newSession() {
45+
return new NewSessionResponse("s1", null, null);
46+
}
47+
48+
@LoadSession
49+
public AcpSchema.LoadSessionResponse load() {
50+
return new AcpSchema.LoadSessionResponse(null);
51+
}
52+
53+
@Prompt
54+
public PromptResponse prompt() {
55+
return new PromptResponse(AcpSchema.StopReason.END_TURN);
56+
}
57+
58+
}
59+
60+
/** What a CGLIB/ArC/Micronaut proxy looks like: an unannotated subclass overriding public methods. */
61+
static class ProxiedAgent$$SpringCGLIB$$0 extends ProxiedAgent {
62+
63+
final AtomicInteger intercepted = new AtomicInteger();
64+
65+
@Override
66+
public PromptResponse prompt() {
67+
intercepted.incrementAndGet();
68+
return super.prompt();
69+
}
70+
71+
@Override
72+
public NewSessionResponse newSession() {
73+
intercepted.incrementAndGet();
74+
return super.newSession();
75+
}
76+
77+
}
78+
79+
@Test
80+
void aProxiedBeanIsDiscoveredOnItsUserClassAndInvokedThroughTheProxy() {
81+
ProxiedAgent$$SpringCGLIB$$0 proxy = new ProxiedAgent$$SpringCGLIB$$0();
82+
83+
InitializeResponse init = session(AcpAgentSupport.create(proxy), client -> {
84+
assertThat(prompt(client)).isEqualTo(AcpSchema.StopReason.END_TURN);
85+
return client.initialize().block(TIMEOUT);
86+
});
87+
88+
assertThat(proxy.intercepted).as("newSession and prompt ran through the proxy").hasValue(2);
89+
assertThat(init.agentCapabilities().loadSession()).isTrue();
90+
assertThat(init.agentInfo().name()).isEqualTo("proxied");
91+
}
92+
93+
@Test
94+
void anAnonymousSubclassIsDiscoveredToo() {
95+
AtomicInteger intercepted = new AtomicInteger();
96+
ProxiedAgent anonymous = new ProxiedAgent() {
97+
@Override
98+
public PromptResponse prompt() {
99+
intercepted.incrementAndGet();
100+
return super.prompt();
101+
}
102+
};
103+
104+
session(AcpAgentSupport.create(anonymous), AgentDiscoveryTest::prompt);
105+
106+
assertThat(intercepted).hasValue(1);
107+
}
108+
109+
abstract static class AbstractAgent {
110+
111+
@NewSession
112+
NewSessionResponse newSession() {
113+
return new NewSessionResponse("s1", null, null);
114+
}
115+
116+
@Prompt
117+
PromptResponse prompt(PromptRequest request) {
118+
return new PromptResponse(AcpSchema.StopReason.MAX_TOKENS);
119+
}
120+
121+
}
122+
123+
@AcpAgent
124+
static class ConcreteAgent extends AbstractAgent {
125+
126+
}
127+
128+
@Test
129+
void handlersOnAnAbstractSuperclassAreDiscovered() {
130+
assertThat(session(AcpAgentSupport.create(new ConcreteAgent()), AgentDiscoveryTest::prompt))
131+
.isEqualTo(AcpSchema.StopReason.MAX_TOKENS);
132+
}
133+
134+
@AcpAgent
135+
static class OverridingAgent extends AbstractAgent {
136+
137+
@Override
138+
@Prompt
139+
PromptResponse prompt(PromptRequest request) {
140+
return new PromptResponse(AcpSchema.StopReason.REFUSAL);
141+
}
142+
143+
}
144+
145+
@Test
146+
void anAnnotatedOverrideIsOneHandlerAndTheOverrideRuns() {
147+
assertThat(session(AcpAgentSupport.create(new OverridingAgent()), AgentDiscoveryTest::prompt))
148+
.isEqualTo(AcpSchema.StopReason.REFUSAL);
149+
}
150+
151+
abstract static class GenericAgent<R> {
152+
153+
@Prompt
154+
abstract PromptResponse prompt(R request);
155+
156+
}
157+
158+
/** The override compiles to a bridge prompt(Object) that carries the annotation too. */
159+
@AcpAgent
160+
static class TypedAgent extends GenericAgent<PromptRequest> {
161+
162+
@NewSession
163+
NewSessionResponse newSession() {
164+
return new NewSessionResponse("s1", null, null);
165+
}
166+
167+
@Override
168+
@Prompt
169+
PromptResponse prompt(PromptRequest request) {
170+
return new PromptResponse(request.sessionId().equals("s1") ? AcpSchema.StopReason.END_TURN
171+
: AcpSchema.StopReason.REFUSAL);
172+
}
173+
174+
}
175+
176+
@Test
177+
void aBridgeMethodIsNotAHandler() {
178+
assertThat(session(AcpAgentSupport.create(new TypedAgent()), AgentDiscoveryTest::prompt))
179+
.isEqualTo(AcpSchema.StopReason.END_TURN);
180+
}
181+
182+
static class NotAnAgent {
183+
184+
}
185+
186+
@Test
187+
void aClassWithNoAcpAgentInItsHierarchyIsRejectedNamingIt() {
188+
assertThatThrownBy(() -> AcpAgentSupport.create(new NotAnAgent()))
189+
.isInstanceOf(IllegalArgumentException.class)
190+
.hasMessageContaining(NotAnAgent.class.getName())
191+
.hasMessageContaining("@AcpAgent");
192+
}
193+
194+
private static AcpSchema.StopReason prompt(AcpAsyncClient client) {
195+
client.newSession(new AcpSchema.NewSessionRequest("/w", List.of())).block(TIMEOUT);
196+
return client.prompt(new PromptRequest("s1", List.of(new AcpSchema.TextContent("hi"))))
197+
.block(TIMEOUT)
198+
.stopReason();
199+
}
200+
201+
private static <T> T session(AcpAgentSupport.Builder builder, Function<AcpAsyncClient, T> scenario) {
202+
InMemoryTransportPair pair = InMemoryTransportPair.create();
203+
AcpAgentSupport agent = builder.transport(pair.agentTransport()).requestTimeout(TIMEOUT).build();
204+
agent.start();
205+
AcpAsyncClient client = AcpClient.async(pair.clientTransport()).requestTimeout(TIMEOUT).build();
206+
try {
207+
client.initialize().block(TIMEOUT);
208+
return scenario.apply(client);
209+
}
210+
finally {
211+
client.closeGracefully().block(TIMEOUT);
212+
agent.close();
213+
}
214+
}
215+
216+
}

‎acp-annotations/src/main/java/com/agentclientprotocol/sdk/annotation/AcpAgent.java‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,13 @@
2222
* qualified.
2323
*
2424
* <p><b>Finding the handler methods.</b> {@code AcpAgentSupport} looks for them once, when the
25-
* object is given to its builder, and rejects a class without this annotation with an
26-
* {@code IllegalArgumentException}. It finds methods of any visibility declared in the class
27-
* itself; methods inherited from a superclass are not found. Declare at most one method for each
25+
* object is given to its builder, and rejects a class with this annotation neither on it nor on a
26+
* superclass with an {@code IllegalArgumentException}. It finds methods of any visibility declared
27+
* in the class and its superclasses, so handlers can live on an abstract base class, and a
28+
* subclass that a framework generates for a bean (a Spring CGLIB, Quarkus ArC or Micronaut AOP
29+
* proxy, which carries no annotations) is found through the annotated class it extends and
30+
* invoked on the proxy, so its interceptors run. An annotated override replaces the method it
31+
* overrides. Declare at most one method for each
2832
* annotation: with two, one of them is used and the other is ignored without an error. With no
2933
* {@link Initialize} method the agent answers {@code initialize} with the response derived from
3034
* its annotations (see Capabilities below); with no {@link NewSession} method it answers {@code session/new}

0 commit comments

Comments
 (0)