You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The step uses the PowerShell call operator '&', but the job/step appears to run under bash (previous step explicitly sets shell: bash, and this step doesn't override it). Using '&' in bash will fail. Either set shell: pwsh for this step or use a bash-compatible command.
The Windows path with spaces is quoted, which is correct, but ensure invocation format matches the chosen shell. For pwsh, & "C:\Program Files (x86)\NSIS\makensis.exe" installer.nsi is fine; for bash on Windows runners, use "/c/Program Files (x86)/NSIS/makensis.exe" or call via cmd.exe.
run: | cd agents/capture-agent-py & "C:\Program Files (x86)\NSIS\makensis.exe" installer.nsi
To make locating NSIS more robust, search for makensis.exe in multiple common installation directories and fall back to Get-Command instead of relying on a single hardcoded path.
- name: Install NSIS
shell: pwsh
run: |
choco install nsis -y
if ($LASTEXITCODE -ne 0) { throw "Chocolatey failed to install NSIS. Exit code: $LASTEXITCODE" }
- $pf86 = ${env:ProgramFiles(x86)}- $nsisRoot = Join-Path -Path $pf86 -ChildPath 'NSIS\Bin'- if (-not (Test-Path -LiteralPath $nsisRoot)) { throw "NSIS Bin folder not found at $nsisRoot" }- $makensis = Join-Path $nsisRoot 'makensis.exe'- if (-not (Test-Path -LiteralPath $makensis)) { throw "makensis.exe not found in path: $nsisRoot" }- & $makensis /VERSION | Out-Host- Add-Content -Path $env:GITHUB_PATH -Value $nsisRoot+ $candidates = @()+ if ($env:ProgramFiles) { $candidates += (Join-Path $env:ProgramFiles 'NSIS\Bin') }+ if ($env:'ProgramFiles(x86)'){ $candidates += (Join-Path $env:'ProgramFiles(x86)' 'NSIS\Bin') }+ $candidates += 'C:\ProgramData\chocolatey\lib\nsis\tools'++ $makensisPath = $null+ foreach ($dir in $candidates) {+ if (Test-Path -LiteralPath $dir) {+ $exe = Join-Path $dir 'makensis.exe'+ if (Test-Path -LiteralPath $exe) { $makensisPath = $exe; break }+ }+ }+ if (-not $makensisPath) {+ $cmd = Get-Command makensis -ErrorAction SilentlyContinue+ if ($cmd) { $makensisPath = $cmd.Source }+ }+ if (-not $makensisPath) { throw "makensis.exe not found in common locations or PATH." }++ & $makensisPath /VERSION | Out-Host+ Add-Content -Path $env:GITHUB_PATH -Value (Split-Path -Path $makensisPath -Parent)+
Apply / Chat
Suggestion importance[1-10]: 8
__
Why: The suggestion correctly identifies that hardcoding the NSIS installation path is brittle and proposes a much more robust solution by searching multiple common locations, making the workflow more resilient to environment changes.
Medium
Possible issue
Harden NSIS install and PATH setup
Improve the NSIS installation script by using a try/catch block for more reliable error handling with choco, adding flags for non-interactive use, and robustly updating the PATH.
shell: pwsh
run: |
- choco install nsis -y- if ($LASTEXITCODE -ne 0) { throw "Chocolatey failed to install NSIS. Exit code: $LASTEXITCODE" }+ try {+ choco install nsis -y --no-progress --limit-output+ } catch {+ throw "Chocolatey failed to install NSIS: $($_.Exception.Message)"+ }
$pf86 = ${env:ProgramFiles(x86)}
+ if (-not $pf86) { throw "ProgramFiles(x86) environment variable is not set." }
$nsisRoot = Join-Path -Path $pf86 -ChildPath 'NSIS\Bin'
if (-not (Test-Path -LiteralPath $nsisRoot)) { throw "NSIS Bin folder not found at $nsisRoot" }
$makensis = Join-Path $nsisRoot 'makensis.exe'
if (-not (Test-Path -LiteralPath $makensis)) { throw "makensis.exe not found in path: $nsisRoot" }
+ # Ensure available in current step and subsequent steps+ $env:PATH = "$nsisRoot;$env:PATH"+ if (-not (Test-Path -LiteralPath $env:GITHUB_PATH)) { throw "GITHUB_PATH file not available." }+ "$nsisRoot" | Out-File -FilePath $env:GITHUB_PATH -Append -Encoding utf8
& $makensis /VERSION | Out-Host
- Add-Content -Path $env:GITHUB_PATH -Value $nsisRoot
Apply / Chat
Suggestion importance[1-10]: 7
__
Why: The suggestion correctly points out that try/catch is more robust for handling choco failures than checking $LASTEXITCODE and proposes several other improvements to make the CI script more reliable.
Medium
Validate makensis execution result
After running makensis /VERSION, check $LASTEXITCODE to ensure the command executed successfully and fail the step if it did not.
Why: The suggestion correctly proposes adding an explicit check on the exit code of makensis to ensure it ran successfully, which improves the script's reliability and prevents potential downstream failures.
✅ Fix invalid env var expansionSuggestion Impact:The code was modified to avoid using ${...} directly in Join-Path; it now assigns the ProgramFiles(x86) env var to $pf86 and uses that in Join-Path. Although it still uses ${env:ProgramFiles(x86)} for the assignment (not $env:...), the change reflects the suggestion’s intent to correct the usage in the Join-Path context.
code diff:
- $nsisRoot = Join-Path -Path ${env:ProgramFiles(x86)} -ChildPath 'NSIS\Bin'+ if ($LASTEXITCODE -ne 0) { throw "Chocolatey failed to install NSIS. Exit code: $LASTEXITCODE" }+ $pf86 = ${env:ProgramFiles(x86)}+ $nsisRoot = Join-Path -Path $pf86 -ChildPath 'NSIS\Bin'
if (-not (Test-Path -LiteralPath $nsisRoot)) { throw "NSIS Bin folder not found at $nsisRoot" }
- if (-not (Test-Path -LiteralPath (Join-Path $nsisRoot "makensis.exe"))) { throw "makensis.exe not found in $nsisRoot" }+ $makensis = Join-Path $nsisRoot 'makensis.exe'+ if (-not (Test-Path -LiteralPath $makensis)) { throw "makensis.exe not found in path: $nsisRoot" }+ & $makensis /VERSION | Out-Host
Correct the invalid PowerShell syntax for accessing the ProgramFiles(x86) environment variable. The ${...} syntax is incorrect; use $env:ProgramFiles(x86) instead.
Why: The suggestion correctly identifies that ${env:ProgramFiles(x86)} is invalid PowerShell syntax and will cause the workflow step to fail. Fixing this syntax error is critical for the functionality of the build installer workflow.
✅ Validate Chocolatey install successSuggestion Impact:The commit added an exit code check right after the Chocolatey install command and threw a descriptive error on failure, aligning with the suggestion. It also refactored path handling and added a makensis presence/version check.
code diff:
choco install nsis -y
- $nsisRoot = Join-Path -Path ${env:ProgramFiles(x86)} -ChildPath 'NSIS\Bin'+ if ($LASTEXITCODE -ne 0) { throw "Chocolatey failed to install NSIS. Exit code: $LASTEXITCODE" }+ $pf86 = ${env:ProgramFiles(x86)}+ $nsisRoot = Join-Path -Path $pf86 -ChildPath 'NSIS\Bin'
if (-not (Test-Path -LiteralPath $nsisRoot)) { throw "NSIS Bin folder not found at $nsisRoot" }
- if (-not (Test-Path -LiteralPath (Join-Path $nsisRoot "makensis.exe"))) { throw "makensis.exe not found in $nsisRoot" }+ $makensis = Join-Path $nsisRoot 'makensis.exe'+ if (-not (Test-Path -LiteralPath $makensis)) { throw "makensis.exe not found in path: $nsisRoot" }+ & $makensis /VERSION | Out-Host
Add-Content -Path $env:GITHUB_PATH -Value $nsisRoot
Add a check for the exit code after the choco install nsis command to handle potential installation failures and provide a clear error message.
- name: Install NSIS
shell: pwsh
run: |
choco install nsis -y
- $nsisRoot = Join-Path -Path ${env:ProgramFiles(x86)} -ChildPath 'NSIS\Bin'+ if ($LASTEXITCODE -ne 0) { throw "Chocolatey failed to install NSIS. Exit code: $LASTEXITCODE" }+ $nsisRoot = Join-Path -Path '${env:ProgramFiles(x86)}' -ChildPath 'NSIS\Bin'
if (-not (Test-Path -LiteralPath $nsisRoot)) { throw "NSIS Bin folder not found at $nsisRoot" }
if (-not (Test-Path -LiteralPath (Join-Path $nsisRoot "makensis.exe"))) { throw "makensis.exe not found in $nsisRoot" }
Add-Content -Path $env:GITHUB_PATH -Value $nsisRoot
[Suggestion processed]
Suggestion importance[1-10]: 7
__
Why: This suggestion correctly identifies a potential point of failure and proposes a robust error handling mechanism, which improves the reliability and debuggability of the workflow script.
Medium
General
✅ Validate NSIS executable runsSuggestion Impact:The commit implemented the suggestion by assigning makensis path to a variable, validating its existence, and executing "& $makensis /VERSION | Out-Host" to verify it runs.
code diff:
+ $makensis = Join-Path $nsisRoot 'makensis.exe'+ if (-not (Test-Path -LiteralPath $makensis)) { throw "makensis.exe not found in path: $nsisRoot" }+ & $makensis /VERSION | Out-Host
Improve the NSIS installation validation by not only checking for the existence of makensis.exe but also verifying it is executable by running makensis.exe /VERSION.
-if (-not (Test-Path -LiteralPath (Join-Path $nsisRoot "makensis.exe"))) { throw "makensis.exe not found in $nsisRoot" }+$makensis = Join-Path $nsisRoot 'makensis.exe'+if (-not (Test-Path -LiteralPath $makensis)) { throw "makensis.exe not found in path: $nsisRoot" }+& $makensis /VERSION | Out-Host
[Suggestion processed]
Suggestion importance[1-10]: 4
__
Why: The suggestion improves the script's robustness by adding a step to verify that makensis.exe is not only present but also executable. This is a good practice for ensuring the tool is correctly installed and configured before it is used in later steps.
✅ Fix incorrect env var expansionSuggestion Impact:The commit replaced the incorrect "${env:ProgramFiles(x86)}\NSIS\Bin" with a Join-Path construction and used Test-Path -LiteralPath, aligning with the suggestion. It also adjusted the shell placement and added an extra makensis.exe check.
code diff:
- $nsisRoot = "${env:ProgramFiles(x86)}\NSIS\Bin"- if (-not (Test-Path $nsisRoot)) { throw "NSIS Bin folder not found at $nsisRoot" }+ $nsisRoot = Join-Path -Path ${env:ProgramFiles(x86)} -ChildPath 'NSIS\Bin'+ if (-not (Test-Path -LiteralPath $nsisRoot)) { throw "NSIS Bin folder not found at $nsisRoot" }+ if (-not (Test-Path -LiteralPath (Join-Path $nsisRoot "makensis.exe"))) { throw "makensis.exe not found in $nsisRoot" }
Correct the invalid PowerShell syntax for environment variable expansion to prevent the workflow step from failing. Use $env:'ProgramFiles(x86)' and Join-Path for robust path construction.
- name: Install NSIS
run: |
choco install nsis -y
- $nsisRoot = "${env:ProgramFiles(x86)}\NSIS\Bin"- if (-not (Test-Path $nsisRoot)) { throw "NSIS Bin folder not found at $nsisRoot" }+ $nsisRoot = Join-Path -Path $env:'ProgramFiles(x86)' -ChildPath 'NSIS\Bin'+ if (-not (Test-Path -LiteralPath $nsisRoot)) { throw "NSIS Bin folder not found at $nsisRoot" }
Add-Content -Path $env:GITHUB_PATH -Value $nsisRoot
shell: pwsh
[Suggestion processed]
Suggestion importance[1-10]: 9
__
Why: The suggestion correctly identifies a bug where the PowerShell syntax for expanding the ProgramFiles(x86) environment variable is incorrect, which would cause the workflow step to fail. The proposed fix is robust and follows best practices.
High
Possible issue
✅ Fix NSIS path handling and exportSuggestion Impact:The commit switched to pwsh for the step, replaced the hardcoded NSIS path with Join-Path, added a makensis.exe existence check, and improved Test-Path usage; however, it kept Add-Content instead of switching to Out-File and did not add the ProgramFiles fallback.
code diff:
+ shell: pwsh
run: |
choco install nsis -y
- $nsisRoot = "${env:ProgramFiles(x86)}\NSIS\Bin"- if (-not (Test-Path $nsisRoot)) { throw "NSIS Bin folder not found at $nsisRoot" }+ $nsisRoot = Join-Path -Path ${env:ProgramFiles(x86)} -ChildPath 'NSIS\Bin'+ if (-not (Test-Path -LiteralPath $nsisRoot)) { throw "NSIS Bin folder not found at $nsisRoot" }+ if (-not (Test-Path -LiteralPath (Join-Path $nsisRoot "makensis.exe"))) { throw "makensis.exe not found in $nsisRoot" }
Add-Content -Path $env:GITHUB_PATH -Value $nsisRoot
Improve the NSIS installation step by using Join-Path for robust path construction, adding a fallback for different CPU architectures, and using Out-File with explicit UTF-8 encoding to correctly update the GITHUB_PATH.
- name: Install NSIS
+ shell: pwsh
run: |
choco install nsis -y
- $nsisRoot = "${env:ProgramFiles(x86)}\NSIS\Bin"+ $programFilesX86 = ${env:ProgramFiles(x86)}+ if (-not $programFilesX86) { $programFilesX86 = $env:ProgramFiles } # fallback on ARM/other images+ $nsisRoot = Join-Path $programFilesX86 "NSIS\Bin"
if (-not (Test-Path $nsisRoot)) { throw "NSIS Bin folder not found at $nsisRoot" }
- Add-Content -Path $env:GITHUB_PATH -Value $nsisRoot- shell: pwsh+ if (-not (Test-Path (Join-Path $nsisRoot "makensis.exe"))) { throw "makensis.exe not found in $nsisRoot" }+ "$nsisRoot" | Out-File -FilePath $env:GITHUB_PATH -Encoding utf8 -Append
[Suggestion processed]
Suggestion importance[1-10]: 8
__
Why: The suggestion correctly identifies multiple robustness issues: lack of a fallback for ProgramFiles(x86) on non-x86 runners, potential encoding problems with Add-Content, and brittle path construction. The proposed changes make the workflow script significantly more reliable and portable.
✅ Fix NSIS PATH and escapingSuggestion Impact:The commit updated the NSIS installation step to use ProgramFiles(x86)\NSIS\Bin, added a Test-Path check with a throw on failure, and replaced echo with Add-Content to append to GITHUB_PATH.
code diff:
- name: Install NSIS
run: |
choco install nsis -y
- echo "C:\Program Files (x86)\NSIS" >> $env:GITHUB_PATH+ $nsisRoot = "${env:ProgramFiles(x86)}\NSIS\Bin"+ if (-not (Test-Path $nsisRoot)) { throw "NSIS Bin folder not found at $nsisRoot" }+ Add-Content -Path $env:GITHUB_PATH -Value $nsisRoot
Correct the NSIS path added to $env:GITHUB_PATH to point to the Bin directory containing the executable. Additionally, improve the script's robustness by using environment variables and adding a path existence check.
- name: Install NSIS
run: |
choco install nsis -y
- echo "C:\Program Files (x86)\NSIS" >> $env:GITHUB_PATH+ $nsisRoot = "${env:ProgramFiles(x86)}\NSIS\Bin"+ if (-not (Test-Path $nsisRoot)) { throw "NSIS Bin folder not found at $nsisRoot" }+ Add-Content -Path $env:GITHUB_PATH -Value $nsisRoot
shell: pwsh
[Suggestion processed]
Suggestion importance[1-10]: 9
__
Why: The suggestion correctly identifies a bug where the wrong directory is added to the PATH, which would cause the workflow to fail. The proposed fix is robust, uses idiomatic PowerShell, and includes helpful error handling.
✅ Avoid hardcoding executable pathSuggestion Impact:The workflow now adds "C:\Program Files (x86)\NSIS" to PATH during NSIS installation and replaces the hardcoded executable path with calling "makensis" directly.
Instead of hardcoding the full path to makensis.exe, add its directory to the PATH environment variable using $env:GITHUB_PATH and then call the executable by name. This improves the workflow's robustness against changes in the runner environment.
Why: The suggestion correctly points out that hardcoding the path to an executable is brittle and proposes a more robust solution by adding the directory to the PATH, which is a standard best practice in GitHub Actions.
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
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.
PR Type
Other
Description
Diagram Walkthrough
File Walkthrough
build-installers.yml
Update NSIS installer build command.github/workflows/build-installers.yml
makensiscommand with explicit path to NSIS executable&for command execution