Skip to content

websocket protocol#509

Merged
samuelcolvin merged 28 commits into
mainfrom
proto-serde
Jun 29, 2026
Merged

websocket protocol#509
samuelcolvin merged 28 commits into
mainfrom
proto-serde

Conversation

@samuelcolvin

@samuelcolvin samuelcolvin commented Jun 24, 2026

Copy link
Copy Markdown
Member

Summary by cubic

Adds a WebSocket transport and a new monty-cpython worker that runs embedded CPython with per‑session installs in ./.venv via uv. Finalizes the wire protocol (Feed, InstallDependencies, version‑skew check), hardens the pool, and returns real CPython tracebacks to the parent; WS workers are single‑use per checkout. Includes a relay script and tests for the WS path, plus teardown and URL‑handling fixes.

  • New Features

    • monty-pool: MontyTransport::{Subprocess, Websocket}; per‑checkout WS via tungstenite with wss:// (installs a rustls aws_lc_rs CryptoProvider), dial bounded by request_timeout; only Subprocess is pre‑warmed; WS workers are single‑use and dropped at checkout end; watchdog killing is transport‑agnostic via a shared Interrupt; WS frame/message limits at MAX_FRAME_LEN, oversize requests return Malformed; rejects empty or flag‑like install_dependencies requirements.
    • New crate monty-cpython: embedded‑CPython child; one session per process — Reset/Shutdown both acknowledge and exit; subcommands: subprocess (stdio) and websocket (client). Uses SandboxGlobals (dict subclass) for name resolution via __missing__; runner.py with run and a single asyncio loop for top‑level await. Supports InstallDependencies into ./.venv via uv pip --python .venv/bin/python, prepends its site‑packages on sys.path and calls site.addsitedir; auto‑installs PEP 723 dependencies on feed; exceptions now carry a reconstructed Monty traceback from CPython, keeping only user frames compiled under Configure.script_name. Dockerfile builds the image, pre‑creates the venv, and sets WORKDIR /app.
    • Protocol: added InstallDependencies; ReplFeedFeed; Configure carries monty_version (MONTY_VERSION) and children reject mismatches with FatalError. write_frame uses encode_to_capped_vec; reset_decode_budget is public for non‑framed transports; oversize turn‑enders are shrunk to a small Error; missing NamedValue.value is a violation.
    • Python: pydantic_monty.AsyncMontyWebsocket(url) for remote sessions; MontySession.install_dependencies(requirements) (sandbox worker rejects with MontyRuntimeError); value‑conversion modules are public in pydantic-monty (cdylib + rlib). Top‑level await runs in one event loop. Added AsyncMontyWebsocket relay tests and a websockets dev dep.
    • JS: MontySession.installDependencies(requirements) with sandbox rejection and an OkTurn with no value; types/tests updated; minor fixes (e.g. CPython sys.stdout.write returns a character count).
    • Small fixes: CLI conflict checks now validate correctly; stdout/stderr handling fixed; docs reflect subprocess vs single‑use WS transport, AsyncMontyWebsocket timeouts, and that remote WS workers are not the Monty sandbox; new scripts/websocket_relay.py bridges WS ↔ monty subprocess for local dev/tests; relay now stops on first pump completion and cancels/reaps both sides; printed ws:// uses proper IPv6 brackets and maps wildcard binds to loopback; WS teardown shuts the TCP socket down directly (avoids blocking on peers that stop draining).
  • Migration

    • Protobuf break: ReplFeedFeed, InstallDependencies added, oneof tags shifted, and a monty_version skew check — regenerate types and update call sites; older frames are rejected.
    • Replace PoolConfig::new(<binary>) with PoolConfig::subprocess(<binary>); use MontyTransport::Websocket(<url>) for remote workers.
    • Update invocations from monty --subprocess to monty subprocess (now errors if combined with execution flags).

Written for commit 1fa66f2. Summary will update on new commits.

Review in cubic

@github-actions

