Skip to content

Support MiniMax image generation schema - #8322

Merged
wenshao merged 4 commits into
QwenLM:mainfrom
octo-patch:octo/20260801-text-to-image-tool-recvqqFfIZ8U41
Aug 24, 2026
Merged

Support MiniMax image generation schema#8322
wenshao merged 4 commits into
QwenLM:mainfrom
octo-patch:octo/20260801-text-to-image-tool-recvqqFfIZ8U41

Conversation

@octo-patch

@octo-patch octo-patch commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

What this PR does

The image generation service now recognises MiniMax image base URLs and talks to MiniMax on its own terms instead of reusing the DashScope request shape. When the configured image base URL points at the MiniMax global or China host, the request is sent to the MiniMax image generation endpoint as a POST with a Bearer API key and the MiniMax field names (model, prompt, n, prompt_optimizer, response_format, plus width/height when an explicit size is requested). Responses are read from the MiniMax payload: the returned image list is accepted both as an HTTPS link, which is downloaded through the existing PNG download pipeline, and as inline base64 data, which is decoded directly and checked for a real PNG signature. Application-level failures that MiniMax reports inside its response envelope, including the ones delivered with an HTTP 200 status, are surfaced with the provider's own status code and message instead of a generic "no image URL" message. Finally, the two MiniMax image-only models are registered in the MiniMax provider preset so they can be picked as the image model.

The DashScope path is untouched: routing happens only for the two official MiniMax hosts on the image generation path, so every existing provider keeps its current behaviour, and the URL download branch continues to go through the same host validation, redirect handling, size cap and PNG signature checks as before.

Why it's needed

Users who run this CLI against MiniMax could configure a MiniMax provider for chat, but the image_gen tool was hardcoded to the DashScope generation path and request schema. Pointing the image model at MiniMax therefore produced a request MiniMax cannot answer, and even a successful MiniMax response could not be parsed, because the response shape differs from the DashScope one. The result was that image generation was simply unavailable for MiniMax users, on both the global and the China endpoint, with a confusing error rather than a clear one. This change makes the existing image_gen tool work for those users without asking them to switch providers for images only, and it covers both regional endpoints so China-based and international accounts are equally supported.

Reviewer Test Plan

How to verify

Unit level, no credentials needed:

npm ci
npm run test --workspace=packages/core -- --run src/services/image-generation-service.test.ts src/providers/__tests__/presets/minimax.test.ts

Expected: the suite passes, including the added cases that pin the MiniMax request URL for a regional base URL and for a fully qualified endpoint URL, the exact MiniMax request body, the base64 response branch, and the error message produced when MiniMax reports a non-zero status code inside its response envelope.

End to end, with a MiniMax API key: configure the MiniMax provider with the global base URL (or the China one), select image-01 as the image model, then ask the agent to generate an image so the image_gen tool runs. Expected: one POST to the MiniMax image generation endpoint carrying the MiniMax field names, and the returned PNG saved locally. Before this change the same configuration sent a DashScope-shaped request to a DashScope path and no image was produced. A failure injected on the MiniMax side (for example an invalid key) should now report MiniMax's own status code and message.

Regression check: the DashScope image path should behave exactly as before, since routing only triggers on the two MiniMax hosts.

Evidence (Before & After)

This is a service and provider-preset change with no TUI surface, so there are no screenshots. The observable before/after is on the wire and in the tool result: before, a MiniMax image base URL yielded a DashScope-shaped request and no image; after, it yields a MiniMax image generation request and a saved PNG. Both response shapes MiniMax can return (link and inline base64) are exercised by the added unit tests, and end-to-end runs against a local stand-in MiniMax origin are recorded in this thread.

Tested on

OS Status
🍏 macOS ⚠️
🪟 Windows ⚠️
🐧 Linux

Environment (optional)

Linux, Node 22, unit tests only for the local run; CI covers the cross-platform matrix and is green on this head.

Risk & Scope

  • Main risk or tradeoff: routing is keyed on an exact host and path match rather than a shared constant, so a future MiniMax endpoint or a proxy host would fall back to the previous behaviour until it is added here.
  • Not validated / out of scope: image-to-image and subject reference inputs, aspect ratio, seed, and generating more than one image per call; only single-image text-to-image is wired up. Non-PNG output is rejected rather than converted.
  • Breaking changes / migration notes: none. No existing provider, path or public API changes; the new models are additive entries in the MiniMax preset.

Linked Issues

None.

中文说明

这个 PR 做了什么

图像生成服务现在能够识别 MiniMax 的图像 base URL,并按 MiniMax 自己的规范发起请求,而不再复用 DashScope 的请求结构。当配置的图像 base URL 指向 MiniMax 的全球或中国站主机时,请求会以 POST 方式发送到 MiniMax 的图像生成端点,使用 Bearer 方式携带 API key,并采用 MiniMax 的字段名(modelpromptnprompt_optimizerresponse_format,在显式指定尺寸时再加上 width/height)。响应从 MiniMax 的返回体中解析:返回的图像列表既接受 HTTPS 链接(通过既有的 PNG 下载流程下载),也接受内联 base64 数据(直接解码,并校验真实的 PNG 签名)。MiniMax 在响应信封中报告的应用级错误,包括以 HTTP 200 状态返回的那些,现在会带上该服务自身的状态码和错误信息抛出,而不是一条笼统的"未包含图像 URL"提示。最后,两个 MiniMax 纯图像模型被注册到 MiniMax provider 预设中,从而可以被选为图像模型。

DashScope 的路径未被改动:只有那两个 MiniMax 官方主机上的图像生成路径才会触发路由,因此所有既有 provider 的行为保持不变,URL 下载分支也继续沿用与之前相同的主机校验、重定向处理、大小上限和 PNG 签名检查。

为什么需要

在 MiniMax 上使用本 CLI 的用户可以为对话配置 MiniMax provider,但 image_gen 工具的生成路径和请求结构是写死为 DashScope 的。因此把图像模型指向 MiniMax 只会产生一个 MiniMax 无法处理的请求;即使 MiniMax 成功返回,也无法被解析,因为其响应结构与 DashScope 不同。结果就是 MiniMax 用户在全球站和中国站上都无法使用图像生成,而且只会看到一个含义不清的报错。这次改动让既有的 image_gen 工具对这些用户可用,无需他们仅为了生成图像而切换 provider;同时覆盖两个区域端点,使中国境内和国际账号获得同等支持。

审查者测试计划

如何验证

单元测试层面,无需凭证:

npm ci
npm run test --workspace=packages/core -- --run src/services/image-generation-service.test.ts src/providers/__tests__/presets/minimax.test.ts

预期:测试套件通过,其中包含新增用例——分别针对区域 base URL 和完整端点 URL 固定 MiniMax 请求 URL、校验准确的 MiniMax 请求体、覆盖 base64 响应分支,以及当 MiniMax 在响应信封中报告非零状态码时所产生的错误信息。

端到端,需要 MiniMax API key:用全球站(或中国站)base URL 配置 MiniMax provider,选择 image-01 作为图像模型,然后让 agent 生成一张图片以触发 image_gen 工具。预期:向 MiniMax 图像生成端点发出一次携带 MiniMax 字段名的 POST,并把返回的 PNG 保存到本地。在此改动之前,同样的配置会把 DashScope 结构的请求发往 DashScope 路径,且不会产生任何图像。若在 MiniMax 侧注入一个失败(例如无效的 key),现在应当报出 MiniMax 自身的状态码和错误信息。

回归检查:DashScope 图像路径的行为应与之前完全一致,因为路由只在那两个 MiniMax 主机上触发。

证据(前后对比)

这是一处服务层与 provider 预设的改动,没有 TUI 界面,因此没有截图。可观察的前后差异体现在网络请求和工具结果上:改动前,MiniMax 图像 base URL 会产生 DashScope 结构的请求且没有图像;改动后,它会产生 MiniMax 图像生成请求并保存一个 PNG。MiniMax 可能返回的两种响应形态(链接与内联 base64)都由新增单元测试覆盖,针对本地替身 MiniMax 源站的端到端运行记录也保留在本讨论串中。

测试环境

系统 状态
🍏 macOS ⚠️
🪟 Windows ⚠️
🐧 Linux

运行环境(可选)

Linux、Node 22,本地只运行了单元测试;CI 覆盖跨平台矩阵,且在当前 head 上为绿色。

风险与范围

  • 主要风险或权衡:路由依赖精确的主机与路径匹配,而非共享常量,因此未来新增的 MiniMax 端点或代理主机在被加入这里之前会回退到原有行为。
  • 未验证/不在范围内:图生图与主体参考输入、宽高比、seed,以及单次调用生成多张图;目前只接通了单图的文生图。非 PNG 输出会被拒绝而不是转换。
  • 破坏性变更/迁移说明:无。没有改动任何既有 provider、路径或公共 API;新增模型只是 MiniMax 预设中的追加条目。

关联 Issue

无。

@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator

Qwen Triage finishedview run. See the stage comments in this thread for the result.

Qwen Triage 已完成 —— 查看运行。结果见本线程中的各阶段评论。

@qwen-code-ci-bot qwen-code-ci-bot 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.

Thanks for the PR, @octo-patch! Before we can review the code, the PR body needs to follow the pull request template.

Right now the description uses free-form sections (Reason, Changes, Checks) and is missing the required headings:

  • ## What this PR does
  • ## Why it's needed — the motivation / user-facing benefit
  • ## Reviewer Test Plan (with ### How to verify, ### Evidence (Before & After), ### Tested on)
  • ## Risk & Scope
  • ## Linked Issues
  • The bilingual <details> Chinese translation

Could you edit the PR body to fill in the template? In particular, "Why it's needed" should explain the motivation (who wants MiniMax image generation and why), and the Reviewer Test Plan should describe how a maintainer can confirm the behavior. Once the body follows the template, re-run with @qwen-code /triage and we'll continue the review.

中文说明

感谢贡献,@octo-patch!在进入代码审查之前,PR 正文需要遵循 PR 模板

目前描述使用的是自由格式的小节(ReasonChangesChecks),缺少必需的标题:

  • ## What this PR does
  • ## Why it's needed —— 动机 / 面向用户的收益
  • ## Reviewer Test Plan(包含 ### How to verify### Evidence (Before & After)### Tested on
  • ## Risk & Scope
  • ## Linked Issues
  • 双语的 <details> 中文翻译

请编辑 PR 正文以填写模板。其中 "Why it's needed" 应说明动机(谁需要 MiniMax 图像生成、为什么需要),Reviewer Test Plan 应描述维护者如何确认该行为。正文遵循模板后,使用 @qwen-code /triage 重新运行,我们会继续审查。

Qwen Code · qwen3.8-max-preview

@wenshao

wenshao commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator

Review

Overview

Adds MiniMax image generation support to generateImage: requests to api.minimax.io / api.minimaxi.com (path /v1 or /v1/image_generation) are routed to a MiniMax-specific branch with the MiniMax request schema, and responses are parsed from data.image_urls (both URL and base64 forms). Two image-only model entries (image-01, image-01-live) are added to the MiniMax preset.

What looks good

  • The URL-download path reuses the existing downloadPng pipeline, so hostname validation/pinning (validateResultUrlresolveNetworkTarget), manual redirect handling, the 10 MB size cap, and the PNG signature check all still apply. No SSRF regression.
  • Routing is keyed on an exact hostname allowlist, so it can't be triggered by lookalike hosts, and the generation POST (with the API key) only ever goes to the hardcoded MiniMax hosts.
  • The base64 branch validates the PNG signature before accepting bytes, and the payload is already bounded by MAX_API_RESPONSE_BYTES.
  • Tests cover the three main paths (regional base URL, full endpoint URL, base64 response).

Issues

  1. MiniMax application-level errors are swallowed (generateMiniMaxImage). MiniMax reports most failures as HTTP 200 with base_resp.status_code != 0 (rate limit 1002, insufficient balance 1008, content moderation, invalid params). Since the code only branches on response.ok, all of these surface as the generic Image generation response did not contain an image URL., hiding the actual cause from the user. Please check base_resp.status_code !== 0 after parsing and surface base_resp.status_msg. Relatedly, formatImageGenerationError only reads top-level code/message, which MiniMax doesn't use, so even non-2xx errors lose their detail — worth passing base_resp fields through there for the MiniMax path. A test for the 200-with-error-base_resp case would lock this in.

  2. Bare-host base URL silently falls through to the DashScope branch. https://api.minimax.io (no /v1) fails isMiniMaxImageGenerationBaseUrl (normalized path is '') and then gets the DashScope schema posted to https://api.minimax.io/services/aigc/multimodal-generation/generation, producing a confusing 404. Consider accepting an empty path for the MiniMax hosts (appending /v1/image_generation), or failing fast with a clear message.

Suggestions (minor)

  • MINIMAX_IMAGE_GENERATION_PATH.slice(3) is a magic offset that only works because the base path is exactly /v1. `${request.baseUrl}/image_generation` (or a dedicated suffix constant) says the same thing without the trap.
  • In findMiniMaxGeneratedImage, anything that isn't https:// is classified as base64 — an http:// URL in the response would fail later with the misleading did not contain a valid PNG image error instead of the existing "not a safe public HTTPS URL" message. An explicit http:// rejection would keep diagnostics consistent with the DashScope path.
  • Base64 images are effectively capped at ~1 MB (MAX_API_RESPONSE_BYTES applies to the whole JSON body) while URL downloads allow 10 MB. Since response_format: 'url' is hardcoded this is unlikely to bite, but if MiniMax ever returns base64 for a large image the failure will be a generic size-limit error.
  • generateMiniMaxImage duplicates ~40 lines of fetch/error-handling from the default path. A small shared postJson-style helper would keep the two branches from drifting.

None of these block the mainline flow — item 1 is the one I'd fix before merging, since it directly affects how users experience every MiniMax-side failure.

@qwen-code-ci-bot qwen-code-ci-bot 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.

Reviewed. Suggestions are inline.

— qwen3.8-max-preview via Qwen Code /review

Comment on lines +198 to +201
throw new Error(formatImageGenerationError(response.status, payload));
}
const payload = await readJsonResponse(response, MAX_API_RESPONSE_BYTES);
const image = findMiniMaxGeneratedImage(payload);

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.

[Suggestion] This !response.ok error branch in generateMiniMaxImage (~10 lines: the JSON-body read with fallback plus formatImageGenerationError) has no test coverage, while the equivalent DashScope branch is covered by two tests (HTTP 429 throttling and non-JSON error body). — Failure scenario: MiniMax returns 429/401 and this path runs in production unexercised; if a later refactor drops the try/catch around readJsonResponse, a malformed error body throws an unhandled ResponseSizeLimitError instead of the user-facing message and no test catches the regression. Suggested fix: add a test that sends a non-200 response (e.g. 429 with a JSON error body) to a MiniMax base URL and asserts the rejection message matches /rate limit/i and fetchFn is called exactly once (no download attempted).

— qwen3.8-max-preview via Qwen Code /review

Comment on lines +332 to +335
(parsed.hostname === 'api.minimax.io' ||
parsed.hostname === 'api.minimaxi.com') &&
(normalizedPath === '/v1' ||
normalizedPath === MINIMAX_IMAGE_GENERATION_PATH)

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.

[Suggestion] This hostname allowlist is implicitly coupled to the preset's baseUrl array in packages/core/src/providers/presets/minimax.ts, with no shared constant, comment, or test keeping them in sync. — Failure scenario: a maintainer adds a new MiniMax regional endpoint (e.g. https://api.minimax.eu/v1) to the preset's baseUrl array; with no signal that this routing function also needs updating, image-01 requests on that endpoint silently fall through to the DashScope request schema and fail with a confusing 404 / "did not contain an image URL" error that gives no hint the root cause is a missing hostname here. Suggested fix: export a shared MINIMAX_IMAGE_HOSTNAMES set used by both the preset and this function, or add a test asserting every hostname in the preset's baseUrl is recognized here.

— qwen3.8-max-preview via Qwen Code /review

Comment on lines +364 to +369
if (/^https:\/\//i.test(value)) {
return { kind: 'url', value };
}
return { kind: 'base64', value };
}
return undefined;

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.

