refactor(python): direct Monty integration, remove worker subprocess#203
refactor(python): direct Monty integration, remove worker subprocess#203
Conversation
…tion Remove the bashkit-monty-worker crate and IPC subprocess architecture. Monty now runs directly in the host process with resource limits enforced by Monty's own runtime (memory, allocations, time, recursion). Changes: - Delete crates/bashkit-monty-worker/ (worker binary, IPC protocol) - Remove PythonIsolation enum (no more Subprocess/InProcess/Auto modes) - Simplify python.rs to direct Monty integration (~660 lines, was ~1468) - Add monty as optional git dep behind `python` feature flag - Add 117 integration tests covering execution, VFS bridging, resource limits, bash interop, error handling, security, and edge cases - Update threat model docs and specs to reflect direct integration - Remove python_subprocess_tests.rs (no longer applicable) - Update threat_model_tests.rs (remove subprocess security module) Python remains optional and disabled by default. Enable with: cargo build --features python https://claude.ai/code/session_01MYaC4cZRkJiRzy81MsZ9tz
…odel Update 006-threat-model.md and 011-python-builtin.md to reflect direct Monty integration: remove PythonIsolation, worker binary, IPC timeout/line limit, and subprocess isolation references. TM-PY-023 through TM-PY-026 removed (subprocess-specific threats). https://claude.ai/code/session_01MYaC4cZRkJiRzy81MsZ9tz
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 51aed44ebe
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| tracing = { workspace = true, optional = true } | ||
|
|
||
| # Embedded Python interpreter (optional) | ||
| monty = { git = "https://github.com/pydantic/monty", version = "0.0.4", optional = true } |
There was a problem hiding this comment.
Remove non-registry dependency from published crate
Adding monty as a git dependency here will block releases because cargo publish -p bashkit (run in .github/workflows/publish.yml line 42) rejects crates that depend on non-registry sources, even when that dependency is optional. This commit therefore makes the publish workflow fail for every release until monty comes from crates.io or is excluded from the published manifest.
Useful? React with 👍 / 👎.
| tracing = { workspace = true, optional = true } | ||
|
|
||
| # Embedded Python interpreter (optional) | ||
| monty = { git = "https://github.com/pydantic/monty", version = "0.0.4", optional = true } |
There was a problem hiding this comment.
Pin Monty git source to a fixed revision
This dependency uses a git URL without rev/tag, so Cargo resolves whatever commit is at the default branch head; that makes --features python builds non-reproducible and can cause sudden breakages when upstream changes. Pinning to an explicit commit or tag would keep builds stable and make failures attributable to local changes.
Useful? React with 👍 / 👎.
Transitive dep from monty; minor version bump from already-vetted 1.0.20. https://claude.ai/code/session_01MYaC4cZRkJiRzy81MsZ9tz
Summary
bashkit-monty-workercrate (subprocess binary, IPC protocol, JSON wire types)MontyRun::start()+ OsCall resume loopPythonIsolationenum (Subprocess/InProcess/Automodes) — only direct mode remainsmontyas optional git dependency behindpythonfeature flag (still not on crates.io)--features python)Test plan
cargo build --features python— compilescargo build(without python) — still compiles (feature is optional)cargo clippy --features python --all-targets -- -D warnings— cleancargo test --features python --lib -- python— 38 unit tests passcargo test --features python --test python_integration_tests— 117 integration tests passcargo test --features python --test threat_model_tests— 114 tests pass (subprocess security module removed)cargo test --features python --test spec_tests— 14 tests passcargo fmt --check— clean