github-actions Bot commented Jun 24, 2026

Copy link
Copy Markdown

Codecov Results 📊

✅ Patch coverage is 86.45%. Project has 38291 uncovered lines.
✅ Project coverage is 49.83%. Comparing base (base) to head (head).

Files with missing lines (9)
File Patch % Lines
crates/monty-pool/src/worker.rs 84.39% ⚠️ 27 Missing and 16 partials
crates/monty-python/src/pool.rs 97.56% ⚠️ 2 Missing and 12 partials
crates/monty-cli/src/main.rs 71.43% ⚠️ 8 Missing and 1 partials
crates/monty-pool/src/pool.rs 80.00% ⚠️ 4 Missing and 4 partials
crates/monty-proto/src/frame.rs 80.00% ⚠️ 4 Missing and 4 partials
crates/monty-pool/src/checkout.rs 85.71% ⚠️ 5 Missing and 2 partials
crates/monty/src/exception_public.rs 0.00% ⚠️ 3 Missing
crates/monty-cli/src/subprocess.rs 93.75% ⚠️ 1 Missing
crates/monty/src/types/set.rs 0.00% ⚠️ 1 Missing
Coverage diff
@@            Coverage Diff             @@
##          main       #PR       +/-##
==========================================
+ Coverage    49.59%    49.83%    +0.24%
==========================================
  Files          302       304        +2
  Lines        75756     76320      +564
  Branches    161805    162811     +1006
==========================================
+ Hits         37564     38029      +465
- Misses       38192     38291       +99
- Partials      3007      3051       +44

Generated by Codecov Action

@codecov

codecov Bot commented Jun 24, 2026

Copy link
Copy Markdown

@codspeed-hq

codspeed-hq Bot commented Jun 24, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 20 untouched benchmarks
⏩ 15 skipped benchmarks1


Comparing proto-serde (1fa66f2) with main (1ddbfd8)

Open in CodSpeed

Footnotes

  1. 15 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

7 issues found across 46 files

Note: This PR contains a large number of files. cubic only reviews up to 40 files per PR, so some files may not have been reviewed. cubic prioritizes the most important files to review.
On a pro plan you can use ultrareview for larger PRs.

Re-trigger cubic

Comment thread crates/monty-cpython/src/session.rs Outdated
Comment thread crates/monty-cpython/src/session.rs Outdated
Comment thread crates/monty-cpython/src/pyexec.rs Outdated
Comment thread crates/monty-cli/src/main.rs
Comment thread crates/monty-cpython/src/transport.rs
Comment thread crates/monty-pool/src/lib.rs Outdated
Comment thread crates/monty-pool/Cargo.toml Outdated
@coderabbitai

coderabbitai Bot commented Jun 24, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The PR changes the parent/child protocol from ReplCreate/ReplFeed to StartSession/Feed, adds monty_version version checking, introduces a new monty-cpython worker crate with stdio and WebSocket transports, updates monty-pool to use transport-aware workers and PoolConfig::websocket, and exports AsyncMontyWebsocket from monty-python.

🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title is related, but it is too generic to clearly identify the main change in the PR. Use a more specific title like 'Add WebSocket transport for worker pool and CPython child'.
✅ Passed checks (3 passed)
Check name Status Explanation
Description check ✅ Passed The description clearly matches the changeset and summarizes the WebSocket transport and CPython worker work.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch proto-serde

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 9

🧹 Nitpick comments (2)
crates/monty-pool/tests/websocket.rs (1)

20-26: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Assert the StartSession.monty_version in the mock child.

Right now every non-Feed request gets a generic Ok, so this test would still pass if the websocket path stopped sending the version field that remote workers use for skew rejection.

