feat: internalize ast-grep with a pinned CI installation - #42
Conversation
…utable Registers ast-grep 0.42.3 as an adapted capability behind edit.ast-grep-plan, modeled on the ripgrep record: installed-version evidence, MIT license obligations with the local-copy gap declared, a capabilityExec operation trace bound to structured_edit.rs and its conformance test, and update/rollback/exit/retirement strategy. Adds a dedicated conformance suite that recomputes the pinned digests, checks command parity with orchestration/integrations.json, and keeps the record research-only. Re-baselines the sentrux ratchet for the new test file (quality 3969 -> 3968, coupling 45.07 -> 45.13, god files and cycles unchanged). Closes #40
Downloads the ast-grep 0.42.3 release artifact on the windows job and the cross-platform smoke matrix, verifies a pinned sha256 per platform artifact, puts the binary on PATH, and asserts the reported version. structured_edit_plan_is_scope_bound_and_preview_only now exercises the real ast-grep execution path on every CI platform instead of skipping. Closes #41
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR adds an ast-grep internalization record and validation test, installs and verifies ast-grep 0.42.3 across CI platforms, and updates Sentrux baseline and ratchet measurements. Changesast-grep enablement
Sentrux measurement updates
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant CIJob
participant ReleaseAsset
participant SHA256Verifier
participant AstGrep
CIJob->>ReleaseAsset: download pinned OS-specific ast-grep 0.42.3 archive
CIJob->>SHA256Verifier: verify SHA256 digest
CIJob->>AstGrep: extract, add to PATH, and validate --version
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)
43-59: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftDuplicated pinned ast-grep install logic across two jobs; digests/version format unverified against upstream release.
Both blocks hard-code the same version, per-OS asset name, and SHA-256 digest, then verify/extract/PATH/
--version-check inline. This is the exact drift risk the internalization record's own update policy calls out (orchestration/internalization/ast-grep.jsonline 12: "refresh the pinned release artifact digests in ci.yml together with this record") — a future bump must be applied consistently in two places. Separately, I could not corroborate the three pinned SHA-256 digests or the exactast-grep --versionoutput format ("ast-grep 0.42.3") against the official 0.42.3 release assets via web search; a mismatch will fail CI loudly, but it's worth confirming against the actual downloaded artifacts before relying on this gate.
.github/workflows/ci.yml#L43-L59: extract the version/asset/digest table and verify/extract/PATH/version-check steps into a composite action (e.g..github/actions/install-ast-grep) parameterized by OS, and call it here..github/workflows/ci.yml#L291-L319: call the same composite action here instead of re-declaring the Windows asset/digest inline alongside the macOS/Linux branches.🤖 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 @.github/workflows/ci.yml around lines 43 - 59, Extract the duplicated ast-grep installation, checksum verification, archive extraction, PATH setup, and version validation into a parameterized composite action, centralizing the pinned version, per-OS asset names, and SHA-256 digests. Update .github/workflows/ci.yml lines 43-59 and 291-319 to invoke that action for each platform, and confirm the pinned digests and expected --version output against the actual upstream 0.42.3 release artifacts.
🤖 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 @.sentrux/baseline.json:
- Line 18: Update the baseline snapshot’s savedAt entry to match the writer
contract from sentrux-lite-core.ps1: use the exact saved_at field name and
ISO-8601 timestamp format. Regenerate .sentrux/baseline.json with the producer
when possible, preserving the remaining baseline content.
In `@crates/code-intel-cli/tests/ast_grep_internalization.rs`:
- Around line 16-35: Update recompute_sha to use a supported pwsh invocation:
remove the invalid -CommandWithArgs usage and pass the path as a safely embedded
literal argument to Get-FileHash, preserving SHA256 hashing and lowercase digest
validation.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 43-59: Extract the duplicated ast-grep installation, checksum
verification, archive extraction, PATH setup, and version validation into a
parameterized composite action, centralizing the pinned version, per-OS asset
names, and SHA-256 digests. Update .github/workflows/ci.yml lines 43-59 and
291-319 to invoke that action for each platform, and confirm the pinned digests
and expected --version output against the actual upstream 0.42.3 release
artifacts.
🪄 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: b0314d62-c7ce-4125-9063-ddd710d8f227
📒 Files selected for processing (5)
.github/workflows/ci.yml.sentrux/baseline.json.sentrux/rules.tomlcrates/code-intel-cli/tests/ast_grep_internalization.rsorchestration/internalization/ast-grep.json
| "total_import_edges": 1029 | ||
| }, | ||
| "savedAt": 1785048977, | ||
| "savedAt": 1785080615, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Match the baseline timestamp to the writer contract.
tools/sentrux-shim/sentrux-lite-core.ps1 writes saved_at as an ISO-8601 timestamp, but this snapshot uses savedAt with epoch seconds. Generated or schema-validated baselines may therefore treat the timestamp as missing or malformed. Regenerate this file with the producer or use its exact field name and format.
🤖 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 @.sentrux/baseline.json at line 18, Update the baseline snapshot’s savedAt
entry to match the writer contract from sentrux-lite-core.ps1: use the exact
saved_at field name and ISO-8601 timestamp format. Regenerate
.sentrux/baseline.json with the producer when possible, preserving the remaining
baseline content.
| fn recompute_sha(relative: &str) -> String { | ||
| let path = root().join(relative); | ||
| let output = Command::new("pwsh") | ||
| .args([ | ||
| "-NoProfile", | ||
| "-CommandWithArgs", | ||
| "(Get-FileHash -LiteralPath $args[0] -Algorithm SHA256).Hash.ToLowerInvariant()", | ||
| path.to_str().unwrap(), | ||
| ]) | ||
| .output() | ||
| .expect("run Get-FileHash"); | ||
| assert!( | ||
| output.status.success(), | ||
| "{}", | ||
| String::from_utf8_lossy(&output.stderr) | ||
| ); | ||
| let digest = String::from_utf8(output.stdout).unwrap().trim().to_string(); | ||
| assert_eq!(digest.len(), 64); | ||
| digest | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== repository files matching target =="
fd -a 'ast_grep_internalization.rs|CARGO.toml|Cargo.toml' . | sed 's#^\./##' | head -50
echo
echo "== target file excerpt =="
if [ -f crates/code-intel-cli/tests/ast_grep_internalization.rs ]; then
wc -l crates/code-intel-cli/tests/ast_grep_internalization.rs
sed -n '1,120p' crates/code-intel-cli/tests/ast_grep_internalization.rs | cat -n
fi
echo
echo "== search pwsh command argument usages =="
rg -n '"pwsh"|pwsh|CommandWithArgs|-CommandWithArgs|Get-FileHash' . -g '*.rs' -g '*.toml' || true
echo
echo "== static verifier: parse the Command args and check pwsh docs via local source if present =="
python3 - <<'PY'
from pathlib import Path
p=Path('crates/code-intel-cli/tests/ast_grep_internalization.rs')
text=p.read_text() if p.exists() else ''
checks={
'has_recompute_sha': 'fn recompute_sha' in text,
'has_pwsh': '"pwsh"' in text or "Command::new(\"pwsh\")" in text,
'has_CommandWithArgs': '-CommandWithArgs' in text,
'uses_Command_string_command': '(Get-FileHash -LiteralPath $args[0] -Algorithm SHA256)' in text,
}
for k,v in checks.items():
print(f'{k}={v}')
PY
echo
echo "== PowerShell help availability =="
if command -v pwsh >/dev/null 2>&1; then
pwsh -NoProfile -Command 'Get-Help about_PowerShell_exe -ShowWindow 0 | Out-String' || true
pwsh -NoProfile -Command 'Get-Command pwsh | Select-Object Name,FileVersionInfo,Source | Out-String' || true
else
echo "pwsh not installed"
fi
echo
echo "== PowerShell behavior probe if pwsh is available (local invocation of invalid parameter) =="
if command -v pwsh >/dev/null 2>&1; then
pwsh -NoProfile -CommandWithArgs "(1+1)" "ignored" || true
echo "exit=$?"
else
echo "Skipped pwsh behavioral probe"
fiRepository: 2233admin/code-intel-pipeline
Length of output: 9161
🌐 Web query:
Microsoft PowerShell pwsh command-line parameters Command -args positional arguments CommandWithArgs
💡 Result:
In PowerShell, command-line parameters and arguments are handled based on whether you are executing a command within the PowerShell environment or launching PowerShell itself (pwsh or powershell.exe) from an external shell [1][2]. Parameter Fundamentals PowerShell commands (cmdlets, functions, and scripts) use parameters to accept input or define options [3]. - Parameter names are prefixed with a hyphen (-) [3]. - You can separate a parameter name and its value with a space or a colon (e.g., -Name "Value" or -Name:"Value") [3]. - Switch parameters (e.g., -Verbose) do not require a value; their presence sets them to true [4][5]. - Parameters can be either named or positional [5]. Named parameters require the parameter name, while positional parameters can be bound by their order in the command line if the name is omitted [4][5]. Invoking PowerShell from an External Shell When calling the PowerShell executable (pwsh or powershell.exe) from an external environment (like cmd.exe), you primarily use two parameters to execute commands: -File []: Used to run a script file (.ps1) [6][2]. Arguments following the script path are passed literally to the script [2]. -Command <ScriptBlock|String>: Used to run a snippet of PowerShell code or a command string [6]. The arguments provided here are interpreted by the PowerShell runtime [2]. When using -Command, PowerShell concatenates the remaining arguments into a single string and executes it as code [2]. If you need to pass complex arguments or require specific quoting, you may need to use escape sequences or consider -EncodedCommand to submit a Base64-encoded string of your command to avoid parsing issues with the host shell [6][2]. Parameter Binding PowerShell uses a process called parameter binding to associate values with parameters [7]. When a command is invoked: 1. It attempts to bind command-line arguments to parameters [7]. 2. It handles positional binding for any arguments that do not have an explicitly named parameter [7][5]. 3. It resolves input from the pipeline if it is provided [7]. Common Parameters Regardless of the cmdlet, PowerShell provides a set of universal "common parameters" implemented by the runtime (e.g., -Verbose, -Debug, -ErrorAction, -WhatIf, -Confirm) [8][9]. These are automatically available to all cmdlets and advanced functions, though their effect depends on whether the specific command supports the underlying action [10][9]. For troubleshooting parameter syntax or requirements for a specific command, use the Get-Help command (e.g., Get-Help -Full) [3].
Citations:
- 1: https://learn.microsoft.com/en-us/powershell/scripting/learn/shell/running-commands?view=powershell-7.6
- 2: https://stackoverflow.com/questions/57441095/running-a-powershell-command-using-full-path-and-arguments-from-command-line
- 3: https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_parameters?view=powershell-7.5
- 4: https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_command_syntax?view=powershell-7.6
- 5: https://learn.microsoft.com/en-us/powershell/scripting/developer/cmdlet/types-of-cmdlet-parameters?view=powershell-7.6
- 6: https://learn.microsoft.com/zh-cn/powershell/module/microsoft.powershell.core/about/about_powershell_exe?view=powershell-5.1
- 7: https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_parameter_binding?view=powershell-7.6
- 8: https://learn.microsoft.com/en-us/powershell/scripting/developer/cmdlet/common-parameter-names?view=powershell-7.5
- 9: https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_commonparameters?view=powershell-7.4
- 10: https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_commonparameters?view=powershell-7.6
🌐 Web query:
PowerShell about_PowerShell_exe command line parameters -Command must be last parameter arguments following it interpreted as part of command to execute
💡 Result:
When using the -Command parameter with powershell.exe (Windows PowerShell 5.1), any arguments provided after the -Command string are interpreted as part of the command itself, rather than as independent parameters for the PowerShell process [1]. Because of this, it is standard practice to ensure that -Command is the last parameter provided in the command line, or that the entire command string (including its arguments) is properly enclosed within quotes [2][1]. This behavior applies because the -Command parameter accepts a string, and once invoked, PowerShell treats all remaining tokens on the command line as elements of that command string [1]. If you place other parameters (like -NoProfile or -ExecutionPolicy) after -Command, they may be mistakenly consumed as part of the command script rather than being processed as switches for the PowerShell executable [1]. For example: - Incorrect: powershell.exe -Command Write-Host "Hello" -NoProfile - Correct: powershell.exe -NoProfile -Command Write-Host "Hello" If you need to pass arguments to a script, the -File parameter is generally preferred over -Command [2][3]. When using -File, the script path is expected to follow the parameter, and any subsequent arguments are passed directly to that script as parameters, maintaining a clear separation between executable flags and script arguments [2][1].
Citations:
- 1: https://ss64.com/ps/powershell.html
- 2: https://stackoverflow.com/questions/71317957/powershell-exe-arguments-when-running-scripts
- 3: https://powershellisfun.com/2024/03/08/powershell-5-and-7-command-line-parameters/
Use a supported pwsh invocation for recompute_sha.
-CommandWithArgs is not a valid pwsh parameter, so this check panics before hashing because output.status.success() is false. Embed the file path in the script as a literal argument to Get-FileHash, or refactor this as a Rust hash computation instead of a Windows-specific pwsh call.
🤖 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/ast_grep_internalization.rs` around lines 16 -
35, Update recompute_sha to use a supported pwsh invocation: remove the invalid
-CommandWithArgs usage and pass the path as a safely embedded literal argument
to Get-FileHash, preserving SHA256 hashing and lowercase digest validation.
… slice holds (#45) * fix(audit): require --repo and validate before render (ai-safety-003) The audit kernel's fail-closed validator (registry membership, evidence grounding, recomputed overall, the zero-findings coverage rule) only ran behind `--operation validate`. `--operation render` parsed a report and printed it with no registry, no evidence grounding, and no shape checks, so a fabricated, drifted, or malformed report could reach human-facing markdown or HTML as if it were authoritative (issue #34, ai-safety-003). `render` now requires `--repo` and runs through the same `validate_and_parse` pipeline `validate` uses before it renders anything; a report that fails validation returns the same error `validate` would have produced instead of being rendered. Update the four cli_tests.rs cases that encoded the old "render never needs --repo" contract, and add a case asserting render fails closed on a report that fails validation. Docs updated to match: `--operation render` is now documented as requiring `--repo` and running the validate pipeline first. * ci(release): validate any produced audit report before it can publish (ai-safety-003) The fail-closed validator existed and was well-tested but ran on no automated path: not CI, not the orchestrator, not the release workflow (issue #34, ai-safety-003). Add an additive "Validate any produced audit report" step, after each self-scan step in both ci.yml jobs and in release.yml, that looks for audit-report.json at the repo root, under orchestration/, and in the self-scan artifact directory, and runs `code-intel audit --operation validate --repo . --report <path>` on any it finds, failing the step on a validation error. Departments are agent-run today, not part of `run execute`, so no workflow currently produces an audit-report.json automatically -- this step is the structural guarantee for whenever one does show up, not an assumption that one exists yet. release.yml's "Validate packaged Skill bootstrap" step also gets an audit-report.json check against the extracted payload using the packaged binary, alongside the existing packaged `sentrux check`/`gate` calls, so release green, self-scan green, and published-artifact green keep referring to the same snapshot for audit evidence too, not just structural evidence. * test(sentrux): pin a fast regression guard for resolved import cycles Issue #14's v0.5.1 acceptance criteria ask for a regression check that fails CI if the dag_run/execution_kernel cycle (removed in #15) comes back, added to the existing cycle-detection mechanism rather than a new one. That mechanism already exists and is already wired into CI: the absolute `max_cycles = 0` rule in .sentrux/rules.toml, evaluated by sentrux_gate.rs's Tarjan-based rust_import_cycles against every push/PR self-scan. Add sentrux_gate::this_repository_has_no_resolved_import_cycles, which calls the same run_check() engine directly against this repository's own source tree as a plain `cargo test`. This gives a reintroduced cycle (this pair or any other) a seconds-fast failure signal instead of waiting for a full release-mode self-scan build. crates/code-intel-cli/src/sentrux_gate.rs is itself pinned by a supply-chain provenance record (orchestration/internalization/sentrux.json, ownedModifications + operationTrace, checked by tests/internalization_record.rs). Editing the file to add the test changes its SHA-256, so the recorded native-gate-source-sha256 digest is updated to match -- the same maintenance step the file's own history (c3a8ca2) took the last time this file changed. This is a provenance identity record, not the structural quality baseline; updating it to reflect a reviewed, intentional, additive source change is expected and is a different action from regenerating .sentrux/baseline.json to paper over a regression. * docs(changelog): record ai-safety-003 fix and re-verify the v0.5.1 slice Document the render/CI/release validate wiring under [Unreleased], and record (with pointers to the evidence, not just an assertion) that the v0.5.1 self-dogfood acceptance criteria from issue #14 -- the dag_run/execution_kernel cycle at zero under an absolute rule, CI and release self-scan against the real compiled binary, and packaged/ self-scan/release snapshot identity -- were already landed by #15 and held through #38/#42, verified against current main rather than re-implemented here. * test(hospital): assert the actionable failure fields for a real structural violation Acceptance criterion 3 from issue #14 (Hospital must name the first failed rule, evidence, target files, and smallest rerun command on a structural failure) was already implemented by hospital_diagnosis.rs's treatment()/render_hospital() -- but no existing test exercised it: the structural() fixture helper in this file only ever seeds `failure:{"kind":"none"}`, even for a "fail" verdict, so the precedence matrix test only checked the diagnosis string, never the rendered text. Add architecture_gate_failure_names_the_rule_targets_and_smallest_rerun_command, which seeds a structural admission carrying the same shape a real max_cycles violation on dag_run.rs/execution_kernel.rs produces, and asserts hospital.md contains the failing rule and message, the target files, a dedicated "## Failing rules" section, and the literal "Rerun the smallest gate: code-intel sentrux --operation check --repo <repo-root>." line -- and that a surgery plan is produced. This closes the coverage gap rather than just re-reading the source to confirm the behavior exists. * fix(provenance): re-pin provider.sentrux-adapt digest for edited sentrux_gate.rs The cycle regression guard added to sentrux_gate.rs changed its sha256; orchestration/internalization/sentrux.json was re-pinned in the same change but the provider.sentrux-adapt toolchainDigests entry in orchestration/integrations.json was missed, failing test-atomic-capability-contract.ps1 on every platform. Recomputed and verified via the contract test (ok: true, 11 toolchain evidence capabilities checked).
1 participant
What changed
orchestration/internalization/ast-grep.json: internalization record for the ast-grep 0.42.3 executable behindedit.ast-grep-plan, modeled on the ripgrep record — installed-version evidence, MIT license obligations with the local-copy gap declared, acapabilityExecoperation trace bound tostructured_edit.rsand its conformance test, and update/rollback/exit/retirement strategy. Lifecycle staysresearch; the capability remains optional.crates/code-intel-cli/tests/ast_grep_internalization.rs: dedicated conformance suite — recomputes the pinned source and conformance digests via the same Get-FileHash semantics as the gate, checks command parity withorchestration/integrations.json, asserts the record is research-only, and keeps the four declared lifecycle gaps present..github/workflows/ci.yml: both jobs download the ast-grep 0.42.3 release artifact, verify a pinned per-platform sha256 (windows x64 / linux x64 / macOS arm64), put the binary on PATH, and assert the reported version..sentrux/: ratchet re-baselined for the new test file (quality 3969 -> 3968, coupling 45.07 -> 45.13; god files and cycles unchanged), with rules.toml evidence comment updated.Why
PR #39 introduced the pipeline's first ast-grep dependency with no internalization record (governance inconsistency vs
rg.json) and no CI installation, sostructured_edit_plan_is_scope_bound_and_preview_onlyskipped its real-execution half on every CI platform.Validation
cargo fmt --checkcargo test -p code-intel: 1809 tests, 0 failures (includes the new suite)run execute: exit 0 after re-baselineCloses #40. Closes #41.