diff --git a/src/wasm_sandbox/guests/python/sandbox_executor.py b/src/wasm_sandbox/guests/python/sandbox_executor.py index 83b30c9..0f49342 100644 --- a/src/wasm_sandbox/guests/python/sandbox_executor.py +++ b/src/wasm_sandbox/guests/python/sandbox_executor.py @@ -139,7 +139,36 @@ def _http_request(method: str, url: str, body: str = "", content_type: str = "") class Executor: - """Implements the WIT executor interface for componentize-py.""" + """Implements the WIT executor interface for componentize-py. + + The executor keeps a single, persistent module-level namespace + (``self._globals``) that is reused across every call to :py:meth:`run`. + Names defined by guest code (``x = 1``, ``def foo(): ...``, + ``class C: ...``) therefore remain visible to subsequent runs on + the same sandbox instance, matching: + + * the snapshot/restore contract documented on ``WasmSandbox`` — + ``snapshot``/``restore`` is the mechanism for rewinding state, + not bare back-to-back ``run`` calls; + * the JavaScript guest's ``globalThis`` persistence story for + explicit global writes; + * the ``python_basics`` example, which sets ``counter = 100`` + and treats ``restore`` (not the next ``run``) as the action + that makes ``counter`` undefined. + + Host-provided helpers (``call_tool``, ``http_get``, ``http_post``) + are seeded once on construction. Guest code may shadow them + locally, but the originals are restored by ``snapshot``/``restore`` + along with the rest of the namespace. + """ + + def __init__(self) -> None: + self._globals: dict = { + "__builtins__": __builtins__, + "call_tool": _call_tool, + "http_get": http_get, + "http_post": http_post, + } def run(self, code: str) -> ExecutionResult: """Execute Python code and capture output.""" @@ -152,7 +181,7 @@ def run(self, code: str) -> ExecutionResult: exit_code = 0 try: - exec(code, {"__builtins__": __builtins__, "call_tool": _call_tool, "http_get": http_get, "http_post": http_post}) + exec(code, self._globals) except SystemExit as e: exit_code = e.code if isinstance(e.code, int) else 1 except Exception as e: diff --git a/src/wasm_sandbox/tests/python_state_persistence.rs b/src/wasm_sandbox/tests/python_state_persistence.rs new file mode 100644 index 0000000..4002e27 --- /dev/null +++ b/src/wasm_sandbox/tests/python_state_persistence.rs @@ -0,0 +1,114 @@ +//! Integration test: Python guest module globals persist across `run()`. +//! +//! This test pins the documented contract that the Python `Executor` +//! reuses one module-level namespace for every call to `run()` on the +//! same sandbox instance. The previous implementation built a fresh +//! `globals` dict on every call (`exec(code, {...})`), which silently +//! discarded any `def`, `class`, or top-level assignment between runs. +//! That contradicted: +//! +//! * the `WasmSandbox` `snapshot`/`restore` contract — the documented +//! mechanism for rewinding guest state — which only makes sense +//! if state otherwise survives a `run()` boundary; +//! * the `python_basics` example's "state was rolled back" narrative; +//! * the JavaScript guest, which preserves `globalThis` across runs. +//! +//! The tests below would have failed on the prior implementation; they +//! pass once `Executor` stores its globals on the instance and reuses +//! them across `run()` calls. + +use std::path::Path; + +use hyperlight_sandbox::SandboxBuilder; +use hyperlight_wasm_sandbox::Wasm; + +fn python_guest_path() -> String { + Path::new(env!("CARGO_MANIFEST_DIR")) + .join("guests/python/python-sandbox.aot") + .display() + .to_string() +} + +/// A `def` at module top level in `run()` #1 must be callable in `run()` #2. +#[tokio::test] +async fn python_function_definition_persists_across_runs() { + let result = tokio::task::spawn_blocking(|| { + let mut sandbox = SandboxBuilder::new() + .guest(Wasm) + .module_path(python_guest_path()) + .build() + .expect("failed to create sandbox"); + + sandbox + .run("def word_count(text): return len(text.split())") + .expect("first run failed"); + + sandbox + .run("print(word_count('hello world from hyperlight'))") + .expect("second run failed") + }) + .await + .unwrap(); + + assert_eq!(result.exit_code, 0, "stderr: {}", result.stderr); + assert_eq!(result.stdout.trim(), "4"); +} + +/// A bare module-level assignment in `run()` #1 must be readable in `run()` #2. +#[tokio::test] +async fn python_top_level_assignment_persists_across_runs() { + let result = tokio::task::spawn_blocking(|| { + let mut sandbox = SandboxBuilder::new() + .guest(Wasm) + .module_path(python_guest_path()) + .build() + .expect("failed to create sandbox"); + + sandbox.run("counter = 100").expect("first run failed"); + sandbox + .run("print(f'counter = {counter}')") + .expect("second run failed") + }) + .await + .unwrap(); + + assert_eq!(result.exit_code, 0, "stderr: {}", result.stderr); + assert_eq!(result.stdout.trim(), "counter = 100"); +} + +/// `snapshot` + `restore` must continue to rewind the persistent +/// namespace, undoing any names defined since the snapshot. This is +/// the contract documented on `WasmSandbox`; the persistence fix must +/// not regress it. +#[tokio::test] +async fn python_restore_rewinds_module_globals() { + let result = tokio::task::spawn_blocking(|| { + let mut sandbox = SandboxBuilder::new() + .guest(Wasm) + .module_path(python_guest_path()) + .build() + .expect("failed to create sandbox"); + + let snap = sandbox.snapshot().expect("snapshot failed"); + sandbox + .run("rolled_back = 'still here'") + .expect("set failed"); + sandbox.restore(&snap).expect("restore failed"); + + sandbox + .run( + r#" +try: + print(rolled_back) +except NameError: + print("rolled_back is undefined") +"#, + ) + .expect("post-restore run failed") + }) + .await + .unwrap(); + + assert_eq!(result.exit_code, 0, "stderr: {}", result.stderr); + assert_eq!(result.stdout.trim(), "rolled_back is undefined"); +}