fix(cli): MCP add/remove now correctly persists headers and server deletions

[B-A-M-N]

Summary

Fixes #3718

Two bugs in qwen mcp add and qwen mcp remove:

1. mcp remove doesn't persist deletions when multiple MCP servers are configured — removed servers reappear because applyUpdates() only merges/adds keys, never deletes them
2. mcp add cannot add/update headers properly — the headers property was always included (even when undefined), causing stale headers to persist

Changes

• settings.ts: Added setValueFullSave() method that persists using the full originalSettings instead of a minimal merge update, ensuring removed keys don't survive via applyUpdates merge semantics
• remove.ts: Uses non-mutating destructuring + setValueFullSave so deleted servers are properly removed from disk
• add.ts: Uses setValueFullSave + conditional spread (...(headers && { headers })) so headers aren't carried forward when not specified
• Tests: Updated mocks for setValueFullSave, added coverage for multi-server removal and header replacement

Verification

All 135 tests pass (add: 27, remove: 5, settings: 95, commentJson: 8). TypeScript typecheck clean for packages/cli.

[wenshao]

[Critical] setValueFullSave 无法真正从磁盘删除 keys — 核心修复不完整

saveSettings(settingsFile) 内部调用 updateSettingsFilePreservingFormat → applyUpdates(parsed, updates)。applyUpdates 是纯合并操作：只遍历 updates 中的 keys，不会删除存在于磁盘但不在 updates 中的 keys。

场景: 用户有 MCP 服务器 A、B、C。remove B 后 remainingServers = {A, C}。磁盘文件仍包含 mcpServers: {A, B, C}。applyUpdates({A,B,C}, {A,C}) 遍历 A 和 C，但 B 从未被触及 — 保留在磁盘上。重启后 B 重新出现。

仅移除最后一个服务器时能正常工作（Object.keys({}).length > 0 为 false 走直接赋值分支），这也正是现有测试覆盖的场景。新增测试 mock 了 setValueFullSave 为空操作，未验证实际持久化。

Suggested change

	* the on-disk file exactly matches the in-memory state for all keys.
	setValueFullSave(scope: SettingScope, key: string, value: unknown): void {
	const settingsFile = this.forScope(scope);
	setNestedPropertySafe(settingsFile.settings, key, value);
	setNestedPropertySafe(settingsFile.originalSettings, key, value);
	this._merged = this.computeMergedSettings();
	// Bypass applyUpdates merge — write originalSettings directly as
	// the full file content to ensure removed keys are actually deleted.
	const dirPath = path.dirname(settingsFile.path);
	if (!fs.existsSync(dirPath)) {
	fs.mkdirSync(dirPath, { recursive: true });
	}
	updateSettingsFilePreservingFormat(
	settingsFile.path,
	// Must deep-clone; commentJson.stringify mutates its input
	JSON.parse(JSON.stringify(settingsFile.originalSettings)),
	);
	}

— deepseek-v4-pro via Qwen Code /review

[wenshao]

[Critical] isModelUnloadedError 匹配范围过宽

两个问题：

1. 'model not found' 对绝大多数模型服务商是永久性配置错误（模型名拼写错误或模型不存在），而非 JIT loading 的 "模型存在但未加载"。每次误判浪费一次 API 调用并掩盖真正的配置错误。
2. 'is not loaded' 无上下文限制 — 会匹配 "API key is not loaded"、"plugin is not loaded" 等不相关错误。

Suggested change

	* such errors so the pipeline can retry the request.
	private isModelUnloadedError(error: unknown): boolean {
	if (!error) return false;
	const errorMessage =
	error instanceof Error
	? error.message.toLowerCase()
	: String(error).toLowerCase();
	// Only match known JIT-loading error patterns from local model servers
	// (LM Studio, llama.cpp). Avoid matching permanent errors like
	// "model not found" which indicate misconfiguration, not a transient
	// unloaded state.
	return (
	errorMessage.includes('model is unloaded') ||
	errorMessage.includes('model not loaded') ||
	errorMessage.includes('model unloaded')
	);
	}

— deepseek-v4-pro via Qwen Code /review

[wenshao]

[Critical] Pipeline 重试完全静默 — 无可观测性

当 isModelUnloadedError 返回 true 并触发重试时，没有任何日志记录。重试成功则完全不可见，重试失败则丢失了原始错误上下文。凌晨 3 点排查间歇性 "模型未加载" 故障时无法判断：(a) 是否发生了重试，(b) 重试的原因，(c) 原始错误内容。