Suggested tightening
-        let kind = match request.kind.expect("request kind") {
+        let kind = match request.kind.expect("request kind") {
+            pb::parent_request::Kind::StartSession(start) => {
+                assert_eq!(start.monty_version, env!("CARGO_PKG_VERSION"));
+                pb::child_event::Kind::Ok(pb::Ok {})
+            }
             pb::parent_request::Kind::Feed(_) => pb::child_event::Kind::Complete(pb::Complete {
                 value: Some(MontyObject::Int(42).into()),
             }),
             // StartSession / Reset / Shutdown / anything else: acknowledge.
             _ => pb::child_event::Kind::Ok(pb::Ok {}),
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/monty-pool/tests/websocket.rs` around lines 20 - 26, The websocket
test’s mock child currently treats every non-Feed request as a generic Ok, so it
never verifies that StartSession carries the monty_version field. Update the
mock handling in websocket.rs around the request.kind match so the StartSession
branch explicitly inspects pb::StartSession and asserts the expected
monty_version value before returning Ok, while leaving Feed behavior unchanged.
Use the request.kind match and pb::parent_request::Kind::StartSession /
pb::StartSession symbols to locate the check.
crates/monty-pool/src/pool.rs (1)

58-64: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Update Pool::new rustdoc for WebSocket mode.

Pool::new no longer eagerly spawns or fails fast for MontyTransport::Websocket, but the public docs above still promise both behaviors. Please scope that docstring to subprocess pools or mention the per-checkout dialing behavior.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/monty-pool/src/pool.rs` around lines 58 - 64, Update the rustdoc on
Pool::new so it no longer claims all transports eagerly spawn workers or fail
fast; the current implementation only pre-warms workers for
MontyTransport::Subprocess and leaves MontyTransport::Websocket to connect per
checkout. Adjust the doc comment above Pool::new to scope the behavior to
subprocess pools or explicitly mention the per-checkout dialing behavior for
websocket mode, using Pool::new and
MontyTransport::Subprocess/MontyTransport::Websocket as the anchors.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/monty-cli/src/main.rs`:
- Around line 85-86: The Cli subcommand parsing in main.rs currently allows
mixing the subprocess subcommand with execution flags, so update the subcommand
field on Cli to use clap conflict rules and make subprocess mutually exclusive
with interactive, command, and file modes. Locate the #[command(subcommand)]
declaration on the subcommand field and add conflicts_with_all there so invalid
combinations fail during argument parsing instead of being silently ignored by
the main handler.

In `@crates/monty-cpython/src/session.rs`:
- Around line 119-130: The StartSession path is still returning Flow::Reply
after handle_start_session() creates a FatalError, so the worker continues
running instead of exiting. Update session::handle() to detect the fatal result
from handle_start_session() and route it through Flow::Exit with fatal_event()
and the appropriate exit code, consistent with the fatal_event() contract in
events.rs. Make the same change in the other StartSession failure path
referenced by the review so any fatal StartSession error causes immediate
process termination rather than a normal reply.

In `@crates/monty-cpython/src/transport.rs`:
- Around line 117-119: Update the WebSocket setup in connect so it no longer
uses tungstenite::connect with default frame/message limits; switch to
tungstenite::client::connect_with_config and explicitly set both max_frame_size
and max_message_size to monty_proto::MAX_FRAME_LEN. Keep the existing
WsTransport construction and error handling via ws_io_error unchanged, but
ensure the new connect path is used in the connect function.

In `@crates/monty-pool/src/lib.rs`:
- Around line 43-54: MontyTransport is defined with tuple variants, so the
pattern matches in pool.rs must use tuple-style syntax instead of struct-style
matching. Update the matches in the pool logic that refer to
MontyTransport::Subprocess and MontyTransport::Websocket to use the correct
tuple variant patterns, keeping the fix localized to the pattern matches in the
relevant pool functions.

In `@crates/monty-pool/src/pool.rs`:
- Line 61: The `matches!` checks against `MontyTransport` in `pool` are using
invalid struct-style patterns for tuple variants and may also move non-`Copy`
fields. Update the pattern matching in the `MontyTransport::Subprocess` and any
similar `MontyTransport::Websocket` checks to use reference-based matching with
tuple-style patterns so the enum variants are matched without compilation errors
or moves.

In `@crates/monty-pool/src/worker.rs`:
- Around line 144-150: The WebSocket worker path in PoolInner::acquire_worker is
calling Worker::connect_ws synchronously without any deadline, so the checkout
can block past checkout_timeout. Update the connect_ws flow to enforce a timeout
around tungstenite::connect, using the configured checkout timeout (or an
equivalent explicit deadline) and propagate a PoolError when the deadline is
exceeded. Keep the existing Worker::connect_ws and PoolInner::acquire_worker
call sites as the main places to wire in the timeout handling.

In `@crates/monty-python/python/pydantic_monty/_monty.pyi`:
- Around line 430-435: Update the docstring for AsyncMontyWebsocket and the
mirrored Rust documentation on PyAsyncMontyWebsocket to describe the WebSocket
peer as the server/relay that the client dials, not as “monty-cpython” or “a
child running a server.” Keep the wording aligned between the Python stub and
Rust docs, and make it clear that AsyncMontyWebsocket connects out to a
configured URL and acts like AsyncMonty but for remote workers.

In `@limitations/pool-architecture.md`:
- Around line 1-8: The intro in the pool-architecture doc is no longer accurate
for pydantic_monty because it now also supports a websocket transport, so it
should not claim the Python package runs exclusively in monty subprocess
workers. Update the wording in the opening description to either scope the
statement to local subprocess mode only or mention both the subprocess and
remote websocket paths. Use the existing subsection text around monty
subprocess, monty-pool, Monty/AsyncMonty to keep the terminology consistent.

---

Nitpick comments:
In `@crates/monty-pool/src/pool.rs`:
- Around line 58-64: Update the rustdoc on Pool::new so it no longer claims all
transports eagerly spawn workers or fail fast; the current implementation only
pre-warms workers for MontyTransport::Subprocess and leaves
MontyTransport::Websocket to connect per checkout. Adjust the doc comment above
Pool::new to scope the behavior to subprocess pools or explicitly mention the
per-checkout dialing behavior for websocket mode, using Pool::new and
MontyTransport::Subprocess/MontyTransport::Websocket as the anchors.

In `@crates/monty-pool/tests/websocket.rs`:
- Around line 20-26: The websocket test’s mock child currently treats every
non-Feed request as a generic Ok, so it never verifies that StartSession carries
the monty_version field. Update the mock handling in websocket.rs around the
request.kind match so the StartSession branch explicitly inspects
pb::StartSession and asserts the expected monty_version value before returning
Ok, while leaving Feed behavior unchanged. Use the request.kind match and
pb::parent_request::Kind::StartSession / pb::StartSession symbols to locate the
check.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: f6f44034-f21f-4f60-99d6-75d3c9084463

📥 Commits

Reviewing files that changed from the base of the PR and between 842232f and 4e40212.

⛔ Files ignored due to path filters (2)
  • Cargo.lock is excluded by !**/*.lock
  • crates/monty-proto/src/generated/monty.v1.rs is excluded by !**/generated/**
📒 Files selected for processing (44)
  • CLAUDE.md
  • Cargo.toml
  • crates/monty-bench/benches/pool.rs
  • crates/monty-cli/Cargo.toml
  • crates/monty-cli/src/main.rs
  • crates/monty-cli/src/subprocess.rs
  • crates/monty-cli/tests/subprocess.rs
  • crates/monty-cpython/Cargo.toml
  • crates/monty-cpython/README.md
  • crates/monty-cpython/build.rs
  • crates/monty-cpython/src/events.rs
  • crates/monty-cpython/src/lib.rs
  • crates/monty-cpython/src/main.rs
  • crates/monty-cpython/src/pyexec.rs
  • crates/monty-cpython/src/session.rs
  • crates/monty-cpython/src/transport.rs
  • crates/monty-cpython/tests/stdio_session.rs
  • crates/monty-js/src/lib.rs
  • crates/monty-js/src/pool.rs
  • crates/monty-js/ts/pool.ts
  • crates/monty-pool/Cargo.toml
  • crates/monty-pool/src/checkout.rs
  • crates/monty-pool/src/lib.rs
  • crates/monty-pool/src/pool.rs
  • crates/monty-pool/src/watchdog.rs
  • crates/monty-pool/src/worker.rs
  • crates/monty-pool/tests/pool_test.rs
  • crates/monty-pool/tests/websocket.rs
  • crates/monty-proto/proto/monty/v1/monty.proto
  • crates/monty-proto/src/frame.rs
  • crates/monty-proto/src/lib.rs
  • crates/monty-proto/src/wire.rs
  • crates/monty-proto/tests/frame.rs
  • crates/monty-proto/tests/oracle/monty.v1.rs
  • crates/monty-proto/tests/roundtrip.rs
  • crates/monty-python/Cargo.toml
  • crates/monty-python/python/pydantic_monty/__init__.py
  • crates/monty-python/python/pydantic_monty/_monty.pyi
  • crates/monty-python/src/dataclass.rs
  • crates/monty-python/src/exceptions.rs
  • crates/monty-python/src/lib.rs
  • crates/monty-python/src/pool.rs
  • crates/monty/src/exception_public.rs
  • limitations/pool-architecture.md

Comment thread crates/monty-cli/src/main.rs
Comment thread crates/monty-cpython/src/session.rs Outdated
Comment thread crates/monty-cpython/src/session.rs Outdated
Comment thread crates/monty-cpython/src/transport.rs
Comment thread crates/monty-pool/src/lib.rs
Comment thread crates/monty-pool/src/pool.rs Outdated
Comment thread crates/monty-pool/src/worker.rs Outdated
Comment thread crates/monty-python/python/pydantic_monty/_monty.pyi
Comment thread limitations/pool-architecture.md Outdated
@samuelcolvin samuelcolvin force-pushed the proto-serde branch 2 times, most recently from e7f9b7e to fa811cd Compare June 27, 2026 15:23

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

4 issues found across 28 files (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread crates/monty-cpython/tests/pep_723.rs
Comment thread crates/monty-cpython/src/session.rs Outdated
Comment thread crates/monty-cpython/src/install.rs Outdated
Comment thread CLAUDE.md Outdated
samuelcolvin added a commit that referenced this pull request Jun 28, 2026
Triage and fix the real review comments on the websocket-protocol PR.

monty-cpython is now one session per process (a checkout dials a fresh
worker): `Reset` acknowledges and exits rather than resetting in-process
state, so an interpreter's `sys.modules`/`sys.path` and any `uv` install
dir die with the process and never leak into another session. The install
dir is a single per-process `tempfile::TempDir` (unique, exclusively
created, auto-removed), replacing the predictable `pid`-counter naming.

Other fixes:
- session: a failed `open_session` now exits after its `FatalError`
  (honouring the fatal_event contract); an oversized turn-ender is replaced
  with a small error and the worker keeps serving instead of crashing; a
  missing `NamedValue.value` is reported as a protocol violation.
- transports: raise tungstenite frame/message limits to `MAX_FRAME_LEN`;
  the pool dial is bounded by `request_timeout` (TCP connect + handshake)
  so a hung dial cannot stall a checkout; oversize WS request frames are
  recoverable (`Malformed`) rather than fatal.
- cli: `monty subprocess` now errors when combined with execution flags
  instead of silently ignoring them.
- pyexec: `sys.stdout.write` returns a character count, not byte length.
- docs: fix the `MontyTransport::Subprocess` rustdoc typo, the CLAUDE.md
  image make-targets, the websocket-peer wording in the stub/Rust docs, and
  the pool-architecture intro (which now covers the websocket transport).
- promote `tempfile` to a workspace dependency.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 15 files (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread limitations/pool-architecture.md
Comment thread crates/monty-cli/src/main.rs Outdated

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 5 files (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread crates/monty-pool/src/worker.rs
Comment thread crates/monty-pool/src/lib.rs Outdated

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 14 files (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread crates/monty-cpython/src/preamble.py Outdated
Comment thread crates/monty-cpython/src/install.rs
samuelcolvin added a commit that referenced this pull request Jun 28, 2026
- monty-cli: fix subprocess conflict check swallowing resource-limit parse
  errors via `.ok().flatten()`; check raw flag presence instead
- monty-cpython preamble: drive a cell's body and trailing expression in a
  single asyncio event loop so async objects keep loop affinity (+ test)
- monty-cpython install: prepend site-packages to sys.path before addsitedir
  so session installs take precedence over the base environment
- monty-pool/lib.rs: document Subprocess vs single-use Websocket transports
- pool-architecture.md: WebSocket transport is Python-only; fix stale heading

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 6 files (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread limitations/pool-architecture.md
Comment thread crates/monty-cpython/src/install.rs
samuelcolvin and others added 13 commits June 28, 2026 15:29
…python child

Remote sandboxing now reuses the existing Rust pool/proto stack instead of a
parallel Python codec package:

- Fold the Python<->MontyObject conversion layer (convert/dataclass/exceptions)
  back into monty-python (now cdylib+rlib); delete the wire-protocol crate and
  its Python wire_protocol package.
- monty-pool: add a MontyTransport { Subprocess, Websocket } enum (replacing
  PoolConfig.binary_path). Worker is now an enum (subprocess pipes vs. blocking
  tungstenite WebSocket); the watchdog kills via a Killable trait (process kill
  vs. shutting down a cloned TcpStream). WebSocket workers are per-checkout.
  Leaves a TODO to make the pool async end-to-end for remote-session scaling.
- New monty-cpython crate: a wire-protocol child that runs fed snippets in
  embedded CPython and routes undefined names back to the parent via a
  dict.__missing__ hook. Transports: --subprocess (drop-in for the pool),
  --connect (relay client), --listen (server).
- New pydantic_monty.AsyncMontyWebsocket(url) async context manager for the
  WebSocket transport; Monty/AsyncMonty stay subprocess-only.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The child only needs to dial out — either to a relay or a parent-as-server —
so the listening transport had no real role. Subcommands are now `subprocess`
and `websocket`; `WsTransport` loses its stream-type generic.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
samuelcolvin and others added 11 commits June 28, 2026 15:30
Triage and fix the real review comments on the websocket-protocol PR.

monty-cpython is now one session per process (a checkout dials a fresh
worker): `Reset` acknowledges and exits rather than resetting in-process
state, so an interpreter's `sys.modules`/`sys.path` and any `uv` install
dir die with the process and never leak into another session. The install
dir is a single per-process `tempfile::TempDir` (unique, exclusively
created, auto-removed), replacing the predictable `pid`-counter naming.

Other fixes:
- session: a failed `open_session` now exits after its `FatalError`
  (honouring the fatal_event contract); an oversized turn-ender is replaced
  with a small error and the worker keeps serving instead of crashing; a
  missing `NamedValue.value` is reported as a protocol violation.
- transports: raise tungstenite frame/message limits to `MAX_FRAME_LEN`;
  the pool dial is bounded by `request_timeout` (TCP connect + handshake)
  so a hung dial cannot stall a checkout; oversize WS request frames are
  recoverable (`Malformed`) rather than fatal.
- cli: `monty subprocess` now errors when combined with execution flags
  instead of silently ignoring them.
- pyexec: `sys.stdout.write` returns a character count, not byte length.
- docs: fix the `MontyTransport::Subprocess` rustdoc typo, the CLAUDE.md
  image make-targets, the websocket-peer wording in the stub/Rust docs, and
  the pool-architecture intro (which now covers the websocket transport).
- promote `tempfile` to a workspace dependency.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Lean on the fact that a monty-cpython worker serves exactly one session per
process, and consolidate worker construction in the pool.

monty-cpython:
- `Reset` and `Shutdown` are now one arm: for a one-shot worker, "end the
  session" and "end the worker" are the same — ack and exit.
- drop the always-`Some` `Option` from `Flow::Exit`'s event.

monty-pool:
- `Checkout::finish` skips the `Reset` round-trip for websocket workers
  (single-use, discarded after every checkout): dropping the worker closes
  the socket, which the child reads as EOF and exits. Subprocess workers
  still reset and return to the idle pool.
- consolidate spawn/connect dispatch behind `Worker::new(config)` (with
  `Worker::{subprocess,websocket}` builders), so the acquire path no longer
  branches on transport.
- add `MontyTransport::is_websocket()` and use it in place of the verbose
  `matches!(…, Websocket { .. })` sites.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replace the per-session `uv pip install --target <tempdir>` model with a
virtualenv at `./.venv` (relative to the worker's working directory):

- install with `uv pip install --python .venv/bin/python`, and make the
  packages importable via `site.addsitedir` (not a bare `sys.path` insert) so
  the venv's `.pth` files run — legacy namespace packages need this.
- the venv is a deployment contract, pre-created by the image's `uv venv`
  pinned to the embedded Python 3.14; a missing venv at install time fails
  with a clear error rather than being created on the fly (which, under a
  wrong CWD or unpinned interpreter, would land in the wrong place or with a
  mismatched ABI).
- Dockerfile: `WORKDIR /app` (fixes the runtime CWD) + `uv venv .venv`, with a
  build-time smoke test that the venv exists.
- the worker is one session per process, so the venv lives for that single
  session and is reclaimed with the per-session sandbox — no cleanup needed
  (drop the `tempfile` dependency).

Docs: new "One session per sandbox" README section; README +
limitations/pool-architecture.md describe the venv model. The ignored install
tests provision their own `./.venv`; `.venv/` is gitignored.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The watchdog held an `Arc<dyn Killable>` plus a separate
`Arc<AtomicBool>` timeout flag, backed by a `Killable` trait and two
newtypes (`SubprocessKill`, `WsShutdown`) — dynamic dispatch and a fresh
heap allocation on every `arm()` for a closed set of two transports.

Collapse both into one `Arc<Interrupt>` shared between the worker and the
watchdog: an `InterruptKind` enum (Subprocess/WebSocket) plus the sticky
`killed_for_timeout` flag, which always travel together. This deletes the
trait, both newtypes, the `KillTarget` struct, and the tuple return.

Hoist the `interrupt` field out of both `WorkerKind` variants up to
`Worker` (it was identical in both), removing the match-accessor.

Make `InterruptKind::WebSocket` hold a plain `TcpStream` instead of an
`Option`: a websocket worker whose socket can't be cloned can never be
interrupted by the watchdog, silently voiding the hard-timeout
guarantee, so the dial now fails rather than building such a worker.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- monty-cli: fix subprocess conflict check swallowing resource-limit parse
  errors via `.ok().flatten()`; check raw flag presence instead
- monty-cpython preamble: drive a cell's body and trailing expression in a
  single asyncio event loop so async objects keep loop affinity (+ test)
- monty-cpython install: prepend site-packages to sys.path before addsitedir
  so session installs take precedence over the base environment
- monty-pool/lib.rs: document Subprocess vs single-use Websocket transports
- pool-architecture.md: WebSocket transport is Python-only; fix stale heading

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Remove the `_CallbackGlobals` Python type: `SandboxGlobals` now
  `extends=PyDict` and is used directly as the feed's execution globals,
  with name resolution folded into `__missing__` (was `HostBridge.get`)
- Rename `HostBridge` -> `SandboxGlobals` (it is the namespace, not a
  standalone bridge object)
- Rename `preamble.py` -> `runner.py` and its `_run` entry point -> `run`,
  wrapped behind a small `Runner` struct
- Seed `__name__ = '__main__'` as a real namespace entry so sandboxed code
  runs as the top-level script (resolves from the dict, no host round trip)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- monty-pool: reject flag-like / empty install_dependencies requirements
  at the pool boundary (a string starting with `-` would be smuggled onto
  the `uv pip install` command line as an option, e.g. --index-url)
- monty-cli: fix misattributed resource_limits/subprocess_conflict docstring
- monty-cpython: README stderr behaviour (both streams are sent to the
  parent), make build-image -> build-cpython-image, StartSession -> Configure
- monty-pool: fix stale Worker::connect_ws test comment
- monty-python: document that AsyncMontyWebsocket's default request_timeout
  is often too low for install_dependencies

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- monty-pool: shut the TCP socket down directly on WebSocket worker teardown
  instead of writing a Close frame first, which could block the caller's
  thread on a peer that stopped draining (write timeout is cleared after the
  handshake). A FIN is a clean EOF to the child, so the Close bought nothing.
- limitations/pool-architecture.md: stop claiming a worker is always "the same
  interpreter" — a WebSocket worker (e.g. monty-cpython) can be real CPython
  with no sandbox, no resource limits, and full host access.
- scripts/websocket_relay.py: standalone WebSocket <-> monty subprocess relay
  (framing translation only) for local dev and tests.
- add AsyncMontyWebsocket pytest coverage driving the real transport against
  the relay; add websockets dev dependency.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3 issues found across 6 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="scripts/websocket_relay.py">

<violation number="1" location="scripts/websocket_relay.py:76">
P1: Bridge cleanup depends on both pump coroutines finishing, so a clean close on one side may not tear down the child promptly and sibling tasks are not explicitly cancelled after errors.</violation>

<violation number="2" location="scripts/websocket_relay.py:97">
P2: The relay URL is malformed for IPv6 and wildcard bind addresses. IPv6 literals from getsockname() (e.g., '::1') must be bracketed in URIs (ws://[::1]:port), and wildcard addresses like '0.0.0.0' or '::' are not usable client destinations. Use urllib.parse or ipaddress logic to bracket IPv6 and consider mapping wildcard binds to a connectable fallback (e.g., 127.0.0.1) before printing.</violation>
</file>

<file name="limitations/pool-architecture.md">

<violation number="1" location="limitations/pool-architecture.md:16">
P2: WebSocket/CPython caveat is not carried through later unqualified worker safety guarantees, making the architecture doc internally inconsistent about sandboxing, resource limits, and subprocess behavior.</violation>
</file>

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread scripts/websocket_relay.py Outdated
Comment thread scripts/websocket_relay.py
Comment thread limitations/pool-architecture.md
samuelcolvin and others added 3 commits June 28, 2026 16:16
Sandbox exceptions now carry their stack back over the wire instead of a
bare `Type: message`. Add `MontyException::add_traceback`, and in the
CPython worker walk the CPython traceback into Monty `StackFrame`s, keeping
only user frames (those compiled under `script_name`) and dropping the
runner.py driver frames. Compile fed code under `Configure.script_name` so
the frames report the parent's filename.

Tier 1 fidelity: filename, line, and function name only (no column ranges,
source previews, or carets).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- relay: stop on first pump completion and cancel/reap both tasks so a
  clean close tears the child down promptly without orphaning a pump
- relay: format the printed ws:// URL via format_ws_url — bracket IPv6
  literals and map wildcard binds to loopback so it's always dialable
- pool-architecture.md: scope the Execution model guarantees to the Monty
  sandbox worker; a WebSocket/monty-cpython remote honours the protocol
  but none of the sandbox guarantees (it shells out to uv for installs)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@samuelcolvin samuelcolvin merged commit e240adb into main Jun 29, 2026
36 checks passed
@samuelcolvin samuelcolvin deleted the proto-serde branch June 29, 2026 00:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant