From b94a9f8d256b6746a8913dfd7ffbb45b455211f5 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 26 Aug 2026 04:15:01 +0000 Subject: [PATCH] refactor: retire runners/lua_runner now that workflow_engine replaces it Executes issue #73's decision record: prototypes/workflow_engine (via #74/#75, now fully landed) replaces runners/lua_runner as a whole rather than running alongside it as a second, parallel Lua execution path. Nothing in production depended on the SimpleCode+LUA path (it was only a few days old, never adopted for a real manifest), so this is a straight deletion, not a migration. Removed: - runners/lua_runner (crate deleted entirely), and its workspace member entry. - Its dependency and `lua` Cargo feature in binary/apid. - Its construction/registration in apid::construct_execution_engine and the now-dead LUA_LANG constant. - The `Ok(Language::LUA) => ...` arm of execution_engine::Engine::run's SimpleCode dispatch (a comment marks why it's gone, referencing #73). The LUA protobuf enum variant itself stays defined, per the issue's own plan - harmless, and a smaller footprint than changing a wire enum. - binary/apid/tests/lua_e2e.rs and the CI step that ran it (.github/workflows/rust.yml) - the only e2e coverage that existed specifically for the retired path. runners/workflow_runner's own tests used Language::LUA purely as a convenient "any SimpleCode language" stand-in for its api.run bridge tests (registering a FakeCodeRunner, never the real lua_runner) - switched to Language::JAVASCRIPT so those tests keep exercising Engine::run's real dispatch after the LUA arm is gone. Verified this still genuinely tests dispatch by temporarily registering under a mismatched key and confirming the test fails. Verified: cargo build/test --workspace clean, cargo build -p apid with every feature combination (including workflow) clean, cargo clippy --workspace --all-targets clean, cargo fmt --all -- --check clean. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_019EKR96FrBygNE1sfscQ6CG --- .github/workflows/rust.yml | 3 - Cargo.lock | 16 -- Cargo.toml | 1 - binary/apid/Cargo.toml | 2 - binary/apid/src/constants.rs | 4 - binary/apid/src/main.rs | 6 - binary/apid/tests/lua_e2e.rs | 176 ------------ runners/lua_runner/Cargo.toml | 21 -- runners/lua_runner/src/constants.rs | 4 - runners/lua_runner/src/error.rs | 44 --- runners/lua_runner/src/lib.rs | 389 --------------------------- runners/workflow_runner/src/lib.rs | 21 +- usecases/execution_engine/src/lib.rs | 26 +- 13 files changed, 19 insertions(+), 694 deletions(-) delete mode 100644 binary/apid/tests/lua_e2e.rs delete mode 100644 runners/lua_runner/Cargo.toml delete mode 100644 runners/lua_runner/src/constants.rs delete mode 100644 runners/lua_runner/src/error.rs delete mode 100644 runners/lua_runner/src/lib.rs diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 67732c1..5ee9f6a 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -96,9 +96,6 @@ jobs: echo "apid started and bound its gRPC port successfully" - - name: Lua runner e2e smoke test - run: cargo test -p apid --features lua --test lua_e2e - build-apid-image: name: build-apid-image runs-on: ubuntu-latest diff --git a/Cargo.lock b/Cargo.lock index 8d43dae..1b6fb23 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -165,7 +165,6 @@ dependencies = [ "in_memory_storage", "javascript_runner", "local_file_loader", - "lua_runner", "notify", "prost", "protobuf", @@ -1414,21 +1413,6 @@ dependencies = [ "cc", ] -[[package]] -name = "lua_runner" -version = "0.1.0" -dependencies = [ - "chrono", - "common_data_structures", - "core_entities", - "credential_entities", - "execution_engine", - "mlua", - "serde_json", - "tempfile", - "thiserror", -] - [[package]] name = "luajit-src" version = "210.5.12+a4f56a4" diff --git a/Cargo.toml b/Cargo.toml index 4b6e04d..0479472 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -22,7 +22,6 @@ members = [ "runners/api_caller", "runners/python_runner", "runners/javascript_runner", - "runners/lua_runner", "runners/user_input", "runners/filtered_runner", "runners/workflow_runner", diff --git a/binary/apid/Cargo.toml b/binary/apid/Cargo.toml index c669a49..82cef20 100644 --- a/binary/apid/Cargo.toml +++ b/binary/apid/Cargo.toml @@ -34,7 +34,6 @@ local_file_loader = { path = "../../storage/local_file_loader" } api_caller = { path = "../../runners/api_caller" } python_runner = { path = "../../runners/python_runner" } javascript_runner = { path = "../../runners/javascript_runner" } -lua_runner = { path = "../../runners/lua_runner" } user_input = { path = "../../runners/user_input" } filtered_runner = { path = "../../runners/filtered_runner" } workflow_runner = { path = "../../runners/workflow_runner" } @@ -56,7 +55,6 @@ dhat-ad-hoc = [] default = ["python", "input", "javascript", "wrapper"] python = [] javascript = [] -lua = [] workflow = [] input = [] wrapper = [] diff --git a/binary/apid/src/constants.rs b/binary/apid/src/constants.rs index dac1e67..6fd73de 100644 --- a/binary/apid/src/constants.rs +++ b/binary/apid/src/constants.rs @@ -10,7 +10,3 @@ pub const PYTHON_LANG: &str = "python"; /// The [`execution_engine::Engine`] language key registered for the /// JavaScript code runner. pub const JAVASCRIPT_LANG: &str = "js"; - -/// The [`execution_engine::Engine`] language key registered for the Lua -/// code runner. -pub const LUA_LANG: &str = "lua"; diff --git a/binary/apid/src/main.rs b/binary/apid/src/main.rs index 8faa4e3..813ffdf 100644 --- a/binary/apid/src/main.rs +++ b/binary/apid/src/main.rs @@ -532,9 +532,6 @@ fn construct_execution_engine( let js_runner = javascript_runner::JsActionRunner::new(Arc::clone(&engine), workflow_logger.clone()); - #[cfg(feature = "lua")] - let lua_runner = lua_runner::LuaActionRunner::new(Arc::clone(&engine), workflow_logger.clone()); - #[cfg(feature = "workflow")] let workflow_adapter = workflow_runner::WorkflowAdapter::spawn(Arc::clone(&engine), workflow_logger.clone()); @@ -561,9 +558,6 @@ fn construct_execution_engine( #[cfg(feature = "javascript")] engine.register_language(constants::JAVASCRIPT_LANG, Box::new(js_runner)); - #[cfg(feature = "lua")] - engine.register_language(constants::LUA_LANG, Box::new(lua_runner)); - #[cfg(feature = "workflow")] engine.register_workflow_runner(Arc::new(workflow_adapter)); diff --git a/binary/apid/tests/lua_e2e.rs b/binary/apid/tests/lua_e2e.rs deleted file mode 100644 index e0a7b4a..0000000 --- a/binary/apid/tests/lua_e2e.rs +++ /dev/null @@ -1,176 +0,0 @@ -//! End-to-end smoke test: does a registered Lua `SimpleCode` operation -//! actually run correctly through a real `apid` daemon? -//! -//! `lua_runner`'s own unit tests already exercise `LuaActionRunner` -//! directly (including its `api.run` binding against a real `Engine`), but -//! nothing before this proved the daemon actually wires a Lua script up -//! end to end: background-loads a `manifest.json` from disk, dispatches a -//! `RunService` gRPC call through `execution_engine::Engine::run`'s `LUA` -//! arm, and returns the real result over `GetRunResult`. Complements the -//! startup-only smoke test in `.github/workflows/rust.yml`, which proves -//! the binary starts but never exercises a registered operation. -#![cfg(feature = "lua")] - -use std::{ - process::{Child, Command, Stdio}, - time::Duration, -}; - -use engine_entities::engine::{ - engine_client::EngineClient, get_run_result_response, GetRunResultRequest, ListRequest, - RunServiceRequest, -}; -use tempfile::TempDir; -use tokio::{net::TcpStream, time::Instant}; - -const PORT: u16 = 50097; - -/// Kills the spawned `apid` process on drop, so a failed assertion -/// (which unwinds through this guard) doesn't leave an orphaned daemon -/// running. -struct ApidProcess(Child); - -impl Drop for ApidProcess { - fn drop(&mut self) { - let _ = self.0.kill(); - let _ = self.0.wait(); - } -} - -/// Writes a `lua_smoke` connector (a `SimpleCode` Lua manifest) and a -/// matching `config.toml` under `root`. -fn write_fixture(root: &std::path::Path) { - let connector_dir = root.join("connectors").join("lua_smoke"); - std::fs::create_dir_all(&connector_dir).expect("create connector dir"); - - let manifest = r#"{ - "v2": { - "simpleCode": { - "code": { - "codeString": "local input = ...\nreturn { greeting = 'hello ' .. input.name, doubled = input.value * 2 }", - "language": "LUA" - } - } - } -}"#; - std::fs::write(connector_dir.join("manifest.json"), manifest).expect("write manifest"); - - let config = format!( - "[connector]\npath = \"{}\"\n\n[log]\napi_path = \"{}\"\nworkflow_path = \"{}\"\n\n[server]\nport = {PORT}\nhost = \"127.0.0.1\"\n", - root.join("connectors").display(), - root.join("api.log").display(), - root.join("workflow.log").display(), - ); - std::fs::write(root.join("config.toml"), config).expect("write config"); -} - -async fn wait_for_port(port: u16, timeout: Duration) -> bool { - let deadline = Instant::now() + timeout; - while Instant::now() < deadline { - if TcpStream::connect(("127.0.0.1", port)).await.is_ok() { - return true; - } - tokio::time::sleep(Duration::from_millis(100)).await; - } - false -} - -#[tokio::test] -async fn lua_simple_code_runs_end_to_end_through_a_real_apid_daemon() { - let dir = TempDir::new().expect("create tempdir"); - write_fixture(dir.path()); - - let apid_log = std::fs::File::create(dir.path().join("apid.log")).expect("create log file"); - let child = Command::new(env!("CARGO_BIN_EXE_apid")) - .env("APID_CONFIG_PATH", dir.path().join("config.toml")) - .env("HOME", dir.path()) - .stdout(Stdio::from(apid_log.try_clone().expect("clone log handle"))) - .stderr(Stdio::from(apid_log)) - .spawn() - .expect("spawn apid"); - let _apid = ApidProcess(child); - - let log_contents = || std::fs::read_to_string(dir.path().join("apid.log")).unwrap_or_default(); - - assert!( - wait_for_port(PORT, Duration::from_secs(15)).await, - "apid never bound its gRPC port:\n{}", - log_contents() - ); - - let mut client = EngineClient::connect(format!("http://127.0.0.1:{PORT}")) - .await - .unwrap_or_else(|err| panic!("failed to connect to apid: {err}\n{}", log_contents())); - - // The background watcher/loader loads connectors asynchronously, so - // the fixture connector may not be visible immediately after the - // gRPC port opens - poll List() until it shows up. - let deadline = Instant::now() + Duration::from_secs(15); - loop { - let list = client - .list(ListRequest {}) - .await - .expect("list rpc failed") - .into_inner(); - - if list - .items - .iter() - .any(|item| item.name == "(code) lua_smoke.execute") - { - break; - } - - assert!( - Instant::now() < deadline, - "lua_smoke connector was never loaded by the background watcher:\n{}", - log_contents() - ); - tokio::time::sleep(Duration::from_millis(200)).await; - } - - let run = client - .run_service(RunServiceRequest { - id: "lua_smoke.execute".to_owned(), - input: serde_json::json!({ "name": "world", "value": 21 }).to_string(), - limit: None, - execution_id: None, - }) - .await - .expect("run_service rpc failed") - .into_inner(); - assert!(!run.execution_id.is_empty()); - - let deadline = Instant::now() + Duration::from_secs(15); - let output = loop { - let result = client - .get_run_result(GetRunResultRequest { - execution_id: run.execution_id.clone(), - }) - .await - .expect("get_run_result rpc failed") - .into_inner(); - - if result.status() == get_run_result_response::Status::Completed { - break result.output.expect("completed run has no output"); - } - - assert!( - Instant::now() < deadline, - "run never completed (status {:?}):\n{}", - result.status(), - log_contents() - ); - tokio::time::sleep(Duration::from_millis(100)).await; - }; - - let parsed: serde_json::Value = serde_json::from_str(&output) - .unwrap_or_else(|err| panic!("output wasn't JSON: {err}\n{output}")); - - assert_eq!( - parsed, - serde_json::json!([{ "greeting": "hello world", "doubled": 42 }]), - "unexpected output from the real daemon:\n{}", - log_contents() - ); -} diff --git a/runners/lua_runner/Cargo.toml b/runners/lua_runner/Cargo.toml deleted file mode 100644 index c6314dc..0000000 --- a/runners/lua_runner/Cargo.toml +++ /dev/null @@ -1,21 +0,0 @@ -[package] -name = "lua_runner" -version = "0.1.0" -edition = "2021" - -# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html - -[dependencies] -serde_json = { workspace = true } -chrono = { workspace = true } -mlua = { workspace = true } - -execution_engine = { path = "../../usecases/execution_engine" } -common_data_structures = { path = "../../common/data_structures" } - -thiserror = { workspace = true } - -[dev-dependencies] -tempfile = "3" -core_entities = { path = "../../entities/core" } -credential_entities = { path = "../../entities/credentials" } diff --git a/runners/lua_runner/src/constants.rs b/runners/lua_runner/src/constants.rs deleted file mode 100644 index 60b5b47..0000000 --- a/runners/lua_runner/src/constants.rs +++ /dev/null @@ -1,4 +0,0 @@ -//! Shared constants for the Lua runner. - -/// `chrono` format string used to timestamp `api.run` log entries. -pub const DATETIME_FORMAT: &str = "%a %b %e %Y %I:%M:%S %p"; diff --git a/runners/lua_runner/src/error.rs b/runners/lua_runner/src/error.rs deleted file mode 100644 index 50064a2..0000000 --- a/runners/lua_runner/src/error.rs +++ /dev/null @@ -1,44 +0,0 @@ -//! Errors produced while running a -//! [`LuaActionRunner`](crate::LuaActionRunner) operation. - -use execution_engine::error::ExecutionEngine; -use thiserror::Error; - -/// Failure modes of [`LuaActionRunner::run`](crate::LuaActionRunner). -#[derive(Error, Debug)] -#[non_exhaustive] -pub enum LuaActionRunner { - /// The embedded Lua interpreter raised an error while loading or - /// running the script. - #[error("Lua error: {0}")] - LuaError(String), - - /// The script's return value couldn't be converted to JSON. - #[error("Unable to convert Lua return value to JSON: {0}")] - ConversionError(String), - - /// The shared engine's lock was poisoned by a panic in another - /// thread while holding it. - #[error("Get out! The lock has been poisoned: {0}")] - PoisonedLock(String), -} - -impl From for LuaActionRunner { - #[inline] - fn from(value: mlua::Error) -> Self { - Self::LuaError(value.to_string()) - } -} - -impl From for ExecutionEngine { - #[inline] - fn from(value: LuaActionRunner) -> Self { - Self::Other { - source: value.into(), - } - } -} - -/// Shorthand for a [`Result`](core::result::Result) using -/// [`LuaActionRunner`] as its error type. -pub type Result = core::result::Result; diff --git a/runners/lua_runner/src/lib.rs b/runners/lua_runner/src/lib.rs deleted file mode 100644 index 8c99820..0000000 --- a/runners/lua_runner/src/lib.rs +++ /dev/null @@ -1,389 +0,0 @@ -#![warn(clippy::restriction, clippy::pedantic)] -#![allow( - clippy::blanket_clippy_restriction_lints, - clippy::mod_module_files, - clippy::self_named_module_files, - clippy::implicit_return, - clippy::shadow_reuse, - clippy::shadow_unrelated, - clippy::match_ref_pats, - clippy::separated_literal_suffix, - clippy::question_mark_used, - clippy::single_call_fn, - clippy::absolute_paths -)] - -//! A [`CodeRunner`] adapter that executes a Lua operation body in a fresh, -//! sandboxed, time-limited embedded [`mlua::Lua`] interpreter per call. -//! -//! **Sandboxing.** Only the `table`/`string`/`math` standard libraries are -//! loaded — no `io`, `os`, `package`, or `debug`. The Lua base library -//! (`error`, `pairs`, `pcall`, etc.) is always loaded regardless of that -//! selection and includes `dofile`/`loadfile`, which read arbitrary files -//! from disk despite `io` being disabled; both are explicitly removed -//! from the sandbox's globals after construction to close that gap. -//! -//! **Time budget.** Each run gets a fixed [`EXECUTION_TIMEOUT`], enforced -//! via an `mlua` instruction hook that checks elapsed wall-clock time -//! roughly every [`HOOK_INSTRUCTION_INTERVAL`] VM instructions; a script -//! that runs past the budget is aborted with an error. -//! -//! **Bindings.** Exposes one: `api.run(id, params, options)`, mirroring -//! `javascript_runner`'s surface — lets a script synchronously invoke -//! another registered operation. Unlike `python_runner`, there's no -//! `task`/`workflow`/`action` surface (no deferred/human-in-the-loop -//! scheduling, no structured outcome reporting distinct from the -//! script's return value) — deliberately out of scope for now, see #59. -//! -//! Constructing a fresh `Lua` interpreter per call is cheap enough -//! (measured ~60-150µs, no one-time boot cost) that — unlike -//! `python_runner`'s process-wide interpreter or `javascript_runner`'s -//! thread-local `MiniV8` — no reuse strategy is needed here. - -extern crate alloc; -use alloc::sync::Arc; - -use std::{ - sync::RwLock, - time::{Duration, Instant}, -}; - -mod constants; -pub mod error; - -use common_data_structures::log_writer::LogWriter; -use execution_engine::services::{CodeRunner, EngineInputContext}; -use mlua::{HookTriggers, Lua, LuaOptions, LuaSerdeExt, StdLib}; - -/// How long a Lua script is allowed to run before being aborted. -const EXECUTION_TIMEOUT: Duration = Duration::from_secs(5); - -/// How often (in VM instructions) the execution-time budget is checked. -const HOOK_INSTRUCTION_INTERVAL: u32 = 1000; - -/// A [`CodeRunner`] that executes a Lua operation body in a sandboxed, -/// time-limited [`Lua`] interpreter, with an `api.run` binding for -/// invoking other registered operations (see the crate-level docs for -/// what's deliberately not supported yet). -pub struct LuaActionRunner { - /// The engine used to resolve `api.run` calls made from Lua. - engine: Arc>, - - /// Where the `api.run` binding logs each nested call it makes. - logger: LogWriter, -} - -impl LuaActionRunner { - /// Creates a [`LuaActionRunner`] that dispatches `api.run` calls - /// through `engine` and logs them to `logger`. - #[must_use] - #[inline] - pub fn new(engine: Arc>, logger: LogWriter) -> Self { - Self { engine, logger } - } - - /// Builds a sandboxed [`Lua`] instance with [`EXECUTION_TIMEOUT`] - /// enforced (see the crate-level docs for exactly what's sandboxed). - /// - /// # Errors - fn sandboxed_lua() -> error::Result { - Self::sandboxed_lua_with_timeout(EXECUTION_TIMEOUT) - } - - /// Same as [`Self::sandboxed_lua`], but with `timeout` instead of the - /// real [`EXECUTION_TIMEOUT`] — split out so tests can prove the - /// abort mechanism itself works without waiting out the real budget. - /// - /// # Errors - fn sandboxed_lua_with_timeout(timeout: Duration) -> error::Result { - let lua = Lua::new_with( - StdLib::TABLE | StdLib::STRING | StdLib::MATH, - LuaOptions::default(), - )?; - - lua.globals().set("dofile", mlua::Value::Nil)?; - lua.globals().set("loadfile", mlua::Value::Nil)?; - - let start = Instant::now(); - lua.set_hook( - HookTriggers::default().every_nth_instruction(HOOK_INSTRUCTION_INTERVAL), - move |_lua, _debug| { - if start.elapsed() > timeout { - return Err(mlua::Error::RuntimeError( - "script exceeded its execution time budget".into(), - )); - } - - Ok(()) - }, - ); - - Ok(lua) - } - - /// Installs the `api.run(id, params, options)` binding into `lua`, - /// dispatching through `self.engine` (as a nested call from `name`'s - /// running script within `execution_id`) and logging each call to - /// `self.logger`. - /// - /// # Errors - fn install_api_binding(&self, lua: &Lua, name: &str, execution_id: &str) -> error::Result<()> { - let engine = Arc::clone(&self.engine); - let logger = self.logger.clone(); - let name = name.to_owned(); - let execution_id = execution_id.to_owned(); - - let run_fn = lua.create_function( - move |lua, (id, params, options): (String, mlua::Value, Option)| { - let now = chrono::offset::Local::now(); - let now = now.format(constants::DATETIME_FORMAT).to_string(); - - logger - .write_all(format!("{now} ({name}) [API] {id}\n").as_bytes()) - .map_err(|err| mlua::Error::ExternalError(Arc::new(err)))?; - - let params: serde_json::Value = lua.from_value(params)?; - let options: serde_json::Value = options - .map(|value| lua.from_value(value)) - .transpose()? - .unwrap_or(serde_json::Value::Null); - - let context = - EngineInputContext::new(Some(name.clone()), execution_id.clone(), false); - - let engine = engine.read().map_err(|err| { - mlua::Error::ExternalError(Arc::new(error::LuaActionRunner::PoisonedLock( - err.to_string(), - ))) - })?; - let result = engine - .run(&id, params, options, &context) - .map_err(|err| mlua::Error::ExternalError(Arc::new(err)))?; - - lua.to_value(&result) - }, - )?; - - let api = lua.create_table()?; - api.set("run", run_fn)?; - lua.globals().set("api", api)?; - - Ok(()) - } - - /// Wraps `source_code` via `local input = ...`, evaluates it in a - /// fresh sandboxed interpreter (with `params` bound as `input` and - /// the `api.run` binding installed), and converts the return value - /// back to JSON. - fn run_internal( - &self, - name: &str, - _operation_name: &str, - source_code: &str, - params: &serde_json::Value, - ctx: &EngineInputContext, - ) -> error::Result { - let lua = Self::sandboxed_lua()?; - self.install_api_binding(&lua, name, &ctx.execution_id)?; - - let input = lua.to_value(params)?; - let wrapped = format!("local input = ...\n{source_code}"); - let func: mlua::Function = lua.load(&wrapped).into_function()?; - let result: mlua::Value = func.call(input)?; - - lua.from_value(result) - .map_err(|err| error::LuaActionRunner::ConversionError(err.to_string())) - } -} - -impl CodeRunner for LuaActionRunner { - #[inline] - fn run( - &self, - name: &str, - operation_name: &str, - source_code: &str, - params: serde_json::Value, - ctx: &EngineInputContext, - ) -> execution_engine::error::Result { - let result = self.run_internal(name, operation_name, source_code, ¶ms, ctx)?; - Ok(result) - } -} - -#[cfg(test)] -mod tests { - use std::sync::Mutex; - - use execution_engine::services::EngineLookup; - use serde_json::json; - - use super::{error::LuaActionRunner, Arc, Duration, EngineInputContext, LogWriter, RwLock}; - - struct FakeLookup; - - impl EngineLookup for FakeLookup { - fn get_service(&self, _id: &str) -> Option { - None - } - - fn get_credentials( - &self, - _id: &str, - ) -> Option { - None - } - } - - fn test_runner() -> super::LuaActionRunner { - let (logger, _handle) = LogWriter::spawn(tempfile::tempfile().unwrap()); - let lookup: Arc> = Arc::new(Mutex::new(FakeLookup)); - let engine = Arc::new(RwLock::new(execution_engine::Engine::new( - lookup, - logger.clone(), - ))); - - super::LuaActionRunner::new(engine, logger) - } - - fn test_ctx() -> EngineInputContext { - EngineInputContext::new(None, "test-execution".to_owned(), false) - } - - #[test] - fn runs_a_simple_lua_script_and_round_trips_json() { - let runner = test_runner(); - - let result = runner - .run_internal( - "svc", - "op", - "return { greeting = 'hello ' .. input.name }", - &json!({ "name": "world" }), - &test_ctx(), - ) - .unwrap(); - - assert_eq!(result, json!({ "greeting": "hello world" })); - } - - #[test] - fn surfaces_a_lua_syntax_error_instead_of_panicking() { - let runner = test_runner(); - - let result = runner.run_internal( - "svc", - "op", - "this is not valid lua (((", - &json!({}), - &test_ctx(), - ); - - assert!( - matches!(result, Err(LuaActionRunner::LuaError(_))), - "expected a LuaError, got {result:?}" - ); - } - - #[test] - fn surfaces_a_lua_runtime_error_instead_of_panicking() { - let runner = test_runner(); - - let result = runner.run_internal("svc", "op", "error('boom')", &json!({}), &test_ctx()); - - assert!( - matches!(result, Err(LuaActionRunner::LuaError(_))), - "expected a LuaError, got {result:?}" - ); - } - - #[test] - fn api_run_binding_reaches_the_engine_and_surfaces_its_error() { - let runner = test_runner(); - - // FakeLookup never finds a service, so this proves api.run really - // dispatches through the shared Engine (not a stub): the call - // fails with a real NotFound error from Engine::run, caught here - // by pcall rather than propagating out as a Rust-level error. - let result = runner - .run_internal( - "svc", - "op", - "local ok = pcall(function() return api.run('other.op', {}) end)\n\ - return { called = true, ok = ok }", - &json!({}), - &test_ctx(), - ) - .unwrap(); - - assert_eq!(result, json!({ "called": true, "ok": false })); - } - - #[test] - fn sandbox_has_no_os_or_io_access() { - let runner = test_runner(); - - let result = runner.run_internal("svc", "op", "return os.time()", &json!({}), &test_ctx()); - assert!( - matches!(result, Err(LuaActionRunner::LuaError(_))), - "expected calling os.time() to fail (os should not be loaded), got {result:?}" - ); - - let result = runner.run_internal( - "svc", - "op", - "return io.open('/etc/hostname')", - &json!({}), - &test_ctx(), - ); - assert!( - matches!(result, Err(LuaActionRunner::LuaError(_))), - "expected calling io.open(...) to fail (io should not be loaded), got {result:?}" - ); - } - - #[test] - fn sandbox_cannot_read_files_via_dofile_or_loadfile() { - let runner = test_runner(); - - let result = runner.run_internal( - "svc", - "op", - "return dofile('/etc/hostname')", - &json!({}), - &test_ctx(), - ); - assert!( - matches!(result, Err(LuaActionRunner::LuaError(_))), - "expected dofile to be removed from the sandbox, got {result:?}" - ); - - let result = runner.run_internal( - "svc", - "op", - "return loadfile('/etc/hostname')", - &json!({}), - &test_ctx(), - ); - assert!( - matches!(result, Err(LuaActionRunner::LuaError(_))), - "expected loadfile to be removed from the sandbox, got {result:?}" - ); - } - - #[test] - fn a_runaway_script_is_aborted_after_its_time_budget() { - let start = std::time::Instant::now(); - - let lua = - super::LuaActionRunner::sandboxed_lua_with_timeout(Duration::from_millis(50)).unwrap(); - let result: mlua::Result<()> = lua.load("while true do end").exec(); - - assert!(result.is_err(), "expected the runaway script to error out"); - assert!( - start.elapsed() < Duration::from_secs(5), - "expected the script to be aborted well before its actual runtime would end, took {:?}", - start.elapsed() - ); - } -} diff --git a/runners/workflow_runner/src/lib.rs b/runners/workflow_runner/src/lib.rs index e47ae08..b340e41 100644 --- a/runners/workflow_runner/src/lib.rs +++ b/runners/workflow_runner/src/lib.rs @@ -464,14 +464,17 @@ mod tests { } } - /// A `VersionedServiceTree` wrapping a single `SimpleCode` (Lua) - /// manifest - what `api.run` inside a workflow script needs to find - /// via `Engine::run` for a nested call to actually dispatch anywhere. - fn lua_simple_code_service() -> core_entities::service::VersionedServiceTree { + /// A `VersionedServiceTree` wrapping a single `SimpleCode` (JavaScript - + /// any dispatched-to language works equally well here, since the test + /// registers a `FakeCodeRunner` rather than a real runtime) manifest - + /// what `api.run` inside a workflow script needs to find via + /// `Engine::run` for a nested call to actually dispatch anywhere. + fn simple_code_service() -> core_entities::service::VersionedServiceTree { let mut code = core_entities::service::CodeResource::new(); code.set_codeString("ignored - FakeCodeRunner doesn't execute it".to_owned()); - code.language = - protobuf::EnumOrUnknown::new(core_entities::service::code_resource::Language::LUA); + code.language = protobuf::EnumOrUnknown::new( + core_entities::service::code_resource::Language::JAVASCRIPT, + ); let mut simple_code = core_entities::service::SimpleCodeService::new(); simple_code.code = protobuf::MessageField::some(code); @@ -565,7 +568,7 @@ mod tests { let (logger, _handle) = common_data_structures::log_writer::LogWriter::spawn(tempfile::tempfile().unwrap()); let lookup: Arc> = - Arc::new(Mutex::new(SingleServiceLookup(lua_simple_code_service()))); + Arc::new(Mutex::new(SingleServiceLookup(simple_code_service()))); let engine = Arc::new(RwLock::new(execution_engine::Engine::new( lookup, logger.clone(), @@ -606,7 +609,7 @@ mod tests { let (logger, _handle) = common_data_structures::log_writer::LogWriter::spawn(tempfile::tempfile().unwrap()); let lookup: Arc> = - Arc::new(Mutex::new(SingleServiceLookup(lua_simple_code_service()))); + Arc::new(Mutex::new(SingleServiceLookup(simple_code_service()))); let engine = Arc::new(RwLock::new(execution_engine::Engine::new( lookup, logger.clone(), @@ -616,7 +619,7 @@ mod tests { { let mut engine = engine.write().unwrap(); engine.register_language( - "lua", + "js", Box::new(FakeCodeRunner { calls: Arc::clone(&calls), }), diff --git a/usecases/execution_engine/src/lib.rs b/usecases/execution_engine/src/lib.rs index 4c0a3ac..bb53a1d 100644 --- a/usecases/execution_engine/src/lib.rs +++ b/usecases/execution_engine/src/lib.rs @@ -358,25 +358,13 @@ impl Engine { )) } } - Ok(Language::LUA) => { - if let Some(code_runner) = self.code_runners.get("lua") { - self.log(identifier, "SIMPLE_CODE", "STARTED")?; - let result = code_runner.run( - service_name, - operation_name, - simple_code.code.codeString(), - params, - context, - )?; - self.log(identifier, "SIMPLE_CODE", "COMPLETED")?; - - Ok(result) - } else { - Err(error::ExecutionEngine::NotFound( - "Code runner not found for lua".into(), - )) - } - } + // LUA is deliberately not dispatched to here - see #73: + // `Workflow`-kind manifests (via `WorkflowRunner`) are + // the replacement for Lua `SimpleCode` operations, not + // a second parallel Lua execution path through this + // arm. The `LUA` enum variant itself stays defined + // (harmless, and a smaller footprint than removing a + // wire enum value), it's just unreachable here now. _ => Err(error::ExecutionEngine::NotFound("Unknown language".into())), } }