Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@

import io.agentscope.core.message.ContentBlock;
import io.agentscope.core.message.ThinkingBlock;
import java.util.HashMap;
import java.util.Map;

/**
* Thinking content accumulator for accumulating streaming thinking chunks.
Expand All @@ -28,6 +30,7 @@
public class ThinkingAccumulator implements ContentAccumulator<ThinkingBlock> {

private final StringBuilder accumulated = new StringBuilder();
private final Map<String, Object> metadata = new HashMap<>();

/**
* @hidden
Expand All @@ -37,14 +40,17 @@ public void add(ThinkingBlock block) {
if (block != null && block.getThinking() != null) {
accumulated.append(block.getThinking());
}
if (block != null && block.getMetadata() != null && !block.getMetadata().isEmpty()) {
metadata.putAll(block.getMetadata());
}
}

/**
* @hidden
*/
@Override
public boolean hasContent() {
return accumulated.length() > 0;
return accumulated.length() > 0 || !metadata.isEmpty();
}

/**
Expand All @@ -55,7 +61,11 @@ public ContentBlock buildAggregated() {
if (!hasContent()) {
return null;
}
return ThinkingBlock.builder().thinking(accumulated.toString()).build();
ThinkingBlock.Builder builder = ThinkingBlock.builder().thinking(accumulated.toString());
if (!metadata.isEmpty()) {
builder.metadata(metadata);
}
return builder.build();
}

/**
Expand All @@ -64,6 +74,7 @@ public ContentBlock buildAggregated() {
@Override
public void reset() {
accumulated.setLength(0);
metadata.clear();
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,30 +72,57 @@ public class AnthropicChatModel extends ChatModelBase {
private final GenerateOptions defaultOptions;
private final AnthropicBaseFormatter formatter;

public AnthropicChatModel(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Info] This commit dropped the Javadoc from the 8-arg convenience constructor. It is still a public constructor and it now silently encodes a contract ("delegates with authToken = null", i.e. API-key / X-Api-Key auth). A one-line comment plus @param/@see to the 9-arg overload would keep that visible to callers who pick it from IDE completion; the module publishes javadoc for these public types.

String baseUrl,
String apiKey,
String modelName,
boolean streamEnabled,
GenerateOptions defaultOptions,
AnthropicBaseFormatter formatter,
ProxyConfig proxyConfig,
String cacheTtl) {
this(
baseUrl,
apiKey,
null,
modelName,
streamEnabled,
defaultOptions,
formatter,
proxyConfig,
cacheTtl);
}

/**
* Creates a new Anthropic chat model instance.
* Creates an Anthropic chat model with optional bearer token authentication.
*
* @param baseUrl the base URL for Anthropic API (null for default)
* @param apiKey the API key for authentication (null to load from
* ANTHROPIC_API_KEY env var)
* @param modelName the model name to use (e.g.,
* "claude-sonnet-4-5-20250929")
* @param streamEnabled whether streaming should be enabled
* <p>{@code apiKey} and {@code authToken} are mutually exclusive.
*
* @param baseUrl the base URL for the Anthropic API (null for default)
* @param apiKey the API key for authentication (null to omit)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor javadoc drift: the pre-existing constructor documents apiKey as "null to load from ANTHROPIC_API_KEY env var", while this one says "null to omit", and the new @throws line doesn't mention that the two-credential check is what changed. Aligning the wording (and noting explicitly that env-var fallback happens in AnthropicModelProvider, not in this constructor) would keep the two overloads from reading as different contracts.

* @param authToken the bearer token without the {@code Bearer } prefix (null to omit)
* @param modelName the model name to use
* @param streamEnabled whether streaming should be enabled
* @param defaultOptions default generation options
* @param formatter the message formatter to use (null for default
* Anthropic formatter)
* @param proxyConfig the proxy configuration (null for no proxy)
* @param cacheTtl the TTL for prompt-caching markers (null for default 5m)
* @param formatter the message formatter to use (null for the default formatter)
* @param proxyConfig the proxy configuration (null for no proxy)
* @param cacheTtl the TTL for prompt-caching markers (null for default 5m)
* @throws IllegalArgumentException if both API key and bearer token are configured
*/
public AnthropicChatModel(
String baseUrl,
String apiKey,
String authToken,
String modelName,
boolean streamEnabled,
GenerateOptions defaultOptions,
AnthropicBaseFormatter formatter,
ProxyConfig proxyConfig,
String cacheTtl) {
if (apiKey != null && authToken != null) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The guard is null-based, but Builder.apiKey("") / authToken(" ") are stored verbatim, so a blank credential slips through: apiKey="" + authToken="x" reaches the SDK with both headers, and authToken="" sends a malformed Authorization: Bearer . The starter trims blanks (trimToNull) before calling the builder, but this class is public API, so the same policy should be enforced here too — please normalize blank → null in the setters/constructor and only reject when two present credentials conflict, so the documented guarantee holds for direct builder users as well.

throw new IllegalArgumentException(
"apiKey and authToken are mutually exclusive; configure only one credential");
}
this.baseUrl = baseUrl;
this.apiKey = apiKey;
this.modelName = modelName;
Expand All @@ -112,6 +139,10 @@ public AnthropicChatModel(
clientBuilder.apiKey(apiKey);
}

if (authToken != null) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feature-scope gap: the SPI / model-registry path (AnthropicModelProvider.create()firstNonBlank(context.getApiKey(), ANTHROPIC_API_KEY)) still has no way to pass a bearer token — ModelCreationContext exposes only apiKey/baseUrl/options, and AnthropicCredential requires a non-null apiKey. So gateway users who configure models through the registry (non-Spring) get no authToken, while the docs page added here presents bearer auth as a general model capability. Could you either extend provider + credential + context with auth_token, or state in the doc that bearer auth currently covers the builder and Spring starter paths only?

