Skip to content

Commit 43869e2

Browse files
ouyyyyangouyyyyang
committed
fix: stop-local-agent.ps1 never killed anything on Windows
Two bugs made stop.bat a no-op on Windows: 1. The script assigned to $pid, which is a PowerShell automatic variable holding the current process's PID. In PS 7 it is read-only so the assignment throws and the script exits before Stop-Process is even called. In PS 5.1 it is writable but overrides a well-known variable and confuses anything later in the same scope. 2. Even when the kill ran, it only targeted the tracked PID (the node.exe server process, written by server.ts). On Windows, killing a process does not cascade to its children or parents, so the wrapper powershell.exe spawned by start-local-agent.ps1 kept running and held the 'Out-File -Append agent.log' handle open - the user could not delete runtime/agent/. Rewrite: - Rename $pid -> $agentPid (also $match -> $netstatLine). - Walk the full Win32_Process tree (children + the wrapper powershell.exe parent) via Get-CimInstance and force-kill every node in the tree. - Fall back to 'taskkill /F /T' if Stop-Process left anyone alive. - Fail loudly with the list of surviving PIDs instead of printing 'Local agent stopped.' when the kill silently failed. Extend CI: - Replace the inline 'Stop-Process' step with an actual 'stop.bat' call, so the real launcher + the real stop script are exercised. - Verify the tracked node.exe is gone, the pid/port files are cleaned up, and - critically - runtime/agent/ can actually be deleted (proves no leaked file handles).
1 parent 723ec5c commit 43869e2

2 files changed

Lines changed: 170 additions & 31 deletions

File tree

‎.github/workflows/windows-smoke.yml‎

