refactor(cli): remove superseded settings dialog helpers

[qqqys]

What this PR does

Removes thirteen exported helpers from the settings utility module, together with the tests that existed only to cover them. Every one of them was reachable from nothing but its own unit test: no runtime code path, in this repository or any package it publishes, called them.

They are the remains of an earlier generation of the settings-dialog API. The dialog was rebuilt on a different set of helpers in the same module and the old set was never removed, so the two generations have been sitting side by side. One of the removed predicates was a character-for-character duplicate of a helper the dialog does use; another was a one-line wrapper around a second wrapper.

Three neighbouring helpers that a naive search also flags were deliberately kept: one is still called by a live function, one is named by an in-flight design document as an API it intends to reuse, and one was needed by a surviving test as an assertion helper, so that test now calls the live function it delegated to anyway. Behaviour is unchanged — nothing that runs today loses a code path.

Why it's needed

AGENTS.md opens with Simplicity First: minimum code that solves the problem, nothing speculative. A superseded API generation that only its own tests exercise is the opposite — it costs every future reader a decision about which of two near-identical helpers to call, and it costs every future refactor a set of tests that pin behaviour nothing depends on. Deleting it makes the module's real surface visible.

This is the first candidate filed on #9375 and the first PR produced by that sweep. It is deliberately one candidate, not a batch.

Reviewer Test Plan

How to verify

The claim to check is that nothing outside the module used these helpers. Search the repository for any of the removed names — packages/, integration-tests/, scripts/, docs/, and .github/ — and confirm every hit is inside this diff. There is no star re-export of the module and no namespace import of it, so a name search is complete.

Then confirm the settings dialog is untouched in behaviour: it imports a different set of helpers from the same module, and its suite still passes in full.

cd packages/cli && npm run typecheck                       # clean
cd packages/cli && npx vitest run src/utils/settingsUtils.test.ts
    ✓ 52 tests
cd packages/cli && npx vitest run $(rg -l settingsUtils src --glob '*.test.*')
    ✓ 6 files, 188 tests — includes SettingsDialog (61)
npm run lint:ci                                            # exit 0

Type checking is the gate that matters most here: it is what proves no surviving code referenced a removed symbol, and it does not run in CI.

Evidence (Before & After)

N/A — no user-visible or TUI change.

Tested on

OS	Status
🍏 macOS	⚠️
🪟 Windows	⚠️
🐧 Linux	✅

Environment (optional)

Unit tests, typecheck and lint only; no runtime invocation needed.

Risk & Scope

• Main risk or tradeoff: an out-of-repository consumer importing these helpers by deep path. The CLI package publishes only its binary entry point and an export subpath, neither of which reaches this module, so the risk is theoretical rather than semver-visible.
• Not validated / out of scope: the other two candidates on [find-simplifications] candidate ledger #9375 — the orphaned locale keys and a three-line unused wrapper in the i18n module — are left for separate PRs. macOS and Windows were not exercised locally; the change is platform-independent.
• Breaking changes / migration notes: none.

Linked Issues

Candidate settings-utils-generation-a from #9375.

Removed symbols and their evidence

Removed from packages/cli/src/utils/settingsUtils.ts (177 lines) and packages/cli/src/utils/settingsUtils.test.ts (336 lines), plus one now-unused type import:

getSettingsByCategory, getSettingsByType, getSettingsRequiringRestart, isValidSettingKey, getSettingCategory, shouldShowInDialog, getDialogSettingsByCategory, getDialogSettingsByType, getSettingValue, isSettingModified, hasRestartRequiredSettings, isValueInherited, getEffectiveDisplayValue

Consumers found for each, before removal: exactly one file, packages/cli/src/utils/settingsUtils.test.ts. No production file, no other package, no integration test, no documentation, no dynamic or string-keyed lookup, no export * re-export, no namespace import.

Kept deliberately:

• setNestedPropertyForce — called by the live setPendingSettingValue.
• getRestartRequiredSettings — named at docs/design/hot-reload/settings-change-detection.md:426 as an API that design intends to reuse; that design has no implementation in the tree yet, so the helper is reserved, not dead.
• isDefaultValue — the surviving twin of the removed isValueInherited; the two had identical bodies and the dialog imports this one.

