Skip to content

feat(openai-official): add OpenAI Responses API module - #3078

Open
jujn wants to merge 2 commits into
mainfrom
feat/openai-official-responses
Open

feat(openai-official): add OpenAI Responses API module#3078
jujn wants to merge 2 commits into
mainfrom
feat/openai-official-responses

Conversation

@jujn

@jujn jujn commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

Summary

This PR adds a new model extension module, agentscope-extensions-model-openai-official, which integrates OpenAI models through the official OpenAI Java SDK (com.openai:openai-java) against the Responses API. This complements the existing agentscope-extensions-model-openai module, which uses a hand-rolled HTTP client against the Chat Completions API.

The Responses API is OpenAI's newer interface with built-in support for reasoning models, encrypted reasoning replay across turns, structured outputs, and server-side conversation state. This module brings first-class support for those capabilities into AgentScope.

What's included

架构图 openai-official-architecture
流式时序图 openai-official-sequence

Module: agentscope-extensions-model-openai-official (14 production classes, ~2,500 LOC)

The module is structured around a clean separation of concerns:

  • OpenAIResponsesChatModel — the ChatModelBase implementation. Owns the streaming/non-streaming dispatch, retry classification, and builder API. SDK retry is disabled (maxRetries = 0); AgentScope owns all retry logic via a module-level retryOn predicate that inspects x-should-retry headers and SDK exception types.
  • OpenAIOfficialModelProvider — SPI provider registered via ModelProvider service loader. Supports the openai-official:<model> model-id convention and resolves OPENAI_API_KEY / OPENAI_BASE_URL from the environment.
  • ResponsesRequestMapper — maps AgentScope Msg objects, ToolSchema definitions, and GenerateOptions to SDK ResponseCreateParams. Handles reasoning replay (encrypted content from previous turns), structured output (JSON object / JSON schema), tool-call mapping with strict mode, and fail-fast validation for unsupported fields.
  • ResponsesResponseParser — parses a non-streaming Response into a single ChatResponse, assembling content blocks in fixed order (ThinkingBlock -> TextBlock -> ToolUseBlock) with full metadata extraction.
  • ResponsesStreamingAssembler — assembles a Flux<ResponseStreamEvent> into incremental ChatResponse chunks. Routes text deltas, reasoning summary deltas, function-call argument deltas, and terminal events (completed/incomplete/failed/error) with proper resource cleanup via doFinally.
  • ResponsesMultiAgentFormatter — multi-agent conversation formatter. Groups messages by type (SYSTEM, TOOL_SEQUENCE, AGENT_CONVERSATION, BYPASS), merges agent conversation messages into <history>-tagged user messages, and passes through tool sequences and system messages unchanged. Supports a customizable conversation history prompt.
  • OpenAIErrorTranslator — normalizes SDK exceptions (OpenAIServiceException, OpenAIIoException, OpenAIRetryableException, OpenAIInvalidDataException, TimeoutException) into OpenAIOfficialModelException with HTTP status codes preserved.
  • OpenAISdkClientFactory — the single production entry point for OpenAIClient creation. Sets maxRetries = 0, injects builder-level additional headers, and fail-fasts on missing API key.
  • OpenAIOfficialCredential — JSON-serializable credential type (openai_official_credential) for use with AgentScope's credential system.
  • OpenAIOfficialConstants — shared constants for metadata namespace keys (openai.*) and the additionalBodyParams whitelist.

Key capabilities:

  • Streaming and non-streaming Responses API calls
  • Tool calling with per-tool and builder-level strict mode
  • Native structured output
  • Reasoning effort control (low / medium / high / minimal) with encrypted reasoning content automatically replayed across turns via Msg.metadata — no manual management needed
  • Responses-specific parameters via whitelisted additionalBodyParams: reasoning.summary, reasoning.context, reasoning.mode, service_tier, prompt_cache_key, prompt_cache_options, max_tool_calls, safety_identifier
  • Builder-level additional headers (constant across all requests)
  • Image input (URL and base64) in user messages and tool results
  • Credential support and SPI auto-registration
  • E2E test provider integration (OpenAIOfficialResponsesProvider)

Supporting changes

Several improvements to core and harness modules were needed to support this integration:

  • ModelContextWindows — added context window sizes for GPT-5.x, GPT-6, and GLM-5.3 models.
  • ModelUtils — timeout exceptions now wrap a TimeoutException as cause, so module-level retryOn predicates can detect timeouts in the cause chain.
  • Distributionagentscope-all and agentscope-bom updated to include the new module.
  • Docs — bilingual (English and Chinese) integration documentation added, including table-of-contents and overview updates.

Testing

The module includes 13 test files totaling ~5,600 lines, covering:

  • Request mapping: history, tools, options, structured output, reasoning validation, rejected fields, prompt cache options, additional body params
  • Response parsing: text, reasoning, tool calls, refusal, metadata/usage extraction
  • Streaming assembly: text deltas, reasoning deltas, function-call deltas, terminal events, failed/error events, stream cleanup
  • Multi-agent formatter: agent conversations, tool sequences, system messages, media handling, edge cases, bypass
  • Error translation: all SDK exception types, cause-chain timeout, status-code classification
  • Model provider: SPI support detection, context resolution, advanced options, stream defaults
  • Cross-turn behavior: reasoning replay, tool-set changes across turns, option merging, streaming vs. non-streaming modes
  • Client factory and credential validation

Not yet supported

The following are intentionally out of scope for this initial PR. They are tracked for follow-up work:

  1. Spring Boot starter — no dedicated starter is provided.
  2. store parameter — hardcoded to false in ResponsesRequestMapper. OpenAI's server-side conversation storage is not used; AgentScope manages conversation history client-side. Making this configurable is a straightforward future enhancement.
  3. Multimodal input/output — only TextBlock, ImageBlock (URL and base64), and image-type DataBlock are supported. AudioBlock and VideoBlock are not mapped; the request mapper fail-fasts on unsupported block types, and the multi-agent formatter silently skips them. The existing OpenAI and DashScope modules handle audio and video blocks.
  4. ProxyConfig — not supported. This is a real gap for enterprise and restricted-network environments. The official SDK's OpenAIOkHttpClient.Builder supports proxy configuration, so wiring is feasible.
  5. MultiModalTool — no MultiModalTool implementation. The OpenAI module provides OpenAIMultiModalTool (text-to-image, image-to-text, text-to-audio, audio-to-text) and the DashScope module provides DashScopeMultiModalTool. A corresponding tool for this module is optional but would improve feature parity.

Usage

Via explicit builder:

OpenAIResponsesChatModel model = OpenAIResponsesChatModel.builder()
    .apiKey(System.getenv("OPENAI_API_KEY"))
    .modelName("gpt-4o")
    .stream(true)
    .build();

With reasoning and multi-agent formatter:

GenerateOptions options = GenerateOptions.builder()
    .reasoningEffort("high")
    .additionalBodyParam("reasoning.summary", "auto")
    .build();

OpenAIResponsesChatModel model = OpenAIResponsesChatModel.builder()
    .apiKey(apiKey)
    .modelName("gpt-5.4")
    .generateOptions(options)
    .formatter(new ResponsesMultiAgentFormatter())
    .build();

Copilot AI lite review requested due to automatic review settings September 9, 2026 14:48

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

There are correctness/usability issues in provider configuration and connection-field validation (env base URL resolution and blank-value normalization) that should be fixed before merging.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR introduces a new model extension module, agentscope-extensions-model-openai-official, integrating OpenAI models via the official OpenAI Java SDK against the Responses API, and wires it into docs, distributions, and the e2e harness.

Changes:

  • Adds the openai-official provider implementation (request mapping, response parsing, streaming assembly, error translation, credential type, SPI registration).
  • Updates distributions/BOM and extensions aggregator to include the new module, and adds e2e provider coverage.
  • Updates v2 docs (EN/ZH) and TOC to document the new provider and module.
