[codex] cross-platform PowerShell 7 support - #2
Conversation
|
Warning Review limit reached
Next review available in: 6 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 Run ID: 📒 Files selected for processing (27)
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 |
![gemini-code-assist[bot]](https://avatars.github.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces cross-platform support (Windows, macOS, and Linux) to the Code Intel Pipeline by abstracting platform-specific paths, environment variables, and tool invocations into a new helper module, tools/code-intel-platform.psm1. The scripts have been updated to accept a -Platform parameter and handle non-Windows environments gracefully. The review feedback highlights three important issues: the use of exit 0 in Install-SentruxVlangOverlay.ps1 which terminates the calling PowerShell process, the lack of directory exclusions when using Copy-Item on macOS/Linux in Invoke-ScopedRepowise.ps1 (where rsync should be preferred), and a potential terminating error in New-CodeIntelLink when Split-Path -Parent returns an empty string for relative paths.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| missing = $sourcePath | ||
| message = "No vlang grammar artifact is bundled for this platform; skipping overlay install." | ||
| } | ConvertTo-Json -Depth 4 | ||
| exit 0 |
There was a problem hiding this comment.
Using exit 0 inside a script that is called by other scripts via the call operator (&) will terminate the entire PowerShell process. This prevents the calling scripts (such as Test-SentruxVlangOverlay.ps1 and install-code-intel-pipeline.ps1) from receiving the output and continuing their execution.
Use return instead of exit 0 to safely exit the script and return the JSON output to the caller.
return
| if (Test-Path -LiteralPath $Destination -PathType Container) { | ||
| Remove-Item -LiteralPath $Destination -Recurse -Force | ||
| } | ||
| Copy-Item -LiteralPath $Source -Destination $Destination -Recurse -Force |
There was a problem hiding this comment.
On non-Windows platforms (macOS/Linux), using Copy-Item recursively without exclusions will copy massive ignored directories like node_modules, .venv, or build artifacts (e.g., target, dist) into the shadow worktree. This completely defeats the purpose of a sparse shadow worktree and can cause severe performance issues, disk space waste, and potential indexing pollution.
Since macOS and Linux almost always have rsync pre-installed, we should use rsync with the same exclusions as robocopy when available, and only fall back to Copy-Item if rsync is missing.
if (Get-Command rsync -ErrorAction SilentlyContinue) {
New-Item -ItemType Directory -Force -Path $Destination | Out-Null
& rsync -a --delete --exclude='.git' --exclude='.repowise' --exclude='node_modules' --exclude='.venv' --exclude='venv' --exclude='__pycache__' --exclude='.pytest_cache' --exclude='.mypy_cache' --exclude='tmp' --exclude='dist' --exclude='build' --exclude='target' --exclude='.understand-anything' --exclude='.sentrux' --exclude='*.egg-info' --exclude='uv.lock' --exclude='uv.lock.bak' --exclude='*.bak' --exclude='=*' "$Source/" "$Destination/"
return
}
if (Test-Path -LiteralPath $Destination -PathType Container) {
Remove-Item -LiteralPath $Destination -Recurse -Force
}
Copy-Item -LiteralPath $Source -Destination $Destination -Recurse -Force
| $parent = Split-Path -Parent $Path | ||
| New-Item -ItemType Directory -Force -Path $parent | Out-Null |
There was a problem hiding this comment.
If $Path is a relative path with no parent directory component (e.g., "vlang"), Split-Path -Parent will return an empty string (""). Passing an empty string to New-Item -Path will throw a terminating error.
Add a check to ensure $parent is not null or whitespace before attempting to create the directory.
$parent = Split-Path -Parent $Path
if (-not [string]::IsNullOrWhiteSpace($parent)) {
New-Item -ItemType Directory -Force -Path $parent | Out-Null
}
![coderabbitai[bot]](https://avatars.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
run-code-intel.ps1 (2)
1612-1618:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPass
-Platformto scoped repowise calls.Lines 1612–1618 and 1624–1629 call
Invoke-ScopedRepowise.ps1without-Platform, so explicit platform overrides are dropped for scoped runs.Suggested fix
$repowiseStep = Invoke-LoggedStep "repowise scoped docs" { & $scopedRepowiseScript ` -RepoPath $repoPath ` + -Platform $effectivePlatform ` -ShadowRoot $RepowiseShadowRoot ` -ScopePaths $RepowiseScopePaths ` -RootFiles $RepowiseRootFiles ` -TimeoutSeconds $RepowiseTimeoutSeconds ` -Docs } ... $repowiseStep = Invoke-LoggedStep "repowise scoped index" { & $scopedRepowiseScript ` -RepoPath $repoPath ` + -Platform $effectivePlatform ` -ShadowRoot $RepowiseShadowRoot ` -ScopePaths $RepowiseScopePaths ` -RootFiles $RepowiseRootFiles ` -TimeoutSeconds $RepowiseTimeoutSeconds }Also applies to: 1624-1629
🤖 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 `@run-code-intel.ps1` around lines 1612 - 1618, The calls to the scoped repowise script at lines 1612-1618 and 1624-1629 are missing the `-Platform` parameter in the & $scopedRepowiseScript invocation. Add the `-Platform` parameter to both calls to pass through explicit platform overrides, ensuring consistent behavior between scoped and regular repowise runs. Use the same variable or value that would be used for the non-scoped invocations.
1647-1653:⚠️ Potential issue | 🔴 CriticalRemove Windows-only
cmd /cfrom repowise execution path.Lines 1647 and 1652 use
cmd /cto pipe input to repowise commands, which is unavailable on macOS/Linux and breaks the unscoped repowise flow on non-Windows systems. Line 1659 demonstrates repowise commands work without this wrapper.Suggested fix
if ((Test-Path -LiteralPath $repowiseStatePath -PathType Leaf) -or (Test-Path -LiteralPath $repowiseDbPath -PathType Leaf)) { $steps.Add((Invoke-LoggedStep "repowise update" { - cmd /c "exit" | repowise update --workspace --index-only + repowise update --workspace --index-only })) } else { $steps.Add((Invoke-LoggedStep "repowise init" { - cmd /c "echo n" | repowise init . --index-only -y --no-claude-md --no-onboarding --embedder mock --provider mock + repowise init . --index-only -y --no-claude-md --no-onboarding --embedder mock --provider mock })) }🤖 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 `@run-code-intel.ps1` around lines 1647 - 1653, Remove the Windows-only `cmd /c` wrappers from the repowise command execution paths to make them cross-platform compatible. In the Invoke-LoggedStep calls for "repowise update" and "repowise init", remove the `cmd /c "exit" |` prefix from the update command and the `cmd /c "echo n" |` prefix from the init command, keeping only the repowise commands themselves to ensure they work on macOS/Linux systems as well as Windows.bootstrap-new-machine.ps1 (1)
96-102:⚠️ Potential issue | 🟠 Major | ⚡ Quick winThread
-Platforminto smoke test invocation.The smoke step (Lines 96–102) does not pass
-Platform, so bootstrap can run install/doctor under one platform and smoke under auto-detected platform.Suggested fix
$smokeParams = @{ RepoPath = $repo Mode = $Mode + Platform = $effectivePlatform SkipRepowise = $true }🤖 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 `@bootstrap-new-machine.ps1` around lines 96 - 102, The $smokeParams hashtable passed to the smoke test invocation via Invoke-JsonScript (line 96-102) is missing the -Platform parameter, causing platform inconsistency between the bootstrap install/doctor steps and the smoke test execution. Add the -Platform parameter to the $smokeParams hashtable (which currently includes RepoPath, Mode, and SkipRepowise) and ensure it uses the same $Mode or platform value that was used earlier in the bootstrap script so the smoke test runs under the same platform context.invoke-code-intel.ps1 (1)
152-152:⚠️ Potential issue | 🟠 Major | ⚡ Quick winThread
-Platforminto artifact indexing to keep roots consistent.Line 152 invokes
update-code-intel-index.ps1without-Platform, so explicit platform runs can index a different artifact root than the one used by the pipeline run.Suggested fix
if (Test-Path -LiteralPath $Config -PathType Leaf) { $indexConfigData = Get-Content -LiteralPath $Config -Raw | ConvertFrom-Json $configuredArtifactRoot = Get-JsonProperty $indexConfigData "artifactRoot" if (-not [string]::IsNullOrWhiteSpace([string]$configuredArtifactRoot)) { $indexParams.ArtifactRoot = [string]$configuredArtifactRoot } } + $indexParams.Platform = $Platform & $indexer `@indexParams` | Out-Host🤖 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 `@invoke-code-intel.ps1` at line 152, The invocation of the indexer at the line with `& $indexer `@indexParams` | Out-Host` is missing the `-Platform` parameter, which causes inconsistency in artifact root paths between explicit platform runs and pipeline runs. Add the `-Platform` parameter to the `@indexParams` hashtable or include it directly in the call to `$indexer` so that the platform value is properly threaded through to the `update-code-intel-index.ps1` script execution.Invoke-SentruxAgentTool.ps1 (2)
275-277:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse separator-agnostic matching for
.sentrux/baseline.jsondiscovery.Line 276 hardcodes Windows separators in the regex, so POSIX paths won’t match and
scope_candidatescan be empty on macOS/Linux.Proposed fix
- Where-Object { $_.FullName -match "\\.sentrux\\baseline\.json$" } | + Where-Object { + (Normalize-RelativeFilePath $_.FullName) -match '(^|/)\.sentrux/baseline\.json$' + } |🤖 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 `@Invoke-SentruxAgentTool.ps1` around lines 275 - 277, The Where-Object filter on line 276 uses a hardcoded Windows path separator pattern that only matches backslashes, causing the regex to fail on macOS/Linux where forward slashes are used as separators. Modify the regex pattern in the Where-Object filter to use a character class that matches both forward slashes and backslashes (such as [\\\/] or [\\/]) so that the pattern correctly identifies baseline.json files regardless of the operating system's path separator convention.
324-333:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winNormalize noisy-dir keys before ignored-path comparison.
Line 332 compares
$normalizedDiragainst values normalized withDirectorySeparatorChar, but Lines 324-325 now use slash-form paths. On Windows,"static/assets"won’t match"static\assets"and exclusions are silently ignored.Proposed fix
- $normalizedDir = $dir.ToLowerInvariant() + $normalizedDir = $dir.ToLowerInvariant().Replace("/", [System.IO.Path]::DirectorySeparatorChar) if ($ignored -contains $normalizedDir) { continue }🤖 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 `@Invoke-SentruxAgentTool.ps1` around lines 324 - 333, The paths defined in the $noisyDirs array on lines 324-325 use forward slashes, but the comparison on line 332 checks $normalizedDir against $ignored which contains paths normalized with DirectorySeparatorChar. On Windows, this causes paths like "static/assets" to fail matching against "static\assets", silently ignoring exclusions. After creating $normalizedDir on line 328, also normalize the directory separators by replacing forward slashes with [IO.Path]::DirectorySeparatorChar to ensure consistent path format matching across all platforms.
🧹 Nitpick comments (1)
Invoke-ScopedRepowise.ps1 (1)
212-220: Remove unnecessary-WindowStyleparameter on non-Windows platforms.The
-WindowStyle Hiddenparameter is Windows-only and is silently ignored on Linux and macOS in PowerShell 7.2+. While it doesn't cause breakage, it's cleaner to exclude this parameter on non-Windows platforms.Suggested fix
- $process = Start-Process ` - -FilePath $FilePath ` - -ArgumentList $ArgumentList ` - -WorkingDirectory $WorkingDirectory ` - -RedirectStandardOutput $stdout ` - -RedirectStandardError $stderr ` - -PassThru ` - -WindowStyle Hidden + $startProcessParams = @{ + FilePath = $FilePath + ArgumentList = $ArgumentList + WorkingDirectory = $WorkingDirectory + RedirectStandardOutput = $stdout + RedirectStandardError = $stderr + PassThru = $true + } + if ($effectivePlatform -eq "windows") { + $startProcessParams.WindowStyle = "Hidden" + } + $process = Start-Process `@startProcessParams`🤖 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 `@Invoke-ScopedRepowise.ps1` around lines 212 - 220, The Start-Process command currently always includes the -WindowStyle Hidden parameter, which is unnecessary on non-Windows platforms. Modify the Start-Process call to conditionally include the -WindowStyle Hidden parameter only when running on Windows. Check if the platform is Windows using the appropriate PowerShell condition (such as $PSVersionTable.Platform or $IsWindows) and only add the -WindowStyle parameter when that condition is true, removing it from the current unconditional parameter list.
🤖 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 @.github/workflows/ci.yml:
- Around line 10-20: The cross-platform-smoke job is missing an explicit
permissions block, which means it inherits default GitHub token permissions that
may be broader than necessary. Add a permissions block at the same indentation
level as the strategy key in the cross-platform-smoke job definition, specifying
only the minimum permissions needed for this job to run (typically read-only
permissions like contents: read for accessing the repository). This ensures the
CI workflow follows the principle of least privilege.
In `@install-code-intel-pipeline.ps1`:
- Around line 319-327: The post-install verification in the installer execution
block uses Get-Command to check if CommandName is available, which fails to
detect Python in python3-only environments and incorrectly marks successful
Python installations as needing a restart. Replace the Get-Command based
verification with the Python resolver approach that is already used elsewhere in
the script to properly detect installations across all environment types. This
should occur in the section where you check the after variable following the
installer execution.
In `@tools/code-intel-platform.psm1`:
- Around line 160-165: The Set-Content cmdlet used when writing to the $envFile
path overwrites the entire env.ps1 file each time this function is called,
causing previously persisted environment variables to be lost. Replace
Set-Content with Add-Content on the line where the escaped environment variable
value is written to $envFile (the line with the backtick-escaped $Name and
$escaped variables). This will append new environment variable entries to the
file instead of replacing the entire contents, preserving all previously
persisted entries.
---
Outside diff comments:
In `@bootstrap-new-machine.ps1`:
- Around line 96-102: The $smokeParams hashtable passed to the smoke test
invocation via Invoke-JsonScript (line 96-102) is missing the -Platform
parameter, causing platform inconsistency between the bootstrap install/doctor
steps and the smoke test execution. Add the -Platform parameter to the
$smokeParams hashtable (which currently includes RepoPath, Mode, and
SkipRepowise) and ensure it uses the same $Mode or platform value that was used
earlier in the bootstrap script so the smoke test runs under the same platform
context.
In `@invoke-code-intel.ps1`:
- Line 152: The invocation of the indexer at the line with `& $indexer
`@indexParams` | Out-Host` is missing the `-Platform` parameter, which causes
inconsistency in artifact root paths between explicit platform runs and pipeline
runs. Add the `-Platform` parameter to the `@indexParams` hashtable or include
it directly in the call to `$indexer` so that the platform value is properly
threaded through to the `update-code-intel-index.ps1` script execution.
In `@Invoke-SentruxAgentTool.ps1`:
- Around line 275-277: The Where-Object filter on line 276 uses a hardcoded
Windows path separator pattern that only matches backslashes, causing the regex
to fail on macOS/Linux where forward slashes are used as separators. Modify the
regex pattern in the Where-Object filter to use a character class that matches
both forward slashes and backslashes (such as [\\\/] or [\\/]) so that the
pattern correctly identifies baseline.json files regardless of the operating
system's path separator convention.
- Around line 324-333: The paths defined in the $noisyDirs array on lines
324-325 use forward slashes, but the comparison on line 332 checks
$normalizedDir against $ignored which contains paths normalized with
DirectorySeparatorChar. On Windows, this causes paths like "static/assets" to
fail matching against "static\assets", silently ignoring exclusions. After
creating $normalizedDir on line 328, also normalize the directory separators by
replacing forward slashes with [IO.Path]::DirectorySeparatorChar to ensure
consistent path format matching across all platforms.
In `@run-code-intel.ps1`:
- Around line 1612-1618: The calls to the scoped repowise script at lines
1612-1618 and 1624-1629 are missing the `-Platform` parameter in the &
$scopedRepowiseScript invocation. Add the `-Platform` parameter to both calls to
pass through explicit platform overrides, ensuring consistent behavior between
scoped and regular repowise runs. Use the same variable or value that would be
used for the non-scoped invocations.
- Around line 1647-1653: Remove the Windows-only `cmd /c` wrappers from the
repowise command execution paths to make them cross-platform compatible. In the
Invoke-LoggedStep calls for "repowise update" and "repowise init", remove the
`cmd /c "exit" |` prefix from the update command and the `cmd /c "echo n" |`
prefix from the init command, keeping only the repowise commands themselves to
ensure they work on macOS/Linux systems as well as Windows.
---
Nitpick comments:
In `@Invoke-ScopedRepowise.ps1`:
- Around line 212-220: The Start-Process command currently always includes the
-WindowStyle Hidden parameter, which is unnecessary on non-Windows platforms.
Modify the Start-Process call to conditionally include the -WindowStyle Hidden
parameter only when running on Windows. Check if the platform is Windows using
the appropriate PowerShell condition (such as $PSVersionTable.Platform or
$IsWindows) and only add the -WindowStyle parameter when that condition is true,
removing it from the current unconditional parameter list.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9f7a1784-13c7-4506-871d-b71bc000efa2
📒 Files selected for processing (25)
.github/workflows/ci.ymlInstall-SentruxVlangOverlay.ps1Invoke-CodeNexusLite.ps1Invoke-ScopedRepowise.ps1Invoke-SentruxAgentTool.ps1README.mdTest-SentruxVlangOverlay.ps1bootstrap-new-machine.ps1check-code-intel-tools.ps1crates/code-nexus-lite/README.mddocs/code-intel-architecture.mdinstall-code-intel-pipeline.ps1invoke-code-intel.ps1overlays/sentrux/vlang/README.mdrun-code-intel.ps1skill/SKILL.mdtemplates/minimax-deploy-checklist.mdtest-code-intel-pipeline.ps1test-code-intel-provider.ps1tools/code-intel-platform.psm1tools/sentrux-shim/sentruxtools/sentrux-shim/sentrux-lite-core.ps1tools/sentrux-shim/sentrux-shim.ps1tools/sentrux-shim/sentrux.cmdupdate-code-intel-index.ps1
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 2 file(s) based on 3 unresolved review comments. A stacked PR containing fixes has been created.
Time taken:
Lines 8–13
jobs:
cross-platform-smoke:
+ permissions:
+ contents: read
+
strategy:
fail-fast: false
matrix: |
ee314d6 to
c1e3155
Compare
c1e3155 to
6b6f0cb
Compare
- test-skill-development-benchmark.ps1: match SKILL.md's forward-slash doc links - run-code-intel.ps1: derive file byte count from already-read content instead of a second Get-Item filesystem call, avoiding a TOCTOU failure on macOS/Ubuntu - skill/SKILL.md: replace hardcoded D:\\ absolute repo paths with portable \ references; fix stray backslash doc links - tools/check-hardcoded-paths.ps1: stop flagging legitimate \C:\Users\Administrator\AppData\Local/USERPROFILE/APPDATA references and doc example paths; add a precise self-referential absolute-path detector for the real bug class Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
…oc link Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
1 participant
What changed
Why
The pipeline was still Windows-machine-shaped in runtime paths and launcher assumptions even after some documentation cleanup. This makes the PowerShell scripts portable while keeping the implementation in PS1 and containing platform differences in a small adapter module.
Validation
Notes
The repo itself currently fails a real sentrux check under the encoded rules once lite-core output/exit propagation is fixed. CI smoke therefore skips sentrux check/gate explicitly; structural rule enforcement remains available for real governed scopes.