test(v1.1.3-modules): resolver, cache, validator, kind-rejection coverage
Adds ~46 new tests across the v1.1.3 surface:
executor-core/tests/modules.rs (NEW, 23 tests):
- resolver_loads_simple_module / endpoint_can_import_module /
module_can_import_module — end-to-end through Engine::execute.
- resolver_cross_app_blocked / resolver_cross_app_module_not_found /
module_cache_keyed_by_app — same-name modules in different apps
resolve independently; cross-app lookup returns ModuleNotFound.
- resolver_self_import_detected / resolver_circular_detected —
cycle detector reports the chain.
- resolver_depth_limit_enforced / resolver_depth_limit_just_under_succeeds.
- resolver_module_not_found / resolver_backend_error_surfaces.
- resolver_runtime_validation_rejects_top_level_expr — defense-in-
depth: a module with a top-level expression that bypassed the
admin gate is rejected at resolve time.
- module_cache_hit_reuses_compiled_module /
module_cache_stale_invalidated_on_updated_at_change /
module_cache_lru_evicts_when_capacity_exceeded.
- validate_module_{accepts_fn_const_import_only,
rejects_top_level_let, rejects_top_level_expr,
rejects_top_level_while}.
- validate_endpoint_{extracts_literal_imports,
top_level_expr_still_allowed,
skips_dynamic_imports_in_imports_list}.
orchestrator-core/src/client.rs cache_tests (6 tests):
- cache_hit_when_identity_matches / cache_invalidated_when_updated_at_changes
/ distinct_script_ids_cache_independently / lru_eviction_caps_cache_size
/ script_identity_is_copy / compile_error_does_not_poison_cache.
shared/src/script.rs kind_tests (3 tests):
- default_is_endpoint / round_trips_through_serde_lowercase
/ parse_str_round_trip.
manager-core/src/triggers_api.rs v1.1.3 tests (6 tests):
- kv_trigger_rejects_module_target / docs_trigger_rejects_module_target
/ dl_trigger_rejects_module_target — modules cannot be trigger
targets.
- kv_trigger_rejects_missing_script / kv_trigger_rejects_cross_app_script
— closes the latent v1.1.1/v1.1.2 isolation gap.
- kv_trigger_accepts_endpoint_target — happy path through the
validate_trigger_target check.
picloud/tests/api.rs (8 #[ignore]'d Postgres-gated integration tests):
- create_script_default_kind_is_endpoint / create_module_kind_persists.
- create_module_with_top_level_expr_rejected /
create_module_with_reserved_name_rejected.
- route_bind_rejects_module.
- endpoint_imports_module_end_to_end /
module_edit_visible_on_next_invocation / cross_app_import_blocked.
Lint cleanup along the way:
- `ScriptKind::from_str` renamed to `parse_str` to dodge the
`should_implement_trait` lint (FromStr's `Result<…,Err>` shape
doesn't fit a 0-info lookup).
- `derive(Default)` on `ScriptKind` (Endpoint marked `#[default]`).
- Match-arm collapse in `check_module_shape` for Import + Noop.
- `#[allow(clippy::too_many_lines)]` on `resolve()` (the bridge
logic is genuinely cohesive and would lose clarity if split).
- Elided `'r` lifetime on `StackGuard`.
Three gates clean on this commit's HEAD:
- cargo fmt --all -- --check: clean
- cargo clippy --all-targets --all-features -- -D warnings: clean
- cargo test --workspace: 358 passed, 140 ignored (Postgres-gated)
- npm run check: 0 errors, 0 warnings
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -134,11 +134,7 @@ impl Engine {
|
||||
/// v1.1.3: execute a pre-compiled AST. The orchestrator's script
|
||||
/// cache hands compiled ASTs in directly; this path skips the
|
||||
/// per-call compile.
|
||||
pub fn execute_ast(
|
||||
&self,
|
||||
ast: &Arc<AST>,
|
||||
req: ExecRequest,
|
||||
) -> Result<ExecResponse, ExecError> {
|
||||
pub fn execute_ast(&self, ast: &Arc<AST>, req: ExecRequest) -> Result<ExecResponse, ExecError> {
|
||||
let effective_limits = self.limits.with_overrides(&req.sandbox_overrides);
|
||||
let logs: Arc<Mutex<Vec<LogEntry>>> = Arc::new(Mutex::new(Vec::new()));
|
||||
let mut engine = build_engine(effective_limits, Some(logs.clone()));
|
||||
|
||||
@@ -130,8 +130,7 @@ impl PicloudModuleResolver {
|
||||
for stmt in ast.statements() {
|
||||
match stmt {
|
||||
rhai::Stmt::Var(_, opts, _) if opts.intersects(ASTFlags::CONSTANT) => {}
|
||||
rhai::Stmt::Import(..) => {}
|
||||
rhai::Stmt::Noop(..) => {}
|
||||
rhai::Stmt::Import(..) | rhai::Stmt::Noop(..) => {}
|
||||
other => {
|
||||
return Err(format!(
|
||||
"module {name:?}: top-level {} is not allowed; \
|
||||
@@ -221,6 +220,7 @@ fn stmt_kind_label(stmt: &rhai::Stmt) -> &'static str {
|
||||
}
|
||||
|
||||
impl ModuleResolver for PicloudModuleResolver {
|
||||
#[allow(clippy::too_many_lines)]
|
||||
fn resolve(
|
||||
&self,
|
||||
engine: &RhaiEngine,
|
||||
@@ -237,7 +237,7 @@ impl ModuleResolver for PicloudModuleResolver {
|
||||
depth: &'r Mutex<u32>,
|
||||
armed: bool,
|
||||
}
|
||||
impl<'r> Drop for StackGuard<'r> {
|
||||
impl Drop for StackGuard<'_> {
|
||||
fn drop(&mut self) {
|
||||
if !self.armed {
|
||||
return;
|
||||
@@ -298,11 +298,14 @@ impl ModuleResolver for PicloudModuleResolver {
|
||||
armed: true,
|
||||
};
|
||||
|
||||
// Bridge to async. The resolver always runs on a `spawn_blocking`
|
||||
// thread (see LocalExecutorClient in orchestrator-core), which
|
||||
// still carries a Tokio handle. `try_current` makes the failure
|
||||
// mode explicit when callers wire up an `Engine` from a non-
|
||||
// Tokio context (typically a test harness).
|
||||
// Bridge to async. The resolver typically runs on a
|
||||
// `spawn_blocking` thread (see LocalExecutorClient in
|
||||
// orchestrator-core), but tests may invoke `Engine::execute`
|
||||
// directly from a multi-threaded Tokio task. `try_current` +
|
||||
// `block_in_place` covers both — on a blocking thread it's a
|
||||
// no-op, on a worker thread it tells the runtime to relocate
|
||||
// other tasks. `current_thread` runtimes still panic; non-
|
||||
// Tokio contexts surface a clean Runtime error.
|
||||
let handle = tokio::runtime::Handle::try_current().map_err(|_| {
|
||||
Box::new(EvalAltResult::ErrorInModule(
|
||||
path.to_string(),
|
||||
@@ -317,7 +320,7 @@ impl ModuleResolver for PicloudModuleResolver {
|
||||
})?;
|
||||
|
||||
let lookup_result: Result<Option<picloud_shared::ModuleScript>, ModuleSourceError> =
|
||||
handle.block_on(self.source.lookup(&self.cx, path));
|
||||
tokio::task::block_in_place(|| handle.block_on(self.source.lookup(&self.cx, path)));
|
||||
|
||||
let module_row = match lookup_result {
|
||||
Ok(Some(m)) => m,
|
||||
@@ -403,13 +406,8 @@ impl ModuleResolver for PicloudModuleResolver {
|
||||
// are resolved through the same `engine.set_module_resolver`
|
||||
// (which is THIS resolver), so cycle/depth tracking carries
|
||||
// through naturally.
|
||||
let module = Module::eval_ast_as_new(rhai::Scope::new(), &ast, engine).map_err(|e| {
|
||||
Box::new(EvalAltResult::ErrorInModule(
|
||||
path.to_string(),
|
||||
e,
|
||||
pos,
|
||||
))
|
||||
})?;
|
||||
let module = Module::eval_ast_as_new(rhai::Scope::new(), &ast, engine)
|
||||
.map_err(|e| Box::new(EvalAltResult::ErrorInModule(path.to_string(), e, pos)))?;
|
||||
let shared: SharedRhaiModule = module.into();
|
||||
|
||||
// Insert (possibly evicting via LRU). Subsequent imports of
|
||||
|
||||
Reference in New Issue
Block a user