Repository navigation
Add library Plugin loading MVP - #163
Merged
Merged
Conversation
Remove the section opt-in that offered the model four task tools copied from the Lua task functions, because a model needs a different shape than futures that can outlive their section. The scheduler loses every path that served only those tools: the built-in tool route, parking a model call until one of its tasks ends, and the queue of task-end notices pushed into the next model round. The four tool names are no longer reserved, so a catalog tool can take one. The Lua task functions keep their behavior, and the task origin type stays. - `crates/runvil-internal/engine/src/execute/scheduler/builtins.rs` is deleted with `builtins-schemas.rs`, `builtins-tests.rs`, `await_tasks.rs`, and `notices.rs`. `ToolRoute` loses `Builtin`, `DispatchTarget` loses `Builtin`, and `ToolCallDispatch` keeps only `Issued` and `Answered`. - `Chain` drops `awaiting` and `task_notices`, so `wake_waiter` always delivers the member as a `JoinAny` answer and now returns nothing. - `cancel_task` returns `Result<()>` and becomes private to `waits.rs`, and `prepare_timer` becomes private to `timer.rs`. - `task_lifecycle` and `terminals_per_started_task` move unchanged into `fixtures.rs` for the fanout and run-termination suites. - `offer_refusal` no longer refuses the names `task`, `task_cancel`, `task_status`, and `await_tasks`; `tool_route` sends them to a local tool or the catalog like any other name. - `prepare_chat` no longer pushes notices onto the list, so an empty list is refused before any round, as `an_empty_list_raises_before_any_round` pins. - `dispatch_cancel` no longer queues a notice when the author cancels a task. - `drop_timer` wakes its waiter with the interrupted `JoinAny` error in every case. - `a_section_tools_table_holds_only_its_api_and_its_round_advertises_only_the_offered_tool` pins the exact keys of a section's `tools` table and a `models.loop` round that advertises only `tools_echo`. - `unofferable_tool` becomes `hidden/shape` with a scalar schema, since a wire name no longer makes a tool unofferable. - `abandon_owned_tasks` still leaves `TaskOrigin::Model` tasks out of the leaked ids. Design: removes surface-growth @ crates/runvil-internal/lua/src/tools.rs::install_allow_tasks boundary: pub Design: removes value-object @ crates/runvil-internal/lua/src/scope.rs::TaskAllowlist Design: removes surface-growth @ crates/runvil-internal/engine/src/execute/scheduler/builtins-schemas.rs::advertise_task_builtins deps: BTreeMap,TaskAllowlist,Vec boundary: wire Design: removes dispatch-on-tag @ crates/runvil-internal/engine/src/execute/scheduler/builtins.rs::answer_task_builtin Design: replaces pure-function @ crates/runvil-internal/engine/src/execute/tests/fixtures.rs::task_lifecycle deps: Observation,String was: crates/runvil-internal/engine/src/execute/tests/model_task_acceptance.rs::task_lifecycle Deferred: An Engine test that calls `tools.allow_tasks()` and pins Lua's nil-field call error. Plan: vibe/2026-10-20-1-remove-allow-tasks.md
The model can no longer start tasks, so every task is the author's and the record of who started a task goes away. Task start events, task status, and spawn requests stop carrying an origin, and every task left running when its owner ends now counts as leaked. The pending-tasks call loses its filter: any argument is the call's error naming its type, and the protocol parser owns that check rather than the Lua wrapper. The task notice event and its emitter are removed because nothing produces them any more. Logs written before this change still load, since the extra origin key on a task start entry is ignored.
- `TaskOrigin` and `ParseTaskOriginError` are deleted from the types crate and the facade's `ids` module; `public-api.txt` drops them along with `Event::TaskStarted::origin` and every `Event::TaskNotice` field.
- `Request::Pending` becomes a unit variant. `tasks_pending` passes its argument through as `argument`, and `parse_pending` turns any non-nil value into the call's error, so the Lua shim holds no check of its own.
- `prepare_spawn` and `dispatch_spawn` lose their `origin` parameter and with it their `clippy::too_many_arguments` expectations; `live_tasks_of` and `dispatch_pending` lose their origin filter.
- `on_task_notice` and `TaskNoticeReport` leave the recording test support together with the event they forwarded.
- `abandon_owned_tasks` counts every abandoned live task as leaked, so any task left live turns its owner's success into `tasks_live`. The `Error::TasksLive` message now reads "chain ended with tasks still live", without "author".
- `parse_pending` refuses an argument with exactly `tasks.pending takes no arguments, got <type>`; tests cover `{}`, `{ origin = 'author' }`, and a string at the parser, and both tables through `pcall` in a run.
- `task_status_table` no longer sets an `origin` key, and `status_reports_a_parked_task_and_then_a_finished_one` pins the full key list of both status tables.
- `shim_origin` is gone, so `parse_spawn` no longer reads or requires an `origin` key on a spawn yield.
- `a_task_started_line_carrying_an_origin_still_reads_back` pins that an older `task_started` line carrying `"origin":"author"` still deserializes.
Design: removes value-object @ crates/runvil-internal/types/src/ids.rs::TaskOrigin boundary: pub
Design: removes value-object @ crates/runvil-internal/types/src/ids.rs::ParseTaskOriginError boundary: pub
Plan: vibe/2026-10-20-1-remove-allow-tasks.md
close cppalliance#162 Plan: vibe/2026-10-20-1-remove-allow-tasks.md
Removing the model task tests left no run-level test of why a running task is abandoned when its owner ends, so three chain-end tests and a phrase test now pin each reason. The pending shim now refuses an argument in any position, and helpers that only the removed notice code needed are narrowed.
- `run_abandoning_a_running_task` holds a child on a model round while its owner returns, fails, or exhausts its tool loop, and asserts the child's observations are exactly `TaskStarted` then `TaskAbandoned` with `OwnerReturned`, `OwnerFailed`, or `ToolLoopExhausted`.
- `every_abandon_reason_renders_its_own_phrase` pins the `why()` text of all five `AbandonReason` variants.
- `tasks_pending` scans every argument and passes the first non-nil one to the parser, so `tasks.pending(nil, {})` raises the same error as `tasks.pending({})`.
- `MessageList::push` is `pub(crate)`, and `live_tasks_of` and `task_status` are private to `waits.rs`.
Plan: vibe/2026-10-20-1-remove-allow-tasks.md
Only a local Lua tool's answer is now settled on the driver thread; a preparation failure returns Err and is answered by the caller's error arm. Plan: vibe/2026-10-20-1-remove-allow-tasks.md
The tasks.pending shim raises on any non-nil argument itself, like its sibling shims, so the pending request carries no fields and the parse reads none. complete_task stores the outcome through the slot it already holds. The emitter doc drops the removed task built-in, and the doc comments the removal rewrote are rewrapped.
Each run now builds its tool catalog once, when it takes its snapshot of the available Packs, and keeps that catalog for the whole run. Before, every Pack held its own tool list and the catalog was rebuilt from those lists each time a caller asked for it. Now the list a run offers and the list its tool calls are routed through are the same shared list. The order of tools in the catalog and the way a call finds its Pack stay the same. - `HostRunContext` stores one `ToolCatalog` built in `snapshot` and hands out clones. A run holds one tool list instead of one per Pack. - `PackState` replaces `RunPack`. Its `Usable` variant no longer carries a tool list, so each tool is held once, in the catalog. - `offering` finds the tool in the catalog, then its Pack by the tool id's first segment. It answers only for a usable Pack and relies on validation keeping each tool under its own Pack id. - `usable_tools` holds each usable Pack's validated tools until every Pack is collected. The catalog is then built in Pack id order, the order the old per-call build used, because the input arrives in pick order. - `ToolCatalog::new` still falls back to an empty catalog, with a logged warning, if the build fails. The fallback moves from `catalog` into `snapshot`, and a comment there says per-tool validation rules that failure out. - `fmt::Debug` output for `HostRunContext` now lists the catalog's tool ids. - `snapshot` gains no test in this diff for catalog order, the empty-catalog fallback, or the `offering` lookup. The commit changes no test file. Design: extends encapsulated-invariant @ crates/harness-internal/runner/src/host-run.rs::HostRunContext Plan: vibe/2026-10-20-1-library-plugin-loading.md
A Plugin puts its Packs into the Host's Pack registry with a method that is now called insert instead of add. The three Plugin crates, the Harness tests, and the docs and test messages around them use the new name and the matching wording. The registry's own doc now says the Host owns it and a Plugin only inserts its Packs into it. Behavior and error cases do not change. - `PackRegistry::insert` takes the same arguments, returns the same `InstallError` cases, and keeps its body. The old name is removed with no forwarding method. - `PackRegistry` - Its doc, and a new sentence in the crate doc in `crates/runvil-plugin/src/lib.rs`, say the Host owns the registry and a Plugin only inserts its Packs into it. The registry doc also says `HostContext::new` reads it through `into_entries` and calls each Pack's `construct` once at startup. - `crates/workshop/server/src/agents.rs` - The three `tracing::warn!` messages change from "added no Pack" to "inserted no Pack". Anyone who matches on the old log text needs the new wording. - `InstallError` - Its variants, fields, and `#[error]` messages stay as they were. Only the variant docs move to insert wording. - `add_refuses_an_fqn_that_is_not_a_vendor_name_pair` - The registry test cases keep their `add_` names and their coverage. Only the calls and the failure messages change. Plan: vibe/2026-10-20-1-library-plugin-loading.md
A Plugin, whether it comes from a dynamic library or is linked into a Host, now has one C entry point shape to follow. The new module declares the status codes, the two verbs, the version numbers, and the string, info, host, and call structs. A linked Plugin crate that depends only on this crate can name them. Tests pin each struct's size and field offsets on 64-bit targets, along with every number no version may renumber. The crate doc now describes the entry point and says these Plugins insert no Packs yet. - `PluginEntry` takes one `RvCall` that names a verb, so every operation reaches a Plugin through a single function and the Plugin dispatches on the verb. - `RvVerb` is a `u32` and not a C enum, so its size never depends on the compiler. - `RvHost` and `RvCall` each have six public fields and no constructor, and both start with a `size` field. - `pub mod abi;` puts the contract on the crate's public surface, so a linked Plugin crate that depends on `runvil-plugin` alone can name the types. The module doc says version 0.1 makes no compatibility promise. - `RV_OK` is 0, `RV_ERR_MISUSE` is 4, `RV_ERR_REFUSED` is 5, and `RV_ERR_UNSUPPORTED` is 6. Codes 1 to 3 are reserved for the Lua API's own errors. - `describe_plugin` runs exactly once during `RV_PLUGIN_LOAD`, and `refuse` runs at most once, according to the field docs. This commit enforces neither rule. - `crates/runvil-plugin/src/abi-tests.rs` pins the size and field offsets of all four structs on 64-bit targets, and the value of every constant. - `RV_PLUGIN_LOAD` and `RV_PLUGIN_UNLOAD` are the only verbs with constants. The module doc reserves verbs 3 to 7 and declares none of them. - `PluginEntry` has no caller in the touched files. The crate doc already says the Harness loads these entry points. Design: new surface-growth @ crates/runvil-plugin/src/abi.rs boundary: pub Design: new bag-of-state @ crates/runvil-plugin/src/abi.rs::RvHost boundary: pub Design: new bag-of-state @ crates/runvil-plugin/src/abi.rs::RvCall boundary: pub Design: new temporal-coupling @ crates/runvil-plugin/src/abi.rs::RvCall boundary: pub Design: new dispatch-on-tag @ crates/runvil-plugin/src/abi.rs::PluginEntry boundary: pub Deferred: constants for the reserved verbs 3 to 7 Plan: vibe/2026-10-20-1-library-plugin-loading.md
Adds a loader that takes a Plugin linked into the Host, sends it a load call, and returns a shared handle to it. The Host gives every Plugin a small table of callbacks so it can name itself or decline to load. Those callbacks accept only the load call in progress and reject malformed names, versions, and reasons. Dropping the last handle sends one unload call, and a load that fails is reported with a readable reason and never unloaded. The unsafe allowlist now covers the new crate. - `Instance` sends one unload when it drops. It claims `Send` and `Sync` by `unsafe impl` because it has no method that takes `&self`. - `SESSION` holds the address of the load call running on this thread. The `RvHost` functions compare the pointer they receive against it and never dereference it. - `OWNED` gains the new crate, so the unsafe allowlist covers five crates. - `Field` checks a string's length against its cap before it reads any byte. It reads the pointer only when the length is nonzero, with caps of 64 bytes for names and versions and 1024 for refusal reasons. - `send_load` reports a refusal first, then misuse of the table, then a non-OK status, then a missing description. A load inside a load restores the outer session. - `send_unload` opens no session, so the `RvHost` functions answer misuse during an unload. It ignores the unload's status. - `insert_plugin` takes the Pack registry and leaves it untouched, and a test pins that. - `PluginSource` declares a `Library` variant that nothing in this commit constructs. Design: new temporal-coupling @ crates/harness-internal/plugin-loader/src/host.rs::describe_plugin deps: RvCall,RvPluginInfo boundary: pub Design: new swallowed-exception @ crates/harness-internal/plugin-loader/src/host.rs::send_unload deps: PluginEntry,c_void Design: new hidden-dependency @ crates/harness-internal/plugin-loader/build.rs Design: new facade @ crates/harness-internal/plugin-loader/src/lib.rs boundary: pub Deferred: Plugins insert no Packs until insert_pack exists Plan: vibe/2026-10-20-1-library-plugin-loading.md
This adds a small dynamic library that acts as a real Plugin, so the loader has something it can open in end-to-end tests. The library depends on nothing from Runvil and carries its own copy of the C types, which shows that a library Plugin needs no Runvil code. On load it names itself test-plugin at version 0.1.0 and creates an instance, and on unload it frees that instance. The new crate joins the workspace, the unsafe code allowlist, and the checks that count the harness crates. - `crates/harness-internal/test-plugin/Cargo.toml` builds the crate as a dynamic library and as an rlib, and declares no dependencies. The dynamic library gives the loader a real Plugin to open, and the rlib lets the crate's own tests link the entry point. - `RvCall` and its three sibling types are private copies of the contract's C types. Unit tests pin each type's size and field offsets. - `runvil_plugin` is the only public item. It matches on the verb, sends load and unload to private handlers, and answers any other verb with `RV_ERR_UNSUPPORTED`. - `build.rs` includes the file-size check's source through a path attribute instead of depending on its crate. Harness crates may name no build crate, and the check still runs for this one. - `OWNED` gains the new crate, so the unsafe allowlist covers six crates. The harness bans test now expects the new crate in its covered set. - `load` checks the host table's size and major version before it allocates anything, and a short or foreign table gets `RV_ERR_UNSUPPORTED` with no instance set. It then calls `describe_plugin` once and sets the instance pointer only when that returns `RV_OK`, and a failing describe drops the instance and returns its status. - `unload` frees the instance behind the plugin pointer. A NULL pointer returns `RV_ERR_MISUSE`. - `LIVE_INSTANCES` counts live instances under test only. Tests use it to show that a failing load and an unload each free the instance. - `refuse` is never called by the Plugin, so `RV_ERR_REFUSED` is only pinned and returned in tests. - `lib-tests.rs` calls the export directly with hand-built tables and never opens the built library. Design: new parallel-abstraction @ crates/harness-internal/test-plugin/src/lib.rs boundary: wire Design: new dispatch-on-tag @ crates/harness-internal/test-plugin/src/lib.rs::runvil_plugin deps: RvCall boundary: pub Design: new surface-growth @ crates/harness-internal/test-plugin/src/lib.rs::runvil_plugin deps: RvCall boundary: pub Design: new hidden-dependency @ crates/harness-internal/test-plugin/build.rs Design: new shotgun-surgery @ crates/build-xtask/src/unsafe_allowlist.rs::OWNED Plan: vibe/2026-10-20-1-library-plugin-loading.md
Plugins can now come from dynamic libraries as well as from code linked into a Host. A Host points the Harness at a directory, and it opens each library there in file-name order, loads the Plugin inside, and reports what loaded and what failed, so one bad file never stops the rest. A library stays open until its Plugin has been told to unload, and a failed load closes its library at once. The public facade now exposes the insert functions and the report and Plugin types. Plugins still insert no Packs, and end-to-end tests load a real built library. - `Loaded` declares its instance before its library, so Rust's drop order sends the unload call before the library closes, with no hand-written `Drop`. A Windows-only test pins that a library stays open while any `Arc` of its Plugin lives. - `load` is the one unsafe function that sends the load call and builds the Plugin, for library and linked entry points alike. Its callers pass the library or none, and a failing load drops that library before returning. - `crates/harness/src/lib.rs` gains a `plugin` module that re-exports the two insert functions and the four Plugin types, one item per line. - `plugin_files` returns the top-level regular files with the platform's library extension, sorted by file name, as absolute paths. It follows symbolic links and skips a link that leads nowhere. - `is_regular_file` treats a missing entry as not a file and returns every other metadata error, so a permission error surfaces instead of an entry vanishing from the listing. - `open_library` opens with the Windows flags that resolve a library's own dependencies only from its directory and System32. Elsewhere it opens the library plainly. - `open_error` appends the underlying cause's text to the message, because on Windows `libloading` alone reports only a bare failure. - `insert_plugin_dir` records each file that fails to open, lacks the entry point, fails to load, or repeats a Plugin name as a failure and keeps going; a repeat is loaded, then unloaded and dropped. It is a safe function that runs foreign code, so its doc tells a Host to point it only at trusted libraries. - `insert_plugin_dir` takes the Pack registry but inserts nothing into it yet. The parameter is unused behind an expect attribute. - `crates/harness-internal/test-plugin/tests/it/insert.rs` has no case where a library's load call fails; its failures are a garbage file and a repeated name. Design: new pure-function @ crates/harness-internal/plugin-loader/src/load.rs::is_regular_file deps: Result Design: new pure-function @ crates/harness-internal/plugin-loader/src/load.rs::open_error deps: Error Design: extends constructor-injection @ crates/harness-internal/plugin-loader/src/load.rs::Loaded Design: new facade @ crates/harness/src/lib.rs::plugin boundary: pub Design: new surface-growth @ crates/harness/src/lib.rs::plugin boundary: pub Deferred: insert_plugin_dir inserts no Packs into the registry until insert_pack exists Plan: vibe/2026-10-20-1-library-plugin-loading.md
Plan: vibe/2026-10-20-1-library-plugin-loading.md
Listing the plugins in a directory now compares each entry's extension with the platform library extension before it reads any metadata. One unreadable entry that was not a library used to fail the whole listing, so no plugin in that directory loaded. Such an entry is now judged by its name alone. A library candidate whose metadata cannot be read still returns its error. The metadata read is now a parameter, so a test can make one chosen entry fail on any platform. - `list_plugin_files` holds the former body of `plugin_files` and takes the metadata read as a parameter. The production caller and the tests each pass a different implementation. - `plugin_files` is a one-line call that passes a closure over `std::fs::metadata`, because the function item is not `Fn(&Path)` for every lifetime. - `DLL_EXTENSION` is compared before any metadata call, so an entry with another extension is skipped by name and cannot fail the listing. - `is_regular_file` still decides every library candidate, so a candidate whose metadata fails with anything but not-found still returns that error. - `plugin_files_returns_the_metadata_error_of_an_entry_with_the_library_extension` pins that a library candidate with unreadable metadata still returns its permission error. - `plugin_files` and `insert_plugin_dir` now document that only an entry with the platform's library extension has its metadata read. - `entry?` is unchanged, so an error from reading the directory's own entries still fails the whole listing. Design: new strategy @ crates/harness-internal/plugin-loader/src/load.rs::list_plugin_files deps: Fn,Path Repairs: non-library entries are never examined beyond their names @ crates/harness-internal/plugin-loader/src/load.rs::list_plugin_files - an unreadable non-library entry made the whole listing return an error Plan: vibe/2026-10-20-2-plugin-loading-debt-removal.md
The C plugin contract now states which thread each call runs on, what unload must finish before it returns, and which failed loads get no unload. A safety comment in the loader cited the thread rule, and the contract now holds that rule in writing. The change edits documentation and one comment. It changes no code. - `unsafe impl Send for Instance` now has a SAFETY comment that points at the `# Threads and lifetime` section of `runvil_plugin::abi`. The comment keeps its second reason, that the loader never reads `plugin`. - `RV_PLUGIN_LOAD` runs on the thread of the insert call. Each `RvHost` function answers only the `RvCall` it was handed, only while that load call runs, and only on its thread; any other use, including during `RV_PLUGIN_UNLOAD`, returns `RV_ERR_MISUSE`. - `RV_PLUGIN_UNLOAD` may come on any thread, so an instance must be freeable from a thread other than the loading one. It must stop every thread the Plugin started and release everything before it returns, because a library Plugin's library closes right after. - `RV_OK` does not earn an unload when the loader rejects the load - a refusal, a recorded misuse, a non-OK status, or no description. The Plugin frees whatever it made before returning on each such path, and a load that succeeds is unloaded later even if the loader drops the Plugin. - `refuse` and `describe_plugin` now document that each call must come on the thread running the load call. - `crates/runvil-plugin/src/abi.rs` gains doc text only, with no type, signature, verb number, or layout change. No test pins the new text. Plan: vibe/2026-10-20-2-plugin-loading-debt-removal.md
Plan: vibe/2026-10-20-2-plugin-loading-debt-removal.md
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
PackStatereplacesRunPack) and renamePackRegistry::addtoinsert.runvil-pluginabi) plus a Harness loader for linked and directory-based library Plugins.test-plugincdylib and exposeinsert_plugin/insert_plugin_diron the harness facade. Pack insertion through the entry point is still deferred.Test plan
cargo fmt --check/ workspace clippy / nextest as in the plan verify stepinsert_plugin_dirand reportstest-plugin 0.1.0insert_plugin