Python: reset $LASTEXITCODE per command in persistent PowerShell sessions - #8206
Merged
Eduard van Valkenburg (eavanvalkenburg) merged 3 commits intoSep 10, 2026
Conversation
…ions $LASTEXITCODE is a session-wide automatic variable that only native (external) processes update, so in a long-lived session it stays set after such a command. Every later cmdlet-only command then took the $LASTEXITCODE branch in the sentinel script and reported that stale value instead of 0. Clear it in the global scope before invoking the user command so a non-null value can only have come from this command. The global scope qualifier matters: assigning the unqualified name inside the script block shadows the global the engine writes to, which would hide real native exit codes.
Leo Camus (Dev-next-gen)
temporarily deployed
to
github-app-auth
September 9, 2026 20:33 — with
GitHub Actions
Inactive
Leo Camus (Dev-next-gen)
temporarily deployed
to
github-app-auth
September 9, 2026 20:33 — with
GitHub Actions
Inactive
Leo Camus (Dev-next-gen)
temporarily deployed
to
github-app-auth
September 9, 2026 20:34 — with
GitHub Actions
Inactive
Copilot started reviewing on behalf of
Leo Camus (Dev-next-gen)
September 9, 2026 20:34
View session
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Resetting $global:LASTEXITCODE breaks observable state across persistent commands.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Fixes stale PowerShell exit codes in persistent Python shell sessions.
Changes:
- Resets
$LASTEXITCODEbefore each command. - Adds a Windows regression test.
File summaries
| File | Description |
|---|---|
python/packages/tools/agent_framework_tools/shell/_session.py |
Adjusts PowerShell exit-code handling. |
python/packages/tools/tests/test_local_shell_tool.py |
Tests stale exit-code prevention. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Balanced
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
Contributor
Author
|
@microsoft-github-policy-service agree |
Review feedback: clearing $global:LASTEXITCODE before the user's command destroys state persistent mode exists to carry. After `cmd /c exit 3`, a following `Write-Output $LASTEXITCODE` printed nothing instead of 3. Snapshot the value before the command and compare afterwards instead. The variable is never assigned, so the user's own commands still read it, and a value that differs can only have been written by this command. $? is read on the statement right after Invoke-Expression, since anything else clobbers it. The regression test now also asserts the readback prints 3 while its own exit code is 0, which fails against the previous approach.
Leo Camus (Dev-next-gen)
temporarily deployed
to
github-app-auth
September 10, 2026 01:21 — with
GitHub Actions
Inactive
Leo Camus (Dev-next-gen)
temporarily deployed
to
github-app-auth
September 10, 2026 01:23 — with
GitHub Actions
Inactive
Eduard van Valkenburg (eavanvalkenburg)
approved these changes
Sep 10, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Motivation & Context
I was going through the shell tool on Windows and noticed that
LocalShellTool(mode="persistent")reports the exit code of an earlier command for any later command that only runs cmdlets.ShellSession._build_scriptderives the return code like this:$LASTEXITCODEis a session-wide automatic variable that PowerShell only updates when a native (external) executable exits — cmdlets leave it untouched. The session is long-lived, so once any command in it has run an external process,$LASTEXITCODEstays set and every subsequent cmdlet-only command takes the first branch and reports that stale value instead of 0.Practically, the tool tells the agent a command failed when it succeeded, and keeps doing it for the rest of the session until another external command happens to reset the variable.
Here is what I get on
main(Windows 11, Windows PowerShell 5.1, Python 3.12.9), drivingShellSessiondirectly:The last two printed their output fine, they just carry rc 3 from the
cmd /c exit 3two commands earlier.The reason this survived is that the CI matrix is
ubuntu-latestin every.github/workflows/python-*.yml, so the PowerShell tests already intest_local_shell_tool.py— the ones behind@pytest.mark.skipif(sys.platform != "win32", ...)— never actually execute anywhere.Description & Review Guide
One line in the PowerShell branch of
_build_script: clear$LASTEXITCODEbefore invoking the user command, so a non-null value below can only have come from this command.Plus a Windows-gated regression test alongside the existing PowerShell ones.
Only the rc computed for cmdlet-only commands in persistent PowerShell mode changes, and only from a wrong value to the right one. Native exit codes, cmdlet errors and stateless mode are untouched. Stateless mode was never affected — it spawns a fresh process per command and takes the rc from the process itself.
Same script after the change:
New test on
main:And with the fix, the whole
packages/toolssuite on Windows:ruff check packages/tools/andruff format --checkare clean.The
$global:scope qualifier is load-bearing and I want a second pair of eyes on it. The engine writes$LASTEXITCODEinto the global scope, so assigning the unqualified name inside the& { ... }block creates a local that shadows it, and real native exit codes then become invisible. I checked both spellings in a plain shell before picking one:One thing I deliberately left alone:
dotnet/src/Microsoft.Agents.AI.Tools.Shell/ShellSession.cshas the same two lines around line 592 and looks like it has the same behaviour, but I have not built or run the .NET side so I did not want to touch it on a hunch. Happy to open a separate .NET PR if you confirm it, or it may be simpler for someone with that toolchain set up to port the one-liner.Related Issue
Fixes #8205
Contribution Checklist
AI tools used