feat(agents): add Qwen Coder (qwen3-coder) agent (#803)#1128
feat(agents): add Qwen Coder (qwen3-coder) agent (#803)#1128jSydorowicz21 wants to merge 5 commits into
Conversation
Promotes the hidden qwen3-coder stub to a real, selectable BETA agent mirroring gemini-cli: stream-json output, --resume, and model-arg selection. Adds a registered QwenOutputParser, Qwen error patterns, NewInstanceModal entry, capabilities, context window, and README. Covered by parser + definition + capabilities + metadata tests; agent-completeness passes.
Greptile SummaryThis PR adds Qwen3 Coder as a selectable beta agent. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (3): Last reviewed commit: "fix(agents): qwen context window + error..." | Re-trigger Greptile |
|
Caution Review failedAn error occurred during the review process. Please try again later. ✨ 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 |
📝 WalkthroughWalkthroughThis PR adds Qwen3 Coder support across documentation, agent metadata, capability and CLI wiring, parser registration, error classification, and tests. It also marks ChangesQwen3 Coder support
🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main/agents/capabilities.ts`:
- Around line 181-182: The qwen capability currently advertises image support
even though there is no image-passing path configured, so attached images will
be silently ignored. Update the `capabilities` entry for the `qwen` agent in
`supportsImageInput` to match the actual implementation state, and keep it
disabled until `imageArgs` or an `imagePromptBuilder` is wired through the
spawner flow used by `ChildProcessSpawner`.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 7d861c79-0a12-4ca9-bdc8-3472f9fe6f1f
📒 Files selected for processing (14)
README.mdsrc/__tests__/main/agents/capabilities.test.tssrc/__tests__/main/agents/definitions.test.tssrc/__tests__/main/parsers/qwen-output-parser.test.tssrc/__tests__/shared/agentMetadata.test.tssrc/main/agents/capabilities.tssrc/main/agents/definitions.tssrc/main/parsers/error-patterns.tssrc/main/parsers/index.tssrc/main/parsers/parser-factory.tssrc/main/parsers/qwen-output-parser.tssrc/renderer/components/NewInstanceModal/types.tssrc/shared/agentConstants.tssrc/shared/agentMetadata.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 528706e284
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| supportsReadOnlyMode: false, // Prompt-only enforcement via -y | ||
| supportsJsonOutput: true, | ||
| supportsSessionId: true, | ||
| supportsImageInput: true, // Qwen is multimodal; imageArgs unset for beta |
There was a problem hiding this comment.
Disable Qwen image input until it is wired
supportsImageInput makes the renderer enable attachments (App.tsx and InputArea.tsx gate on this capability), but the Qwen definition leaves imageArgs and imagePromptBuilder undefined and also sets supportsStreamJsonInput: false. In ChildProcessSpawner.spawn, images are only serialized through one of those paths, so a first Qwen message with screenshots reaches the CLI as only -p <prompt> and the images are silently dropped. Until Qwen image delivery is implemented, this capability should stay false.
Useful? React with 👍 / 👎.
| supportsJsonOutput: false, | ||
| supportsSessionId: false, | ||
| supportsImageInput: false, | ||
| supportsResume: false, // Defer until --resume verified in headless mode |
There was a problem hiding this comment.
Mark Qwen as resumable when adding resume args
Qwen headless docs list --resume [sessionId] as resuming a specific session, and this commit already adds resumeArgs for Qwen, but the capability remains false. Anything that gates continuity on supportsResume will skip Qwen; for example src/cli/services/goal-runner.ts:318 only asks the current session for a handoff when this capability is true, so Qwen goal runs lose resumable context despite returning session_id.
Useful? React with 👍 / 👎.
| // Qwen Code is a Gemini CLI fork; read-only behavior is enforced via system prompt | ||
| // instructions. The -y flag is still needed for non-interactive execution to prevent | ||
| // approval prompts from hanging batch mode. | ||
| readOnlyArgs: ['-y'], |
There was a problem hiding this comment.
Use Qwen plan mode for read-only runs
When a Qwen tab is marked read-only through keyboard/quick actions or queued as read-only, buildAgentArgs appends these readOnlyArgs, but -y is Qwen's yolo mode. I checked the Qwen CLI docs: --yolo, -y auto-approves all actions, while --plan or --approval-mode plan is the mode that generates plans without executing. This means a read-only Qwen send can still auto-approve write/edit tools instead of preventing them.
Useful? React with 👍 / 👎.
| supportsGroupChatModeration: false, | ||
| usesJsonLineOutput: true, | ||
| usesCombinedContextWindow: false, | ||
| supportsAppendSystemPrompt: false, |
There was a problem hiding this comment.
Use Qwen's native system-prompt flag
Qwen headless mode supports --append-system-prompt, but leaving this capability false sends Maestro through the fallback in process.ts: the system prompt is embedded as user text on the first turn and then skipped on resumed turns. For Qwen conversations that depend on Maestro's system instructions, resumed sends can therefore run without those instructions even though the CLI has a per-run native flag for them.
Useful? React with 👍 / 👎.
| export class QwenOutputParser extends ClaudeOutputParser { | ||
| readonly agentId: ToolType = 'qwen3-coder'; |
There was a problem hiding this comment.
Treat Qwen error results as failures
Qwen JSON output marks failed terminal results with is_error: true, but this subclass inherits Claude's result handling, which treats every type: 'result' as a successful assistant result and does not inspect is_error. When Qwen emits a failure result on stdout, Maestro can render the failure payload as a normal response before exit handling, and CLI callers that only see a parsed result can miss the structured failure state.
Useful? React with 👍 / 👎.
…803) CI full suite caught that parsers/index.test.ts hardcodes the registered-parser count, which the new QwenOutputParser bumped 6 -> 7. Update the count assertions + title and add a 'should register Qwen parser' test mirroring the Pi one.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9ad50aef15
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| export class QwenOutputParser extends ClaudeOutputParser { | ||
| readonly agentId: ToolType = 'qwen3-coder'; |
There was a problem hiding this comment.
Preserve Qwen's 256K context window
Because this inherits ClaudeOutputParser.extractUsageFromRaw, Qwen result events that contain only top-level usage get aggregateModelUsage's Claude fallback contextWindow of 200000. StdoutHandler.buildUsageStats then prefers that parser-supplied value over the spawned process's configured 262144 window, so any Qwen run that reports usage drives the context meter and summarization thresholds from the wrong limit. Override Qwen usage parsing to omit the fallback context window or supply Qwen's configured/default window instead.
Useful? React with 👍 / 👎.
Review fixes: set supportsImageInput=false (no image args wired, attachments would be dropped); set supportsResume=true to match the wired --resume args; honor Qwen's is_error:true in QwenOutputParser (reclassify failed results to error events + emit structured AgentError); add a session_not_found error pattern for expired --resume sessions. Deferred plan-mode read-only (matches gemini convention) and native --append-system-prompt (Windows file-variant unverified). Tests updated.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 720bac9a0a
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (event && event.type === 'result' && this.isFailedResult(parsed)) { | ||
| // Reclassify a failed terminal result so downstream handlers (and | ||
| // isResultMessage) treat it as an error rather than a successful response. | ||
| return { ...event, type: 'error' }; |
There was a problem hiding this comment.
Populate Qwen error text from error.message
The Qwen SDK docs for its stream-json protocol show failed results using message.get("error") and error.get("message", "Unknown error"), but this return preserves the Claude parser's text, which only reads result or message.content. When Qwen emits an is_error result with only error.message, CLI batch handling only records errors with event.text, so these failures can lose the provider message and may fall back to a generic exit/code error instead of the actionable Qwen error.
Useful? React with 👍 / 👎.
| workingDirArgs: (dir: string) => ['--include-directories', dir], | ||
| imageArgs: undefined, | ||
| modelArgs: (modelId: string) => ['-m', modelId], | ||
| promptArgs: (prompt: string) => ['-p', prompt], |
There was a problem hiding this comment.
Preserve Qwen's prompt flag for stdin spawns
On Windows text sends and desktop SSH sends, Maestro transports the prompt over stdin, and ChildProcessSpawner / buildSshCommandWithStdin then skip promptArgs, so this definition invokes qwen -y --output-format stream-json without -p / --prompt. I checked the Qwen CLI docs: stdin headless usage still keeps the prompt flag (echo ... | qwen --prompt), while omitting it starts the interactive CLI, so these sends can hang or exit without a parsed stream-json result instead of running the user's prompt.
Useful? React with 👍 / 👎.
Round-2 review fixes: strip the inherited Claude 200000 fallback contextWindow from Qwen usage so the spawned process's configured 262144 window drives the context meter/summarization (genuinely larger reported windows are preserved); populate is_error result text from Qwen's error.message so failed runs surface the actionable provider message instead of an empty/generic error. Declined the stdin prompt-flag finding: per qwen-code source, piped child spawns are non-interactive (stdin.isTTY=false) and read stdin as the prompt, so adding -p would duplicate it and forcing it as an arg reintroduces the cmd.exe arg-length limit the stdin path avoids. +4 tests.
|
@greptile re-review please |
1 similar comment
|
@greptile re-review please |
…pdown qwen-code's -m/--model is declared type:'string' with no choices enum and is an unconstrained pass-through for all non-OAuth auth types (API-key/third-party). It's multi-provider (OpenAI/Anthropic/Gemini/Vertex/OpenRouter/vLLM/Ollama). The hardcoded 5-item select omitted the actual default (coder-model) and the entire multi-provider space; switched to type:'text' like omp/codex/opencode. Test updated.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/__tests__/main/agents/definitions.test.ts (1)
367-375: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winActually assert the trimming behavior here.
This test title says the
argBuildertrims, but it never checks a padded non-empty value. A regression fromvalue.trim()tovaluewould still pass. Add one assertion for something like' qwen3-coder-plus '→['-m', 'qwen3-coder-plus'].Suggested diff
expect(modelOption?.default).toBe(''); expect(modelOption?.argBuilder?.('qwen3-coder-plus')).toEqual(['-m', 'qwen3-coder-plus']); + expect(modelOption?.argBuilder?.(' qwen3-coder-plus ')).toEqual([ + '-m', + 'qwen3-coder-plus', + ]); expect(modelOption?.argBuilder?.('')).toEqual([]); expect(modelOption?.argBuilder?.(' ')).toEqual([]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/__tests__/main/agents/definitions.test.ts` around lines 367 - 375, The test for qwen3-coder’s model config argBuilder does not actually verify trimming behavior, so a regression using the raw value would still pass. Update the assertions in definitions.test.ts around getAgentDefinition('qwen3-coder') and the modelOption.argBuilder checks to include a padded non-empty input and expect the trimmed ['-m', 'qwen3-coder-plus'] result, alongside the existing empty/blank cases.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/__tests__/main/agents/definitions.test.ts`:
- Around line 367-375: The test for qwen3-coder’s model config argBuilder does
not actually verify trimming behavior, so a regression using the raw value would
still pass. Update the assertions in definitions.test.ts around
getAgentDefinition('qwen3-coder') and the modelOption.argBuilder checks to
include a padded non-empty input and expect the trimmed ['-m',
'qwen3-coder-plus'] result, alongside the existing empty/blank cases.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b00e905c-635b-43ca-b95d-8b015493340c
📒 Files selected for processing (2)
src/__tests__/main/agents/definitions.test.tssrc/main/agents/definitions.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/agents/definitions.ts
|
Superseded by #1138, which bundles Qwen Coder (qwen3-coder) and Oh My Pi (omp) into a single PR. The two agents edit the same shared registry files (parser registry, agent metadata/constants, error patterns, supported-agents list, parser-count test), so they cannot merge independently without conflicts; #1138 resolves that union once. Closing in favor of #1138. |
Summary
Closes #803. Promotes the hidden, non-functional
qwen3-coderstub to a real, selectable BETA agent, mirroring thegemini-cliintegration.Changes
definitions.ts: full definition (binaryqwen,-p/stream-json output,--resume, model-arg selection via SELECT, context window).capabilities.ts: real capability flags (json/stream/model-selection/session-id/usage/etc).QwenOutputParserregistered inparser-factory.tsplusparsers/index.ts;QWEN_ERROR_PATTERNSinerror-patterns.ts.agentConstantscontext window (256K),BETA_AGENTSbadge,NewInstanceModalSUPPORTED_AGENTS entry, README.Verification
npm run lint(tsc x3): cleanvitest runparser + definitions + capabilities + metadata + agent-completeness: 140 tests passeslint+prettier: cleanSummary by CodeRabbit
New Features
Bug Fixes
Tests