fix: four self-audit defects — release-profile test gate, obsolete overlay verdict, skill-store ownership, dead resume duplicate - #67
Conversation
`cargo test --release` failed on `inventory_rejects_simulated_rg_extra_path_without_publication`: it expected exit 65 but got 0. The `CODE_INTEL_TEST_RG_EXTRA_PATH` hook it drives is `#[cfg(debug_assertions)]` in `capability_inventory.rs` so the shipped binary carries no inventory fault injection, but the test itself was unconditional. CI only runs `cargo test` (debug), so the failure never surfaced there. Gate the test on the same cfg as the hook it exercises, and document why. The contract under test — `verify_inventory_path_sets` refusing to publish on a set mismatch — is compiled into every profile; only the simulation is debug-only. Also silence the resulting release-profile `unused_mut` on `actual_baseline` with a targeted `cfg_attr` rather than dropping `mut`, which debug builds need.
… one `Repair-RepowiseThinkingBlockPatch` recognised two shapes of the installed repowise anthropic provider: our patched form, and the pre-fix `content=response.content[0].text`. Anything else was reported `install_failed: expected pattern not found`. Upstream repowise 0.32.0 fixed the ThinkingBlock crash on its own — it now iterates `response.content` and takes the first block with `.text`, which skips a ThinkingBlock (it carries `.thinking`). So every healthy machine has been reporting `install_failed` on every install run, which turns a real failure signal into noise we learn to scroll past. Add a `not_needed` verdict for the upstream-fixed shape, keep `install_failed` for a layout matching neither, and leave upstream source untouched in both read-only cases. The overlay stays until every supported machine runs repowise >= 0.32.0, since it is the only thing standing between an older pinned install and a silent `total_pages=0`. Covered by three regression cases (not_needed / installed / install_failed) that point the APPDATA-derived lookup at a scratch tree.
`Ensure-SkillLink` accepted any path with a `SKILL.md` in it. Agent hosts share those directories with other skill managers, so a manager we do not control can own the path and the installer will still report `OK skill:claude`. That is not hypothetical. On the machine this was found on, `~/.claude/skills/code-intel-pipeline` was a junction into an unrelated `~/.skillz` store holding a 2026-06-21 SKILL.md whose canonical paths all pointed at a legacy clone. Every `-RepairSkillLinks` run refreshed `.agents` and `.codex`, reported `OK skill:claude`, and left the skill Claude Code actually loads five weeks stale. Accept only a link that resolves to the bundled-skill source, or a plain copy whose SKILL.md bytes match it (link-less macOS/Linux installs). Report anything else as occupied. On repair, move the previous occupant aside rather than deleting it: unlink a reparse point, since the store it pointed at keeps every byte, and rename a real directory so a foreign skill stays recoverable. Also stop copying `__pycache__` / `*.pyc` into the installed skill. A single `bootstrap.cpython-313.pyc` left by a local `bootstrap.py` run made the byte-parity check report `skill:source` outdated forever, which is how a genuinely stale install stayed camouflaged.
…s extraction `main.rs` still carried `ResumeSummary`, `bool_at`, `existing_path`, `string_path`, `string_first`, `next_read`, a `#[cfg(test)]` `build_resume_summary`, and a verbatim copy of two resume contract tests. All of it duplicates `artifacts.rs` / `artifacts_tests.rs`, which took over the job: `cmd_resume` delegates to `artifacts::resume`, so nothing has been serving the copy in `main.rs`. Coverage is unchanged — `artifacts_tests.rs` already defines the same two contract tests under the same names. Also drop three unused imports, and move `MAX_JSON_DEPTH` into the single test that uses it rather than re-exporting it crate-wide from `capability.rs`. This does NOT chase the remaining ~100 dead-code warnings, and deliberately adds no `-D warnings` gate. The crate has no `lib.rs`; modules are shared by `#[path]` re-inclusion, so each instantiation warns about the slice of the file it happens not to use. Those warnings are structural, the count is not a debt metric, and deleting what one instantiation ignores breaks the one that does not. AGENTS.md now says so, next to a note on re-syncing the orchestration digest pins that source edits invalidate.
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
Warning Review limit reached
Next review available in: 19 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. 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 (2)
📝 WalkthroughWalkthroughThe PR removes stale Rust resume code, updates debug-only test behavior and digest pins, improves PowerShell skill and Repowise installer repairs, adds regression coverage, and documents verification and release changes. ChangesRust cleanup and contract updates
Installer repair behavior
Contract and evidence synchronization
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant TestHarness
participant Installer
participant RepowiseProvider
participant SkillStore
TestHarness->>Installer: run repair and skill-install scenarios
Installer->>RepowiseProvider: inspect provider response.content
RepowiseProvider-->>Installer: upstream layout
Installer->>SkillStore: validate, displace, or copy skill files
SkillStore-->>Installer: target and parity result
Installer-->>TestHarness: not_needed, installed, install_failed, or repair result
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![coderabbitai[bot]](https://avatars.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
overlays/repowise/README.md (1)
35-37: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLead the verification instructions with
code-intel.This new command example starts with
rg; document the compiledcode-intelequivalent first and retain the direct file search only as a legacy PowerShell compatibility fallback. As per coding guidelines, “New documentation and command examples should lead with the compiledcode-intelCLI and mention PowerShell only for existing compatibility paths.”🤖 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 `@overlays/repowise/README.md` around lines 35 - 37, Update the verification instructions in the README to lead with the compiled code-intel CLI equivalent for the Anthropic source search. Retain the existing rg PowerShell command only as a clearly labeled legacy compatibility fallback, preserving the same search targets and intent.Source: Coding guidelines
install-code-intel-pipeline.ps1 (1)
746-762: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winResolve relative link targets from the link location.
Path.GetFullPath($item.LinkTarget)resolves relative link targets from the current working directory, so an existing relative skill link is incorrectly treated as foreign under-RepairSkillLinks. UseResolveLinkTarget($true)and compareResolveLinkTarget(...).FullNameto the normalized target path instead.🤖 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 `@install-code-intel-pipeline.ps1` around lines 746 - 762, Update the link-detection logic in the surrounding skill-link validation function to resolve relative targets from the link location by using the item’s ResolveLinkTarget($true) result. Compare its FullName with the normalized target path, while preserving the existing fallback hash comparison and false-on-error behavior.
🤖 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 `@install-code-intel-pipeline.ps1`:
- Around line 894-902: The installation flow around
Get-BundledSkillRelativeFiles must preserve a foreign or outdated $InstalledPath
occupant after replacement instead of deleting its $backup; retain the moved
content under a recoverable .replaced-* name, matching Ensure-SkillLink
behavior, and add a regression fixture covering this replacement path.
In `@overlays/repowise/README.md`:
- Line 39: Update the “Status: superseded upstream” heading in the README to use
the actual verification date, July 28, 2026, or remove the verification date
entirely.
---
Nitpick comments:
In `@install-code-intel-pipeline.ps1`:
- Around line 746-762: Update the link-detection logic in the surrounding
skill-link validation function to resolve relative targets from the link
location by using the item’s ResolveLinkTarget($true) result. Compare its
FullName with the normalized target path, while preserving the existing fallback
hash comparison and false-on-error behavior.
In `@overlays/repowise/README.md`:
- Around line 35-37: Update the verification instructions in the README to lead
with the compiled code-intel CLI equivalent for the Anthropic source search.
Retain the existing rg PowerShell command only as a clearly labeled legacy
compatibility fallback, preserving the same search targets and intent.
🪄 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: aeeda794-a7a5-44ff-8b01-b076ccdb67e1
📒 Files selected for processing (16)
AGENTS.mdCHANGELOG.mdcrates/code-intel-cli/src/capability.rscrates/code-intel-cli/src/capability_inventory.rscrates/code-intel-cli/src/main.rscrates/code-intel-cli/src/sentrux_gate.rscrates/code-intel-cli/tests/artifact_ref.rscrates/code-intel-cli/tests/capability_exec.rsinstall-code-intel-pipeline.ps1orchestration/integrations.jsonorchestration/internalization/ast-grep.jsonorchestration/internalization/rg.jsonorchestration/internalization/sentrux.jsonoverlays/repowise/README.mdrun-code-intel.ps1scripts/tests/test-regression-fixes.ps1
💤 Files with no reviewable changes (2)
- crates/code-intel-cli/src/sentrux_gate.rs
- crates/code-intel-cli/src/main.rs
`cross-platform-smoke (windows-latest)` failed on `tools/check-hardcoded-paths.ps1`. The scanner strips `$env:VAR` references before matching, so it flags a scanned Windows user-directory variable named bare — and the new helper's comment named one in prose. Reworded to the `$env:`-prefixed form, which is more precise anyway. Documented the trap in AGENTS.md, since prose and comments are exactly where it is easy to miss: the scan covers tracked `.md` too, and the first draft of that very note tripped the scan for the same reason.
1 participant
Four defects found by auditing the pipeline against itself at
v0.7.0-beta.1, plus the machine-state repairs they exposed. No product behavior changes beyond the defects named below.What was wrong
cargo test --releasefailed on a clean tree.inventory_rejects_simulated_rg_extra_path_without_publicationexpected exit 65 and got 0. TheCODE_INTEL_TEST_RG_EXTRA_PATHhook it drives is#[cfg(debug_assertions)]incapability_inventory.rs, deliberately, so the shipped binary carries no inventory fault injection — but the test itself was unconditional. CI only runscargo test(debug) and thencargo build --release, so this never surfaced there.Gated the test on the same cfg as the hook it exercises. The contract it guards (
verify_inventory_path_setsrefusing to publish on a set mismatch) is compiled into every profile; only the simulation is debug-only.Every install run reported
repowise-thinking-patch: install_failed.Repair-RepowiseThinkingBlockPatchknew two shapes of the installed repowise anthropic provider: our patched form, and the pre-fixcontent=response.content[0].text. Anything else was a failure.Upstream repowise 0.32.0 fixed the ThinkingBlock crash itself — it iterates
response.contentand takes the first block with.text, skipping a ThinkingBlock (which carries.thinking). So a healthy machine reportedinstall_failedon every run, which turns a real failure signal into noise. Added anot_neededverdict for the upstream-fixed shape;install_failedstill fires for a layout matching neither. The overlay stays until every supported machine runs repowise >= 0.32.0, since it is the only thing between an older pinned install and a silenttotal_pages=0.skill:claudeverified that a SKILL.md existed, not whose it was.Agent hosts share those directories with other skill managers. On the machine this was found on,
~/.claude/skills/code-intel-pipelinewas a junction into an unrelated~/.skillzstore holding a 2026-06-21 SKILL.md whose canonical paths all pointed at a legacy clone. Every-RepairSkillLinksrun refreshed.agentsand.codex, reportedOK skill:claude, and left the skill Claude Code actually loads five weeks stale.Now only two shapes pass: a link that resolves to the bundled-skill source, or a plain copy whose SKILL.md bytes match it (link-less macOS/Linux installs). On repair the previous occupant is moved aside, never deleted — a reparse point is unlinked, since the store it pointed at keeps every byte, and a real directory is renamed so a foreign skill stays recoverable.
Connected bug found while fixing it: installing the bundled skill copied
__pycache__/*.pycinto the agent host's skill directory. Onebootstrap.cpython-313.pycleft by a localbootstrap.pyrun made the byte-parity check reportskill:sourceoutdated permanently — which is how a genuinely stale install stayed camouflaged.main.rscarried a dead duplicate of the resume contract.ResumeSummary, four JSON helpers,next_read, a#[cfg(test)]build_resume_summary, and a verbatim copy of two resume contract tests — all superseded byartifacts.rs/artifacts_tests.rs.cmd_resumedelegates toartifacts::resume, so nothing was serving the copy, andartifacts_tests.rsalready defines the same two tests under the same names. 274 lines removed, coverage unchanged.What this deliberately does not do
The crate reports ~100 dead-code warnings on a clean tree and this PR does not chase them, nor add a
-D warningsgate.There is no
lib.rs; modules are shared by#[path = "..."] mod x;re-inclusion, so one source file compiles once per including module and each instantiation warns about the slice it happens not to use.hardened_git.rsalready keys a test temp directory onmodule_path!()for exactly this reason. Deleting an item one instantiation ignores breaks the instantiation that does not. A-D warningsgate would fail on a clean tree, and the count is not a debt metric.AGENTS.mdnow says so, next to a note on re-syncing theorchestration/**/*.jsondigest pins that source edits invalidate — those must be re-synced by literal replacement, never by reserializing the JSON, because several of those files are historical attestations whose formatting and unrelated digests must not move.Three findings that did not hold up
Recorded because the reasoning is worth not repeating:
EXIT=0reading was$LASTEXITCODEsampled after aSelect-Stringpipeline, which does not reflect the native command. The pipeline reportsrepowise status failed exit=1correctly. Hardening was written and reverted.deletedcapabilities are unflagged orphans."deletedis a first-class terminal state carrying areviewedDeletionrecord with evidence, and the helper scripts are retained on purpose with tests and CI parser coverage.Verification
cargo test -p code-intel --locked— exit 0cargo test -p code-intel --release— exit 0 (this is the one that was failing)cargo fmt -p code-intel -- --check— exit 0cargo build -p code-intel --release --locked— exit 0scripts/tests/test-regression-fixes.ps1— 49 passed, 0 failed (was 42; +7 cases covering the four-state overlay verdict, skill-occupancy detection, non-destructive repair, and__pycache__exclusion)run-code-intel.ps1 -Mode liteagainst a real repo — all steps as expectedcode-intel run executefull DAG — 7 nodes succeeded, publishedDigest pins in
orchestration/re-synced once, after every other edit, by literal replacement.