Skip to content

vcperf chain command fix - #67

Open
es110 wants to merge 1 commit into
github:mainfrom
es110:add-build-perf-cpp
Open

vcperf chain command fix#67
es110 wants to merge 1 commit into
github:mainfrom
es110:add-build-perf-cpp

Conversation

@es110

@es110 es110 commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Modified vcperf invocation guidance in the build-perf-cpp skill so CLI agents can't chain commands in a way that runs /stop when /start failed.

Changes to SKILL.md

  • Route vcperf through cmd.exe (& $env:ComSpec /d /c ...) instead of & $vcperf directly, so $LASTEXITCODE is reliable (direct PowerShell invocation can surface an internal COM HRESULT instead of vcperf's true exit code).
  • Gate the workflow on /start — the build, /stop, and /stopnoanalyze never run unless /start returned exit code 0. After a successful start, /stop still runs on build failure to preserve the partial trace.
  • Updated the Invocation rule (2 rules → 3), Step 3 (elevated grant via cmd.exe with real exit code), Step 4 (gated permission probe), and the MSBuild/CMake/cmd.exe workflow blocks accordingly.

Mirrors the equivalent change in the agency plugin.

@es110
es110 marked this pull request as ready for review August 6, 2026 21:58
Copilot AI balanced review requested due to automatic review settings August 6, 2026 21:58

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Updates the build-perf-cpp vcperf usage guidance to route invocation through cmd.exe and to prevent /stop from running when /start fails, improving correctness of exit-code handling for CLI agents.

Changes:

  • Switches documented vcperf invocation from direct PowerShell (& $vcperf) to cmd.exe-mediated execution for more reliable exit codes.
  • Adds explicit gating so build + /stop only execute if /start succeeds, while still allowing /stop after build failure when /start succeeded.
  • Updates the permission grant + permission probe scripts to follow the new invocation/gating rules.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.


$buildExit = & {
& $vcperf /start /noadmin /level3 MySession | Out-Host
$startOutput = (& $env:ComSpec /d /c "`"$vcperf`" /start /noadmin /level3 MySession" 2>&1 | Out-String).Trim()
msbuild Project.sln /m /t:Rebuild /p:Configuration=Release | Out-Host
$exitCode = $LASTEXITCODE
& $vcperf /stop /templates MySession out.etl /jsonAnalysis out.json | Out-Host
& $env:ComSpec /d /c "`"$vcperf`" /stop /templates MySession `"out.etl`" /jsonAnalysis `"out.json`"" | Out-Host
```powershell
$buildExit = & {
& $vcperf /start /noadmin /level3 MySession | Out-Host
$startOutput = (& $env:ComSpec /d /c "`"$vcperf`" /start /noadmin /level3 MySession" 2>&1 | Out-String).Trim()
cmake --build build --parallel | Out-Host
$exitCode = $LASTEXITCODE
}
& $env:ComSpec /d /c "`"$vcperf`" /stop /templates MySession `"out.etl`" /jsonAnalysis `"out.json`"" | Out-Host
Comment on lines +252 to +254
& $env:ComSpec /d /c "`"$vcperf`" /stop /templates MySession `"out.etl`" /jsonAnalysis `"out.json`"" | Out-Host
$stopExit = $LASTEXITCODE
if ($stopExit -ne 0) { throw "vcperf /stop failed with exit code $stopExit." }
Comment on lines +145 to +150
$stopCmd = "`"$vcperf`" /stopnoanalyze $probe `"$probeEtl`""

$stopOutput = (& $env:ComSpec /d /s /c "`"$stopCmd`"" 2>&1 | Out-String).Trim()

}
$started = ($startExit -eq 0) -and (Test-Path $probeEtl)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants