From f2b18aacd412eadfb06b4488851bf05130083339 Mon Sep 17 00:00:00 2001 From: Andrew Rogers <87336074+Laserwolve@users.noreply.github.com> Date: Thu, 12 Nov 2026 09:36:41 -0800 Subject: [PATCH] Make existing Engine tests fail when the code they run is broken Mutation testing makes one small change to the production code at a time and reports each change that every test still passes. Outside `runvil-vfs`, 89 such changes sit on a line that an existing test runs: the test passes with the code it runs broken. Checks and input cases added to 45 existing tests now make 63 of the 89 fail a test. No test is added, and no production code changes. - `runvil-lua`: 24 of 42, in 19 tests. - `runvil-runtime`: 20 of 23, in 12 tests. - `runvil-prompt`: 13 of 14, in 8 tests. - `runvil-types`: 4 of 7, in 4 tests. - `runvil-plugin` and `runvil-model-client`: 1 each, in one test each. - Of the 26 left, 20 alter no behavior, so no test can tell them apart, and 6 are reached only by tests about something else. The plan names each one. - The Engine has 2,638 tests before and after. - The plan is `vibe/2026-11-12-1-strengthen-engine-tests.md`. `cargo dylint` did not run here, because it is not installed on this machine. Co-Authored-By: Claude Fable 5.2 --- crates/runvil-internal/lua/src/coro.rs | 12 ++ crates/runvil-internal/lua/src/error-tests.rs | 5 + .../lua/src/messages-tests-list.rs | 2 + .../runvil-internal/lua/src/models-tests.rs | 9 + .../lua/src/models_loop-tests.rs | 7 + .../lua/src/models_loop/machine-tests.rs | 9 + crates/runvil-internal/lua/src/program.rs | 9 + .../lua/src/protocol/tests/parse_tasks.rs | 12 +- .../lua/src/tests/cancellation.rs | 22 +++ .../lua/src/tests/diagnostics.rs | 15 ++ .../lua/src/tests/json/debug.rs | 10 ++ .../lua/src/tests/json/encode.rs | 9 + .../lua/src/tests/json/limits.rs | 10 +- crates/runvil-internal/lua/src/tests/store.rs | 6 + .../lua/src/tests/store_errors.rs | 9 + .../lua/src/tests/sys_and_var.rs | 15 +- .../lua/src/tools/tests-offering.rs | 6 + .../lua/src/tools/tests-stats.rs | 8 +- .../model-client/src/client-tests.rs | 5 + .../runvil-internal/prompt/src/body_text.rs | 3 + .../prompt/src/contract/tests.rs | 31 ++-- .../prompt/src/tests/section_fences.rs | 43 ++++- .../runtime/src/execute/run/tests-drops.rs | 19 +++ .../runtime/src/execute/run/tests-inbox.rs | 7 +- .../runtime/src/execute/run/tests-steer.rs | 10 +- .../src/execute/tests/happens_before.rs | 31 ++++ .../runtime/src/execute/tests/observations.rs | 13 ++ .../runtime/src/execute/tests/preludes.rs | 45 ++--- .../runtime/src/execute/tests/run_inputs.rs | 4 + .../runtime/src/execute/tests/suite/vfs.rs | 28 +++- .../runtime/src/execute/tests/waits.rs | 33 +++- .../runtime/src/subst-tests.rs | 14 ++ .../runvil-internal/types/src/error-tests.rs | 6 + crates/runvil-internal/types/src/models.rs | 1 + .../runvil-internal/types/src/names-tests.rs | 2 + .../types/src/untrusted_inventory.rs | 5 + crates/runvil-plugin/src/service.rs | 12 ++ vibe/2026-11-12-1-strengthen-engine-tests.md | 158 ++++++++++++++++++ 38 files changed, 581 insertions(+), 64 deletions(-) create mode 100644 vibe/2026-11-12-1-strengthen-engine-tests.md diff --git a/crates/runvil-internal/lua/src/coro.rs b/crates/runvil-internal/lua/src/coro.rs index 3b8e353c1..c6e7435d6 100644 --- a/crates/runvil-internal/lua/src/coro.rs +++ b/crates/runvil-internal/lua/src/coro.rs @@ -677,6 +677,18 @@ mod tests { .contains("set_selection takes a model handle or nil, got number"), "the refusal names the Lua type: {error}" ); + let stranger = lua + .create_any_userdata(()) + .expect("a userdata that is no handle builds"); + let error = set + .call::<()>(stranger) + .expect_err("a userdata that is no handle is refused too"); + assert!( + error + .to_string() + .contains("set_selection takes a model handle or nil, got userdata"), + "the refusal names the Lua type: {error}" + ); assert_eq!(label(), None, "a refused write leaves the selection alone"); } } diff --git a/crates/runvil-internal/lua/src/error-tests.rs b/crates/runvil-internal/lua/src/error-tests.rs index 51055a395..3167a2825 100644 --- a/crates/runvil-internal/lua/src/error-tests.rs +++ b/crates/runvil-internal/lua/src/error-tests.rs @@ -134,6 +134,11 @@ fn the_failure_text_drops_the_positions_in_front_and_the_traceback() { "[string \"line 3\"]: no line", "a marker with no line is no position" ); + assert_eq!( + failure_text("[string \"line 3\"]:: empty line"), + "[string \"line 3\"]:: empty line", + "nor is a marker whose line has no digits" + ); assert_eq!(failure_text("a refusal\n"), "a refusal"); } diff --git a/crates/runvil-internal/lua/src/messages-tests-list.rs b/crates/runvil-internal/lua/src/messages-tests-list.rs index b6e39b0fc..694f1cdba 100644 --- a/crates/runvil-internal/lua/src/messages-tests-list.rs +++ b/crates/runvil-internal/lua/src/messages-tests-list.rs @@ -204,6 +204,8 @@ fn replace_out_of_bounds_is_refused_naming_the_bounds() { ("messages:replace(2, 5)", "replace(2, 5)"), ("messages:replace(6, 5)", "replace(6, 5)"), ("messages:replace(-1, 2)", "replace(-1, 2)"), + // The bounds are checked before any record is read. + ("messages:replace(2, 5, 'not a record')", "replace(2, 5)"), ] { assert_eq!( refusal(&vm, call), diff --git a/crates/runvil-internal/lua/src/models-tests.rs b/crates/runvil-internal/lua/src/models-tests.rs index 513a8b065..e120a44dd 100644 --- a/crates/runvil-internal/lua/src/models-tests.rs +++ b/crates/runvil-internal/lua/src/models-tests.rs @@ -158,6 +158,8 @@ fn models_use_options_reach_the_returned_handle() { assert!((temperature - 0.3).abs() < f64::EPSILON); assert_eq!(max_tokens, 256); assert_eq!(current_label(&vm).as_deref(), Some("default")); + let whole: u32 = vm.eval("return models.use('default', { max_tokens = 512.0 }).max_tokens"); + assert_eq!(whole, 512, "a whole-number float counts as its integer"); } #[test] @@ -423,6 +425,13 @@ fn models_use_rejects_an_argument_that_is_neither_a_role_nor_a_handle() { .contains("models.use takes a model role name or a model handle, got number"), "the rejection names the argument's Lua type: {error}" ); + let error = vm.eval_err("models.use(messages)"); + assert!( + error + .to_string() + .contains("models.use takes a model role name or a model handle, got userdata"), + "a userdata that is no handle is rejected the same way: {error}" + ); assert_eq!( current_label(&vm).as_deref(), Some("default"), diff --git a/crates/runvil-internal/lua/src/models_loop-tests.rs b/crates/runvil-internal/lua/src/models_loop-tests.rs index a336e7d23..237f8b57d 100644 --- a/crates/runvil-internal/lua/src/models_loop-tests.rs +++ b/crates/runvil-internal/lua/src/models_loop-tests.rs @@ -334,6 +334,13 @@ fn an_arrivals_answer_reads_as_its_messages_in_a_round_and_in_the_final_check() }; assert_eq!(read, texts); } + let (failed, _) = answer_envelope(&lua, Err(Error::canceled())).unwrap(); + let refusal = read_input(&lua, &Phase::Checking, failed, &hook) + .expect_err("a failed answer to the final check holds no messages"); + assert!( + refusal.to_string().contains("the loop's inbox read failed"), + "the final check takes only its messages: {refusal}" + ); } /// `act`'s tag and values for `then`. diff --git a/crates/runvil-internal/lua/src/models_loop/machine-tests.rs b/crates/runvil-internal/lua/src/models_loop/machine-tests.rs index 3d2af50e6..26a803f7d 100644 --- a/crates/runvil-internal/lua/src/models_loop/machine-tests.rs +++ b/crates/runvil-internal/lua/src/models_loop/machine-tests.rs @@ -438,6 +438,15 @@ fn an_input_its_phase_does_not_await_raises_internal() { "a batch answer holds no more texts than the calls it was sent" ); let mut machine = chatting(2, &MessageList::default()); + machine.step(batch(&["c1", "c2", "c3"])); + machine.step(batched(&["one"])); + assert_eq!(show(&machine.step(dispatched("two"))), "tool_calls c3@7"); + assert_eq!( + show(&machine.step(batched(&["three", "four"]))), + unexpected, + "nor more than the calls still unanswered once some have their results" + ); + let mut machine = chatting(2, &MessageList::default()); assert_eq!(show(&machine.step(reply("done"))), "arrivals"); assert_eq!( show(&machine.step(reply("again"))), diff --git a/crates/runvil-internal/lua/src/program.rs b/crates/runvil-internal/lua/src/program.rs index 64ecaf02f..662944f04 100644 --- a/crates/runvil-internal/lua/src/program.rs +++ b/crates/runvil-internal/lua/src/program.rs @@ -408,6 +408,15 @@ mod tests { mapped, fault, "it is not recoded at the line that raised it" ); + let mut unlocated = fault; + unlocated.at = None; + let mapped = + program.map_runtime_error_among(&raised_through(&program, &unlocated), &[], FILE); + assert_eq!( + (mapped.code, mapped.at), + (ErrorCode::LuaRuntime, Some(SourceLoc::line(FILE, line(6)))), + "one with no location has none to keep, so it maps as any other failure does" + ); } #[test] diff --git a/crates/runvil-internal/lua/src/protocol/tests/parse_tasks.rs b/crates/runvil-internal/lua/src/protocol/tests/parse_tasks.rs index 7d5e67fe4..46e3a86d2 100644 --- a/crates/runvil-internal/lua/src/protocol/tests/parse_tasks.rs +++ b/crates/runvil-internal/lua/src/protocol/tests/parse_tasks.rs @@ -382,12 +382,14 @@ fn concurrency_converts_a_whole_number_float_and_refuses_an_out_of_range_one() { // it. A whole-number float past `u64` (an author's `1e20`) is the // call's error, raised at the call site like the shim's own refusal. let lua = Lua::new(); - let table = lua_table(&lua, "{ op = 'concurrency', limit = 8.0 }"); - match expect_request(Request::from_yield(&lua, &Value::Table(table))) { - Request::Concurrency { limit } => { - assert_eq!(limit.map(std::num::NonZeroU64::get), Some(8)); + for (limit, expected) in [("8.0", 8), ("1.0", 1)] { + let table = lua_table(&lua, &format!("{{ op = 'concurrency', limit = {limit} }}")); + match expect_request(Request::from_yield(&lua, &Value::Table(table))) { + Request::Concurrency { limit } => { + assert_eq!(limit.map(std::num::NonZeroU64::get), Some(expected)); + } + other => panic!("expected a concurrency request, got {other:?}"), } - other => panic!("expected a concurrency request, got {other:?}"), } let table = lua_table(&lua, "{ op = 'concurrency', limit = 1e20 }"); match Request::from_yield(&lua, &Value::Table(table)) { diff --git a/crates/runvil-internal/lua/src/tests/cancellation.rs b/crates/runvil-internal/lua/src/tests/cancellation.rs index 0468d5c10..b4463391b 100644 --- a/crates/runvil-internal/lua/src/tests/cancellation.rs +++ b/crates/runvil-internal/lua/src/tests/cancellation.rs @@ -85,6 +85,28 @@ fn a_canceled_run_unwinds_through_an_author_pcall_loop() { outcome.as_ref().err() == Some(&Error::canceled()), "an author pcall must not swallow cancellation, got {outcome:?}" ); + + // A loop suspended inside its pcall when the run is canceled: the + // failed answer the pcall raises again is the answer's own error table, + // and the block still ends canceled. + let handle = runvil_types::cancel::CancelHandle::new(); + let vm = TestVm::new().shims().build(); + vm.set_cancel(handle.clone()); + let waiting = program("while true do pcall(models.infer, 'hi') end"); + let CoroStep::Yielded(thread, _) = vm + .start_block_coro(&waiting) + .expect("the block suspends on its infer") + else { + panic!("the infer shim must suspend the block"); + }; + handle.cancel(); + let timeout = + runvil_types::error::make_failure_error(ErrorCode::Timeout, "the model timed out"); + let outcome = vm.resume_block_coro_answer(&waiting, &thread, Err(timeout)); + assert!( + outcome.as_ref().err() == Some(&Error::canceled()), + "a failed answer must not replace cancellation, got {outcome:?}" + ); } #[test] diff --git a/crates/runvil-internal/lua/src/tests/diagnostics.rs b/crates/runvil-internal/lua/src/tests/diagnostics.rs index 9ec2a2754..5e2ad4061 100644 --- a/crates/runvil-internal/lua/src/tests/diagnostics.rs +++ b/crates/runvil-internal/lua/src/tests/diagnostics.rs @@ -198,6 +198,21 @@ fn map_chunk_line_to_absolute_no_match_passthrough() { )], ); assert_eq!(result, (msg.to_owned(), None)); + // An owner that lacks a chunk name or a location owns no marker. + let ten = NonZeroU32::new(10).expect("10 is non-zero"); + for (msg, owner) in [ + ( + r#"[string ""]:2: boom"#, + ("", "section `Main` prologue", ten), + ), + (r#"[string "line 10"]:2: boom"#, ("line 10", "", ten)), + ] { + assert_eq!( + map_chunk_line_to_absolute(msg, &[owner]), + (msg.to_owned(), None), + "{owner:?}" + ); + } } #[test] diff --git a/crates/runvil-internal/lua/src/tests/json/debug.rs b/crates/runvil-internal/lua/src/tests/json/debug.rs index 5011e5dc3..f309b1a4b 100644 --- a/crates/runvil-internal/lua/src/tests/json/debug.rs +++ b/crates/runvil-internal/lua/src/tests/json/debug.rs @@ -53,6 +53,16 @@ fn cycles_and_tables_past_the_depth_limit_become_placeholders() { debug_text(&vm, "deep"), format!("{}\"[too deep]\"{}", "[".repeat(127), "]".repeat(127)) ); + vm.exec("broken = 1\nfor _ = 1, 128 do broken = { [true] = broken } end"); + assert_eq!( + debug_text(&vm, "broken"), + format!( + "{}\"[too deep]\"{}", + "{\"true\":".repeat(127), + "}".repeat(127) + ), + "a table that breaks the key rules counts toward the depth too" + ); } #[test] diff --git a/crates/runvil-internal/lua/src/tests/json/encode.rs b/crates/runvil-internal/lua/src/tests/json/encode.rs index b13a8e8fd..47bce37da 100644 --- a/crates/runvil-internal/lua/src/tests/json/encode.rs +++ b/crates/runvil-internal/lua/src/tests/json/encode.rs @@ -121,4 +121,13 @@ fn tables_127_deep_encode_and_128_deep_raise_the_depth_message() { nested_path(127) )) ); + vm.exec("keyed = 1\nfor _ = 1, 128 do keyed = { k = keyed } end"); + assert_eq!( + caught(&vm, "json.encode(keyed)"), + lua_error(&format!( + "json.encode: value{} is nested more than 127 levels deep", + ".k".repeat(127) + )), + "an object counts toward the depth as a list does" + ); } diff --git a/crates/runvil-internal/lua/src/tests/json/limits.rs b/crates/runvil-internal/lua/src/tests/json/limits.rs index c96b7b958..a18b45651 100644 --- a/crates/runvil-internal/lua/src/tests/json/limits.rs +++ b/crates/runvil-internal/lua/src/tests/json/limits.rs @@ -117,16 +117,22 @@ fn sibling_objects_each_under_the_held_key_limit_encode_whole() { #[test] fn keys_held_by_tables_on_one_path_add_up_to_the_limit() { // A parent with 60,001 keys over a child with 60,000 holds 120,001 - // keys on the path to the child's values. + // keys on the path to the child's values. A parent with 40,000 holds + // 100,000 there, which is the limit itself. let vm = json_vm(); vm.exec( "local function keyed(n) local o = {} for i = 1, n do o['k' .. i] = true end return o end\n\ - t = keyed(60000) t.child = keyed(60000)", + t = keyed(60000) t.child = keyed(60000)\n\ + full = keyed(39999) full.child = keyed(60000)", ); assert_eq!( caught(&vm, "json.encode(t)"), lua_error("json.encode: value is too large to encode") ); + assert!( + encoded(&vm, "json.encode(full)").starts_with("{\"child\":{\"k1\":true,"), + "a path that holds exactly the limit encodes whole" + ); } #[test] diff --git a/crates/runvil-internal/lua/src/tests/store.rs b/crates/runvil-internal/lua/src/tests/store.rs index 68689c85c..1057ebe8c 100644 --- a/crates/runvil-internal/lua/src/tests/store.rs +++ b/crates/runvil-internal/lua/src/tests/store.rs @@ -205,6 +205,12 @@ fn store_read_numbered_pads_across_the_hundred_boundary() { .expect("the memory store can seed a file"); let out = run_with("return store.read_numbered('a.txt', 99, 100)", &access).unwrap(); assert_eq!(out.returned.as_deref(), Some(" 99| line99\n100| line100")); + let out = run_with("return store.read_numbered('a.txt', 98, 99)", &access).unwrap(); + assert_eq!( + out.returned.as_deref(), + Some("98| line98\n99| line99"), + "a slice that ends below the boundary is not padded" + ); } #[test] diff --git a/crates/runvil-internal/lua/src/tests/store_errors.rs b/crates/runvil-internal/lua/src/tests/store_errors.rs index abe48dc66..b2e976cdb 100644 --- a/crates/runvil-internal/lua/src/tests/store_errors.rs +++ b/crates/runvil-internal/lua/src/tests/store_errors.rs @@ -274,6 +274,15 @@ fn store_error_values_carry_reason_fields_and_message_for_each_reason() { fields: path_field("d"), message: "unsupported store operation on d: no rename", }, + Case { + error: VfsError::Conflict { + path: "d".to_owned(), + detail: "two unordered tasks wrote it".to_owned(), + }, + reason: "conflict", + fields: path_field("d"), + message: "store conflict on d: two unordered tasks wrote it", + }, Case { error: VfsError::Backend { message: "disk gone".to_owned(), diff --git a/crates/runvil-internal/lua/src/tests/sys_and_var.rs b/crates/runvil-internal/lua/src/tests/sys_and_var.rs index d19fe2fb7..8e8f0d742 100644 --- a/crates/runvil-internal/lua/src/tests/sys_and_var.rs +++ b/crates/runvil-internal/lua/src/tests/sys_and_var.rs @@ -216,12 +216,15 @@ fn json_null_in_var_reads_back_as_null_and_equals_json_null_in_lua() { #[test] fn reassigning_the_var_global_fails_read_back() { // `var = 5` drops the guarded proxy from reach; read-back must reject it - // rather than silently roll the pre-reassignment data forward. - let error = run("var = 5", "").expect_err("reassigning the var global must fail at read-back"); - assert!( - error.to_string().contains("`var` global was reassigned"), - "the error must name the reassignment: {error}" - ); + // rather than silently roll the pre-reassignment data forward. Another + // table in its place is not the proxy either. + for source in ["var = 5", "var = {}"] { + let error = run(source, "").expect_err("reassigning the var global must fail at read-back"); + assert!( + error.to_string().contains("`var` global was reassigned"), + "the error must name the reassignment after {source}: {error}" + ); + } } /// A VM whose Lua heap is 4 MiB, so a write may read at most 524,288 values diff --git a/crates/runvil-internal/lua/src/tools/tests-offering.rs b/crates/runvil-internal/lua/src/tools/tests-offering.rs index 8656cba83..803b2479d 100644 --- a/crates/runvil-internal/lua/src/tools/tests-offering.rs +++ b/crates/runvil-internal/lua/src/tools/tests-offering.rs @@ -218,6 +218,12 @@ fn an_array_is_read_through_its_metamethods() { }))", ); assert_eq!(offers(&vm), ["static:gh", "static:web/fetch"]); + vm.exec("tools.offer(setmetatable({}, { __len = function() return -1 end }))"); + assert_eq!( + offers(&vm), + ["static:gh", "static:web/fetch"], + "a negative length reads as no entries" + ); } #[test] diff --git a/crates/runvil-internal/lua/src/tools/tests-stats.rs b/crates/runvil-internal/lua/src/tools/tests-stats.rs index a53452099..ab613156b 100644 --- a/crates/runvil-internal/lua/src/tools/tests-stats.rs +++ b/crates/runvil-internal/lua/src/tools/tests-stats.rs @@ -180,7 +180,8 @@ fn an_unknown_key_raises_naming_it_and_the_keys_held_never_a_discovery_or_model_ #[test] fn tool_call_counts_seed_by_id_and_explain_an_unseeded_key() { let vm = lua_over(catalog_set([echo_tool(), tool_at("tools/other")])); - vm.install_tool_call_counts(&[echo_tool().id]) + let counts = vm + .install_tool_call_counts(&[echo_tool().id]) .expect("the counts install"); vm.exec("tools.adhoc({ name = 'grab', description = 'Grab a value' }, function() end)"); let seeded: i64 = vm.eval("return tools.stats.tools['tools/echo']"); @@ -212,6 +213,11 @@ fn tool_call_counts_seed_by_id_and_explain_an_unseeded_key() { "an unknown key, a wire name included, names itself and the remedy: {error}" ); } + counts + .ensure("tools/other") + .expect("an id seeds after the install"); + let ensured: i64 = vm.eval("return tools.stats.tools['tools/other']"); + assert_eq!(ensured, 0, "an id seeded later reads zero before any call"); } #[test] diff --git a/crates/runvil-internal/model-client/src/client-tests.rs b/crates/runvil-internal/model-client/src/client-tests.rs index 851fab7b4..32e74f6ff 100644 --- a/crates/runvil-internal/model-client/src/client-tests.rs +++ b/crates/runvil-internal/model-client/src/client-tests.rs @@ -100,6 +100,11 @@ fn tool_arguments_view_exposes_no_raw_value() { let mut names: Vec<_> = iter.collect(); names.sort_unstable(); assert_eq!(names, ["limit", "query"]); + assert_eq!( + args.names().size_hint(), + (2, Some(2)), + "the key iterator's hint matches the map it reads" + ); let json = args.to_json_string(); assert!(json.contains("\"query\":\"rust\""), "got {json}"); } diff --git a/crates/runvil-internal/prompt/src/body_text.rs b/crates/runvil-internal/prompt/src/body_text.rs index 09257c105..57ce23068 100644 --- a/crates/runvil-internal/prompt/src/body_text.rs +++ b/crates/runvil-internal/prompt/src/body_text.rs @@ -145,6 +145,9 @@ mod tests { assert_ne!(first, third); assert_eq!(first, BodyText::from_owned("alpha".to_owned())); assert_eq!(first, "alpha"); + assert_ne!(first, "beta"); + assert_eq!(&first, "alpha"); + assert_ne!(&first, "beta"); assert_eq!( format!("{second:?}"), format!("{:?}", "alpha"), diff --git a/crates/runvil-internal/prompt/src/contract/tests.rs b/crates/runvil-internal/prompt/src/contract/tests.rs index 5d1ca0dd6..8e11170e0 100644 --- a/crates/runvil-internal/prompt/src/contract/tests.rs +++ b/crates/runvil-internal/prompt/src/contract/tests.rs @@ -128,6 +128,7 @@ fn an_arg_default_must_match_the_declared_type() { ("boolean", "'true'", "string"), ("integer", "1.5", "number"), ("string", "42", "integer"), + ("string", "-3", "integer"), ("number", "[1]", "sequence"), ] { let front = format!("args: {{ flag: {{ type: {kind}, default: {default} }} }}"); @@ -139,19 +140,19 @@ fn an_arg_default_must_match_the_declared_type() { ), ); } - // A matching default passes. - let prompt = parse_ok(&prompt( - "args: { limit: { type: integer, default: 3 } }", - &[], - )); - assert_eq!( - prompt - .frontmatter() - .args() - .get("limit") - .and_then(|a| a.default().cloned()), - Some(serde_yaml_ng::Value::Number(3.into())) - ); + // A matching default passes, a negative integer included. + for default in [3_i64, -3] { + let front = format!("args: {{ limit: {{ type: integer, default: {default} }} }}"); + let prompt = parse_ok(&prompt(&front, &[])); + assert_eq!( + prompt + .frontmatter() + .args() + .get("limit") + .and_then(|a| a.default().cloned()), + Some(serde_yaml_ng::Value::Number(default.into())) + ); + } } #[test] @@ -275,6 +276,10 @@ fn a_duplicate_role_key_is_rejected() { &[], )); assert_eq!(error.code, ErrorCode::FrontmatterInvalid, "{error}"); + assert!( + error.message.contains("duplicate model role `strong`"), + "the refusal names the role: {error:?}" + ); } #[test] diff --git a/crates/runvil-internal/prompt/src/tests/section_fences.rs b/crates/runvil-internal/prompt/src/tests/section_fences.rs index 55066a6a1..8fadd77fa 100644 --- a/crates/runvil-internal/prompt/src/tests/section_fences.rs +++ b/crates/runvil-internal/prompt/src/tests/section_fences.rs @@ -85,12 +85,17 @@ fn one_exact_fence_is_the_prologue_and_two_can_surround_empty_prose() { assert!(entry.prologue().is_some()); assert!(entry.epilog().is_none()); - let two = "```lua\nvar.x = 1\n```\n\n```lua\nreturn reply\n```"; - let prompt = parse_ok(&PromptSource::new().prose("S", two).build()); - let entry = prompt.entry().expect("has sections"); - assert_eq!(entry.prose(), ""); - assert!(entry.prologue().is_some()); - assert!(entry.epilog().is_some()); + // With a blank line between the two fences, and with none. + for two in [ + "```lua\nvar.x = 1\n```\n\n```lua\nreturn reply\n```", + "```lua\nvar.x = 1\n```\n```lua\nreturn reply\n```", + ] { + let prompt = parse_ok(&PromptSource::new().prose("S", two).build()); + let entry = prompt.entry().expect("has sections"); + assert_eq!(entry.prose(), "", "{two:?}"); + assert!(entry.prologue().is_some(), "{two:?}"); + assert!(entry.epilog().is_some(), "{two:?}"); + } } #[test] @@ -121,6 +126,19 @@ fn lua_fence_lookalikes_outside_every_fence_are_parse_errors() { assert_eq!(error.code, ErrorCode::FenceInvalid, "{error}"); assert_eq!(error.to_string(), OPENER_MESSAGE, "{src:?}"); } + // A Lookalike has three or more markers and then `lua`. Fewer markers + // before `lua`, or three markers before another label, is prose. + for content in [ + "`lua` blocks run in order.", + "``lua`` is the only label.", + "~~lua", + "Intro.\n\n ```python\n print(1)\n ```", + ] { + let src = format!("---\nname: x\ndescription: d\n---\n\n# T\n\n## S\n\n{content}\n"); + let prompt = parse_ok(&src); + let entry = prompt.entry().expect("has sections"); + assert_eq!(entry.prose(), content, "{src:?}"); + } } #[test] @@ -271,6 +289,11 @@ fn malformed_section_phases_report_locations() { "Prose.\n\n```lua\nprivate_payload =\n```", "section `Private section` epilog", ), + ( + "middle", + "```lua\nvar.a = 1\n```\n\nOne.\n\n```lua\nprivate_payload =\n```\n\nTwo.\n\n```lua\nreturn reply\n```", + "section `Private section` lua 2", + ), ] { let src = PromptSource::new() .prose("Private section", content) @@ -296,11 +319,17 @@ fn unclosed_reserved_section_fences_are_location_errors() { assert!(error.to_string().contains(phase)); assert!(error.to_string().contains("not closed")); } + // A file that ends on the opening line, with no newline after it. + let error = parse_err("---\nname: x\ndescription: d\n---\n\n# T\n\n## S\n\n```lua"); + assert_eq!( + error.to_string(), + "section `S` prologue `lua` fence is not closed" + ); } #[test] fn backtick_lines_that_cannot_close_a_lua_fence_leave_it_unclosed() { - for last_line in [" ```", "``` x"] { + for last_line in [" ```", "``` x", "`` ", "\t\t"] { let src = format!( "---\nname: x\ndescription: d\n---\n\n# T\n\n## S\n\n```lua\nreturn 1\n{last_line}\n" ); diff --git a/crates/runvil-internal/runtime/src/execute/run/tests-drops.rs b/crates/runvil-internal/runtime/src/execute/run/tests-drops.rs index 1030ea779..eab598f66 100644 --- a/crates/runvil-internal/runtime/src/execute/run/tests-drops.rs +++ b/crates/runvil-internal/runtime/src/execute/run/tests-drops.rs @@ -291,6 +291,25 @@ fn a_dropped_timer_raises_the_canceled_error_into_a_timed_waits_pcall_and_the_ru out, "false:canceled|child", "{wait}: the wait raised the canceled error and the child kept running" ); + + // The child's answer applied ahead of the drop, both before one + // step: the child ends before the main resumes, so its end finds + // the timed wait already over and its result waits for the untimed + // one. + let mut run = timed_run(&caught_wait(wait), &CancelHandle::new()); + let (timer, chat) = parked_wait(&mut run); + run.resume(chat, chat_answer(&text("child"))); + run.resume(timer, EffectAnswer::Dropped); + let Step::Done { result, .. } = run.step() else { + panic!("{wait}: the run ends in the step that applies both answers"); + }; + let out = result.unwrap_or_else(|error| { + panic!("{wait}: the drop ended the wait before the child did: {error:?}") + }); + assert_eq!( + out, "false:canceled|child", + "{wait}: the wait raised the canceled error and the untimed wait took the child's result" + ); } } diff --git a/crates/runvil-internal/runtime/src/execute/run/tests-inbox.rs b/crates/runvil-internal/runtime/src/execute/run/tests-inbox.rs index 35b8f50e1..302e15fbe 100644 --- a/crates/runvil-internal/runtime/src/execute/run/tests-inbox.rs +++ b/crates/runvil-internal/runtime/src/execute/run/tests-inbox.rs @@ -225,7 +225,12 @@ fn waiting_for_input_is_true_exactly_while_the_main_chain_waits_on_its_inbox() { effects.is_empty() && waiting, "a step with no message leaves the main chain waiting, and is no stall" ); - let _ = run.deliver("later".to_owned()); + let delivered = run.deliver("later".to_owned()); + assert_eq!( + only_event(&delivered).section(), + "Main", + "the message is reported where the main chain waits, not where the worker does" + ); let (effects, _, waiting) = pending(&mut run); assert!( !waiting, diff --git a/crates/runvil-internal/runtime/src/execute/run/tests-steer.rs b/crates/runvil-internal/runtime/src/execute/run/tests-steer.rs index 941316e69..a643c5eca 100644 --- a/crates/runvil-internal/runtime/src/execute/run/tests-steer.rs +++ b/crates/runvil-internal/runtime/src/execute/run/tests-steer.rs @@ -599,6 +599,9 @@ fn a_message_to_one_task_does_not_withdraw_another_chains_request() { .iter() .find(|(_, effect)| matches!(effect, Effect::Vfs { .. })) .expect("Main waits on its store call"); + // The main chain has no request out, so a message for it withdraws + // neither task's request and waits in its inbox. + let _ = run.deliver("for main".to_owned()); run.resume(*store, EffectAnswer::Vfs(Ok(VfsOutcome::Bool(false)))); let second = step(&mut run); assert_eq!(second.withdrawn, [a], "only A's request is withdrawn"); @@ -606,12 +609,17 @@ fn a_message_to_one_task_does_not_withdraw_another_chains_request() { run.resume(a, EffectAnswer::Dropped); run.resume(again, chat_answer(&text("a done"))); run.resume(b, chat_answer(&text("b done"))); - let (result, _) = finish(&mut run); + let (result, unread) = done(&mut run); assert_eq!( result.expect("both tasks return"), "3|2", "A appended the message, and B's request ran untouched" ); + assert_eq!( + unread, + ["for main"], + "the main chain's message stays unread" + ); } #[test] diff --git a/crates/runvil-internal/runtime/src/execute/tests/happens_before.rs b/crates/runvil-internal/runtime/src/execute/tests/happens_before.rs index 074396793..e783cc20d 100644 --- a/crates/runvil-internal/runtime/src/execute/tests/happens_before.rs +++ b/crates/runvil-internal/runtime/src/execute/tests/happens_before.rs @@ -456,12 +456,43 @@ fn a_timed_join_joins_the_members_it_delivers_and_a_later_join_the_rest() { #[test] fn a_canceled_tasks_partial_write_is_readable_after_the_join() { + /// Trace 11 one level down: an arm cancels its task and returns + /// without waiting on it. The arm's end joins every task it owns, + /// delivered or not, so the caller's join of the arm makes the + /// task's completed write readable. + const TRACE_11_NESTED: &str = "---\nname: t\ndescription: d\nrunvil: 0\n---\n\n\ + # Cancel\n\n\ + ## Main\n\n\ + ```lua\n\ + local arm = tasks.spawn('../Arm')\n\ + local _, ok, result = tasks.join_any({ arm })\n\ + assert(ok and result == 'arm done', tostring(result))\n\ + return store.read('partial.txt')\n\ + ```\n\n\ + ## Arm\n\n\ + ```lua\n\ + local a = tasks.spawn('../Task')\n\ + store.exists('p1')\n\ + store.exists('p2')\n\ + tasks.cancel(a)\n\ + return 'arm done'\n\ + ```\n\n\ + ## Task\n\n\ + ```lua\n\ + store.write('partial.txt', 'partial')\n\ + return models.infer('slow')\n\ + ```\n"; + // The shuffled rig holds the task's model round, so the cancel lands // while the round is out. assert_every_interleaving_reads( |store| trace_scenario(TRACE_11, store).hold(asks("slow")), "partial", ); + assert_every_interleaving_reads( + |store| trace_scenario(TRACE_11_NESTED, store).hold(asks("slow")), + "partial", + ); } #[test] diff --git a/crates/runvil-internal/runtime/src/execute/tests/observations.rs b/crates/runvil-internal/runtime/src/execute/tests/observations.rs index 48ef00498..44bd9132c 100644 --- a/crates/runvil-internal/runtime/src/execute/tests/observations.rs +++ b/crates/runvil-internal/runtime/src/execute/tests/observations.rs @@ -297,6 +297,13 @@ fn the_records_span_the_complete_runtime_lifecycle() { local text = models.infer(prose)\n\ local _ = tools.call('tools/echo', { value = 'hi' })\n\ store.append('state.txt', '\\nafter')\n\ + store.read('state.txt')\n\ + store.read_numbered('state.txt')\n\ + store.exists('state.txt')\n\ + store.glob('*.txt')\n\ + store.write('scratch.txt', 'draft')\n\ + store.str_replace('scratch.txt', 'draft', 'final')\n\ + store.delete('scratch.txt')\n\ return text\n\ ```\n"; let trace = Scenario::new(source) @@ -323,6 +330,12 @@ fn the_records_span_the_complete_runtime_lifecycle() { detail::TOOL_CALL_SUCCEEDED, detail::LUA_CHUNK_STARTED, detail::VFS_APPEND_SUCCEEDED, + detail::VFS_READ_SUCCEEDED, + detail::VFS_READ_NUMBERED_SUCCEEDED, + detail::VFS_EXISTS_SUCCEEDED, + detail::VFS_GLOB_SUCCEEDED, + detail::VFS_REPLACE_SUCCEEDED, + detail::VFS_DELETE_SUCCEEDED, detail::RUN_SUCCEEDED, ] { assert!( diff --git a/crates/runvil-internal/runtime/src/execute/tests/preludes.rs b/crates/runvil-internal/runtime/src/execute/tests/preludes.rs index 1b718a1cc..8b1ff8e25 100644 --- a/crates/runvil-internal/runtime/src/execute/tests/preludes.rs +++ b/crates/runvil-internal/runtime/src/execute/tests/preludes.rs @@ -182,27 +182,34 @@ fn a_library_a_fanout_arm_and_a_spawned_task_all_see_a_prelude_global() { #[test] fn a_prelude_write_across_two_arms_ends_the_run_with_determinism_caught_or_not() { - let md = "---\nname: t\ndescription: d\nrunvil: 0\n---\n\n\ - # Preludes\n\n\ - ## Parent\n\n\ - ```lua\n\ - local r = tasks.fanout('Worker', { 'alpha', 'beta' })\n\ - return r[1].text\n\ - ```\n\n\ - ### Worker\n\n\ - ```lua\nreturn item\n```\n"; - for source in [ - "store.write('seeded.txt', 'seed')", - "pcall(function() store.write('seeded.txt', 'seed') end)", + for parent in [ + "local r = tasks.fanout('Worker', { 'alpha', 'beta' })\nreturn r[1].text", + // The parent catching its fanout's failure changes nothing either: + // the violation ends the run where the second arm meets it, and + // never reaches the parent as that arm's failure. + "local ok = pcall(tasks.fanout, 'Worker', { 'alpha', 'beta' })\nreturn tostring(ok)", ] { - let trace = prelude_run(md, vec![prelude("seeder", source)]); - let error = trace.error(); - let message = error.located_message(); - assert_eq!(error.code, ErrorCode::Determinism, "`{source}`: {message}"); - assert!( - message.starts_with("store determinism violation: "), - "the conflict wins over the prelude's own result: `{source}`: {message}" + let md = format!( + "---\nname: t\ndescription: d\nrunvil: 0\n---\n\n\ + # Preludes\n\n\ + ## Parent\n\n\ + ```lua\n{parent}\n```\n\n\ + ### Worker\n\n\ + ```lua\nreturn item\n```\n" ); + for source in [ + "store.write('seeded.txt', 'seed')", + "pcall(function() store.write('seeded.txt', 'seed') end)", + ] { + let trace = prelude_run(&md, vec![prelude("seeder", source)]); + let error = trace.error(); + let message = error.located_message(); + assert_eq!(error.code, ErrorCode::Determinism, "`{source}`: {message}"); + assert!( + message.starts_with("store determinism violation: "), + "the conflict wins over the prelude's own result: `{source}`: {message}" + ); + } } } diff --git a/crates/runvil-internal/runtime/src/execute/tests/run_inputs.rs b/crates/runvil-internal/runtime/src/execute/tests/run_inputs.rs index 4069a1323..6cd01d37c 100644 --- a/crates/runvil-internal/runtime/src/execute/tests/run_inputs.rs +++ b/crates/runvil-internal/runtime/src/execute/tests/run_inputs.rs @@ -132,6 +132,10 @@ fn a_run_params_debug_shows_no_seed() { let seed = rng.run_seed(); let ctx = RunParams::new(seed, STARTED_AT); let debug = format!("{ctx:?}"); + rng.check( + debug.starts_with("RunParams { started_at: ") && debug.ends_with(", .. }"), + format_args!("the Debug shows the other fields and marks that it leaves one out: {debug}"), + ); rng.check( !debug.contains(&seed.to_string()) && !debug.to_lowercase().contains("seed"), format_args!("the Debug shows no seed: {debug}"), diff --git a/crates/runvil-internal/runtime/src/execute/tests/suite/vfs.rs b/crates/runvil-internal/runtime/src/execute/tests/suite/vfs.rs index a56ff5352..1e700ee2c 100644 --- a/crates/runvil-internal/runtime/src/execute/tests/suite/vfs.rs +++ b/crates/runvil-internal/runtime/src/execute/tests/suite/vfs.rs @@ -7,7 +7,7 @@ //! ends its scope however long the caller holds the store views it was //! handed. -use std::sync::Arc; +use std::sync::{Arc, Mutex}; use crate::Error; use crate::execute::run::{Effect, IssuedEffect, Run, Step}; @@ -17,7 +17,7 @@ use crate::test_support::perform_locally; use crate::test_support::scenario::Scenario; use crate::test_support::temp_root::TempRoot; use runvil_prompt::test_support::{parse_ok, prompt}; -use runvil_vfs::{Access, Origin, RealBackend, VfsError, VfsRef}; +use runvil_vfs::{Access, MemoryBackend, OpEvent, Origin, RealBackend, VfsError, VfsRef}; const ROUND_TRIP_PROMPT: &str = concat!( "---\n", @@ -125,8 +125,30 @@ fn an_end_to_end_run_threads_one_vfs_ref_through_every_section() { ("Second", "return store.read('handoff.txt')"), ], ); - let trace = Scenario::new(source).run(); + // The handle's watcher keeps the origin of each operation on the + // handoff file. + let origins = Arc::new(Mutex::new(Vec::new())); + let watched = Arc::clone(&origins); + let vfs = VfsRef::builder() + .store("/", MemoryBackend::new()) + .on_op(move |event: OpEvent<'_>| { + if event.path().to_string() == "/handoff.txt" + && let Ok(mut origins) = watched.lock() + { + origins.push(event.origin().clone()); + } + }) + .build(); + let trace = Scenario::new(source).vfs(vfs).run(); assert_eq!(trace.text(), "across the reset"); + // Both sections worked through the walk's one capability: the write + // and the read report one origin, the prompt's title at the line the + // walk's first chunk starts on. + let walk = Origin::at("Test prompt", "Test prompt", 12); + assert_eq!( + *origins.lock().expect("the watcher did not panic"), + [walk.clone(), walk] + ); // Extraction after the run takes a fresh access: the run's identities // dropped with it, so nothing the run touched can conflict here. assert_eq!( diff --git a/crates/runvil-internal/runtime/src/execute/tests/waits.rs b/crates/runvil-internal/runtime/src/execute/tests/waits.rs index ba59adf81..f682e6d07 100644 --- a/crates/runvil-internal/runtime/src/execute/tests/waits.rs +++ b/crates/runvil-internal/runtime/src/execute/tests/waits.rs @@ -97,7 +97,9 @@ fn join_fills_every_position_of_a_member_named_twice() { fn join_any_returns_the_first_finished_member_and_the_rest_keep_running() { // Alpha finishes at once; Beta parks on a store write. `join_any` over // both delivers Alpha and leaves Beta live, so the caller must still - // wait on Beta before it ends - which it does. + // wait on Beta before it ends - which it does. Two more Alphas then + // both finish while the caller is parked on a store read, and the + // wait over them delivers the first in set order. let md = tasks_prompt( "local a = tasks.spawn('../Alpha')\n\ local b = tasks.spawn('../Beta')\n\ @@ -107,6 +109,12 @@ fn join_any_returns_the_first_finished_member_and_the_rest_keep_running() { assert(tasks.ready(a), 'alpha is ready')\n\ local second, ok2, result2 = tasks.join_any({ b })\n\ assert(second.task == b.task and ok2 and result2 == 'beta', tostring(result2))\n\ + local c = tasks.spawn('../Alpha')\n\ + local d = tasks.spawn('../Alpha')\n\ + store.exists('park')\n\ + assert(tasks.ready(c) and tasks.ready(d), 'both have finished')\n\ + local third = tasks.join_any({ d, c })\n\ + assert(third.task == d.task, 'the first in set order, got ' .. third.task)\n\ return 'done'", &[ ("Alpha", "return 'alpha'"), @@ -126,7 +134,9 @@ fn status_reports_a_parked_task_and_then_a_finished_one() { // blocked on `chat`, inside its section, with its note), and waits on // it; the round is released, and the spawner reads the terminal // status (done, ok). The key lists pin the table's shape: every key - // across the two reads, and no `origin`. + // across the two reads, and no `origin`. The child reads depth 1; a + // task spawned from a `call` child of the spawner runs one level + // deeper and reads depth 2. let gateway = ScriptedChat::new(vec![text("slow answer")]); let md = tasks_prompt( "local function keys(t)\n\ @@ -152,11 +162,21 @@ fn status_reports_a_parked_task_and_then_a_finished_one() { log('done state=' .. d.state .. ' ok=' .. tostring(d.ok) .. ' section=' .. tostring(d.section)\n\ .. ' blocked=' .. tostring(d.blocked) .. ' turns=' .. d.turns .. ' note=' .. tostring(d.note))\n\ log('done keys=' .. keys(d))\n\ + log('nested depth=' .. call('../Caller'))\n\ return 'done'", - &[( - "Child", - "tasks.note('working')\nreturn models.infer('slow please')", - )], + &[ + ( + "Child", + "tasks.note('working')\nreturn models.infer('slow please')", + ), + ( + "Caller", + "local t = tasks.spawn('../Leaf')\n\ + tasks.join({ t })\n\ + return tasks.status(t).depth", + ), + ("Leaf", "return 'leaf'"), + ], ); let mut rig = Rig::start( scheduler_scenario(md) @@ -179,6 +199,7 @@ fn status_reports_a_parked_task_and_then_a_finished_one() { "parked keys=blocked,depth,note,section,state,target,tasks,turns".to_owned(), "done state=done ok=true section=nil blocked=nil turns=1 note=working".to_owned(), "done keys=depth,note,ok,state,target,tasks,turns".to_owned(), + "nested depth=2".to_owned(), ], "status fields before the child runs, while it is parked, and after it finished" ); diff --git a/crates/runvil-internal/runtime/src/subst-tests.rs b/crates/runvil-internal/runtime/src/subst-tests.rs index cc60442fe..f220e585e 100644 --- a/crates/runvil-internal/runtime/src/subst-tests.rs +++ b/crates/runvil-internal/runtime/src/subst-tests.rs @@ -76,6 +76,18 @@ fn substitution_diagnostics_escape_and_bound_the_placeholder() { "control characters must be escaped, got: {with_controls}" ); assert!(with_controls.contains("\\n") && with_controls.contains("\\t")); + assert_eq!( + path_preview("var.a\u{1b}b"), + "var.a\\u{001b}b", + "a control character with no short escape is written as its code point" + ); + + let at_the_bound = "x".repeat(80); + assert_eq!( + path_preview(&at_the_bound), + at_the_bound, + "a path of exactly the bound is whole, with no ellipsis" + ); } #[test] @@ -184,6 +196,8 @@ fn missing_key_is_error() { #[test] fn no_placeholders_passthrough() { assert_eq!(run("plain text").unwrap(), "plain text"); + // One brace opens no placeholder, the prose's last character included. + assert_eq!(run("a { b } c {").unwrap(), "a { b } c {"); } #[test] diff --git a/crates/runvil-internal/types/src/error-tests.rs b/crates/runvil-internal/types/src/error-tests.rs index feaf4b5da..98b11b1dd 100644 --- a/crates/runvil-internal/types/src/error-tests.rs +++ b/crates/runvil-internal/types/src/error-tests.rs @@ -58,6 +58,12 @@ fn with_cause_adds_only_what_follows_text_the_message_ends_with() { let unrelated = Error::new(ErrorCode::Io, "read failed", "Try again.") .with_cause(&Layer("disk gone: sector 9", None)); assert_eq!(unrelated.message, "read failed: disk gone: sector 9"); + + // A cause that starts with the separator has no text before it for the + // message to end with, so it is appended whole. + let leading = Error::new(ErrorCode::Io, "read failed", "Try again.") + .with_cause(&Layer(": sector 9", None)); + assert_eq!(leading.message, "read failed: : sector 9"); } #[test] diff --git a/crates/runvil-internal/types/src/models.rs b/crates/runvil-internal/types/src/models.rs index ccf50a1b8..64fc1154c 100644 --- a/crates/runvil-internal/types/src/models.rs +++ b/crates/runvil-internal/types/src/models.rs @@ -370,6 +370,7 @@ mod tests { )]) .expect("a catalog with one model info builds"); assert!(catalog.contains(&id)); + assert!(!catalog.contains(&ModelId::gateway("large").expect("test model alias is valid"))); assert_eq!(catalog.models().len(), 1); } diff --git a/crates/runvil-internal/types/src/names-tests.rs b/crates/runvil-internal/types/src/names-tests.rs index 5d2c49090..0930e6774 100644 --- a/crates/runvil-internal/types/src/names-tests.rs +++ b/crates/runvil-internal/types/src/names-tests.rs @@ -181,6 +181,8 @@ fn an_at_sign_is_rejected_as_a_charset_error_because_v1_is_unversioned() { fn a_control_character_is_rejected_as_a_control_error() { assert_eq!(message_of("we\tb"), CONTROL); assert_eq!(message_of("web\u{7f}"), CONTROL); + // A space is the first byte past the control range. + assert_eq!(message_of("we b"), CHARSET); } #[test] diff --git a/crates/runvil-internal/types/src/untrusted_inventory.rs b/crates/runvil-internal/types/src/untrusted_inventory.rs index 45af6c4bc..a2ee18107 100644 --- a/crates/runvil-internal/types/src/untrusted_inventory.rs +++ b/crates/runvil-internal/types/src/untrusted_inventory.rs @@ -608,6 +608,11 @@ mod tests { None, "fullwidth punctuation pairs stay as typed" ); + assert_eq!( + delimiter_len("<\u{ff5c}a\u{4e2d}\u{6587}\u{ff5c}>", false), + None, + "a name with CJK content stays as typed" + ); } #[test] diff --git a/crates/runvil-plugin/src/service.rs b/crates/runvil-plugin/src/service.rs index cce744174..d1c45e714 100644 --- a/crates/runvil-plugin/src/service.rs +++ b/crates/runvil-plugin/src/service.rs @@ -244,6 +244,7 @@ mod tests { //! and the map's provide, get, and provides rules. use std::collections::HashSet; + use std::hash::{DefaultHasher, Hash, Hasher}; use std::sync::Arc; use runvil_types::error::{Error, ErrorCode, ErrorFact}; @@ -417,6 +418,17 @@ mod tests { assert_ne!(GREETING.id(), ServiceKey::::new("acme/farewell").id()); let ids: HashSet = [GREETING.id(), GREETING_AS_NUMBER.id()].into(); assert_eq!(ids.len(), 1, "one literal hashes once"); + let hash_of = |id: ServiceId| { + let mut hasher = DefaultHasher::new(); + id.hash(&mut hasher); + hasher.finish() + }; + assert_eq!(hash_of(GREETING.id()), hash_of(GREETING_AS_NUMBER.id())); + assert_ne!( + hash_of(GREETING.id()), + hash_of(ServiceKey::::new("acme/farewell").id()), + "another literal hashes apart" + ); assert_eq!(GREETING.id().to_string(), "acme/greeting"); assert_eq!( format!("{:?}", GREETING.id()), diff --git a/vibe/2026-11-12-1-strengthen-engine-tests.md b/vibe/2026-11-12-1-strengthen-engine-tests.md new file mode 100644 index 000000000..b98cd2afa --- /dev/null +++ b/vibe/2026-11-12-1-strengthen-engine-tests.md @@ -0,0 +1,158 @@ +--- +name: Strengthen Engine tests +overview: Make existing Engine tests fail when the code they run is broken. Mutation testing found 89 changes to production code, outside `runvil-vfs`, that an existing test runs and every test still passes. Checks and input cases added to 45 existing tests now catch 63 of them. No test is added and no production code changes. +todos: + - id: strengthen-tests + content: "Strengthen the existing tests in `runvil-lua`, `runvil-runtime`, `runvil-prompt`, `runvil-types`, `runvil-plugin`, and `runvil-model-client`, in one commit with this plan" + status: completed +isProject: false +--- + +# Strengthen Engine tests + + + +## Product Requirements + +A test that runs a line does not always fail when the line is wrong. Mutation testing finds those cases: it makes one small change to the production code at a time and reports each change that every test still passes. Per-test coverage then names the tests that run the changed line and pass anyway. This plan strengthens those tests, and changes nothing else. + +- Problem and users: + - On 2026-11-11 at `fa9936f64`, `cargo-mutants` 27.1.0 made 4,178 single changes to the Engine's production code, each judged by every Engine test. Outside `runvil-vfs`, 145 passed every test. + - On 2026-11-12 at `2d40bafa2` the 145 were tested again. Two fail a test there. Of the other 143, 89 are on a line that at least one of the 2,638 Engine tests runs, and 54 are on a line no test runs. + - The 89 are tests that pass with the code they run broken: 42 in `runvil-lua`, 23 in `runvil-runtime`, 14 in `runvil-prompt`, 7 in `runvil-types`, 2 in `runvil-plugin`, and 1 in `runvil-model-client`. + - The users are the maintainers and reviewers, who rely on these tests to catch a regression. +- Goals: + - Each of the 89 that alters behavior fails an existing test. +- Non-goals: + - New tests. + - Any change to production code. + - `runvil-vfs`. + - The 54 changes on lines no test runs. + - Mutation testing in CI. +- Success criteria: + - Testing the 89 again on the changed tree leaves only the ones the Decision Record lists. + - The Engine has the same 2,638 tests before and after. +- Constraints: + - Only existing test functions change: a check is added on a value the test already produces, or an input case is added to the test. + - A test's name stays true to what it checks. +- Open questions: None + +## Functional Specification + +No product behavior changes. Each added check states a behavior the code already has, and each failed for at least one of the 89 changes when that change was applied. + +- Acceptance criteria: + + | Crate | Run by a test and passing | Now fail a test | Alter no behavior | No fitting test | + |---|---|---|---|---| + | `runvil-lua` | 42 | 24 | 14 | 4 | + | `runvil-runtime` | 23 | 20 | 3 | 0 | + | `runvil-prompt` | 14 | 13 | 1 | 0 | + | `runvil-types` | 7 | 4 | 1 | 2 | + | `runvil-plugin` | 2 | 1 | 1 | 0 | + | `runvil-model-client` | 1 | 1 | 0 | 0 | + | Total | 89 | 63 | 20 | 6 | + + - The 63 are caught by 45 strengthened tests: 19 in `runvil-lua`, 12 in `runvil-runtime`, 8 in `runvil-prompt`, 4 in `runvil-types`, and 1 each in `runvil-plugin` and `runvil-model-client`. + - The Decision Record names the 20 that alter no behavior and the 6 with no fitting test. + + + + +## Technical Design + +Test code only: 37 files in six crates, 423 lines added and 64 removed. No public item, production line, or dependency changes. + +- File and public API changes: + - `crates/runvil-internal/lua/src`: 18 files, among them the test modules of `coro.rs` and `program.rs`. + - `crates/runvil-internal/runtime/src`: 9 files under `execute/`, and `subst-tests.rs`. + - `crates/runvil-internal/prompt/src`: `tests/section_fences.rs`, `contract/tests.rs`, and the test module of `body_text.rs`. + - `crates/runvil-internal/types/src`: `error-tests.rs`, `names-tests.rs`, and the test modules of `models.rs` and `untrusted_inventory.rs`. + - `crates/runvil-plugin/src/service.rs` and `crates/runvil-internal/model-client/src/client-tests.rs`: one test each. + + + + +## Testing Plan + +The measurement that found the 89 is also the check that they are closed. Each was applied to the tree before and after this change and judged by every Engine test. + +- Regression, security, and performance: + + | Tree | Tested | Fail a test | Pass every test | + |---|---|---|---| + | `2d40bafa2` | 89 | 0 | 89 | + | `2d40bafa2` with this change | 89 | 63 | 26 | + + How the tests were found: every Engine test ran once under coverage with its own profile, which gives the tests that run each line. For each change, the test chosen is one whose subject is the changed code, in that code's own test module where there is one. + +- Exit criteria: + - `cargo nextest run --locked --all-features` over the nine Engine packages: 2,638 tests, the count on `2d40bafa2`. + - `CARGO_BUILD_WARNINGS=deny cargo clippy --locked --all-targets --all-features` over the six changed packages. + - `cargo fmt --all --check` + - `cargo test -p build-xtask` + - `node --test crates/workshop/ui/test/docs-claims.mjs` + - The diff against `2d40bafa2` adds no file, adds or removes no `#[test]`, and touches no line outside test code. + + + + +## Decision Record + +- Decisions: + - Only existing tests change. Rationale: the purpose is to repair tests that pass when the code they run is broken. A change on a line no test runs is missing coverage, which is a different problem with a different fix. + - The test that gets the check is the one whose subject is the changed code, not every test that passes through the line. Rationale: one line is run by a median of 25 tests and by as many as 1,550, nearly all on their way to checking something else. + - A new input case in an existing test counts as strengthening it. Rationale: several tests ran the line but never with an input on which the change makes a difference, such as two `lua` fences with no line between them. + - One change for six crates. Rationale: it is one kind of edit throughout and 37 files. + - No test for the 20 changes that alter no behavior: + - `runvil-lua`, 14: the seven changes to the library flags of the temporary VM in `compiler` (`program.rs:23` and `:24`), which only parses and dumps a chunk; `dispatch.rs:207`, whose guard is true for every error that can reach it; `json/encode.rs:425`, which zero never reaches; `models_loop/machine.rs:384` and `:387`, where the guard is already implied; `projection.rs:86`, where an empty batch accepts and rejects what no batch does; and two of the three changes at `tools/offer.rs:212`, which differ only for a negative length, where every form gives 0. + - `runvil-runtime`, 3: `Admission::clear` (`admission.rs:284`), which runs only as a run ends; the guard at `chain_record.rs:535`, which its one caller has already made true; and the guard at `step.rs:255`, where `finish` does the same things in the same order. + - `runvil-prompt`, 1: `parse.rs:148`, where the extra heading let through is the H1, which the next line filters out. + - `runvil-types`, 1: `seed.rs:106`, `|` as `^` on two values with no bit in common. + - `runvil-plugin`, 1: `registry.rs:274`, `Vec::new()` as `vec![]`. + - No check for the 6 changes with no fitting test. Rationale: the tests that reach these lines are about something else, so a check there would make the test's name untrue. + - `runvil-lua`, 4: `route_store_to_shims` (`engine_globals_store.rs:391`) and `call_optional_line` (`protocol/parse/store.rs:17`), which only other crates' tests reach; `clear_jump` (`vm/run.rs:36`) and the guard at `vm/run.rs:282`, which differ only when a lock is poisoned. + - `runvil-types`, 2: `attribute_len` (`untrusted_inventory.rs:419`), which the tests reach without ever passing an attribute opener; and `Debug for ToolCatalog` (`tools/registry.rs:32`), reached only by a `runvil-runtime` test of another type's `Debug`. +- Rejected alternatives: + - New tests for the 54 changes on lines no test runs and for the 6 with no fitting test. Reason: they are missing coverage, not weak tests. Revisit: as separate work on coverage. + - A check in every test that runs a changed line. Reason: it would put unrelated assertions in hundreds of tests. Revisit: never. +- Assumptions, risks, and notes: + - The measurement makes simple changes only: a swapped operator, a deleted `match` arm or negation, a function body replaced by a default value. A clean result does not show that the tests catch a subtler fault. + - The list of changes comes from `fa9936f64`. Code added between it and `2d40bafa2` was measured only in `runvil-prompt`, and `runvil-markdown`, which is newer than the list, was not measured. + - The measurement ran on Linux only. + - One of the 89, the guard at `execute/scheduler/effects.rs:290` in `runvil-runtime`, failed a test on unchanged `2d40bafa2` when the machine was heavily loaded and passed every test when it was not. With this change it fails either way. + +### Deferred and Out of Scope + +- Deferred: `runvil-vfs`, which held 56 such changes at `fa9936f64`, until the maintainers say whether more tests there are wanted. +- Deferred: the 54 changes on lines no test runs, and the 6 with no fitting test, as coverage work. +- Deferred: `runvil-markdown`, until it is measured. +- Out of scope: mutation testing in CI. + + + + +## Project Survey + +None + + + + +## Execution Instructions + + + +### Step 1: Strengthen the tests [completed] + +- Component: `strengthen-tests` +- Tests: + - The exit criteria in the Testing Plan + +Build: for each of the 89 changes, add a check or an input case to the existing test whose subject is the changed code. Leave the 26 the Decision Record lists. Test the 89 again and record both rows in the Testing Plan. + +Commit: one commit. Suggested subject: `Make existing Engine tests fail when the code they run is broken`. + + + +