fix(capability): reject bin forwarder manifests, fall back to CODE_INTEL_HOME (#218) - #227
fix(capability): reject bin forwarder manifests, fall back to CODE_INTEL_HOME (#218)#2272233admin wants to merge 1 commit into
Conversation
|
Warning Review limit reached
Next review available in: 24 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (98)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe change centralizes manifest-root resolution, handles installer environment values explicitly, tolerates disappearing filesystem entries, improves test isolation, and refreshes capability and conformance digests. ChangesPipeline resolution and validation
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
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 |
|
✅ Health of changed files: 3.7 (unchanged) 📋 At a glance Files & modules (2)
✅ Health gate: passed 📌 Before you merge
🔎 More signals (4)🗺️ Change map flowchart LR
subgraph PR ["Changed in this PR (6 with dependents)"]
f_crates_code_intel_cli_src_capability_rs[".../src/capability.rs 🔥"]:::changed
f_crates_code_intel_cli_src_orchestration_rs[".../src/orchestration.rs 🔥"]:::changed
f_crates_code_intel_cli_src_capability_inventory_rs[".../src/capability_inventory.rs"]:::changed
f_crates_code_intel_cli_src_file_gate_walk_rs[".../file_gate/walk.rs"]:::changed
f_crates_code_intel_cli_tests_artifact_ref_rs[".../tests/artifact_ref.rs"]:::changed
f_crates_code_intel_cli_tests_capability_exec_rs[".../tests/capability_exec.rs"]:::changed
end
f_crates_code_intel_cli_src_admissibility_rs[".../src/admissibility.rs"]
f_crates_code_intel_cli_src_capability_rs --> f_crates_code_intel_cli_src_admissibility_rs
f_crates_code_intel_cli_src_artifact_index_rs[".../src/artifact_index.rs"]
f_crates_code_intel_cli_src_capability_rs --> f_crates_code_intel_cli_src_artifact_index_rs
f_crates_code_intel_cli_src_builtin_provider_evidence_rs[".../src/builtin_provider_evidence.rs"]
f_crates_code_intel_cli_src_capability_rs --> f_crates_code_intel_cli_src_builtin_provider_evidence_rs
f_crates_code_intel_cli_src_compatibility_retirement_gate_rs[".../src/compatibility_retirement_gate.rs"]
f_crates_code_intel_cli_src_capability_rs --> f_crates_code_intel_cli_src_compatibility_retirement_gate_rs
f_crates_code_intel_cli_src_main_rs[".../src/main.rs"]
f_crates_code_intel_cli_src_orchestration_rs --> f_crates_code_intel_cli_src_main_rs
f_crates_code_intel_cli_src_anchor_verification_mod_rs[".../anchor_verification/mod.rs"]
f_crates_code_intel_cli_src_capability_inventory_rs --> f_crates_code_intel_cli_src_anchor_verification_mod_rs
f_crates_code_intel_cli_src_edit_impact_rs[".../src/edit_impact.rs"]
f_crates_code_intel_cli_src_capability_inventory_rs --> f_crates_code_intel_cli_src_edit_impact_rs
f_crates_code_intel_cli_src_capability_inventory_rs --> f_crates_code_intel_cli_src_main_rs
f_crates_code_intel_cli_src_file_gate_mod_rs[".../file_gate/mod.rs"]
f_crates_code_intel_cli_src_file_gate_walk_rs --> f_crates_code_intel_cli_src_file_gate_mod_rs
f_crates_code_intel_cli_tests_artifact_ref_rs --> f_crates_code_intel_cli_src_main_rs
f_crates_code_intel_cli_tests_capability_exec_rs --> f_crates_code_intel_cli_src_main_rs
more(["+15 more dependents"])
PR --> more
w_orchestration_internalization_rg_json(["⚠️ orchestration/internalization/rg.json changed together 11×, not in PR"]):::warn
f_crates_code_intel_cli_tests_capability_exec_rs -.- w_orchestration_internalization_rg_json
w_orchestration_integrations_json(["⚠️ orchestration/integrations.json changed together 11×, not in PR"]):::warn
f_crates_code_intel_cli_src_capability_inventory_rs -.- w_orchestration_integrations_json
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 (23 direct dependents, from the last indexed snapshot). Dashed: history/tests. 🔥 Hotspots touched (5)
2 more
🔗 Hidden coupling (2 files)
💀 Dead code (1 finding)
📊 See the full report for this PR |
Code Intel change risk
Top signals
revspec: |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
crates/code-intel-cli/tests/snapshot_identity.rs (1)
477-481: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winGet
HEADthrough Git instead of reading.git/HEAD.Read the local
.git/HEADfile when the repository uses reftable storage andHEADpoints to an invalid ref name; the test needs to create a valid repository from the clone instead of parsing the ref target.Run
git -C &full rev-parse HEAD, then write that ID to.git/shallow.🤖 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/snapshot_identity.rs` around lines 477 - 481, Update the shallow-repository setup around the shallow variable to obtain the current commit via Git using rev-parse HEAD in the cloned repository, rather than reading and parsing .git/HEAD and its referenced ref file. Write the returned commit ID, trimmed, followed by a newline to .git/shallow, preserving the existing shallow assignment.
🤖 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/src/file_gate/walk.rs`:
- Around line 26-35: The read_dir NotFound handling in the walk_candidates flow
must reject a missing scan root while still treating disappeared descendant
directories as empty. When directory == root, propagate an error; retain the
existing Ok(()) behavior otherwise. Add focused cargo tests covering both a
missing root and a disappearing child directory, plus the relevant
integration-contract checks.
---
Nitpick comments:
In `@crates/code-intel-cli/tests/snapshot_identity.rs`:
- Around line 477-481: Update the shallow-repository setup around the shallow
variable to obtain the current commit via Git using rev-parse HEAD in the cloned
repository, rather than reading and parsing .git/HEAD and its referenced ref
file. Write the returned commit ID, trimmed, followed by a newline to
.git/shallow, preserving the existing shallow assignment.
🪄 Autofix
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: 211ebedb-5088-467f-b810-29ae7805f7f5
📒 Files selected for processing (20)
CHANGELOG.mdcrates/code-intel-cli/src/capability.rscrates/code-intel-cli/src/capability_inventory.rscrates/code-intel-cli/src/doctor_adapter.rscrates/code-intel-cli/src/file_gate/walk.rscrates/code-intel-cli/src/orchestration.rscrates/code-intel-cli/tests/artifact_ref.rscrates/code-intel-cli/tests/capability_exec.rscrates/code-intel-cli/tests/internalization_record.rscrates/code-intel-cli/tests/native_code_evidence.rscrates/code-intel-cli/tests/snapshot_identity.rslegacy/install-code-intel-pipeline.ps1legacy/run-code-intel.ps1orchestration/acceptance/native-code-evidence-candidate.jsonorchestration/integrations.jsonorchestration/internalization/ast-grep.jsonorchestration/internalization/git.jsonorchestration/internalization/native-code-evidence.jsonorchestration/internalization/rg.jsonskills/code-intel-pipeline/scripts/bootstrap.py
| let entries = match fs::read_dir(directory) { | ||
| Ok(entries) => entries, | ||
| Err(error) if error.kind() == std::io::ErrorKind::NotFound => { | ||
| // The directory vanished between enumeration and now (for example | ||
| // a parallel test or tool created and removed a temp directory | ||
| // under `target/` while this walk was in flight). Treat it as | ||
| // empty rather than failing the whole scan: concurrent cleanup is | ||
| // not a repository integrity problem. Other IO errors (permission, | ||
| // hardware) still propagate. | ||
| return Ok(()); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Reject NotFound for the scan root.
evaluate calls walk_candidates(repo, repo, ...). If repo disappears before this call, Lines 26-35 return success and produce a report with zero candidates. This can make a failed scan appear clean.
Return an error when directory == root. Keep the current skip behavior only for descendant directories. Add focused coverage for both a missing root and a disappearing child directory.
Proposed fix
- Err(error) if error.kind() == std::io::ErrorKind::NotFound => {
+ Err(error)
+ if error.kind() == std::io::ErrorKind::NotFound && directory != root =>
+ {
// The directory vanished between enumeration and now (for example
// a parallel test or tool created and removed a temp directory
// under `target/` while this walk was in flight). Treat it asAs per coding guidelines, “Rust changes require focused cargo test coverage plus the relevant integration-contract checks.”
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let entries = match fs::read_dir(directory) { | |
| Ok(entries) => entries, | |
| Err(error) if error.kind() == std::io::ErrorKind::NotFound => { | |
| // The directory vanished between enumeration and now (for example | |
| // a parallel test or tool created and removed a temp directory | |
| // under `target/` while this walk was in flight). Treat it as | |
| // empty rather than failing the whole scan: concurrent cleanup is | |
| // not a repository integrity problem. Other IO errors (permission, | |
| // hardware) still propagate. | |
| return Ok(()); | |
| let entries = match fs::read_dir(directory) { | |
| Ok(entries) => entries, | |
| Err(error) | |
| if error.kind() == std::io::ErrorKind::NotFound && directory != root => | |
| { | |
| // The directory vanished between enumeration and now (for example | |
| // a parallel test or tool created and removed a temp directory | |
| // under `target/` while this walk was in flight). Treat it as | |
| // empty rather than failing the whole scan: concurrent cleanup is | |
| // not a repository integrity problem. Other IO errors (permission, | |
| // hardware) still propagate. | |
| return Ok(()); |
🤖 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/src/file_gate/walk.rs` around lines 26 - 35, The
read_dir NotFound handling in the walk_candidates flow must reject a missing
scan root while still treating disappeared descendant directories as empty. When
directory == root, propagate an error; retain the existing Ok(()) behavior
otherwise. Add focused cargo tests covering both a missing root and a
disappearing child directory, plus the relevant integration-contract checks.
Source: Coding guidelines
317ba14 to
edb7458
Compare
…TEL_HOME (#218) Installer copies orchestration/integrations.json into <bin>/orchestration/ as a forwarder, but discover_manifest prefers that copy (exe ancestor walk) and root_for_manifest treats <bin> as the repository root, so every entrypoint resolves under <bin> and fails (~40 'entrypoint missing' errors). The pipeline then fails at the doctor node on any fresh Windows install. - capability.rs: add is_repo_like/manifest_root; auto-discovered manifests must resolve to a real checkout (pipeline.config.json/Cargo.toml/.git), otherwise discovery falls through to CODE_INTEL_HOME. manifest_root_from is the pure, env-injected variant so tests avoid racing on process env. - orchestration.rs: root_for_manifest distinguishes explicit --manifest (honored as-is; doctor_envelope regression guard) from auto-discovery (requires checkout). - doctor_adapter.rs / capability_inventory.rs: pipeline_root uses shared manifest_root. - bootstrap.py: installer subprocess now runs with CODE_INTEL_HOME pinned to the release root, so a caller shell's MSYS-style /d/... value is no longer written verbatim into the user registry as C:\d\... Also hardens the test suite against developer-machine git/env state: - file_gate/walk.rs tolerates directories vanishing mid-walk (NotFound), fixing a parallel race between sentrux cycle checks and tool_path temp dirs - internalization_record fixture git clears core.excludesFile (user global ignore no longer blocks git add binary.bin) - native_code_evidence clears PIPELINE_VARS for the legacy pwsh facade (shell CODE_INTEL_HOME no longer points at an installed release's stale manifest) - snapshot_identity builds the shallow repo via .git/shallow boundary instead of file:// clone, which fails under Windows 8.3 short temp names Verified: cargo test 3794 passed / 0 failed; code-intel doctor bootstrap passes with the bin forwarder present; orchestrate Validate clean; repin clean; check-hardcoded-paths OK.
edb7458 to
cea9338
Compare
|
处置判词(撞车对账,依 DR-0004 撤退方留痕):#218 的修法由 #228 承接——两案差异见 docs/decisions/DR-0003(probe 验不变量而非形态;manifest 与 root 不分家;orchestrate 补 env var 档)。本 PR 请 rebase 成只保留五件独立真货:bootstrap.py MSYS CODE_INTEL_HOME 毒化修复、file_gate/walk.rs NotFound 竞态容错(#178 根修)、internalization_record 清 core.excludesFile、native_code_evidence 清 PIPELINE_VARS、snapshot_identity 绕 8.3 短路径——并按保留集重算 digest pin。这五件每一件都该进主干。 |
…st-suite hermeticity Follow-up extracted from the closed #227 (the #218 fix itself landed via #228). This branch carries the parts of that work #228 did not cover: - bootstrap.py: the installer subprocess now runs with CODE_INTEL_HOME pinned to the release root (was inheriting the caller shell). A git-bash MSYS-style CODE_INTEL_HOME like /d/projects/... was written verbatim into the user registry as C:\d\projects\... and poisoned every later run; now installer and doctor both use the same pinned environment. - file_gate/walk.rs: tolerate a directory vanishing mid-walk (NotFound) — treats it as empty instead of failing the whole scan. Fixes a parallel race between sentrux cycle checks and tool_path test temp-dir cleanup on developer machines. - internalization_record: fixture git clears core.excludesFile so a user global ignore (e.g. *.bin) can no longer block git add binary.bin. - native_code_evidence: the legacy pwsh facade clears PIPELINE_VARS so a shell CODE_INTEL_HOME pointing at an installed release's stale manifest cannot poison the comparison run. - snapshot_identity: builds the shallow repo via a .git/shallow boundary instead of file:// clone, which fails under Windows 8.3 short temp names (git parses file:///C:/... as a relative /C:/... path). Verified: cargo test 3827 passed / 0 failed (MSVC toolchain, clean target); repin clean; check-hardcoded-paths OK.
…st-suite hermeticity Follow-up extracted from the closed #227 (the #218 fix itself landed via #228). This branch carries the parts of that work #228 did not cover: - bootstrap.py: the installer subprocess now runs with CODE_INTEL_HOME pinned to the release root (was inheriting the caller shell). A git-bash MSYS-style CODE_INTEL_HOME like /d/projects/... was written verbatim into the user registry as C:\d\projects\... and poisoned every later run; now installer and doctor both use the same pinned environment. - file_gate/walk.rs: tolerate a directory vanishing mid-walk (NotFound) — treats it as empty instead of failing the whole scan. Fixes a parallel race between sentrux cycle checks and tool_path test temp-dir cleanup on developer machines. - internalization_record: fixture git clears core.excludesFile so a user global ignore (e.g. *.bin) can no longer block git add binary.bin. - native_code_evidence: the legacy pwsh facade clears PIPELINE_VARS so a shell CODE_INTEL_HOME pointing at an installed release's stale manifest cannot poison the comparison run. - snapshot_identity: builds the shallow repo via a .git/shallow boundary instead of file:// clone, which fails under Windows 8.3 short temp names (git parses file:///C:/... as a relative /C:/... path). Verified: cargo test 3827 passed / 0 failed (MSVC toolchain, clean target); repin clean; check-hardcoded-paths OK.
…st-suite hermeticity Follow-up extracted from the closed #227 (the #218 fix itself landed via #228). This branch carries the parts of that work #228 did not cover: - bootstrap.py: the installer subprocess now runs with CODE_INTEL_HOME pinned to the release root (was inheriting the caller shell). A git-bash MSYS-style CODE_INTEL_HOME like /d/projects/... was written verbatim into the user registry as C:\d\projects\... and poisoned every later run; now installer and doctor both use the same pinned environment. - file_gate/walk.rs: tolerate a directory vanishing mid-walk (NotFound) — treats it as empty instead of failing the whole scan. Fixes a parallel race between sentrux cycle checks and tool_path test temp-dir cleanup on developer machines. - internalization_record: fixture git clears core.excludesFile so a user global ignore (e.g. *.bin) can no longer block git add binary.bin. - native_code_evidence: the legacy pwsh facade clears PIPELINE_VARS so a shell CODE_INTEL_HOME pointing at an installed release's stale manifest cannot poison the comparison run. - snapshot_identity: builds the shallow repo via a .git/shallow boundary instead of file:// clone, which fails under Windows 8.3 short temp names (git parses file:///C:/... as a relative /C:/... path). Verified: cargo test 3827 passed / 0 failed (MSVC toolchain, clean target); repin clean; check-hardcoded-paths OK.
…st-suite hermeticity (#233) Follow-up extracted from the closed #227 (the #218 fix itself landed via #228). This branch carries the parts of that work #228 did not cover: - bootstrap.py: the installer subprocess now runs with CODE_INTEL_HOME pinned to the release root (was inheriting the caller shell). A git-bash MSYS-style CODE_INTEL_HOME like /d/projects/... was written verbatim into the user registry as C:\d\projects\... and poisoned every later run; now installer and doctor both use the same pinned environment. - file_gate/walk.rs: tolerate a directory vanishing mid-walk (NotFound) — treats it as empty instead of failing the whole scan. Fixes a parallel race between sentrux cycle checks and tool_path test temp-dir cleanup on developer machines. - internalization_record: fixture git clears core.excludesFile so a user global ignore (e.g. *.bin) can no longer block git add binary.bin. - native_code_evidence: the legacy pwsh facade clears PIPELINE_VARS so a shell CODE_INTEL_HOME pointing at an installed release's stale manifest cannot poison the comparison run. - snapshot_identity: builds the shallow repo via a .git/shallow boundary instead of file:// clone, which fails under Windows 8.3 short temp names (git parses file:///C:/... as a relative /C:/... path). Verified: cargo test 3827 passed / 0 failed (MSVC toolchain, clean target); repin clean; check-hardcoded-paths OK. Co-authored-by: Curry <curry@test.com>
…230) * docs(decisions): stand up decision records + agent work-intake rules Five records born from the 2026-08-08 install-campaign postmortem, where the core finding was not any single bug but the machine that produces them: a verification tool whose own verification points inward. - DR-0001 install-topology gate: every install-class bug's repro must join the install-smoke CI gate in the same PR. 3794 checkout-topology tests missed every installed-topology bug that shipped with v0.7.0 (#218, #216, skill-required, README platform claims). - DR-0002 version pins are floors (owner decree; the counterfactual is #208, a parallel session arguing exact pins after the decree existed only in chat). - DR-0003 manifest discovery precedence + entrypoint-probe semantics; probes validate the invariant (entrypoints resolve), not the shape (repo-like markers); manifest and root never split. - DR-0004 issue claim protocol (the counterfactual is #227/#228: two sessions, one issue, two incompatible fixes). - DR-0005 integration debt ceiling: >=5 open fix PRs stops feature work. The cure for the install campaign (#214) sat CLEAN and unmerged while GA shipped without it. AGENTS.md gains a 'Before starting any work' intake section wiring all five into every future session's first minute. Refs #99 #148 #180 #184 #206 #226 * test(repowise): isolate hook fixtures --------- Co-authored-by: Curry <curry@test.com>
Closes #218
Problem
On Windows v0.7.0, a fresh install fails at the doctor node:
The installer copies
orchestration/integrations.jsonto<bin>/orchestration/as a forwarder, butdiscover_manifest(exe ancestor walk) prefers that copy androot_for_manifesttreats<bin>as the repository root — every relative entrypoint resolves under<bin>and is missing (~40integration ... entrypoint missingerrors).Fix
is_repo_like/manifest_root; auto-discovered manifests must resolve to a real checkout (pipeline.config.json/Cargo.toml/.git), else discovery falls through toCODE_INTEL_HOME.manifest_root_fromis the pure env-injected variant so unit tests don't race on process env.root_for_manifestdistinguishes explicit--manifest(honored as-is; keeps doctor_envelope behavior) from auto-discovery (requires checkout).pipeline_rootuses the sharedmanifest_root.CODE_INTEL_HOMEpinned to the release root — a caller shell's MSYS-style/d/...value is no longer written verbatim to the user registry asC:\d\....Test-suite hardening (same PR, developer-machine flakiness)
file_gate/walk.rs: tolerate directories vanishing mid-walk (NotFound) — parallel race between sentrux cycle checks andtool_pathtemp dirs.internalization_record: fixture git clearscore.excludesFileso a user global ignore (e.g.*.bin) can't blockgit add binary.bin.native_code_evidence: legacy pwsh facade clears PIPELINE_VARS so a shellCODE_INTEL_HOMEpointing at an installed release's stale manifest can't poison the run.snapshot_identity: builds the shallow repo via a.git/shallowboundary instead offile://clone (fails under Windows 8.3 short temp names).Verification
cargo test: 3794 passed / 0 failedcode-intel doctor bootstrappasses with the bin forwarder present (and withoutCODE_INTEL_INTEGRATIONS_MANIFEST)orchestrate --action Validateclean on the forwarder copy (was ~40 errors)code-intel repinclean;legacy/tools/check-hardcoded-paths.ps1OK