clientBuilder.authToken(authToken);
}

if (baseUrl != null) {
clientBuilder.baseUrl(baseUrl);
}
Expand Down Expand Up @@ -282,6 +313,7 @@ public static Builder builder() {
public static class Builder {
private String baseUrl;
private String apiKey;
private String authToken;
private String modelName = "claude-sonnet-4-5-20250929";
private boolean streamEnabled = true;
private GenerateOptions defaultOptions;
Expand Down Expand Up @@ -312,6 +344,20 @@ public Builder apiKey(String apiKey) {
return this;
}

/**
* Sets the bearer token for authentication with an Anthropic-compatible gateway.
*

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The javadoc says that if both apiKey and authToken are set the SDK sends both headers, and the builder just forwards them. Silently sending two conflicting credentials is the kind of thing that produces a confusing 401 from the gateway; for an Anthropic-compatible proxy with a static Authorization header this is also a footgun in the Spring starter wiring. Consider rejecting the combination in build() (or ignoring apiKey when authToken is present and logging a warning) so the effective credential is unambiguous.

* <p>The SDK adds the {@code Bearer } prefix to the {@code Authorization} header.
* Configuring both an API key and a bearer token causes model construction to fail.
*
* @param authToken the token without the {@code Bearer } prefix (null to omit)
* @return this builder
*/
public Builder authToken(String authToken) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldNotExposeBearerTokenInModelOrBuilderToString passes only because neither Builder nor AnthropicChatModel overrides toString() today. That makes the guarantee accidental — a future @ToString/record refactor on the builder would silently start printing authToken (and apiKey) into logs. Since this PR is exactly about credential hygiene, it would be nice to add an explicit redacting toString() here (e.g. print authToken=***) so the test pins real behaviour.

this.authToken = authToken;
return this;
}

/**
* Sets the model name.
*
Expand Down Expand Up @@ -397,6 +443,7 @@ public AnthropicChatModel build() {
new AnthropicChatModel(
baseUrl,
apiKey,
authToken,
modelName,
streamEnabled,
defaultOptions,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@
*/
package io.agentscope.extensions.model.anthropic;

import static io.agentscope.core.model.ModelProviderSupport.firstNonBlank;
import static io.agentscope.core.model.ModelProviderSupport.intOption;
import static io.agentscope.core.model.ModelProviderSupport.stringOption;
import static io.agentscope.core.model.ModelProviderSupport.trimToNull;

import io.agentscope.core.model.GenerateOptions;
Expand All @@ -27,12 +27,20 @@
import io.agentscope.extensions.model.anthropic.formatter.AnthropicBaseFormatter;
import java.util.regex.Pattern;

/** Anthropic provider registered through {@link java.util.ServiceLoader}. */
/**
* Anthropic provider registered through {@link java.util.ServiceLoader}.
*
* <p>Credentials come from the context's standard {@code apiKey} field or the {@code
* "authToken"} context option (a bearer token for Anthropic-compatible gateways; the two are
* mutually exclusive). When neither is set, the {@code ANTHROPIC_API_KEY} environment variable
* is used, then {@code ANTHROPIC_AUTH_TOKEN}.
*/
public final class AnthropicModelProvider implements ModelProvider {

private static final String PREFIX = "anthropic:";
private static final Pattern MODEL_ID = Pattern.compile("anthropic:.+");
private static final String OPTION_CONTEXT_WINDOW_SIZE = "contextWindowSize";
private static final String OPTION_AUTH_TOKEN = "authToken";

@Override
public String providerId() {
Expand All @@ -55,10 +63,22 @@ public Model create(String modelId, ModelCreationContext context) {
throw new IllegalArgumentException("Unsupported Anthropic model id: " + modelId);
}
String modelName = modelId.substring(PREFIX.length());
String apiKey = firstNonBlank(context.getApiKey(), System.getenv("ANTHROPIC_API_KEY"));
String apiKey = trimToNull(context.getApiKey());
String authToken = stringOption(context, OPTION_AUTH_TOKEN);
if (apiKey == null && authToken == null) {
// No explicit credential: fall back to the environment, keeping the historical
// precedence of ANTHROPIC_API_KEY over ANTHROPIC_AUTH_TOKEN.
apiKey = trimToNull(System.getenv("ANTHROPIC_API_KEY"));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The env fallback is only consulted when neither explicit credential is present, so an explicit authToken context option correctly wins over a stale ANTHROPIC_API_KEY in the environment. That precedence is the part easy to break in a later refactor and there is no test pinning it: AnthropicModelProviderTest covers explicit apiKey + explicit authToken, non-string authToken, and the bearer round-trip, but not "authToken option set while ANTHROPIC_API_KEY is set in the environment" (nor the reverse). Since the environment is process-wide state, consider extracting the resolution into a package-private helper that takes the two env values as parameters and unit-testing the matrix there, instead of relying on System.getenv in the test.

if (apiKey == null) {
authToken = trimToNull(System.getenv("ANTHROPIC_AUTH_TOKEN"));
}
}
AnthropicChatModel.Builder builder =
AnthropicChatModel.builder().apiKey(apiKey).modelName(modelName).stream(
context.getStream() != null ? context.getStream() : true);
AnthropicChatModel.builder()
.apiKey(apiKey)
.authToken(authToken)
.modelName(modelName)
.stream(context.getStream() != null ? context.getStream() : true);
String baseUrl = trimToNull(context.getBaseUrl());
if (baseUrl != null) {
builder.baseUrl(baseUrl);
Expand Down
Loading
Loading