feat(native-server): implements minimal QwenEngine

[oshliaer]

This is a basic implementation (working implementation), but with limitations:

What works:

• ✅ Running qwen -y -p via child_process
• ✅ Parsing output (tool calls, messages)
• ✅ Sending events to UI (status, message)
• ✅ Cancellation support (abort signal)
• ✅ Model selection via -m flag

What is NOT implemented (unlike ClaudeEngine/CodexEngine):

• ❌ No session support (Qwen CLI doesn't save session IDs)
• ❌ No MCP integration via settings (only via ~/.qwen/settings.json)
• ❌ No token/usage statistics parsing
• ❌ No attachments support (images/files)
• ❌ No real-time streaming (parsing post-factum)

Status: This is an MVP implementation -- sufficient for testing, but needs further work for production use.

[Copilot]

Pull request overview

This PR introduces a minimal working implementation of QwenEngine, integrating the Qwen Code CLI as a new agent engine option alongside the existing ClaudeEngine and CodexEngine. The implementation provides basic functionality for running Qwen queries in non-interactive mode using qwen -y -p "<instruction>".

Changes:

• Adds QwenEngine class that spawns and manages the Qwen CLI process with basic output parsing
• Registers QwenEngine in the server's engine list
• Updates UI placeholder text from "Ask Claude" to "Ask AI" for engine-agnostic messaging

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 13 comments.

File	Description
app/native-server/src/agent/engines/qwen.ts	New engine implementation with CLI process spawning, stdout/stderr parsing, signal handling, and event emission
app/native-server/src/server/index.ts	Adds QwenEngine to the engines array in AgentChatService initialization
app/chrome-extension/entrypoints/sidepanel/components/AgentChat.vue	Updates input placeholder from engine-specific "Ask Claude" to generic "Ask AI"

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

All stderr output is emitted as status events with status: 'running'. This could be problematic because stderr often contains actual error messages, warnings, or diagnostic information that should be handled differently. Error messages would be presented to the user as "running" status, which is confusing. Consider differentiating between informational stderr output and actual errors, or only emit stderr as status for informational messages while logging errors separately.

[Copilot]

On successful completion, the entire stdoutBuffer is emitted as a single final message. This means the user would see all the accumulated output at once, including previously emitted line-by-line messages on lines 88-100. This creates duplicate content in the chat UI. Consider either: (1) only emitting the full buffer as the final message and removing the line-by-line emission, or (2) tracking what has already been emitted and only include new content in the final message.

Suggested change

	// Emit final output as assistant message
	if (stdoutBuffer.trim()) {
	const message: AgentMessage = {
	id: randomUUID(),
	sessionId: options.sessionId,
	role: 'assistant',
	content: stdoutBuffer,
	messageType: 'chat',
	isFinal: true,
	createdAt: new Date().toISOString(),
	};
	ctx.emit({
	type: 'message',
	data: message,
	});
	}

[Copilot]

The implementation creates an internal AbortController but passes the external signal directly to the spawn() call. This differs from how other engines handle signals. Both ClaudeEngine and CodexEngine use signal.addEventListener('abort', ...) to manually handle cancellation, without passing the signal to the spawn options. The current approach may cause the internal abortController to become out of sync with the process state, as the process might be terminated by Node.js before the internal controller is notified. Consider adopting the pattern used in CodexEngine where the signal is only listened to via addEventListener, not passed to spawn.

Suggested change

	signal: signal as any,
	});
	});
	const handleAbort = () => {
	if (!qwen.killed) {
	qwen.kill('SIGTERM');
	}
	};
	// Wire engine-level cancellation (this.abortController) to the spawned process
	const internalSignal = this.abortController?.signal;
	internalSignal?.addEventListener('abort', handleAbort);
	// Wire external cancellation (options.signal) to the spawned process
	const externalSignal = options.signal;
	const externalAbortHandler =
	externalSignal && !externalSignal.aborted ? handleAbort : null;
	if (externalAbortHandler && externalSignal) {
	externalSignal.addEventListener('abort', externalAbortHandler);
	} else if (externalSignal?.aborted) {
	// If already aborted, ensure the process is terminated promptly
	handleAbort();
	}
	// Ensure we clean up abort listeners once the process exits
	qwen.once('close', () => {
	internalSignal?.removeEventListener('abort', handleAbort);
	if (externalAbortHandler && externalSignal) {
	externalSignal.removeEventListener('abort', externalAbortHandler);
	}
	});

[Copilot]

The signal handling logic adds an event listener but there's a potential issue: if the external options.signal is already aborted (checked on line 24), the listener registered on line 204 will never fire because spawn() with the signal will fail immediately. However, there's still a race condition - the signal could be aborted between line 24 and line 50. In this case, the internal abortController would never be notified. Consider removing the signal from spawn options and relying solely on the addEventListener pattern, or ensure the internal abortController is aborted when the signal is already aborted.

[Copilot]

The QwenEngine class does not define the supportsMcp property. According to the AgentEngine interface, this is an optional property, but it should be explicitly set to false to indicate that this engine does not support MCP natively (as documented in the PR description). This is consistent with CodexEngine which explicitly sets supportsMcp = false, while ClaudeEngine sets it to true.

