fix(install): pin CODE_INTEL_HOME for installer subprocess; harden test-suite hermeticity - #233
Merged
Merged
Conversation
|
✅ Health of changed files: 3.9 (unchanged) 📋 At a glance Files & modules (2)
✅ Health gate: passed 📌 Before you merge
🔎 More signals (3)🗺️ Change map flowchart LR
subgraph PR ["Changed in this PR (1 with dependents)"]
f_crates_code_intel_cli_src_file_gate_walk_rs[".../file_gate/walk.rs 🔥"]:::changed
end
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
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 (1 direct dependent, from the last indexed snapshot). Dashed: history/tests. 🔥 Hotspots touched (5)
2 more
💀 Dead code (1 finding)
📊 See the full report for this PR |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (22)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR pins installer environment variables to the selected release root, hardens filesystem and Git-based tests against host-specific state, replaces fragile shallow-clone setup, ignores ChangesEnvironment and test hermeticity
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant bootstrap.run
participant command_result
participant installer
bootstrap.run->>bootstrap.run: Pin CODE_INTEL_HOME and prepend data-root bin to PATH
bootstrap.run->>command_result: Pass prepared environment
command_result->>installer: Invoke installer with pinned environment
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
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.
Actionable comments posted: 2
🧹 Nitpick comments (1)
crates/code-intel-cli/tests/snapshot_identity.rs (1)
476-483: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueMake the clone helper hermetic.
The local
git()helper atcrates/code-intel-cli/tests/snapshot_identity.rs:29-42inherits the parent Git process state. The clone atcrates/code-intel-cli/tests/snapshot_identity.rs:476-483can therefore pick up external Git configuration and alternate-object directory settings. Reuse therepository_git()-style cleanup fromcrates/code-intel-cli/tests/internalization_record.rs:1282-1300or share one fixture Git command builder.🤖 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 476 - 483, Make the local git helper used by the clone in snapshot_identity hermetic by applying the same repository_git-style environment/config cleanup used in internalization_record, or reuse a shared fixture Git command builder. Update the helper around git and ensure the clone invoked in the snapshot identity test cannot inherit external Git configuration or alternate-object directory settings.
🤖 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-50: Add focused tests around walk_candidates covering a removed
nested directory, a removed directory entry, missing file metadata, and
propagation of non-NotFound I/O errors. Ensure the relevant integration cases
invoke file_gate::evaluate and assert transient NotFound conditions are
tolerated while other I/O failures are returned.
- Around line 26-38: Preserve NotFound-as-empty handling only for directories
discovered during recursion, but make the initial directory read in
walk_candidates return an error when the scan root is missing. Distinguish the
root invocation from recursive walks and keep other I/O errors propagating
through the existing read-directory error path, ensuring evaluate cannot accept
a missing root as an empty GateReport.
---
Nitpick comments:
In `@crates/code-intel-cli/tests/snapshot_identity.rs`:
- Around line 476-483: Make the local git helper used by the clone in
snapshot_identity hermetic by applying the same repository_git-style
environment/config cleanup used in internalization_record, or reuse a shared
fixture Git command builder. Update the helper around git and ensure the clone
invoked in the snapshot identity test cannot inherit external Git configuration
or alternate-object directory settings.
🪄 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: 6f74cfbc-e1fa-462a-bde5-e0312d0d416d
📒 Files selected for processing (10)
.gitignoreCHANGELOG.mdcrates/code-intel-cli/src/file_gate/walk.rscrates/code-intel-cli/tests/internalization_record.rscrates/code-intel-cli/tests/native_code_evidence.rscrates/code-intel-cli/tests/snapshot_identity.rsorchestration/acceptance/native-code-evidence-candidate.jsonorchestration/internalization/git.jsonorchestration/internalization/native-code-evidence.jsonskills/code-intel-pipeline/scripts/bootstrap.py
Comment on lines
+26
to
+38
| 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(()); | ||
| } | ||
| Err(error) => return Err(format!("read {}: {error}", directory.display())), | ||
| }; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 10 \
'fn verify_identity|verify_identity\(|walk_candidates\(|governed_visible_files\(|GateReport' \
crates/code-intel-cli/src crates/code-intel-cli/testsRepository: 2233admin/code-intel-pipeline
Length of output: 35044
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file_gate/mod.rs relevant call sites =="
rg -n -C 8 'file_gate::evaluate|evaluate\(&repo|GateReport|governed_visible_files|verify_identity\(\)' crates/code-intel-cli/src crates/code-intel-cli/tests crates/code-intel-cli/src/file_gate/mod.rs crates/code-intel-cli/src/file_gate/report.rs
echo
echo "== walk_candidates/governed_visible_files implementation =="
cat -n crates/code-intel-cli/src/file_gate/walk.rs | sed -n '1,130p'
echo
echo "== report/evaluate sections =="
cat -n crates/code-intel-cli/src/file_gate/report.rs | sed -n '1,110p'
cat -n crates/code-intel-cli/src/file_gate/mod.rs | sed -n '137,236p'Repository: 2233admin/code-intel-pipeline
Length of output: 50385
Keep a missing scan root as an error.
If the root disappears, walk_candidates(repo, repo, ...) returns Ok(()), evaluate builds a zero-candidate GateReport, and verify_identity() accepts it. That reaches both sentrux scan and DSM (sentrux_analysis::source_inventory). Return an error when the initial directory is missing, or otherwise make callers reject an empty report.
Suggested boundary fix
- Err(error) if error.kind() == std::io::ErrorKind::NotFound => {
+ Err(error)
+ if error.kind() == std::io::ErrorKind::NotFound && directory != root =>📝 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.
Suggested change
| 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(()); | |
| } | |
| Err(error) => return Err(format!("read {}: {error}", directory.display())), | |
| }; | |
| 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(()); | |
| } | |
| Err(error) => return Err(format!("read {}: {error}", directory.display())), | |
| }; |
🤖 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 - 38, Preserve
NotFound-as-empty handling only for directories discovered during recursion, but
make the initial directory read in walk_candidates return an error when the scan
root is missing. Distinguish the root invocation from recursive walks and keep
other I/O errors propagating through the existing read-directory error path,
ensuring evaluate cannot accept a missing root as an empty GateReport.
Comment on lines
+26
to
+50
| 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(()); | ||
| } | ||
| Err(error) => return Err(format!("read {}: {error}", directory.display())), | ||
| }; | ||
| for entry in entries { | ||
| let entry = entry.map_err(|error| format!("read {}: {error}", directory.display()))?; | ||
| let entry = match entry { | ||
| Ok(entry) => entry, | ||
| Err(error) if error.kind() == std::io::ErrorKind::NotFound => continue, | ||
| Err(error) => return Err(format!("read {}: {error}", directory.display())), | ||
| }; | ||
| let path = entry.path(); | ||
| let file_type = entry | ||
| .file_type() | ||
| .map_err(|error| format!("inspect {}: {error}", path.display()))?; | ||
| let file_type = match entry.file_type() { | ||
| Ok(file_type) => file_type, | ||
| Err(error) if error.kind() == std::io::ErrorKind::NotFound => continue, | ||
| Err(error) => return Err(format!("inspect {}: {error}", path.display())), | ||
| }; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 8 \
'walk_candidates|governed_visible_files|NotFound|read_dir|file_type' \
crates/code-intel-cli/src crates/code-intel-cli/testsRepository: 2233admin/code-intel-pipeline
Length of output: 50385
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files crates/code-intel-cli src crates/code-intel-cli/src file_gate walk | sed -n '1,200p'
echo
echo "== file_gate files =="
git ls-files | rg '(^|/)crates/code-intel-cli/(src/test|tests|src/file_gate/walk\.rs|src/file_gate/mod\.rs|src/file_gate/test)' | sed -n '1,200p'
echo
echo "== file_gate walk/mod tests and relevant symbols =="
rg -n \
'#\[test\]|walk_candidates|governed_visible_files|file_gate::evaluate|gate::evaluate|GATE|GateConfig|FileGate|evaluate' \
crates/code-intel-cli/src/file_gate crates/code-intel-cli/tests crates/code-intel-cli/src 2>/dev/null | sed -n '1,260p'Repository: 2233admin/code-intel-pipeline
Length of output: 38216
Add focused coverage for the new removal branches.
The existing file_gate tests cover normal and configuration paths but not the transient removal cases in walk_candidates. Add targeted tests for a missing nested directory, a missing directory entry, missing file metadata, and propagation of a different I/O error, with file_gate::evaluate in the relevant integration contract.
🤖 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 - 50, Add
focused tests around walk_candidates covering a removed nested directory, a
removed directory entry, missing file metadata, and propagation of non-NotFound
I/O errors. Ensure the relevant integration cases invoke file_gate::evaluate and
assert transient NotFound conditions are tolerated while other I/O failures are
returned.
Source: Coding guidelines
2233admin
force-pushed
the
fix/test-hermeticity-followup
branch
2 times, most recently
from
August 8, 2026 05:43
c0d9704 to
53b59d1
Compare
…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.
2233admin
force-pushed
the
fix/test-hermeticity-followup
branch
from
August 8, 2026 11:00
53b59d1 to
12f975d
Compare
2233admin
added a commit
that referenced
this pull request
Aug 8, 2026
Pulls in #233 (pin CODE_INTEL_HOME for installer subprocess, harden test-suite hermeticity), which touched snapshot_identity.rs and native_code_evidence.rs. Resolves conflicts in git.json and native-code-evidence.json by taking origin/main's conformance-sha256 (the file each test actually pins changed on main's side) while keeping this branch's correct measurement-sha256; regenerates the e07-native-code retirement packet wholesale via New-NativeCodeRetirementPacket.ps1 rather than hand-merging it. repin --write then resynced the resulting cascade across all six records that share c03-r05-r12-measurements.json. Verified via the atomic-capability-contract test, the full retirement-packet suite (8 packets, 2 audits, 0 known-blocked), and the internalization_record test suite (58/58) -- all green.
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.
Follow-up extracted from the closed #227 — the #218 fix itself landed via #228. This PR carries the parts #228 did not cover.
1. bootstrap.py: pin CODE_INTEL_HOME for the installer subprocess
The installer persists
CODE_INTEL_HOMEinto the user environment and reads it from the child process env first. Previously the installer ran without a pinned env, so a caller shell exporting an MSYS-style/d/projects/...value got it written verbatim into the user registry asC:\d\projects\..., poisoning every later run. Now installer and doctor both run with the same env pinned to the release root.2. Test-suite hermeticity on developer machines
sentrux_gatecycle checks andtool_pathtest temp-dir cleanup.core.excludesFileso a user global ignore (e.g.*.bin) can't blockgit add binary.bin.CODE_INTEL_HOMEpointing at an installed release's stale manifest can't poison the comparison run..git/shallowboundary instead offile://clone, which fails under Windows 8.3 short temp names (git parsesfile:///C:/...as a relative/C:/...path).Verification
cargo test(MSVC toolchain, clean target): 3827 passed / 0 failedcode-intel repincleanlegacy/tools/check-hardcoded-paths.ps1OK