The single added line replaces a deleted helper used as an assertion aid in a surviving test with getEffectiveValue, which the deleted helper delegated to.

中文说明

这个 PR 做了什么

从设置工具模块中移除十三个导出的辅助函数，以及仅为覆盖它们而存在的测试。这些函数的唯一可达路径就是它们自己的单元测试：本仓库以及它发布的任何 package 中，都没有任何运行时代码调用它们。

它们是上一代设置对话框 API 的遗留物。对话框已在同一模块中基于另一组辅助函数重建，而旧的一组从未被移除，因此两代 API 一直并存。被移除的其中一个判定函数与对话框实际使用的某个函数逐字符完全相同；另一个则是对另一层包装的一行包装。

有三个相邻的辅助函数虽然也会被朴素搜索标记出来，但被刻意保留：一个仍被在用的函数调用；一个被一份尚在推进中的设计文档列为将要复用的 API；还有一个是某个存活测试用作断言辅助的，现在该测试改为直接调用它本就委托的那个在用函数。行为没有变化——今天在运行的代码没有失去任何路径。

为什么需要

AGENTS.md 开篇即是 Simplicity First：解决问题的最小代码量，不做任何投机性设计。一代仅由自身测试驱动的、已被取代的 API 恰恰相反——它让每位后来的读者都要判断两个近乎相同的函数该调用哪个，也让每次重构都要面对一批固定着无人依赖之行为的测试。删除它能让该模块真实的代码面显现出来。

这是 #9375 上记录的第一个候选，也是该轮扫描产出的第一个 PR。它刻意只包含一个候选，而非一批。

审阅测试计划

如何验证

需要核实的论断是：模块之外没有任何地方使用这些辅助函数。在仓库中搜索任一被移除的名称——packages/、integration-tests/、scripts/、docs/、.github/——确认所有命中都在本 diff 之内。该模块没有星号再导出，也没有命名空间导入，因此按名称搜索是完备的。

然后确认设置对话框的行为未受影响：它从同一模块导入的是另一组辅助函数，其测试套件仍全部通过。

cd packages/cli && npm run typecheck                       # 通过
cd packages/cli && npx vitest run src/utils/settingsUtils.test.ts
    ✓ 52 个测试
cd packages/cli && npx vitest run $(rg -l settingsUtils src --glob '*.test.*')
    ✓ 6 个文件，188 个测试——含 SettingsDialog 的 61 个
npm run lint:ci                                            # exit 0

这里最关键的门禁是类型检查：它才是"没有存活代码引用被删符号"的证明，而它并不在 CI 中运行。

证据（前后对比）

N/A——无用户可见或 TUI 变化。

测试环境

操作系统	状态
🍏 macOS	⚠️
🪟 Windows	⚠️
🐧 Linux	✅

运行环境（可选）

仅单元测试、类型检查与 lint，无需运行时调用。

风险与范围

• 主要风险或权衡：仓库之外可能有消费者通过深路径导入这些辅助函数。CLI package 只发布其二进制入口点和一个 export 子路径，二者都无法触及该模块，因此这一风险是理论上的，而非 semver 层面的破坏。
• 未验证 / 不在范围内：[find-simplifications] candidate ledger #9375 上的另外两个候选——孤儿语言键，以及 i18n 模块中一个三行的未使用包装函数——留待各自独立的 PR。macOS 与 Windows 未在本地验证；该改动与平台无关。
• 破坏性变更 / 迁移说明：无。

关联 Issue

来自 #9375 的候选 settings-utils-generation-a。

[qwen-code-ci-bot]

✅ Qwen Triage finished — view run. See the stage comments in this thread for the result.

✅ Qwen Triage 已完成 —— 查看运行。结果见本线程中的各阶段评论。

[qwen-code-ci-bot]

Thanks for the PR!

Template looks good ✓

