fix(windows): release the Job Object on every disconnect and Stop path (follow-up to #1234)

[Dumbris]

Follow-up to @LocoLoboZ's #1234 (Windows Job Objects for child-process cleanup). This branch is based on pr-1234, so until #1234 lands the diff here also shows its commit; once #1234 is merged first (as planned) only the five follow-up commits remain. No separate issue exists for this — it closes the gaps found in the cross-model review of #1234. Related: #1234.

Problem

#1234 wraps every stdio/launcher child in a Job Object with KILL_ON_JOB_CLOSE, but the Job was only ever closed on connect/init failure and at mcpproxy exit. The two normal paths never reached it:

1. core Disconnect (F1). mcp-go's Stdio.Close() kills only the immediate cmd.exe and always returns within ~8s, under the 10s mcpClientCloseTimeout, so the "graceful close failed" force-kill step (the only caller of killProcessGroup) was effectively unreachable on Windows. Grandchildren that ignore stdin EOF (node.exe, python.exe) survived every restart, and the Job handle + windowsJobs entry leaked per disconnect until mcpproxy exited. PID reuse then silently overwrote the stale entry, orphaning its handle.
2. launcher Stop (F2). reap() waits for the stdout/stderr pumps before closing the Job, but a grandchild holding the inherited pipe handles never lets the pumps reach EOF once cmd.exe alone is killed. Stop() hung until the caller's 10s ctx expired and the Job was never closed. The comment justifying the deferral ("give the child a chance to shut down gracefully") did not hold: Process.Kill() is already TerminateProcess on Windows.
3. winjob (F3/F5). Hand-rolled JOBOBJECT_EXTENDED_LIMIT_INFORMATION + kernel32 LazyDLL although x/sys/windows v0.47.0 exports everything needed; the hand-rolled layout is 4 bytes short on 386/arm; not gofmt-clean; IsProcessAlive/isProcessGroupAlive were dead code.

Root cause

The Job's lifetime was tied to code paths that only run when something has already gone wrong (timeouts, failures) or when the whole tree has already exited, never to the ordinary disconnect/stop sequence.

Fix

• releaseProcessGroup(pgid, cmd, …) platform hook: Unix no-op; Windows pops the registry entry and closes the Job. DisconnectWithContext calls it (new Step 5b) for every non-Docker stdio disconnect regardless of how the graceful close went.
• Ownership by *exec.Cmd, not PID alone. Disconnect captures pgid+processCmd before Close() reaps the child, and Windows reuses PIDs eagerly, so a concurrently connecting server can register a new Job under the same PID in that window. Registry entries record the owning cmd; releaseProcessGroup and killProcessGroup only take a matching entry, do nothing when the slot belongs to a newer connection, and registerWindowsJob closes any stale entry it displaces. The no-Job fallback is handle-based (cmd.Process.Kill, ErrProcessDone benign) instead of os.FindProcess(pid).
• Launcher: stopLocked's two escalation steps go through h.terminate()/h.kill() handle methods. Unix = the same process-group signals as before. Windows = close the Job (kills the tree, pipes close, pumps drain, reap() proceeds); Process.Kill only on the no-Job fallback. reap()'s pumps-before-Wait ordering is unchanged; its job.Close() is now an idempotent no-op on the Stop path and only does real work on natural whole-tree exit. Natural exit of cmd.exe with a surviving grandchild is deliberately left matching Unix (detected at Stop, not eagerly).
• winjob: windows.SetInformationJobObject / JOBOBJECT_EXTENDED_LIMIT_INFORMATION / JobObjectExtendedLimitInformation / JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE; hand-rolled structs, consts and LazyDLL removed (-35 lines, fixes the 386/arm layout and gofmt). Job.Close/Assign are mutex-guarded and Close idempotent, since Stop and the reaper can now both close the same Job. Dead IsProcessAlive/isProcessGroupAlive removed (note: syscall.STILL_ACTIVE does not exist in Go 1.25, contrary to the review note).

Unix/macOS behaviour is unchanged: process_unix.go gains an ignored cmd parameter and a no-op; launcher_unix.go gains two one-line delegating methods.

Testing

Windows-tagged tests (run on main's windows-latest job; cannot run on the macOS dev host):

• internal/winjob/winjob_test.go — cmd.exe /c set /p x=& ping -n 30 127.0.0.1: the shell blocks on stdin so Assign provably precedes the grandchild; ping.exe inherits the stdout pipe so EOF is the proof it died. Kill cmd.exe alone → no EOF (guards against a vacuous pass); Job.Close() → EOF. Plus nil-safety/idempotency.
• internal/upstream/core/process_windows_test.go — releaseProcessGroup kills the tree and drops the entry; killProcessGroup drops the entry; PID-reuse regression with a live test-owned ping.exe as the new owner: the old owner's release and force-kill must leave both its Job and its process untouched (a PID-based fallback would close the pipe and fail the test), and the takeover must close the stale Job.
• internal/upstream/launcher/launcher_windows_test.go — Stop() with a grandchild holding the pipes returns nil promptly instead of timing out.
• internal/upstream/core/process_release_test.go (all platforms) — releaseProcessGroup is immediate for 0/-1/unregistered.

Local (macOS): go build ./...; GOOS=windows GOARCH=amd64|arm64|386 go build; GOOS=windows go vet ./internal/winjob/ ./internal/upstream/...; go test -race ./internal/upstream/... -count=1; go test -tags server ./internal/serveredition/...; golangci-lint v2 with .github/.golangci.yml → 0 issues on darwin; under GOOS=windows only the three unused hits already present on main (processGracefulTimeout, processTerminationPollInterval, ProcessGroup.logger). Cross-model review (opencode / gpt-6-astra, three file-scoped chunks): CLEAN after 3 fix→re-review rounds (findings fixed: test spawn race, PID-reuse identity on release and force-kill, launcher kill() log noise, vacuous PID-reuse test).

🤖 Generated with Claude Code

[cloudflare-workers-and-pages]

Deploying mcpproxy-docs with    Cloudflare Pages

Latest commit:	8ec33dc
Status:	✅  Deploy successful!
Preview URL:	https://bc632ad7.mcpproxy-docs.pages.dev
Branch Preview URL:	https://fix-1234-followup-job-releas.mcpproxy-docs.pages.dev

View logs

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 36.36364% with 7 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
internal/upstream/core/connection_lifecycle.go	0.00%	2 Missing and 1 partial ⚠️
internal/upstream/launcher/launcher.go	0.00%	0 Missing and 2 partials ⚠️
internal/upstream/core/connection.go	0.00%	1 Missing ⚠️
internal/upstream/core/connection_stdio.go	0.00%	1 Missing ⚠️

📢 Thoughts on this report? Let us know!

[github-actions]

📦 Build Artifacts

Workflow Run: View Run
Branch: fix/1234-followup-job-release

Available Artifacts

• archive-darwin-amd64 (29 MB)
• archive-darwin-arm64 (27 MB)
• archive-linux-amd64 (17 MB)
• archive-linux-arm64 (16 MB)
• archive-windows-amd64 (29 MB)
• archive-windows-arm64 (26 MB)
• frontend-dist-pr (0 MB)
• installer-dmg-darwin-amd64 (24 MB)
• installer-dmg-darwin-arm64 (21 MB)

How to Download

Option 1: GitHub Web UI (easiest)

1. Go to the workflow run page linked above
2. Scroll to the bottom "Artifacts" section
3. Click on the artifact you want to download

Option 2: GitHub CLI

gh run download 34675753601 --repo smart-mcp-proxy/mcpproxy-go

Note: Artifacts expire in 14 days.