fix(installer): 让 supply-chain-003 的 pin 真的被执行,并给 CLI 加 --version - #88
Conversation
Install-MissingTool 的判据一直是「命令在不在 PATH 上」,不是「版本对不对」。 Get-Command 找到工具就记 already_present 并 return,那个 $Installer scriptblock 永远不会被调用。对 repowise 来说这意味着 `repowise==0.36.0` 这个 pin 从来没有在 一台装过 repowise 的机器上生效过——本机实测 0.32.0,落后 pin 四个 minor,而安装 报告与一台正确的机器完全一样。能力(Invoke-PipInstall 支持精确版本)早就有了, 闸门没加。 Get-InstallMetadata 现在带 pinnedVersion;新增 Get-ToolVersion 读工具自报版本; 不匹配报 version_drift,-InstallMissing 下报 upgraded / upgrade_failed。没有 pin 的工具行为完全不变。 探测遵守 crates/code-intel-cli/src/tool_path.rs 为本项目定下的启动规则("only ever launches by absolute path"、"relative PATH entries are skipped outright"): Test-ToolVersionProbeAllowed 拒绝非 rooted、不存在、或脚本类的源,因为 PATH 上的 repowise.ps1 会被 `& $Source` 在安装器自己的进程里执行。被拒绝不等于探测失败—— 回落到原本的 presence-only 行为,所以一个不可验证的源永远无法诱发重装。子进程用 Start-Process -PassThru -Wait 启动并读它自己的 ExitCode,而不是读 $LASTEXITCODE (只有原生命令会设,否则要么读到陈旧值,要么在 StrictMode 下抛错中止整个安装器)。 只解析 stdout,并在知道工具名时锚定匹配,所以 banner 里的 `setuptools 3.11.0` 既不能伪造通过也不能伪造漂移。 确认的漂移现在会让安装失败。ok 只由 checks 计算,所以只进 installActions 的状态 对所有消费者都不可见——bootstrap-new-machine.ps1 只读 installResult.ok,一台漂移 的机器本会报 Install OK: True。Add-VersionComplianceChecks 从 actions 派生 version:<tool> check。探测被拒或读不出时该 check 不 required:不确定要被看见,但 不能因为一个没测出来的版本让安装失败。 code-intel --version(含 -V 与 --version --json / code-intel-version.v1):装机的 二进制此前无法报告自己是什么。bin/repo.json 记的是安装器从哪跑的,不是它产出了 什么,所以一台跑着旧构建的机器和一台当前的看起来完全一样——实测一个 v0.6.0 的装机 二进制跑 snapshot identity 要 10.9s,含 git cat-file --batch 修复的构建是 0.28s, 而机器上没有任何东西能分辨这两者。这是自报的构建身份,不是 provenance;安装器 记录的 sha256= 才是。 测试按 #78/#80 的方向落在 Rust 侧:driver 由测试生成到临时目录,仓里不新增 .ps1 (AGENTS.md)。12 个场景覆盖解析格式、拒绝规则、banner 噪音、漂移/升级/升级失败、 合规 check 派生,以及真实 Get-InstallMetadata switch 确实带着 pin——最后这条是 regression-proof:删掉那一行,只有它会失败,其余全过。 Refs #59
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
🔒 Repowise is not analyzing this repository The PR bot is free on public repositories. This one is private, which needs a Pro plan. |
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe CLI adds ChangesVersion reporting and installer compliance
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant Installer
participant PinnedTool
participant ComplianceChecks
CLI->>Installer: run installation or doctor mode
Installer->>PinnedTool: execute --version
PinnedTool-->>Installer: version output or probe status
Installer->>ComplianceChecks: record drift or upgrade result
ComplianceChecks-->>CLI: final installation checks
Possibly related PRs
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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
crates/code-intel-cli/tests/installer_version_gate.rs (1)
371-387: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDrop the single-element loop and call
scenarioonce.The loop iterates one tag, and the body calls
scenario(tag)twice. Each call writes a driver and spawnspwsh, so this test starts two processes where one is enough.♻️ Proposed refactor
fn a_probe_that_ran_but_read_nothing_is_unknown_not_a_match() { // The gate exists to surface exactly this state; treating it as a match // would restore the presence-only behaviour it replaces. - for tag in ["parse-unparseable"] { - assert_eq!( - scenario(tag)["refused"], - false, - "{tag} is executable; it ran" - ); - assert_eq!( - scenario(tag)["parsed"], - "", - "{tag} must read as unknown, not as a version" - ); - } + let result = scenario("parse-unparseable"); + assert_eq!(result["refused"], false, "the stub is executable; it ran"); + assert_eq!( + result["parsed"], "", + "an unparseable answer must read as unknown, not as a version" + ); }🤖 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/installer_version_gate.rs` around lines 371 - 387, Refactor a_probe_that_ran_but_read_nothing_is_unknown_not_a_match to remove the single-element loop, call scenario once for "parse-unparseable", and reuse the returned result for both refused and parsed assertions so the test launches only one process.CHANGELOG.md (1)
10-28: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMerge the new entries into the existing
AddedandFixedsections.The
[Unreleased]section now contains two### Addedheadings (Line 10 and Line 168) and two### Fixedheadings (Line 28 and Line 124). One subsection per change type keeps the release notes readable and keeps changelog tooling from splitting the same category.Move the two new bullets under the existing headings, or delete the duplicate headings.
🤖 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 `@CHANGELOG.md` around lines 10 - 28, Consolidate the duplicate changelog subsections under the [Unreleased] section: merge the new version-reporting bullet into the existing Added section and the new fix bullet into the existing Fixed section, removing the extra Added and Fixed headings while preserving all entries.legacy/install-code-intel-pipeline.ps1 (1)
301-321: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftConfirm that this behavior belongs in the legacy script.
The coding guidelines treat existing
.ps1entry points as legacy compatibility surfaces and limit edits to critical fixes or thin forwarding shims while Rust replacements land. This change adds about 100 lines of new product behavior to the installer: probe admission, version parsing, drift classification, and compliance-check derivation. The pin enforcement itself reads as a critical fix, so the intent fits. The implementation location does not.Consider moving
Test-ToolVersionProbeAllowed,Get-ToolVersion, and the version comparison into the Rust CLI and calling it from the installer, in line with the existingcode-intel doctor bootstrapforwarder pattern documented at Line 107 ofCHANGELOG.md. The tests incrates/code-intel-cli/tests/installer_version_gate.rsalready have to lift these functions out by AST to test them, which is a signal that the logic wants a Rust home.As per coding guidelines: "Treat current PowerShell entry points as legacy compatibility surfaces; limit edits to critical fixes or thin forwarding shims while Rust replacements are delivered."
🤖 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 `@legacy/install-code-intel-pipeline.ps1` around lines 301 - 321, Move the version-probe admission, version retrieval, comparison, drift classification, and compliance derivation out of the legacy PowerShell installer into the Rust CLI. Add a thin forwarding path in the installer, following the existing code-intel doctor bootstrap pattern, while preserving the pin-enforcement behavior and the public installer flow. Update the installer_version_gate tests to exercise the Rust implementation rather than extracting PowerShell functions.Source: Coding guidelines
🤖 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 `@legacy/install-code-intel-pipeline.ps1`:
- Around line 444-449: Update the post-reinstall Get-ToolVersion invocation in
the installer flow to pass -ExpectedName $CommandName, matching the anchored
probe used before reinstall and the rule defined near Get-ToolVersion. Keep the
existing command discovery and upgrade result handling unchanged.
- Around line 391-397: Update the $confirmed calculation near Add-Check to match
the exact “unknown” phrase emitted by the detail producers, rather than using a
substring wildcard that can match source paths. Preserve the existing behavior
of marking the version unconfirmed only when $action.detail contains that
producer-written phrase.
- Around line 340-355: The external version probe using Start-Process must not
wait indefinitely. Update the process flow to omit -Wait, call
WaitForExit(15000) on the returned process, and on timeout kill the child tree
with $process.Kill($true) before returning ""; preserve the existing
nonzero-exit, stdout-reading, catch, and cleanup behavior.
---
Nitpick comments:
In `@CHANGELOG.md`:
- Around line 10-28: Consolidate the duplicate changelog subsections under the
[Unreleased] section: merge the new version-reporting bullet into the existing
Added section and the new fix bullet into the existing Fixed section, removing
the extra Added and Fixed headings while preserving all entries.
In `@crates/code-intel-cli/tests/installer_version_gate.rs`:
- Around line 371-387: Refactor
a_probe_that_ran_but_read_nothing_is_unknown_not_a_match to remove the
single-element loop, call scenario once for "parse-unparseable", and reuse the
returned result for both refused and parsed assertions so the test launches only
one process.
In `@legacy/install-code-intel-pipeline.ps1`:
- Around line 301-321: Move the version-probe admission, version retrieval,
comparison, drift classification, and compliance derivation out of the legacy
PowerShell installer into the Rust CLI. Add a thin forwarding path in the
installer, following the existing code-intel doctor bootstrap pattern, while
preserving the pin-enforcement behavior and the public installer flow. Update
the installer_version_gate tests to exercise the Rust implementation rather than
extracting PowerShell functions.
🪄 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: 3ab351ce-d172-4bc8-9e6d-b8d8aa5df217
📒 Files selected for processing (5)
CHANGELOG.mdcrates/code-intel-cli/src/main.rscrates/code-intel-cli/tests/installer_version_gate.rscrates/code-intel-cli/tests/version_cli.rslegacy/install-code-intel-pipeline.ps1
| $stdoutFile = [System.IO.Path]::GetTempFileName() | ||
| $stderrFile = [System.IO.Path]::GetTempFileName() | ||
| try { | ||
| $process = Start-Process -FilePath $Source -ArgumentList "--version" ` | ||
| -RedirectStandardOutput $stdoutFile -RedirectStandardError $stderrFile ` | ||
| -NoNewWindow -PassThru -Wait -ErrorAction Stop | ||
| if ($process.ExitCode -ne 0) { return "" } | ||
| # stdout only: a deprecation banner on stderr must not win the match. | ||
| $raw = Get-Content -Raw -LiteralPath $stdoutFile -ErrorAction SilentlyContinue | ||
| } | ||
| catch { | ||
| return "" | ||
| } | ||
| finally { | ||
| Remove-Item -LiteralPath $stdoutFile, $stderrFile -Force -ErrorAction SilentlyContinue | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find any existing bounded-wait helper in the repo to reuse instead of a new pattern.
rg -nP --iglob '*.ps1' --iglob '*.psm1' -C3 'WaitForExit|Wait-Process|-TimeoutSec'Repository: 2233admin/code-intel-pipeline
Length of output: 167
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | sed -n '1,120p'
echo
echo "== target file outline/size =="
wc -l legacy/install-code-intel-pipeline.ps1 || true
ast-grep outline legacy/install-code-intel-pipeline.ps1 --view compact 2>/dev/null | sed -n '1,200p' || true
echo
echo "== relevant target section =="
sed -n '300,380p' legacy/install-code-intel-pipeline.ps1
echo
echo "== PowerShell header/comments/Shebang =="
sed -n '1,40p' legacy/install-code-intel-pipeline.ps1
echo
echo "== callers/readme references if any =="
rg -n "install-code-intel-pipeline|repowise|Find-command|code-intel" -S . --iglob '*.ps1' --iglob '*.md' --iglob '*.yml' --iglob '*.yaml' | sed -n '1,200p'Repository: 2233admin/code-intel-pipeline
Length of output: 38521
🌐 Web query:
PowerShell 7.2 System.Diagnostics.Process WaitForExit int seconds Kill exit children behavior
💡 Result:
In PowerShell 7.2 and later (as part of.NET 6+), the System.Diagnostics.Process class provides specific methods for handling process termination and waiting. WaitForExit(int milliseconds) Behavior The WaitForExit(int milliseconds) method blocks the current PowerShell thread until the process exits or the specified timeout in milliseconds elapses [1][2]. - Return Value: It returns true if the process exited within the specified time, and false if the timeout was reached [2][3]. - Timeout Handling: If it returns false, the process is still running. You must explicitly call a termination method (such as Kill) if you want to stop the process after a timeout [3]. - Threading: It should not be used in the main UI thread of a graphical application as it will block until the process exits or the time expires [1][2]. Kill and Child Processes Behavior The System.Diagnostics.Process.Kill method is used to force a process to terminate [4][5]. - Kill: The parameterless Kill method stops only the specific process on which it is called [4][5]. It does not automatically terminate child or descendant processes [4][5]. - Kill(bool entireProcessTree): Introduced in.NET Core 3.0 and available in PowerShell 7.2+, you can call Kill($true) to immediately terminate the process and all of its child and descendant processes [6][7]. This is the recommended approach for ensuring an entire process tree is cleaned up [6]. Summary of Usage To implement a timeout and cleanup in PowerShell 7.2: 1. Start the process (e.g., using Start-Process -PassThru). 2. Call $process.WaitForExit(milliseconds). 3. If the method returns $false (meaning the process timed out), call $process.Kill($true) to terminate the process and its entire tree [6][7]. 4. Always follow up with a call to $process.Dispose (or use a script block/try-finally) to ensure OS handles are released [3][7].
Citations:
- 1: https://learn.microsoft.com/en-us/dotnet/api/system.diagnostics.process.waitforexit?view=net-8.0
- 2: https://learn.microsoft.com/en-us/dotnet/api/system.diagnostics.process.waitforexit?view=net-10.0
- 3: https://chandlergray.com/blog/how-to-timeout-an-external-process-in-powershell/
- 4: https://learn.microsoft.com/en-us/dotnet/api/system.diagnostics.process.kill?view=net-9.0
- 5: https://learn.microsoft.com/en-us/dotnet/api/system.diagnostics.process.kill?view=net-10.0
- 6: https://stackoverflow.com/questions/5901679/kill-process-tree-programmatically-in-c
- 7: https://source.dot.net/System.Diagnostics.Process/System/Diagnostics/Process.NonUap.cs.html
🌐 Web query:
.NET Process WaitForExit FlushStandardHandle stdout redirected after wait
💡 Result:
When using.NET Process redirection, relying solely on WaitForExit to flush output can lead to deadlocks or incomplete data retrieval if handled incorrectly [1][2][3]. To ensure that all redirected standard output is captured correctly, you must manage how the process streams are consumed relative to the exit of the process [1][4]. For Asynchronous Reads: When using BeginOutputReadLine and the OutputDataReceived event, you should call the parameterless WaitForExit method [5][6][7]. This method is designed to wait for the process to exit and ensures that all asynchronous event handlers have completed processing, effectively flushing the output stream [5][6][7]. Note that if you use the WaitForExit(int) overload (with a timeout), it does not guarantee that asynchronous events have finished; in such cases, you should follow it with a call to the parameterless WaitForExit to ensure completion [5][6][7]. For Synchronous Reads: If reading synchronously using methods like ReadToEnd, you must perform the read operation before calling WaitForExit [1][8][4]. If you call WaitForExit first while the child process has a large amount of output buffered, the child process may block waiting for the buffer to be cleared, while your parent process waits for the child to exit—creating a classic deadlock [1][2][4]. Calling ReadToEnd first drains the buffer, allowing the child process to complete its execution [1][3]. In summary, avoid deadlocks by: 1. Reading the stream synchronously before WaitForExit [1][9]. 2. Or, using asynchronous events (BeginOutputReadLine) and ensuring you use the parameterless WaitForExit to guarantee all asynchronous events have finished [5][10][11].
Citations:
- 1: https://learn.microsoft.com/en-us/dotnet/api/system.diagnostics.processstartinfo.redirectstandardoutput?view=netframework-4.8.1
- 2: https://learn.microsoft.com/en-us/dotnet/api/system.diagnostics.process.standardoutput?view=net-9.0
- 3: https://stackoverflow.com/questions/139593/processstartinfo-hanging-on-waitforexit-why
- 4: https://learn.microsoft.com/en-us/dotnet/api/system.diagnostics.processstartinfo.redirectstandardoutput?view=net-9.0
- 5: https://learn.microsoft.com/en-us/dotnet/api/system.diagnostics.process.waitforexit?view=netframework-4.8.1
- 6: https://learn.microsoft.com/en-us/dotnet/api/system.diagnostics.process.waitforexit?view=net-8.0
- 7: https://learn.microsoft.com/en-us/dotnet/api/system.diagnostics.process.waitforexit?view=net-10.0
- 8: https://learn.microsoft.com/en-us/dotnet/api/system.diagnostics.processstartinfo.redirectstandardoutput?view=net-10.0
- 9: https://learn.microsoft.com/en-us/dotnet/api/system.diagnostics.process.standardoutput?view=netframework-4.8.1
- 10: https://learn.microsoft.com/en-us/dotnet/api/system.diagnostics.process.outputdatareceived?view=net-10.0
- 11: https://learn.microsoft.com/en-us/dotnet/api/system.diagnostics.process.beginoutputreadline?view=netframework-4.8.1
Bound the external version probe with a timeout.
Start-Process -Wait waits on a third-party executable from PATH, so a prompt, stuck prompt, or long-running --version can make the installer hang indefinitely. Run the probe with -PassThru only, wait with a bounded timeout such as WaitForExit(15000), kill the child tree on timeout with $process.Kill($true), and return "" like other unreadable probes.
🧰 Tools
🪛 PSScriptAnalyzer (1.25.0)
[warning] Missing BOM encoding for non-ASCII encoded file 'install-code-intel-pipeline.ps1'
(PSUseBOMForUnicodeEncodedFile)
🤖 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 `@legacy/install-code-intel-pipeline.ps1` around lines 340 - 355, The external
version probe using Start-Process must not wait indefinitely. Update the process
flow to omit -Wait, call WaitForExit(15000) on the returned process, and on
timeout kill the child tree with $process.Kill($true) before returning "";
preserve the existing nonzero-exit, stdout-reading, catch, and cleanup behavior.
| # "unknown" is the single word both branches use for an unreadable | ||
| # version (Install-MissingTool's $observed, and the post-reinstall | ||
| # detail). Matching on it keeps this derivation in one place; the | ||
| # alternative is a structured field on every Add-InstallAction call | ||
| # site, which is a wider change than this fix warrants. | ||
| $confirmed = -not ([string]$action.detail -like "*unknown*") | ||
| Add-Check $Checks "version:$($action.name)" "version" $confirmed $false ([string]$action.detail) ([string]$action.fix) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Anchor the uncertainty test to the phrase the producer writes.
-like "*unknown*" matches anywhere in detail. detail embeds $existing.Source (Line 432) and $afterDrift.Source (Line 452). An install path that contains the substring unknown therefore marks confirmed drift as not required, and the gate stops failing the install — the exact outcome this function prevents.
Match the phrase the two producers actually write.
🐛 Proposed fix
- $confirmed = -not ([string]$action.detail -like "*unknown*")
+ # Both producers write the literal phrase "reports version unknown" /
+ # "still reports unknown"; matching the phrase keeps an install path
+ # that merely contains "unknown" from disarming the gate.
+ $confirmed = [string]$action.detail -notmatch '(?i)(reports|still reports)\s+(version\s+)?unknown\b'📝 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.
| # "unknown" is the single word both branches use for an unreadable | |
| # version (Install-MissingTool's $observed, and the post-reinstall | |
| # detail). Matching on it keeps this derivation in one place; the | |
| # alternative is a structured field on every Add-InstallAction call | |
| # site, which is a wider change than this fix warrants. | |
| $confirmed = -not ([string]$action.detail -like "*unknown*") | |
| Add-Check $Checks "version:$($action.name)" "version" $confirmed $false ([string]$action.detail) ([string]$action.fix) | |
| # "unknown" is the single word both branches use for an unreadable | |
| # version (Install-MissingTool's $observed, and the post-reinstall | |
| # detail). Matching on it keeps this derivation in one place; the | |
| # alternative is a structured field on every Add-InstallAction call | |
| # site, which is a wider change than this fix warrants. | |
| # Both producers write the literal phrase "reports version unknown" / | |
| # "still reports unknown"; matching the phrase keeps an install path | |
| # that merely contains "unknown" from disarming the gate. | |
| $confirmed = [string]$action.detail -notmatch '(?i)(reports|still reports)\s+(version\s+)?unknown\b' | |
| Add-Check $Checks "version:$($action.name)" "version" $confirmed $false ([string]$action.detail) ([string]$action.fix) |
🧰 Tools
🪛 PSScriptAnalyzer (1.25.0)
[warning] Missing BOM encoding for non-ASCII encoded file 'install-code-intel-pipeline.ps1'
(PSUseBOMForUnicodeEncodedFile)
[info] 397-397: Cmdlet 'Add-Check' has positional parameter. Please use named parameters instead of positional parameters when calling a command.
(PSAvoidUsingPositionalParameters)
🤖 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 `@legacy/install-code-intel-pipeline.ps1` around lines 391 - 397, Update the
$confirmed calculation near Add-Check to match the exact “unknown” phrase
emitted by the detail producers, rather than using a substring wildcard that can
match source paths. Preserve the existing behavior of marking the version
unconfirmed only when $action.detail contains that producer-written phrase.
| try { | ||
| & $Installer | ||
| $afterDrift = if ($CommandName -eq "python") { Get-CodeIntelPythonCommand } else { Get-Command $CommandName -ErrorAction SilentlyContinue } | ||
| $afterVersion = if ($afterDrift) { Get-ToolVersion $afterDrift.Source } else { "" } | ||
| if ($afterVersion -eq $pinned) { | ||
| Add-InstallAction $Actions $CommandName "upgraded" "$($afterDrift.Source) (version $afterVersion, was $observed)" "" $metadata.packageManager ([bool]$metadata.requiresElevation) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Pass -ExpectedName $CommandName to the post-reinstall probe.
Line 418 anchors the version match to the tool name. Line 447 omits -ExpectedName, so the post-reinstall probe falls through to the unanchored [regex]::Match($raw, $versionPattern) at Line 367. A deprecation banner or an unrelated version-shaped number in the tool's stdout can then decide the result. Two outcomes are possible: a banner number equal to the pin reports upgraded when the tool did not change, or a banner number ahead of the real version line reports upgrade_failed after a correct upgrade.
The comment at Line 359 states the anchoring rule. Apply the same rule on both sides of the reinstall.
🐛 Proposed fix
- $afterVersion = if ($afterDrift) { Get-ToolVersion $afterDrift.Source } else { "" }
+ $afterVersion = if ($afterDrift) { Get-ToolVersion $afterDrift.Source -ExpectedName $CommandName } else { "" }📝 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.
| try { | |
| & $Installer | |
| $afterDrift = if ($CommandName -eq "python") { Get-CodeIntelPythonCommand } else { Get-Command $CommandName -ErrorAction SilentlyContinue } | |
| $afterVersion = if ($afterDrift) { Get-ToolVersion $afterDrift.Source } else { "" } | |
| if ($afterVersion -eq $pinned) { | |
| Add-InstallAction $Actions $CommandName "upgraded" "$($afterDrift.Source) (version $afterVersion, was $observed)" "" $metadata.packageManager ([bool]$metadata.requiresElevation) | |
| try { | |
| & $Installer | |
| $afterDrift = if ($CommandName -eq "python") { Get-CodeIntelPythonCommand } else { Get-Command $CommandName -ErrorAction SilentlyContinue } | |
| $afterVersion = if ($afterDrift) { Get-ToolVersion $afterDrift.Source -ExpectedName $CommandName } else { "" } | |
| if ($afterVersion -eq $pinned) { | |
| Add-InstallAction $Actions $CommandName "upgraded" "$($afterDrift.Source) (version $afterVersion, was $observed)" "" $metadata.packageManager ([bool]$metadata.requiresElevation) |
🧰 Tools
🪛 PSScriptAnalyzer (1.25.0)
[warning] Missing BOM encoding for non-ASCII encoded file 'install-code-intel-pipeline.ps1'
(PSUseBOMForUnicodeEncodedFile)
[info] 449-449: Cmdlet 'Add-InstallAction' has positional parameter. Please use named parameters instead of positional parameters when calling a command.
(PSAvoidUsingPositionalParameters)
🤖 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 `@legacy/install-code-intel-pipeline.ps1` around lines 444 - 449, Update the
post-reinstall Get-ToolVersion invocation in the installer flow to pass
-ExpectedName $CommandName, matching the anchored probe used before reinstall
and the rule defined near Get-ToolVersion. Keep the existing command discovery
and upgrade result handling unchanged.
一致性自检(这台机器跑的是不是仓里声明的版本)现在得读五个文件、五种格式: rust-toolchain.toml 的 TOML 字段、requirements-ci.txt 的 pip + hash、ci.yml 里 硬编码在 shell 变量中的 ast-grep 版本、package.json、以及安装器里的 $script:RepowisePinnedVersion。让自检器去解析这五种,等于让它在别人改了 ci.yml 的引号写法时静默少检一项——正是这个仓反复出现的那个形状。 orchestration/toolchain-versions.v1.json 收敛成一种格式,附 code-intel-toolchain-versions.v1 schema。每条带 rationale:一个没有记录理由的 pin 会腐烂成 cargo cult,而这里每个 pin 的理由都是现成的(可复现构建、 supply-chain-001 的 hash 锁定、supply-chain-003 的不拉未审核版本)。 第二真源如果会跟第一真源分叉,比没有第二真源更糟,所以 tests/toolchain_versions.rs 把清单逐条绑回真实声明处。绑定按「每一处匹配都必须 一致」而不是「第一处匹配」:ci.yml 在 windows job 和 cross-platform matrix 里各 装了一次 ast-grep,各有一份硬编码版本,改一处漏一处就是无声分叉。现在有一条 测试专门盯着这个重复。 范围上刻意排除了「上游最新版本」。pin 是供应链控制和可复现性控制,把它跟 「追最新」混在一起就等于把 pin 变成自动升级。上游新鲜度是独立的一层,只报告 不强制,另开。 本次只落声明侧。探测侧(真的去读机器版本并比对)接着做,它会复用 #88 落地的 Get-ToolVersion / Test-ToolVersionProbeAllowed。 Refs #59
问题
Install-MissingTool的判据一直是「命令在不在 PATH 上」,不是「版本对不对」:repowise是唯一带 pin 的外部工具(repowise==0.36.0,源码里标着 supply-chain-003,注释写着「pin the exact version so--upgradecannot pull a newer, unreviewed release onto the machine」)。这个 pin 从来没有在一台装过 repowise 的机器上生效过。本机实测:
repowise 0.32.0,落后 pin 四个 minor,而安装报告与一台正确的机器完全一样。能力早就有了——
Invoke-PipInstall支持精确版本——只是拿不到执行机会。声明存在,强制不存在。改了什么
Get-InstallMetadata带pinnedVersionGet-ToolVersion读工具自报版本version_drift;-InstallMissing下报upgraded/upgrade_failed探测遵守本项目自己的启动规则
crates/code-intel-cli/src/tool_path.rs为这个项目定下的规则是 "only ever launches by absolute path" 和 "relative PATH entries are skipped outright"——因为安装器跑在一个它不信任的仓库旁边。Test-ToolVersionProbeAllowed拒绝非 rooted、不存在、或脚本类的源。理由具体:PATH 上的一个repowise.ps1会被& $Source在安装器自己的进程里执行,那是任意代码,不是沙箱子进程。被拒绝不等于探测失败。 回落到原本的 presence-only 行为,所以一个不可验证的源永远无法诱发重装。
子进程用
Start-Process -PassThru -Wait启动并读它自己的ExitCode,不读$LASTEXITCODE——后者只有原生命令会设,否则要么读到一次无关调用的陈旧值,要么在Set-StrictMode下抛错、被$ErrorActionPreference = "Stop"放大成整个安装器中止。只解析 stdout,并在知道工具名时锚定匹配,所以 banner 里的
setuptools 3.11.0既不能伪造通过也不能伪造漂移。⚠ 确认的漂移现在会让安装失败
ok只由checks计算,所以只进installActions的状态对所有消费者都不可见——bootstrap-new-machine.ps1只读installResult.ok,一台漂移的机器本会报Install OK: True。那正是doctor_provider_rows.rs:13-18写下的形状:Add-VersionComplianceChecks从 actions 派生version:<tool>check。已配置的机器需要动作:repowiseoff-pin 的机器现在报ok: false,missingRequired里出现version:repowise。用-InstallMissing解决,或者把 pin 挪到你真正想跑的版本。探测被拒或读不出时该 check 不 required:不确定要被看见,但不能因为一个没测出来的版本让安装失败。
code-intel --version含
-V与--version --json(code-intel-version.v1)。装机的二进制此前无法报告自己是什么:bin/repo.json记的是安装器从哪跑的,不是它产出了什么。实测代价:一个 v0.6.0 的装机二进制跑
snapshot identity要 10.9s,含git cat-file --batch修复的构建是 0.28s,而机器上没有任何东西能分辨这两者。这是自报的构建身份,不是 provenance——值来自被质疑的那个二进制自己的
CARGO_PKG_VERSION,只能分辨陈旧与当前,不能分辨真实与替换。安装器记录的sha256=才是 provenance 信号。测试
按 #78/#80 的方向落在 Rust 侧。driver 由测试生成到临时目录,仓里不新增
.ps1(AGENTS.md 第一条)。12 个场景:解析格式(标准 / prerelease)、拒绝规则(空 / 不存在 / 相对路径 /
.ps1)、banner 噪音、漂移 / 升级 / 升级失败、合规 check 派生,以及真实Get-InstallMetadataswitch 确实带着 pin。最后一条是 regression-proof,已实测:把
pinnedVersion = $script:RepowisePinnedVersion从真实 switch 删掉(即完全恢复本 PR 修的 bug),只有the_real_metadata_switch_carries_the_pin失败,其余全过。New-VersionStub按平台产出.cmd或#!/bin/sh+chmod +x,因为cross-platform-smoke在 macos/ubuntu 上也跑cargo test -p code-intel --locked。验证
cargo test -p code-intel:新增 17 个测试全过cargo fmt -p code-intel -- --check:cleanlegacy/tools/check-hardcoded-paths.ps1:OK(129 files)repowise version_drift ... reports version 0.32.0; pinned version is 0.36.0,ok: false,missingRequired含version:repowise已知的既有失败(与本 PR 无关,已用
git stash在干净树上复现):internalization_record::ticket_r11_tree_sitter_v_record_does_not_claim_sentrux_or_grammar_ownership—overlays/sentrux/vlang/src/sentrux_vlang_alias.c digest is not bound into the record。未做,建议另开票
复核提出但本 PR 未处理的,都不是 bug 而是设计取舍:
-AuditInstallPlan模式仍会探测。「先给我看计划」的模式不该以执行被审计的二进制为代价。-InstallMissing下直接触发pip install(无--require-hashes、无 index 锁定)。诱导本地故障即可反复触发远程拉取。upgrade_failed的两个分支(工具消失 / installer 抛错)无测试覆盖。version_cli.rs里那条正则测试比实际正则弱,0.7.0+build.5这类会漏。Refs #59