Lines changed: 49 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -82,19 +82,58 @@ jobs:
8282
run: |
8383
start.bat
8484
85-
- name: Stop agent started by start.bat
85+
- name: Capture PID before stop.bat
8686
shell: pwsh
8787
run: |
8888
$pidFile = "runtime/agent/agent.pid"
89-
if (Test-Path $pidFile) {
90-
$agentPid = (Get-Content $pidFile -Raw).Trim()
91-
try {
92-
Stop-Process -Id $agentPid -Force -ErrorAction Stop
93-
Write-Host "Stopped start.bat agent PID $agentPid"
94-
} catch {
95-
Write-Host "start.bat agent PID $agentPid already gone"
96-
}
97-
Remove-Item -Force -ErrorAction SilentlyContinue $pidFile, "runtime/agent/agent.port"
89+
if (-not (Test-Path $pidFile)) {
90+
Write-Error "agent.pid missing after start.bat — agent never wrote it"
91+
exit 1
92+
}
93+
$capturedPid = (Get-Content $pidFile -Raw).Trim()
94+
Write-Host "agent PID before stop: $capturedPid"
95+
"AGENT_PID_BEFORE_STOP=$capturedPid" | Out-File -FilePath $env:GITHUB_ENV -Append
96+
97+
- name: Run stop.bat end-to-end
98+
shell: cmd
99+
run: |
100+
stop.bat
101+
102+
- name: Verify stop.bat killed the full process tree
103+
shell: pwsh
104+
run: |
105+
$ErrorActionPreference = "Stop"
106+
107+
# 1. The tracked node.exe must be gone.
108+
$tracked = [int]$env:AGENT_PID_BEFORE_STOP
109+
try {
110+
$null = Get-Process -Id $tracked -ErrorAction Stop
111+
Write-Error "stop.bat left node.exe PID $tracked running"
112+
exit 1
113+
} catch {
114+
Write-Host "tracked node.exe PID $tracked is gone"
115+
}
116+
117+
# 2. The pid / port files must be cleaned up.
118+
if (Test-Path "runtime/agent/agent.pid") {
119+
Write-Error "stop.bat left runtime/agent/agent.pid behind"
120+
exit 1
121+
}
122+
if (Test-Path "runtime/agent/agent.port") {
123+
Write-Error "stop.bat left runtime/agent/agent.port behind"
124+
exit 1
125+
}
126+
127+
# 3. The runtime/agent folder must be deletable — i.e. no
128+
# wrapper powershell.exe is still holding agent.log open.
129+
try {
130+
Copy-Item -Recurse -Force "runtime/agent" "runtime/agent.stop-test-copy"
131+
Remove-Item -Recurse -Force "runtime/agent"
132+
Write-Host "runtime/agent deleted cleanly — no leaked handles"
133+
Move-Item "runtime/agent.stop-test-copy" "runtime/agent"
134+
} catch {
135+
Write-Error "runtime/agent could not be deleted after stop.bat: $_"
136+
exit 1
98137
}
99138
100139
- name: Start agent via start-local-agent.ps1 (foreground)

‎scripts/dev/stop-local-agent.ps1‎

Lines changed: 121 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@ function Resolve-AgentPid {
1111
$pidText = (Get-Content $PidFile -Raw).Trim()
1212
if ($pidText) {
1313
try {
14-
$process = Get-Process -Id ([int]$pidText) -ErrorAction Stop
15-
return $process.Id
14+
$proc = Get-Process -Id ([int]$pidText) -ErrorAction Stop
15+
return $proc.Id
1616
} catch {
1717
}
1818
}
@@ -22,9 +22,9 @@ function Resolve-AgentPid {
2222
$portText = (Get-Content $PortFile -Raw).Trim()
2323
if ($portText) {
2424
$escaped = [regex]::Escape(":$portText")
25-
$match = netstat -ano -p tcp | Select-String "$escaped\s+.*LISTENING" | Select-Object -First 1
26-
if ($match) {
27-
$parts = ($match -replace "\s+", " ").Trim().Split(" ")
25+
$netstatLine = netstat -ano -p tcp | Select-String "$escaped\s+.*LISTENING" | Select-Object -First 1
26+
if ($netstatLine) {
27+
$parts = ($netstatLine.ToString() -replace "\s+", " ").Trim().Split(" ")
2828
if ($parts.Count -gt 0) {
2929
return [int]$parts[-1]
3030
}
@@ -53,34 +53,134 @@ function Resolve-AgentPort {
5353
return $null
5454
}
5555

56-
$pid = Resolve-AgentPid
57-
$port = Resolve-AgentPort
56+
function Get-ProcessTree {
57+
param([int]$RootPid)
5858

59-
if (-not $pid) {
59+
$tree = New-Object System.Collections.Generic.List[int]
60+
$queue = New-Object System.Collections.Generic.Queue[int]
61+
$queue.Enqueue($RootPid)
62+
$tree.Add($RootPid) | Out-Null
63+
64+
while ($queue.Count -gt 0) {
65+
$current = $queue.Dequeue()
66+
try {
67+
$children = Get-CimInstance Win32_Process -Filter "ParentProcessId=$current" -ErrorAction Stop
68+
foreach ($child in $children) {
69+
$childPid = [int]$child.ProcessId
70+
if (-not $tree.Contains($childPid)) {
71+
$tree.Add($childPid) | Out-Null
72+
$queue.Enqueue($childPid)
73+
}
74+
}
75+
} catch {
76+
}
77+
}
78+
79+
# Also include the direct parent, because on Windows start-local-agent.ps1
80+
# spawns node.exe inside a wrapper powershell.exe. The pid file stores the
81+
# node.exe PID (written by server.ts), but the wrapper holds the agent.log
82+
# file handle via Out-File - if we leave the wrapper alive, the runtime
83+
# folder stays locked.
84+
try {
85+
$rootProc = Get-CimInstance Win32_Process -Filter "ProcessId=$RootPid" -ErrorAction Stop
86+
if ($rootProc -and $rootProc.ParentProcessId) {
87+
$parentPid = [int]$rootProc.ParentProcessId
88+
try {
89+
$parent = Get-CimInstance Win32_Process -Filter "ProcessId=$parentPid" -ErrorAction Stop
90+
if ($parent -and ($parent.Name -match '^(powershell|pwsh)\.exe$')) {
91+
if (-not $tree.Contains($parentPid)) {
92+
$tree.Add($parentPid) | Out-Null
93+
}
94+
}
95+
} catch {
96+
}
97+
}
98+
} catch {
99+
}
100+
101+
return ,$tree.ToArray()
102+
}
103+
104+
function Test-ProcessAlive {
105+
param([int]$TargetPid)
106+
try {
107+
$null = Get-Process -Id $TargetPid -ErrorAction Stop
108+
return $true
109+
} catch {
110+
return $false
111+
}
112+
}
113+
114+
$agentPid = Resolve-AgentPid
115+
$agentPort = Resolve-AgentPort
116+
117+
if (-not $agentPid) {
60118
Write-Host "No local agent PID file found."
119+
Remove-Item -Force -ErrorAction SilentlyContinue $PidFile, $PortFile
61120
exit 0
62121
}
63122

64-
if ($port) {
123+
Write-Host "Stopping local agent (PID $agentPid)..."
124+
125+
# 1. Try a graceful shutdown via the HTTP control endpoint first.
126+
if ($agentPort) {
65127
try {
66-
Invoke-WebRequest -UseBasicParsing -Method Post -Uri "http://127.0.0.1:$port/api/local-control/stop" -TimeoutSec 5 | Out-Null
128+
Invoke-WebRequest -UseBasicParsing -Method Post `
129+
-Uri "http://127.0.0.1:$agentPort/api/local-control/stop" `
130+
-TimeoutSec 5 | Out-Null
67131
for ($attempt = 0; $attempt -lt 15; $attempt += 1) {
68-
try {
69-
$null = Get-Process -Id $pid -ErrorAction Stop
70-
Start-Sleep -Seconds 1
71-
} catch {
72-
break
73-
}
132+
if (-not (Test-ProcessAlive -TargetPid $agentPid)) { break }
133+
Start-Sleep -Seconds 1
74134
}
75135
} catch {
76136
}
77137
}
78138

79-
try {
80-
$process = Get-Process -Id $pid -ErrorAction Stop
81-
Stop-Process -Id $process.Id -Force
82-
} catch {
139+
# 2. Walk the full process tree (children + wrapper parent) and force-kill.
140+
$tree = Get-ProcessTree -RootPid $agentPid
141+
$killed = @()
142+
$failed = @()
143+
foreach ($treePid in $tree) {
144+
if (-not (Test-ProcessAlive -TargetPid $treePid)) { continue }
145+
try {
146+
Stop-Process -Id $treePid -Force -ErrorAction Stop
147+
$killed += $treePid
148+
} catch {
149+
$failed += $treePid
150+
}
151+
}
152+
153+
# 3. Verify nothing survived. If anything did, fall back to taskkill /F /T.
154+
$stillAlive = @()
155+
foreach ($treePid in $tree) {
156+
if (Test-ProcessAlive -TargetPid $treePid) {
157+
$stillAlive += $treePid
158+
}
159+
}
160+
161+
if ($stillAlive.Count -gt 0) {
162+
Write-Host "Stop-Process left $($stillAlive.Count) process(es) alive, falling back to taskkill..."
163+
foreach ($stuckPid in $stillAlive) {
164+
& taskkill.exe /F /T /PID $stuckPid 2>&1 | Out-Null
165+
}
166+
}
167+
168+
$finalSurvivors = @()
169+
foreach ($treePid in $tree) {
170+
if (Test-ProcessAlive -TargetPid $treePid) {
171+
$finalSurvivors += $treePid
172+
}
173+
}
174+
175+
if ($finalSurvivors.Count -gt 0) {
176+
Write-Error "Could not terminate PID(s): $($finalSurvivors -join ', '). The runtime folder may still be locked."
177+
exit 1
83178
}
84179

85180
Remove-Item -Force -ErrorAction SilentlyContinue $PidFile, $PortFile
86-
Write-Host "Local agent stopped."
181+
182+
if ($killed.Count -gt 0) {
183+
Write-Host "Local agent stopped. Killed PID(s): $($killed -join ', ')."
184+
} else {
185+
Write-Host "Local agent stopped."
186+
}

0 commit comments

Comments
 (0)