File summaries
File Description
docs/v2/zh/integration/overview.md Adds openai-official provider row to the ZH integration overview table.
docs/v2/zh/integration/model/openai-official.md New ZH provider doc page for the official SDK / Responses API module.
docs/v2/zh/integration/model/index.md Links the new ZH provider doc in the model index.
docs/v2/zh/docs/building-blocks/model.md Updates ZH “model building blocks” doc to include the new module/provider references.
docs/v2/zh/docs/building-blocks/agent.md Mentions openai-official as a supported ModelRegistry provider in ZH agent docs.
docs/v2/en/integration/overview.md Adds openai-official provider row to the EN integration overview table.
docs/v2/en/integration/model/openai-official.md New EN provider doc page for the official SDK / Responses API module.
docs/v2/en/integration/model/index.md Links the new EN provider doc in the model index.
docs/v2/en/docs/building-blocks/model.md Updates EN “model building blocks” doc to include the new module/provider references.
docs/v2/en/docs/building-blocks/agent.md Mentions openai-official as a supported ModelRegistry provider in EN agent docs.
docs/_toc.yml Adds TOC entries for the new EN/ZH provider doc pages.
agentscope-extensions/agentscope-extensions-model/pom.xml Enables the new agentscope-extensions-model-openai-official module in the extensions reactor build.
agentscope-extensions/agentscope-extensions-model/agentscope-extensions-model-openai-official/src/test/java/io/agentscope/extensions/model/openaiofficial/TestSdkFixtures.java Shared SDK object/exception fixtures for unit tests.
agentscope-extensions/agentscope-extensions-model/agentscope-extensions-model-openai-official/src/test/java/io/agentscope/extensions/model/openaiofficial/ResponsesStreamingAssemblerTest.java Unit tests for streaming event assembly to ChatResponse chunks.
agentscope-extensions/agentscope-extensions-model/agentscope-extensions-model-openai-official/src/test/java/io/agentscope/extensions/model/openaiofficial/ResponsesResponseParserTest.java Unit tests for non-streaming ResponseChatResponse parsing.
agentscope-extensions/agentscope-extensions-model/agentscope-extensions-model-openai-official/src/test/java/io/agentscope/extensions/model/openaiofficial/ResponsesMultiAgentFormatterTest.java Unit tests for multi-agent conversation merging/formatting.
agentscope-extensions/agentscope-extensions-model/agentscope-extensions-model-openai-official/src/test/java/io/agentscope/extensions/model/openaiofficial/ResponsesHelperTest.java Unit tests for metadata/usage extraction helpers.
agentscope-extensions/agentscope-extensions-model/agentscope-extensions-model-openai-official/src/test/java/io/agentscope/extensions/model/openaiofficial/OpenAISdkClientFactoryTest.java Unit tests for official SDK client construction and error wrapping.
agentscope-extensions/agentscope-extensions-model/agentscope-extensions-model-openai-official/src/test/java/io/agentscope/extensions/model/openaiofficial/OpenAIResponsesChatModelTest.java Unit tests for streaming/non-streaming flows, retry predicate behavior, and builder boundaries.
agentscope-extensions/agentscope-extensions-model/agentscope-extensions-model-openai-official/src/test/java/io/agentscope/extensions/model/openaiofficial/OpenAIOfficialModelProviderTest.java Unit tests for SPI provider supports/create behavior and advanced options.
agentscope-extensions/agentscope-extensions-model/agentscope-extensions-model-openai-official/src/test/java/io/agentscope/extensions/model/openaiofficial/OpenAIOfficialModelExceptionTest.java Unit tests for provider exception type and retryable-status classification.
agentscope-extensions/agentscope-extensions-model/agentscope-extensions-model-openai-official/src/test/java/io/agentscope/extensions/model/openaiofficial/OpenAIErrorTranslatorTest.java Unit tests for SDK exception translation coverage.
agentscope-extensions/agentscope-extensions-model/agentscope-extensions-model-openai-official/src/test/java/io/agentscope/extensions/model/openaiofficial/credential/OpenAIOfficialCredentialTest.java Unit tests for credential JSON round-tripping and validation.
agentscope-extensions/agentscope-extensions-model/agentscope-extensions-model-openai-official/src/main/resources/META-INF/services/io.agentscope.core.model.spi.ModelProvider Registers OpenAIOfficialModelProvider via ServiceLoader.
agentscope-extensions/agentscope-extensions-model/agentscope-extensions-model-openai-official/src/main/java/io/agentscope/extensions/model/openaiofficial/ResponsesStreamingAssembler.java Implements Responses API streaming event → incremental ChatResponse assembly.
agentscope-extensions/agentscope-extensions-model/agentscope-extensions-model-openai-official/src/main/java/io/agentscope/extensions/model/openaiofficial/ResponsesResponseParser.java Parses non-streaming SDK Response objects into ChatResponse.
agentscope-extensions/agentscope-extensions-model/agentscope-extensions-model-openai-official/src/main/java/io/agentscope/extensions/model/openaiofficial/ResponsesRequestMapper.java Maps AgentScope history/options/tools into SDK ResponseCreateParams.
agentscope-extensions/agentscope-extensions-model/agentscope-extensions-model-openai-official/src/main/java/io/agentscope/extensions/model/openaiofficial/ResponsesMultiAgentFormatter.java Implements multi-agent history grouping/merge into Responses API input items.
agentscope-extensions/agentscope-extensions-model/agentscope-extensions-model-openai-official/src/main/java/io/agentscope/extensions/model/openaiofficial/ResponsesHelper.java Extracts response metadata and usage into AgentScope-friendly structures.
agentscope-extensions/agentscope-extensions-model/agentscope-extensions-model-openai-official/src/main/java/io/agentscope/extensions/model/openaiofficial/OpenAISdkClientFactory.java Centralizes OpenAI SDK client creation with retries disabled and optional headers/timeout.
agentscope-extensions/agentscope-extensions-model/agentscope-extensions-model-openai-official/src/main/java/io/agentscope/extensions/model/openaiofficial/OpenAIResponsesChatModel.java Main ChatModelBase implementation for Responses API (streaming + non-streaming).
agentscope-extensions/agentscope-extensions-model/agentscope-extensions-model-openai-official/src/main/java/io/agentscope/extensions/model/openaiofficial/OpenAIOfficialModelProvider.java Adds SPI provider supporting openai-official:<model> resolution.
agentscope-extensions/agentscope-extensions-model/agentscope-extensions-model-openai-official/src/main/java/io/agentscope/extensions/model/openaiofficial/OpenAIOfficialModelException.java Provider-specific exception type implementing ModelHttpException.
agentscope-extensions/agentscope-extensions-model/agentscope-extensions-model-openai-official/src/main/java/io/agentscope/extensions/model/openaiofficial/OpenAIOfficialConstants.java Centralizes provider id, metadata keys, and additionalBodyParams whitelist.
agentscope-extensions/agentscope-extensions-model/agentscope-extensions-model-openai-official/src/main/java/io/agentscope/extensions/model/openaiofficial/OpenAIErrorTranslator.java Translates SDK exceptions into OpenAIOfficialModelException.
agentscope-extensions/agentscope-extensions-model/agentscope-extensions-model-openai-official/src/main/java/io/agentscope/extensions/model/openaiofficial/credential/OpenAIOfficialCredential.java Adds credential type for the new provider.
agentscope-extensions/agentscope-extensions-model/agentscope-extensions-model-openai-official/pom.xml New module POM with openai-java dependency and test deps.
agentscope-extensions/agentscope-extensions-model/agentscope-extensions-model-e2e-tests/src/test/java/io/agentscope/core/e2e/providers/OpenAIOfficialResponsesProvider.java Adds e2e provider(s) for official SDK Responses API path.
agentscope-extensions/agentscope-extensions-model/agentscope-extensions-model-e2e-tests/src/test/java/io/agentscope/core/e2e/ProviderFactory.java Registers the new e2e providers in the factory list.
agentscope-extensions/agentscope-extensions-model/agentscope-extensions-model-e2e-tests/pom.xml Adds test-scope dependency on the new openai-official module.
agentscope-distribution/agentscope-bom/pom.xml Adds the new module to the published BOM.
agentscope-distribution/agentscope-all/pom.xml Adds the new module as an optional dependency to the “all” distribution.
agentscope-core/src/main/java/io/agentscope/core/model/ModelUtils.java Wraps timeouts with a TimeoutException cause for retry predicates to detect.
agentscope-core/src/main/java/io/agentscope/core/model/ModelContextWindows.java Adds context window mappings for new OpenAI/GLM model names.
Review details

