temporal-spring-ai: attach activity summaries for chat and MCP calls#2852
Merged
donald-pinckney merged 9 commits intomasterfrom Apr 23, 2026
Merged
temporal-spring-ai: attach activity summaries for chat and MCP calls#2852donald-pinckney merged 9 commits intomasterfrom
donald-pinckney merged 9 commits intomasterfrom
Conversation
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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>
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>
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>
Planning scratchpad — not part of the shipped artifact. Removed before merge. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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>
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>
JasonSteving99
approved these changes
Apr 22, 2026
Quinn-With-Two-Ns
previously requested changes
Apr 23, 2026
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>
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>
mjameswh
approved these changes
Apr 23, 2026
donald-pinckney
added a commit
that referenced
this pull request
Apr 23, 2026
… 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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What was changed
ActivityChatModelattaches an activity Summary of the formchat: <model>(model label only — user prompts may contain PII, so they are deliberately not included) to each chat-model activity call.ActivityMcpClientgains acallTool(clientName, request, summary)overload;McpToolCallbackuses it to attachmcp: <client>.<tool>.forModel/createfactories). When the user supplies their own pre-built stub to the public constructors, behavior is unchanged, as they are non-generic tools that are up to the user to annotate.ActivitySummaryTestruns a workflow, fetches history, and asserts theActivityTaskScheduledevent forCallChatModelcarries the expecteduserMetadata.summary.Activity-backed tool calls, Nexus tool calls, and
@SideEffectToolcalls are intentionally out of scope — either the stub is user-owned with no clean per-call option override (activity and Nexus tools), or there is no Summary slot on the resulting history event (@SideEffectToolwrites aMarkerRecorded, not anActivityTaskScheduled).Why?
Without Summaries, every chat and MCP step in a Spring AI workflow shows up as an opaque
callChatModelorcallToolrow in the Temporal UI, making multi-step agents hard to follow. Chat and MCP are the cases where the plugin owns stub creation end-to-end, so adding the Summary is a cheap, low-risk overlay; broader coverage would require reaching into user-owned stubs and was deliberately left out to keep this change small and review-friendly.