refactor(ci): drop the dag-facade and run-commit PowerShell call points (#78) - #79
Conversation
…ts (#78) The first batch of the CI de-PS1 sweep. Two call points, two dispositions. `test-run-commit-contract.ps1` (A / already covered): every assertion it made has a Rust counterpart, most of them stronger. Marker schema is asserted at tests/decision_record.rs:318, the manifest digest and verbatim byte preservation at tests/run_commit.rs::cli_publication_preserves_the_callers_ manifest_bytes_and_digest, the CLI publication path at ::production_run_commit_cli_restages_a09_refs_through_a06_and_publishes, and index admission at tests/dag_run.rs::production_dag_output_commits_and_enters_ the_authoritative_index plus the mutated-marker rejections in tests/artifact_index.rs:159-182. The one assertion not carried over — that the marker lacks the legacy generatedAt/reportSha256 fields — guarded the PowerShell producer, which no longer writes the marker. The script is deleted. `test-dag-facade.ps1` (B / ported): its hospital, doctor and manifest assertions were already covered, but three were not, and the contract behind them lived only in PowerShell. run-code-intel.ps1:3271 composed `<artifact root>/<repo name>/<stamp>.dag-staging-<nonce>` and handed the result to `run dag-coordinate --out`; the binary itself had no way to reach an artifact root. So the composition moves into the binary as `run dag-coordinate --artifact-root`, which also reads CODE_INTEL_ARTIFACT_ROOT through the existing resolve_artifact_root, and the guard moves into tests/dag_run.rs: a run lands as a direct child of the repository artifact root, the repository name never repeats inside the path, the explicit flag and the environment default produce byte-identical inventories, and a repository named `repo & 文` survives the round trip. --out and --artifact-root are mutually exclusive, because honouring one and dropping the other silently is worse than refusing. The script itself stays on disk: New-PublicationRetirementPacket.ps1:15 executes it as E05's golden-parity evidence, so deleting it is gated on E05 closing. Off CI is as far as this batch goes. Also re-pins tests/dag_run.rs in the graph and sentrux internalization records, which pin conformance-test digests. Refs #78, #47
|
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: 17 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 (20)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe CLI now supports ChangesDAG artifact-root coordination
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant CLI
participant dag-coordinate
participant compose_dag_staging_dir
participant DAGExecution
CLI->>dag-coordinate: parse staging-path options
dag-coordinate->>compose_dag_staging_dir: compose path when artifact root is selected
compose_dag_staging_dir-->>dag-coordinate: return unique staging directory
dag-coordinate->>DAGExecution: run DAG with staging directory
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 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 |
![coderabbitai[bot]](https://avatars.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/run_cli.rs`:
- Around line 252-259: Update the validation in the dag-coordinate run path to
accept ARTIFACT_ROOT_ENV only when it contains a non-empty UTF-8 value, matching
resolve_artifact_root’s selection rule; otherwise return the existing
missing-artifact-root error. Add an integration test covering
CODE_INTEL_ARTIFACT_ROOT="" and verifying the command rejects it.
🪄 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: 5719f362-3faf-44b2-b9d4-79652e63ea33
📒 Files selected for processing (8)
.github/workflows/ci.ymlcrates/code-intel-cli/src/artifacts.rscrates/code-intel-cli/src/run_cli.rscrates/code-intel-cli/tests/dag_run.rsdocs/ps1-exit/contract-inventory.mdlegacy/scripts/tests/test-run-commit-contract.ps1orchestration/internalization/graph.jsonorchestration/internalization/sentrux.json
💤 Files with no reviewable changes (2)
- legacy/scripts/tests/test-run-commit-contract.ps1
- .github/workflows/ci.yml
| if out.is_none() | ||
| && artifact_root.is_none() | ||
| && env::var_os(ARTIFACT_ROOT_ENV).is_none() | ||
| { | ||
| return Err(format!( | ||
| "run dag-coordinate requires --out, --artifact-root, or {ARTIFACT_ROOT_ENV}" | ||
| )); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reject an empty artifact-root environment value.
env::var_os(ARTIFACT_ROOT_ENV).is_some() accepts an empty or non-UTF-8 value. resolve_artifact_root ignores that value and selects LOCALAPPDATA or HOME instead. This can write the DAG run outside the user-selected artifact-root route.
Make this validation use the same non-empty UTF-8 rule as resolve_artifact_root. Add an integration test for CODE_INTEL_ARTIFACT_ROOT="".
Proposed fix
+ let has_artifact_root_env = env::var(ARTIFACT_ROOT_ENV)
+ .ok()
+ .is_some_and(|value| !value.trim().is_empty());
if out.is_none()
&& artifact_root.is_none()
- && env::var_os(ARTIFACT_ROOT_ENV).is_none()
+ && !has_artifact_root_env
{📝 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.
| if out.is_none() | |
| && artifact_root.is_none() | |
| && env::var_os(ARTIFACT_ROOT_ENV).is_none() | |
| { | |
| return Err(format!( | |
| "run dag-coordinate requires --out, --artifact-root, or {ARTIFACT_ROOT_ENV}" | |
| )); | |
| } | |
| let has_artifact_root_env = env::var(ARTIFACT_ROOT_ENV) | |
| .ok() | |
| .is_some_and(|value| !value.trim().is_empty()); | |
| if out.is_none() | |
| && artifact_root.is_none() | |
| && !has_artifact_root_env | |
| { | |
| return Err(format!( | |
| "run dag-coordinate requires --out, --artifact-root, or {ARTIFACT_ROOT_ENV}" | |
| )); | |
| } |
🤖 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/run_cli.rs` around lines 252 - 259, Update the
validation in the dag-coordinate run path to accept ARTIFACT_ROOT_ENV only when
it contains a non-empty UTF-8 value, matching resolve_artifact_root’s selection
rule; otherwise return the existing missing-artifact-root error. Add an
integration test covering CODE_INTEL_ARTIFACT_ROOT="" and verifying the command
rejects it.
…ance test `Test-NativeCodeRetirementPacket.ps1:33-37` folds `crates/code-intel-cli/tests/dag_run.rs` into E07's frozen source set, so adding the artifact-root routing tests to that file made the packet stale and the Windows lane failed on `E07 packet is stale relative to its frozen source set`. Regenerated through `New-NativeCodeRetirementPacket.ps1` against the current working tree, reusing the packet's original `EvaluatedAt` (1785441780) so the compatibility window keeps its real start date — only the source snapshot changed, not when the retirement was evaluated. The diff is 19 files, one line each, all `snapshotIdentity`; every other field is byte-identical. `test-retirement-packets.ps1` now passes all 8 packets and both audits. Refs #78
…ough pwsh `recompute_sha` shelled out to `Get-FileHash` once per pinned file, and every operation trace pins two. The macOS runner killed the resulting burst of concurrent `pwsh` processes with `Stack overflow.`, which surfaced as 26 of 40 records failing at once and read exactly like real digest drift. The same helper flaked on Windows under full-suite parallelism earlier in this branch. `content_contract::sha256_hex` over the same bytes is the same digest, so nothing about what these records assert changes — only how the number is obtained. The suite also drops from ~34s to ~21s locally. Refs #78
1 participant
First batch of the CI de-PS1 sweep (#78). Two call points off, 43 left.
test-run-commit-contract.ps1test-dag-facade.ps1A —
test-run-commit-contract.ps1每条断言都有 Rust 对等，多数更强：
schema=code-intel-run-commit.v1tests/decision_record.rs:318marker.manifest.sha256为真实 digest；manifest 字节原样保留tests/run_commit.rs::cli_publication_preserves_the_callers_manifest_bytes_and_digestRunCommit*后发布成功tests/run_commit.rs::production_run_commit_cli_restages_a09_refs_through_a06_and_publishesrunIdentity/snapshotIdentity相干tests/artifact_index.rs:159-182（篡改 digest / identity / snapshot 三种 marker 均被拒——PS1 只做正向检查）tests/dag_run.rs::production_dag_output_commits_and_enters_the_authoritative_index（走 Rustartifact index，PS1 走update-code-intel-index.ps1的字符串包含检查）唯一没有搬过来的断言：marker 不含 legacy 的
generatedAt/reportSha256字段。它守的是 PowerShell producer 会不会写出旧形状；marker 现在由 Rust 的固定结构写出，这两个字段不可能出现。理由记在docs/ps1-exit/contract-inventory.md。B —
test-dag-facade.ps1hospital / doctor / manifest 三段早已覆盖，且更强：
tests/dag_run.rs::ungoverned_repository_completes_instead_of_failing_the_architecture_gate—— 除 PS1 断言的succeeded/pass外，还断言 hospital triage、gate 不发布 violation details、以及命令观测保留引擎真实退出码；配套的::baselined_repository_that_regresses_still_fails_the_architecture_gate守住豁免不变成漏洞::production_run_preserves_doctor_domain_failure_and_completes_unrelated_branches—— 覆盖 PS1 那段「容忍 doctor bootstrap 缺口」的逻辑没覆盖的三条，且契约只存在于 PowerShell：
run-code-intel.ps1:3271组装<artifact root>/<repo name>/<stamp>.dag-staging-<nonce>再喂给run dag-coordinate --out。二进制自己够不到 artifact root——--out是必填的。所以这不是「切换」，是「port」。做法：
run dag-coordinate新增--artifact-root，缺省时经既有的resolve_artifact_root读CODE_INTEL_ARTIFACT_ROOT。组装逻辑放在artifacts::compose_dag_staging_dir，与resume/artifact index的读取侧（artifacts.rs:38-42）同一份路径语义。--out与--artifact-root互斥。两条路线对「谁拥有这个路径」的答案不同，静默采纳一个丢掉另一个比报错更糟。dag_run.rs:114用fs::create_dir独占声明，这个 fail-closed 语义必须留着。yyyyMMdd-HHmmss前缀（Howard Hinnantcivil_from_days，不引入日期依赖）：latest_run_dir按名字排序，新写的 run 必须排在 launcher 写的 run 之后。新增测试
tests/dag_run.rs：artifact_root_routes_runs_where_readers_look_and_matches_the_environment_default—— run 是 repo artifact root 的直接子目录、repo 名不重复出现在路径里、显式 flag 与环境变量两条路线的inventory.rg/files.txt字节相同、仓库名repo & 文能跑通out_and_artifact_root_cannot_both_name_the_staging_directory两个测试都不断言 run 结果——那由宿主工具链决定，
production_run_route_executes_snapshot_then_inventory在配好 doctor 的前提下覆盖它。脚本为什么留盘
legacy/tools/compatibility/New-PublicationRetirementPacket.ps1:15执行test-dag-facade.ps1作为 E05 的 golden-parity 证据，冻结的orchestration/retirements/e05-publication/evidence/golden-parity.json也记着这条 command。删文件会打断 E05 packet 的再生路径。本批只做到「下 CI」，删除留给 E05 收口。其他
tests/dag_run.rs在orchestration/internalization/{graph,sentrux}.json的 conformance digest（这两处 pin 测试文件哈希）。已核对 internalization 目录内无其他 stale pin；retirement packet 的 overlay 冻结未动。docs/ps1-exit/contract-inventory.md第 80、82 行改为指向具体的 Rust test fn。验证
cargo test全绿：2631 passed / 45 suitescargo fmt --all -- --check干净ci.ymlYAML 解析通过，windows job 37 steps观察到的既有 flake（非本 PR 引入，clean tree 亦复现）：满并发跑全量时，
internalization_record.rs:660的recompute_sha偶发 pwsh 子进程启动失败；dag_run.rs:392的change impact --staleness current也偶发。单独跑均稳定通过。已单开跟进。Refs #78, #47