Skip to content

Commit ccaf4a6

Browse files
temporal-spring-ai: accept ActivityOptions and classify non-retryable AI errors (#2853)
* temporal-spring-ai: plan — activity summaries for debuggability Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * temporal-spring-ai: amend plan — limit summaries to chat + MCP Narrows the activity-summaries scope to cases where the plugin itself owns activity-stub creation. Activity-backed tool calls, Nexus tool calls, and @SideEffectTool calls are now explicitly out of scope; the first two would require per-call option overrides on user-owned stubs (no clean API), and the third writes MarkerRecorded events which have no Summary field. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * temporal-spring-ai: add ActivitySummaryTest (chat path) Runs a workflow that drives a single chat call through ActivityChatModel, fetches the resulting history, and asserts the ActivityTaskScheduled event for callChatModel carries a userMetadata Summary that starts with "chat: default" and includes the user prompt. Intentionally fails against unmodified chat code — the implementation follows in a subsequent commit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * temporal-spring-ai: attach activity summaries for chat and MCP calls ActivityChatModel.forModel() now stores the ActivityOptions it built and, on each chat call, rebuilds the stub with a per-call Summary of the form "chat: <model> · <first 60 chars of user prompt>". When a caller passes a pre-built stub directly via the public constructors, behavior is unchanged (no options known → no summary overlay). ActivityMcpClient.create() does the same and adds a callTool(clientName, request, summary) overload. McpToolCallback passes "mcp: <client>.<tool>". Also fixes the activity-type-name casing in ActivitySummaryTest — Temporal capitalizes the first character of method-name-derived activity types, so the event carries "CallChatModel", not "callChatModel". Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * temporal-spring-ai: drop PLAN.md Planning scratchpad — not part of the shipped artifact. Removed before merge. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * temporal-spring-ai: drop user prompt from chat activity Summary The Summary now carries only the model label ("chat: <model>") instead of "chat: <model> · <first 60 chars of user prompt>". Including even a truncated prompt leaks whatever the prompt contains — PII, secrets, internal identifiers — into workflow history, server logs, and the Temporal UI, which is a surprising default for an observability label. An opt-in API for callers who explicitly want the prompt in the Summary can be added later if there's demand. ActivitySummaryTest.chatActivity_carriesModelOnlySummary_neverLeaksUserPrompt asserts the Summary equals "chat: default" exactly and defensively checks that no part of the prompt leaked in. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * temporal-spring-ai: wrap nullable fields/params in Optional<> Addresses review feedback (thread on #2852) preferring typesafe Optional<T> over nullable fields and raw null delegation. Three sites changed: - ActivityChatModel: modelName and baseOptions fields + private ctor params are now Optional<String> / Optional<ActivityOptions>. getModelName() returns Optional<String>. Public ctors and factories still accept nullable String modelName at the API boundary (matches prior javadoc: "null for default"); they normalize via Optional.ofNullable before storing. Internal readers use .map / .orElse instead of null checks. - ActivityMcpClient: the 3-arg callTool's summary parameter is now Optional<String>. The 2-arg convenience overload passes Optional.empty(). The rebuild-with-summary branch uses .isPresent() + .get() instead of null checks. - McpToolCallback.call(...) wraps its generated summary string in Optional.of(...) before passing to callTool. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * temporal-spring-ai: use @nullable annotations instead of Optional<> Reverses the previous Optional<> commit in favor of @nullable on the nullable fields/params. Matches the dominant convention in temporal-sdk (431+ @nullable usages on public API surface) and avoids a thicker runtime story for what is fundamentally a documentation / IDE-hint concern (no NullAway in the build either way). - ActivityChatModel: modelName and baseOptions fields + private ctor params are now @nullable String / @nullable ActivityOptions. Public ctors/factories accept nullable String modelName at the API boundary directly. getModelName() returns @nullable String. - ActivityMcpClient: baseOptions field and callTool(..., summary) param use @nullable instead of Optional<>. 2-arg callTool passes null; McpToolCallback passes a plain String. - ChatModelTypes.ChatModelActivityInput record: @nullable on modelName and modelOptions fields so deserialized readers see the nullability in the signature (per the reviewer's concern about the deserialization-side typesafety). Consistent with org.springframework.lang.Nullable already used elsewhere in this module (TemporalChatClient, SpringAiPlugin). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * temporal-spring-ai: use javax.annotation.Nullable for consistency Switch the five files in this module that imported org.springframework.lang.Nullable over to javax.annotation.Nullable to match the dominant convention in sdk-java (197 usages vs. 7). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * temporal-spring-ai: plan — ActivityOptions overloads and non-retryable error classification Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * temporal-spring-ai: amend plan — ActivityOptions overloads fix summaries UX wart The activity-summaries branch only overlays Summaries when the factories (forModel/create) built the stub. Users who need custom timeouts today fall back to the public constructor, which silently drops UI Summaries. The ActivityOptions overloads planned here are the proper fix: they let users customize the stub and keep Summary labels. Plan now also covers deprecating the public constructors with javadoc pointing at the factories. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * temporal-spring-ai: accept ActivityOptions and classify non-retryable AI errors - ActivityChatModel.forDefault(ActivityOptions) and forModel(String, ActivityOptions) overloads added. New public defaultActivityOptions() returns the plugin's default bundle so callers can tweak one field without losing the other sensible defaults. - ActivityMcpClient.create(ActivityOptions) + defaultActivityOptions() added, mirroring the chat side. - Default RetryOptions for chat calls now mark org.springframework.ai.retry.NonTransientAiException and java.lang.IllegalArgumentException non-retryable. Default options for MCP calls mark IllegalArgumentException non-retryable. User-supplied ActivityOptions pass through verbatim — the plugin does not augment them. - new ActivityChatModel(...) and new ActivityMcpClient(activity) constructors are @deprecated with javadoc pointing at the factories — they still work at runtime but skip the UI Summary labels the plugin-owned stub path attaches, which is now called out explicitly. - README: new "Activity options and retry behavior" section documents the defaults, how to customize, and the Summary/factory connection. - Tests: two new suites — ActivityOptionsAndRetryTest covers the non-retryable classification (1 attempt for NonTransientAiException, 3 attempts for transient RuntimeException, custom task queue landing on the scheduled activity); ActivitySummaryTest gains a regression test asserting forDefault(customOptions) still emits UI Summaries. - build.gradle: spring-ai-retry added as a testImplementation so tests can reference NonTransientAiException directly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * temporal-spring-ai: drop PLAN.md Planning scratchpad — not part of the shipped artifact. Removed before merge. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * temporal-spring-ai: remove (Duration, int) factory overloads Now that forDefault(ActivityOptions) / forModel(String, ActivityOptions) / create(ActivityOptions) exist, the (Duration, int) convenience overloads are asymmetric dead weight — they expose two of N ActivityOptions fields as positional parameters, and callers wanting anything else (heartbeats, task queue, custom retry backoff, ...) have to drop to the ActivityOptions path anyway. Removed pre-release so the API surface is consistent: no-arg → plugin defaults; ActivityOptions arg → caller options. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent cad6172 commit ccaf4a6

7 files changed

Lines changed: 415 additions & 113 deletions

File tree

temporal-spring-ai/README.md

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,23 @@ public String run(String goal) {
5151
}
5252
```
5353

54+
## Activity options and retry behavior
55+
56+
`ActivityChatModel.forDefault()` / `forModel(name)` build the chat activity stub with sensible defaults: a 2-minute start-to-close timeout, 3 attempts, and `org.springframework.ai.retry.NonTransientAiException` + `java.lang.IllegalArgumentException` marked non-retryable so a bad API key or invalid prompt fails fast instead of churning through retries.
57+
58+
When you need finer control — a specific task queue, heartbeats, priority, or a custom `RetryOptions` — pass an `ActivityOptions` directly:
59+
60+
```java
61+
ActivityChatModel chatModel = ActivityChatModel.forDefault(
62+
ActivityOptions.newBuilder(ActivityChatModel.defaultActivityOptions())
63+
.setTaskQueue("chat-heavy")
64+
.build());
65+
```
66+
67+
`ActivityMcpClient.create()` / `create(ActivityOptions)` work the same way with a 30-second default timeout.
68+
69+
The Temporal UI labels chat and MCP rows with a short Summary (`chat: <model>`, `mcp: <client>.<tool>`). `ActivityChatModel` and `ActivityMcpClient` are constructed only via these factories — there is no public constructor, so users can't accidentally end up in a code path that skips UI labels. Prompt text is deliberately not included in chat summaries to avoid leaking user input (which may contain PII, credentials, or other sensitive data) into workflow history and server logs.
70+
5471
## Tool Types
5572

5673
Tools passed to `defaultTools()` are handled based on their type:

temporal-spring-ai/build.gradle

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,9 @@ dependencies {
4646
testImplementation "org.mockito:mockito-core:${mockitoVersion}"
4747
testImplementation 'org.springframework.boot:spring-boot-starter-test'
4848
testImplementation 'org.springframework.ai:spring-ai-rag'
49+
// Needed only so tests can reference Spring AI's NonTransientAiException to
50+
// verify the plugin's default retry classification.
51+
testImplementation 'org.springframework.ai:spring-ai-retry'
4952

5053
testRuntimeOnly group: 'ch.qos.logback', name: 'logback-classic', version: "${logbackVersion}"
5154
testRuntimeOnly "org.junit.platform:junit-platform-launcher"

temporal-spring-ai/src/main/java/io/temporal/springai/chat/TemporalChatClient.java

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,7 @@
2929
* @WorkflowInit
3030
* public MyWorkflowImpl() {
3131
* // Create the activity-backed chat model
32-
* ChatModelActivity chatModelActivity = Workflow.newActivityStub(
33-
* ChatModelActivity.class, activityOptions);
34-
* ActivityChatModel activityChatModel = new ActivityChatModel(chatModelActivity);
32+
* ActivityChatModel activityChatModel = ActivityChatModel.forDefault();
3533
*
3634
* // Create tools
3735
* WeatherActivity weatherTool = Workflow.newActivityStub(WeatherActivity.class, opts);

temporal-spring-ai/src/main/java/io/temporal/springai/mcp/ActivityMcpClient.java

Lines changed: 58 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import io.temporal.common.RetryOptions;
66
import io.temporal.workflow.Workflow;
77
import java.time.Duration;
8+
import java.util.List;
89
import java.util.Map;
910
import javax.annotation.Nullable;
1011

@@ -48,61 +49,82 @@ public class ActivityMcpClient {
4849
/** Default maximum retry attempts for MCP activity calls. */
4950
public static final int DEFAULT_MAX_ATTEMPTS = 3;
5051

51-
private final McpClientActivity activity;
52-
@Nullable private final ActivityOptions baseOptions;
53-
private Map<String, McpSchema.ServerCapabilities> serverCapabilities;
54-
private Map<String, McpSchema.Implementation> clientInfo;
55-
5652
/**
57-
* Creates a new ActivityMcpClient with the given activity stub.
53+
* Error types that the default retry policy treats as non-retryable. {@link
54+
* IllegalArgumentException} covers unknown-client-name lookups. Client-not-found is already
55+
* thrown as an {@code ApplicationFailure} with {@code nonRetryable=true} and wins on its own.
5856
*
59-
* @param activity the activity stub for MCP operations
57+
* <p>Applied only to the factories that build {@link ActivityOptions} internally. When callers
58+
* pass their own {@link ActivityOptions} via {@link #create(ActivityOptions)}, their {@link
59+
* RetryOptions} are used verbatim.
6060
*/
61-
public ActivityMcpClient(McpClientActivity activity) {
62-
this(activity, null);
63-
}
61+
public static final List<String> DEFAULT_NON_RETRYABLE_ERROR_TYPES =
62+
List.of("java.lang.IllegalArgumentException");
6463

65-
/**
66-
* Creates a new ActivityMcpClient. When {@code baseOptions} is non-null, {@link #callTool(String,
67-
* McpSchema.CallToolRequest, String)} rebuilds the activity stub with a per-call Summary on top
68-
* of those options. When null, the caller supplied a pre-built stub whose options we don't know,
69-
* so we call through it as-is and drop any requested summary.
70-
*/
71-
private ActivityMcpClient(McpClientActivity activity, @Nullable ActivityOptions baseOptions) {
64+
private final McpClientActivity activity;
65+
private final ActivityOptions baseOptions;
66+
private Map<String, McpSchema.ServerCapabilities> serverCapabilities;
67+
private Map<String, McpSchema.Implementation> clientInfo;
68+
69+
/** Use one of the {@link #create()} / {@link #create(ActivityOptions)} factories. */
70+
private ActivityMcpClient(McpClientActivity activity, ActivityOptions baseOptions) {
7271
this.activity = activity;
7372
this.baseOptions = baseOptions;
7473
}
7574

7675
/**
77-
* Creates an ActivityMcpClient with default options.
76+
* Creates an ActivityMcpClient with the plugin's default {@link ActivityOptions} (30-second
77+
* start-to-close timeout, 3 attempts, {@link IllegalArgumentException} marked non-retryable).
7878
*
7979
* <p><strong>Must be called from workflow code.</strong>
8080
*
8181
* @return a new ActivityMcpClient
8282
*/
8383
public static ActivityMcpClient create() {
84-
return create(DEFAULT_TIMEOUT, DEFAULT_MAX_ATTEMPTS);
84+
return create(defaultActivityOptions(DEFAULT_TIMEOUT, DEFAULT_MAX_ATTEMPTS));
8585
}
8686

8787
/**
88-
* Creates an ActivityMcpClient with custom options.
88+
* Creates an ActivityMcpClient using the supplied {@link ActivityOptions}. Pass this when you
89+
* need a specific task queue, heartbeat, priority, or custom {@link RetryOptions}. The provided
90+
* options are used verbatim — the plugin does not augment the caller's {@link RetryOptions}.
8991
*
9092
* <p><strong>Must be called from workflow code.</strong>
9193
*
92-
* @param timeout the activity start-to-close timeout
93-
* @param maxAttempts the maximum number of retry attempts
94+
* @param options the activity options to use for each MCP call
9495
* @return a new ActivityMcpClient
9596
*/
96-
public static ActivityMcpClient create(Duration timeout, int maxAttempts) {
97-
ActivityOptions options =
98-
ActivityOptions.newBuilder()
99-
.setStartToCloseTimeout(timeout)
100-
.setRetryOptions(RetryOptions.newBuilder().setMaximumAttempts(maxAttempts).build())
101-
.build();
97+
public static ActivityMcpClient create(ActivityOptions options) {
10298
McpClientActivity activity = Workflow.newActivityStub(McpClientActivity.class, options);
10399
return new ActivityMcpClient(activity, options);
104100
}
105101

102+
/**
103+
* Returns the plugin's default {@link ActivityOptions} for MCP calls. Useful as a starting point
104+
* when you want to tweak a field without losing the sensible defaults:
105+
*
106+
* <pre>{@code
107+
* ActivityMcpClient.create(
108+
* ActivityOptions.newBuilder(ActivityMcpClient.defaultActivityOptions())
109+
* .setTaskQueue("mcp-heavy")
110+
* .build());
111+
* }</pre>
112+
*/
113+
public static ActivityOptions defaultActivityOptions() {
114+
return defaultActivityOptions(DEFAULT_TIMEOUT, DEFAULT_MAX_ATTEMPTS);
115+
}
116+
117+
private static ActivityOptions defaultActivityOptions(Duration timeout, int maxAttempts) {
118+
return ActivityOptions.newBuilder()
119+
.setStartToCloseTimeout(timeout)
120+
.setRetryOptions(
121+
RetryOptions.newBuilder()
122+
.setMaximumAttempts(maxAttempts)
123+
.setDoNotRetry(DEFAULT_NON_RETRYABLE_ERROR_TYPES.toArray(new String[0]))
124+
.build())
125+
.build();
126+
}
127+
106128
/**
107129
* Gets the server capabilities for all connected MCP clients.
108130
*
@@ -144,9 +166,7 @@ public McpSchema.CallToolResult callTool(String clientName, McpSchema.CallToolRe
144166

145167
/**
146168
* Calls a tool on a specific MCP client, attaching the given activity Summary to the scheduled
147-
* activity so it renders meaningfully in the Temporal UI. Falls back to the base stub when no
148-
* {@link ActivityOptions} are known (e.g. when this client was constructed from a user-supplied
149-
* stub rather than one of the {@link #create} factories).
169+
* activity so it renders meaningfully in the Temporal UI.
150170
*
151171
* @param clientName the name of the MCP client
152172
* @param request the tool call request
@@ -155,17 +175,14 @@ public McpSchema.CallToolResult callTool(String clientName, McpSchema.CallToolRe
155175
*/
156176
public McpSchema.CallToolResult callTool(
157177
String clientName, McpSchema.CallToolRequest request, @Nullable String summary) {
158-
// Overlay the summary onto a fresh stub only when both a summary is requested AND we have
159-
// a recipe to rebuild the stub from (baseOptions). If either is missing, fall through to
160-
// the cached activity — it already has baseOptions baked in if we knew them at construction.
161-
if (summary != null && baseOptions != null) {
162-
McpClientActivity stub =
163-
Workflow.newActivityStub(
164-
McpClientActivity.class,
165-
ActivityOptions.newBuilder(baseOptions).setSummary(summary).build());
166-
return stub.callTool(clientName, request);
178+
if (summary == null) {
179+
return activity.callTool(clientName, request);
167180
}
168-
return activity.callTool(clientName, request);
181+
McpClientActivity stub =
182+
Workflow.newActivityStub(
183+
McpClientActivity.class,
184+
ActivityOptions.newBuilder(baseOptions).setSummary(summary).build());
185+
return stub.callTool(clientName, request);
169186
}
170187

171188
/**

0 commit comments

Comments
 (0)