[Suggestion] This for loop never iterates past the first non-empty string: once a non-empty candidate is reached, the unconditional return { kind: 'base64', value } exits, so the for/continue structure implies multi-candidate scanning that the control flow defeats. — Failure scenario: if MiniMax ever returns a non-image string before a valid entry (e.g. image_urls: ["content_filter_notice", "https://cdn.example.com/img.png"]), the first string is classified as base64, decodePngBase64Image throws "did not contain a valid PNG image", and the valid HTTPS URL at index 1 is never examined (n: 1 is pinned today, so this is unlikely in practice, but the structure misleads anyone extending to n > 1). Suggested fix: continue past unrecognized entries instead of returning unconditionally, falling through to return undefined only when no usable entry is found.

— qwen3.8-max-preview via Qwen Code /review

Comment on lines +372 to +374
function decodePngBase64Image(value: string): Buffer {
const match = value.match(/^data:image\/png;base64,(.+)$/i);
const base64 = (match?.[1] ?? value).trim();

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.

[Suggestion] The PNG-signature validation in decodePngBase64Image has no negative test — removing the check leaves every existing test green (the only base64 test feeds valid PNG bytes). The DashScope download path has two equivalent negative tests ("rejects a download that is not a PNG image" and "rejects a download with only a partial PNG signature"); this base64 path has none. — Failure scenario: if the signature check is accidentally removed or inverted, non-PNG base64 data (a JPEG or truncated payload from a misbehaving endpoint) is returned to the caller as mimeType: 'image/png' with no error, producing a corrupt image downstream. Suggested fix: add a test that puts base64-encoded non-PNG bytes (e.g. Buffer.from('not a png').toString('base64')) in data.image_urls and asserts the rejection message matches /valid PNG/i.

— qwen3.8-max-preview via Qwen Code /review

@doudouOUC doudouOUC 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.

Reviewed — no blockers. Suggestions are inline.

— qwen3.7-max via Qwen Code /review

): Promise<GeneratedImage> {
const generationUrl = request.baseUrl.endsWith(MINIMAX_IMAGE_GENERATION_PATH)
? request.baseUrl
: `${request.baseUrl}${MINIMAX_IMAGE_GENERATION_PATH.slice(3)}`;

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.

[Suggestion] .slice(3) is a magic-number extraction that silently depends on the internal structure of MINIMAX_IMAGE_GENERATION_PATH ('/v1/image_generation'). If the version prefix changes (e.g. '/v2/image_generation'), .slice(3) produces '2/image_generation' instead of '/image_generation', yielding a malformed URL like https://api.minimax.io/v12/image_generation.

Consider extracting the subpath semantically:

Suggested change
: `${request.baseUrl}${MINIMAX_IMAGE_GENERATION_PATH.slice(3)}`;
: `${request.baseUrl}${MINIMAX_IMAGE_GENERATION_SUBPATH}`;

Where MINIMAX_IMAGE_GENERATION_SUBPATH = '/image_generation' is a separate constant, or use MINIMAX_IMAGE_GENERATION_PATH.replace(/^\/v\d+/, '') to derive it.

— qwen3.7-max via Qwen Code /review

} catch {
// non-JSON error body — formatImageGenerationError handles missing fields
}
throw new Error(formatImageGenerationError(response.status, payload));

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.

[Suggestion] MiniMax error responses nest diagnostic information under base_resp: { status_code, status_msg }, but formatImageGenerationError reads only top-level code and message fields. MiniMax-specific error details are silently discarded.

Additionally, MiniMax can return HTTP 200 with application-level errors (base_resp.status_code !== 0 and empty image_urls), which produces the misleading "Image generation response did not contain an image URL." error instead of the real cause (authentication failure, rate limit, content moderation).

Consider extracting base_resp error fields before calling formatImageGenerationError, and checking base_resp.status_code after parsing the success response.

— qwen3.7-max via Qwen Code /review

@wenshao

wenshao commented Aug 8, 2026

Copy link
Copy Markdown
Collaborator

@qwen-code /triage

@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 8, 2026

Copy link
Copy Markdown
Collaborator

Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run

Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check.

Scripted assertions: 113 passed · 0 failed · 113 total

中文 — 判定:✅ 通过 · 可合入(agent 判定)

沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查

脚本断言:113 通过 · 0 失败 · 113 总计

Verification report

PR 8322 — Support MiniMax image generation schema — deep verification (follow-up round)

Verdict: merge-ready — 113/113 scripted assertions passed, 0 unexpected failures. Verified head: 434d911e6b9117c209f337baf8dad8f06a901bc1 (git rev-parse HEAD^2, == metadata headRefOid). Base tip: 0e8a361b3723abae6b4cfbff3568714ed6dd0975. This is a follow-up round; the previous round verified head a98e3f6 and reported F1 (medium), F2 (low), F3 (nit). The PR since added 959c8e6 (error surfacing — targets F1) and 434d911 (path construction); every carried measurement was rebuilt and re-run at the new head, not diffed from the old report.

中文 — 判定:✅ 通过(merge-ready)· 113/113 脚本断言全部通过
  • 结论:merge-ready。上轮核心 Finding 1(base_resp 错误原因被丢弃)已修复并经 A/B 复测:head 对 HTTP 200 + base_resp.status_code != 0 抛 "Image generation failed (1004: API key invalid)."(不再含误导性 "with HTTP 200"),非 2xx 错误信息带厂商 status_code: status_msg 后缀(见表 2 与 01-/02- 截图);变异矩阵 MA/MB/ME 均被新增单测精确钉死。
  • 遗留(不阻塞):Finding 2(低)仍成立——base64 分支 PNG 签名守卫无测试钉住(MC 变异存活;守卫本身经 05- 探针 5/5 证实有效,属覆盖缺口)。Finding 3(微)仍成立——0*0 等退化尺寸仍原样转发(H14 复测)。
  • 未覆盖:真实 MiniMax API 契约(沙箱无外网);URL 下载成功路径端到端(SSRF 策略按设计拦截回环,H2 断言守卫);UI 层 E2E;逐 commit 归因(浅克隆 depth 2,仅 3 个 commit 可达,验证的是聚合 diff)。

Previous-finding status at the new head

# Finding (round 1) Severity Status at 434d911 Evidence
F1 base_resp error cause discarded (HTTP-200 envelope → generic "no image URL"; non-2xx loses status_msg) medium fixed Table 2 cells H5/H6/H8/H9 vs base B2/B3; new unit test pins it; mutants MA/MB/ME killed
F2 base64-branch PNG signature guard not pinned by any test low stands (coverage gap, non-blocking) Mutant MC survives at new head (21/21 green); guard itself re-proven effective, witness 05-guard-probe.png
F3 parseImageSize forwards degenerate dimensions (0*0) nit stands (non-blocking) Cell H14 re-measured: width:0/height:0 still sent on the wire

On F1's fix shape: the author's implementation differs from the round-1 suggested patch and is a superset of it — readStringOrNumber accepts numeric and string status_code (cells H5/H6), top-level code/message keep precedence over base_resp (H12), and the with HTTP 2xx text is now suppressed for application-error envelopes (H5), which the round-1 patch would still have printed.

Central claim + A/B

Central claim: generateImage() routes MiniMax base URLs (api.minimax.io / api.minimaxi.com, path /v1 or /v1/image_generation) to the MiniMax image_generation schema end-to-end (request shape on the wire, URL + base64 response parsing, error surfacing), and the preset's image-01/image-01-live imageOnly entries exist in the built artifact.

Oracle: a real TLS peer on 127.0.0.1:8443 impersonating the vendor hostnames (self-signed CA via NODE_EXTRA_CA_CERTS + process-level dns.lookup shim; default fetchFn, unit loaded from compiled dist; every byte crosses a real socket). Witnesses: 01-ab-head-wire-oracle.png (head), 02-ab-base-control.png (base).

Table 1 — routing, wire schema, response parsing