Suggested change

	readonly name: EngineName = 'qwen';
	readonly name: EngineName = 'qwen';
	readonly supportsMcp = false;

[Copilot]

The QwenEngine extends EventEmitter but never emits any events through the EventEmitter interface. All events are emitted through the ctx.emit() method from EngineExecutionContext. This inheritance appears unnecessary and could be removed to simplify the class structure. Other engines like ClaudeEngine and CodexEngine do not extend EventEmitter.

[Copilot]

The variable stdoutBuffer is initialized twice - once on line 56 and again on line 61. The first initialization is redundant and should be removed.

Suggested change

stdoutBuffer = '';

[Copilot]

The cancel() method only calls this.abortController?.abort() but the internal abortController is not connected to the external signal from options. The pattern used here differs from other engines: ClaudeEngine and CodexEngine don't implement a cancel() method because they rely on the external signal passed via options. Since the AbortController interface is not part of the AgentEngine interface, this method won't be called by the chat service. The chat service uses the AbortController it creates and passes via options.signal. This cancel() method is effectively dead code.

[Copilot]

The instruction is directly passed to the command-line via the -p flag without proper escaping or validation beyond trimming. While spawn() with an array of arguments is safer than shell execution, the qwen CLI itself might interpret special characters or sequences in unexpected ways. Consider adding input validation to detect and reject potentially problematic characters, or documenting the security assumptions about the qwen CLI's input handling.

[Copilot]

The output parsing approach is fragile and may capture noise. The condition on line 87 (trimmedLine.startsWith('Qwen') || trimmedLine.includes('Thinking')) will match any line containing the word "Thinking" anywhere, which could result in false positives. For example, a file path or error message containing "Thinking" would be emitted as an assistant message. Consider making the pattern matching more specific, such as checking for exact prefixes or using more structured patterns.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 8 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The final assistant message also omits requestId/cliSource (and will likely duplicate content already emitted from per-line parsing). Consider emitting either incremental messages with isStreaming or a single final message, and include the standard correlation fields (requestId, cliSource) to match the other engines' message shape.

Suggested change

	createdAt: new Date().toISOString(),
	createdAt: new Date().toISOString(),
	requestId: options.requestId,
	cliSource: 'qwen',

[Copilot]

The Promise can settle multiple times: qwen.on('error', ...) calls reject, but qwen.on('close', ...) will still run and may resolve/reject again, and you may emit multiple status events. Consider adding a settled flag (like CodexEngine does) and performing one centralized cleanup/settlement path.

[Copilot]

There are unused imports here (join from node:path and RealtimeEvent), and the EventEmitter base class doesn't appear to be used. Consider removing the unused imports/inheritance to keep the engine minimal and avoid confusion about an event-emitter API that isn't actually exposed.

[Copilot]

Creating repoPath with mkdir(..., { recursive: true }) can mask configuration errors (e.g., a mistyped/nonexistent project root) by silently creating an empty directory and running qwen there. Other engines assume the project root exists. Consider validating the directory exists (and erroring with a clear message) instead of creating it.

[Copilot]

Cancellation is wired to a new internal AbortController, but AgentChatService passes cancellation via options.signal. As a result, UI-driven cancellation (abortController.abort() in chat-service) won't terminate the spawned qwen process. Use options.signal for spawn/abort handling (or forward options.signal to the internal controller) instead of creating a separate controller here.

Suggested change

	this.abortController = new AbortController();
	const { signal } = this.abortController;
	let signal: AbortSignal;
	if (options.signal) {
	// Use the caller-provided signal so UI-driven cancellation propagates to the spawned process.
	const externalSignal = options.signal;
	signal = externalSignal;
	// Maintain an internal controller for the engine's cancel() API while mirroring external aborts.
	this.abortController = new AbortController();
	const internalController = this.abortController;
	const onAbort = () => {
	internalController.abort(externalSignal.reason);
	externalSignal.removeEventListener('abort', onAbort);
	};
	if (externalSignal.aborted) {
	onAbort();
	} else {
	externalSignal.addEventListener('abort', onAbort);
	}
	} else {
	this.abortController = new AbortController();
	signal = this.abortController.signal;
	}

[Copilot]

This engine emits its own 'completed' status event, but AgentChatService already emits 'completed' after initializeAndRun resolves. This will cause duplicate completion events in the UI/stream. Consider removing this status emission and letting the chat-service handle lifecycle statuses consistently across engines.

Suggested change

	// Emit completion status
	ctx.emit({
	type: 'status',
	data: {
	sessionId: options.sessionId,
	status: 'completed',
	requestId: options.requestId,
	},
	});

[Copilot]

Similarly to 'completed', this engine emits 'error'/'cancelled' status (and also emits a top-level 'error' event on spawn failure) while AgentChatService already normalizes errors/cancellation into stream events. This can lead to duplicated/conflicting error/cancel events. Prefer throwing/rejecting and letting AgentChatService publish the canonical error/status events (keep only tool/progress messages if needed).

[Copilot]

Engine-emitted assistant messages here omit fields that other engines include (requestId and cliSource). AgentChatService persists events using these fields, and the UI may rely on them for correlation/filtering. Consider setting cliSource: this.name and requestId: options.requestId on all emitted AgentMessage objects.