Suggested change

	// Retry once for model-unloaded errors.
	if (this.isModelUnloadedError(error)) {
	debugLogger.warn(
	'Retrying request after model-unloaded error:',
	error instanceof Error ? error.message : String(error),
	);
	try {
	const openaiRequest = await this.buildRequest(
	request,
	userPromptId,
	context,
	isStreaming,
	);
	const result = await executor(openaiRequest, context);
	debugLogger.info('Retry succeeded after model-unloaded error');
	return result;
	} catch (retryError) {
	debugLogger.warn(
	'Retry failed after model-unloaded error:',
	retryError instanceof Error ? retryError.message : String(retryError),
	);
	return await this.handleError(retryError, context, request);
	}
	}

— deepseek-v4-pro via Qwen Code /review

[wenshao]

[Suggestion] /directory remove 路径解析不一致

workspaceContext.removeDirectory(directory) 使用原始用户输入（未经过 expandHomeDir），而 canonicalDirectory（第 350 行）已正确展开 ~ 并解析 realpathSync。

后果：

1. ~/my-project 无法被移除（removeDirectory 内部将 ~ 作为字面目录名处理）
2. 被删除目录的规范路径与 scope 搜索路径可能不匹配，导致目录从内存移除但持久化条目保留

Suggested change

	}
	const removed = workspaceContext.removeDirectory(expandedDir);

— deepseek-v4-pro via Qwen Code /review

[wenshao]

[Suggestion] Memory 刷新逻辑在 /directory add 和 /directory remove 中完全重复

loadServerHierarchicalMemory 调用及后续 config.setUserMemory、config.setGeminiMdFileCount、config.setConditionalRulesRegistry、context.ui.setGeminiMdFileCount 在两个子命令中几乎完全相同（add 约 230-260 行，remove 约 386-405 行）。建议提取为共享辅助函数，避免未来维护中出现偏差。

— deepseek-v4-pro via Qwen Code /review

[wenshao]

[Suggestion] /directory remove 缺少限制性沙箱检查

/directory add 子命令会检查 config.isRestrictiveSandbox() 并在为 true 时返回错误。新的 remove 子命令缺少此检查，允许在限制性沙箱会话中修改工作目录，与 add 的安全策略不一致。

