feat: Add Qwen model recognition tests and documentation#1085
Closed
LoneWolf36 wants to merge 6 commits intocoleam00:devfrom
Closed
feat: Add Qwen model recognition tests and documentation#1085LoneWolf36 wants to merge 6 commits intocoleam00:devfrom
LoneWolf36 wants to merge 6 commits intocoleam00:devfrom
Conversation
Integrate Qwen as a first-class assistant across config, workflows, CLI setup, API schemas, and the web UI. Add a Qwen smoke test and tighten the docs so the remaining provider-specific limits are explicit. This keeps Archon aligned with the new Qwen path while documenting where behavior still differs from Claude and Codex. Co-Authored-By: Codex GPT-5 <noreply@openai.com>
Preserve existing Qwen settings when assistant config is updated so the web settings flow no longer drops qwen entries from global config. Stop forcing OpenAI auth in the Qwen client and allow the SDK to follow its native auth resolution unless Archon is configured explicitly. Also pass through OpenAI-compatible env vars and wire the Qwen smoke test into the package test script so the reviewed behavior stays covered. Co-Authored-By: Codex GPT-5 <noreply@openai.com>
Prevent duplicate tool-call emission when partial Qwen streaming emits a tool start before the final assistant message, and pass native Qwen auth environment variables through the subprocess allowlist. Co-Authored-By: Codex GPT-5 <noreply@openai.com>
- Config persistence: unconditionally preserve qwen in updateGlobalConfig() - CLI setup: filter default assistant options by actually configured assistants - Add mode: read hasQwen and defaultAssistant from existing config instead of hardcoding - Missing env vars: inject config.envVars into Qwen node options in dag-executor - Provider misrouting: return undefined instead of 'codex' for unknown models - Streaming dedup: track sawPartialText separately from tool_use partials - Security: scope Qwen credentials to QwenClient only, remove from shared allowlist - UI robustness: add optional chaining for config.assistants.qwen?.model in SettingsPage Co-Authored-By: Codex GPT-5 <noreply@openai.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
…ator warnings - Schema: make qwen optional+nullable in safeConfigSchema for unset state - Docstring: fix authType docstring to reflect SDK-resolved default (not 'openai') - Validator: add Qwen warnings for MCP, skills, hooks, and tool restrictions (previously only warned for Codex; now warns for both non-Claude providers) Co-Authored-By: Codex GPT-5 <noreply@openai.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
- Improve model recognition for Qwen variants (qwen-coder, qwen-max, qwen-turbo, qwen-plus, qwq-*, qvq-*) - Add comprehensive test coverage for Qwen model validation and provider inference - Create detailed Qwen setup guide in docs/qwen-setup.md - Update README.md to highlight Qwen as first-class AI assistant - Add configuration examples and migration guide for binary v0.3.5 users - Ensure all Qwen model patterns are properly recognized in isQwenModel() - Add tests for provider inference with various Qwen model names - Document Qwen-specific workflow configuration patterns Closes: Qwen support gap between source code and binary v0.3.5 Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
📝 WalkthroughWalkthroughThis PR adds comprehensive Qwen (通义千问) support as a first-class AI assistant alongside Claude and Codex. Changes include documentation, configuration/setup workflows, a new QwenClient implementation with streaming and error handling, type system updates, and UI enhancements for assistant selection and model configuration. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~70 minutes
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/workflows/src/validator.ts (1)
339-355:⚠️ Potential issue | 🟠 MajorRemove the Qwen MCP “ignored” warning.
This is now backwards:
packages/workflows/src/dag-executor.tsloadsnode.mcpfor Qwen and forwards it asoptions.mcpServers, so validation will warn about a feature the executor actually supports.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/workflows/src/validator.ts` around lines 339 - 355, The Qwen-specific warning in the validator is incorrect because dag-executor.ts does read node.mcp and forwards it as options.mcpServers; remove the else if (provider === 'qwen') { ... } block that pushes the 'mcp' warning (the issues.push call referencing node.id and field: 'mcp') so only the Codex check remains (i.e., keep the provider === 'codex' branch and delete the Qwen branch and its message/hint).packages/workflows/src/dag-executor.ts (1)
1713-1734:⚠️ Potential issue | 🟠 MajorQwen loop nodes never get the same env/config wiring as normal Qwen nodes.
resolveNodeProviderAndModel()injectsconfig.envVarsand Qwen-specific MCP/tool settings for prompt/command nodes, butbuildLoopNodeOptions()only returns{ model }for Qwen. That means a workflow can succeed with Qwen in regular nodes and then fail only when it enters a loop. Please reuse the same provider-specific option builder for loop iterations.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/workflows/src/dag-executor.ts` around lines 1713 - 1734, buildLoopNodeOptions currently only returns { model } for non-codex/claude providers so Qwen loop iterations miss env/config wiring; update buildLoopNodeOptions to reuse the same provider-specific option builder used by resolveNodeProviderAndModel (i.e., include config.envVars and Qwen-specific MCP/tool settings from config.assistants.qwen) so that when provider === 'qwen' the returned WorkflowAssistantOptions includes model plus envVars and any MCP/tool/prompt-related settings; ensure existing codex and claude branches remain unchanged and merge all option objects the same way as the other builder.packages/cli/src/commands/setup.ts (1)
1483-1498:⚠️ Potential issue | 🔴 Critical
addmode still rewrites.envfrom incomplete state.This bootstrap preserves the Qwen flags, but it still drops every existing value that is not re-collected here:
DATABASE_URL, Claude/Codex auth, and credentials for already-configured platforms. SincewriteEnvFiles()fully overwrites both env files, choosing “Add platforms” can silently destroy a working setup.Also applies to: 1561-1562
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/commands/setup.ts` around lines 1483 - 1498, The current config construction in setup.ts (the local variable config used before calling writeEnvFiles()) clobbers existing env vars in "add" mode; update the logic to merge existing environment values into the new config instead of replacing them — preserve DATABASE_URL, existing AI auth keys (Claude/Codex/Qwen tokens), and any platform credentials for already-configured platforms by reading from the existing env/object (existing or existingEnv) and copying those keys into the config or the env object passed to writeEnvFiles(); alternatively change writeEnvFiles() call sites (also where referenced around the other block at lines 1561-1562) to accept a mergedEnv that overlays the new choices on top of the existing env so no previously-collected secrets are dropped.
🧹 Nitpick comments (1)
packages/core/src/clients/factory.ts (1)
39-41: Consider usingASSISTANT_TYPE_VALUESfor the error message.The error message hardcodes
'claude', 'codex', 'qwen'whileconfig-types.tsalready definesASSISTANT_TYPE_VALUES = ['claude', 'codex', 'qwen']. Using the shared constant would keep them in sync automatically.♻️ Suggested improvement
+import { ASSISTANT_TYPE_VALUES } from '../config/config-types'; // ... throw new Error( - `Unknown assistant type: ${type}. Supported types: 'claude', 'codex', 'qwen'` + `Unknown assistant type: ${type}. Supported types: ${ASSISTANT_TYPE_VALUES.map(t => `'${t}'`).join(', ')}` );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/clients/factory.ts` around lines 39 - 41, The thrown error hardcodes supported assistant names; replace the literal list in the Error thrown (the throw new Error(...) in factory.ts) with the shared constant ASSISTANT_TYPE_VALUES so they stay in sync—use ASSISTANT_TYPE_VALUES.join(', ') (or similar) inside the template string to produce the same message, e.g. reference ASSISTANT_TYPE_VALUES in the Error construction instead of the hardcoded 'claude', 'codex', 'qwen'.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/qwen-setup.md`:
- Around line 76-81: The fenced code block that starts with "? Which AI
assistant(s) will you use?" is missing a language tag, causing markdownlint
MD040; update that block (the triple-backtick fence enclosing the lines starting
with "? Which AI assistant(s) will you use?" and the list lines containing
"Claude", "Codex", and "Qwen") to include a language/tag such as "text"
immediately after the opening ``` so the fence reads ```text, preserving the
block content exactly.
- Around line 119-123: Update the YAML example for assistants.qwen.authType to
remove the invalid entries and show only the accepted authType values defined by
the config schema (e.g., replace "dashscope-api-key, bailian-api-key" with the
supported value(s) such as "qwen-oauth" or the exact allowed identifiers from
the config type), ensuring the example under assistants.qwen.authType matches
the config type definition.
In `@packages/cli/src/commands/setup.ts`:
- Around line 243-262: The setup writes Qwen-related guidance only as comments
so checkExistingConfig() can't detect it later (hasQwen ends up false); update
generateEnvContent() (and the block that writes DEFAULT_AI_ASSISTANT) to emit a
real machine-readable assignment like DEFAULT_AI_ASSISTANT=qwen when Qwen auth
is chosen, and similarly persist any chosen Qwen keys (e.g., QWEN_AUTH_TYPE or
DASHSCOPE_API_KEY/BAILIAN_CODING_PLAN_API_KEY) so hasEnvValue(content, ...) and
the envDefaultAssistant parsing logic will recognize Qwen on subsequent runs;
ensure the same change is applied for the other occurrence mentioned (around the
second block at ~1134-1143).
In `@packages/core/src/clients/qwen.ts`:
- Around line 78-95: The buildQwenEnv function currently reapplies global
process.env credentials after layering mergedEnv and extraEnv, which can
overwrite request-scoped values; modify buildQwenEnv so it only injects
process.env.DASHSCOPE_API_KEY and process.env.BAILIAN_CODING_PLAN_API_KEY when
those keys are not already present in the assembled env (i.e., check if
env.DASHSCOPE_API_KEY and env.BAILIAN_CODING_PLAN_API_KEY are undefined before
assigning from process.env) to preserve mergedConfig.envVars/options.env
overrides.
- Around line 272-311: The code currently processes and emits partial events
even when includePartialMessages is false; change the
isSDKPartialAssistantMessage handling so that partial events are ignored when
shouldStreamPartials is false (do not call emitPartialMessage and do not update
sawPartialText or partialToolCallIds). Concretely, wrap the
isSDKPartialAssistantMessage branch with a guard on shouldStreamPartials (or
early continue when !shouldStreamPartials) so no bookkeeping or yields occur for
partials; keep the rest of the flow (isSDKAssistantMessage and
emitAssistantMessageBlocks) unchanged and still reference sawPartialText and
partialToolCallIds only when partials were actually processed.
In `@packages/core/src/db/conversations.ts`:
- Around line 76-78: The current guard only normalizes the initial assistantType
but later overrides can set it to unsupported values; before persisting a
conversation, re-validate the final assistantType variable (assistantType)
against the allowed set ('claude', 'codex', 'qwen') and throw an explicit Error
if it is not one of them instead of silently defaulting—i.e., locate the code
path that performs the DB insert (where the conversation is persisted) and add a
fail-fast check that throws a clear exception when assistantType is invalid.
In `@packages/core/src/handlers/clone.ts`:
- Around line 108-118: The log event names used in the getLog().debug calls do
not follow the required {domain}.{action}_{state} convention; update the calls
that currently use 'assistant_detected_qwen', 'assistant_detected_claude', and
'assistant_default_claude' to follow the pattern (for example
'assistant.detected_qwen', 'assistant.detected_claude', and
'assistant.default_claude') where these strings are passed to getLog().debug
near the qwenFolder/claudeFolder checks and where suggestedAssistant is set.
In `@packages/core/src/utils/env-allowlist.test.ts`:
- Around line 91-93: The test incorrectly asserts that
SUBPROCESS_ENV_ALLOWLIST.has('DASHSCOPE_API_KEY') is true even though
'DASHSCOPE_API_KEY' is not part of SUBPROCESS_ENV_ALLOWLIST (it’s added
separately in buildQwenEnv() in qwen.ts); update the test in
env-allowlist.test.ts to expect false (or remove the assertion) so it reflects
actual behavior, or alternatively add 'DASHSCOPE_API_KEY' to the
SUBPROCESS_ENV_ALLOWLIST if you intend it to be globally allowed—refer to
SUBPROCESS_ENV_ALLOWLIST, 'DASHSCOPE_API_KEY', and buildQwenEnv() when making
the change.
- Around line 58-64: The test fails because buildCleanSubprocessEnv() only
returns vars from SUBPROCESS_ENV_ALLOWLIST but DASHSCOPE_API_KEY and
BAILIAN_CODING_PLAN_API_KEY are not in that allowlist; to fix, add the two keys
(DASHSCOPE_API_KEY and BAILIAN_CODING_PLAN_API_KEY) to SUBPROCESS_ENV_ALLOWLIST
in env-allowlist.ts so buildCleanSubprocessEnv() will include them, or
alternatively remove the expectations and the allowlist assertion from the
env-allowlist.test.ts test (or change the test to reflect that clients/qwen.ts
adds those keys separately) — update either SUBPROCESS_ENV_ALLOWLIST or the test
to keep behavior and assertions consistent.
In `@packages/web/src/lib/api.generated.d.ts`:
- Around line 2502-2504: The generated SafeConfig type incorrectly defines the
field qwen as a non-null object; update the generation so SafeConfig has qwen?:
{ model?: string } | null to match the server schema (.optional().nullable()) —
regenerate the OpenAPI/TypeScript client (the generator that produces
packages/web/src/lib/api.generated.d.ts) from the server schema in
config.schemas.ts, ensuring the SafeConfig type and the qwen property are
emitted as optional and nullable; verify the produced declaration contains
"qwen?: { model?: string } | null" and commit the updated generated file.
In `@packages/workflows/src/model-validation.ts`:
- Around line 14-26: isQwenModel is too permissive and misclassifies IDs like
"qwenish-test" or "mycompany-coder-model"; change the matcher to only accept
documented Qwen prefixes and explicit aliases: require prefix matches like
"qwen-", "qwq-", "qvq-" (note the trailing dash) and allow only explicit
whole-token aliases you truly support such as "qwen-max", "qwen-turbo",
"qwen-plus" (avoid using endsWith('-coder-model'), equality to "coder-model", or
generic includes). Update isQwenModel to use strict prefix checks and exact
alias checks, and add negative regression tests asserting that values like
"qwenish-test" and "mycompany-coder-model" do NOT match while valid patterns
(e.g., "qwen-max", "qwen-123") DO.
In `@packages/workflows/src/validator.ts`:
- Around line 248-250: The resolveProvider function currently only checks
node.provider and workflowProvider, so nodes with node.model (e.g., model:
qwen-max) are incorrectly validated as the workflow/default provider; update
resolveProvider to call inferProviderFromModel(node.model) after checking
node.provider and before falling back to workflowProvider so the inferred
provider (from node.model) is returned when present; reference the
resolveProvider function, the node.model property, and the
inferProviderFromModel helper when making this change.
---
Outside diff comments:
In `@packages/cli/src/commands/setup.ts`:
- Around line 1483-1498: The current config construction in setup.ts (the local
variable config used before calling writeEnvFiles()) clobbers existing env vars
in "add" mode; update the logic to merge existing environment values into the
new config instead of replacing them — preserve DATABASE_URL, existing AI auth
keys (Claude/Codex/Qwen tokens), and any platform credentials for
already-configured platforms by reading from the existing env/object (existing
or existingEnv) and copying those keys into the config or the env object passed
to writeEnvFiles(); alternatively change writeEnvFiles() call sites (also where
referenced around the other block at lines 1561-1562) to accept a mergedEnv that
overlays the new choices on top of the existing env so no previously-collected
secrets are dropped.
In `@packages/workflows/src/dag-executor.ts`:
- Around line 1713-1734: buildLoopNodeOptions currently only returns { model }
for non-codex/claude providers so Qwen loop iterations miss env/config wiring;
update buildLoopNodeOptions to reuse the same provider-specific option builder
used by resolveNodeProviderAndModel (i.e., include config.envVars and
Qwen-specific MCP/tool settings from config.assistants.qwen) so that when
provider === 'qwen' the returned WorkflowAssistantOptions includes model plus
envVars and any MCP/tool/prompt-related settings; ensure existing codex and
claude branches remain unchanged and merge all option objects the same way as
the other builder.
In `@packages/workflows/src/validator.ts`:
- Around line 339-355: The Qwen-specific warning in the validator is incorrect
because dag-executor.ts does read node.mcp and forwards it as
options.mcpServers; remove the else if (provider === 'qwen') { ... } block that
pushes the 'mcp' warning (the issues.push call referencing node.id and field:
'mcp') so only the Codex check remains (i.e., keep the provider === 'codex'
branch and delete the Qwen branch and its message/hint).
---
Nitpick comments:
In `@packages/core/src/clients/factory.ts`:
- Around line 39-41: The thrown error hardcodes supported assistant names;
replace the literal list in the Error thrown (the throw new Error(...) in
factory.ts) with the shared constant ASSISTANT_TYPE_VALUES so they stay in
sync—use ASSISTANT_TYPE_VALUES.join(', ') (or similar) inside the template
string to produce the same message, e.g. reference ASSISTANT_TYPE_VALUES in the
Error construction instead of the hardcoded 'claude', 'codex', 'qwen'.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2fdd635c-93dd-42fb-8f5b-d1323cfb4f8b
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (43)
CLAUDE.mdREADME.mddocs/qwen-setup.mdpackages/cli/src/commands/setup.test.tspackages/cli/src/commands/setup.tspackages/core/package.jsonpackages/core/src/clients/factory.test.tspackages/core/src/clients/factory.tspackages/core/src/clients/index.tspackages/core/src/clients/qwen.test.tspackages/core/src/clients/qwen.tspackages/core/src/config/config-loader.test.tspackages/core/src/config/config-loader.tspackages/core/src/config/config-types.tspackages/core/src/db/conversations.tspackages/core/src/handlers/clone.test.tspackages/core/src/handlers/clone.tspackages/core/src/index.tspackages/core/src/orchestrator/orchestrator.test.tspackages/core/src/services/title-generator.tspackages/core/src/utils/env-allowlist.test.tspackages/core/src/utils/env-allowlist.tspackages/server/src/routes/api.tspackages/server/src/routes/schemas/config.schemas.tspackages/web/src/components/workflows/BuilderToolbar.tsxpackages/web/src/components/workflows/NodeInspector.tsxpackages/web/src/components/workflows/WorkflowBuilder.tsxpackages/web/src/lib/api.generated.d.tspackages/web/src/routes/SettingsPage.tsxpackages/workflows/src/dag-executor.test.tspackages/workflows/src/dag-executor.tspackages/workflows/src/deps.tspackages/workflows/src/executor-preamble.test.tspackages/workflows/src/executor.test.tspackages/workflows/src/executor.tspackages/workflows/src/loader.tspackages/workflows/src/model-validation.test.tspackages/workflows/src/model-validation.tspackages/workflows/src/schemas/dag-node.tspackages/workflows/src/schemas/workflow.tspackages/workflows/src/script-node-deps.test.tspackages/workflows/src/validator.test.tspackages/workflows/src/validator.ts
Comment on lines
+76
to
+81
| ``` | ||
| ? Which AI assistant(s) will you use? | ||
| ◯ Claude (Recommended) | ||
| ◯ Codex | ||
| ● Qwen ← Select this | ||
| ``` |
There was a problem hiding this comment.
Add a language tag to the fenced block.
This block is missing a fenced-code language and will keep markdownlint warning MD040 active.
✏️ Proposed doc fix
-```
+```text
? Which AI assistant(s) will you use?
◯ Claude (Recommended)
◯ Codex
● Qwen ← Select this</details>
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.22.0)</summary>
[warning] 76-76: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against the current code and only fix it if needed.
In @docs/qwen-setup.md around lines 76 - 81, The fenced code block that starts
with "? Which AI assistant(s) will you use?" is missing a language tag, causing
markdownlint MD040; update that block (the triple-backtick fence enclosing the
lines starting with "? Which AI assistant(s) will you use?" and the list lines
containing "Claude", "Codex", and "Qwen") to include a language/tag such as
"text" immediately after the opening so the fence readstext, preserving
the block content exactly.
</details>
<!-- fingerprinting:phantom:poseidon:hawk:50ce6ddc-d5f3-48c4-9b83-d49c27f81696 -->
<!-- This is an auto-generated comment by CodeRabbit -->
Comment on lines
+119
to
+123
| ```yaml | ||
| assistants: | ||
| qwen: | ||
| authType: qwen-oauth # or 'dashscope-api-key', 'bailian-api-key' | ||
| model: qwen-coder |
There was a problem hiding this comment.
authType example lists unsupported values.
dashscope-api-key and bailian-api-key are not valid assistants.qwen.authType values based on the current config type definition. This should only document accepted values to avoid invalid configs.
✏️ Proposed doc fix
- authType: qwen-oauth # or 'dashscope-api-key', 'bailian-api-key'
+ authType: qwen-oauth # or 'openai'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/qwen-setup.md` around lines 119 - 123, Update the YAML example for
assistants.qwen.authType to remove the invalid entries and show only the
accepted authType values defined by the config schema (e.g., replace
"dashscope-api-key, bailian-api-key" with the supported value(s) such as
"qwen-oauth" or the exact allowed identifiers from the config type), ensuring
the example under assistants.qwen.authType matches the config type definition.
Comment on lines
+243
to
+262
| // Detect Qwen: either Qwen SDK auth files or OpenAI-compatible endpoint configured | ||
| const hasQwen = | ||
| hasEnvValue(content, 'DASHSCOPE_API_KEY') || | ||
| hasEnvValue(content, 'BAILIAN_CODING_PLAN_API_KEY') || | ||
| hasEnvValue(content, 'QWEN_AUTH_TYPE'); | ||
|
|
||
| // Detect default assistant from env, fallback to claude | ||
| const envDefaultAssistant = hasEnvValue(content, 'DEFAULT_AI_ASSISTANT') | ||
| ? (content.match(/^DEFAULT_AI_ASSISTANT=(.+)$/m)?.[1]?.trim() as | ||
| | 'claude' | ||
| | 'codex' | ||
| | 'qwen' | ||
| | undefined) | ||
| : undefined; | ||
| const defaultAssistant: 'claude' | 'codex' | 'qwen' = | ||
| envDefaultAssistant === 'claude' || | ||
| envDefaultAssistant === 'codex' || | ||
| envDefaultAssistant === 'qwen' | ||
| ? envDefaultAssistant | ||
| : 'claude'; |
There was a problem hiding this comment.
Persist the Qwen selection in a machine-readable way.
checkExistingConfig() only recognizes Qwen from actual env assignments, but generateEnvContent() writes comments only. After a successful setup that relies on default Qwen auth, rerunning archon setup will read hasQwen = false, so add-mode cannot preserve the user's Qwen selection.
Also applies to: 1134-1143
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/cli/src/commands/setup.ts` around lines 243 - 262, The setup writes
Qwen-related guidance only as comments so checkExistingConfig() can't detect it
later (hasQwen ends up false); update generateEnvContent() (and the block that
writes DEFAULT_AI_ASSISTANT) to emit a real machine-readable assignment like
DEFAULT_AI_ASSISTANT=qwen when Qwen auth is chosen, and similarly persist any
chosen Qwen keys (e.g., QWEN_AUTH_TYPE or
DASHSCOPE_API_KEY/BAILIAN_CODING_PLAN_API_KEY) so hasEnvValue(content, ...) and
the envDefaultAssistant parsing logic will recognize Qwen on subsequent runs;
ensure the same change is applied for the other occurrence mentioned (around the
second block at ~1134-1143).
Comment on lines
+78
to
+95
| function buildQwenEnv( | ||
| mergedEnv?: Record<string, string>, | ||
| extraEnv?: Record<string, string> | ||
| ): Record<string, string> { | ||
| const env: Record<string, string> = {}; | ||
| for (const source of [buildCleanSubprocessEnv(), mergedEnv ?? {}, extraEnv ?? {}]) { | ||
| for (const [key, value] of Object.entries(source)) { | ||
| if (typeof value === 'string') { | ||
| env[key] = value; | ||
| } | ||
| } | ||
| } | ||
| // Add Qwen-specific credentials that are not in the shared allowlist | ||
| if (process.env.DASHSCOPE_API_KEY) env.DASHSCOPE_API_KEY = process.env.DASHSCOPE_API_KEY; | ||
| if (process.env.BAILIAN_CODING_PLAN_API_KEY) | ||
| env.BAILIAN_CODING_PLAN_API_KEY = process.env.BAILIAN_CODING_PLAN_API_KEY; | ||
| return env; | ||
| } |
There was a problem hiding this comment.
Don't let global Qwen credentials override request-scoped env.
mergedConfig.envVars and options.env are layered on top of the sanitized subprocess env, but DASHSCOPE_API_KEY / BAILIAN_CODING_PLAN_API_KEY are reapplied afterward. That means a codebase-specific or per-request credential is silently replaced by the parent process value.
Suggested fix
function buildQwenEnv(
mergedEnv?: Record<string, string>,
extraEnv?: Record<string, string>
): Record<string, string> {
const env: Record<string, string> = {};
- for (const source of [buildCleanSubprocessEnv(), mergedEnv ?? {}, extraEnv ?? {}]) {
+ const qwenProcessEnv: Record<string, string> = {};
+ if (process.env.DASHSCOPE_API_KEY) {
+ qwenProcessEnv.DASHSCOPE_API_KEY = process.env.DASHSCOPE_API_KEY;
+ }
+ if (process.env.BAILIAN_CODING_PLAN_API_KEY) {
+ qwenProcessEnv.BAILIAN_CODING_PLAN_API_KEY = process.env.BAILIAN_CODING_PLAN_API_KEY;
+ }
+
+ for (const source of [buildCleanSubprocessEnv(), qwenProcessEnv, mergedEnv ?? {}, extraEnv ?? {}]) {
for (const [key, value] of Object.entries(source)) {
if (typeof value === 'string') {
env[key] = value;
}
}
}
- // Add Qwen-specific credentials that are not in the shared allowlist
- if (process.env.DASHSCOPE_API_KEY) env.DASHSCOPE_API_KEY = process.env.DASHSCOPE_API_KEY;
- if (process.env.BAILIAN_CODING_PLAN_API_KEY)
- env.BAILIAN_CODING_PLAN_API_KEY = process.env.BAILIAN_CODING_PLAN_API_KEY;
return env;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| function buildQwenEnv( | |
| mergedEnv?: Record<string, string>, | |
| extraEnv?: Record<string, string> | |
| ): Record<string, string> { | |
| const env: Record<string, string> = {}; | |
| for (const source of [buildCleanSubprocessEnv(), mergedEnv ?? {}, extraEnv ?? {}]) { | |
| for (const [key, value] of Object.entries(source)) { | |
| if (typeof value === 'string') { | |
| env[key] = value; | |
| } | |
| } | |
| } | |
| // Add Qwen-specific credentials that are not in the shared allowlist | |
| if (process.env.DASHSCOPE_API_KEY) env.DASHSCOPE_API_KEY = process.env.DASHSCOPE_API_KEY; | |
| if (process.env.BAILIAN_CODING_PLAN_API_KEY) | |
| env.BAILIAN_CODING_PLAN_API_KEY = process.env.BAILIAN_CODING_PLAN_API_KEY; | |
| return env; | |
| } | |
| function buildQwenEnv( | |
| mergedEnv?: Record<string, string>, | |
| extraEnv?: Record<string, string> | |
| ): Record<string, string> { | |
| const env: Record<string, string> = {}; | |
| const qwenProcessEnv: Record<string, string> = {}; | |
| if (process.env.DASHSCOPE_API_KEY) { | |
| qwenProcessEnv.DASHSCOPE_API_KEY = process.env.DASHSCOPE_API_KEY; | |
| } | |
| if (process.env.BAILIAN_CODING_PLAN_API_KEY) { | |
| qwenProcessEnv.BAILIAN_CODING_PLAN_API_KEY = process.env.BAILIAN_CODING_PLAN_API_KEY; | |
| } | |
| for (const source of [buildCleanSubprocessEnv(), qwenProcessEnv, mergedEnv ?? {}, extraEnv ?? {}]) { | |
| for (const [key, value] of Object.entries(source)) { | |
| if (typeof value === 'string') { | |
| env[key] = value; | |
| } | |
| } | |
| } | |
| return env; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/clients/qwen.ts` around lines 78 - 95, The buildQwenEnv
function currently reapplies global process.env credentials after layering
mergedEnv and extraEnv, which can overwrite request-scoped values; modify
buildQwenEnv so it only injects process.env.DASHSCOPE_API_KEY and
process.env.BAILIAN_CODING_PLAN_API_KEY when those keys are not already present
in the assembled env (i.e., check if env.DASHSCOPE_API_KEY and
env.BAILIAN_CODING_PLAN_API_KEY are undefined before assigning from process.env)
to preserve mergedConfig.envVars/options.env overrides.
Comment on lines
+272
to
+311
| const shouldStreamPartials = includePartialMessages; | ||
| let sawPartialText = false; | ||
| const partialToolCallIds = new Set<string>(); | ||
| const attemptOptions: QueryOptions = { | ||
| ...queryOptions, | ||
| resume: resumeEnabled ? resumeSessionId : undefined, | ||
| }; | ||
|
|
||
| try { | ||
| const stream = query({ prompt, options: attemptOptions }); | ||
| for await (const message of stream) { | ||
| if (options?.abortSignal?.aborted) { | ||
| throw new Error('Query aborted'); | ||
| } | ||
|
|
||
| if (isSDKPartialAssistantMessage(message)) { | ||
| if ( | ||
| message.event.type === 'content_block_start' && | ||
| message.event.content_block.type === 'tool_use' && | ||
| message.event.content_block.id | ||
| ) { | ||
| partialToolCallIds.add(message.event.content_block.id); | ||
| } else if ( | ||
| message.event.type === 'content_block_delta' && | ||
| (message.event.delta.type === 'text_delta' || message.event.delta.type === 'thinking_delta') | ||
| ) { | ||
| sawPartialText = true; | ||
| } | ||
| for (const chunk of emitPartialMessage(message)) { | ||
| yield chunk; | ||
| } | ||
| continue; | ||
| } | ||
|
|
||
| if (isSDKAssistantMessage(message)) { | ||
| for (const chunk of emitAssistantMessageBlocks( | ||
| message, | ||
| !shouldStreamPartials || !sawPartialText, | ||
| partialToolCallIds | ||
| )) { |
There was a problem hiding this comment.
Honor includePartialMessages before emitting or deduping partials.
Partial events are processed unconditionally here. When includePartialMessages is false, the client still yields incremental chunks, and the dedup bookkeeping (partialToolCallIds / sawPartialText) acts as if those partials were intentionally shown. That makes the option ineffective and will suppress final tool output once partial emission is gated.
Suggested fix
if (isSDKPartialAssistantMessage(message)) {
- if (
+ if (
+ shouldStreamPartials &&
message.event.type === 'content_block_start' &&
message.event.content_block.type === 'tool_use' &&
message.event.content_block.id
) {
partialToolCallIds.add(message.event.content_block.id);
} else if (
+ shouldStreamPartials &&
message.event.type === 'content_block_delta' &&
(message.event.delta.type === 'text_delta' || message.event.delta.type === 'thinking_delta')
) {
sawPartialText = true;
}
- for (const chunk of emitPartialMessage(message)) {
- yield chunk;
+ if (shouldStreamPartials) {
+ for (const chunk of emitPartialMessage(message)) {
+ yield chunk;
+ }
}
continue;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/clients/qwen.ts` around lines 272 - 311, The code currently
processes and emits partial events even when includePartialMessages is false;
change the isSDKPartialAssistantMessage handling so that partial events are
ignored when shouldStreamPartials is false (do not call emitPartialMessage and
do not update sawPartialText or partialToolCallIds). Concretely, wrap the
isSDKPartialAssistantMessage branch with a guard on shouldStreamPartials (or
early continue when !shouldStreamPartials) so no bookkeeping or yields occur for
partials; keep the rest of the flow (isSDKAssistantMessage and
emitAssistantMessageBlocks) unchanged and still reference sawPartialText and
partialToolCallIds only when partials were actually processed.
Comment on lines
+58
to
+64
| it('includes native Qwen auth vars when present', () => { | ||
| process.env.DASHSCOPE_API_KEY = 'dashscope-test'; | ||
| process.env.BAILIAN_CODING_PLAN_API_KEY = 'bailian-test'; | ||
| const env = buildCleanSubprocessEnv(); | ||
| expect(env.DASHSCOPE_API_KEY).toBe('dashscope-test'); | ||
| expect(env.BAILIAN_CODING_PLAN_API_KEY).toBe('bailian-test'); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify DASHSCOPE_API_KEY is not in the allowlist
rg -n 'DASHSCOPE_API_KEY' packages/core/src/utils/env-allowlist.ts
echo "---"
# Check how qwen.ts adds these keys
rg -n -A3 'DASHSCOPE_API_KEY' packages/core/src/clients/qwen.tsRepository: coleam00/Archon
Length of output: 306
🏁 Script executed:
# Get the buildCleanSubprocessEnv implementation
rg -n -A15 'function buildCleanSubprocessEnv|const buildCleanSubprocessEnv' packages/core/src/utils/env-allowlist.ts | head -40Repository: coleam00/Archon
Length of output: 342
🏁 Script executed:
# Also check what SUBPROCESS_ENV_ALLOWLIST looks like
rg -n -B5 'SUBPROCESS_ENV_ALLOWLIST' packages/core/src/utils/env-allowlist.ts | head -60Repository: coleam00/Archon
Length of output: 709
🏁 Script executed:
# Verify the complete SUBPROCESS_ENV_ALLOWLIST contents to see all allowed keys
sed -n '11,62p' packages/core/src/utils/env-allowlist.tsRepository: coleam00/Archon
Length of output: 1299
🏁 Script executed:
# Check the complete test file context around lines 58-64
sed -n '50,70p' packages/core/src/utils/env-allowlist.test.tsRepository: coleam00/Archon
Length of output: 933
🏁 Script executed:
# Check if there are any other references to these Qwen auth vars in the codebase
rg -n 'DASHSCOPE_API_KEY|BAILIAN_CODING_PLAN_API_KEY' packages/core/src --type tsRepository: coleam00/Archon
Length of output: 1038
🏁 Script executed:
# Check if there are any test cleanup mechanisms and verify the test structure
sed -n '1,100p' packages/core/src/utils/env-allowlist.test.ts | head -50Repository: coleam00/Archon
Length of output: 1749
Tests will fail: Qwen auth vars are not in the subprocess allowlist.
The SUBPROCESS_ENV_ALLOWLIST does not contain DASHSCOPE_API_KEY or BAILIAN_CODING_PLAN_API_KEY. Since buildCleanSubprocessEnv() only includes variables present in the allowlist, the test expectations at lines 62–63 will fail (both values will be undefined). Additionally, the test at line 92 expecting SUBPROCESS_ENV_ALLOWLIST.has('DASHSCOPE_API_KEY') to be true will also fail.
According to packages/core/src/clients/qwen.ts:91–93, these keys are manually added after buildCleanSubprocessEnv() returns, not from the shared allowlist.
Either:
- Add
DASHSCOPE_API_KEYandBAILIAN_CODING_PLAN_API_KEYtoSUBPROCESS_ENV_ALLOWLISTinenv-allowlist.ts, or - Remove this test and the allowlist assertion since
buildCleanSubprocessEnv()is not responsible for Qwen-specific credentials
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/utils/env-allowlist.test.ts` around lines 58 - 64, The test
fails because buildCleanSubprocessEnv() only returns vars from
SUBPROCESS_ENV_ALLOWLIST but DASHSCOPE_API_KEY and BAILIAN_CODING_PLAN_API_KEY
are not in that allowlist; to fix, add the two keys (DASHSCOPE_API_KEY and
BAILIAN_CODING_PLAN_API_KEY) to SUBPROCESS_ENV_ALLOWLIST in env-allowlist.ts so
buildCleanSubprocessEnv() will include them, or alternatively remove the
expectations and the allowlist assertion from the env-allowlist.test.ts test (or
change the test to reflect that clients/qwen.ts adds those keys separately) —
update either SUBPROCESS_ENV_ALLOWLIST or the test to keep behavior and
assertions consistent.
Comment on lines
+91
to
+93
| it('contains DASHSCOPE_API_KEY', () => { | ||
| expect(SUBPROCESS_ENV_ALLOWLIST.has('DASHSCOPE_API_KEY')).toBe(true); | ||
| }); |
There was a problem hiding this comment.
Test will fail: DASHSCOPE_API_KEY is not in SUBPROCESS_ENV_ALLOWLIST.
This assertion expects SUBPROCESS_ENV_ALLOWLIST.has('DASHSCOPE_API_KEY') to be true, but the allowlist does not include DASHSCOPE_API_KEY. The key is added separately in buildQwenEnv() in qwen.ts.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/utils/env-allowlist.test.ts` around lines 91 - 93, The test
incorrectly asserts that SUBPROCESS_ENV_ALLOWLIST.has('DASHSCOPE_API_KEY') is
true even though 'DASHSCOPE_API_KEY' is not part of SUBPROCESS_ENV_ALLOWLIST
(it’s added separately in buildQwenEnv() in qwen.ts); update the test in
env-allowlist.test.ts to expect false (or remove the assertion) so it reflects
actual behavior, or alternatively add 'DASHSCOPE_API_KEY' to the
SUBPROCESS_ENV_ALLOWLIST if you intend it to be globally allowed—refer to
SUBPROCESS_ENV_ALLOWLIST, 'DASHSCOPE_API_KEY', and buildQwenEnv() when making
the change.
Comment on lines
+2502
to
+2504
| qwen: { | ||
| model?: string; | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find and examine the server schema file
fd "config.schemas.ts" packages/serverRepository: coleam00/Archon
Length of output: 111
🏁 Script executed:
# Then examine the generated type file around the specified lines
sed -n '2490,2520p' packages/web/src/lib/api.generated.d.tsRepository: coleam00/Archon
Length of output: 917
🏁 Script executed:
# Also get broader context of SafeConfig type definition
rg "type SafeConfig" -A 30 packages/web/src/lib/api.generated.d.tsRepository: coleam00/Archon
Length of output: 41
🏁 Script executed:
# Search for SafeConfig or similar type definitions in the generated file
rg "SafeConfig|type.*Config" packages/web/src/lib/api.generated.d.ts | head -20Repository: coleam00/Archon
Length of output: 127
🏁 Script executed:
# Look at the actual server schema to see how assistants.qwen is declared
cat -n packages/server/src/routes/schemas/config.schemas.tsRepository: coleam00/Archon
Length of output: 3626
Generated SafeConfig type for qwen doesn't match server schema.
The server schema declares qwen as .optional().nullable() (lines 18–23 of packages/server/src/routes/schemas/config.schemas.ts), but the generated type here requires it as a non-null object. This causes the web client to assume config.assistants.qwen always exists, even for older configs that omit it. Regenerate the OpenAPI types so qwen becomes qwen?: { model?: string } | null;.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/web/src/lib/api.generated.d.ts` around lines 2502 - 2504, The
generated SafeConfig type incorrectly defines the field qwen as a non-null
object; update the generation so SafeConfig has qwen?: { model?: string } | null
to match the server schema (.optional().nullable()) — regenerate the
OpenAPI/TypeScript client (the generator that produces
packages/web/src/lib/api.generated.d.ts) from the server schema in
config.schemas.ts, ensuring the SafeConfig type and the qwen property are
emitted as optional and nullable; verify the produced declaration contains
"qwen?: { model?: string } | null" and commit the updated generated file.
Comment on lines
+14
to
+26
| export function isQwenModel(model: string): boolean { | ||
| const normalized = model.trim().toLowerCase(); | ||
| return ( | ||
| normalized.startsWith('qwen') || | ||
| normalized.startsWith('qwq') || | ||
| normalized.startsWith('qvq') || | ||
| normalized.endsWith('-coder-model') || | ||
| normalized === 'coder-model' || | ||
| normalized.includes('qwen-coder') || | ||
| normalized.includes('qwen-max') || | ||
| normalized.includes('qwen-turbo') || | ||
| normalized.includes('qwen-plus') | ||
| ); |
There was a problem hiding this comment.
Tighten Qwen detection before it starts rerouting unknown models.
This matcher is broad enough to classify arbitrary IDs like qwenish-test or mycompany-coder-model as Qwen, and inferProviderFromModel() will then override the configured provider. Please restrict inference to documented Qwen patterns (qwen-, qwq-, qvq-, plus any explicit aliases you truly support) and add a negative regression test for the generic coder-model cases.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/workflows/src/model-validation.ts` around lines 14 - 26, isQwenModel
is too permissive and misclassifies IDs like "qwenish-test" or
"mycompany-coder-model"; change the matcher to only accept documented Qwen
prefixes and explicit aliases: require prefix matches like "qwen-", "qwq-",
"qvq-" (note the trailing dash) and allow only explicit whole-token aliases you
truly support such as "qwen-max", "qwen-turbo", "qwen-plus" (avoid using
endsWith('-coder-model'), equality to "coder-model", or generic includes).
Update isQwenModel to use strict prefix checks and exact alias checks, and add
negative regression tests asserting that values like "qwenish-test" and
"mycompany-coder-model" do NOT match while valid patterns (e.g., "qwen-max",
"qwen-123") DO.
Comment on lines
+248
to
250
| function resolveProvider(node: DagNode, workflowProvider?: AssistantProvider): AssistantProvider { | ||
| if ('provider' in node && node.provider) return node.provider; | ||
| return workflowProvider ?? 'claude'; |
There was a problem hiding this comment.
Infer the provider from node.model here as well.
Validation currently treats model: qwen-max as the workflow/default provider unless node.provider is set, so non-Claude capability warnings can be skipped even though execution will route that node to Qwen. Reuse inferProviderFromModel(node.model) before falling back to workflowProvider.
Based on learnings: "Workflow-level model configuration (in YAML) overrides config file defaults, which override SDK defaults; validate workflows at load time for provider/model compatibility"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/workflows/src/validator.ts` around lines 248 - 250, The
resolveProvider function currently only checks node.provider and
workflowProvider, so nodes with node.model (e.g., model: qwen-max) are
incorrectly validated as the workflow/default provider; update resolveProvider
to call inferProviderFromModel(node.model) after checking node.provider and
before falling back to workflowProvider so the inferred provider (from
node.model) is returned when present; reference the resolveProvider function,
the node.model property, and the inferProviderFromModel helper when making this
change.
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #1050 — adds test coverage, documentation, and enhanced model recognition for Qwen provider. Should be reviewed after #1050.
Summary by CodeRabbit
New Features
Documentation