Suppressed comments (2)

agentscope-extensions/agentscope-extensions-model/agentscope-extensions-model-openai-official/src/main/java/io/agentscope/extensions/model/openaiofficial/OpenAIResponsesChatModel.java:175

  • validateConnectionFields also treats a blank baseUrl as an override mismatch (null vs ""), but OpenAISdkClientFactory already normalizes blank baseUrl to “use SDK default”. This can incorrectly fail-fast on semantically equivalent values.
        String effectiveBaseUrl = effectiveOptions.getBaseUrl();
        if (effectiveBaseUrl != null && !Objects.equals(effectiveBaseUrl, baseUrl)) {
            throw new OpenAIOfficialModelException(

agentscope-extensions/agentscope-extensions-model/agentscope-extensions-model-e2e-tests/src/test/java/io/agentscope/core/e2e/providers/OpenAIOfficialResponsesProvider.java:91

  • Same mismatch here: the Javadoc says “GPT-5.4-mini” but the provider uses "gpt-5.4".
    /** GPT-5.4-mini with Multi-Agent Formatter. */
  • Files reviewed: 46/46 changed files
  • Comments generated: 4
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@muranchenhui

Copy link
Copy Markdown

LGTM

Comment thread agentscope-distribution/agentscope-all/pom.xml
Comment thread agentscope-distribution/agentscope-bom/pom.xml
Comment thread agentscope-distribution/agentscope-all/pom.xml
@jujn jujn closed this Sep 10, 2026
@jujn
jujn force-pushed the feat/openai-official-responses branch from 97b8f26 to 30a9821 Compare September 10, 2026 06:39
Add agentscope-extensions-model-openai-official module integrating OpenAI
models via the official OpenAI Java SDK (com.openai:openai-java) against
the Responses API. Includes request mapping, response parsing, streaming
assembly, error translation, credential type, SPI registration, e2e test
provider, bilingual docs, and distribution/BOM updates.

Resolve documentation conflicts from Mintlify migration (#3081):
- Replace deleted _toc.yml with docs.json entries
- Adapt agent.md/index.md/overview.md to Mintlify syntax
- Add Mintlify front matter to openai-official.md pages
@jujn jujn reopened this Sep 10, 2026

@Aias00 Aias00 left a comment

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.

已审。重点检查了新增 openai-official Responses 模块的请求映射、非流式解析、流式事件拼装、usage/metadata/finishReason、工具调用回放、错误转换、provider SPI、BOM/distribution 接入和相关测试覆盖。\n\n本地验证:mvn -pl agentscope-extensions/agentscope-extensions-model/agentscope-extensions-model-openai-official -am test -DskipITs 通过,274 tests, 0 failures/errors, 2 skipped。\n\n未发现需要阻塞合并的问题。

Aias00

This comment was marked as duplicate.

Aias00

This comment was marked as duplicate.

Aias00

This comment was marked as duplicate.

@Aias00 Aias00 left a comment

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.

Requesting changes after re-checking PR head 0bcf019. I found release-blocking correctness issues that should be fixed before merge.

[HIGH] Multi-agent formatter drops encrypted reasoning replay

OpenAIResponsesChatModel sends all history through formatter::formatHistory when a ResponsesMultiAgentFormatter is configured (OpenAIResponsesChatModel.java:112-122). In that path, ordinary assistant messages are classified as AGENT_CONVERSATION unless they contain tool blocks (ResponsesMultiAgentFormatter.java:172-181), and mergeAgentConversation rewrites them into a user <history> message (ResponsesMultiAgentFormatter.java:209-237). processMessage then serializes ThinkingBlock as visible text (ResponsesMultiAgentFormatter.java:305-312) and never delegates to ResponsesRequestMapper.mapAssistantMessage, which is the only code path that emits the Responses reasoning replay item from openai.reasoning.encrypted_content metadata (ResponsesRequestMapper.java:433-445, ResponsesRequestMapper.java:477-493).

This breaks stateless reasoning replay exactly for the documented reasoning + multi-agent formatter usage. Please preserve assistant messages with encrypted reasoning metadata as Responses reasoning input items, or route them through a passthrough/delegation path, and add a regression test that configures OpenAIResponsesChatModel with new ResponsesMultiAgentFormatter() and asserts second-turn input contains the encrypted ResponseReasoningItem.

[MEDIUM] Schema-level strict mode is ignored for JSON schema structured output

JsonSchema exposes strict (JsonSchema.java:91-96), but ResponsesRequestMapper only writes schemaBuilder.strict(...) from the model-level strictJsonSchema flag (ResponsesRequestMapper.java:178-187). A caller setting ResponseFormat.json_schema with JsonSchema.strict(true) silently gets a request without strict: true unless they also configure the model builder flag.

Please resolve strict as schema-level first, then builder-level fallback, e.g. schema.getStrict() != null ? schema.getStrict() : strictJsonSchema, and add tests for schema-level override plus builder-level fallback.

[HIGH] all-in-one shaded artifact can lose ModelProvider service registration

The new module is added to agentscope-all (agentscope-all/pom.xml:111-116) and has its own SPI file (agentscope-extensions-model-openai-official/src/main/resources/META-INF/services/io.agentscope.core.model.spi.ModelProvider:1). But the shade config only includes artifacts and has no ServicesResourceTransformer or equivalent service merge (agentscope-all/pom.xml:449-466). There are multiple model-provider service descriptors across extension modules, so the shaded jar can keep only one descriptor and drop OpenAIOfficialModelProvider, making ModelRegistry discovery fail for users depending on the all-in-one agentscope artifact.

Please add a service resource transformer to the shade plugin and a packaging regression check that the shaded jar's META-INF/services/io.agentscope.core.model.spi.ModelProvider contains io.agentscope.extensions.model.openaiofficial.OpenAIOfficialModelProvider.

CI is green, but these are behavioral/package correctness issues not covered by the current checks.

@Aias00 Aias00 left a comment

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.

One additional blocking issue affects the default formatter path as well.

[HIGH] The production ReActAgent path discards encrypted reasoning metadata before the next turn

ResponsesResponseParser stores openai.reasoning.encrypted_content only in the terminal ChatResponse.metadata (lines 105-106), and the streaming path does the same in ResponsesStreamingAssembler (lines 261-263). However, the production consumer, ReasoningContext.processChunk, never reads chunk.getMetadata(); buildFinalMessage() creates a fresh metadata map containing only MessageMetadataKeys.CHAT_USAGE. The resulting assistant Msg therefore cannot contain the key that ResponsesRequestMapper.mapAssistantMessage looks up at lines 434-444.

As a result, encrypted reasoning replay is unreachable through a normal ReActAgent call even without ResponsesMultiAgentFormatter. This is particularly important for stateless reasoning/tool-call loops, where the reasoning item from the prior response must be sent back with the function-call output. The current cross-turn tests miss this because they manually construct an AssistantMessage with openai.reasoning.encrypted_content already present instead of exercising ChatResponse -> ReasoningContext -> Msg -> next request.

Please preserve the terminal response metadata in the final assistant message (or place the replay data on a content block and ensure its accumulator preserves it), and add a ReActAgent-level two-iteration regression test that captures the second ResponseCreateParams and asserts it contains the prior encrypted reasoning item.

@zouyx zouyx self-assigned this Sep 11, 2026
@mintlify

mintlify Bot commented Sep 11, 2026

Copy link
Copy Markdown

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated
agentscope-java 🟡 Building Sep 11, 2026, 3:42 AM

💡 Tip: Enable Automations to automatically generate PRs for you.

Comment on lines +207 to +218
if (current instanceof OpenAIIoException
|| current instanceof OpenAIRetryableException) {
return true;
}

if (current instanceof IOException) {
return true;
}

if (current instanceof TimeoutException) {
return true;
}

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.

这几个不能整合吗?

oss-maintainer

This comment was marked as abuse.

@oss-maintainer oss-maintainer left a comment

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.

Summary

Introduces a new agentscope-extensions-model-openai-official module implementing the OpenAI Responses API (as distinct from the existing chat-completions extension): SDK-backed client factory, request mapper, response parser, a streaming assembler over the SDK's StreamResponse, an error translator, credential type, ModelProvider SPI registration, an e2e ProviderFactory provider, and docs in both locales. 46 files, ~9.3k diff lines, of which the substantive new module is roughly 1.6k lines of main and the rest tests and docs.

This is a strong, unusually careful module. I specifically checked the areas this repo's review guidance calls out and they hold up:

  • Credential handlingOpenAIOfficialCredential.toString() redacts the key to apiKey=***, and per-request apiKey overrides are explicitly rejected rather than silently honoured, which closes the usual leak-into-logs path.
  • Streaming reliabilitydoFinally(signal -> closeQuietly(streamResponse)) covers complete/error/cancel, terminal failed/error/incomplete events are translated rather than swallowed, and maxRetries(0) delegates retry to AgentScope with a moduleRetryOn() predicate rather than double-retrying.
  • Plugin isolation — SPI registration via META-INF/services plus aggregation/BOM wiring, and Check Module Sync is green.

The one real defect is a narrow stream-leak path; the rest is a scoping question about core.

Findings

  • [Warning] OpenAIResponsesChatModel.java:154StreamResponse is not closed if assemble(...) throws before its doFinally guard is attached (see inline comment for the mechanism and a fix).
  • [Warning] ModelContextWindows.java:72 — seven new model→context-window entries are added to agentscope-core, but they describe models served by this new extension and the same OPENAI table is consumed by the existing OpenAI extension. Core changes cascade to harness/distribution/extensions, so an inflated value mis-sizes compaction silently. A citable source per value plus a resolution test would make the table trustworthy.
  • [Info] ModelContextWindows.java:87 — the glm-5.3 entry is unrelated to this feature and would be easier to review and revert as its own change.

Suggestions

The stream cleanup is the only thing I would fix before merge; it is a few lines. If you prefer to keep the provider's model metadata with the provider, a per-extension context-window table that core consults through the SPI would also avoid the core edit entirely — not something to change in this PR, just worth a follow-up issue since every new provider module currently has to touch core.

Tests

Coverage is comprehensive (error translation, credential, SDK client factory, request mapper, response parser, streaming assembler, cross-turn, plus e2e wiring), and CI is green on ubuntu-latest with 8 checks passing. Note that a 9.3k-line diff means I reviewed the new module's main sources and the core/pom edits closely and sampled the tests and docs rather than auditing every assertion — the maintainers should still read the assembler state machine end to end.


Automated review by github-manager-bot

try {
StreamResponse<ResponseStreamEvent> streamResponse =
client.responses().createStreaming(params);
return ResponsesStreamingAssembler.assemble(

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.

[Warning] StreamResponse leak on the assembly failure path.

ResponsesStreamingAssembler.assemble(...) evaluates streamResponse.stream() eagerly as an argument to Flux.fromStream(...), and the doFinally(signal -> closeQuietly(streamResponse)) guard is only attached to the flux after that call returns. If stream() (or anything else in assemble before doFinally) throws, the exception propagates out of assemble, this catch (RuntimeException e) converts it to Flux.error(...), and the SDK stream is never closed — leaking the underlying HTTP connection for the remainder of the client's lifetime.

The window is narrow but it is exactly the path that fires when the upstream is already misbehaving, which is when connection hygiene matters most. Closing in the catch keeps the invariant local and obvious:

StreamResponse<ResponseStreamEvent> streamResponse = null;
try {
    streamResponse = client.responses().createStreaming(params);
    Flux<ChatResponse> assembled =
            ResponsesStreamingAssembler.assemble(streamResponse, modelName, start);
    streamResponse = null;
    return assembled;
} catch (RuntimeException e) {
    if (streamResponse != null) {
        closeQuietly(streamResponse);
    }
    return Flux.error(OpenAIErrorTranslator.translate(e, modelName));
}

Alternatively, move the doFinally attach inside assemble onto a local Flux built before anything else can throw.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants