feat(sandbox): Windows backend via Job Objects - #20
Merged
Conversation
The PR #19 merge dropped the upper half of sessions.py (imports, Session, save_session, load_session), which broke test collection repo-wide with a NameError. Restore it from the last good revision. delete_session also had a contradictory contract: the implementation returned Path and raised on a missing file, while the bridge handler and the newer tests expect a bool (True if removed, False if absent). Align the implementation with that contract and drop the stale test that asserted the raise behavior.
execute_python now dispatches on sys.platform in api.py: darwin keeps the existing Seatbelt backend untouched, win32 routes to a new executor_windows.py, anything else raises a clear SandboxError. Callers (CLI, desktop bridge) are unchanged. The Windows backend uses Job Objects (stdlib ctypes, no pywin32) for resource isolation matching the macOS rlimit tier: process-memory cap, ActiveProcessLimit fork-bomb protection, per-process CPU-time limit, and KILL_ON_JOB_CLOSE + TerminateJobObject for whole-tree wall-clock kill. The interpreter is launched CREATE_SUSPENDED, assigned to the job, then resumed so limits apply first. This is a deliberately weaker tier than Seatbelt on filesystem/network isolation (documented honestly in README's parity table); AppContainer is left for a future version. runtime_python() now resolves Scripts/python.exe on Windows. Tests: test_executor_windows.py mirrors the macOS cases (Windows-only, skips elsewhere); test_dispatch.py covers the platform routing on every OS. macOS executor.py and the Seatbelt profile are byte-for-byte unchanged. See WINDOWS_TESTING.md for the manual checklist to validate on a real Windows host.
…ows backend list_sessions() returns SessionSummary dataclasses, but the bridge handler still unpacked them as (name, saved_at) tuples — a pre-existing master mypy failure. Read the dataclass fields directly (and drop the now-redundant per-session load_session call). Also apply ruff format to executor_windows.py so the CI format check passes.
6 tasks
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
1 participant
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.
Summary
Adds Windows support to the sandbox without touching the macOS Seatbelt backend.
execute_pythonnow dispatches onsys.platforminapi.py:darwin→ the existing Seatbelt backend (byte-for-byte unchanged),win32→ a newexecutor_windows.py, anything else → a clearSandboxError. The CLI and desktop-bridge callers are unchanged.Windows isolation (Job Objects, stdlib
ctypes— no pywin32)Resource isolation matching the macOS rlimit tier:
ProcessMemoryLimit)ActiveProcessLimit=1→ fork-bomb protectionKILL_ON_JOB_CLOSE+TerminateJobObjecton timeout → whole-tree wall-clock killCREATE_SUSPENDED, assigned to the job, then resumed, so limits apply before any code runs.Honest scope
The Windows backend is a deliberately weaker tier than Seatbelt on filesystem/network isolation, documented in the README parity table; AppContainer is left for a future version.
runtime_python()now resolvesScripts/python.exeon Windows.Commits
fix(sessions)— Repairs a regression introduced by the PR feat(sessions): add delete command and enrich sessions list #19 merge: the upper half ofsessions.py(imports,Session,save_session,load_session) had been dropped, breaking test collection repo-wide with aNameError. Restored from the last good revision.delete_sessionalso had a contradictory contract (returnedPathand raised on a missing file, while the bridge handler and newer tests expect abool); aligned the implementation to returnbool(True if removed, False if absent) and dropped the stale test that asserted the raise behavior.feat(sandbox)— The Windows support described above.fix(bridge)—list_sessions()now returnsSessionSummarydataclasses, but the bridgesessions.list_handler still unpacked them as(name, saved_at)tuples (a pre-existing master mypy failure). Reads the dataclass fields directly and drops the redundant per-sessionload_sessioncall. Also appliesruff formattoexecutor_windows.pyso the CI format check passes.Tests / CI gates (all green locally)
ruff check .— cleanruff format --check .— cleanmypy packages shared— 0 errors (the pre-existing bridge failure is now fixed)pytest --cov --cov-fail-under=75— 210 passed, coverage 83% (integration tests skip without Ollama, as in CI)executor.pyand the Seatbelt profile are byte-for-byte unchangedtest_executor_windows.pyis Windows-only (skips on macOS/Linux CI);test_dispatch.pycovers the platform routing on every OS.packages/sandbox/WINDOWS_TESTING.mdhas a step-by-step manual checklist..exepackaging are separate follow-ups.