fix(plugins): honor plugin enable/disable without corrupting cordis.patch.yml - #37
Conversation
…atch.yml Toggling a plugin's switch in the instance settings showed as updated but had no effect after restart (issue dsh-plugins#28). The cordis.patch.yml editor used a flat line scan: disabling a plugin mounted via an `- insert:` block deleted the child's `- id:` line, collapsing `insert:` from an array into an object, which the loader's applyEntryPatches then rejected with a spread error; disabling a plain top-level `- id:` override appended an unreferenced block that applyEntryPatches skipped, so the plugin kept running. `read_disabled_ids` also misread the file, so the UI reported "disabled" while dsh still ran the plugin. - Rework the three cordis.patch.yml editors (set_disabled_row, read_disabled_ids/disabled_ids, strip_cordis_rows) to be block-aware: `disabled: true` is added/removed on the matched entry's own row — both a plain top-level `- id:` entry and an `- insert:` block's child is recognized, and the insert wrapper itself is never touched - Direct-child indentation lets a `disabled:` inside a plugin's own `config:` mapping (e.g. an MCP server's `disabled` option) be ignored, so it is never mistaken for a plugin-level toggle - Bundle-provided entries (no row in this document) are disabled by appending a `- id:` + `disabled: true` override and re-enabled by dropping it, matching the loader's "bundle layers first, then the user layer" composition - Preserve everything else byte-for-byte: comments, `!!js` scalars, unrelated entries, blank separators, and the `[]` placeholder of an empty document - Add regression tests covering the real profile shape (multiple insert blocks plus a plain override), in-block toggling, bundle override round-trip, placeholder restore, and ignoring `disabled:` under `config:` Fixes dsh-plugins#28
Gu-ZT
left a comment
There was a problem hiding this comment.
感谢这个 PR——问题定位准确,block-aware 重写方向正确,修掉了一个真实且严重的损坏 bug(已对照 dsh loader 源码 applyEntryPatches 验证:insert 塌陷后 data.push(...insert) 对不可迭代值 spread → profile 无法启动;顶层 entry 误删 id 行会把 config 行孤立进前一个 entry)。但探针测试实证了两个残余 bug,本次 request changes(修复都很小):
阻塞项
1. [HIGH] 写入路径:插件自身 config: 内含 disabled: 键时,禁用操作静默无效
plugins.rs set_disabled_row 的子行扫描中(约 L1353),tl.strip_prefix("disabled:") 分支没有像 has_other(L1356)那样要求 indent_of(line) == t.child_indent。插件 config 里嵌套的 disabled: true|false(MCP server 类配置很常见)会置上 has_disabled/disabled_on,导致禁用路径既不 insert 也不 flip → 文件不变、开关无效——恰好复现 issue #28 的原始症状(开关显示变了但实际没生效)。读路径修了这类误报,写路径漏了。
修法:给该分支加上 child_indent 守卫,并补一个写路径回归测试(禁用 config 内含 disabled 键的插件)。
2. [MED] 读路径:entry 级 disabled: true 位于 config: 块之后时被漏读
disabled_ids 中 inside_config 的关闭条件要求 ind < current_child_indent(约 L958-964),但同级键正好位于 current_child_indent,所以 entry 内 inside_config 永不关闭:- id: x → config: → a: 1 → disabled: true(合法 YAML)会把 x 报为启用,UI 误报——正是本 PR 要修的 bug 类别。虽然本工具写入时把 disabled: 放在 config: 之前(读写自洽),但手改文件会踩中。
修法:非 disabled 键出现在 ind <= inside_config(或 == current_child_indent)时关闭 inside_config,补读路径测试。
3. [LOW] 裸 id: 在任意深度都会重置 current_id
约 L916 丢掉了旧版的无缩守卫:config: 内嵌套的 id:(如 MCP server 配置的 server: {id: fake, …})会重置 current_id/inside_config,其后更深处的 disabled: true 会误报一个伪造 id。建议恢复缩进守卫。
非阻塞小项
- CRLF 文件经任何编辑后被归一为 LF(
lines()+join("\n")),「byte-for-byte」的说法对 CRLF 不成立; - 启用无匹配行时 L1309 提前返回
raw.trim_end() + "\n",绕过了replace_placeholder——空 patch 文件会得到"\n"而非其余代码依赖的[]\n占位; - 追加 override 时 id 不加引号(含空格/
:的 id 会产出非法 YAML——实践中 cordis id 是 slug,可接受); - 剥离
- insert:的唯一子项后会留下空 insert 壳(合法 YAML、loader no-op,纯外观); - PR 描述称新增 5 个测试,实为 4 个(
[]占位恢复测试是既有用例)。
验证情况
cargo fmt --check干净;PR 自身测试 95 passed / 0 failed;- 上述 Bug 1/2/3 均以临时探针测试实证复现(已按要求移除);
- 分支基于 #34 合入前的 main,但仅改 plugins.rs,与 main 无冲突,rebase 非必需。
鉴于 insert 塌陷是数据损坏级 bug,修完 1-3 后应尽快合入。
- 写路径:entry 子行扫描仅承认 child_indent 层的 disabled 键,config 内嵌套 disabled 不再使禁用静默失效 - 读路径:config 块同级或更浅的键会关闭 inside_config,位于 config 之后的 entry 级 disabled: true 不再漏读 - 读路径:恢复裸 id: 的无缩守卫,config 内嵌套 id 不再伪造 entry - 启用无匹配行时经由 replace_placeholder,空文档保持 [] 占位 - 新增 4 个回归测试(写路径嵌套 disabled、config 后 entry 级 disabled、嵌套 id 守卫、占位保持)
What
Fixes #28 — toggling a plugin's enable/disable switch in instance settings showed a new state but had no effect after restart, and disabling a plugin mounted via
- insert:could even corrupt the profile'scordis.patch.yml.Root cause
The
cordis.patch.ymleditor used a flat line scan (set_disabled_row,read_disabled_ids,strip_cordis_rows):- id:row and deleted that line, collapsinginsert:from an array into an object. dsh'sapplyEntryPatchesthen threw a spread error on the un-iterable value, so the profile failed to boot.- id: <id>+disabled: trueoverride could target an entry the user layer never references, which the loader skips — so the plugin kept running.read_disabled_idsscanned the same way, so the UI reported "disabled" while dsh still ran the plugin.Fix
Reworked the three editors to be block-aware:
disabled: trueis added/removed on the matched entry's own row — both a plain top-level- id:entry and an- insert:block's child are recognized; the insert wrapper itself is never touched.disabled:inside a plugin's ownconfig:mapping (e.g. an MCP server option) is never mistaken for a plugin-level toggle.- id:+disabled: trueoverride and re-enabled by dropping it — matching the loader's "bundle layers first, then the user layer" composition.!!jsscalars, unrelated entries, blank separators, and the[]placeholder of an empty document.Verification
cargo fmt --all -- --checkcleancargo clippy --workspace --all-targets --locked -- -D warningscleancargo test --workspace --locked: 95 passed, 0 failed[]placeholder restore, and ignoringdisabled:underconfig:Tested against a real
dsh-tui/cordis.patch.yml: after the fix the disabled plugin's own row carriesdisabled: true, the YAML still parses, and dsh'sapplyEntryPatchesreports the plugin as disabled.