Suggested change

	return t('Remove a directory from the workspace');
	action: async (context: CommandContext, args: string) => {
	const {
	ui: { addItem },
	services: { config, settings },
	} = context;
	if (!config) {
	addItem(
	{
	type: MessageType.ERROR,
	text: t('Configuration is not available.'),
	},
	Date.now(),
	);
	return;
	}
	if (config.isRestrictiveSandbox()) {
	return {
	type: 'message' as const,
	messageType: 'error' as const,
	content: t(
	'The /directory remove command is not supported in restrictive sandbox profiles.',
	),
	};
	}

— deepseek-v4-pro via Qwen Code /review

[wenshao]

[Suggestion] 模型卸载重试无延迟

当 isModelUnloadedError 返回 true 时，重试立即触发。本地模型服务器进行 JIT 加载可能需要数秒时间，立即重试可能同样失败，导致重试机制在实践中失效。

Suggested change

	// Local model servers like LM Studio support Just-In-Time (JIT) model
	if (this.isModelUnloadedError(error)) {
	// Give the model server a moment to complete JIT loading
	await new Promise((resolve) => setTimeout(resolve, 2000));
	try {

— deepseek-v4-pro via Qwen Code /review

[wenshao]

[Critical] The retry wrapper only catches failures that happen while creating/returning the stream. executeStream() returns the async generator from processStreamWithLogging(), so errors raised later while the caller iterates it (for example an error_finish chunk containing Model is unloaded) happen after this try/catch has already returned and will never reach the new retry branch. As a result, streaming providers can still fail on the common SSE/content-error form even though non-streaming requests retry.

Please wrap the returned async generator and catch iteration-time StreamContentError for the first attempt, or move the model-unloaded retry into the stream consumption path so the retry is triggered when the stream emits the unloaded-model error.

— gpt-5.5 via Qwen Code /review

[wenshao]

[Suggestion] This persistence lookup still compares the canonical realpath against the raw strings in originalSettings.context.includeDirectories. If the user originally saved the directory as $HOME/project, ~/project, or a symlink spelling, the live workspace removal succeeds because WorkspaceContext canonicalizes paths, but this loop finds no matching raw entry and leaves the setting on disk; the directory then reappears on restart. The same can happen when the same canonical directory is present in multiple scopes, because only the first exact match is updated.

Please resolve each persisted raw entry using the same expansion/realpath fallback before comparing, remove the matching raw entry from every relevant scope, and report a warning/error if the in-memory removal succeeded but no persisted entry was updated.

— gpt-5.5 via Qwen Code /review

[wenshao]

[Suggestion] Writing the whole settings file with JSON.stringify bypasses the existing comment-json preserving update path. A normal qwen mcp add/remove will now rewrite the user's entire settings file as plain JSON, dropping comments and hand formatting outside mcpServers.

Please preserve the existing file format by adding a helper that replaces/deletes just the target subtree (for example mcpServers) using the comment-json representation, rather than serializing originalSettings for the whole file.

— gpt-5.5 via Qwen Code /review

[wenshao]

⚠️ CI status: Lint failing.

[Critical] processStreamWithLogging 过早调用 handleError——成功重试仍产生错误遥测（pipeline.ts:228，未修改代码，无 diff 行可关联）

当 SSE 流迭代中发生 model-unloaded 错误时，调用链为：

1. processStreamWithLogging catch → await this.handleError(error, ...) → 运行错误处理副作用（日志、遥测）→ throw
2. wrapStreamWithRetry catch → isModelUnloadedError → true → 重试成功
3. 用户获得正确响应，但监控系统已记录一次失败

与非流式路径对比：executeWithErrorHandling 先检查 isModelUnloadedError，仅当重试失败或不可重试时才调用 handleError。建议 processStreamWithLogging 不自行调用 handleError，或将 isModelUnloadedError 检查前置到其 catch 块中。

— DeepSeek/deepseek-v4-pro via Qwen Code /review

[wenshao]

[Critical] setValueFullSave 回归：updateSettingsFilePreservingFormat → applyUpdates 无法删除 keys

上一个提交使用 JSON.stringify(originalSettings) + writeWithBackupSync 直接覆写完整文件，正确实现了删除。此提交改为调用 updateSettingsFilePreservingFormat，其内部（commentJson.ts:46）调用 applyUpdates(parsed, updates) —— 纯合并操作，只遍历 updates 中的 key，永远不会删除 parsed 中存在但 updates 中不存在的 key。

当用户执行 qwen mcp remove serverB：

• 内存中 originalSettings.mcpServers = { serverA }（正确）
• 磁盘仍为 mcpServers: { serverA, serverB }
• applyUpdates 只遍历 serverA，serverB 永远不被触及
• 写回磁盘——serverB 复活，重启后重新出现

方法注释写的是 "Bypass applyUpdates merge"，但代码实际通过该函数执行，注释与实现自相矛盾。

Suggested change

	// Bypass applyUpdates merge — write originalSettings directly as
	const content = JSON.stringify(settingsFile.originalSettings, null, 2);
	writeWithBackupSync(settingsFile.path, content);

— DeepSeek/deepseek-v4-pro via Qwen Code /review

[wenshao]

[Suggestion] 流式重试失败时 handleError 被调用 3 次

一次失败产生 3 条遥测事件。修复方案：将 handleError 从内部迭代器移到顶层调用者，仅在实际无法恢复时调用。

— DeepSeek/deepseek-v4-pro via Qwen Code /review

[wenshao]

[Suggestion] wrapStreamWithRetry 未使用的 userPromptId 参数

参数已声明但在方法体中从未被引用。应移除该参数，或在重试路径中使用。

Suggested change

	userPromptId: string,
	private async *wrapStreamWithRetry(
	generator: AsyncGenerator<GenerateContentResponse>,
	request: GenerateContentParameters,
	context: RequestContext,
	openaiRequest: OpenAI.Chat.ChatCompletionCreateParams,
	): AsyncGenerator<GenerateContentResponse> {

— DeepSeek/deepseek-v4-pro via Qwen Code /review

[wenshao]

[Suggestion] 流式重试复用 openaiRequest 而非调用 buildRequest

非流式重试调用 await this.buildRequest(...) 重新构建请求；流式重试直接复用捕获的 openaiRequest。不一致——如果 buildRequest 包含 per-request 标识符，重试将使用过期数据。

— DeepSeek/deepseek-v4-pro via Qwen Code /review

[wenshao]

[Suggestion] wrapStreamWithRetry 零测试覆盖

新增 63 行方法（第 577-640 行），pipeline.test.ts 中完全不存在相关测试。建议至少覆盖：正常迭代、重试成功、重试失败、非 model-unloaded 错误直接抛出。

— DeepSeek/deepseek-v4-pro via Qwen Code /review

[wenshao]

[Suggestion] isModelUnloadedError 收窄后缺少否定测试

移除 'model not found' 和 'is not loaded' 匹配后，无测试验证这些错误不再触发重试。建议补充否定测试和新增模式的肯定测试。

— DeepSeek/deepseek-v4-pro via Qwen Code /review

[wenshao]

[Critical] setValueFullSave still cannot delete keys from disk — core fix is incomplete

setValueFullSave passes the full originalSettings to updateSettingsFilePreservingFormat, which internally calls applyUpdates(parsed, updates). applyUpdates only iterates over keys in updates — it never iterates keys in parsed (the on-disk content). For nested objects like mcpServers, this recurses: when mcp remove deletes a server from originalSettings.mcpServers, applyUpdates only iterates the remaining servers’ keys. The deleted server entry in parsed.mcpServers is never touched and survives on disk.

This means /mcp remove reports success but the server reappears after restart — the exact bug this PR claims to fix.

Suggested change

	updateSettingsFilePreservingFormat(
	// Instead of going through updateSettingsFilePreservingFormat (which uses applyUpdates merge),
	// write originalSettings directly to bypass the merge semantics entirely.
	// Use writeWithBackupSync for atomic safety.
	writeWithBackupSync(
	settingsFile.path,
	JSON.stringify(settingsFile.originalSettings, null, 2),
	);

— DeepSeek/deepseek-v4-pro via Qwen Code /review

[wenshao]

[Critical] wrapStreamWithRetry reuses stale openaiRequest without re-running buildRequest()

The stream retry path calls this.client.chat.completions.create(openaiRequest, ...) using the original openaiRequest parameter passed in from the first attempt. The non-streaming retry path (catch block at ~line 546) does call this.buildRequest() again, re-running provider interceptors, inference config, and cache-control logic. The stream retry skips all of this.

If a provider’s buildRequest adds per-attempt metadata (e.g., DashScope sessionId/promptId), stream retry will reuse the original attempt’s identifiers — breaking server-side retry dedup or producing incorrect telemetry.

Suggested change

	await new Promise((resolve) => setTimeout(resolve, 2000));
	const freshRequest = await this.buildRequest(
	request, userPromptId, context, true,
	);
	const retryResult = await this.client.chat.completions.create(
	freshRequest,
	{ signal: request.config?.abortSignal },
	) as AsyncIterable<OpenAI.Chat.ChatCompletionChunk>;

— DeepSeek/deepseek-v4-pro via Qwen Code /review

[wenshao]

[Critical] wrapStreamWithRetry has zero test coverage for its stream retry logic

The 5 new pipeline tests all exercise the non-streaming path via pipeline.execute(). No test sets up streaming mocks (AsyncGenerator, streaming response format) to cover wrapStreamWithRetry. The following code paths are completely untested: (a) catch-and-retry when iterator.next() throws a model-unloaded error during stream consumption; (b) re-throw of non-model-unloaded errors; (c) retry-failure path where the second attempt also fails.

Streaming is the most common runtime path. Any bug in the async generator error handling or retry flow will only surface at production runtime during model-unloaded scenarios.

Suggested change

	private async *wrapStreamWithRetry(
	// Add streaming retry tests via pipeline.executeStream():
	// 1. Stream whose first iterator.next() throws model-unloaded → retry succeeds
	// 2. Stream whose first iterator.next() throws model-unloaded → retry fails → error handler called

— DeepSeek/deepseek-v4-pro via Qwen Code /review

[wenshao]

[Critical] isModelUnloadedError pattern 'model not loaded' still matches permanent errors

The pattern errorMessage.includes('model not loaded') matches both transient JIT-loading errors and permanent errors like "model not loaded: insufficient memory", "model not loaded: invalid configuration", "model not loaded: /path/model.gguf does not exist". This triggers an unnecessary 2s delay + guaranteed-to-fail retry for misconfiguration.

Suggested change

	if (!error) return false;
	// Remove 'model not loaded' pattern, keep only 'model is unloaded' and 'model unloaded'
	// which are the specific LM Studio / llama.cpp JIT-loading phrases.
	return (
	errorMessage.includes('model is unloaded') ||
	errorMessage.includes('model unloaded')
	);

— DeepSeek/deepseek-v4-pro via Qwen Code /review

[wenshao]

[Critical] [tsc] TS2503: Cannot find namespace 'vi' (lines 50, 54).

The vi.Mock type annotation requires the vitest global types to be available.

— DeepSeek/deepseek-v4-pro via Qwen Code /review

[wenshao]

[Suggestion] Dead optional chain on non-optional method isInitialDirectory

workspaceContext.isInitialDirectory?.(expandedDir) ?? workspaceContext.getInitialDirectories().includes(expandedDir) — isInitialDirectory is a regular method on WorkspaceContext, not optional, so ?. never short-circuits and the ?? fallback is dead code. Additionally, the fallback uses expandedDir (tilde-expanded only) instead of the canonicalDirectory (realpath-resolved), which would cause a path mismatch for symlinked directories.

Suggested change

	workspaceContext.isInitialDirectory?.(expandedDir) ??
	workspaceContext.isInitialDirectory(expandedDir)

— DeepSeek/deepseek-v4-pro via Qwen Code /review

[wenshao]

[Suggestion] Hierarchical memory refresh logic (loadServerHierarchicalMemory + setUserMemory/setGeminiMdFileCount/setConditionalRulesRegistry/ui.setGeminiMdFileCount) is duplicated verbatim between /directory add and /directory remove (~30 lines). Extract to a shared helper refreshHierarchicalMemory(config, context) to prevent drift.

— DeepSeek/deepseek-v4-pro via Qwen Code /review

[wenshao]

[Suggestion] JSON.parse(JSON.stringify(...)) silently drops comments and undefined values from originalSettings before writing. Use structuredClone (available since Node 17) instead.

Suggested change

	);
	updateSettingsFilePreservingFormat(
	settingsFile.path,
	structuredClone(settingsFile.originalSettings),
	);

— DeepSeek/deepseek-v4-pro via Qwen Code /review

[wenshao]

[Suggestion] Skipped directories warning uses bare process.stderr.write without debugLogger or telemetry. In containerized/CI environments this may be separated from application logs, making the warning invisible when debugging missing --include-directories context.

Suggested change

	const skippedDirs = this.workspaceContext.getSkippedDirectories();
	const msg = `Warning: The following --include-directories paths were skipped because they do not exist or are not readable:\n${skippedDirs.map((d) => ` - ${d}`).join('\n')}`;
	debugLogger.warn(msg);
	process.stderr.write(msg + '\n');

— DeepSeek/deepseek-v4-pro via Qwen Code /review

[wenshao]

[Suggestion] /directory remove uses settings.setValue() for persistence while MCP commands use settings.setValueFullSave() — inconsistent API for the same kind of operation. While includeDirectories is an array (where setValue works for full replacement), future refactoring to a map structure would silently break the remove path.

— DeepSeek/deepseek-v4-pro via Qwen Code /review

[B-A-M-N]

Closing this PR. Sorry for the scope creep — this ballooned to 145 files / 15k+ lines because of a merge from main that pulled in a ton of unrelated changes, plus several features got mixed together.

I've split this into 4 focused PRs:

1. MCP add/remove persist fix (the original issue MCP add/remove bug #3718): fix(cli): MCP add/remove now correctly persists headers and server deletions B-A-M-N/qwen-code#7

   • 5 files changed — settings.ts, add.ts, remove.ts, and tests
   • Fixes mcp remove not deleting servers and mcp add not updating headers

2. Model-unloaded retry for LM Studio (/model switch to local LM Studio model fails with "Model is unloaded" — pre-flight check blocks JIT loading #3802): fix(core): retry API request on model-unloaded errors for local model servers B-A-M-N/qwen-code#8

   • 2 files changed — pipeline.ts and tests
   • Retries API requests when local model servers return "model is unloaded"

3. Directory remove command: feat(cli): add /directory remove subcommand B-A-M-N/qwen-code#9

   • 8 files changed — directoryCommand.tsx, i18n, config, workspaceContext
   • Adds /directory remove subcommand with proper path resolution and sandbox checks

4. Memory fast model routing: fix(memory): route auto-memory recall selector to fast model B-A-M-N/qwen-code#10

   • 1 file changed — relevanceSelector.ts
   • Routes auto-memory recall to fast model for background queries

Each PR is now small, focused, and reviewable on its own. Will keep future PRs more focused.