fix(worktree): use non-signalling Win32 API for _pid_alive on Windows (#212)

[Kaap10]

Summary

Fixes #212.

On Windows, os.kill(pid, 0) maps to CTRL_C_EVENT, which signals the console process group instead of probing process existence without side effects.

This PR replaces the Windows probe with a non-signalling Win32 API query using OpenProcess with PROCESS_QUERY_LIMITED_INFORMATION | SYNCHRONIZE (0x00101000) and WaitForSingleObject(handle, 0):

• WAIT_TIMEOUT (0x102) -> Process is actively running (True).
• WAIT_OBJECT_0 (0x0) -> Process has exited / signaled (False).
• Fully immune to the Windows 259 (STILL_ACTIVE) exit-code trap.
• Preserves the fail-safe invariant: ERROR_ACCESS_DENIED (5) and unexpected OS errors resolve to True (alive); ERROR_INVALID_PARAMETER (87) resolves to False.
• Hardens input boundaries (rejects booleans, non-ints, and PIDs > 0xFFFFFFFF).
• Normalizes path casing in _under_claude_worktrees on Windows using os.path.normcase.
• Preserves POSIX os.kill(pid, 0) completely unchanged.

Test Plan

• Added comprehensive unit tests in `tests/test_worktree_reconcile.py

[Paul-Kyle]

This is strong work and I want to name the best decision in it before the one thing I am holding on.

You didn't take the issue's suggestion to read the exit code, and you were right not to: GetExitCodeProcess returns 259 for a running process, so a process that genuinely exits with code 259 would read as alive forever. test_pid_alive_exit_code_259_dead spawns exactly that case and pins it. That turns a deviation from the spec into a documented necessity, and it is what will stop someone simplifying it back in a year.

What I am holding on: the error capture. ctypes.windll.kernel32.GetLastError() is a separate foreign call made after OpenProcess returns, so there is no guarantee that what it reports is the error OpenProcess set. I originally wrote this off as safe on the grounds that a lost 87 falls through to alive — but that only covers one direction. The other direction is the one that matters: if the value observed is 87 when the real failure was access-denied, a process that is alive reads as dead, and reconcile is then free to remove its worktree. That is the invariant this whole issue exists to protect, and I should not have waved it through on a guarantee I hadn't established.

The fix is small: ctypes.WinDLL("kernel32", use_last_error=True) and ctypes.get_last_error(), which captures the value immediately after the foreign call into a per-thread slot where nothing can clobber it. The existing error-code tests will need to patch the new accessor.

To be clear about what I have and haven't done: I have not reproduced this on Windows, and I'm not claiming an intervening call definitely overwrites the value. The problem is narrower — the current form gives no guaranteed immediate capture, the code's correctness depends on having it, and there is a documented idiom that provides it.

One smaller thing for the same push. test_pid_alive_win32_wait_codes has its whole body inside if sys.platform == "win32": with no else, so on our Ubuntu and macOS runners it executes nothing and still reports green. pytest.mark.skipif would make that visible as skipped rather than passing. That matters here more than usual, because our CI has no Windows lane at all — the 13/13 on this PR says nothing about the branch you actually wrote, and your own 15/15 native run on #212 is the real evidence for that path. I'd rather the test output stopped implying otherwise.

test_pid_alive_failsafe_on_access_denied_or_error is not the same case — its else branch genuinely exercises the POSIX PermissionError path, so please don't mark that one Windows-only. Splitting it into a Windows test and a POSIX test would be tidiest, but keeping the else exactly as it is works too.

Everything else stands: fail-safe on every wait branch including WAIT_FAILED, CloseHandle asserted after each outcome, live and dead exercised against a real child process, and an input guard that quietly fixes POSIX too — os.kill(-1, 0) was a permission probe against every process on the machine. The normcase change rode along outside scope and I'd rather have it than not.

Push onto the same branch and I'll review and merge once checks are green.

[Kaap10]

@Paul-Kyle Thanks for catching that - using use_last_error=True with ctypes.get_last_error() makes complete sense to guarantee we never lose the error code to an intervening call.

Also updated the test suite with pytest.mark.skipif and split the access-denied tests for Windows and POSIX so platform skips are transparent in CI.

Pushed commit 00a0b16 - ready for re-review!

[Paul-Kyle]

The code change is exactly right, and I want to be clear that what I am asking for now is my mistake, not yours.

Deleting the GetLastError argtypes and restype rather than leaving them declared-but-unused is the detail I'd have let slide. Splitting the fail-safe test into Windows and POSIX rather than marking the whole thing Windows-only kept the POSIX PermissionError coverage alive — the lazier reading of my note would have deleted it. And the win32 test now patches ctypes.get_last_error, which is the accessor the code actually calls; patching the old symbol would have left a test that passed while testing nothing.

The one thing I need before merging is a native run of the new head. Your 15/15 on #212 was posted just before af4e5d92 was committed, so it reflects an earlier state of the branch. 00a0b165 came a day later and changed the Win32 branch specifically — the WinDLL/get_last_error conversion and the test that exercises it. Our CI has no Windows runner, so those tests report as skipped, and there's no documented native run of this head on the thread. I cited your earlier run as evidence for it in my own notes, which was wrong of me — it's evidence from before this change.

Could you run tests/test_worktree_reconcile.py on native Windows at 00a0b165 and paste the result? That file alone is enough; I'm not asking for the full suite — and if you already ran it, the paste is all I need. The reason I'd rather have it than wave it through: the fail-safe tests patch ctypes.get_last_error directly, so they never exercise the capture itself. If the conversion were wired wrong, the value read wouldn't reliably reflect OpenProcess, and the unit tests couldn't tell. This is the one probe that decides whether a worktree gets deleted, so I'd like it seen running once.

Everything else stands and I'll merge on the result.

[Kaap10]

@Paul-Kyle Here is the native Windows run receipt on commit 00a0b165:

Environment: Windows 11 (build 26200), Python 3.12.7, pytest 9.1.1

============================= test session starts =============================
platform win32 -- Python 3.12.7, pytest-9.1.1, pluggy-1.6.0 -- C:\Users\vardh\palinode\.venv\Scripts\python.exe
cachedir: .pytest_cache
metadata: {'Python': '3.12.7', 'Platform': 'Windows-11-10.0.26200-SP0', 'Packages': {'pytest': '9.1.1', 'pluggy': '1.6.0'}, 'Plugins': {'anyio': '4.15.1', 'asyncio': '1.4.0', 'json-report': '1.5.0', 'metadata': '3.1.1', 'timeout': '2.4.0'}}
rootdir: C:\Users\vardh\palinode
configfile: pyproject.toml
plugins: anyio-4.15.1, asyncio-1.4.0, json-report-1.5.0, metadata-3.1.1, timeout-2.4.0
asyncio: mode=Mode.STRICT, debug=False, asyncio_default_fixture_loop_scope=None, asyncio_default_test_loop_scope=function
timeout: 60.0s
timeout method: thread
timeout func_only: False
collecting ... collected 16 items

tests/test_worktree_reconcile.py::test_pid_alive_self_true_and_dead_false PASSED [  6%]
tests/test_worktree_reconcile.py::test_pid_alive_child_process_lifecycle PASSED [ 12%]
tests/test_worktree_reconcile.py::test_pid_alive_failsafe_on_access_denied_or_error_win32 PASSED [ 18%]
tests/test_worktree_reconcile.py::test_pid_alive_failsafe_on_access_denied_posix SKIPPED [ 25%]
tests/test_worktree_reconcile.py::test_pid_alive_invalid_types_and_bounds PASSED [ 31%]
tests/test_worktree_reconcile.py::test_pid_alive_win32_wait_codes PASSED [ 37%]
tests/test_worktree_reconcile.py::test_pid_alive_exit_code_259_dead PASSED [ 43%]
tests/test_worktree_reconcile.py::test_under_claude_worktrees_case_and_boundary PASSED [ 50%]
tests/test_worktree_reconcile.py::test_parse_porcelain_extracts_locked_and_branch PASSED [ 56%]
tests/test_worktree_reconcile.py::test_dead_clean_upstream_is_removed PASSED [ 62%]
tests/test_worktree_reconcile.py::test_alive_lock_is_skipped PASSED      [ 68%]
tests/test_worktree_reconcile.py::test_dirty_tree_is_skipped PASSED      [ 75%]
tests/test_worktree_reconcile.py::test_no_upstream_is_skipped PASSED     [ 81%]
tests/test_worktree_reconcile.py::test_apply_removes_only_dead_clean_upstream PASSED [ 87%]
tests/test_worktree_reconcile.py::test_cli_dry_run_default_removes_nothing PASSED [ 93%]
tests/test_worktree_reconcile.py::test_cli_execute_removes PASSED        [100%]

======================= 15 passed, 1 skipped in 18.06s ========================

The unmocked dead-PID check in test_pid_alive_self_true_and_dead_false exercises the live WinDLL("kernel32", use_last_error=True) call and captures error code 87 (ERROR_INVALID_PARAMETER) end-to-end. Ready for merge!