fix(installer): honor explicit Code Intel root over stale ambient CODE_INTEL_HOME (#363) - #408
Merged
Merged
Conversation
|
Warning Review limit reachedNext included review available in 11 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (4)
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: |
Code Intel Quality SignalCompleteness: complete · snapshot
Bottleneck: none
|
PR Summary by QodoHonor explicit release roots over stale CODE_INTEL_HOME
AI Description
Diagram
High-Level Assessment
Files changed (4)
|
Code Review by Qodo🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)
Great, no issues found!Qodo reviewed your code and found no material issues that require reviewTip of the day💡 Did you know, you can add REVIEW.md to your repo root and Qodo follows it on every PR |
2233admin
added a commit
that referenced
this pull request
Oct 1, 2026
Rebased #123 onto main b17d401 (#406, #408, #392 merged). Every pin and E04-packet conflict was resolved by taking main's side; the old "resync after rebase" (48e83af) and "regenerate E04" (03a8350) commits became empty and were dropped. Then, once, after all other edits: - `code-intel repin --write`: ast-grep/codenexus/graph/rg internalization records re-pinned to the PR's capability_exec.rs and graph_adapter.rs; `code-intel repin` reports clean. - E04 is stale again because provider.codenexus-adapt pins admissibility.rs. Regenerated with the existing generator at main's packet evaluation time: pwsh -NoProfile -File legacy/tools/compatibility/New-CodeNexusDirectRetirementPacket.ps1 \ -OutDir <tmp>/packet -EvaluatedAt 1788500000 -CodeIntel target/release/code-intel.exe test-retirement-packets.ps1: 8 packets, 2 audits pass.
2233admin
added a commit
that referenced
this pull request
Oct 1, 2026
…s own max_bytes (#123) (#407) * fix(evidence): bound the duplicate-key JSON scanner to each contract's own max_bytes, not a fixed 8 MiB default (#123) Root cause: `verify_artifact_ref` already bounds artifact bytes to the Artifact Ref contract's declared `max_bytes` (e.g. 64 MiB for `observed.evidence.payload`, 16 MiB for `evidence.admission`, 128 MiB for `verification.session-evidence`) via `stable_artifact::read_beneath` before `validate_payload` runs. But every `validate_payload` closure called `content_contract::reject_duplicate_json_keys`, hard-coded to `MAX_JSON_BYTES = 8 MiB`, independent of and smaller than the contract's already-enforced budget. Any in-budget payload between 8 MiB and its real contract ceiling was silently reclamped and rejected with "JSON input exceeds 8388608 bytes" -- this is what broke the normal pipeline's `evidence.graph` node against large repositories. Fix (part 1, the reported bug): - content_contract.rs: add `reject_duplicate_json_keys_within(text, max_bytes)`, parameterized over an explicit ceiling. `reject_duplicate_json_keys` becomes a thin wrapper that keeps the 8 MiB default for callers whose contract does not exceed it. - artifact_ref.rs: every validator whose registered contract exceeds 8 MiB (evidence.admission 16 MiB, observed.evidence.payload 64 MiB, benchmark.orientation-observations 64 MiB, delivery.run-timing-events 64 MiB, verification.session-evidence 128 MiB, verification.anchors 64 MiB, and the 8 native_code_contract schemas via the shared `parse_native_object` helper) now passes its own contract's `max_bytes` instead of the default. - admissibility.rs: its separate local `validate_payload` for `observed.evidence.payload` -- the one that actually caused the k-atana `evidence.graph` failure -- now uses its own existing `MAX_PAYLOAD_BYTES` (64 MiB) ceiling. - Regression coverage in content_contract.rs and admissibility.rs proves a payload strictly between 8 MiB and the real contract budget is now accepted, and one beyond the real budget is still rejected. Fix (part 2, a related correctness bug found during the investigation): - budget_dispatch.rs::run_to_completion_with_estimator_and_oversize_policy: a required node marked `SkippedOversize` (issue #307's pre-dispatch refusal) could let the whole run report `RunOutcome::Completed` because `Coordinator::manifest()`'s outcome chain has no `SkippedOversize`/`DependencyBlocked` arm and falls through to `Completed` once the rest of the DAG is terminal -- reporting success even though required evidence was never produced. Now, when `stopped_at` is `None` but some node is `SkippedOversize` and the manifest outcome is `Completed`, override to `BudgetStopped` (if something else executed) or `Failed` (if nothing did), the same way the existing `stopped_at` branch already does, without touching that branch. - budget_dispatch_oversize.rs: the pre-existing test that asserted a single-node all-oversize run reports `Completed` encoded the old buggy semantics as an intentional assertion; updated to assert `Failed`, matching the corrected outcome logic. Verification: - `cargo build -p code-intel --release --locked`: succeeds. - `cargo test --bins --tests`: 100% green across the full suite. - `code-intel lint hardcoded-paths`: OK (289 files). - Re-ran the release binary's normal pipeline against `k-atana/处理其他分支合并` (isolated via `CODE_INTEL_ARTIFACT_ROOT` to work around an unrelated stale `%CODE_INTEL_HOME%` default): outcome=completed, exitCode=0, failureNode=null, diagnostic=null. `evidence.graph`'s `observed.evidence.payload` artifact is 3,338,281 bytes (3.18 MiB) -- well within the new 64 MiB ceiling. - `legacy/Invoke-SentruxAgentTool.ps1 session_end`: pass, no structural degradation (quality 4265 -> 4338). Issue #123's "Bug 2" (长 artifact-root os error 3) is not addressed by this branch. * test(evidence): add real end-to-end oversized-payload regression + minor clippy fix (#123 follow-up) Adds `oversized_current_graph_payload_clears_the_real_admission_path_within_contract_budget` to graph_adapter.rs: drives a real 9 MiB-plus `observed.evidence.payload` artifact through the actual production admission path -- `admissibility::validate_for_consumer` (i.e. `validate_sealed`) reading a real `payload.json` off disk via `artifact_ref::verify_artifact_ref`, exactly as `builtin_provider_evidence::graph_admission` does for the live `evidence.graph` node -- not just the in-memory `validate_payload` unit added in the prior commit. Verified this test genuinely catches the regression: temporarily reverting `admissibility.rs::validate_payload`'s scanner call back to the unparameterized `reject_duplicate_json_keys` makes it fail with the exact original diagnostic ("JSON input exceeds 8388608 bytes"); restored the fix, it passes. Also includes a one-line clippy fix in content_contract.rs (`!x.is_some_and(f)` -> `x.is_none_or(!f)` in `validate_artifact_ref_shape`, verified logically equivalent) that this machine's write-time lint hook applied while editing the test file, and the corresponding declared-pin resync in orchestration/internalization/graph.json (via `code-intel repin --write`) for the new test file's changed sha256. Verification: cargo test --bins --tests 100% green (incl. the new test and `declared_pins::repin_reports_a_consistent_tree`), lint hardcoded-paths OK. * fix(ci): restore sentrux coupling, rustfmt, and make #123 regressions discriminate CI on 48e83af failed three ways; all three are fixed here. - Rust format: `cargo fmt -p code-intel -- --check` rejected the long `use crate::capability::{...}` line in admissibility.rs, two `format!` calls in its tests, and one `parse_contract_json_within` call in artifact_ref.rs. Every downstream step (tests, self-scan) never ran. - sentrux-capability-gate: diagnosis.hospital failed with `sentrux_gate: Coupling: 63.33 -> 63.4`. The engine counts import lines (2052 -> 2054 over 324 files); the PR added two test-module `use super::` lines. Merge budget_dispatch's two `use super::` lines and call admissibility's test items through `super::` paths: back to 2052 edges, `code-intel sentrux --operation check` reports "No degradation detected". - The rebase onto main brought #382's MAX_JSON_BYTES = 24 MiB, so the 9 MiB fixtures no longer exceeded the scanner default: content_contract's `padded.len() > MAX_JSON_BYTES` assertion panics, and the admissibility / graph_adapter regressions passed even with the fix reverted. Size all three fixtures from MAX_JSON_BYTES + 1 MiB (still under the 64 MiB contract budget). Reverting `validate_payload` to `reject_duplicate_json_keys` now fails both regressions. Pins re-synced once afterwards with `code-intel repin --write` (25 substitutions across 6 files); `code-intel repin` reports clean. * chore(repin): resync pins and regenerate E04 after rebase onto b17d401 Rebased #123 onto main b17d401 (#406, #408, #392 merged). Every pin and E04-packet conflict was resolved by taking main's side; the old "resync after rebase" (48e83af) and "regenerate E04" (03a8350) commits became empty and were dropped. Then, once, after all other edits: - `code-intel repin --write`: ast-grep/codenexus/graph/rg internalization records re-pinned to the PR's capability_exec.rs and graph_adapter.rs; `code-intel repin` reports clean. - E04 is stale again because provider.codenexus-adapt pins admissibility.rs. Regenerated with the existing generator at main's packet evaluation time: pwsh -NoProfile -File legacy/tools/compatibility/New-CodeNexusDirectRetirementPacket.ps1 \ -OutDir <tmp>/packet -EvaluatedAt 1788500000 -CodeIntel target/release/code-intel.exe test-retirement-packets.ps1: 8 packets, 2 audits pass.
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.
Fixes #363.
Why this PR exists at all
The fix was written 2026-08-27 as commit
e923a70on branchcodex/fix-provider-manifest-alignmentand shipped in PR #364 — which wasclosed unmerged with the note "Closing this misrouted PR. The requested
change belongs to Designer Pipeline."
That routing call was wrong on the merits. The bug is in this repo's
legacy/tools/code-intel-platform.psm1: the installedcode-intelbinaryresolves its Provider manifest through a stale ambient
CODE_INTEL_HOMEthat shadows the explicit release root, so a newer binary reads an older
manifest. Designer Pipeline merely has that same dependency; it is not
where the faulty resolution lives.
So the fix was complete and correct the whole time — it just never landed.
Verified on current
main(63b6463) before opening this PR:The defect is live on
mainright now. This branch cherry-picks the singlecommit onto current
main; it applied cleanly with no conflicts.DR-0001 is satisfied in this PR
The repository rule is that an install-class bug is not complete until its
reproduction joins the install-smoke gate in the same PR. It does — the
same commit adds a
Packaged installer stale-home smokestep towindows-build-test-packagethat:$env:CODE_INTEL_HOME,-RepoPath,paths.codeIntelHomeequals the release root, failing with amessage naming both paths.
Without this step the bug class is invisible: 3794 checkout-topology tests
missed every installed-topology bug that shipped with v0.7.0.
A unit-level regression case for explicit / ambient / whitespace-root
resolution also lands in
legacy/scripts/tests/test-regression-fixes.ps1.Verification
repin(check-only): clean — no stale pinscargo testnot run here. DR-0013 excludes this host while bug: repeated Windows host crashes; bound whole-tree snapshot memory #403 isopen. The new smoke runs in CI, which is where the install-class evidence
has to come from anyway.
code-intel lint hardcoded-pathscould not run locally — the installedbinary is 0.7.1, which predates that command. See chore(gate): code-intel lint hardcoded-paths 无法在隔离主机执行（已安装 0.7.1 缺该命令） #405. Note the Windows
CI job does run it, so this PR is still covered there.
Relationship to #401
#401 replaces the PowerShell installer with a native Rust one and will also
touch
code-intel-platform.psm1. This PR is deliberately small and shouldmerge first: it makes the correct precedence the state of
main, so the Rustinstaller inherits the right ordering instead of re-deciding it. If #401
lands first instead, rebase this onto it — one hunk.
Note on the branch
Do not reuse
codex/fix-provider-manifest-alignment. Its remote head isdab8d18, which is older than the locale923a70— the fix was neverpushed to its own remote branch. This PR cherry-picks from the local
commit onto current
maininstead.🤖 Generated with Claude Code