fix(doctor): --require-understand 真的检查 Understand Anything，终结 fail-open 空转 - #144
Merged
Merged
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
🔒 Repowise is not analyzing this repository The PR bot is free on public repositories. This one is private, which needs a Pro plan. |
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe doctor bootstrap flow now reports a missing Understand Anything skill or plugin when required components are absent. New JSON integration tests validate isolated discovery and selective requirement removal. The doctor integration also uses a new toolchain digest. ChangesUnderstand requirement validation
Doctor toolchain metadata
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Code Intel change risk
Top signals
revspec: |
…en 空转
`--require-understand` / `--doctor-require-understand` 从来没有检查过 Understand
Anything。`missing_list` 里唯一消费 `options.require_understand` 的两个分支只看
`/graphProvider/sourceFound` 和 `/graphProvider/cargoFound`——这两个在 pipeline
checkout 内部恒真（就是 crates/code-intel-cli/src/graph.rs 和 Cargo.toml 在不在）。
而同一份文档里算出来的 `understandAnything` 块被发布之后再也没人读过。
于是：
doctor bootstrap --repo-path . --no-require-repowise --require-understand --json
在一台完全没装 Understand Anything 的机器上退出 0、报 `"ok": true`、
`"missing": []`，同时自己的文档里白纸黑字写着
`"understandAnything": {"skillFound": false, "pluginFound": false}`。
被退休的 PowerShell 探针有同样的洞（archive/check-code-intel-tools.ps1 里
`$RequireUnderstand` 也只 gate graphProvider），Rust 端口忠实地把缺陷一起搬了过来。
口径按 installer 自己的 RequireUnderstand 检查对齐：Understand Anything 以 agent
skill 或 plugin 目录两种形态发布，任一存在即满足，两者皆无才进 `missing`。
- 检查严格 opt-in：不带旗标的默认路径一个字节都没变（仍退出 0）。
- 现有两条 graphProvider 检查原样保留，避免把当前的绿灯换成错误的红灯。
- toolchainDigests 随 mod.rs 改动 repin，并用 sha256sum 手工核对（#133 盲区）。
修复后同一条命令退出 1、`ok: false`、`missing: ["Understand Anything skill or plugin"]`，
与它自己发布的 understandAnything 块一致。
2233admin
force-pushed
the
dogfood/F1-require-understand-noop
branch
from
August 3, 2026 16:43
5b92421 to
0dda4f2
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/code-intel-cli/tests/doctor_envelope.rs`:
- Around line 331-418: Update doctor_bootstrap to return both the parsed JSON
and process exit status from Output, then assert the absent case exits with code
1 and reports ok: false. Extend
require_understand_reports_missing_understand_anything with a discovered plugin
fixture and verify it removes the same “Understand Anything skill or plugin”
missing entry without introducing others. Run focused cargo test coverage and
the relevant integration-contract checks.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f98b26fd-9ffa-412a-aaef-845db950daac
📒 Files selected for processing (3)
crates/code-intel-cli/src/doctor_bootstrap/mod.rscrates/code-intel-cli/tests/doctor_envelope.rsorchestration/integrations.json
Comment on lines
+331
to
+418
| fn doctor_bootstrap(pipeline_root: &Path, home: &Path) -> Value { | ||
| let output = Command::new(env!("CARGO_BIN_EXE_code-intel")) | ||
| .args(["doctor", "bootstrap", "--pipeline-root"]) | ||
| .arg(pipeline_root) | ||
| .args(["--no-require-repowise", "--require-understand", "--json"]) | ||
| // The probe derives the Understand Anything candidates from the home | ||
| // directory, so pinning it is what makes this test independent of | ||
| // whatever the developer or CI runner happens to have installed. | ||
| .env("USERPROFILE", home) | ||
| .env("HOME", home) | ||
| .output() | ||
| .expect("doctor bootstrap"); | ||
| serde_json::from_slice(&output.stdout).expect("bootstrap JSON") | ||
| } | ||
|
|
||
| /// Regression: `--require-understand` was a fail-open no-op. It gated only on | ||
| /// `/graphProvider/{sourceFound,cargoFound}` — both trivially true in a | ||
| /// checkout — and never once read the `understandAnything` block the very same | ||
| /// document publishes, so the flag answered `ok: true, missing: []` on a | ||
| /// machine reporting `skillFound: false, pluginFound: false`. | ||
| #[test] | ||
| fn require_understand_reports_missing_understand_anything() { | ||
| let root = temp_dir(); | ||
| let pipeline = root.join("pipeline"); | ||
| let home = root.join("home"); | ||
| pipeline_scratch(&pipeline); | ||
| fs::create_dir_all(&home).unwrap(); | ||
|
|
||
| let absent = doctor_bootstrap(&pipeline, &home); | ||
| // Precondition: the checks the flag *used* to rely on are both satisfied, | ||
| // so nothing but the Understand Anything check can fail this run. | ||
| assert_eq!( | ||
| absent["checks"]["graphProvider"]["sourceFound"], | ||
| json!(true) | ||
| ); | ||
| assert_eq!(absent["checks"]["graphProvider"]["cargoFound"], json!(true)); | ||
| assert_eq!( | ||
| absent["checks"]["understandAnything"]["skillFound"], | ||
| json!(false) | ||
| ); | ||
| assert_eq!( | ||
| absent["checks"]["understandAnything"]["pluginFound"], | ||
| json!(false) | ||
| ); | ||
| assert_eq!(absent["strict"]["requireUnderstand"], json!(true)); | ||
|
|
||
| // An installed skill satisfies the requirement, so the check cannot be a | ||
| // blanket failure that merely looks like enforcement. | ||
| let skill = home.join(".claude").join("skills").join("understand"); | ||
| fs::create_dir_all(&skill).unwrap(); | ||
| fs::write(skill.join("SKILL.md"), "# understand\n").unwrap(); | ||
| let present = doctor_bootstrap(&pipeline, &home); | ||
| assert_eq!( | ||
| present["checks"]["understandAnything"]["skillFound"], | ||
| json!(true) | ||
| ); | ||
|
|
||
| // A differential rather than a bare membership check: the scratch root is | ||
| // deliberately incomplete in other ways (no pipeline script, no config), | ||
| // so the honest claim is that installing Understand Anything removes | ||
| // exactly one entry and changes nothing else. | ||
| let entries = |observation: &Value| { | ||
| observation["missing"] | ||
| .as_array() | ||
| .expect("missing array") | ||
| .iter() | ||
| .map(|entry| entry.as_str().expect("missing entry").to_string()) | ||
| .collect::<Vec<_>>() | ||
| }; | ||
| let (before, after) = (entries(&absent), entries(&present)); | ||
| let removed = before | ||
| .iter() | ||
| .filter(|entry| !after.contains(entry)) | ||
| .cloned() | ||
| .collect::<Vec<_>>(); | ||
| assert_eq!( | ||
| removed, | ||
| vec!["Understand Anything skill or plugin".to_string()], | ||
| "installing the skill must clear exactly the understand requirement\n\ | ||
| before={before:?}\nafter={after:?}" | ||
| ); | ||
| assert!( | ||
| after.iter().all(|entry| before.contains(entry)), | ||
| "installing the skill must not introduce new missing entries" | ||
| ); | ||
|
|
||
| fs::remove_dir_all(root).ok(); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Cover the complete strict-mode contract.
doctor_bootstrap discards Output.status, so this test cannot detect a regression from exit code 1 to exit code 0. The test also does not assert absent["ok"] == false or verify that a discovered plugin clears the requirement.
Return the exit status with the JSON result. Assert that the absent state exits with code 1 and has ok: false. Add a plugin fixture that removes the same missing entry.
As per coding guidelines, Rust changes require focused cargo test coverage plus the relevant integration-contract checks.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/code-intel-cli/tests/doctor_envelope.rs` around lines 331 - 418,
Update doctor_bootstrap to return both the parsed JSON and process exit status
from Output, then assert the absent case exits with code 1 and reports ok:
false. Extend require_understand_reports_missing_understand_anything with a
discovered plugin fixture and verify it removes the same “Understand Anything
skill or plugin” missing entry without introducing others. Run focused cargo
test coverage and the relevant integration-contract checks.
Source: Coding guidelines
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
问题
--require-understand/--doctor-require-understand从来没有检查过 Understand Anything。missing_list（crates/code-intel-cli/src/doctor_bootstrap/mod.rs）里唯一消费options.require_understand的两个分支只看/graphProvider/sourceFound和/graphProvider/cargoFound。这两个检查的是crates/code-intel-cli/src/graph.rs和crates/code-intel-cli/Cargo.toml在不在 —— 在 pipeline checkout 内部恒真。而同一份文档里算出来的understandAnything块（skill / plugin 探测）被发布之后再也没有任何人读过。复现（修复前，本机没装 Understand Anything）:
同一份文档一边说
ok: true，一边说什么都没找到。被退休的 PowerShell 探针有同样的洞（
archive/check-code-intel-tools.ps1里$RequireUnderstand也只 gategraphProvider，understandAnything只进输出不进$missing），Rust 端口把缺陷一起忠实搬运了过来。口径依据
按 installer 自己的
RequireUnderstand检查对齐 ——legacy/install-code-intel-pipeline.ps1:1227用完全相同的候选路径把 "skill:Understand Anything" 标成 required，补救文案是 "Install or link the Understand Anything skill/plugin"。Understand Anything 以 agent skill 或 plugin 目录两种形态发布，任一存在即满足，两者皆无才进missing。改动
missing_list增加一条分支：require_understand且 skill/plugin 都没找到 → push"Understand Anything skill or plugin"。graphProvider检查原样保留，不把当前的绿灯换成错误的红灯。toolchainDigests随mod.rs改动 repin，并用sha256sum手工核对（bug(repin): operationTrace 与 evidenceIds 内嵌 digest 不在扫描面——clean 报告失真 #133 说 repin 对部分 digest 盲）。修复后同一条命令：退出 1、
ok: false、missing: ["Understand Anything skill or plugin"]，与它自己发布的understandAnything块一致。测试
两条测试，都验证过 revert 即红：
require_understand_reports_missing_understand_anything（tests/doctor_envelope.rs）—— 端到端跑真实二进制。用USERPROFILE/HOME指向 scratch home 使探测结果确定，不依赖开发机/CI runner 上装了什么。scratch pipeline root 特意把graph.rs+Cargo.toml造出来，让旧的两条检查都为真 —— 正是要证明只有它们拦不住。断言是差分式的（scratch root 本来就缺 pipeline script / config）：装上 skill 后missing恰好少掉一条、且不多出任何一条。Revert 后失败：
missing_list_preserves_the_retired_scripts_wording_and_order（既有单测，按 finding 建议扩展）—— 锁住新条目的措辞与顺序。Revert 后同样失败。门禁
cargo test --workspace --locked：2959 passed（51 suites）cargo fmt --check：cleanrun execute ... --final-name dogfood-self-scan）：outcome: completed,exitCode: 0,failures: []—— god-file 棘轮通过（mod.rs786 非空行 / 上限 800，未新增函数）没有削弱、豁免或改名任何门禁。
范围外（已知、未改）
render_human的 "external Understand fallback" 那一行用的是 AND（skillFound && pluginFound）判 OK/MISSING，与本 PR 采用的 OR 口径不一致 —— 只装 skill 的机器上读者会看到 MISSING 而missing列表是空的。这是修复前就存在的展示层不一致，与本 finding 无关，没有一并改动。