Cell Build Oracle observation Result
H1 anchor head POST /v1/image_generation, body {model,prompt,n:1,prompt_optimizer:true,response_format:'url',width:1024,height:1024}, Bearer auth; base64 PNG decoded; requestId from base_resp.request_id; exactly 1 wire request PASS
anchor base same scenario → POST /v1/services/aigc/multimodal-generation/generation with DashScope body (input.messages, parameters.size string, no prompt_optimizer/width); MiniMax success envelope unparseable ("did not contain an image URL") predicted failure observed (7/7 control assertions)
H2 head URL response → download leg enters validateResultUrl, loopback resolution blocked by the SSRF guard ("not a safe public HTTPS URL"), exactly 1 wire request PASS (defense preserved on the new path)
H3/H4 head full-path /v1/image_generation baseUrl used verbatim (no duplication); trailing-slash baseUrl normalized (no //image_generation on the wire) PASS
H13 head size matrix over the wire: 1024x768 / 2048 * 1152 / 1024X512 → int width/height; undefined / banana / unsafe-int → fields omitted PASS (6 sub-cells)
H15–H17 head api.minimax.io.evil.test/v1, api.notminimax.test/v1, api.minimax.io (bare host) all stay DashScope-shaped — hostname compared by exact equality, path requires /v1 PASS (no over-matching)
preset head vs base compiled dist: head exposes imageOnly models ["image-01","image-01-live"], base exposes [] PASS, witness 06-preset-ab.png

Table 2 — error surfacing (the delta since round 1)

Cell Build Scenario → observed message Result
H5 (was F1/cell8) head HTTP 200 + base_resp{status_code:1004, status_msg:'API key invalid'}Image generation failed (1004: API key invalid). PASS — F1 fixed; base cell B2 on the same payload still shows the cause-less generic error
H6 head string status_code:'2013'Image generation failed (2013: internal error). PASS
H7 head numeric status_code:0 → success path, bytes returned (zero not treated as error) PASS
H8 head 401 + base_resp body → Image generation access denied (1004: API key invalid). Check the API key…; base B3 lacks the vendor suffix PASS — non-2xx half of F1 fixed
H9 head 429 + base_respImage generation rate limit reached (1014: system busy). PASS
H10 head 502 non-JSON body → Image generation failed with HTTP 502. PASS
H11/H12 head shared formatter unchanged for top-level payloads: moderation branch identical; top-level code/message win over base_resp PASS
H14 (F3) head size:'0*0'width:0/height:0 still forwarded PASS (documents standing nit)

Test pinning: vacuity + mutation matrix at the new head

Vacuity (witness 04-vacuity-base-src.png): head's test file run against base source in the base worktree — exactly the 4 new service tests fail on behavioral assertions (e.g. expected Image generation failed (1008: insufficient balance)., received Image generation response did not contain an image URL.); all 17 pre-existing tests stay green (4 failed | 17 passed of 21). The tests pin behavior, not tautology.

Mutation matrix (witness 03-mutation-matrix.png, logs logs/mutant-M*.log); unmutated control green at 21/21; every kill names its killing test, and the MA revert failure is the intended expected-vs-received mismatch:

Mutant Guard removed Suite result Killing test / classification
MA HTTP-200 base_resp throw on success path killed (1 failed | 20 passed) surfaces MiniMax application errors returned with HTTP 200
MB base_resp fallback in formatImageGenerationError killed (1 failed | 20 passed) same test
MC PNG signature check in decodePngBase64Image survived (21/21 green) coverage gap — guard proven effective by probe G1–G5; no test supplies an invalid-PNG base64 fixture (F2 stands)
MD verbatim /v1/image_generation branch (always append suffix) killed (1 failed | 20 passed) accepts a full MiniMax image generation endpoint
ME numeric status_code handling in readStringOrNumber killed (1 failed | 20 passed) surfaces MiniMax application errors returned with HTTP 200

Positive control: MA/MB/ME are each killed by exactly the test the commit added for that behavior; restores verified byte-identical (sha256) after every mutant.

Findings

No new findings this round. Carried over, both non-blocking:

  1. F2 (low, stands) — the base64 branch's PNG signature guard is still pinned by no test. It demonstrably works (probe: JPEG bytes behind a data:image/png;base64, prefix, raw non-PNG base64, a data:image/jpeg URI, and a short body all rejected with "valid PNG"; valid PNG still accepted — 5/5, 05-guard-probe.png), so this is a completeness gap, not a defect. Suggested pin (unchanged from round 1): one unit test feeding data:image/png;base64,<JPEG-magic base64> expecting /valid PNG/ — that fixture is exactly what mutant MC needs to die.
  2. F3 (nit, stands)parseImageSize('0*0'){width:0,height:0} still forwarded to the vendor (cell H14); out-of-range values likewise pass through to server-side rejection. Harmless; a range check would match the care elsewhere in this path.

Not covered

  • Per-commit attribution. Metadata lists 3 commits; the checkout is depth 2 (only merge, base tip, PR head reachable), so 959c8e6 and 434d911 were verified only as the aggregate HEAD^1..HEAD diff. (git rev-list --count HEAD^1..HEAD^2 returns 1 here — a shallow-boundary artifact, not evidence of a single commit.)
  • Real MiniMax API contract. No outbound network in this sandbox; the peer's envelopes are modeled on the PR's own tests plus the vendor's documented base_resp style. Whether MiniMax actually returns HTTP-200 application errors (now handled and unit-pinned either way) or enforces width/height ranges (F3) remains unverified here. This reproduces the wire shape the PR targets, not a live vendor session.
  • URL-download happy path end-to-end. resolveNetworkTarget(…, 'public') blocks every loopback/private resolution by design, so no loopback peer can serve the download leg. The URL branch is covered by H2 (enters validation, SSRF guard holds, no second wire request), the PR's mock-based test, and the pre-existing downloadPng tests (unchanged code, green in the 21/21 control).
  • Port fidelity. Harness uses :8443 (uid 1000 cannot bind 443); the matcher compares parsed.hostname only, so the code path is identical.
  • UI/CLI layer (/model --image, ModelDialog) not driven E2E; the imageOnly pipeline it consumes is pre-existing and gated here by the preset dist A/B (06-), minimax.test.ts 4/4, and a code-path read of resolveImageGenerationModel (preset default baseUrls https://api.minimax.io/v1 / https://api.minimaxi.com/v1 both match the router).
  • Repo-wide gates not re-run (the PR's CI covers them). Targeted gates run here: core typecheck clean; eslint clean on the 4 changed files with the gate proven live (planted unused-var caught: @typescript-eslint/no-unused-vars, then reverted); PR test files 25/25 at head.

Methodology

Environment: CI node:22-bookworm container, merge-ref checkout (HEAD^1=0e8a361 base tip, HEAD^2=434d911 head); head dist CI-built. Wire oracle: harness/harness-ab.mjs + harness/probe-guard.mjs load the compiled unit by absolute path and drive it with the default fetchFn against an in-process TLS peer on 127.0.0.1:8443 (CA trusted via NODE_EXTRA_CA_CERTS, hostnames redirected by the dns-shim.cjs --require shim patching dns.lookup/dns.promises.lookup); the peer records method/path/headers/body of every request. Base control: git worktree add tmp/base-tree HEAD^1, built with the repo's own build_package.js after restoring the resolution layout the nested tree lacks (symlinks of the root and package-local node_modules, both installed from the PR-untouched lockfile — final build exit 0, log in logs/base-build.log). The first two build attempts exited 1 on type errors caused purely by the nested layout (the tsconfig paths map resolves @lydell/node-pty against <tree-root>/node_modules, and hoisted deps live in the main tree's package-local node_modules); no source change was needed, only the symlinks, so the control differs from head by nothing but the PR diff. Control purity asserted in-harness: loaded unit realpath inside the arm tree, MiniMax marker count 5 at head vs 0 at base (CTL.1/CTL.2), and identical undici realpath (7.28.0) on both arms (logs/dep-parity.log). Vacuity: head test file copied into the base worktree and run against base source via vitest. Mutations applied to a scratch copy of the head source with sha256-verified restores (harness/mutate.mjs, harness/run-mutations.sh). Raw per-cell logs in logs/; all harnesses rerunnable from harness/. Assertion totals: head harness 42, base harness 9, guard probe 5, preset A/B 2, mutation matrix 5, head test gate 25, vacuity 21 (4 expected-red counted as passing vacuity assertions), typecheck 1, eslint 2 (clean + planted-live), dependency parity 1 → 113 pass / 0 fail / 113 total.

Evidence images

01-ab-head-wire-oracle

02-ab-base-control

03-mutation-matrix

04-vacuity-base-src

05-guard-probe

06-preset-ab

Harness scripts and raw logs are in the workflow run artifacts (7-day retention).

Qwen Code · sandboxed verification

@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 8, 2026

Copy link
Copy Markdown
Collaborator

Triage re-ran on Aug 12 at the same head 42c0ea44 — no new commits since the Aug 8 run, and the PR body is unchanged — so it stops at the same Stage 1a gate as before: the PR body still does not follow the pull request template. All required sections are missing — What this PR does, Why it's needed, Reviewer Test Plan (How to verify / Evidence / Tested on), Risk & Scope, Linked Issues, and the Chinese <details> translation. The body is still the free-form Reason / Changes / Checks layout.

The request-changes review from Aug 1 still gates the PR on this, so no duplicate review was submitted — and per the gate rules this run stops here: no code review pass, no approval.

The split from last time still holds:

  • The code side is in good shape. The follow-up commits surfaced MiniMax base_resp errors and cleaned up the endpoint matching; the maintainer re-verified on a real stack at this exact head (19/19 scenario assertions, "still works as intended — merge-ready") and re-approved. Nothing on the code side blocks.
  • The gate is about the PR description, not the code, and only the author can clear it.

@octo-patch same one thing left from you: rewrite the PR body following the template. Your existing verification story (unit tests + the maintainer's real-stack runs) slots straight into the Reviewer Test Plan section. After that, trigger @qwen-code /triage again and the run will continue with code review + CI evidence.

Maintainer notes:

  • CI has moved since Aug 8: the fork's workflow on this head is approved and running — at the time of this run Test (ubuntu-latest, Node 22.x) is still in progress, all other lanes are success or skipped, none red. (For the record: the red Test lane on the previous head was an infrastructure failure — the fork branch predated Aug 5, so its tree was missing .github/actions/verify-checkout-head — not a test failure.)
  • If you'd rather waive the template gate given the approvals already standing, dismissing the bot's stale request-changes review unblocks the PR; otherwise the body fix above is the unblock path.
中文说明

Triage 于 8 月 12 日在同一个 head 42c0ea44 上重新运行 —— 自 8 月 8 日以来没有新提交,PR 正文也未改动 —— 因此仍停在同一个 Stage 1a 门禁:PR 正文仍未遵循 PR 模板。所有必填小节均缺失 —— What this PR doesWhy it's neededReviewer Test Plan(How to verify / Evidence / Tested on)、Risk & ScopeLinked Issues,以及中文 <details> 翻译。正文仍是自由格式的 Reason / Changes / Checks 布局。

8 月 1 日的 request-changes 评审 至今仍因此卡住该 PR,所以没有重复提交评审 —— 按照门禁规则,本次运行到此为止:不做代码审查,也不做批准。

上次的结论依然成立:

  • 代码侧状态良好。 后续提交透出了 MiniMax base_resp 错误并清理了端点匹配;维护者已在这个 head 上重新做了真实栈验证(19/19 场景断言,"still works as intended — merge-ready")并再次批准。代码侧没有阻塞项。
  • 门禁针对的是 PR 描述而非代码,且只有作者能解除。

@octo-patch 仍然只差一件事:按模板重写 PR 正文。你已有的验证材料(单测 + 维护者的真实栈验证)可以直接填入 Reviewer Test Plan 小节。完成后再触发 @qwen-code /triage,流程将继续进行代码审查与 CI 证据核验。

维护者注意:

  • CI 状态相比 8 月 8 日已有变化:该 fork 在此 head 上的工作流已获批准并正在运行 —— 截至本次运行,Test (ubuntu-latest, Node 22.x) 仍在进行中,其余通道为 success 或 skipped,无失败。(历史说明:上一个 head 上 Test 通道的失败是基础设施故障 —— fork 分支基于 8 月 5 日之前的 main,树中缺少 .github/actions/verify-checkout-head —— 而非测试失败。)
  • 如果考虑到已有批准、希望豁免模板门禁,dismiss 机器人的过期 request-changes 评审即可解除阻塞;否则请按上述作者操作处理。

Qwen Code · qwen3.8-max

Reviewed at 42c0ea44046cdac011d20ffda485cc133ffa38a2 · re-run with @qwen-code /triage

@wenshao

wenshao commented Aug 8, 2026

Copy link
Copy Markdown
Collaborator

Local verification on a real stack

I built this PR locally and ran the real CLI against a fake MiniMax origin to see what actually goes over the wire, rather than relying on the unit tests alone. Summary: the happy paths work end to end, and the two issues from my earlier review reproduce in the real stack. Neither blocks the mainline flow, but issue ① changes what a user sees on every MiniMax-side failure.

How this was verified

PR head a98e3f61fe
Base (merge-base with main) 1be199ae82
Runner sealed Docker container (Debian 12, Node v22.23.2), host macOS 15 (arm64)
CLI under test dist/cli.js bundled from this PR's source (npm ci && npm run build && npm run bundle)

The container has no internet. Inside it:

  • api.minimax.io and api.minimaxi.com resolve (via /etc/hosts) to a local HTTPS server holding a leaf certificate for those exact names, signed by a throwaway CA trusted through NODE_EXTRA_CA_CERTS. So hostname routing, TLS and DNS are all exercised for real — nothing is stubbed inside the process.
  • The image CDN host resolves to 93.184.216.34, a publicly-routable address assigned to the container from a custom Docker subnet. This matters: downloadPngvalidateResultUrlresolveNetworkTarget(…, 'public') rejects private addresses, so pointing the CDN at loopback would have silently skipped the download path instead of testing it.
  • A scripted OpenAI-compatible endpoint drives the real agent loop into a genuine image_gen tool call. It never touches the image code.
  • Every request to the fake MiniMax and CDN origins is appended to a JSONL ledger. The ledger is the evidence — the assertions below are about what the CLI actually sent, not about what a mock was told to return.

Model routing came from settings, the same way a user would have it:

{
  "imageModel": "image-01",
  "modelProviders": { "openai": [
    { "id": "image-01", "envKey": "MINIMAX_API_KEY",
      "baseUrl": "https://api.minimax.io/v1", "imageOnly": true }
  ]}
}

Results

# Scenario Request the CLI actually sent Outcome
1 baseUrl = …minimax.io/v1, size=1024*1024 POST /v1/image_generation + width/height ✅ PNG saved, 189 561 bytes
2 baseUrl = …minimaxi.com/v1/image_generation, image-01-live POST /v1/image_generation (no path doubling) ✅ PNG saved
3 base64 (data:image/png;base64,…) response POST /v1/image_generation ✅ PNG saved, CDN never contacted
4 baseUrl = …minimax.io (no /v1) POST /services/aigc/multimodal-generation/generation ⚠️ issue ② — HTTP 404
5 HTTP 200 + base_resp.status_code = 1008 POST /v1/image_generation ⚠️ issue ① — "did not contain an image URL"
6 HTTP 429 POST /v1/image_generation ✅ "Image generation rate limit reached."
7 result URL is http://… POST /v1/image_generation ⚠️ misleading message (see below)
8 result URL is https://127.0.0.1/… POST /v1/image_generation ✅ correctly refused as unsafe

Scenario 1, verbatim from the ledger:

{"host":"api.minimax.io","method":"POST","path":"/v1/image_generation",
 "authorization":"Bearer <redacted>",
 "body":{"model":"image-01","prompt":"A teal product card with a bold border",
         "n":1,"prompt_optimizer":true,"response_format":"url",
         "width":1024,"height":1024}}
{"host":"cdn.minimax-assets.io","method":"GET","path":"/gen/demo.png","accept":"image/png"}

The saved artifact is byte-identical to what the fake origin served (sha256 c1b42cde… on both sides), so nothing in the download/verify/write chain mutates the image. In scenario 3 the base64 branch produced the 256×256 fixture byte-for-byte with exactly one HTTP request — the download really is skipped.

Before / After

Same prompt, same settings; only the CLI bundle differs.

After (this PR) — the tool succeeds and writes the artifact:

after

Before (main) — the DashScope schema is posted to …minimax.io/v1/services/aigc/multimodal-generation/generation and 404s:

before

The generated artifact itself, read back from .qwen/generated-images/:

Preset change, end to end

I walked the real /auth wizard rather than hand-writing settings. image-01 and image-01-live show up as recommended models:

wizard

Completing the wizard writes them with "imageOnly": true, they are excluded from the normal /model picker (which only offered the chat model), and they appear in /model --image:

image picker

One thing to note from that screenshot: both entries are labelled text in the modality column, and every recommended model is checked by default. Functionally harmless — imageOnly is written correctly and the chat picker filters them — but the label is wrong for an image-only model and is worth a follow-up in the wizard, not necessarily in this PR.

Findings

base_resp.status_code != 0 on an HTTP 200 is swallowed. MiniMax reports rate limits (1002), insufficient balance (1008), moderation rejections and invalid params as HTTP 200 with a non-zero base_resp.status_code. generateMiniMaxImage only branches on response.ok, so all of them collapse into:

Image generation failed: Image generation response did not contain an image URL.

The real cause ("insufficient balance") was sitting in the response body and never reaches the user. Checking base_resp.status_code !== 0 after parsing and surfacing base_resp.status_msg would fix every one of these at once. This is the one I'd want addressed before merge — it is the difference between a user knowing they are out of credit and a user filing a bug. A test for the 200-with-error-base_resp case would lock it in.

② A bare-host base URL silently falls through to the DashScope branch. https://api.minimax.io (no /v1) fails isMiniMaxImageGenerationBaseUrl — the normalized path is '' — and then gets the DashScope schema posted to a MiniMax host:

{"host":"api.minimax.io","path":"/services/aigc/multimodal-generation/generation",
 "body":{"model":"image-01","input":{"messages":[]},
         "parameters":{"n":1,"prompt_extend":true,"watermark":false}}}

The user sees a bare HTTP 404 with no hint that the base URL is the problem. Accepting an empty path for the MiniMax hosts (and appending /v1/image_generation) or failing fast with a clear message would both work.

③ Minor: an http:// result URL produces the wrong diagnostic. In findMiniMaxGeneratedImage anything that is not https:// is classified as base64, so an http:// URL fails later in the PNG-signature check as "did not contain a valid PNG image" instead of the existing "not a safe public HTTPS URL" message the DashScope path gives. No security impact — scenario 8 confirms a private-IP https:// URL is still correctly refused, and the URL branch still goes through validateResultUrlresolveNetworkTarget with hostname pinning, the 10 MB cap, manual redirect handling and the PNG signature check. It is purely a diagnostics inconsistency.

Checks run against this PR

Check Result
npm run typecheck --workspace=packages/core
eslint on the four changed files, --max-warnings 0
packages/coresrc/services, src/providers, image-gen.test.ts, config.test.ts ✅ 2 645 passed, 3 skipped, 70 files
PR's own new tests ✅ 32 passed

One process note that cost me a false failure: minimax.test.ts imports minimaxProvider from the package entry @qwen-code/qwen-code-core, so it reads packages/core/dist, not the source. A stale dist makes the new preset assertion fail even when the source is correct — run npm run build --workspace=packages/core first. That is pre-existing repo behaviour, not something this PR introduced.

Verdict

The routing, request schema, both response shapes and the SSRF protections all behave correctly under a real stack, and the preset change works from wizard to tool invocation. I'd like ① fixed before merge — it is a small change with a large effect on how every MiniMax-side failure reads. ② and ③ are fine as follow-ups.

中文版本

本地真实环境验证

我在本地构建了这个 PR,并让真实 CLI 打到一个伪造的 MiniMax 源站上,以便观察实际发出的请求,而不是只依赖单元测试。结论:主要路径端到端可用,我此前 review 中提出的两个问题在真实环境中复现了。 两者都不阻塞主流程,但问题 ① 会影响用户在每一次 MiniMax 侧失败时看到的内容。

验证方式

PR head a98e3f61fe
Base(与 main 的 merge-base) 1be199ae82
运行环境 隔离的 Docker 容器(Debian 12,Node v22.23.2),宿主机 macOS 15 (arm64)
被测 CLI 由本 PR 源码打包的 dist/cli.jsnpm ci && npm run build && npm run bundle

容器内无外网。在容器里:

  • api.minimax.ioapi.minimaxi.com 通过 /etc/hosts 解析到本地 HTTPS 服务,该服务持有覆盖这两个域名的叶证书,由一次性 CA 签发并通过 NODE_EXTRA_CA_CERTS 信任。因此域名路由、TLS、DNS 都是真实走通的,进程内没有任何打桩。
  • 图片 CDN 域名解析到 93.184.216.34,这是从自定义 Docker 子网分配给容器的公网可路由地址。这一点很关键:downloadPngvalidateResultUrlresolveNetworkTarget(…, 'public') 会拒绝私有地址,若把 CDN 指向回环地址,下载路径会被悄悄跳过而不是真正被测到。
  • 一个脚本化的 OpenAI 兼容端点驱动真实 agent 循环产生真正的 image_gen 工具调用,它完全不接触图像相关代码。
  • 对伪 MiniMax 和 CDN 的每一次请求都会写入 JSONL 账本。账本才是判据 —— 下面的结论说的是 CLI 实际发出了什么,而不是 mock 被要求返回了什么。

模型路由完全走 settings,与用户的真实用法一致:

{
  "imageModel": "image-01",
  "modelProviders": { "openai": [
    { "id": "image-01", "envKey": "MINIMAX_API_KEY",
      "baseUrl": "https://api.minimax.io/v1", "imageOnly": true }
  ]}
}

结果

# 场景 CLI 实际发出的请求 结果
1 baseUrl = …minimax.io/v1size=1024*1024 POST /v1/image_generation + width/height ✅ 保存 PNG,189 561 字节
2 baseUrl = …minimaxi.com/v1/image_generationimage-01-live POST /v1/image_generation(未重复拼接路径) ✅ 保存 PNG
3 base64(data:image/png;base64,…)响应 POST /v1/image_generation ✅ 保存 PNG,完全未访问 CDN
4 baseUrl = …minimax.io(无 /v1 POST /services/aigc/multimodal-generation/generation ⚠️ 问题 ② —— HTTP 404
5 HTTP 200 且 base_resp.status_code = 1008 POST /v1/image_generation ⚠️ 问题 ① —— 提示 "did not contain an image URL"
6 HTTP 429 POST /v1/image_generation ✅ "Image generation rate limit reached."
7 返回 http:// 图片地址 POST /v1/image_generation ⚠️ 报错信息误导(见下)
8 返回 https://127.0.0.1/… POST /v1/image_generation ✅ 正确拒绝为不安全地址

场景 1 的账本原文:

{"host":"api.minimax.io","method":"POST","path":"/v1/image_generation",
 "authorization":"Bearer <redacted>",
 "body":{"model":"image-01","prompt":"A teal product card with a bold border",
         "n":1,"prompt_optimizer":true,"response_format":"url",
         "width":1024,"height":1024}}
{"host":"cdn.minimax-assets.io","method":"GET","path":"/gen/demo.png","accept":"image/png"}

保存下来的产物与源站返回的字节完全一致(两侧 sha256 c1b42cde…),说明下载、校验、写盘链路没有改动图像内容。场景 3 中 base64 分支逐字节还原了 256×256 的样本图,且全过程只有一次 HTTP 请求 —— 下载确实被跳过了。

前后对比

同一 prompt、同一配置,只有 CLI 构建不同。

打了本 PR —— 工具成功并写出产物:

after

main(未打 PR) —— DashScope 的 schema 被发到 …minimax.io/v1/services/aigc/multimodal-generation/generation 并 404:

before

.qwen/generated-images/ 读回的产物本身:

preset 改动的端到端验证

我没有手写配置,而是走了真实的 /auth 向导。image-01image-01-live 出现在推荐模型中:

wizard

完成向导后二者被写入 "imageOnly": true;它们不会出现在普通 /model 选择器中(那里只有对话模型),并且出现在 /model --image 中:

image picker

截图里有个细节值得一提:两个条目在能力列里都标成 text,而且所有推荐模型默认勾选。功能上无害 —— imageOnly 写入正确,对话选择器也会过滤掉它们 —— 但对图像专用模型来说这个标签是错的,建议作为向导侧的后续改进,不一定要在本 PR 处理。

发现

① HTTP 200 下的 base_resp.status_code != 0 被吞掉。 MiniMax 会把限流(1002)、余额不足(1008)、内容审核拒绝、参数错误都以 HTTP 200 + 非零 base_resp.status_code 返回。generateMiniMaxImage 只判断 response.ok,于是这些情况全部退化成:

Image generation failed: Image generation response did not contain an image URL.

真正的原因("insufficient balance")就在响应体里,却完全没有传达给用户。在解析后检查 base_resp.status_code !== 0 并透出 base_resp.status_msg,一次就能覆盖所有这类情况。这一条我希望在合并前处理 —— 它决定了用户是知道"自己欠费了",还是去提一个 bug。补一个 200 + 错误 base_resp 的用例可以锁住这个行为。

② 裸域名 baseUrl 会静默落到 DashScope 分支。 https://api.minimax.io(没有 /v1)无法通过 isMiniMaxImageGenerationBaseUrl(归一化后的路径是 ''),随后把 DashScope 的 schema 发到了 MiniMax 域名上:

{"host":"api.minimax.io","path":"/services/aigc/multimodal-generation/generation",
 "body":{"model":"image-01","input":{"messages":[]},
         "parameters":{"n":1,"prompt_extend":true,"watermark":false}}}

用户只看到一个光秃秃的 HTTP 404,完全无从判断问题出在 baseUrl。对 MiniMax 域名接受空路径(并补上 /v1/image_generation),或者直接给出明确报错并快速失败,都可以。

③ 次要:http:// 图片地址给出的诊断信息不对。 findMiniMaxGeneratedImage 中凡是不以 https:// 开头的都被归类为 base64,因此 http:// 地址会在后面的 PNG 签名校验处报 "did not contain a valid PNG image",而不是 DashScope 路径给出的 "not a safe public HTTPS URL"。没有安全影响 —— 场景 8 证明私网 https:// 地址仍被正确拒绝,URL 分支依旧经过 validateResultUrlresolveNetworkTarget 的域名 pin、10 MB 上限、手动重定向处理和 PNG 签名校验。这纯粹是诊断信息不一致。

针对本 PR 执行的检查

检查项 结果
npm run typecheck --workspace=packages/core
对 4 个改动文件执行 eslint --max-warnings 0
packages/core —— src/servicessrc/providersimage-gen.test.tsconfig.test.ts ✅ 2 645 通过,3 跳过,70 个文件
PR 自带的新增测试 ✅ 32 通过

一个让我误报过一次失败的流程细节:minimax.test.ts 是从包入口 @qwen-code/qwen-code-core 导入 minimaxProvider 的,也就是读 packages/core/dist 而不是源码。dist 陈旧时,即使源码正确,新增的 preset 断言也会失败 —— 需要先执行 npm run build --workspace=packages/core。这是仓库既有行为,不是本 PR 引入的。

结论

在真实环境下,路由、请求 schema、两种响应形态以及 SSRF 防护都表现正确,preset 改动也从向导一路贯通到工具调用。我希望 ① 在合并前修掉 —— 改动很小,但对每一次 MiniMax 侧失败的可读性影响很大。② 和 ③ 可以作为后续处理。

@octo-patch

Copy link
Copy Markdown
Contributor Author

Thanks for the review. I now surface non-zero MiniMax base_resp status codes and messages for HTTP 200 and non-2xx responses, and added a regression test. I ran git diff --check; the targeted Vitest test could not run in this worker because dependencies are not installed.

@octo-patch

Copy link
Copy Markdown
Contributor Author

Thanks for the review. I replaced the MiniMax endpoint magic offset with an explicit image-generation suffix and pushed a new commit. I ran npm ci, the two targeted core test files (25 tests), core typecheck, ESLint on the changed service, and git diff --check.

@qwen-code-ci-bot qwen-code-ci-bot 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.

Reviewed. Suggestions are inline.

Not explored to full depth (tool budget reached): Reverse audit round 2 of the PR 8322 review (MiniMax imag...: none — all checks above completed within budget.; This PR adds MiniMax image generation support to qwen-cod...: none — all checks above completed within budget.; This PR adds MiniMax image generation support to qwen-cod...: none — all checks completed (tests executed, not just read).; This PR adds MiniMax image generation support to qwen-cod...: none — completed all checks I had planned. Actually, one thing I can note: I did not run the test suite. Unnecessary for the performance lens.; This PR adds MiniMax image generation support to qwen-cod...: none — all planned checks completed within budget., and 1 more.

— qwen3.8-max via Qwen Code /review (v0.21.10)

Comment on lines +344 to +345
(parsed.hostname === 'api.minimax.io' ||
parsed.hostname === 'api.minimaxi.com') &&

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.

[Suggestion] This hostname allowlist exact-matches only the two official hosts, while the chat provider (MINIMAX_HOST_SUFFIXES in core/openaiContentGenerator/provider/minimax.ts) deliberately also matches subdomains of minimax.io / minimaxi.com for proxies (with a documented rationale), and the same two hosts are already declared in two more places (MINIMAX_KNOWN_HOSTS in that provider module and telemetry/gen-ai-provider.ts). — Failure scenario: a user configures an imageOnly MiniMax model against a subdomain proxy or corporate gateway (e.g. https://gateway.minimax.io/v1) — a configuration resolveImageGenerationModel supports — chat works via suffix matching, but image generation silently POSTs the DashScope schema to …/services/aigc/multimodal-generation/generation and fails with a 404/schema error that does not mention the real cause (verified with a live probe against this code). When MiniMax ships a new regional host, all three copies must also be updated in tandem or image generation silently breaks.

Suggested change
(parsed.hostname === 'api.minimax.io' ||
parsed.hostname === 'api.minimaxi.com') &&
(parsed.hostname === 'api.minimax.io' ||
parsed.hostname === 'api.minimaxi.com' ||
parsed.hostname.endsWith('.minimax.io') ||
parsed.hostname.endsWith('.minimaxi.com')) &&

Longer-term, hoist one shared known-hosts/suffixes constant, or carry the wire schema from the resolved provider config instead of sniffing hostnames.

— qwen3.8-max via Qwen Code /review (v0.21.10)

prompt_optimizer: true,
response_format: 'url',
};
const dimensions = parseImageSize(request.size);

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.

[Suggestion] The size-absent branch (request body omits width/height) has no test assertion: the schema test pins the body only with size, and 'accepts a full MiniMax image generation endpoint' omits size but asserts only the request URL. — Failure scenario: a mutation that always appends dimensions (e.g. Number(undefined) serialized as null, or a hardcoded default) passes the suite green and sends MiniMax a malformed or wrong-dimension request for users who run image generation without a configured size. Current behavior was probe-confirmed correct (body carries no width/height keys when size is absent) — this only pins it.

In the full-endpoint test, also assert the body:

expect(JSON.parse(String(fetchFn.mock.calls[0]?.[1]?.body))).toEqual({
  model: 'image-01',
  prompt: expect.any(String),
  n: 1,
  prompt_optimizer: true,
  response_format: 'url',
});

— qwen3.8-max via Qwen Code /review (v0.21.10)

Comment on lines +385 to +386
const match = value.match(/^data:image\/png;base64,(.+)$/i);
const base64 = (match?.[1] ?? value).trim();

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.

[Suggestion] The ?? value fallback — raw base64 without the data:image/png;base64, prefix — is untested: the diff's test-efficacy probe (harness validated) deleted it and every affected test stayed green (mutant survived). findMiniMaxGeneratedImage routes any non-https: candidate to this branch, so bare base64 is a live shape. — Failure scenario: if a later change drops or breaks this fallback (e.g. a "simplification" to prefixed data URIs only), bare-base64 responses would throw a TypeError on (match?.[1]).trim() in production while the suite stays green.

Add a case to image-generation-service.test.ts where image_urls contains raw base64 (no prefix) and assert the decoded bytes:

// image_urls: [Buffer.from(PNG_BYTES).toString('base64')]
// expect the decoded result.bytes to equal PNG_BYTES and fetchFn not to be called

— qwen3.8-max via Qwen Code /review (v0.21.10)

Comment on lines +290 to +292
const code =
readStringOrNumber(payload, 'code') ??
readStringOrNumber(baseResponse, 'status_code');

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.

[Suggestion] MiniMax application errors delivered with HTTP 200 — the exact shape this PR adds the statusCode && statusCode !== '0' gate for — can never reach the access-denied or content-moderation branches below, because those match HTTP status (401/403) or regex-test the code string only, while MiniMax codes are numeric base_resp.status_code strings like '1004'. The rate-limit branch already tests `${code} ${message}`, so only the access and moderation branches are asymmetric. — Failure scenario: an invalid/expired MINIMAX_API_KEY returned as { base_resp: { status_code: 1004, status_msg: 'API key not valid' } } yields Image generation failed (1004: API key not valid). instead of the access-denied message with its remediation hint — verified with a live probe against this code, including a flip check; even a status_msg literally containing 'access denied' misses the branch. status_msg still surfaces in the suffix, so this is message quality, not lost information.

Mirror the rate-limit branch and test code+message (and/or map known MiniMax auth/moderation codes):

  if (
    status === 401 ||
    status === 403 ||
    /access|permission/i.test(`${code} ${message}`)
  ) {

Note this alone only flips payloads whose status_msg carries access/permission wording — a complete fix would also map known MiniMax auth codes (e.g. 1004).

— qwen3.8-max via Qwen Code /review (v0.21.10)

@wenshao

wenshao commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator

Local real-stack verification report (maintainer)

Verdict: works as intended. Verified at head 434d911 against merge-base 1be199ae82. The MiniMax routing, request schema, both response shapes (URL / base64), and the base_resp error mapping all behave correctly in a real end-to-end run; the DashScope path is unaffected. A/B against the merge-base bundle confirms this PR is exactly what makes MiniMax image generation work. A few non-blocking observations at the end.

Method

Real stack, no unit-test shortcuts: both head and merge-base were bundled (npm run build && npm run bundle) and run as the real CLI inside a Linux container where api.minimax.io, api.minimaxi.com, and dashscope.aliyuncs.com resolve to a local TLS fake origin (self-signed CA via NODE_EXTRA_CA_CERTS). The fake origin implements the official MiniMax response schema and logs every request to a ledger. A scripted OpenAI-compatible chat model drives the real image_gen tool. The hosts were mapped to a public-looking IP on loopback, so the image-URL download path passes the real resolveNetworkTarget('public') DNS guard rather than bypassing it.

MiniMax models were configured via modelProviders exactly as the preset installs them (baseUrl: https://api.minimax.io/v1, envKey: MINIMAX_API_KEY, imageOnly: true), with imageModel: "image-01".

Results — 19/19 scenario assertions passed

# Scenario Result
H1 URL flow, size 1024*768 (intl host) ✅ POST /v1/image_generation, PNG downloaded from returned URL, saved bytes sha256-identical to the fixture served
H2 base64 in data.image_urls (raw) ✅ decoded + PNG signature validated, no download request made
H3 base64 as data:image/png;base64, URI ✅ decoded correctly
H4 HTTP 200 + base_resp.status_code: 1008 ✅ tool fails with Image generation failed (1008: insufficient balance).no misleading "HTTP 200" in the message (the 2xx suppression in formatImageGenerationError works)
H5 HTTP 429 + base_resp 1002 Image generation rate limit reached (1002: rate limit triggered).
H6 China host api.minimaxi.com/v1, model image-01-live ✅ routed to /v1/image_generation, image saved
H7 DashScope regression (dashscope.aliyuncs.com/api/v1) ✅ still posts the legacy endpoint with the legacy input.messages/parameters schema, image saved
B1 Merge-base bundle, same MiniMax config ✅ (expected failure) posts POST /v1/services/aigc/multimodal-generation/generation on api.minimax.io → 404 → Image generation failed with HTTP 404.

Request captured by the fake origin in H1 — matches the official API reference (endpoint, field names, and types):

POST https://api.minimax.io/v1/image_generation
Authorization: Bearer $MINIMAX_API_KEY
{
  "model": "image-01",
  "prompt": "a lighthouse at dawn",
  "n": 1,
  "prompt_optimizer": true,
  "response_format": "url",
  "width": 1024,
  "height": 768
}

Unit tests: image-generation-service.test.ts + presets/minimax.test.ts25/25 passed on head.

Screenshots

URL flow success in the real TUI (image saved into .qwen/generated-images/…):

URL flow success in TUI

Business error (HTTP 200 + base_resp 1008) surfaced with code and message, no bogus HTTP status:

base_resp business error in TUI

/model --image picker showing the new preset entries with the MiniMax base URL and env key:

image model picker with MiniMax entries

Non-blocking observations

  1. Official base64 responses live in data.image_base64, not data.image_urls. The PR pins response_format: "url", so this never triggers against the real API, and the base64 handling inside image_urls is purely defensive. If base64 mode is ever requested, findMiniMaxGeneratedImage would need to read data.image_base64 (and note the 1 MB API-response cap would likely be exceeded by real base64 payloads).
  2. https://api.minimax.io without /v1 silently falls through to the DashScope-style path (host matches but path is ''), producing a confusing 404. The preset always writes /v1, so this only affects hand-edited configs; widening the matcher or an explicit error would be friendlier.
  3. The official t2i reference lists only image-01 for this endpoint; worth double-checking image-01-live acceptance upstream. If rejected, the server error surfaces cleanly (verified by H4/H5 shape).
  4. prompt_optimizer is hardcoded true while the API default is false — consistent with the DashScope path's prompt_extend: true, so presumably intentional.
  5. Cosmetic, pre-existing: the image-model picker shows Modality: text-only / Context Window: 200,000 tokens defaults for image-only entries (see third screenshot).
中文版本(Chinese version)

本地真实环境验证报告(维护者)

结论:符合预期,可作为合并参考。 验证基于 head 434d911(对照 merge-base 1be199ae82)。MiniMax 路由、请求 schema、URL/base64 两种响应解析、base_resp 错误映射在真实端到端链路中均表现正确;DashScope 路径无回归。与 merge-base bundle 的 A/B 对照证明:正是本 PR 让 MiniMax 图像生成从"打错端点 404"变为可用。文末附几条非阻塞观察项。

方法

真实栈验证,不走单测捷径:head 与 merge-base 分别 npm run build && npm run bundle 后以真实 CLI 运行在 Linux 容器中;容器内 api.minimax.ioapi.minimaxi.comdashscope.aliyuncs.com 解析到本地 TLS 假源站(自签 CA + NODE_EXTRA_CA_CERTS),假源站按 MiniMax 官方响应 schema 应答并把每个请求记入台账;脚本化的 OpenAI 兼容模型驱动真实 image_gen 工具。域名映射到 loopback 上的公网观感 IP,因此图片 URL 下载路径是真实通过 resolveNetworkTarget('public') 的 DNS 守卫,而不是绕过它。

MiniMax 模型按 preset 安装后的形态写入 modelProviders(baseUrl: https://api.minimax.io/v1envKey: MINIMAX_API_KEYimageOnly: true),并设置 imageModel: "image-01"

结果 — 19/19 场景断言全部通过

# 场景 结果
H1 URL 流,size 1024*768(国际站) ✅ POST /v1/image_generation,从返回 URL 下载 PNG,落盘字节与假源站所服务的 fixture sha256 一致
H2 data.image_urls 内裸 base64 ✅ 正确解码并校验 PNG 签名,未发起下载请求
H3 data:image/png;base64, 前缀 ✅ 正确解码
H4 HTTP 200 + base_resp.status_code: 1008 ✅ 报 Image generation failed (1008: insufficient balance).,错误信息不再出现误导性的 "HTTP 200"
H5 HTTP 429 + base_resp 1002 Image generation rate limit reached (1002: rate limit triggered).
H6 国内站 api.minimaxi.com/v1 + image-01-live ✅ 正确路由,图片落盘
H7 DashScope 回归(dashscope.aliyuncs.com/api/v1) ✅ 仍走旧端点旧 schema,图片落盘
B1 merge-base bundle,同样的 MiniMax 配置 ✅(预期失败)向 api.minimax.ioPOST /v1/services/aigc/multimodal-generation/generation → 404

H1 假源站抓到的请求体与官方 API 文档一致(见上方英文版 JSON)。单测:两个相关测试文件在 head 上 25/25 通过

非阻塞观察项

  1. 官方 base64 响应位于 data.image_base64 而非 data.image_urls 本 PR 固定请求 response_format: "url",真实 API 下不会触发;image_urls 里的 base64 处理属于防御性代码。若将来要请求 base64 模式,解析需改读 data.image_base64(且真实 base64 载荷大概率超过 1 MB 的 API 响应上限)。
  2. https://api.minimax.io 不带 /v1 时会静默落入 DashScope 风格路径,产生难懂的 404。preset 总是写 /v1,只影响手改配置;放宽匹配或显式报错会更友好。
  3. 官方 t2i 文档仅列出 image-01,image-01-live 是否被该端点接受建议向上游确认;若被拒,错误也能干净地透出(H4/H5 已验证该形态)。
  4. prompt_optimizer 硬编码为 true(API 默认 false),与 DashScope 路径的 prompt_extend: true 一致,应属有意为之。
  5. 外观问题(先前已存在):图像模型选择器对 image-only 条目显示 Modality: text-only / Context Window: 200,000 tokens 默认值(见第三张截图)。

@wenshao

wenshao commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator

@qwen-code /triage

@wenshao
wenshao enabled auto-merge August 12, 2026 02:48

@qwen-code-ci-bot qwen-code-ci-bot 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.

Reviewed. Suggestions are inline.

Not explored to full depth (tool budget reached): This PR adds MiniMax image generation support: it routes ...: none — all planned checks completed within budget.; This PR adds MiniMax image generation support to qwen-cod...: none — all checks above completed within budget.; This PR adds MiniMax image generation support: it routes ...: none — all planned checks completed within budget..

— qwen3.8-max via Qwen Code /review (v0.21.10)

Comment on lines +208 to +210
if (!image) {
throw new Error('Image generation response did not contain an image URL.');
}

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.

[Suggestion] This if (!image) guard on the MiniMax path has no test coverage: none of the four new MiniMax tests sends a 200 response with missing data, a non-array or empty image_urls, or non-string candidates. A mutation probe confirmed the gap — deleting this guard leaves all 21 tests green, while a payload like {"base_resp":{"status_code":0},"data":{"image_urls":[]}} then crashes with TypeError: Cannot read properties of undefined (reading 'kind') instead of the clean error below. — Failure scenario: a future edit deletes or reorders this guard → a MiniMax 200 response with empty image_urls crashes with an unhandled TypeError instead of throwing 'Image generation response did not contain an image URL.', and no test fails. Suggested fix: add a test asserting rejects.toThrow('Image generation response did not contain an image URL.') for a payload with data: { image_urls: [] } (optionally with a [null, '', 'https://…'] candidate list to pin the skip logic).

— qwen3.8-max via Qwen Code /review (v0.21.10)

@wenshao

wenshao commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator

Re-verified on a real stack at the current head 42c0ea44

Following up on my earlier report at head 434d911: the branch has since been merged with main, so I rebuilt both arms and re-ran the whole verification against the current head.

Verdict: still works as intended — merge-ready. 19/19 end-to-end scenario assertions pass at 42c0ea44 against the new merge-base 5e97fc8f, and the targeted unit tests pass 25/25. Non-blocking observations from the previous round are unchanged and repeated at the end for convenience.

What the merge changed

The merge commit 42c0ea44 brought in 301 commits of main but the four PR files are byte-identical to 434d911 (git diff 434d911 42c0ea44 -- <the 4 files> is empty). None of main's commits touch any file on this code path either:

File on the code path Touched by the merge?
services/image-generation-service.ts no
providers/presets/minimax.ts no
tools/image-gen.ts no
extension/network-policy.ts (resolveNetworkTarget) no
imageModel resolution in config/config.ts no
/model --image dialog no

So this re-run is a regression check on the surrounding tree rather than a review of new code — and the tree is clean.

Method

Unchanged from the previous round, re-run from scratch. Both head and merge-base were built and bundled (npm ci && npm run build && npm run bundle) and executed as the real CLI inside a Linux container where api.minimax.io, api.minimaxi.com, and dashscope.aliyuncs.com resolve to a local TLS fake origin (self-signed CA via NODE_EXTRA_CA_CERTS) that implements the official MiniMax response schema and ledgers every request. A scripted OpenAI-compatible provider drives the real image_gen tool.

The hosts are mapped to a public-looking IP on loopback (11.99.0.7/32 on lo — inside 11/8, which is not in the loopback/RFC1918/TEST-NET/CGNAT block lists), so the image-URL download really passes the resolveNetworkTarget('public') DNS guard instead of bypassing it. Bundle identity was printed inside the container to rule out stale builds:

/repo-head: sha256 b900ed33602e5cd0…  (42c0ea44)
/repo-base: sha256 958b70973ba28080…  (5e97fc8f)

MiniMax models were configured exactly as the preset installs them (baseUrl: https://api.minimax.io/v1, envKey: MINIMAX_API_KEY, imageOnly: true), with imageModel: "image-01".

Results — 19/19 assertions passed

# Scenario Result
H1 URL flow, size 1024*768 (intl host) ✅ POST /v1/image_generation, PNG downloaded from the returned URL, saved bytes sha256-identical to the fixture served (c39093d10fb3bb17…)
H2 base64 in data.image_urls (raw) ✅ decoded + PNG signature validated, no download request made
H3 base64 as data:image/png;base64, URI ✅ decoded correctly
H4 HTTP 200 + base_resp.status_code: 1008 Image generation failed (1008: insufficient balance). — no misleading "HTTP 200"
H5 HTTP 429 + base_resp 1002 Image generation rate limit reached (1002: rate limit triggered).
H6 China host api.minimaxi.com/v1, model image-01-live ✅ routed to /v1/image_generation, image saved
H7 DashScope regression (dashscope.aliyuncs.com/api/v1) ✅ still posts the legacy endpoint with the legacy input.messages/parameters schema, image saved
B1 merge-base bundle, same MiniMax config ✅ (expected failure) posts /v1/services/aigc/multimodal-generation/generation on api.minimax.io → 404 → Image generation failed with HTTP 404.

The A/B is what makes the change load-bearing — same config, only the bundle differs:

head  (42c0ea44):  TOOL_OUTCOME: Generated image saved to …/generated-images/…/….png
base  (5e97fc8f):  TOOL_OUTCOME: Image generation failed with HTTP 404.

Request captured by the fake origin in H1 — endpoint, field names, and types match the official API reference:

POST https://api.minimax.io/v1/image_generation
Authorization: Bearer $MINIMAX_API_KEY
{
  "model": "image-01",
  "prompt": "a lighthouse at dawn",
  "n": 1,
  "prompt_optimizer": true,
  "response_format": "url",
  "width": 1024,
  "height": 768
}

Full fake-origin ledger for the run (note line 12 — the pre-PR bundle hitting the wrong endpoint):

 2 200 minimax-generation   api.minimax.io          POST /v1/image_generation
 3 200 png-download         api.minimax.io          GET  /files/gen-2.png
 4 200 minimax-generation   api.minimax.io          POST /v1/image_generation      (raw base64, no download follows)
 5 200 minimax-generation   api.minimax.io          POST /v1/image_generation      (data-URI base64)
 6 200 minimax-generation   api.minimax.io          POST /v1/image_generation      (base_resp 1008)
 7 429 minimax-generation   api.minimax.io          POST /v1/image_generation      (base_resp 1002)
 8 200 minimax-generation   api.minimaxi.com        POST /v1/image_generation      (image-01-live)
 9 200 png-download         api.minimaxi.com        GET  /files/gen-8.png
10 200 dashscope-generation dashscope.aliyuncs.com  POST /api/v1/services/aigc/multimodal-generation/generation
11 200 png-download         dashscope.aliyuncs.com  GET  /files/gen-10.png
12 404 not-found            api.minimax.io          POST /v1/services/aigc/multimodal-generation/generation   <- base bundle

Unit tests at head: image-generation-service.test.ts (21) + presets/minimax.test.ts (4) → 25/25 passed.

Screenshots (re-captured at 42c0ea44)

URL flow success in the real TUI — image saved under .qwen/generated-images/…:

URL flow success in the TUI at head 42c0ea44

Business error (HTTP 200 + base_resp 1008) surfaced with code and message, no bogus HTTP status:

base_resp business error in the TUI

/model --image picker showing the two new preset entries with the MiniMax base URL and env key:

image model picker with the MiniMax entries

(Glyphs are transliterated to ASCII in these captures — the renderer used for the screenshots hangs on box-drawing characters. The .ans captures are byte-exact.)

Non-blocking observations (unchanged, none block merge)

  1. Official base64 responses live in data.image_base64, not data.image_urls. The PR pins response_format: "url", so this never triggers against the real API, and the base64 handling inside image_urls is purely defensive. If base64 mode is ever requested, findMiniMaxGeneratedImage would need to read data.image_base64 — and note the 1 MB API-response cap would likely be exceeded by real base64 payloads.
  2. https://api.minimax.io without /v1 silently falls through to the DashScope-style path (host matches, path is ''), producing a confusing 404. The preset always writes /v1, so this only affects hand-edited configs; widening the matcher or raising an explicit error would be friendlier.
  3. The official t2i reference lists only image-01 for this endpoint; worth confirming image-01-live is accepted upstream. If it is rejected, the error surfaces cleanly (H4/H5 verify that shape).
  4. prompt_optimizer is hardcoded true while the API default is false — consistent with the DashScope path's prompt_extend: true, so presumably intentional.
  5. Cosmetic, pre-existing: the image-model picker shows Modality: text-only / Context Window: 200,000 tokens defaults for image-only entries (visible in the third screenshot).
中文版本(Chinese version)

在当前 head 42c0ea44 上重新做了真实环境验证

接上一轮在 head 434d911 的报告:该分支之后合并了 main,因此我重建两侧产物,针对当前 head 重跑了全部验证。

结论:依然符合预期,可以合并。42c0ea44(对照新 merge-base 5e97fc8f)上 19/19 条端到端场景断言全部通过,相关单测 25/25 通过。上一轮的非阻塞观察项没有变化,文末重列一遍备查。

这次 merge 改变了什么

merge commit 42c0ea44 带入了 301 个 main 的提交,但 PR 的四个文件与 434d911 逐字节一致(git diff 434d911 42c0ea44 -- <四个文件> 为空)。并且 main 的这些提交没有触碰该代码路径上的任何文件:

代码路径上的文件 本次 merge 是否改动
services/image-generation-service.ts
providers/presets/minimax.ts
tools/image-gen.ts
extension/network-policy.ts(resolveNetworkTarget)
config/config.ts 中的 imageModel 解析
/model --image 选择器

所以这一轮是对周边代码树的回归检查,而不是对新代码的评审 —— 检查结果是干净的。

方法

与上一轮相同,完整重跑。head 与 merge-base 分别 npm ci && npm run build && npm run bundle,以真实 CLI 运行在 Linux 容器中;容器内 api.minimax.ioapi.minimaxi.comdashscope.aliyuncs.com 解析到本地 TLS 假源站(自签 CA + NODE_EXTRA_CA_CERTS),假源站按 MiniMax 官方响应 schema 应答并把每个请求记入台账;脚本化的 OpenAI 兼容模型驱动真实 image_gen 工具。

这些域名被映射到 loopback 上的公网观感 IP(11.99.0.7/32,属于 11/8,不在 loopback/RFC1918/TEST-NET/CGNAT 的封锁表里),因此图片 URL 下载是真实通过 resolveNetworkTarget('public') 的 DNS 守卫,而不是绕过它。容器内打印了产物指纹,排除用到旧构建的可能:

/repo-head: sha256 b900ed33602e5cd0…  (42c0ea44)
/repo-base: sha256 958b70973ba28080…  (5e97fc8f)

MiniMax 模型按 preset 安装后的形态配置(baseUrl: https://api.minimax.io/v1envKey: MINIMAX_API_KEYimageOnly: true),并设置 imageModel: "image-01"

结果 — 19/19 断言通过

# 场景 结果
H1 URL 流,size 1024*768(国际站) ✅ POST /v1/image_generation,从返回 URL 下载 PNG,落盘字节与假源站所服务的 fixture sha256 一致(c39093d10fb3bb17…)
H2 data.image_urls 内裸 base64 ✅ 正确解码并校验 PNG 签名,发起下载请求
H3 data:image/png;base64, 前缀 ✅ 正确解码
H4 HTTP 200 + base_resp.status_code: 1008 ✅ 报 Image generation failed (1008: insufficient balance).,不再出现误导性的 "HTTP 200"
H5 HTTP 429 + base_resp 1002 Image generation rate limit reached (1002: rate limit triggered).
H6 国内站 api.minimaxi.com/v1 + image-01-live ✅ 正确路由到 /v1/image_generation,图片落盘
H7 DashScope 回归(dashscope.aliyuncs.com/api/v1) ✅ 仍走旧端点与旧 input.messages/parameters schema,图片落盘
B1 merge-base 产物,同样的 MiniMax 配置 ✅(预期失败)向 api.minimax.io/v1/services/aigc/multimodal-generation/generation → 404 → Image generation failed with HTTP 404.

A/B 对照说明这个改动是真正起作用的 —— 配置完全相同,只换产物:

head  (42c0ea44):  TOOL_OUTCOME: Generated image saved to …/generated-images/…/….png
base  (5e97fc8f):  TOOL_OUTCOME: Image generation failed with HTTP 404.

H1 被假源站抓到的请求(端点、字段名与类型均与官方 API 文档一致)以及完整台账见上方英文版;台账第 12 行就是 merge-base 产物打错端点的证据。单测在 head 上 25/25 通过

截图(在 42c0ea44 重新采集)

三张图分别是:真实 TUI 中 URL 流成功并落盘、HTTP 200 + base_resp 1008 业务错误按 code + message 透出、/model --image 选择器中新增的两个 MiniMax 条目(含 base URL 与环境变量名)。图片见上方英文版。

(截图中的字形已转写为 ASCII —— 用于渲染截图的工具在遇到制表符号时会挂死;.ans 原始抓取是逐字节准确的。)

非阻塞观察项(与上一轮相同,均不阻塞合并)

  1. 官方 base64 响应位于 data.image_base64 而非 data.image_urls 本 PR 固定请求 response_format: "url",真实 API 下不会触发,image_urls 里的 base64 处理属于防御性代码。若将来要用 base64 模式,findMiniMaxGeneratedImage 需改读 data.image_base64,且真实 base64 载荷大概率会超过 1 MB 的响应上限。
  2. https://api.minimax.io 不带 /v1 时会静默落入 DashScope 风格路径(host 匹配但 path 为空),产生难懂的 404。preset 总是写 /v1,只影响手改配置;放宽匹配或显式报错会更友好。
  3. 官方 t2i 文档仅列出 image-01,建议向上游确认 image-01-live 是否被该端点接受;若被拒,错误也能干净地透出(H4/H5 已验证该形态)。
  4. prompt_optimizer 硬编码为 true(API 默认为 false),与 DashScope 路径的 prompt_extend: true 一致,应属有意为之。
  5. 外观问题(先前已存在):图像模型选择器对 image-only 条目显示 Modality: text-only / Context Window: 200,000 tokens 默认值(见第三张截图)。

@wenshao

wenshao commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator

@qwen-code /triage

@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator

Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run

Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check.

Scripted assertions: 505 passed · 0 failed · 505 total

中文 — 判定:✅ 通过 · 可合入(agent 判定)

沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查

脚本断言:505 通过 · 0 失败 · 505 总计

Verification report

PR 8322 Deep Verification (follow-up round) — Support MiniMax image generation schema

Verdict: merge-ready — 505/505 scripted assertions passed, 0 failed (breakdown in Methodology).
Verified head: 42c0ea44046cdac011d20ffda485cc133ffa38a2 (git rev-parse HEAD^2, unchanged from the previous round) against the new base tip 05079297d26c9c42013c3699743350d1d272fac2 (HEAD^1; main advanced from 5e97fc8 — the metadata snapshot's baseRefOid is stale on exactly this point). Merge ref 9f8eaeed is conflict-free and differs from the new base by exactly this PR's 4 files.

中文 — 判定:✅ 通过 · 可合入(agent 判定)

这是跟进轮:PR head 与上一轮完全相同(42c0ea44),base 从 5e97fc8 前进到 05079297(main 合入了 #8732 等 131 个文件的变更)。已验证 base 前进未触碰图像生成闭包中的任何文件(131 个文件中 0 个匹配 image/models/providers/network-policy/fetch/vision),且合并引用与新 base 的差异恰好只有本 PR 的 4 个文件。按跟进轮规则,所有上一轮测量均在新 head 上重新执行而非沿用旧报告。

  • 结论: merge-ready。505/505 脚本化断言通过,0 失败。
  • A/B 结论: 中心主张重新证明为 load-bearing:head 侧 36/36 通过;base 侧 26/26 按预测失败/持平(9 个 MiniMax 单元格 broken→fixed 翻转,5 个 parity 单元格两臂逐字节一致,含真实 TLS 与 SSRF 拦截)。见下表与 01-ab-head-arm.png / 02-ab-base-arm.png
  • 上一轮 findings 状态: 全部 stands(head 未变,无新提交;均为覆盖缺口与 nit,非阻塞)。详见状态表。
  • 未覆盖范围: 逐 commit 归因(depth-2 浅克隆,3 个实质 commit 仍不可达);MiniMax 真实生产端点联调(无凭据);downloadPng 内部机制(PR 未改动);仓库级全量测试/lint。

Previous-finding status (follow-up round)

The previous round verified the same head OID (42c0ea44…) and found no blockers. Head is byte-identical and there are no new commits, so nothing could have been fixed; every carried measurement was nonetheless re-run at the new head (base rebuilt from the new base tip 05079297…, full A/B, mutation matrix, probes, and gates re-executed — not diffed against the old report). The base advance (5e97fc805079297, 131 files, incl. #8732 "Goal v3 in ACP sessions") touched zero files in the image-generation closure (git diff --name-only HEAD^2..HEAD filtered for image/models/providers/network-policy/fetch/vision → empty), so no finding could have moved through the base either. There were no declined or deferred rows last round.

# Previous finding Severity Status at new head
1 Four mutation survivors — base64 PNG-signature guard, parseImageSize safe-integer guard, hostname allowlist, /v1 path restriction — classified as coverage gaps, not dead code Minor Stands. Mutation matrix re-run: the same 4 mutants survive (suite green), the 3 killed mutants (routing branch, base_resp check, 2xx status-text suppression) still die with exactly the predicted tests red, unmutated control green (03-mutation-matrix.png). The recommended pinning fixture for the base64 gap was re-verified: green against the intact guard, red with the intended assertion (promise resolved "{ …(2) }" instead of rejecting) when the guard is disabled. Completeness reporting, not a merge condition.
2 Misleading error for non-HTTPS / non-PNG URL candidates (http://… and data:image/jpeg;base64,… classified as base64 → "did not contain a valid PNG image") Nit Stands. Re-measured on the new head dist (probe-siblings.mjs S1/S2): identical misleading messages; the safe part also holds — exactly 1 wire request in both cases, the candidate URL is never fetched.
3 size: '0x0' passes service-level parsing and is forwarded as width: 0, height: 0 Nit Stands. Re-measured (harness cell M13, head arm); the shipped tool layer still blocks it (image-gen.ts validates 512×512–2048×2048 total pixels before calling the service), so only standalone service callers are exposed. Note, not a defect.

Central claim and A/B proof (re-run at the new head)

Central claim: base URLs on api.minimax.io / api.minimaxi.com (path /v1 or /v1/image_generation) are routed away from the generic dashscope schema to the MiniMax schema — POST <base>/image_generation with body {model, prompt, n: 1, prompt_optimizer: true, response_format: 'url', width?, height?} — and responses are parsed from data.image_urls[] in both URL and base64 forms.
Secondary claim 1: base_resp.status_code ≠ 0 application errors delivered with HTTP 200 are surfaced (base swallowed them).
Secondary claim 2: image-01 / image-01-live preset entries are image-only models wired through the pre-existing imageOnly machinery — re-verified not a dead switch at the new base: read sites present at provider-config.ts:125, modelRegistry.ts:239/299/347, modelsConfig.ts:510/1041, content-generator-config.ts:59, config.ts:4102/4181/4438/6695, vision-bridge-service.ts:70.

The harness (harness.mjs) drives the compiled dist of each tree through the unit's documented fetchFn seam (recording fake peer: exact URL/method/headers/body per request) plus real-TLS loopback cells on the default global fetch (self-signed cert via NODE_EXTRA_CA_CERTS). Expectations are per-arm: a base cell passes when it breaks exactly as predicted. Witnesses: 01-ab-head-arm.png, 02-ab-base-arm.png. Raw logs: logs/harness-{head,base}.txt; JSON: logs/results-{head,base}.json.

# Cell (observable oracle) HEAD BASE
M0 loaded dist has/lacks MiniMax markers ✅ (0 markers)
M1 minimax.io/v1 + 1024*1024 + base64 reply → wire: POST …/v1/image_generation, exact MiniMax body, Bearer header; bytes + requestId ✅ 1 request, exact body ❌→predicted: dashscope endpoint + body, reply unparseable
M2 minimaxi.com/v1/image_generation full endpoint, no size → used as-is, body omits width/height ❌→predicted: dashscope path appended to MiniMax endpoint
M3 base_resp {status_code:1008} over HTTP 200 Image generation failed (1008: insufficient balance). ❌→predicted: swallowed → did not contain an image URL
M4 HTTP 429, empty body, neutral host (parity) ✅ identical rate-limit msg ✅ identical
M5 HTTP 500 + base_resp detail … with HTTP 500 (1023: server busy). ❌→predicted: detail dropped (… with HTTP 500.)
M6 HTTP 200, image_urls: [] (parity) ✅ identical error ✅ identical
M7 base64 that is not a PNG … did not contain a valid PNG image. ❌→predicted: base64 never seen
M8 URL-form reply → download hop (real resolveNetworkTarget DNS pinning ran on example.com) ✅ GET exactly image_urls[0], Accept: image/png, no Authorization leaked to CDN ❌→predicted: 1 call, parse failure, no download
M9 trailing-slash baseUrl …/v1/// ✅ normalized → single clean endpoint ❌→predicted: dashscope path
M10 non-MiniMax host keeps generic dashscope schema + download (A/A parity, wire+result snapshot compared byte-for-byte) ✅ identical ✅ identical
M11a real TLS loopback, default fetch, 429 ✅ identical msg; server saw dashscope wire ✅ identical
M11b real TLS, reply URL → private IP refused before download ✅ SSRF block holds, no extra traffic ✅ identical
M12 1024x768 parsed; top-level request_id precedence over base_resp ❌→predicted: raw size in dashscope parameters
M13 size boundaries: 0x0 forwarded; astral digits dropped (not coerced) ❌→predicted: raw sizes in parameters

Score: head 36/36; base 26/26 — 9 cells flip broken→fixed, 5 parity cells + the marker control identical on both arms. The parity cells are the control: the flip is attributable to this PR alone, and the generic dashscope path is behaviorally byte-identical on both arms (wire-level, including over real TLS). Sibling probe S3 additionally re-confirmed on both arms that a non-MiniMax host with a /v1 path (proxy.example.com/v1) stays generic (allowlist parity preserved).

Corrections

One correction to the previous verification round's own methodology note (not to any claim about the PR): the prior round recorded a base-side tsc error in the unrelated shellExecutionService.ts (@lydell/node-pty exports/typing) as an unexplained environmental anomaly that head's rebuild did not share. This round diagnosed it: packages/core/tsconfig.json carries a relative paths mapping — "@lydell/node-pty": ["../../node_modules/@lydell/node-pty/node-pty.d.ts"] — and ../../node_modules resolves to the worktree root, which a bare base worktree lacks (head's check: tsc -p packages/core/tsconfig.json --noEmit → exit 0). With the root node_modules linked into the base tree, the base build is fully clean (exit 0). The anomaly was an artifact of the control's worktree layout, not of the code on either side; the previous round's behavioral verdict is unaffected.

Findings

None blocking, none new. The three carried findings all stand unchanged at the new head (status table above); they are re-measured, not assumed:

  1. (Minor — coverage gaps, not merge conditions) Four mutation survivors. Reproduce: node tmp/pr8322-verify-20260812-175543/mutation-runner.mjs --repo <repo> — applies each single-hunk mutation to packages/core/src/services/image-generation-service.ts, runs the two affected suites, restores via git checkout (restore-clean asserted after every mutant; tree verified empty afterwards). Killed set: routing branch (4 MiniMax tests red, attribution exact), base_resp check (1 red), 2xx status-text suppression (1 red) — these also serve as the run's positive control. Survivors: base64 PNG-signature guard, parseImageSize safe-integer guard, hostname allowlist (widening to any host stays green because the generic fixtures use /api/v1), /v1 path restriction. The pinning fixture for the base64 gap was re-proven (packages/core/src/services/image-gen-base64-guard-fixture.test.ts, scratch, deleted after the round): green intact, red when the guard is disabled — the fix should ship with that fixture.
  2. (Nit) Misleading error for unsupported URL candidates. Reproduce: node tmp/pr8322-verify-20260812-175543/probe-siblings.mjs --tree <repo> --arm head. http:// and data:image/jpeg;base64,… candidates are classified as base64 and report "did not contain a valid PNG image"; the URL is never fetched (1 wire request). An explicit unsupported-scheme/encoding branch would make triage cheaper; left to the author.
  3. (Nit) size: '0x0' forwarded as width: 0, height: 0 at the service level (cell M13); the tool layer blocks it upstream. Note, not a defect.

Not covered

  • Per-commit attribution: re-checked at this checkout — git rev-list HEAD^1..HEAD^2 returns 1 OID (the shallow-boundary artifact), and the three substantive commits (a98e3f61, 959c8e6f, 434d911e) all fail git cat-file -t; only the head merge 42c0ea44 is reachable. Matches the metadata's 4-commit list in count but not in reachable objects. Verified the aggregate HEAD^1..HEAD diff; the schema / error-surfacing / magic-offset split still could not be exercised individually.
  • Live MiniMax production endpoint — no credentials in this sandbox; wire evidence comes from the recording peer and real-TLS loopback. The request schema is verified against the fields the PR's own tests declare, not against MiniMax's live API.
  • downloadPng internals (redirect chains, byte limits): pre-existing, untouched by this PR; exercised only transitively (M8/M10/M11b ran the real network policy and download path).
  • Real-TLS execution of the MiniMax hostnames themselves: mapping api.minimax.io → loopback requires /etc/hosts write access this container's user does not have; MiniMax-hostname cells ran over the fetchFn seam, the generic path ran over real TLS as the transport control (M11a/b).
  • Repo-wide gates: targeted gates only (21 files / 411 tests, all green; head tsc --noEmit for packages/core clean; base rebuild clean). PR's own CI covers the rest. The base advance (feat(cli): adopt Goal v3 in ACP sessions #8732 et al.) is CLI/web-shell/webui/Goal work with zero overlap with this PR's closure, so no trial-merge re-run beyond the already-clean merge ref was warranted.
  • The PR body's check list (npm ci / core build / the two test files) was reproduced: builds OK, the two named test files green (25/25 inside the gate and every mutation control run).

Methodology

One container (node v22.23.2, bookworm), working tree at merge commit 9f8eaeed (PR head 42c0ea44 merged into new base 05079297). Base control: git worktree add tmp/base-tree HEAD^1, nested packages/core/node_modules hardlinked from the root install (same lockfile on both sides — the PR touches no package.json/lockfile), root node_modules symlinked to repair the tsconfig paths mapping, then scripts/build_package.js → exit 0. Realpath checks: the nested undici resolves inside the base tree (readlink -ftmp/base-tree/packages/core/node_modules/undici); root-level deps are shared and identical, and the harness imports each tree's dist by absolute path with the M0 marker control proving the base dist contains zero MiniMax markers. Harnesses drove the compiled dist directly (harness.mjs, 15 cells × 2 arms; probe-siblings.mjs, 3 probes × 2 arms); mutations were single-hunk source edits run through vitest and restored (mutation-runner.mjs, restore-clean asserted per mutant, final git status --porcelain empty). Targeted gate: npx vitest run src/services/image-generation-service.test.ts src/tools/image-gen.test.ts src/providers src/models → 21 files, 411/411. Evidence images: evidence/01..03 via scripts/verify-capture.mjs; raw logs and per-cell JSON in logs/. Assertion accounting (all scripted, all executed): harness head 36 + harness base 26 + sibling probes 12 + mutation-matrix run checks 8 + mutation restore-clean checks 7 + pinning-fixture checks 2 + extended gate 411 + head typecheck 1 + base rebuild 1 + commit-reachability comparison 1 = 505 pass, 0 fail. Expected base-arm breaks are encoded as predictions in the harness, so fail counts only unexpected outcomes — there were none.

Evidence images

01-ab-head-arm

02-ab-base-arm

03-mutation-matrix

Harness scripts and raw logs are in the workflow run artifacts (7-day retention).

Qwen Code · sandboxed verification

@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator

Triage re-ran again on Aug 12 at the same head 42c0ea44 — no new commits, PR body unchanged — so the result is unchanged: it stops at the Stage 1a gate. The PR body still does not follow the pull request template — all required sections (What this PR does, Why it's needed, Reviewer Test Plan with How to verify / Evidence / Tested on, Risk & Scope, Linked Issues, and the Chinese <details> translation) are missing; the body is still the free-form Reason / Changes / Checks layout.

The request-changes review from Aug 1 still gates the PR on this, so no duplicate review was submitted — and per the gate rules this run stops here: no code review pass, no approval.

What changed since the last run — only CI, which has now fully settled on this head: Test (ubuntu-latest, Node 22.x) ✅ success (it was still in flight last time), Desktop Shell (ubuntu-22.04) and Desktop Shell (windows-2022) ✅, web-shell E2E Smoke (ubuntu-latest, Node 22.x) ✅, precheck-pr / precheck ✅; the macOS/Windows test lanes and the integration-test lane are skipped, and there are no red checks. Together with the sandboxed verification (484/484 assertions) and the maintainer's real-stack re-verification at this exact head ("still works as intended — merge-ready"), nothing on the code side is blocking.

The split remains: the gate is about the PR description, not the code — and only the author can clear it (or a maintainer can edit the body on the author's behalf).

@octo-patch same one thing left from you: rewrite the PR body following the template. Your existing verification story (unit tests + the maintainer's real-stack runs) slots straight into the Reviewer Test Plan section. After that, trigger @qwen-code /triage again and the run will continue with code review + CI evidence.

Maintainer note: if you'd rather waive the template gate given the approvals and verification already standing, dismissing the bot's stale request-changes review unblocks the PR; otherwise the body fix above is the unblock path.

中文说明

Triage 于 8 月 12 日再次在同一个 head 42c0ea44 上重新运行 —— 没有新提交,PR 正文也未改动 —— 结果不变:仍停在 Stage 1a 门禁。PR 正文仍未遵循 PR 模板 —— 所有必填小节(What this PR doesWhy it's neededReviewer Test Plan(How to verify / Evidence / Tested on)、Risk & ScopeLinked Issues,以及中文 <details> 翻译)均缺失;正文仍是自由格式的 Reason / Changes / Checks 布局。

8 月 1 日的 request-changes 评审 至今仍因此卡住该 PR,所以没有重复提交评审 —— 按照门禁规则,本次运行到此为止:不做代码审查,也不做批准。

上次运行以来唯一的变化是 CI —— 现已在该 head 上全部落定:Test (ubuntu-latest, Node 22.x) ✅ success(上次运行时仍在进行中)、Desktop Shell (ubuntu-22.04)Desktop Shell (windows-2022) ✅、web-shell E2E Smoke (ubuntu-latest, Node 22.x) ✅、precheck-pr / precheck ✅;macOS/Windows 测试通道与集成测试通道为 skipped,无失败检查。结合沙箱验证(484/484 断言)和维护者在该 head 上的真实栈复验("still works as intended — merge-ready"),代码侧没有任何阻塞项。

结论依然不变:门禁针对的是 PR 描述而非代码 —— 且只有作者能解除(维护者也可以代作者修改正文)。

@octo-patch 仍然只差一件事:按模板重写 PR 正文。你已有的验证材料(单测 + 维护者的真实栈验证)可以直接填入 Reviewer Test Plan 小节。完成后再触发 @qwen-code /triage,流程将继续进行代码审查与 CI 证据核验。

维护者注意:如果考虑到已有的批准与验证、希望豁免模板门禁,dismiss 机器人的过期 request-changes 评审即可解除阻塞;否则请按上述作者操作处理。

Qwen Code · qwen3.8-max

Reviewed at 42c0ea44046cdac011d20ffda485cc133ffa38a2 · re-run with @qwen-code /triage

@wenshao

wenshao commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator

@qwen-code /triage

@octo-patch

Copy link
Copy Markdown
Contributor Author

Thanks for the review. I rewrote the PR body to follow the pull request template: What this PR does, Why it's needed, Reviewer Test Plan (How to verify / Evidence / Tested on), Risk & Scope, Linked Issues, and the Chinese translation. This update changes the description only — no code, test or commit changes, so CI on the current head is unaffected.

@qwen-code /triage

@wenshao

wenshao commented Aug 24, 2026

Copy link
Copy Markdown
Collaborator

@qwen-code /triage

@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 24, 2026

Copy link
Copy Markdown
Collaborator

Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run

Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check.

Scripted assertions: 634 passed · 0 failed · 634 total

Flakiness gate: ✅ 2 changed test file(s) x 5 identical rounds, no divergence

中文 — 判定:✅ 通过 · 可合入(agent 判定)

沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查

脚本断言:634 通过 · 0 失败 · 634 总计

抖动门:✅ 2 changed test file(s) x 5 identical rounds, no divergence

Verification report

PR 8322 Deep Verification (follow-up round 2) — Support MiniMax image generation schema

Verdict: merge-ready — 634/634 scripted assertions passed, 0 failed (breakdown in Methodology).
Verified head: 42c0ea44046cdac011d20ffda485cc133ffa38a2 (git rev-parse HEAD^2 — identical to the head verified in both previous rounds) against the new base tip 7385b278b2017a0b6bfeff59380d23b57848fd4a (HEAD^1; main advanced again from 05079297 — the metadata snapshot's baseRefOid 5e97fc8 is two advances stale). Merge ref 33242f4e checked out clean and differs from the new base by exactly this PR's 4 files; all four base-side pre-image blobs are byte-identical to the PR diff's index OIDs, so the same diff applied onto the new base.

中文 — 判定:✅ 通过 · 可合入(agent 判定)

这是第二次跟进轮:PR head 与前两轮完全相同(42c0ea44,blob 级一致),base 从 05079297 前进到 7385b27(新 base tip 为 web-shell 修复 #9770)。四个被改文件在新 base 上的 blob 与本 PR diff 的 pre-image OID 逐一相同,合并引用与新 base 的差异恰好只有本 PR 的 4 个文件。按跟进轮规则,所有测量均在新 base 上重新执行而非沿用旧报告。

  • 结论: merge-ready。634/634 脚本化断言通过,0 失败(明细见 Methodology)。
  • A/B 结论: 中心主张再次证明为 load-bearing:head 侧 83/83 通过;base 侧 71/71 按预测失败/持平。18 个单元格中 12 个 broken→fixed 翻转,6 个 parity 单元格(含真实 TLS 与 SSRF 拦截)两臂逐字节一致。见下表与 01-ab-head-arm.png / 02-ab-base-arm.png / 03-parity-compare.png
  • 上一轮 findings 状态: 3 项全部 stands(head 未变;均已重新测量)。本轮新增 mutation 组合行(MUTE-8)将其中 allowlist 两项幸存者的分类细化为"冗余防御"(见状态表与 Corrections)。
  • 未覆盖范围: 逐 commit 归因(depth-2 浅克隆,3 个实质 commit 仍不可达);MiniMax 真实生产端点联调(无凭据);downloadPng 内部机制(PR 未改动);真实 MiniMax 主机名的 TLS 执行(容器无 /etc/hosts 写权限);仓库级全量测试/lint。

Previous-finding status (follow-up round 2)

Head is byte-identical to the rounds that verified 42c0ea44… before (same OID, same blob OIDs in the diff's index lines), so nothing could have been fixed; every carried measurement was nonetheless re-run at the new head and new base — base rebuilt from 7385b27 (scripts/build_package.js exit 0), full A/B, TLS cells, mutation matrix (now with a combined row), sibling probes, pinning fixture, and gates. The base advance (050792977385b27) could not be enumerated locally (depth-2 checkout), but the new base-tip commit is the web-shell fix #9770, and the four files of this PR's closure are byte-identical on the new base to the diff's pre-images, so the re-runs are the operative evidence. There were no declined or deferred rows in previous rounds.

# Previous finding Severity Status at new head
1 Four mutation survivors — base64 PNG-signature guard, parseImageSize safe-integer guard, hostname allowlist, /v1 path restriction — classified as coverage gaps, not dead code Minor Stands, refined. Mutation matrix re-run with exact counts (04-mutation-matrix.png): control green (25/25); the same 3 mutants die with exactly the predicted tests red — routing branch (4 red, attribution exact: the PR's four MiniMax tests), base_resp check (1 red), 2xx status-text suppression (1 red) — and they double as the run's positive control, landing in the same file as the mutant. The 4 survivors again survive. New this round: a combined row reverting hostname + path restrictions together (MUTE-8) kills 11 tests, so those two survivors are redundant defence (defence in depth — either restriction alone still protects every tested input), while the base64 and safe-integer guards remain genuine coverage gaps — each proven to decide real behavior by scratch-dist mutations (P4: non-PNG base64 resolves when the guard is gone; P5: astral width 1e20 is forwarded; logs/survivor-proofs.txt, 12/12). The pinning fixture for the base64 gap was re-proven: green intact, red with the intended assertion (promise resolved "{ …(2) }" instead of rejecting) when disabled. Completeness reporting, not a merge condition.
2 Misleading error for non-HTTPS / non-PNG URL candidates (http://… and data:image/jpeg;base64,… classified as base64 → "did not contain a valid PNG image") Nit Stands. Re-measured at the new head (cells S1/S2, logs/harness-head.txt): identical misleading messages; the safe part also holds — exactly 1 wire request in both cases, the candidate URL is never fetched.
3 size: '0x0' passes service-level parsing and is forwarded as width: 0, height: 0 Nit Stands. Re-measured (cell M13a, head arm). The shipped tool layer still blocks it at the new head: image-gen.ts:24-25,219-231 rejects sizes outside 512×512–2048×2048 total pixels before calling the service, so only standalone service callers are exposed. Note, not a defect.

Central claim and A/B proof (re-run at the new head/base)

Central claim: base URLs on api.minimax.io / api.minimaxi.com (path /v1 or /v1/image_generation, trailing slashes normalized first) are routed away from the generic dashscope schema to the MiniMax schema — POST <base>/image_generation with body {model, prompt, n: 1, prompt_optimizer: true, response_format: 'url', width?, height?} — and responses are parsed from data.image_urls[] in both URL and base64 forms.
Secondary claim 1: base_resp.status_code ≠ 0 application errors delivered with HTTP 200 are surfaced with the provider's own code/message (base swallowed them).
Secondary claim 2: image-01 / image-01-live preset entries are image-only models wired through the pre-existing imageOnly machinery — re-verified not a dead switch at the new head: read sites present at provider-config.ts:126, modelRegistry.ts:240/300/348, content-generator-config.ts:59, modelsConfig.ts:513/1044, config.ts:4426/4505/4762, vision-bridge-service.ts:70, image-generation-capability.ts:11.

The harness (harness.mjs, 16 cells) drives the compiled dist of each tree through the unit's documented fetchFn seam (recording fake peer: exact URL/method/headers/body per request); tls-harness.mjs adds 2 cells on the default global fetch against a real-TLS loopback server (self-signed cert via NODE_EXTRA_CA_CERTS). Expectations are per-arm: a base cell passes when it breaks exactly as predicted. Controls: M0 asserts the head dist carries the MiniMax markers and the base dist carries zero, and that the 554-file dist import closure of the service imports no @qwen-code/* package (so the head tree's workspace symlinks are unreachable from this code path — the internal-link trap is closed). Witnesses: 01-ab-head-arm.png, 02-ab-base-arm.png, 03-parity-compare.png. Raw logs: logs/harness-{head,base}.txt, logs/tls-{head,base}.txt, per-cell JSON in logs/results-*.json.

# Cell (observable oracle) HEAD BASE
M0 loaded dist has/lacks MiniMax markers; closure imports no workspace package ✅ (0 markers)
M1 minimax.io/v1 + 1024*1024 + base64 reply → wire: POST …/v1/image_generation, exact MiniMax body, Bearer header; bytes + requestId from base_resp ✅ 1 request, exact body ❌→predicted: dashscope endpoint + dashscope body, reply unparseable
M2 minimaxi.com/v1/image_generation full endpoint, no size → used as-is, body omits width/height ❌→predicted: dashscope path appended to MiniMax endpoint
M3 base_resp {status_code:1008} over HTTP 200 Image generation failed (1008: insufficient balance). ❌→predicted: swallowed → did not contain an image URL.
M4 HTTP 429, empty body, neutral host (parity) ✅ identical rate-limit msg ✅ identical
M5 HTTP 500 + base_resp detail … failed with HTTP 500 (1023: server busy). ❌→predicted: detail dropped (… failed with HTTP 500.)
M6 HTTP 200, image_urls: [] on a MiniMax host (message parity; wire legitimately differs) ✅ identical error ✅ identical error
M7 base64 that is not a PNG … did not contain a valid PNG image. ❌→predicted: base64 never seen
M8 URL-form reply → download hop (real resolveNetworkTarget DNS pinning ran on example.com) ✅ GET exactly image_urls[0], Accept: image/png, no Authorization leaked to CDN ❌→predicted: 1 call, parse failure, no download
M9 trailing-slash baseUrl …/v1/// ✅ normalized pre-routing → single clean endpoint …/v1/image_generation ❌→predicted: dashscope path
M10 non-MiniMax host full happy path incl. download (A/A parity, wire+result compared byte-for-byte) ✅ identical ✅ identical
M11a real TLS loopback, default fetch, 429 ✅ identical msg; server saw dashscope wire ✅ identical
M11b real TLS, reply URL → private IP refused before download ✅ SSRF block holds; decoy listener saw 0 connections ✅ identical
M12 1024x768 parsed; top-level request_id precedence over base_resp.request_id width:1024,height:768, requestId=top-1 ❌→predicted: raw size in dashscope parameters
M13a/b size boundaries: 0x0 forwarded as 0/0; astral 20-digit width dropped, not coerced ❌→predicted: raw sizes in parameters
S1/S2 siblings: http:// and data:image/jpeg;base64, candidates → misleading PNG error, never fetched (1 wire request) ✅ measured ✅ measured (different error, 1 request)
S3 non-MiniMax host with /v1 path (proxy.example.com/v1) stays generic (allowlist parity) ✅ identical dashscope wire ✅ identical

Score: head 83/83 + 7/7 TLS; base 71/71 + 7/7 TLS — 12 cells differ broken→fixed (or message-shaped), 6 parity cells identical on both arms, plus 5/5 cross-arm parity comparisons (compare-parity.mjs, ephemeral loopback ports normalized). The parity cells are the control: the flip is attributable to this PR alone, and the generic dashscope path is behaviorally byte-identical on both arms — wire-level, including over real TLS.

Corrections

One refinement to the previous verification round's analysis (not to any claim about the PR's code): the prior rounds classified all four mutation survivors as "coverage gaps, not dead code". This round's added combined row (MUTE-8, hostname + path restrictions reverted together → 11 tests red, including the entire redirect/SSRF block of the generic suite) shows the two allowlist restrictions are redundant defence of each other within the suite — reverting either alone is unobservable because the sibling still protects every tested input — while the base64-signature and safe-integer guards remain genuine coverage gaps (no sibling hunk closes those hazards, and the scratch-dist proofs P4/P5 show each decides real behavior). The prior classification was not wrong ("not dead code" holds for all four) but under-specified; the author should read the allowlist pair as defence in depth and the other two as tests to write.

Findings

None blocking, none new. The three carried findings all stand unchanged at the new head (status table above); they were re-measured, not assumed:

  1. (Minor — completeness reporting, not merge conditions) Four unpinned guards. Reproduce: node tmp/pr8322-verify-20260824-014212/mutation-runner.mjs /__w/qwen-code/qwen-code (single-hunk source mutations, vitest on the two affected suites, restore via file rewrite from the pristine copy, restore-clean asserted after every mutant, final git status --porcelain empty; raw vitest per mutant in logs/vitest-raw/). Killed: routing branch (4 red — exactly the PR's four MiniMax tests), base_resp check (1 red), 2xx status-text suppression (1 red); these also serve as the positive control and land in the same file as the mutant. Survivors: base64 PNG-signature guard and parseImageSize safe-integer guard (coverage gaps — proven behavior-deciding by survivor-proofs.mjs: with each guard mutated in a scratch dist copy, non-PNG base64 resolves and astral width 1e20 is forwarded, then the dist is restored sha256-identical); hostname allowlist and /v1 path restriction (redundant defence — MUTE-8 combined revert kills 11 tests; proxy.example.com/v1 and api.minimax.io/chat probes show what each guard decides in isolation). Recommended: ship the base64 pinning fixture (fixture-check.mjs proves it green intact / red disabled with the intended assertion).
  2. (Nit) Misleading error for unsupported URL candidates. Reproduce: node tmp/pr8322-verify-20260824-014212/harness.mjs --arm head --tree <repo> --out … (cells S1/S2). http:// and data:image/jpeg;base64,… candidates are classified as base64 and report "did not contain a valid PNG image"; the candidate is never fetched (exactly 1 wire request). An explicit unsupported-scheme/encoding branch would make triage cheaper; left to the author.
  3. (Nit) size: '0x0' forwarded as width: 0, height: 0 at the service level (cell M13a); the tool layer blocks it upstream (image-gen.ts:24-25,219-231). Note, not a defect.

Not covered

  • Per-commit attribution: re-checked at this checkout — git rev-list HEAD^1..HEAD^2 returns 1 OID (the shallow-boundary artifact), and the three substantive commits (a98e3f61, 959c8e6f, 434d911e) all fail git cat-file -t; only the head merge 42c0ea44 is reachable. Matches the metadata's 4-commit list in count but not in reachable objects. Verified the aggregate HEAD^1..HEAD diff; the schema / error-surfacing / magic-offset split still could not be exercised individually.
  • The base advance itself (050792977385b27): not enumerable at depth 2. Bounded by: new base-tip commit is fix(web-shell): cap React dev performance.measure accumulation to stop renderer OOM #9770 (web-shell), the four closure files are blob-identical to the diff pre-images on the new base, and every behavioral measurement was re-run at the new base anyway.
  • Live MiniMax production endpoint — no credentials in this sandbox; wire evidence comes from the recording peer and real-TLS loopback. The request schema is verified against the fields the PR's own tests declare, not against MiniMax's live API.
  • downloadPng internals (redirect chains, byte limits): pre-existing, untouched by this PR; exercised only transitively (M8/M10/M11b ran the real network policy and download path).
  • Real-TLS execution of the MiniMax hostnames themselves: mapping api.minimax.io → loopback requires /etc/hosts write access this container's user does not have; MiniMax-hostname cells ran over the fetchFn seam, the generic path ran over real TLS as the transport control (M11a/b).
  • Repo-wide gates: targeted gates only (22 files / 422 tests green on the merged tree, 05-gate-extended.png; head tsc --noEmit for packages/core clean; base rebuild clean). The PR body's exact command was reproduced green (25/25 — those 25 tests are a subset of the extended gate and not double-counted). The PR's own CI covers the rest.

Methodology

One container (node v22.23.2, bookworm), working tree at merge commit 33242f4e (PR head 42c0ea44 merged into new base 7385b27). Base control: git worktree add --detach tmp/base-tree HEAD^1, root node_modules symlinked into the worktree (repairs the relative paths mapping in packages/core/tsconfig.json, per the previous round's correction), nested packages/core/node_modules hardlinked from the root install (same lockfile on both sides — the PR touches no package.json/lockfile), then scripts/build_package.js → exit 0. Internal-link trap closed two ways: the service's 554-file dist import closure contains zero @qwen-code/* specifiers (M0), and harnesses import each tree's dist by absolute path with the M0 marker control proving the base dist contains zero MiniMax markers. Harnesses drove the compiled dist directly (harness.mjs, 16 cells × 2 arms through the fetchFn seam; tls-harness.mjs, 2 cells × 2 arms on the default global fetch against a self-signed loopback origin). Two harness iterations were needed and are disclosed: the first parity comparator wrongly included M6 (a MiniMax-host cell whose wire legitimately differs; only its message is parity) and compared ephemeral loopback ports verbatim — both fixed, and only the fixed run's counts are booked; the first mutation run parsed vitest's Test Files line instead of the Tests line — fixed with ANSI-stripped parsing and per-mutant raw logs (logs/vitest-raw/), then re-run. Mutations were single-hunk source edits restored from the pristine in-memory copy (restore-clean asserted per mutant, final git status --porcelain empty); survivor behavior proofs mutated a scratch copy of the dist and sha256-verified the restore. Targeted gate: npx vitest run src/services/image-generation-service.test.ts src/tools/image-gen.test.ts src/providers src/models → 22 files, 422/422 (the new base added one file / eleven tests to these directories since the previous round's 21/411). Evidence images evidence/01..05 via scripts/verify-capture.mjs. Assertion accounting (all scripted, all executed): harness head 83 + harness base 71 + TLS 7 + TLS 7 + parity comparator 5 + mutation-matrix/restore checks 19 + pinning-fixture checks 5 + survivor proofs 12 + extended gate 422 + head typecheck 1 + base rebuild 1 + commit-reachability comparison 1 = 634 pass, 0 fail. Expected base-arm breaks are encoded as predictions in the harness, so fail counts only unexpected outcomes — there were none. Base worktree removed after capture; final tree clean.

Flakiness gate log

rounds=5 files=2 skipped=0
file packages/core/src/providers/__tests__/presets/minimax.test.ts: (cd packages/core) npx --no-install vitest run ./src/providers/__tests__/presets/minimax.test.ts
file packages/core/src/services/image-generation-service.test.ts: (cd packages/core) npx --no-install vitest run ./src/services/image-generation-service.test.ts


per-file results (P=pass F=fail I=infra-exit, one letter per run):
  packages/core/src/providers/__tests__/presets/minimax.test.ts: PPPPP
  packages/core/src/services/image-generation-service.test.ts: PPPPP

verdict: pass
summary: 2 changed test file(s) x 5 identical rounds, no divergence

--- per-invocation detail (full copy in the artifact) ---
round 1 · packages/core/src/providers/__tests__/presets/minimax.test.ts: P (exit 0)
round 1 · packages/core/src/services/image-generation-service.test.ts: P (exit 0)
round 2 · packages/core/src/providers/__tests__/presets/minimax.test.ts: P (exit 0)
round 2 · packages/core/src/services/image-generation-service.test.ts: P (exit 0)
round 3 · packages/core/src/providers/__tests__/presets/minimax.test.ts: P (exit 0)
round 3 · packages/core/src/services/image-generation-service.test.ts: P (exit 0)
round 4 · packages/core/src/providers/__tests__/presets/minimax.test.ts: P (exit 0)
round 4 · packages/core/src/services/image-generation-service.test.ts: P (exit 0)
round 5 · packages/core/src/providers/__tests__/presets/minimax.test.ts: P (exit 0)
round 5 · packages/core/src/services/image-generation-service.test.ts: P (exit 0)

Evidence images

01-ab-head-arm

02-ab-base-arm

03-parity-compare

04-mutation-matrix

05-gate-extended

Harness scripts and raw logs are in the workflow run artifacts (7-day retention).

Qwen Code · sandboxed verification

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

The Stage 1a template gate from Aug 1 is cleared — thanks @octo-patch for the rewrite, the body now follows the template with all required sections and the Chinese translation ✓

  • Problem: real, not theoretical. The base image generation service hard-codes the DashScope generation path and request/response schema, so a MiniMax image base URL could only ever produce a request MiniMax cannot answer. The hard-coding is visible in the code on main, and the maintainer's real-stack verification in this thread observed the before/after on the wire directly.
  • Direction: aligned. Configurable per-provider image generation is a shipped feature (feat(core): add configurable image generation models, feat(core): add configurable image generation models #7607; provider-requirements docs, docs: document compaction and image model selection #8348), and MiniMax already ships as a chat provider preset — this completes the image side for it. No matching reference in the reference-agent CHANGELOG, but the area is clearly relevant to this project's own direction.
  • Size: core paths are touched (services, providers): 180 production lines (image service 178, preset 2) vs 135 test lines across 4 files — below every escalation threshold.
  • Approach: scope feels right. Host-keyed routing for the two official MiniMax endpoints, a dedicated MiniMax request/response path that reuses the existing bounded-read, error-formatting, and PNG download/validation pipeline, two imageOnly preset entries, and an explicit fallback to today's behaviour for every other host. No drive-by changes in the diff.
  • Risk: no elevated-risk paths matched (Stage 1e).

Moving on to code review. 🔍

中文说明

8 月 1 日的 Stage 1a 模板门禁已解除 —— 感谢 @octo-patch 重写正文,现已遵循模板,所有必填小节及中文翻译齐全 ✓

  • 问题: 真实存在,而非理论性问题。基础代码中的图像生成服务写死了 DashScope 的生成路径与请求/响应结构,因此 MiniMax 图像 base URL 只能得到 MiniMax 无法应答的请求。这一写死在 main 分支代码中直接可见,本讨论串中维护者的真实栈验证也在网络层面直接观测到了前后差异。
  • 方向: 对齐。可配置的按 provider 图像生成是已上线的功能(feat(core): add configurable image generation modelsfeat(core): add configurable image generation models #7607;provider 要求文档,docs: document compaction and image model selection #8348),而 MiniMax 已经是内置的对话 provider —— 本 PR 补齐了它的图像侧。参考 agent 的 CHANGELOG 中没有直接对应条目,但该方向与本项目自身方向明显相关。
  • 规模: 触及核心路径(servicesproviders):180 行生产代码(图像服务 178 行、预设 2 行)对 135 行测试代码,共 4 个文件 —— 低于所有升级阈值。
  • 方案: 范围合理。按两个 MiniMax 官方主机做路由,专用的 MiniMax 请求/响应路径复用既有的有界读取、错误格式化与 PNG 下载/校验流程,新增两个 imageOnly 预设条目,其余所有主机明确回退到现有行为。diff 中没有顺手改动。
  • 风险: 未命中高风险路径(Stage 1e)。

进入代码审查 🔍

Qwen Code · qwen3.8-max

Reviewed at 42c0ea44046cdac011d20ffda485cc133ffa38a2 · re-run with @qwen-code /triage

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Code review. Before reading the diff I wrote down my own proposal for "make image_gen work for MiniMax": host-keyed routing inside the existing service, a dedicated MiniMax request/response branch that reuses the bounded-read, error-formatting and PNG download pipeline, and imageOnly entries in the MiniMax preset — a per-provider adapter layer would be over-engineering at two providers. The PR matches that shape, so there is no simpler path I can point to. No critical blockers found; specifics worth recording:

  • Safety posture is preserved, not bypassed. The MiniMax URL branch goes through the same downloadPng pipeline as DashScope (HTTPS-only, private-host rejection, network-policy resolution, redirect cap, 10 MB size cap, PNG signature check). The base64 branch is bounded by the same 1 MB response-body cap and signature-checks the decoded bytes before returning.
  • Error envelope handling is correct. base_resp.status_code is treated as success only when 0, both numeric and string forms are handled, and application-level failures reported with HTTP 200 surface MiniMax's own code/message. The shared formatImageGenerationError change only alters output for 2xx statuses, which were unreachable on the DashScope path — its message shape there is unchanged.
  • The preset entries land correctly. imageOnly is the established mechanism: propagated through the install plan and model registry, excluded from chat/fast/voice selection everywhere, and recognised by the image-generation capability check, which is what resolveImageGenerationModel filters on together with the preset's envKey and base URL — so /model --image image-01 wires straight into the MiniMax route.
  • The duplicated request/!response.ok handling between the two provider paths is within tolerance for two providers; extracting it now would be premature abstraction.

Testing evidence — this is an unattended CI run, so nothing was built or executed here; the evidence below is the PR's own CI at 42c0ea44 fetched via the API, plus the verification runs already recorded in this thread:

Check Conclusion
Test (ubuntu-latest, Node 22.x) ✅ success
Test (macos-latest, Node 22.x) ⏭️ skipped (fork lane)
Test (windows-latest, Node 22.x) ⏭️ skipped (fork lane)
Integration Tests (CLI, No Sandbox) ⏭️ skipped (fork lane)
Desktop Shell (ubuntu-22.04) ✅ success
Desktop Shell (windows-2022) ✅ success
web-shell E2E Smoke (ubuntu-latest, Node 22.x) ✅ success
Classify PR ✅ success

No red checks; the skipped lanes are the standard fork-PR set, matching prior runs on this head. The added unit tests pin the MiniMax request URL and exact body, the base64 branch, and the base_resp error message, so the suite exercises the new code paths.

Beyond the unit suite, the behavioural claim is already substantiated at this exact head by two prior sandboxed /verify runs (484/484 assertions, "merge-ready" agent verdict) and by the maintainer's two real-stack wire captures (19/19 scenario assertions at this head — maintainer's evidence, attributed above in the thread). The /verify re-run triggered with today's triage is still in progress and will post its own report here when it completes. Not verified here: live MiniMax credentials were never used in any run — the wire evidence is against a stand-in origin, which pins the protocol but not MiniMax's production behaviour.

中文说明

代码审查。 在读 diff 之前我先写下了自己的方案:在现有服务内按主机路由,为 MiniMax 单独开一个请求/响应分支并复用有界读取、错误格式化与 PNG 下载流程,再在 MiniMax 预设中加入 imageOnly 条目 —— 只有两个 provider 时,做一套 per-provider 适配层属于过度设计。PR 与这一方案一致,因此没有更简的路径可提。未发现关键阻塞项,值得记录的细节:

  • 安全姿态被保留而非绕过。 MiniMax 的 URL 分支与 DashScope 走同一条 downloadPng 流程(仅 HTTPS、拒绝私有主机、网络策略解析、重定向上限、10 MB 大小上限、PNG 签名校验)。base64 分支受同样的 1 MB 响应体上限约束,且解码后的字节先经过签名校验才返回。
  • 错误信封处理正确。 base_resp.status_code 仅在为 0 时视为成功,数字与字符串形态都能处理;以 HTTP 200 返回的应用级错误会带出 MiniMax 自身的错误码与信息。共享的 formatImageGenerationError 改动只影响 2xx 状态码的输出,而 DashScope 路径不可能走到 2xx —— 其错误消息形态保持不变。
  • 预设条目正确落地。 imageOnly 是既有机制:经安装计划与模型注册表传播,在所有对话/快速/语音选择中被排除,并被图像生成能力检查识别 —— resolveImageGenerationModel 正是用它连同预设的 envKey 与 base URL 一起过滤,因此 /model --image image-01 可直接接入 MiniMax 路由。
  • 两条 provider 路径之间重复的请求/!response.ok 处理,在只有两个 provider 的情况下可以接受;现在抽取属于过早抽象。

测试证据 —— 本次为无人值守 CI 运行,未在此构建或执行任何代码;以上证据为通过 API 获取的该 PR 在 42c0ea44 上的自身 CI 结果,以及本讨论串中已记录的验证运行。CI 无失败检查;跳过的通道是 fork PR 的标准集合,与该 head 此前的运行一致。新增单测固定了 MiniMax 请求 URL 与完整请求体、base64 分支以及 base_resp 错误信息,测试套件确实覆盖了新代码路径。

单测之外,行为层面的声明在该 head 上已有佐证:此前两次沙箱 /verify 运行(484/484 断言,代理结论 "merge-ready"),以及维护者两次真实栈网络层抓取(该 head 上 19/19 场景断言 —— 为维护者的证据,见讨论串上方)。随本次 triage 触发的 /verify 复跑仍在进行中,完成后会在本串发布自己的报告。此处未验证:所有运行均未使用真实 MiniMax 凭证 —— 网络层证据针对替身源站,固定的是协议本身,而非 MiniMax 生产环境行为。

Qwen Code · qwen3.8-max

Reviewed at 42c0ea44046cdac011d20ffda485cc133ffa38a2 · re-run with @qwen-code /triage

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Confidence: 5/5 — real problem, minimal focused diff, green CI, tests that pin the new paths, and wire-level verification at this exact head.

Stepping back: this one holds together. The problem was verified against the code on main — the DashScope path really is hard-coded — and against the wire by the maintainer, so I am not approving the PR's framing, I am approving an observed gap being closed. The implementation is the minimal version of the fix: two hostname checks, one dedicated function that reuses every existing safety mechanism instead of re-implementing any of them, and two additive preset entries. Every downstream consumer is nameable (image_gen tool contract unchanged, config resolution, model-selection filters), and nothing outside the MiniMax hosts changes behaviour. The results match what was promised: the suite pins the request/response shapes, CI is green on the head, and the sandboxed + real-stack runs show an image where there was none before. In six months the routing reads as two obvious hostname === ... checks — thank, not curse. I'm approving because it is genuinely good, not because I ran out of objections.

One hygiene action completes the loop: the bot's Aug 1 request-changes review existed only for the template gate, which the author has since cleared — it is dismissed alongside this approval so it no longer blocks the PR.

中文说明

信心:5/5 —— 问题真实存在,diff 最小且聚焦,CI 全绿,测试固定了新路径,且该 head 已有网络层验证。

退一步看:这个 PR 是立得住的。问题既在 main 代码中核实过 —— DashScope 路径确实是写死的 —— 也由维护者在网络层观测过,所以我批准的不是 PR 的自我叙述,而是一个已观测到的缺口被补上。实现是该修复的最小形态:两个主机名判断、一个复用全部既有安全机制而非重新实现它们的专用函数,以及两个追加的预设条目。每个下游消费方都可点名(image_gen 工具契约不变、配置解析、模型选择过滤器),MiniMax 主机之外的行为完全不变。结果与承诺一致:测试套件固定了请求/响应形态,该 head 上 CI 全绿,沙箱与真实栈运行展示了从无图到有图。六个月后再看,这段路由就是两个一目了然的 hostname === ... 判断 —— 值得感谢而非抱怨。我批准是因为它确实好,而不是因为我说不出反对意见。

一项收尾操作:机器人 8 月 1 日的 request-changes 评审仅为模板门禁而存在,作者现已补齐模板 —— 它将随本次批准一并被 dismiss,不再阻塞该 PR。

Qwen Code · qwen3.8-max

Reviewed at 42c0ea44046cdac011d20ffda485cc133ffa38a2 · re-run with @qwen-code /triage

@qwen-code-ci-bot qwen-code-ci-bot 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.

LGTM, looks ready to ship. ✅

@wenshao
wenshao added this pull request to the merge queue Aug 24, 2026
@qwen-code-ci-bot
qwen-code-ci-bot dismissed their stale review August 24, 2026 01:41

Stage 1a template gate resolved: the PR body was rewritten on Aug 17 and now follows the pull request template. Full triage completed at 42c0ea4 — see the stage 1-3 comments and the approval.

Merged via the queue into QwenLM:main with commit 5db4a95 Aug 24, 2026
69 of 70 checks passed
@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Released in v0.22.2.

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.

4 participants