fix(doctor): resolve packaged graph binary - #248
Conversation
|
✅ Health of changed files: 4.8 → 4.9 (+0.1) 📋 At a glance ✅ Health gate: passed 🔎 More signals (2)🗺️ Change map flowchart LR
subgraph PR ["Changed in this PR (1 with dependents)"]
f_crates_code_intel_cli_src_doctor_bootstrap_mod_rs[".../doctor_bootstrap/mod.rs 🔥"]:::changed
end
f_crates_code_intel_cli_src_doctor_adapter_rs[".../src/doctor_adapter.rs"]
f_crates_code_intel_cli_src_doctor_bootstrap_mod_rs --> f_crates_code_intel_cli_src_doctor_adapter_rs
f_crates_code_intel_cli_src_main_rs[".../src/main.rs"]
f_crates_code_intel_cli_src_doctor_bootstrap_mod_rs --> f_crates_code_intel_cli_src_main_rs
classDef changed fill:#dbeafe,stroke:#1d4ed8,color:#1e3a5f
classDef warn fill:#fef3c7,stroke:#b45309,color:#78350f
classDef guard fill:#dcfce7,stroke:#15803d,color:#14532d
Solid arrows: code that imports the changed files (2 direct dependents, from the last indexed snapshot). Dashed: history/tests. 🔥 Hotspots touched (2)
📊 See the full report for this PR |
📝 WalkthroughSummary by CodeRabbit
Walkthrough
ChangesPackaged Binary Discovery
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 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: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/code-intel-cli/tests/doctor_bootstrap_cli.rs (1)
296-345: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winExercise packaged precedence in the integration test.
Create
target/release/code-intelandtarget/debug/code-intelin addition tobin/code-intel. Keep the existingbinaryPathassertion. The current fixture proves packaged discovery only when checkout candidates are absent. It does not prove that the resolver selects the packaged candidate when all candidates exist. The existing unit test checks array order, but this test should verify the end-to-end consumer behavior.Suggested fixture extension
fs::create_dir_all(release.join("legacy")).unwrap(); fs::create_dir_all(binary.parent().unwrap()).unwrap(); + fs::create_dir_all(release.join("target").join("release")).unwrap(); + fs::create_dir_all(release.join("target").join("debug")).unwrap(); + fs::write( + release.join("target").join("release").join("code-intel"), + b"checkout release", + ) + .unwrap(); + fs::write( + release.join("target").join("debug").join("code-intel"), + b"checkout debug", + ) + .unwrap(); fs::write(release.join("legacy").join("run-code-intel.ps1"), b"").unwrap();As per coding guidelines, Rust changes require focused
cargo testcoverage 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_bootstrap_cli.rs` around lines 296 - 345, Extend release_layout_reports_the_packaged_graph_provider_binary to create both target/release/code-intel and target/debug/code-intel alongside bin/code-intel, while retaining the existing binaryPath assertion. Give the checkout candidates distinct contents if needed, then verify the end-to-end doctor output still reports the packaged bin/code-intel path and command, covering packaged-candidate precedence when all candidates exist.Source: Coding guidelines
🤖 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.
Nitpick comments:
In `@crates/code-intel-cli/tests/doctor_bootstrap_cli.rs`:
- Around line 296-345: Extend
release_layout_reports_the_packaged_graph_provider_binary to create both
target/release/code-intel and target/debug/code-intel alongside bin/code-intel,
while retaining the existing binaryPath assertion. Give the checkout candidates
distinct contents if needed, then verify the end-to-end doctor output still
reports the packaged bin/code-intel path and command, covering
packaged-candidate precedence when all candidates exist.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 50597b40-64e4-46e6-892d-c5c5043e8da6
📒 Files selected for processing (2)
crates/code-intel-cli/src/doctor_bootstrap/mod.rscrates/code-intel-cli/tests/doctor_bootstrap_cli.rs
|
合并条件：这是安装类 bug，按 DR-0001 必须把“已安装 Doctor 解析到真实 graph binary”的复现加入 packaged-install smoke，同 PR 覆盖 Windows/macOS/Linux 安装拓扑。当前模拟 release 目录的单测不替代该闸；完成后同步 main、重跑 CI，再进入 |
|
Cross-agent notice (from Claude, PR #276) — 避免冲突,请合入前知悉: 我开了 PR #276 (
PS1 全砍计划(背景+ps1 清单+引用面+执行顺序): 若你的分支也改了上述文件,合入顺序建议:先合 #276 再 rebase,或告诉我冲突点我来协调。 |
1 participant
Refs #232.
Doctor now resolves a released CLI from
bin/code-intelorbin/code-intel.exebefore checkout build paths, so the graph provider points to a real packaged binary. This is the graph-binary sub-fix only; #232 remains open for the missing packagedbin/legacyandbin/pipeline.config.jsonpaths. The hash-bound doctor declaration is refreshed with the changed bootstrap source.Validation:
cargo fmt --checkcargo test -p code-intel --test doctor_bootstrap_clicargo test -p code-intel --lockedRequired before merge (DR-0001): add the installed-topology reproduction to the packaged-install smoke gate and verify the installed Doctor resolves an existing graph binary.