fix(qwen): allow qwen3.6-plus opt-in on Coding Plan #63654#63987
fix(qwen): allow qwen3.6-plus opt-in on Coding Plan #63654#63987jepson-liu wants to merge 2 commits into
Conversation
Greptile SummaryThis PR removes the Confidence Score: 5/5Safe to merge; the production fix is surgical and correct, with only a minor test-quality concern. The code change is a one-hook deletion with a clear, well-scoped intent and no side-effects on the remaining suppressBuiltInModel path. The single P2 finding is a test quality issue (vacuous assertion) that does not affect correctness or runtime behavior. extensions/qwen/index.test.ts — the new test is vacuous and should be strengthened. Prompt To Fix All With AIThis is a comment left during a code review.
Path: extensions/qwen/index.test.ts
Line: 10-31
Comment:
**Vacuous test — always passes trivially**
Because `normalizeConfig` was removed entirely from the plugin definition, `provider.normalizeConfig` is `undefined`, so the optional-chain expression `provider.normalizeConfig?.({...})` always evaluates to `undefined` without executing any logic. The assertion `toBeUndefined()` therefore passes unconditionally regardless of the provider's implementation. The test would equally "pass" if `normalizeConfig` were re-added as a no-op that returns `undefined` for all inputs, silently missing the regression.
A stronger approach is to explicitly assert that the hook is absent, and then add a second case that verifies the built-in suppression (`suppressBuiltInModel`) still fires for Coding Plan endpoints — that guards the half of the contract that remains:
```ts
it("does not have a normalizeConfig hook (no user-config mutation)", () => {
const provider = await registerSingleProviderPlugin(qwenPlugin);
expect(provider.normalizeConfig).toBeUndefined();
});
it("still suppresses qwen3.6-plus from built-in catalog on Coding Plan endpoints", async () => {
const provider = await registerSingleProviderPlugin(qwenPlugin);
const result = provider.suppressBuiltInModel?.({
provider: "qwen",
modelId: QWEN_36_PLUS_MODEL_ID,
baseUrl: QWEN_CN_BASE_URL,
config: undefined,
} as never);
expect(result?.suppress).toBe(true);
});
```
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "fix(qwen): allow qwen3.6-plus opt-in on ..." | Re-trigger Greptile |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: faf8fde06f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Summary
qwenprovider dropsqwen3.6-plusfrom user config whenbaseUrlpoints at the Qwen Coding Plan endpoint, which makes vision/image flows fail withUnknown model.qwen3.6-plusfor image understanding even when they explicitly configure it.qwenprovidernormalizeConfigfilter that strippedqwen3.6-pluson Coding Plan endpoints. Built-in catalog suppression behavior remains unchanged.qwen3.6-plusfor Coding Plan endpoints by default, and the existing suppression hint remains.Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
User-visible / Behavior Changes
Users can now explicitly configure
models.providers.qwen.models: [{ id: "qwen3.6-plus", input: ["text","image"], ... }]on Coding Plan endpoints without OpenClaw removing it during provider config normalization.Security Impact (required)
Repro + Verification
Environment
qwen)https://coding.dashscope.aliyuncs.com/v1Steps
models.providers.qwen.baseUrlto a Coding Plan endpoint and explicitly includeqwen3.6-plusinmodels.providers.qwen.models.imagetool / media-understanding pipeline) usingqwen/qwen3.6-plus.Expected
Actual (before)
qwen3.6-plus, leading toUnknown modelat runtime.Evidence
Human Verification (required)
pnpm test extensions/qwen/provider-catalog.test.tspnpm test extensions/qwen/index.test.tsCompatibility / Migration
Risks and Mitigations