Problem: real and verified, not theoretical. I checked the claim against the base tree: every one of the thirteen removed helpers (getSettingsByCategory, getSettingsByType, getSettingsRequiringRestart, isValidSettingKey, getSettingCategory, shouldShowInDialog, getDialogSettingsByCategory, getDialogSettingsByType, getSettingValue, isSettingModified, hasRestartRequiredSettings, isValueInherited, getEffectiveDisplayValue) has zero references outside settingsUtils.ts and its own test file — a repo-wide name search across all file types confirms it. There is no export * re-export and no namespace import of the module, so a name search is complete. This is candidate 1 on the #9375 ledger, marked landable there.

Direction: aligned — removing a superseded API generation that only its own tests exercise is exactly the Simplicity First cleanup AGENTS.md asks for. Pure internal hygiene; no CHANGELOG signal applies, and none is needed.

Size: not applicable — packages/cli/src/utils/ is not a Stage 0 core-module path. For the record: 178 production lines removed, 340 test lines changed (339 removed + 1 added).

Approach: the scope is right. One candidate, not a batch, and the three "deliberately kept" justifications each check out: setNestedPropertyForce is still called by setPendingSettingValue / setPendingSettingValueAny; getRestartRequiredSettings is reserved by the in-flight hot-reload design (docs/design/hot-reload/settings-change-detection.md:426); isDefaultValue — body identical to the removed isValueInherited — is the one SettingsDialog.tsx imports. No drive-by changes; the other two ledger candidates correctly stay out of this PR.

Risk: no elevated risk signals — none of the revert-correlated paths are touched.

Moving on to code review. 🔍

中文说明

感谢贡献！

模板完整 ✓

问题：真实且已验证，不是理论问题。我在 base 树上核实了 PR 的论断：13 个被移除的辅助函数（getSettingsByCategory、getSettingsByType、getSettingsRequiringRestart、isValidSettingKey、getSettingCategory、shouldShowInDialog、getDialogSettingsByCategory、getDialogSettingsByType、getSettingValue、isSettingModified、hasRestartRequiredSettings、isValueInherited、getEffectiveDisplayValue）在 settingsUtils.ts 及其自身测试文件之外零引用——全仓库、所有文件类型的按名搜索均确认。该模块没有星号再导出，也没有命名空间导入，因此按名搜索是完备的。这是 #9375 清单上的候选 1，在那里标记为可落地。

方向：对齐——删除一代仅由自身测试驱动的已被取代的 API，正是 AGENTS.md Simplicity First 所要求的清理。纯内部卫生改进；无 CHANGELOG 信号，也不需要有。

规模：不适用——packages/cli/src/utils/ 不在 Stage 0 核心模块路径上。备案：178 行生产代码移除，340 行测试改动（339 删 + 1 增）。

方案：范围合理。只含一个候选而非一批，且三个"刻意保留"的理由逐一核实无误：setNestedPropertyForce 仍被 setPendingSettingValue / setPendingSettingValueAny 调用；getRestartRequiredSettings 被尚在推进中的热重载设计文档（docs/design/hot-reload/settings-change-detection.md:426）预留；isDefaultValue——与移除的 isValueInherited 函数体完全相同——是 SettingsDialog.tsx 实际导入的那一个。无顺手改动；清单上另外两个候选正确地留在本 PR 之外。

风险：无升级风险信号——未触及任何与 revert 相关的路径。

进入代码审查 🔍

— Qwen Code · qwen3.8-max

_{Reviewed at f01e4f83d634a4e99e7ea848bc95be2e76e93e20 · re-run with @qwen-code /triage}

[qwen-code-ci-bot]

Code review

Before reading the diff, my independent plan for "remove the superseded settings-dialog helpers" was: name-search every export of the module, keep the ones with live callers, delete the zero-reference ones together with their test blocks, and fix whichever surviving test leans on a removed helper. The PR does exactly this, and I could not find a simpler version of it.

What I verified against the base tree and the diff:

• The deletion is exactly as described. Thirteen helpers leave settingsUtils.ts, plus the SettingsType type import that only the two removed *ByType helpers used. Nothing else in the module is touched.
• Zero external references. The repo-wide search covers packages/, integration-tests/, scripts/, docs/, .github/, all file types. Every live importer of the module — SettingsDialog.tsx, config-command.ts, the serve routes, the config migrations, loadedSettingsAdapter.ts, settingsWatcher.ts, dialogScopeUtils.ts — imports a different set of symbols, none of which is removed here.
• The one added line is semantically equivalent. The surviving setPendingSettingValue nested-key test used to read the value back through getSettingValue; it now calls getEffectiveValue, which the removed wrapper delegated to. For this test's case (key context.fileFiltering.respectGitIgnore set to true in pending settings) both return the same true, so the assertion is unchanged in meaning.
• No semver exposure. The CLI package's exports map publishes only . and ./export, neither of which references this module, and Node's exports-map enforcement blocks deep imports — so the PR's own risk note (an out-of-repo deep-path consumer) is correctly assessed as theoretical, not real.

No blockers, no convention violations. No sequence diagram or files table — this is a two-file deletion with no runtime flow to draw.

Testing evidence (the PR's own CI, read via the API)

Per the gate rules I do not run PR code; the evidence below is the PR's own CI on the reviewed commit. The main unit suite (Test (ubuntu-latest, Node 22.x)) was still in flight at review time — reported as pending, not polled. Every completed check is green; the macOS/Windows test jobs are skipped on this event by workflow design, not failures. The typecheck/tests that will land are exactly the signal that matters here: any surviving reference to a removed symbol fails typecheck, and the surviving dialog tests pin the untouched behaviour. The test outputs quoted in the PR description are the author's own claim, not independently re-run.

Final CI results for f01e4f8 (auto-updated by the triage finalize job after CI completed):

Check	Conclusion
Classify PR	✅ success
Dependency CVE audit	✅ success
Desktop Shell (ubuntu-22.04)	✅ success
Desktop Shell (windows-2022)	✅ success
Secret scan (TruffleHog)	✅ success
Test (ubuntu-latest, Node 22.x)	✅ success
web-shell E2E Smoke (ubuntu-latest, Node 22.x)	✅ success

_{One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。}

Not verified: runtime behaviour after the change — nothing user-visible changes by construction (no live caller loses a code path), and the dialog's own test suite covers it in CI; live execution is off-limits for the gate (PR code is never run here). No sandboxed-lane line either — this PR makes no behavioural claim; the claim is "nothing references these", which the static search above already settles and CI typecheck re-settles.

中文说明

代码审查

在读 diff 之前，我对"移除被取代的设置对话框辅助函数"的独立方案是：按名搜索该模块的每个导出，保留有活跃调用者的，删除零引用的及其测试块，并修正依赖被删辅助函数的存活测试。PR 的做法与此完全一致，我没有找到更简的版本。

我对照 base 树与 diff 核实的内容：

• 删除与描述完全一致。 十三个辅助函数离开 settingsUtils.ts，外加只有两个被移除的 *ByType 函数用到的 SettingsType 类型导入。模块其余部分未动。
• 零外部引用。 全仓库搜索覆盖 packages/、integration-tests/、scripts/、docs/、.github/ 及所有文件类型。该模块的所有活跃导入方——SettingsDialog.tsx、config-command.ts、serve 路由、配置迁移、loadedSettingsAdapter.ts、settingsWatcher.ts、dialogScopeUtils.ts——导入的是另一组符号，均不在移除之列。
• 唯一的新增行语义等价。 存活的 setPendingSettingValue 嵌套键测试原本通过 getSettingValue 回读数值；现在改调用被删包装函数所委托的 getEffectiveValue。就该测试的场景而言（键 context.fileFiltering.respectGitIgnore 在 pending 中被设为 true），两者返回相同的 true，断言含义不变。
• 无 semver 暴露。 CLI package 的 exports 映射只发布 . 与 ./export，二者均不引用本模块，且 Node 的 exports 映射强制会拦截深路径导入——因此 PR 自述的风险（仓库外消费者走深路径）正确地判定为理论性的，而非真实存在。

无阻塞项，无约定违规。不附时序图或文件表——这是两个文件的删除，没有可绘制的运行时流程。

测试证据（PR 自己的 CI，通过 API 读取）

按门禁规则我不运行 PR 代码；以下证据是 PR 自身 CI 在被审 commit 上的结果。主单元测试套件（Test (ubuntu-latest, Node 22.x)）在审查时仍在运行——如实记为 pending，不做轮询。所有已完成检查均为绿色；macOS/Windows 测试任务是工作流在此事件下按设计跳过，不是失败。即将落地的类型检查/测试正是此 PR 最关键的信号：任何对被删符号的残留引用都会在类型检查中失败，存活的对话框测试则固定了未受影响的行为。PR 描述中引用的测试输出为作者自述，未经独立复跑。

未验证：变更后的运行时行为——按构造就没有用户可见的变化（没有任何活跃调用者失去代码路径），对话框自身的测试套件在 CI 中覆盖；门禁禁止实际执行（此处从不运行 PR 代码）。也不附沙箱验证行——本 PR 不做行为性论断；其论断是"没有东西引用这些"，上面的静态搜索已了结，CI 类型检查会再次了结。

— Qwen Code · qwen3.8-max

_{Reviewed at f01e4f83d634a4e99e7ea848bc95be2e76e93e20 · re-run with @qwen-code /triage}

[qwen-code-ci-bot]

Confidence: 3/5 — clean review, but the fork-refactor approval guardrail needs a maintainer's sign-off.

Stepping back: this is what a good cleanup PR looks like. The problem is not a hypothesis — the dead code is statically observable, and I confirmed it myself: thirteen exported helpers whose only caller anywhere in the repository was their own unit test, the remains of a dialog generation that was rebuilt and never swept out. My independent proposal (name-search every export, delete the zero-reference set with its tests, keep what still has callers) is exactly what the PR does; I did not find a simpler path it missed. The three kept helpers each have a real, verified reason to stay. The diff is 100% deletion plus one semantically equivalent line swap — nothing unrelated rode along. Every stage came back clean.

The reason this is not an approval is policy, not doubt:

• Approval guardrail. This is a cross-repository (fork) PR whose title is a refactor type. Those are never auto-approved — a human maintainer must sign off. That rule decides the action here, hence the capped score above.
• CI still pending independently: the main unit suite (Test (ubuntu-latest, Node 22.x)) had not completed at review time, so even without the guardrail, approval would have been deferred until green.

One honest caveat for the maintainer: the author holds write access on this repository and opened the PR from a fork; the guardrail keys on the PR's fork origin, not on the author's trust level, so it still applies — but it also means the escalation is likely a formality. No approve-on-green marker is emitted, since the guardrail blocks approval regardless of how CI lands.

中文说明

置信度：3/5 —— 审查干净，但 fork refactor 审批护栏需要维护者签核。

退一步看：这是一个清理类 PR 该有的样子。问题不是假设——死代码在静态上可观察，我也亲自确认了：十三个导出辅助函数在全仓库的唯一调用者就是它们自己的单元测试，是重建后从未清扫掉的上一代对话框 API 的遗留。我的独立方案（按名搜索每个导出，删除零引用集合及其测试，保留仍有调用者的）与 PR 的做法完全一致；我没有找到它遗漏的更简路径。三个保留下来的辅助函数各有真实且已核实的留存理由。diff 是 100% 删除外加一行语义等价的替换——没有任何无关改动夹带。每个阶段都干净。

不给 approve 的原因是政策而非疑虑：

• 审批护栏。 这是一个跨仓库（fork）PR，且标题为 refactor 类型。此类 PR 从不自动批准——必须由人类维护者签核。该规则决定此处的动作，因此上面的分数封顶。
• CI 也尚未结束：主单元测试套件（Test (ubuntu-latest, Node 22.x)）在审查时未完成，即便没有护栏，批准也要推迟到变绿之后。

给维护者的一个诚实提示：作者在本仓库拥有 write 权限，是从 fork 开出的 PR；护栏以 PR 的 fork 来源为准，与作者的信任级别无关，因此仍然适用——但这也意味着本次升级大概率只是走个形式。由于护栏无论 CI 结果如何都阻止批准，本次不发出 approve-on-green 标记。

— Qwen Code · qwen3.8-max

_{Reviewed at f01e4f83d634a4e99e7ea848bc95be2e76e93e20 · re-run with @qwen-code /triage}

[qwen-code-ci-bot]

⏸️ Deferring to @ZijianZhang989 — fork-refactor approval guardrail: this is a cross-repository PR with a refactor title, which is never auto-approved regardless of review outcome. The review itself is clean (see the staged comments above — dead-code claim verified statically, diff minimal, no blockers); it needs a maintainer's sign-off to land. Note the main unit suite was still running at review time.

— Qwen Code · qwen3.8-max

[qwen-code-ci-bot]

No issues found. LGTM! ✅

中文说明

未发现问题。LGTM！✅

— qwen3.8-max via Qwen Code /review (v0.21.13)

[doudouOUC]

.qwen/tmp/qwen-review-body.txt

[doudouOUC]

Review: PR #9379 — refactor(cli): remove superseded settings dialog helpers

Verdict: COMMENT — no Criticals, no Suggestions, no findings.

Summary

All 14 review agents (correctness, security, code quality, performance, test coverage, adversarial personas, build & test) examined this diff and returned zero findings. This is a clean dead-code removal:

• 13 exported functions removed from packages/cli/src/utils/settingsUtils.ts — every one had zero callers outside the two files being modified (verified by cross-codebase grep).
• ~517 lines of dead code deleted (178 source + 339 test), plus 1 line added to replace a deleted helper in a surviving test assertion.
• Three deliberately kept helpers (setNestedPropertyForce, getRestartRequiredSettings, isDefaultValue) all have live callers or a design-doc reservation.
• The one behavioral change (getSettingValue → getEffectiveValue) in the test file is semantically equivalent for the tested value.

What was verified

• Cross-codebase grep for all 13 removed symbols: zero hits outside the diff.
• All 6 files that import from settingsUtils import only remaining exports.
• SettingsType type import removal is correct (only used by removed functions).
• isValueInherited ↔ isDefaultValue duplicate resolution: correct.
• No security, performance, or test-coverage regressions.

Conclusion

This is a well-scoped, well-verified dead-code cleanup that follows the Simplicity First principle from AGENTS.md. The PR is ready to merge.

[doudouOUC]

Review: PR #9379 — refactor(cli): remove superseded settings dialog helpers

Verdict: APPROVE — no Criticals, no Suggestions, no findings.

Summary

All 8 review agents (issue fidelity, correctness, security, code quality, test coverage, diff specialist, adversarial persona, build & test) examined this diff and returned zero findings. This is a clean dead-code removal:

• 13 exported functions removed from packages/cli/src/utils/settingsUtils.ts — every one had zero callers outside the two files being modified (verified by cross-codebase grep across packages/, integration-tests/, scripts/, docs/, .github/).
• ~517 lines of dead code deleted (178 source + 339 test), plus 1 line added to replace a deleted helper in a surviving test assertion.
• Three deliberately kept helpers (setNestedPropertyForce, getRestartRequiredSettings, isDefaultValue) all have live callers or a design-doc reservation.
• The one behavioral change (getSettingValue → getEffectiveValue) in the test file is semantically equivalent for the tested value.

What was verified

• Cross-codebase grep for all 13 removed symbols: zero hits outside the diff.
• All 6 files that import from settingsUtils import only remaining exports.
• SettingsType type import removal is correct (only used by removed functions).
• isValueInherited ↔ isDefaultValue duplicate resolution: correct (both are !settingExistsInScope(key, settings)).
• No security, performance, or test-coverage regressions.

Conclusion

This is a well-scoped, well-verified dead-code cleanup that follows the Simplicity First principle from AGENTS.md. No issues found. The PR is ready to merge.

[qwen-code-ci-bot]

Released in v0.21.14.