From cf2ff38d86d925f9c948bd6a14e965e00a111b21 Mon Sep 17 00:00:00 2001 From: Vinnie Falco Date: Thu, 5 Nov 2026 21:18:53 -0800 Subject: [PATCH 1/6] Remove `ParsePathReasonError` and the `PathReason` parse `PathReason` no longer parses a tag back from text, so `VfsError` is the only error type in `runvil-vfs`. `error-reason.rs` loses the `FromStr` implementation, `ParsePathReasonError`, `from_tag`, and `ALL`, and the `runvil::vfs` facade loses the re-export. `PathReason::tag` does not change, so a store error value still holds the tag in its `rule` field. - `AGENTS.md` now names `VfsError` as the only exception to the rule that Engine and Harness crates report failures as the universal `Error`. - `every_path_reason_has_its_own_stable_tag` replaces the two parse tests. It pins each tag string and checks that no two reasons share a tag, which the round-trip test checked before. --- AGENTS.md | 2 +- .../lua/src/error-value-store.rs | 3 +- .../runvil-internal/vfs/src/error-reason.rs | 83 +------------------ crates/runvil-internal/vfs/src/error-tests.rs | 37 ++++----- crates/runvil-internal/vfs/src/error.rs | 2 +- crates/runvil-internal/vfs/src/lib.rs | 2 +- crates/runvil/public-api.txt | 9 -- crates/runvil/src/lib.rs | 1 - 8 files changed, 23 insertions(+), 116 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index f053bde87..0a0eb83fe 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -48,7 +48,7 @@ Seven terms have exactly one meaning each, everywhere in this repository: code c - A Cargo feature gates a real constraint such as a toolchain requirement or heavy native build. It does not describe product shape. Feature-disabled builds must not leak optional types into core paths. - Runtime and serve paths never compile native dependencies or invoke build tools. - Unsafe code stays in its explicitly owned boundary. -- Engine and Harness crates report every failure as the universal `Error` in `runvil-types`, which the `runvil` facade re-exports. A new kind of failure is a new `ErrorCode` variant in its hundreds family, with a number never used before, plus a `FactKey` variant only when no existing key names the value. They add no other error enum or struct, no `thiserror` derive, and no other `impl std::error::Error`; a foreign cause joins the message through `Error::with_cause`. `VfsError` and `ParsePathReasonError` in `runvil-vfs` are the only exceptions, and test code may define a throwaway error type to exercise `with_cause`. A crate-private marker returned as `Err` and turned into `Error` by its caller, or a type that pairs an `Error` with a value the caller must still release, such as `InjectFailure`, is not another error type. +- Engine and Harness crates report every failure as the universal `Error` in `runvil-types`, which the `runvil` facade re-exports. A new kind of failure is a new `ErrorCode` variant in its hundreds family, with a number never used before, plus a `FactKey` variant only when no existing key names the value. They add no other error enum or struct, no `thiserror` derive, and no other `impl std::error::Error`; a foreign cause joins the message through `Error::with_cause`. `VfsError` in `runvil-vfs` is the only exception, and test code may define a throwaway error type to exercise `with_cause`. A crate-private marker returned as `Err` and turned into `Error` by its caller, or a type that pairs an `Error` with a value the caller must still release, such as `InjectFailure`, is not another error type. - Comments explain a non-obvious constraint, ordering requirement, or workaround. Every platform or external-bug workaround cites its upstream issue URL in the explanatory comment. - JSON that reaches a recorder or a replay comparison round-trips exactly - `to_value`, `to_string`, `from_str` yield an identical value, object keys stay canonical (sorted), numbers must be finite. Exact parsing (`float_roundtrip`) carries that guarantee; a value derived and then logged is additionally rounded to its meaningful precision at the source. diff --git a/crates/runvil-internal/lua/src/error-value-store.rs b/crates/runvil-internal/lua/src/error-value-store.rs index f1876d040..91c881593 100644 --- a/crates/runvil-internal/lua/src/error-value-store.rs +++ b/crates/runvil-internal/lua/src/error-value-store.rs @@ -92,8 +92,7 @@ pub fn store_error_value_fields(error: &runvil_vfs::VfsError) -> Vec<(String, Er /// it, first, then the variant's own fields: the paths the failure names, /// plus `anchor` and its integer `count` for an anchor error, `rule` for an /// invalid path, or the integer `valid_up_to` for text that is not UTF-8. -/// The rule is the `PathReason` tag, which `PathReason::from_tag` parses -/// back. +/// The rule is the tag `PathReason::tag` returns. /// /// Both store paths, the direct closures a Pack prelude's install calls /// and the executor's effect path, build their failure here, so a store diff --git a/crates/runvil-internal/vfs/src/error-reason.rs b/crates/runvil-internal/vfs/src/error-reason.rs index 4e24b3bcf..47005cfb7 100644 --- a/crates/runvil-internal/vfs/src/error-reason.rs +++ b/crates/runvil-internal/vfs/src/error-reason.rs @@ -1,8 +1,7 @@ //! [`PathReason`], why a path or glob pattern was refused before any -//! backend saw it, and the error its tag parse returns. +//! backend saw it. use std::fmt; -use std::str::FromStr; /// Why a path or glob pattern was rejected before any backend saw it. /// @@ -46,26 +45,9 @@ pub enum PathReason { } impl PathReason { - /// Every reason, in declaration order. - pub(crate) const ALL: &'static [PathReason] = &[ - PathReason::Empty, - PathReason::Absolute, - PathReason::Traversal, - PathReason::Control, - PathReason::EmptySegment, - PathReason::Backslash, - PathReason::ReservedName, - PathReason::UnsafeSuffix, - PathReason::TooLong, - PathReason::Wildcard, - PathReason::IntoDescendant, - ]; - /// Returns the short tag for this reason, such as `empty` or `too_long`. /// /// A store error value holds this tag in its `rule` field. - /// [`PathReason::from_tag`] parses the same word back, so the two - /// round-trip. #[must_use] pub fn tag(self) -> &'static str { match self { @@ -82,27 +64,6 @@ impl PathReason { PathReason::IntoDescendant => "into_descendant", } } - - /// Parses a tag from [`PathReason::tag`] back into its reason. - /// - /// Returns `None` for any other string. - #[must_use] - pub fn from_tag(tag: &str) -> Option { - match tag { - "empty" => Some(PathReason::Empty), - "absolute" => Some(PathReason::Absolute), - "traversal" => Some(PathReason::Traversal), - "control" => Some(PathReason::Control), - "empty_segment" => Some(PathReason::EmptySegment), - "backslash" => Some(PathReason::Backslash), - "reserved_name" => Some(PathReason::ReservedName), - "unsafe_suffix" => Some(PathReason::UnsafeSuffix), - "too_long" => Some(PathReason::TooLong), - "wildcard" => Some(PathReason::Wildcard), - "into_descendant" => Some(PathReason::IntoDescendant), - _ => None, - } - } } impl fmt::Display for PathReason { @@ -125,45 +86,3 @@ impl fmt::Display for PathReason { formatter.write_str(text) } } - -impl FromStr for PathReason { - type Err = ParsePathReasonError; - - /// Parses a reason from its exact tag, as [`PathReason::from_tag`] - /// does. - fn from_str(tag: &str) -> Result { - PathReason::from_tag(tag).ok_or_else(|| ParsePathReasonError { - input: tag.to_owned(), - }) - } -} - -/// The error returned when text is not a [`PathReason`] tag. -#[derive(Debug, Clone, PartialEq, Eq)] -pub struct ParsePathReasonError { - /// The rejected text. - input: String, -} - -impl ParsePathReasonError { - /// Returns the rejected text. - #[must_use] - pub fn input(&self) -> &str { - &self.input - } -} - -impl fmt::Display for ParsePathReasonError { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "invalid path reason `{}`: required one of ", self.input)?; - for (position, reason) in PathReason::ALL.iter().enumerate() { - if position > 0 { - f.write_str(", ")?; - } - write!(f, "`{}`", reason.tag())?; - } - Ok(()) - } -} - -impl std::error::Error for ParsePathReasonError {} diff --git a/crates/runvil-internal/vfs/src/error-tests.rs b/crates/runvil-internal/vfs/src/error-tests.rs index e37f4b573..667e418bc 100644 --- a/crates/runvil-internal/vfs/src/error-tests.rs +++ b/crates/runvil-internal/vfs/src/error-tests.rs @@ -203,24 +203,23 @@ fn every_variant_has_its_own_stable_tag() { } #[test] -fn a_path_reason_parses_from_its_own_tag() { - for reason in PathReason::ALL { - assert_eq!(reason.tag().parse::(), Ok(*reason)); - } -} - -#[test] -fn a_path_reason_parse_refuses_other_text_and_lists_every_tag() { - let Err(error) = "Empty".parse::() else { - panic!("tags parse case-sensitively"); - }; - assert_eq!(error.input(), "Empty"); - let text = error.to_string(); - assert!( - text.starts_with("invalid path reason `Empty`: required one of "), - "{text}" - ); - for reason in PathReason::ALL { - assert!(text.contains(&format!("`{}`", reason.tag())), "{text}"); +fn every_path_reason_has_its_own_stable_tag() { + let reasons = [ + (PathReason::Empty, "empty"), + (PathReason::Absolute, "absolute"), + (PathReason::Traversal, "traversal"), + (PathReason::Control, "control"), + (PathReason::EmptySegment, "empty_segment"), + (PathReason::Backslash, "backslash"), + (PathReason::ReservedName, "reserved_name"), + (PathReason::UnsafeSuffix, "unsafe_suffix"), + (PathReason::TooLong, "too_long"), + (PathReason::Wildcard, "wildcard"), + (PathReason::IntoDescendant, "into_descendant"), + ]; + for (reason, tag) in reasons { + assert_eq!(reason.tag(), tag, "{reason:?}"); } + let distinct: BTreeSet<&str> = reasons.iter().map(|(reason, _)| reason.tag()).collect(); + assert_eq!(distinct.len(), reasons.len(), "no two reasons share a tag"); } diff --git a/crates/runvil-internal/vfs/src/error.rs b/crates/runvil-internal/vfs/src/error.rs index 556586062..f8eb6f16c 100644 --- a/crates/runvil-internal/vfs/src/error.rs +++ b/crates/runvil-internal/vfs/src/error.rs @@ -7,7 +7,7 @@ use std::fmt; #[path = "error-reason.rs"] mod reason; -pub use reason::{ParsePathReasonError, PathReason}; +pub use reason::PathReason; /// The single error type returned by every virtual filesystem operation. /// diff --git a/crates/runvil-internal/vfs/src/lib.rs b/crates/runvil-internal/vfs/src/lib.rs index 933d7f9e7..b7dcbc79b 100644 --- a/crates/runvil-internal/vfs/src/lib.rs +++ b/crates/runvil-internal/vfs/src/lib.rs @@ -35,7 +35,7 @@ mod traits; use std::sync::{Arc, Mutex, MutexGuard, PoisonError}; -pub use error::{ParsePathReasonError, PathReason, VfsError}; +pub use error::{PathReason, VfsError}; pub use handle::{Access, VfsRef}; pub use memory::MemoryBackend; pub use observe::{OpEvent, OpSink, Origin}; diff --git a/crates/runvil/public-api.txt b/crates/runvil/public-api.txt index 75a24e795..4280066ce 100644 --- a/crates/runvil/public-api.txt +++ b/crates/runvil/public-api.txt @@ -131,7 +131,6 @@ auto runvil::vfs::ModePolicy: Freeze, RefUnwindSafe, Send, Sync, Unpin, UnwindSa auto runvil::vfs::Op: Freeze, RefUnwindSafe, Send, Sync, Unpin, UnwindSafe auto runvil::vfs::OpEvent: Freeze, RefUnwindSafe, Send, Sync, Unpin, UnwindSafe auto runvil::vfs::Origin: Freeze, RefUnwindSafe, Send, Sync, Unpin, UnwindSafe -auto runvil::vfs::ParsePathReasonError: Freeze, RefUnwindSafe, Send, Sync, Unpin, UnwindSafe auto runvil::vfs::PathReason: Freeze, RefUnwindSafe, Send, Sync, Unpin, UnwindSafe auto runvil::vfs::RealBackend: Freeze, RefUnwindSafe, Send, Sync, Unpin, UnwindSafe auto runvil::vfs::Stat: Freeze, RefUnwindSafe, Send, Sync, Unpin, UnwindSafe @@ -225,7 +224,6 @@ derives runvil::vfs::ModePolicy: Debug derives runvil::vfs::Op: Clone, Copy, Debug, Eq, Hash, PartialEq derives runvil::vfs::OpEvent: Debug derives runvil::vfs::Origin: Clone, Debug, Eq, Hash, PartialEq -derives runvil::vfs::ParsePathReasonError: Clone, Debug, Eq, PartialEq derives runvil::vfs::PathReason: Clone, Copy, Debug, Eq, PartialEq derives runvil::vfs::RealBackend: Clone, Debug derives runvil::vfs::Stat: Clone, Debug, Eq, Hash, PartialEq @@ -241,7 +239,6 @@ derives runvil::vfs::Writability: Clone, Copy, Debug, Eq, Hash, PartialEq impl core::convert::From for runvil::ids::TaskId impl core::convert::From for runvil::vfs::VfsPathBuf impl core::error::Error for runvil::error::Error -impl core::error::Error for runvil::vfs::ParsePathReasonError impl core::error::Error for runvil::vfs::VfsError impl core::fmt::Display for runvil::RunSeed impl core::fmt::Display for runvil::effect::EffectId @@ -254,7 +251,6 @@ impl core::fmt::Display for runvil::packs::PackId impl core::fmt::Display for runvil::prompt::ArgType impl core::fmt::Display for runvil::timestamp::Timestamp impl core::fmt::Display for runvil::tools::ToolId -impl core::fmt::Display for runvil::vfs::ParsePathReasonError impl core::fmt::Display for runvil::vfs::PathReason impl core::fmt::Display for runvil::vfs::VfsError impl core::fmt::Display for runvil::vfs::VfsPath @@ -275,8 +271,6 @@ impl core::str::traits::FromStr for runvil::packs::PackId impl core::str::traits::FromStr for runvil::packs::PackId { type Err = runvil::error::Error } impl core::str::traits::FromStr for runvil::tools::ToolId impl core::str::traits::FromStr for runvil::tools::ToolId { type Err = runvil::error::Error } -impl core::str::traits::FromStr for runvil::vfs::PathReason -impl core::str::traits::FromStr for runvil::vfs::PathReason { type Err = runvil::vfs::ParsePathReasonError } impl runvil::vfs::Policy for runvil::vfs::AllowAll impl runvil::vfs::Policy for runvil::vfs::ModePolicy impl runvil::vfs::Vfs for runvil::vfs::MemoryBackend @@ -551,8 +545,6 @@ pub fn runvil::vfs::OpEvent::origin(&self) -> &'a runvil::vfs::Origin pub fn runvil::vfs::OpEvent::path(&self) -> &'a runvil::vfs::VfsPath pub fn runvil::vfs::Origin::at(label: impl core::convert::Into, file: impl core::convert::Into, line: u32) -> runvil::vfs::Origin pub fn runvil::vfs::Origin::new(label: impl core::convert::Into) -> runvil::vfs::Origin -pub fn runvil::vfs::ParsePathReasonError::input(&self) -> &str -pub fn runvil::vfs::PathReason::from_tag(tag: &str) -> core::option::Option pub fn runvil::vfs::PathReason::tag(self) -> &'static str pub fn runvil::vfs::Policy::check(&self, op: runvil::vfs::Op, path: &runvil::vfs::VfsPath) -> runvil::vfs::Verdict pub fn runvil::vfs::RealBackend::identity() -> runvil::vfs::RealBackend @@ -1277,7 +1269,6 @@ pub struct runvil::vfs::ExecId(/* private fields */) pub struct runvil::vfs::ModeHandle { /* private fields */ } pub struct runvil::vfs::ModePolicy { /* private fields */ } pub struct runvil::vfs::OpEvent<'a> { /* private fields */ } -pub struct runvil::vfs::ParsePathReasonError { /* private fields */ } pub struct runvil::vfs::VfsPath { /* private fields */ } pub struct runvil::vfs::VfsPathBuf(/* private fields */) pub struct runvil::vfs::VfsRef { /* private fields */ } diff --git a/crates/runvil/src/lib.rs b/crates/runvil/src/lib.rs index 46eadd523..2c79f2415 100644 --- a/crates/runvil/src/lib.rs +++ b/crates/runvil/src/lib.rs @@ -151,7 +151,6 @@ pub mod vfs { pub use runvil_vfs::Op; pub use runvil_vfs::OpEvent; pub use runvil_vfs::Origin; - pub use runvil_vfs::ParsePathReasonError; pub use runvil_vfs::PathReason; pub use runvil_vfs::Policy; pub use runvil_vfs::RealBackend; From 7c0840710c31207627807b30f57f8a135b99d648 Mon Sep 17 00:00:00 2001 From: Vinnie Falco Date: Thu, 5 Nov 2026 22:02:51 -0800 Subject: [PATCH 2/6] Replace `runvil_vfs::detail` with `ScopeHandle` An `Access` no longer gives authority over its scope. Only `VfsRef::acquire_scope` returns a `ScopeHandle`, and the operations that fork, join, and end identities, derive store views, and end the scope are its methods. The Engine takes the run's handle from its root acquisition and passes it through `ChainOps` and `SectionVmSetup` to `install_engine_globals`. - `ScopeHandle::spawn`, `ScopeHandle::store_view`, and `ScopeHandle::probe_store` assert that the `Access` belongs to the handle's scope and panic otherwise. Each has `#[track_caller]` and a `# Panics` section. - `ScopeHandle::join` and `ScopeHandle::end_access` take `ExecId` values and act only on identities in the handle's own scope, so they never panic. - `install_engine_globals` takes the handle and derives the store view once. The Lua VM keeps the view and not the handle. - `Access::id` becomes public, and the `runvil::vfs` facade re-exports `ScopeHandle`. - The test hook that dispatches a store call through a store-less access now installs that access's own handle, because the run's handle refuses an access from another scope. - The new spawn documentation states that a refused acquire still advances the parent's own clock entry. The removed `access_spawn` documentation said the clock stayed untouched. - Four new tests pin the panics on a foreign access and show that another scope's `join`, `end_access`, and `end` change nothing. - The probe-chain test hooks still acquire their chain's access outside the run's scope. --- .../engine/src/execute/context-tests.rs | 22 ++- .../engine/src/execute/context.rs | 13 +- .../engine/src/execute/scheduler.rs | 32 +++- .../src/execute/scheduler/admission-tests.rs | 2 +- .../engine/src/execute/scheduler/apply.rs | 9 +- .../engine/src/execute/scheduler/chain_ops.rs | 30 +++- .../engine/src/execute/scheduler/dispatch.rs | 10 +- .../engine/src/execute/scheduler/drive.rs | 8 +- .../src/execute/scheduler/effects-tests.rs | 2 +- .../engine/src/execute/scheduler/h1.rs | 6 +- .../engine/src/execute/scheduler/task_end.rs | 13 +- .../src/execute/scheduler/tasks-spawn.rs | 15 +- .../src/execute/scheduler/tasks-tests.rs | 2 +- .../src/execute/scheduler/test_hooks.rs | 30 ++-- .../engine/src/execute/scheduler/tool_call.rs | 15 +- .../engine/src/execute/scheduler/walk.rs | 12 +- .../src/execute/section_context-construct.rs | 20 ++- .../engine/src/execute/section_vm.rs | 7 +- .../engine/src/execute/tests/suite/prepare.rs | 16 +- .../engine/src/execute/tests/suite/vfs.rs | 6 +- .../src/execute/tests/tool_call_access.rs | 5 +- .../engine/src/execute/tests/tool_scoping.rs | 2 +- .../runvil-internal/engine/src/lua/tests.rs | 3 +- .../runvil-internal/engine/src/model/tests.rs | 2 +- .../engine/src/test_support.rs | 15 +- .../src/test_support/scenario-driving.rs | 10 +- .../lua/src/engine_globals-store.rs | 10 +- crates/runvil-internal/lua/src/test_vm.rs | 45 +++-- .../runvil-internal/lua/src/test_vm/access.rs | 37 +++++ crates/runvil-internal/lua/src/tests.rs | 22 +-- .../lua/src/tests/section_vm.rs | 22 ++- .../lua/src/tests/store_errors.rs | 11 +- .../lua/src/tests/store_reports.rs | 6 +- crates/runvil-internal/lua/src/vm/install.rs | 17 +- crates/runvil-internal/vfs/src/detail.rs | 129 -------------- crates/runvil-internal/vfs/src/handle.rs | 26 ++- .../runvil-internal/vfs/src/handle/access.rs | 30 ++-- .../vfs/src/handle/scope_handle.rs | 157 ++++++++++++++++++ .../scope_handle/tests.rs} | 113 ++++++++++--- .../vfs/src/handle/store_view.rs | 5 +- .../runvil-internal/vfs/src/handle/tests.rs | 2 +- .../vfs/src/handle/tests/happens_before.rs | 4 +- .../vfs/src/handle/tests/operations.rs | 1 + .../vfs/src/handle/tests/refusals.rs | 9 +- .../vfs/src/handle/tests/regions.rs | 2 +- crates/runvil-internal/vfs/src/lib.rs | 3 +- .../runvil-internal/vfs/src/router-tests.rs | 4 +- crates/runvil/public-api.txt | 11 ++ crates/runvil/src/lib.rs | 1 + 49 files changed, 609 insertions(+), 365 deletions(-) create mode 100644 crates/runvil-internal/lua/src/test_vm/access.rs delete mode 100644 crates/runvil-internal/vfs/src/detail.rs create mode 100644 crates/runvil-internal/vfs/src/handle/scope_handle.rs rename crates/runvil-internal/vfs/src/{detail-tests.rs => handle/scope_handle/tests.rs} (80%) diff --git a/crates/runvil-internal/engine/src/execute/context-tests.rs b/crates/runvil-internal/engine/src/execute/context-tests.rs index c0b397980..aead13a45 100644 --- a/crates/runvil-internal/engine/src/execute/context-tests.rs +++ b/crates/runvil-internal/engine/src/execute/context-tests.rs @@ -79,7 +79,7 @@ fn vm_setup_hands_the_vm_the_runs_argv_in_the_mode_the_caller_asks_for() { let prompt = test_prompt(); let ctx = test_state(&prompt).with_args("{\"query\":\"papers\"}"); let sys = json!({}); - let access = fresh_access(); + let (access, scope) = fresh_access(); let requested = ctx.argv().cloned(); let expected = requested.as_deref(); assert!( @@ -87,13 +87,29 @@ fn vm_setup_hands_the_vm_the_runs_argv_in_the_mode_the_caller_asks_for() { "the fixture's args must derive an argv, or neither mode is observable" ); let selection = Arc::new(Mutex::new(None)); - let frozen = ctx.vm_setup(&sys, VmSeed::default(), &access, &selection, "Only", false); + let frozen = ctx.vm_setup( + &sys, + VmSeed::default(), + &access, + &scope, + &selection, + "Only", + false, + ); assert!( matches!(frozen.argv, Argv::Frozen(value) if value == expected), "an unprivileged constructor must get the run's argv frozen: {:?}", frozen.argv ); - let writable = ctx.vm_setup(&sys, VmSeed::default(), &access, &selection, "Only", true); + let writable = ctx.vm_setup( + &sys, + VmSeed::default(), + &access, + &scope, + &selection, + "Only", + true, + ); assert!( matches!(writable.argv, Argv::Writable(value) if value == expected), "the H1 pass's flag must get the run's argv writable: {:?}", diff --git a/crates/runvil-internal/engine/src/execute/context.rs b/crates/runvil-internal/engine/src/execute/context.rs index c03c227d6..f940c43d8 100644 --- a/crates/runvil-internal/engine/src/execute/context.rs +++ b/crates/runvil-internal/engine/src/execute/context.rs @@ -19,7 +19,7 @@ use runvil_types::event::Event; use runvil_types::ids::{ChainId, TaskId}; use runvil_types::packs::Prelude; use runvil_types::untrusted::GuardNonce; -use runvil_vfs::{Access, VfsRef}; +use runvil_vfs::{Access, ScopeHandle, VfsRef}; use crate::Error; use crate::lua::{Argv, ToolSet}; @@ -391,17 +391,23 @@ impl ChainState { /// the run-wide inputs from this context; the constructor supplies only /// its own deltas: the `sys` JSON, the seed, the chain step's access /// capability (the walk's own, a call chain's borrowed parent - /// capability, a task chain's spawned one), the section's current-model - /// selection, the section name, and whether `argv` installs writable. + /// capability, a task chain's spawned one) and the handle on the run's + /// scope, the section's current-model selection, the section name, and + /// whether `argv` installs writable. /// /// `argv_writable` picks the [`Argv`] mode that /// [`SectionVmSetup::argv`] describes; the H1 pass is the only /// production caller that passes `true`. + #[expect( + clippy::too_many_arguments, + reason = "each input is one of the constructor's own deltas, borrowed separately for the setup's life" + )] pub(super) fn vm_setup<'a>( &'a self, sys: &'a serde_json::Value, seed: VmSeed<'a>, access: &'a Arc, + scope: &'a ScopeHandle, selection: &'a Arc>>, section_name: &'a str, argv_writable: bool, @@ -416,6 +422,7 @@ impl ChainState { argv, sys, access, + scope, selection, seed, emitter: &self.reporting.emitter, diff --git a/crates/runvil-internal/engine/src/execute/scheduler.rs b/crates/runvil-internal/engine/src/execute/scheduler.rs index 4076c2766..6e7f38977 100644 --- a/crates/runvil-internal/engine/src/execute/scheduler.rs +++ b/crates/runvil-internal/engine/src/execute/scheduler.rs @@ -92,11 +92,11 @@ use std::sync::Arc; use runvil_types::cancel::CancelHandle; use runvil_vfs::Origin; -use runvil_vfs::detail::ScopeHandle; +use runvil_vfs::ScopeHandle; -use crate::Result; use crate::lua::Answer; use crate::prompt::{Block, Prompt}; +use crate::{Result, error}; use super::context::ChainState; use super::run::EffectAnswer; @@ -174,11 +174,14 @@ pub(crate) struct Scheduler { /// Where the step loop stands: not started, running, ending with the /// result held until the orphans are answered, or done. phase: Phase, - /// The run's scope, taken where the run acquires its root identity: - /// closed when the run reaches `Done` or is dropped before it, so a - /// store view the caller still holds never outlives the run. The two - /// root-start sites (`h1::start_h1` and `walk::start_root_walk`) are its - /// only writers, each at the acquisition that opens the run's identity. + /// The handle on the run's scope, which the acquisition of the run's + /// root identity returns: every fork, join, and end of the run's + /// identities and every store view of their accesses goes through it, + /// and it closes the scope when the run reaches `Done` or is dropped + /// before it, so a store view the caller still holds never outlives the + /// run. The two root-start sites (`h1::start_h1` and + /// `walk::start_root_walk`) are its only writers in a shipped Engine, + /// each at the acquisition that opens the run's identity. scope: Option, } @@ -208,10 +211,23 @@ impl Scheduler { /// its next operation, and its claims stop conflicting. fn end_scope(&mut self) { if let Some(scope) = self.scope.take() { - runvil_vfs::detail::end_scope(&scope); + scope.end(); } } + /// The handle on the run's scope, for a fork of one of the run's + /// accesses. + /// + /// # Errors + /// Returns an `Internal` failure when the run holds no scope handle: + /// before its root identity is acquired, when no access exists to fork, + /// or after the run ended. + fn run_scope(&self) -> Result<&ScopeHandle> { + self.scope + .as_ref() + .ok_or(error::internal("the run holds no scope handle")) + } + /// The prompt's shared handle: a chain resolves its walk position /// against this clone so the tree borrow never pins the arena. fn prompt(&self) -> Arc { diff --git a/crates/runvil-internal/engine/src/execute/scheduler/admission-tests.rs b/crates/runvil-internal/engine/src/execute/scheduler/admission-tests.rs index 85f7ca9fb..03dbfa647 100644 --- a/crates/runvil-internal/engine/src/execute/scheduler/admission-tests.rs +++ b/crates/runvil-internal/engine/src/execute/scheduler/admission-tests.rs @@ -57,7 +57,7 @@ impl Tree { .expect("the default backend acquires"); let backing = TaskBacking::Chain { chain: id, - exec: runvil_vfs::detail::access_id(&access), + exec: access.id(), }; let task = self.chains[id].task.clone(); self.tasks diff --git a/crates/runvil-internal/engine/src/execute/scheduler/apply.rs b/crates/runvil-internal/engine/src/execute/scheduler/apply.rs index cbca53608..0d04921af 100644 --- a/crates/runvil-internal/engine/src/execute/scheduler/apply.rs +++ b/crates/runvil-internal/engine/src/execute/scheduler/apply.rs @@ -21,7 +21,7 @@ use runvil_types::error::{ErrorCode, place}; use runvil_types::event::lifecycle::{self, Lifecycle}; use runvil_types::ids::{RoundId, TaskId}; use runvil_types::tools::ToolOutput; -use runvil_vfs::{ExecId, VfsError, detail}; +use runvil_vfs::{ExecId, VfsError}; use crate::execute::infer::accept_infer; use crate::lua::{ @@ -207,11 +207,8 @@ impl Scheduler { let Some(scope) = &self.scope else { return; }; - let owner = self.chains[chain] - .access() - .ok() - .map(|access| detail::access_id(access)); - detail::end_access(scope, owner, exec); + let owner = self.chains[chain].access().ok().map(|access| access.id()); + scope.end_access(owner, exec); } /// Applies a dropped timer: `task`'s entry moves to `Canceled`. While diff --git a/crates/runvil-internal/engine/src/execute/scheduler/chain_ops.rs b/crates/runvil-internal/engine/src/execute/scheduler/chain_ops.rs index 0e26d41f6..6bbd91555 100644 --- a/crates/runvil-internal/engine/src/execute/scheduler/chain_ops.rs +++ b/crates/runvil-internal/engine/src/execute/scheduler/chain_ops.rs @@ -1,11 +1,12 @@ //! The one-chain view: the requests that touch only their own chain run //! in a [`ChainOps`], which borrows that chain and the effect table -//! mutably and owns nothing. A request in the view can change only its own -//! chain and the effect table: it can't reach another chain, the task -//! arena, admission, or the ready queue, and the compiler enforces that. +//! mutably, reads the run's scope handle, and owns nothing. A request in +//! the view can change only its own chain and the effect table: it can't +//! reach another chain, the task arena, admission, or the ready queue, and +//! the compiler enforces that. //! [`Scheduler::with_chain`] is the only way into the view. The view's //! requests sit beside the arms they belong to, in the eight -//! `impl ChainOps<'_>` blocks this crate holds: the shared `prompt`, +//! `impl ChainOps` blocks this crate holds: the shared `prompt`, `scope`, //! `answer`, `issue`, and `number_round` live here in `chain_ops`, and the //! arms live in `apply`, `chat`, `dispatch`, `tool_call`, its `adhoc` and //! `discovery` children, and `walk`. @@ -13,11 +14,12 @@ use std::sync::Arc; use runvil_types::event::ReplyOrigin; +use runvil_vfs::ScopeHandle; -use crate::Result; use crate::execute::run::{Effect, EffectId, Round}; use crate::lua::Answer; use crate::prompt::Prompt; +use crate::{Result, error}; use super::{Chain, ChainIndex, Continuation, EffectTable, Scheduler}; @@ -32,6 +34,9 @@ pub(super) struct ChainOps<'a> { pub(super) chain: &'a mut Chain, /// The effect table the view's leaf arms issue into. effects: &'a mut EffectTable, + /// The handle on the run's scope, which forks the chain's access and + /// derives its store view. + scope: Option<&'a ScopeHandle>, /// Whether the view delivered an answer to the chain, which /// [`Scheduler::with_chain`] requeues once. answered: bool, @@ -51,6 +56,7 @@ impl Scheduler { id, chain: &mut self.chains[id], effects: &mut self.effects, + scope: self.scope.as_ref(), answered: false, }; let result = f(&mut ops); @@ -61,7 +67,7 @@ impl Scheduler { } } -impl ChainOps<'_> { +impl<'a> ChainOps<'a> { /// The prompt's shared handle: the chain resolves its walk position /// against this clone so the tree borrow never pins the chain. The /// chain's own context fork is the source, which is the one `ChainState` @@ -70,6 +76,18 @@ impl ChainOps<'_> { Arc::clone(self.chain.ctx.prompt()) } + /// The handle on the run's scope, which forks the chain's access and + /// derives its store view. It borrows from the scheduler rather than + /// the view, so it outlives a borrow of the chain. + /// + /// # Errors + /// Returns an `Internal` failure when the run holds no scope handle, + /// which every chain's start rules out. + pub(super) fn scope(&self) -> Result<&'a ScopeHandle> { + self.scope + .ok_or(error::internal("the run holds no scope handle")) + } + /// Delivers `answer` to the chain's suspended block: the inline-answer /// path of every request in the view. The requeue is recorded for /// [`Scheduler::with_chain`] to perform. diff --git a/crates/runvil-internal/engine/src/execute/scheduler/dispatch.rs b/crates/runvil-internal/engine/src/execute/scheduler/dispatch.rs index 6033be789..25c1059af 100644 --- a/crates/runvil-internal/engine/src/execute/scheduler/dispatch.rs +++ b/crates/runvil-internal/engine/src/execute/scheduler/dispatch.rs @@ -231,13 +231,13 @@ impl ChainOps<'_> { /// /// # Errors /// Returns an `Internal` failure when the live chain's access - /// capability is gone, which only the chain-end paths take, or a store - /// failure when the handle the chain's access came from declares no store, - /// which the run's start probe already ruled out, placed on the store - /// call's line. + /// capability is gone, which only the chain-end paths take, or when the + /// run holds no scope handle, or a store failure when the handle the + /// chain's access came from declares no store, which the run's start + /// probe already ruled out, placed on the store call's line. fn dispatch_vfs(&mut self, op: VfsOp) -> Result<()> { let access = Arc::clone(self.chain.access()?); - let view = Arc::new(runvil_vfs::detail::store_view(&access).map_err(|cause| { + let view = Arc::new(self.scope()?.store_view(&access).map_err(|cause| { let chain = &*self.chain; let line = chain.frame().ok().and_then(|frame| frame.vm().call_line()); place(error::store(&cause), chain.ctx.prompt().file(), line) diff --git a/crates/runvil-internal/engine/src/execute/scheduler/drive.rs b/crates/runvil-internal/engine/src/execute/scheduler/drive.rs index 70a5c0414..56ff0dd65 100644 --- a/crates/runvil-internal/engine/src/execute/scheduler/drive.rs +++ b/crates/runvil-internal/engine/src/execute/scheduler/drive.rs @@ -189,12 +189,14 @@ impl Scheduler { // stats the store root through a throwaway capability, so a handle // with no declaration fails here rather than on the first store call, // and a declared-but-failing backend's error fails the run. - let probe = self + let (probe, scope) = self .ctx .vfs() - .acquire(Origin::new("store mount probe")) + .acquire_scope(Origin::new("store mount probe")) + .map_err(|cause| error::store(&cause))?; + scope + .probe_store(&probe) .map_err(|cause| error::store(&cause))?; - runvil_vfs::detail::probe_store(&probe).map_err(|cause| error::store(&cause))?; if let Some(refusal) = self.ctx.refusal() { return Err(refusal.clone()); } diff --git a/crates/runvil-internal/engine/src/execute/scheduler/effects-tests.rs b/crates/runvil-internal/engine/src/execute/scheduler/effects-tests.rs index 83fe88816..c2f4d9267 100644 --- a/crates/runvil-internal/engine/src/execute/scheduler/effects-tests.rs +++ b/crates/runvil-internal/engine/src/execute/scheduler/effects-tests.rs @@ -48,7 +48,7 @@ fn issue_tool_call(table: &mut EffectTable, chain: ChainIndex) -> (EffectId, Exe let access = VfsRef::default() .acquire(Origin::new("effects-test")) .expect("the default backend acquires"); - let exec = runvil_vfs::detail::access_id(&access); + let exec = access.id(); let tool = ToolId::parse("example/test/echo").expect("a fixture id is a tool id"); let binding = ToolBinding::from_info( "echo", diff --git a/crates/runvil-internal/engine/src/execute/scheduler/h1.rs b/crates/runvil-internal/engine/src/execute/scheduler/h1.rs index 18a5eed2b..194036cee 100644 --- a/crates/runvil-internal/engine/src/execute/scheduler/h1.rs +++ b/crates/runvil-internal/engine/src/execute/scheduler/h1.rs @@ -46,12 +46,12 @@ impl Scheduler { self.ctx.prompt().title(), crate::prompt::detail::h1_blocks(self.ctx.prompt()), ); - let access = self + let (access, scope) = self .ctx .vfs() - .acquire(origin) + .acquire_scope(origin) .map_err(|cause| error::store(&cause))?; - self.scope = Some(runvil_vfs::detail::scope_handle(&access)); + self.scope = Some(scope); // The pass's one frame takes the root lineage's entry 0, and the // walk that follows continues its counters (see `Chain::lineage`). // Its admission limit is the run's ceiling. diff --git a/crates/runvil-internal/engine/src/execute/scheduler/task_end.rs b/crates/runvil-internal/engine/src/execute/scheduler/task_end.rs index ee160ae35..46434dd29 100644 --- a/crates/runvil-internal/engine/src/execute/scheduler/task_end.rs +++ b/crates/runvil-internal/engine/src/execute/scheduler/task_end.rs @@ -104,8 +104,8 @@ impl Scheduler { /// Joins `task` into `owner`: merges the task's final clock into the /// owner's access, so everything the task did happens before the - /// owner's next step. A timer (no identity) and an owner already - /// ended (no access) need no join. + /// owner's next step. A timer (no identity), an owner already ended + /// (no access), and a run whose scope has ended need no join. pub(super) fn join_task(&self, owner: ChainIndex, task: &TaskId) { let Some(exec) = self.tasks.exec_of(task) else { return; @@ -113,15 +113,20 @@ impl Scheduler { let Ok(access) = self.chains[owner].access() else { return; }; - runvil_vfs::detail::access_join(access, exec); + if let Some(scope) = &self.scope { + scope.join(access.id(), exec); + } } /// Joins every task `owner` owns into `access`: the chain-end join, /// so the chain's own final clock transitively covers everything its /// tasks did even when no wait delivered them. pub(super) fn join_owned_tasks(&self, owner: ChainIndex, access: &Access) { + let Some(scope) = &self.scope else { + return; + }; for exec in self.tasks.execs_owned_by(owner) { - runvil_vfs::detail::access_join(access, exec); + scope.join(access.id(), exec); } } diff --git a/crates/runvil-internal/engine/src/execute/scheduler/tasks-spawn.rs b/crates/runvil-internal/engine/src/execute/scheduler/tasks-spawn.rs index c8fc3aed5..99d08ebbf 100644 --- a/crates/runvil-internal/engine/src/execute/scheduler/tasks-spawn.rs +++ b/crates/runvil-internal/engine/src/execute/scheduler/tasks-spawn.rs @@ -11,7 +11,6 @@ use std::sync::Arc; use std::sync::atomic::AtomicU32; use runvil_types::ids::TaskId; -use runvil_vfs::detail; use crate::execute::section_context::TaskSeed; use crate::lua::Answer; @@ -111,15 +110,17 @@ impl Scheduler { // runs ahead of the id allocation and the arena push rather than // orphaning a started chain that is neither enqueued nor entered // in the arena. - let access = detail::access_spawn( - &spawner_access, - prompt_origin(&prompt, worker.name(), worker.blocks()), - ) - .map_err(|cause| error::store(&cause))?; + let access = self + .run_scope()? + .spawn( + &spawner_access, + prompt_origin(&prompt, worker.name(), worker.blocks()), + ) + .map_err(|cause| error::store(&cause))?; // The task's happens-before identity, recorded before the access // moves into the chain: every delivery joins it, and the chain // end joins it last. - let exec = detail::access_id(&access); + let exec = access.id(); // The task's id is the spawner's next child id; the task is its // chain, named from the other side. let lineage = self.chains[id].next_child_id()?; diff --git a/crates/runvil-internal/engine/src/execute/scheduler/tasks-tests.rs b/crates/runvil-internal/engine/src/execute/scheduler/tasks-tests.rs index bf2e334ab..a28d521b1 100644 --- a/crates/runvil-internal/engine/src/execute/scheduler/tasks-tests.rs +++ b/crates/runvil-internal/engine/src/execute/scheduler/tasks-tests.rs @@ -29,7 +29,7 @@ fn chain_backing(raw: u32) -> TaskBacking { .expect("the default backend acquires"); TaskBacking::Chain { chain: chain(raw), - exec: runvil_vfs::detail::access_id(&access), + exec: access.id(), } } diff --git a/crates/runvil-internal/engine/src/execute/scheduler/test_hooks.rs b/crates/runvil-internal/engine/src/execute/scheduler/test_hooks.rs index 5494d64d3..f405cbb36 100644 --- a/crates/runvil-internal/engine/src/execute/scheduler/test_hooks.rs +++ b/crates/runvil-internal/engine/src/execute/scheduler/test_hooks.rs @@ -10,7 +10,7 @@ use std::sync::Arc; use std::sync::atomic::Ordering; use runvil_types::ids::{ChainId, TaskId}; -use runvil_vfs::{MemoryBackend, Origin, VfsRef, detail}; +use runvil_vfs::{MemoryBackend, Origin, VfsRef}; use crate::Result; use crate::execute::run::EffectId; @@ -44,14 +44,17 @@ impl Scheduler { .next_child_id() .expect("the wedged chain has a child id left"); let task = TaskId::from(lineage.clone()); - let access = detail::access_spawn( - self.chains[owner] - .access() - .expect("the wedged chain holds its access"), - Origin::at("wedge", self.ctx.prompt().title(), 1), - ) - .expect("the wedged chain's access forks"); - let exec = detail::access_id(&access); + let access = self + .run_scope() + .expect("the run holds its scope handle") + .spawn( + self.chains[owner] + .access() + .expect("the wedged chain holds its access"), + Origin::at("wedge", self.ctx.prompt().title(), 1), + ) + .expect("the wedged chain's access forks"); + let exec = access.id(); let wedge = self .start_chain(ChainStart { lineage, @@ -158,7 +161,9 @@ impl Scheduler { /// Dispatches `op` again for the chain at arena index 0, whose block /// is suspended on a store request, through an access from a handle /// that declares no store: the store-view failure the run's start - /// probe otherwise rules out. + /// probe otherwise rules out. The access's own scope handle replaces + /// the run's, since a scope handle derives store views for its own + /// scope alone. /// /// # Errors /// Returns the dispatch's store failure. @@ -168,10 +173,11 @@ impl Scheduler { .iter() .next() .expect("the run has started a chain"); - let access = VfsRef::new(MemoryBackend::new()) - .acquire(Origin::new("no store")) + let (access, scope) = VfsRef::new(MemoryBackend::new()) + .acquire_scope(Origin::new("no store")) .expect("a memory backend acquires"); self.chains[id].access = Some(Arc::new(access)); + self.scope = Some(scope); self.dispatch(id, Request::Store { op }) } diff --git a/crates/runvil-internal/engine/src/execute/scheduler/tool_call.rs b/crates/runvil-internal/engine/src/execute/scheduler/tool_call.rs index 30ab95ef7..7047f1d10 100644 --- a/crates/runvil-internal/engine/src/execute/scheduler/tool_call.rs +++ b/crates/runvil-internal/engine/src/execute/scheduler/tool_call.rs @@ -44,7 +44,6 @@ use std::sync::Arc; use std::sync::atomic::Ordering; use runvil_types::tools::ToolId; -use runvil_vfs::detail; use crate::execute::run::{Effect, ToolCallOrigin, ToolCaller}; use crate::lua::{ @@ -364,12 +363,14 @@ impl ChainOps<'_> { // chain's, so two calls in flight at once never share one sequence // of claims. A refused fork fails the call at its caller, and the // counts already taken stay taken. - let access = detail::access_spawn( - chain.access()?, - prompt_origin(&prompt, &binding.id().to_string(), chain.blocks(&prompt)), - ) - .map_err(|cause| error::store(&cause))?; - let exec = detail::access_id(&access); + let access = self + .scope()? + .spawn( + chain.access()?, + prompt_origin(&prompt, &binding.id().to_string(), chain.blocks(&prompt)), + ) + .map_err(|cause| error::store(&cause))?; + let exec = access.id(); let effect = Effect::ToolCall { tool: binding.id().clone(), alias: binding.alias().to_owned(), diff --git a/crates/runvil-internal/engine/src/execute/scheduler/walk.rs b/crates/runvil-internal/engine/src/execute/scheduler/walk.rs index d2188ba69..2ef0061cf 100644 --- a/crates/runvil-internal/engine/src/execute/scheduler/walk.rs +++ b/crates/runvil-internal/engine/src/execute/scheduler/walk.rs @@ -53,12 +53,12 @@ impl Scheduler { let blocks: &[Block] = crate::prompt::detail::entry(prompt).map_or(&[], |section| section.blocks()); let origin = prompt_origin(prompt, prompt.title(), blocks); - let access = self + let (access, scope) = self .ctx .vfs() - .acquire(origin) + .acquire_scope(origin) .map_err(|cause| error::store(&cause))?; - self.scope = Some(runvil_vfs::detail::scope_handle(&access)); + self.scope = Some(scope); self.start_root_chain( ChainKind::Root, after_h1, @@ -104,9 +104,11 @@ impl ChainOps<'_> { /// /// # Errors /// Returns the failure of frame construction, as documented on - /// [`SectionContext::new`]. + /// [`SectionContext::new`], or an `Internal` failure when the run holds + /// no scope handle. fn enter_section(&mut self) -> Result { let prompt = self.prompt(); + let scope = self.scope()?; let chain = &mut *self.chain; if chain.kind.is_h1() { // The H1 pass enters its frame exactly once: section 0 under @@ -117,6 +119,7 @@ impl ChainOps<'_> { let frame = SectionContext::new_h1( &chain.ctx, chain.access()?, + scope, &chain.selection, §ion_id, &chain.task, @@ -137,6 +140,7 @@ impl ChainOps<'_> { let frame = SectionContext::new( &chain.ctx, chain.access()?, + scope, &chain.selection, chain.walk.current(), §ion_id, diff --git a/crates/runvil-internal/engine/src/execute/section_context-construct.rs b/crates/runvil-internal/engine/src/execute/section_context-construct.rs index 64c79b956..6fbec8eb6 100644 --- a/crates/runvil-internal/engine/src/execute/section_context-construct.rs +++ b/crates/runvil-internal/engine/src/execute/section_context-construct.rs @@ -11,7 +11,7 @@ use std::sync::{Arc, Mutex}; use runvil_types::event::lifecycle; use runvil_types::ids::TaskId; -use runvil_vfs::Access; +use runvil_vfs::{Access, ScopeHandle}; use crate::execute::context::ChainState; use crate::execute::section_vm::{self, VmSeed}; @@ -77,7 +77,8 @@ impl SectionContext { /// is the entering chain's `sys.taskid`. `var` is the walk's current /// `var` table, seeded into the section's VM. `seed` holds a spawned /// chain's `item` and `sys.index` on its first entry and is empty on - /// every other entry. + /// every other entry. `scope` is the handle on the run's scope, which + /// derives the `store` table's view of `access`. /// /// # Errors /// Returns an `Internal` failure when `pos` is past its slice, and @@ -85,11 +86,12 @@ impl SectionContext { /// for which failures tear the VM down). #[expect( clippy::too_many_arguments, - reason = "the frame takes the two chain-owned handles (access and selection) beside the section entry's own deltas, and each stays a separate borrow" + reason = "the frame takes the two chain-owned handles (access and selection) and the run's scope handle beside the section entry's own deltas, and each stays a separate borrow" )] pub(crate) fn new( ctx: &ChainState, access: &Arc, + scope: &ScopeHandle, selection: &Arc>>, pos: &SectionPos, section_id: &str, @@ -132,6 +134,7 @@ impl SectionContext { item: seed.item.as_ref(), }, access, + scope, selection, section.name(), false, @@ -165,6 +168,7 @@ impl SectionContext { pub(crate) fn new_h1( ctx: &ChainState, access: &Arc, + scope: &ScopeHandle, selection: &Arc>>, section_id: &str, task_id: &TaskId, @@ -185,7 +189,15 @@ impl SectionContext { // describes; the executor reads the value back at the freeze (see // the scheduler's H1-to-walk handoff). let sys_json = sys.to_json(); - let setup = ctx.vm_setup(&sys_json, VmSeed::default(), access, selection, title, true); + let setup = ctx.vm_setup( + &sys_json, + VmSeed::default(), + access, + scope, + selection, + title, + true, + ); // Setup runs on the bare VM and tears it down itself on failure: // the frame does not exist yet, so its `Drop` cannot own this path. let vm = section_vm::setup_section_vm(vm, &setup, read, resolve, render)?; diff --git a/crates/runvil-internal/engine/src/execute/section_vm.rs b/crates/runvil-internal/engine/src/execute/section_vm.rs index 8c347b18b..33e66ba4a 100644 --- a/crates/runvil-internal/engine/src/execute/section_vm.rs +++ b/crates/runvil-internal/engine/src/execute/section_vm.rs @@ -22,7 +22,7 @@ use std::sync::{Arc, Mutex}; use runvil_types::emitter::Emitter; use runvil_types::packs::Prelude; -use runvil_vfs::Access; +use runvil_vfs::{Access, ScopeHandle}; use crate::lua::{Library, LibraryKey, ProseState, SectionText, SectionVm, Uninjected}; use crate::model::ModelBinding; @@ -65,6 +65,9 @@ pub(crate) struct SectionVmSetup<'a> { /// (a fanout arm included). The store closures share it, so every /// store op is attributed to the chain step's identity. pub(crate) access: &'a Arc, + /// The handle on the run's scope, which derives the `store` table's + /// view of `access`; the VM keeps the view and never the handle. + pub(crate) scope: &'a ScopeHandle, /// The section's current-model selection: it starts on the `default` /// binding, or on nothing when the caller supplied no model, and /// `models.use` rewrites it. @@ -178,7 +181,7 @@ where + 'static, P: Fn(&str, Option, ProseState<'_>) -> mlua::Result + Send + Sync + 'static, { - vm.install_engine_globals(setup.emitter, setup.section_name)?; + vm.install_engine_globals(setup.emitter, setup.section_name, setup.scope)?; if let Some(snapshot) = setup.ui { crate::lua::install_ui(vm.lua(), Arc::clone(snapshot))?; } diff --git a/crates/runvil-internal/engine/src/execute/tests/suite/prepare.rs b/crates/runvil-internal/engine/src/execute/tests/suite/prepare.rs index a1244e449..dcdb1fae8 100644 --- a/crates/runvil-internal/engine/src/execute/tests/suite/prepare.rs +++ b/crates/runvil-internal/engine/src/execute/tests/suite/prepare.rs @@ -37,18 +37,14 @@ const DECLARES_NOTHING: &str = concat!( #[test] fn two_runs_writing_the_same_store_path_do_not_conflict() { let (ctx_a, ctx_b) = (test_params("run-a"), test_params("run-b")); - let access_a = ctx_a + let store_a = ctx_a .vfs - .acquire(Origin::new("run-a")) - .expect("run a acquires"); - let access_b = ctx_b + .acquire_store(Origin::new("run-a")) + .expect("run a's handle declares a store"); + let store_b = ctx_b .vfs - .acquire(Origin::new("run-b")) - .expect("run b acquires"); - let store_a = - runvil_vfs::detail::store_view(&access_a).expect("run a's handle declares a store"); - let store_b = - runvil_vfs::detail::store_view(&access_b).expect("run b's handle declares a store"); + .acquire_store(Origin::new("run-b")) + .expect("run b's handle declares a store"); // Both writes proceed while both accesses are live: each run's store // is its own storage under its own claims table. store_a.write("paper.md", b"from a").expect("run a writes"); diff --git a/crates/runvil-internal/engine/src/execute/tests/suite/vfs.rs b/crates/runvil-internal/engine/src/execute/tests/suite/vfs.rs index 2d9fff00e..4c43a31ab 100644 --- a/crates/runvil-internal/engine/src/execute/tests/suite/vfs.rs +++ b/crates/runvil-internal/engine/src/execute/tests/suite/vfs.rs @@ -63,10 +63,8 @@ const MISSING_OUTPUT: &str = concat!( /// The store view derived from a fresh access on the run's handle: the /// caller's seeding and extraction half of the seed-run-extract round trip. fn fresh_store_view(vfs: &VfsRef, origin: &str) -> Access { - let access = vfs - .acquire(Origin::new(origin)) - .expect("the caller's backend acquires"); - runvil_vfs::detail::store_view(&access).expect("the handle declares a store") + vfs.acquire_store(Origin::new(origin)) + .expect("the handle declares a store") } /// The test's extraction rule: a declared output the run did not leave diff --git a/crates/runvil-internal/engine/src/execute/tests/tool_call_access.rs b/crates/runvil-internal/engine/src/execute/tests/tool_call_access.rs index 4c525d992..690934a27 100644 --- a/crates/runvil-internal/engine/src/execute/tests/tool_call_access.rs +++ b/crates/runvil-internal/engine/src/execute/tests/tool_call_access.rs @@ -12,7 +12,6 @@ use std::fmt; use runvil_vfs::ExecId; -use runvil_vfs::detail::access_id; use super::models_loop::loop_models; use super::*; @@ -125,7 +124,7 @@ fn files_answers( ) -> impl FnMut(EffectId, &Effect) -> EffectAnswer + '_ { move |_, effect| match effect { Effect::ToolCall { access, args, .. } => { - seen.tools.push(access_id(access)); + seen.tools.push(access.id()); let output = use_access(access, args); match reply { Reply::Output => EffectAnswer::ToolCall(output), @@ -134,7 +133,7 @@ fn files_answers( } other => { if let Effect::Vfs { access, .. } = other { - seen.stores.push(access_id(access)); + seen.stores.push(access.id()); } perform_store(other) } diff --git a/crates/runvil-internal/engine/src/execute/tests/tool_scoping.rs b/crates/runvil-internal/engine/src/execute/tests/tool_scoping.rs index 21d7a55ee..1bf3c7293 100644 --- a/crates/runvil-internal/engine/src/execute/tests/tool_scoping.rs +++ b/crates/runvil-internal/engine/src/execute/tests/tool_scoping.rs @@ -118,7 +118,7 @@ fn a_section_offer_by_id_scopes_the_tool_and_dispatches_it() { "Only", ) .expect("the section VM builds") - .inject_test_values("", &json!({}), &fresh_access()) + .inject_test_values("", &json!({}), &fresh_access().0) .expect("values must inject"); // The section's `tools.offer` lands in its offers; the scope diff --git a/crates/runvil-internal/engine/src/lua/tests.rs b/crates/runvil-internal/engine/src/lua/tests.rs index 03d0220b8..9cd8c8fa0 100644 --- a/crates/runvil-internal/engine/src/lua/tests.rs +++ b/crates/runvil-internal/engine/src/lua/tests.rs @@ -123,7 +123,7 @@ fn set_up_vm(binds: &VmBinds<'_>) -> SectionVm { ) .expect("the section VM builds"); let sys = json!({}); - let access = fresh_access(); + let (access, scope) = fresh_access(); let set = binds.models; let selection = Arc::new(Mutex::new(set.binding("default").cloned())); let setup = SectionVmSetup { @@ -131,6 +131,7 @@ fn set_up_vm(binds: &VmBinds<'_>) -> SectionVm { argv: binds.argv, sys: &sys, access: &access, + scope: &scope, selection: &selection, seed: binds.seed, emitter: &emitter, diff --git a/crates/runvil-internal/engine/src/model/tests.rs b/crates/runvil-internal/engine/src/model/tests.rs index 01a68a2ab..d4eb019f7 100644 --- a/crates/runvil-internal/engine/src/model/tests.rs +++ b/crates/runvil-internal/engine/src/model/tests.rs @@ -81,7 +81,7 @@ fn section_vm_with_models( emitter, section, )? - .inject_test_values("", &json!({}), &fresh_access()) + .inject_test_values("", &json!({}), &fresh_access().0) .map_err(|failure| failure.teardown(emitter, section)) } diff --git a/crates/runvil-internal/engine/src/test_support.rs b/crates/runvil-internal/engine/src/test_support.rs index 942c11cfc..02631fb01 100644 --- a/crates/runvil-internal/engine/src/test_support.rs +++ b/crates/runvil-internal/engine/src/test_support.rs @@ -97,16 +97,15 @@ pub(crate) fn with_author_prose(error: crate::Error, line: u32) -> crate::Error error } -/// A fresh default handle's access capability, for tests that inject Engine -/// values into a standalone VM. +/// A fresh default handle's access capability beside the handle on its +/// scope, for tests that inject Engine values into a standalone VM. #[cfg(test)] #[track_caller] -pub(crate) fn fresh_access() -> std::sync::Arc { - std::sync::Arc::new( - runvil_vfs::VfsRef::default() - .acquire(runvil_vfs::Origin::new("test fixture")) - .expect("the stock backend acquires"), - ) +pub(crate) fn fresh_access() -> (std::sync::Arc, runvil_vfs::ScopeHandle) { + let (access, scope) = runvil_vfs::VfsRef::default() + .acquire_scope(runvil_vfs::Origin::new("test fixture")) + .expect("the stock backend acquires"); + (std::sync::Arc::new(access), scope) } /// Performs one effect locally, with no I/O: a store operation runs on the diff --git a/crates/runvil-internal/engine/src/test_support/scenario-driving.rs b/crates/runvil-internal/engine/src/test_support/scenario-driving.rs index 3314319ef..2bcdc871f 100644 --- a/crates/runvil-internal/engine/src/test_support/scenario-driving.rs +++ b/crates/runvil-internal/engine/src/test_support/scenario-driving.rs @@ -411,13 +411,15 @@ impl TestStore { } pub(crate) fn read(&self, path: &str) -> Result { - let access = self.vfs().acquire(Origin::new("TestStore::read"))?; - runvil_vfs::detail::store_view(&access)?.read_string(path) + self.vfs() + .acquire_store(Origin::new("TestStore::read"))? + .read_string(path) } pub(crate) fn glob(&self, pattern: &str) -> Result, VfsError> { - let access = self.vfs().acquire(Origin::new("TestStore::glob"))?; - runvil_vfs::detail::store_view(&access)?.glob(pattern) + self.vfs() + .acquire_store(Origin::new("TestStore::glob"))? + .glob(pattern) } } diff --git a/crates/runvil-internal/lua/src/engine_globals-store.rs b/crates/runvil-internal/lua/src/engine_globals-store.rs index 4c1ccb05d..12a38dff8 100644 --- a/crates/runvil-internal/lua/src/engine_globals-store.rs +++ b/crates/runvil-internal/lua/src/engine_globals-store.rs @@ -10,7 +10,7 @@ use std::sync::{Arc, LazyLock, Mutex, PoisonError}; use mlua::{FromLuaMulti, Function, IntoLuaMulti, Lua, Table}; use runvil_types::emitter::Emitter; use runvil_types::event::lifecycle::{self, Lifecycle}; -use runvil_vfs::{Access, VfsError}; +use runvil_vfs::{Access, ScopeHandle, VfsError}; use crate::error::{self, Error, Result, author_prose}; use crate::error_value::store_error; @@ -234,7 +234,9 @@ fn record_store_conflict( /// error value of kind `store`, so it aborts the chunk and a `pcall` caller /// reads `err.kind`, `err.reason`, and the variant's fields. /// -/// Every closure captures an `Arc` clone of the section's store view. +/// Every closure captures an `Arc` clone of the section's store view, +/// which `scope`, the handle on the scope `access` belongs to, derives +/// once; the table keeps the view and never the handle. /// The view locks the backend per call and is synchronous, so nothing is /// held across an await. The view carries the caller's identity, which is /// what the claims model attributes operations to: a fanout arm's view is @@ -260,6 +262,7 @@ pub(crate) fn install_store_table( lua: &Lua, globals: &mlua::Table, access: &Arc, + scope: &ScopeHandle, emitter: &Emitter, section: &str, conflict: &Arc>>, @@ -270,7 +273,8 @@ pub(crate) fn install_store_table( // run on every operation through it. let shared = StoreShared { view: Arc::new( - runvil_vfs::detail::store_view(access) + scope + .store_view(access) .map_err(|cause| store_failure(&cause, store_probe_message(&cause)))?, ), reporter: Arc::new(StoreReporter { diff --git a/crates/runvil-internal/lua/src/test_vm.rs b/crates/runvil-internal/lua/src/test_vm.rs index cdb6404e4..71117591d 100644 --- a/crates/runvil-internal/lua/src/test_vm.rs +++ b/crates/runvil-internal/lua/src/test_vm.rs @@ -2,8 +2,10 @@ //! section setup builds one, through construction, the run's limits, //! Engine injection, and the persistent Engine globals, with the control //! globals and coroutine shims on request, handed back with named handles -//! to what the VM shares; the one-shot [`run_chunk`]; the tool fixtures as -//! data; and the scope a VM's offers resolve to. +//! to what the VM shares; the access fixture; the one-shot [`run_chunk`]; +//! the tool fixtures as data; and the scope a VM's offers resolve to. + +mod access; use std::collections::BTreeMap; use std::ops::Deref; @@ -15,7 +17,6 @@ use runvil_types::emitter::Emitter; use runvil_types::seed::RunSeed; use runvil_types::tools::{ToolCatalog, ToolId, ToolInfo}; use runvil_types::untrusted::GuardNonce; -use runvil_vfs::{Access, Origin, VfsRef}; use serde_json::{Value as Json, json}; use crate::error::{self, Result}; @@ -23,6 +24,8 @@ use crate::section::scalar_return; use crate::tests::recording::null_emitter; use crate::{Argv, OfferedTool, RosterEntry, SectionText, SectionVm, ToolSet}; +pub(crate) use access::{ScopedAccess, fresh_access}; + /// The one guard-nonce seed of every test VM. pub(crate) const TEST_SEED: RunSeed = RunSeed::from_bytes([0x73; 32]); @@ -36,18 +39,6 @@ pub(crate) fn test_nonce() -> GuardNonce { GuardNonce::from_seed(&TEST_SEED) } -/// A fresh default handle's access capability: the store is declared at -/// the root and the vended identity is the test's own, so seeding through -/// the store view and the VM's store ops never meet a second live identity. -#[track_caller] -pub(crate) fn fresh_access() -> Arc { - Arc::new( - VfsRef::default() - .acquire(Origin::new("lua test fixture")) - .expect("the stock backend acquires"), - ) -} - /// The result of running a section's Lua block with [`run_chunk`]. #[derive(Debug, Clone)] pub(crate) struct LuaOutcome { @@ -75,7 +66,7 @@ pub(crate) fn run_chunk( source: &str, args: &str, sys: &Json, - access: &Arc, + access: &ScopedAccess, emitter: &Emitter, section: &str, ) -> Result { @@ -84,9 +75,9 @@ pub(crate) fn run_chunk( emitter, section, )? - .inject_test_values(args, sys, access) + .inject_test_values(args, sys, &access.access) .map_err(|failure| failure.teardown(emitter, section))?; - vm.install_engine_globals(emitter, section)?; + vm.install_engine_globals(emitter, section, &access.scope)?; let returned: MultiValue = vm.lua().load(source).eval().map_err(error::lua)?; let returned = scalar_return(returned)?; let var = vm.var()?; @@ -247,7 +238,7 @@ type Selection = Arc>>; pub(crate) struct TestVm<'a> { tools: ToolSet, models: ModelSet, - access: Option>, + access: Option, emitter: Emitter, section: &'a str, args: &'a str, @@ -292,9 +283,10 @@ impl<'a> TestVm<'a> { self } - /// The VFS access the `store` table operates through. + /// The VFS access the `store` table operates through, beside the + /// handle on its scope. #[track_caller] - pub(crate) fn access(mut self, access: Arc) -> Self { + pub(crate) fn access(mut self, access: ScopedAccess) -> Self { self.access = Some(access); self } @@ -381,7 +373,7 @@ impl<'a> TestVm<'a> { #[track_caller] pub(crate) fn build(mut self) -> BuiltVm { let (mut vm, selection, access) = self.run_injection(); - vm.install_engine_globals(&self.emitter, self.section) + vm.install_engine_globals(&self.emitter, self.section, &access.scope) .expect("the Engine globals install"); if self.shims { vm.install_control_globals(|_| { @@ -404,7 +396,7 @@ impl<'a> TestVm<'a> { /// injection, handing back the VM beside its selection (which starts on /// the default binding, if any) and the access it shares. #[track_caller] - fn run_injection(&mut self) -> (SectionVm, Selection, Arc) { + fn run_injection(&mut self) -> (SectionVm, Selection, ScopedAccess) { let tools = Arc::new(std::mem::take(&mut self.tools)); let set = Arc::new(std::mem::take(&mut self.models)); let selection = Arc::new(Mutex::new(set.binding("default").cloned())); @@ -425,7 +417,7 @@ impl<'a> TestVm<'a> { .inject_values( self.args, self.sys.unwrap_or(&empty), - &access, + &access.access, self.var, self.argv, &selection, @@ -441,8 +433,9 @@ pub(crate) struct BuiltVm { vm: SectionVm, /// The section's current-model selection, which injection received. pub(crate) selection: Selection, - /// The access the VM's `store` table operates through. - pub(crate) access: Arc, + /// The access the VM's `store` table operates through, beside the + /// handle on its scope. + pub(crate) access: ScopedAccess, emitter: Emitter, section: String, } diff --git a/crates/runvil-internal/lua/src/test_vm/access.rs b/crates/runvil-internal/lua/src/test_vm/access.rs new file mode 100644 index 000000000..26b0fd4ce --- /dev/null +++ b/crates/runvil-internal/lua/src/test_vm/access.rs @@ -0,0 +1,37 @@ +//! The test VM's access fixture: a capability beside the handle on its +//! scope, the pair a section VM's setup takes. + +use std::sync::Arc; + +use runvil_vfs::{Access, Origin, ScopeHandle, VfsRef}; + +/// A test VM's access capability beside the handle on its scope, which +/// derives the access's store view. +#[derive(Clone)] +pub(crate) struct ScopedAccess { + /// The capability the VM's `store` table operates through. + pub(crate) access: Arc, + /// The handle on the capability's scope. + pub(crate) scope: ScopeHandle, +} + +impl ScopedAccess { + /// The root capability `vfs` acquires under `origin`, beside the handle + /// on the scope it starts. + #[track_caller] + pub(crate) fn acquire(vfs: &VfsRef, origin: Origin) -> ScopedAccess { + let (access, scope) = vfs.acquire_scope(origin).expect("the test handle acquires"); + ScopedAccess { + access: Arc::new(access), + scope, + } + } +} + +/// A fresh default handle's access capability: the store is declared at +/// the root and the vended identity is the test's own, so seeding through +/// the store view and the VM's store ops never meet a second live identity. +#[track_caller] +pub(crate) fn fresh_access() -> ScopedAccess { + ScopedAccess::acquire(&VfsRef::default(), Origin::new("lua test fixture")) +} diff --git a/crates/runvil-internal/lua/src/tests.rs b/crates/runvil-internal/lua/src/tests.rs index 958779539..93a274f1b 100644 --- a/crates/runvil-internal/lua/src/tests.rs +++ b/crates/runvil-internal/lua/src/tests.rs @@ -26,7 +26,7 @@ use crate::error::{Error, Result}; use crate::hardening::install_cancel_hook; use crate::section::resolve_section_target; use crate::test_vm::{ - BuiltVm, LuaOutcome, TEST_SECTION, TestVm, fresh_access, run_chunk, test_nonce, + BuiltVm, LuaOutcome, ScopedAccess, TEST_SECTION, TestVm, fresh_access, run_chunk, test_nonce, }; use crate::var::{guarded_var, var_snapshot_table, var_to_json}; use crate::vm::DEFAULT_LUA_LOG_EVENTS; @@ -54,8 +54,11 @@ mod tool_scoping; /// The store view derived from a test's access: the same view the `store` /// table's closures operate through, for seeding and extraction. -fn store_view(access: &Access) -> Access { - runvil_vfs::detail::store_view(access).expect("the handle declares a store") +fn store_view(access: &ScopedAccess) -> Access { + access + .scope + .store_view(&access.access) + .expect("the handle declares a store") } /// The fields every event carries ahead of its payload, sorted: its @@ -180,13 +183,10 @@ impl VfsAccess for FailingAccess { /// The access a failing backend vends, for tests driving the error path: /// the failing backend declared as the store, so the store view derives /// and every operation reaches the backend's refusal. -fn failing_access() -> Arc { - Arc::new( - VfsRef::builder() - .store("/", FailingBackend) - .build() - .acquire(Origin::new("failing backend test")) - .expect("the failing backend still acquires"), +fn failing_access() -> ScopedAccess { + ScopedAccess::acquire( + &VfsRef::builder().store("/", FailingBackend).build(), + Origin::new("failing backend test"), ) } @@ -203,7 +203,7 @@ fn run(source: &str, args: &str) -> Result { /// Runs a chunk against a caller-supplied access, so a test can inspect the /// store through the same identity after the chunk has run. -fn run_with(source: &str, access: &Arc) -> Result { +fn run_with(source: &str, access: &ScopedAccess) -> Result { run_chunk( source, "", diff --git a/crates/runvil-internal/lua/src/tests/section_vm.rs b/crates/runvil-internal/lua/src/tests/section_vm.rs index 93862000b..f47b26376 100644 --- a/crates/runvil-internal/lua/src/tests/section_vm.rs +++ b/crates/runvil-internal/lua/src/tests/section_vm.rs @@ -62,9 +62,9 @@ fn section_vm_preserves_one_environment_across_all_phases() { let vm = TestVm::new() .args("input") .sys(&sys) - .access(access) + .access(access.clone()) .inject(); - vm.install_engine_globals(&null_emitter(), "Test") + vm.install_engine_globals(&null_emitter(), "Test", &access.scope) .expect("Engine globals must install"); run_scalar(&vm, &first, &null_emitter(), "Test") .expect("the first chunk must run with the full environment"); @@ -106,7 +106,7 @@ fn a_refused_injection_hands_back_the_vm_for_its_teardown() { .inject_values( "", &json!({}), - &fresh_access(), + &fresh_access().access, Some(&json!(5)), Argv::Frozen(None), &Arc::new(Mutex::new(None)), @@ -168,6 +168,7 @@ fn section_vm_value_injection_bypasses_chunk_global_metatables() { args .. ',' .. type(fixtures_search) .. ',' .. tools.get('fixtures/search').id", ); let bindings = fixture_set(&[("search", "search the web")]); + let access = fresh_access(); let vm = SectionVm::new_for_section( &test_nonce(), &Arc::new(bindings), @@ -176,10 +177,10 @@ fn section_vm_value_injection_bypasses_chunk_global_metatables() { "Test", ) .expect("VM must build") - .inject_test_values("private input", &json!({}), &fresh_access()) + .inject_test_values("private input", &json!({}), &access.access) .expect("Engine values must inject"); let observer = null_emitter(); - vm.install_engine_globals(&observer, "Test") + vm.install_engine_globals(&observer, "Test", &access.scope) .expect("Engine globals must install"); run_scalar(&vm, &intercept, &null_emitter(), "Test").expect("the intercepting chunk must run"); @@ -196,12 +197,14 @@ fn section_vm_reports_store_operations_in_each_chunk() { let write = program("store.write('state.txt', args)"); let read = program("return store.read('state.txt')"); let recorder = Recorder::default(); + let access = fresh_access(); let vm = TestVm::new() .section("Gather") .args("private input") + .access(access.clone()) .inject(); let observer = recorder.emitter().clone(); - vm.install_engine_globals(&observer, "Gather") + vm.install_engine_globals(&observer, "Gather", &access.scope) .expect("Engine globals must install"); run_scalar(&vm, &write, recorder.emitter(), "Gather").expect("first chunk write must run"); @@ -256,7 +259,7 @@ fn section_vms_isolate_mutated_globals() { let seed = program("counter = 0"); let increment = program("counter = counter + 1; return counter"); let access = fresh_access(); - let first = TestVm::new().access(Arc::clone(&access)).build(); + let first = TestVm::new().access(access.clone()).build(); run_scalar(&first, &seed, &null_emitter(), "First").expect("first VM must seed"); let second = TestVm::new().access(access).build(); run_scalar(&second, &seed, &null_emitter(), "Second").expect("second VM must seed"); @@ -287,12 +290,13 @@ fn section_lifecycle_reports_are_ordered_exact_and_payload_free() { let prologue = program("var.value = args"); let epilog = program("return 'epilog done'"); let recorder = Recorder::default(); + let access = fresh_access(); let vm = SectionVm::new(&test_nonce(), recorder.emitter(), "Gather") .expect("VM must build") - .inject_test_values("private input", &json!({}), &fresh_access()) + .inject_test_values("private input", &json!({}), &access.access) .expect("Engine values must inject"); let observer = recorder.emitter().clone(); - vm.install_engine_globals(&observer, "Gather") + vm.install_engine_globals(&observer, "Gather", &access.scope) .expect("Engine globals must install"); run_scalar(&vm, &first, recorder.emitter(), "Gather").expect("the first chunk must run"); run_scalar(&vm, &prologue, recorder.emitter(), "Gather").expect("prologue must run"); diff --git a/crates/runvil-internal/lua/src/tests/store_errors.rs b/crates/runvil-internal/lua/src/tests/store_errors.rs index b867a8758..9e23cce84 100644 --- a/crates/runvil-internal/lua/src/tests/store_errors.rs +++ b/crates/runvil-internal/lua/src/tests/store_errors.rs @@ -6,14 +6,13 @@ use super::*; #[test] fn a_store_install_without_a_store_names_why_in_its_message() { - let access = Arc::new( - VfsRef::new(runvil_vfs::MemoryBackend::new()) - .acquire(Origin::new("no store test")) - .expect("a handle without a store still acquires"), + let access = ScopedAccess::acquire( + &VfsRef::new(runvil_vfs::MemoryBackend::new()), + Origin::new("no store test"), ); - let vm = TestVm::new().access(access).inject(); + let vm = TestVm::new().access(access.clone()).inject(); let error = vm - .install_engine_globals(&null_emitter(), "Test") + .install_engine_globals(&null_emitter(), "Test", &access.scope) .expect_err("a handle without a store cannot install the store table"); assert_eq!( error.code, diff --git a/crates/runvil-internal/lua/src/tests/store_reports.rs b/crates/runvil-internal/lua/src/tests/store_reports.rs index 28bb34107..c86c4e4b8 100644 --- a/crates/runvil-internal/lua/src/tests/store_reports.rs +++ b/crates/runvil-internal/lua/src/tests/store_reports.rs @@ -106,12 +106,12 @@ fn every_store_operation_reports_its_exact_success_and_failure() { source: &'static str, success: Observation, failure: Observation, - prepare: fn(&Arc), + prepare: fn(&ScopedAccess), } - fn empty(_access: &Arc) {} + fn empty(_access: &ScopedAccess) {} - fn existing(access: &Arc) { + fn existing(access: &ScopedAccess) { store_view(access) .write("a.txt", b"old") .expect("the memory store can prepare a file"); diff --git a/crates/runvil-internal/lua/src/vm/install.rs b/crates/runvil-internal/lua/src/vm/install.rs index c8748e0a0..4bf866d26 100644 --- a/crates/runvil-internal/lua/src/vm/install.rs +++ b/crates/runvil-internal/lua/src/vm/install.rs @@ -8,6 +8,7 @@ use std::sync::{Arc, PoisonError}; use mlua::Value; use runvil_types::emitter::Emitter; +use runvil_vfs::ScopeHandle; use serde_json::Value as Json; use super::{JUMP_SLOT_POISONED, SectionVm, pack_sequence}; @@ -29,7 +30,9 @@ impl SectionVm { /// Called once after Engine injection. /// The closures capture owned strings, a clone of the emitter, and Arc /// clones of the log budget counters and the store view, so they stay - /// valid across every chunk this VM runs. + /// valid across every chunk this VM runs. `scope` is the handle on the + /// scope of the access Engine injection received: it derives the store + /// view, and the VM keeps none of it. /// The direct store closures record a claims-model conflict into the /// VM's slot for the executor to read with /// [`take_store_conflict`](Self::take_store_conflict). @@ -37,7 +40,16 @@ impl SectionVm { /// # Errors /// Returns a `LuaRuntime` failure if the globals cannot be installed, or /// the store's failure if the handle declares no store. - pub fn install_engine_globals(&self, emitter: &Emitter, section: &str) -> Result<()> { + /// + /// # Panics + /// Panics when the injected access belongs to another scope than + /// `scope`'s. + pub fn install_engine_globals( + &self, + emitter: &Emitter, + section: &str, + scope: &ScopeHandle, + ) -> Result<()> { install_log( &self.lua, emitter, @@ -49,6 +61,7 @@ impl SectionVm { &self.lua, &self.lua.globals(), &self.state.access, + scope, emitter, section, &self.store_conflict, diff --git a/crates/runvil-internal/vfs/src/detail.rs b/crates/runvil-internal/vfs/src/detail.rs deleted file mode 100644 index 88df1a787..000000000 --- a/crates/runvil-internal/vfs/src/detail.rs +++ /dev/null @@ -1,129 +0,0 @@ -//! Operations on [`Access`] that only the Engine performs. -//! -//! The `runvil` facade never re-exports this module, so only Engine -//! crates reach it: the caller passes a run's capability through, and -//! the Engine alone forks it for concurrent arms and for each tool call, -//! joins an arm's identity back on delivery, ends a tool call's identity -//! when the call is answered or aborted, derives the store view from a -//! chain's access for store calls, and ends the run's scope when the run -//! ends. The -//! caller, holding the handle, acquires a store view in a scope of its own -//! with [`VfsRef::acquire_store`](crate::VfsRef::acquire_store). - -use std::fmt; -use std::sync::{Arc, Weak}; - -use crate::handle::Scope; -use crate::{Access, ExecId, Origin, VfsError}; - -/// An opaque handle on the scope an [`Access`] belongs to, which the -/// Engine holds for the life of a run so [`end_scope`] can close it. It -/// never keeps the scope alive. -#[derive(Clone)] -pub struct ScopeHandle(Weak); - -impl fmt::Debug for ScopeHandle { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.debug_tuple("ScopeHandle").finish_non_exhaustive() - } -} - -/// The handle on `access`'s scope: the scope of every identity spawned -/// from it, however deep. -#[must_use] -pub fn scope_handle(access: &Access) -> ScopeHandle { - ScopeHandle(Arc::downgrade(access.scope())) -} - -/// Ends the scope behind `scope`: its claims stop conflicting at once, -/// and every access still held in it - a store view the caller kept past the -/// run, a forwarded mount session - refuses its next operation, spawn, -/// or store view with [`VfsError::ScopeEnded`]. Idempotent, and a -/// no-op once every access in the scope has dropped. -pub fn end_scope(scope: &ScopeHandle) { - if let Some(scope) = scope.0.upgrade() { - scope.close(); - } -} - -/// Derives the store view from a chain's [`Access`]: an ordinary -/// [`Access`] rooted at the handle's declared store root, whose -/// operations reach the store's own mount alone. The view keeps the -/// chain's identity and scope - so one chain never conflicts with -/// itself through its view - applies the store's strict logical-path -/// rules, and reports error paths as the caller supplied them. -/// -/// # Errors -/// Returns [`VfsError::NoStore`] when the handle the chain's access came -/// from declares no store. -pub fn store_view(access: &Access) -> Result { - access.store_view() -} - -/// Probes the declared store before a run starts: derives the store -/// view from `access` and stats the store root through it, which makes -/// the view's router acquire the store backend. A root the backend -/// reports [`VfsError::NotFound`] passes, so a real directory created -/// lazily still runs. -/// -/// # Errors -/// Returns [`VfsError::NoStore`] when the handle declares no store, or -/// an error when the store backend refuses the session or fails the stat. -pub fn probe_store(access: &Access) -> Result<(), VfsError> { - access.store_view()?.probe_root() -} - -/// Returns the capability for a new concurrent thread of execution. -/// -/// The child gets a fresh [`ExecId`] in `parent`'s scope, -/// and the spawn is the fork: the child shares a snapshot of the parent's -/// vector clock plus its own entry, while the parent's entry advances, so -/// the parent's later accesses are not ordered before the child's. -/// Everything the parent did before the spawn happens before the child's -/// first step. `origin` labels the child's operation events, as in -/// [`VfsRef::acquire`](crate::VfsRef::acquire). -/// -/// # Errors -/// Returns an error when the backend refuses to acquire the child's -/// identity. A failed spawn leaves `parent`'s clock untouched. -pub fn access_spawn(parent: &Access, origin: Origin) -> Result { - parent.spawn(origin) -} - -/// Merges `child`'s final clock into `owner`'s: the join, the matching -/// edge of [`access_spawn`]'s fork. Everything `child` did - its writes -/// included - happens before `owner`'s next step, so the owner's reads of -/// the child's files no longer conflict. A child whose identity is still -/// live (its last access not yet dropped) contributes its current clock; -/// a child whose last access dropped contributes its final clock, which -/// the scope keeps for exactly this. A child ended through [`end_access`] -/// keeps no seen snapshot, so joining it again contributes only its own -/// entry and fork chain. Joining a timer's identity - one the Engine -/// never spawned - is a no-op because no such identity exists. -pub fn access_join(owner: &Access, child: ExecId) { - owner.join(child); -} - -/// Ends `child`, the identity of one tool call, in the scope behind -/// `scope`: merges its clock into `owner`'s, as [`access_join`] does, -/// when `owner` is `Some`, and drops its seen snapshot. Every access -/// still holding `child` - the tool's own, a store view derived from it - -/// refuses its next operation, spawn, or store view with -/// [`VfsError::ScopeEnded`] while the scope stays open. A no-op -/// once every access in the scope has dropped. -pub fn end_access(scope: &ScopeHandle, owner: Option, child: ExecId) { - if let Some(scope) = scope.0.upgrade() { - scope.end(owner, child); - } -} - -/// The identity `access` was acquired or spawned under: how the Engine -/// records a task's [`ExecId`] at spawn, to join it on delivery. -#[must_use] -pub const fn access_id(access: &Access) -> ExecId { - access.exec_id() -} - -#[cfg(test)] -#[path = "detail-tests.rs"] -mod tests; diff --git a/crates/runvil-internal/vfs/src/handle.rs b/crates/runvil-internal/vfs/src/handle.rs index eafae3d9d..89e02ede5 100644 --- a/crates/runvil-internal/vfs/src/handle.rs +++ b/crates/runvil-internal/vfs/src/handle.rs @@ -13,14 +13,14 @@ //! # Scopes, fork and join //! //! [`VfsRef::acquire`] starts a *scope*: the acquired [`Access`] is the -//! root identity, and every identity [`Access::spawn`] forks joins the -//! scope. Each identity holds a vector clock, and every admitted access +//! root identity, and every identity [`ScopeHandle::spawn`] forks joins +//! the scope. Each identity holds a vector clock, and every admitted access //! records an *epoch* - its identity paired with its own clock entry at //! that moment - on each region it touches. One epoch is ordered before //! another identity's next step exactly when the other's clock has seen //! the epoch's entry, so conflicts follow happens-before (FastTrack-style, //! Flanagan and Freund, PLDI 2009) instead of liveness: a spawn forks the -//! parent's clock into the child, and a join - [`crate::detail::access_join`] - +//! parent's clock into the child, and a join - [`ScopeHandle::join`] - //! merges the child's final clock back into the owner's. An identity's own //! entry lives outside the shared snapshot, so a spawn reuses the parent's //! map read-only and records the parent's entry as one frozen fork step: @@ -28,7 +28,7 @@ //! released during a scope's life. An identity ends when its last //! [`Access`] drops, its final clock stays in the scope for late joins, //! and a scope ends with its last identity or when its run closes it -//! through [`crate::detail::end_scope`], whichever comes first; its +//! through [`ScopeHandle::end`], whichever comes first; its //! claims are then ignored and purged lazily, and a closed scope's //! accesses refuse every later operation. Two live scopes never order each other, so their //! claims always conflict. See the reference docs on the facade for the @@ -45,6 +45,7 @@ mod operations; mod prune; mod region; mod scope; +mod scope_handle; mod store_view; #[cfg(test)] mod tests; @@ -61,6 +62,7 @@ use crate::traits::{AcquireContext, AllowAll, ExecId, Policy, Vfs, VfsAccess}; pub(crate) use case_fold::CaseFolds; use claims::Claims; pub(crate) use scope::Scope; +pub use scope_handle::ScopeHandle; /// One mounted filesystem instance: its backend and the ledger of who is /// touching what. The two are separately `Arc`-shareable so `overlay()` @@ -204,6 +206,22 @@ impl VfsRef { ) } + /// Acquires an `Access` capability for a new serial thread of + /// execution, beside the [`ScopeHandle`] on the scope it starts. + /// + /// This is [`VfsRef::acquire`] plus the authority over the new scope: + /// the scope handle forks, joins, and ends the scope's identities, + /// derives their store views, and ends the scope. No other call returns + /// a scope handle, so an `Access` alone never carries that authority. + /// + /// # Errors + /// Returns an error when the backend refuses to acquire the identity. + pub fn acquire_scope(&self, origin: Origin) -> Result<(Access, ScopeHandle), VfsError> { + let access = self.acquire(origin)?; + let scope = ScopeHandle::new(&access.scope); + Ok((access, scope)) + } + /// Acquires a store view for a new serial thread of execution. /// /// The store view is an [`Access`] rooted at the declared store root. diff --git a/crates/runvil-internal/vfs/src/handle/access.rs b/crates/runvil-internal/vfs/src/handle/access.rs index 2d1b5f3ae..cf005a1f1 100644 --- a/crates/runvil-internal/vfs/src/handle/access.rs +++ b/crates/runvil-internal/vfs/src/handle/access.rs @@ -4,12 +4,12 @@ use std::sync::{Arc, Mutex, MutexGuard, PoisonError}; use super::Access; -#[cfg(doc)] -use super::VfsRef; use super::claims::Claims; use super::forward::StoreScoped; use super::scope::Scope; use super::store_view::{relativize_error, validate_store_path}; +#[cfg(doc)] +use super::{ScopeHandle, VfsRef}; use crate::error::VfsError; use crate::observe::{OpEvent, Origin}; use crate::path::{VfsPath, canonicalize}; @@ -20,9 +20,9 @@ impl Access { /// The spawn is the fork: the child gets a fresh [`ExecId`] and a /// shared snapshot of this capability's clock plus its own entry, /// while this capability's entry advances, so its later accesses are - /// not ordered before the child's. [`crate::detail::access_join`] is - /// the matching join. `origin` labels the child's operation events, - /// as in [`VfsRef::acquire`]. + /// not ordered before the child's. [`ScopeHandle::join`] is the + /// matching join. `origin` labels the child's operation events, as in + /// [`VfsRef::acquire`]. /// /// # Errors /// Returns an error when the backend refuses to acquire the child's @@ -32,7 +32,7 @@ impl Access { /// never ran. Returns [`VfsError::ScopeEnded`], before the fork, when /// the run or the tool call that owned this capability has ended. /// - /// Crate-internal: backs [`crate::detail::access_spawn`]. + /// Crate-internal: backs [`ScopeHandle::spawn`]. pub(crate) fn spawn(&self, origin: Origin) -> Result { self.scope .refuse_if_closed(&self.root) @@ -78,23 +78,13 @@ impl Access { } /// The identity every operation through this capability is - /// attributed to. Crate-internal: the Engine reads it through - /// [`crate::detail::access_id`]. - pub(crate) const fn exec_id(&self) -> ExecId { + /// attributed to, as [`ScopeHandle::join`] and + /// [`ScopeHandle::end_access`] name it. + #[must_use] + pub const fn id(&self) -> ExecId { self.id } - /// The scope this identity belongs to. Crate-internal: backs - /// [`crate::detail::scope_handle`]. - pub(crate) const fn scope(&self) -> &Arc { - &self.scope - } - - /// Crate-internal: backs [`crate::detail::access_join`]. - pub(crate) fn join(&self, child: ExecId) { - self.scope.join(self.id, child); - } - /// The claims-table half of one operation: refuses once the run that /// owned this access has ended, runs `claim` against this access's /// identity and scope on `path`'s claim key - whose view refuses once diff --git a/crates/runvil-internal/vfs/src/handle/scope_handle.rs b/crates/runvil-internal/vfs/src/handle/scope_handle.rs new file mode 100644 index 000000000..516462ce8 --- /dev/null +++ b/crates/runvil-internal/vfs/src/handle/scope_handle.rs @@ -0,0 +1,157 @@ +//! [`ScopeHandle`], the authority over one scope: forking, joining, and +//! ending its identities, deriving their store views, and ending the +//! scope itself. + +use std::fmt; +use std::sync::{Arc, Weak}; + +#[cfg(doc)] +use super::VfsRef; +use super::{Access, Scope}; +use crate::error::VfsError; +use crate::observe::Origin; +use crate::traits::ExecId; + +/// The authority over one scope, which [`VfsRef::acquire_scope`] returns +/// beside the scope's root [`Access`]. +/// +/// No other call makes one, and an `Access` never yields one, so code that +/// holds only accesses - a tool given its call's access, a caller given a +/// store view - cannot fork, join, or end an identity, derive a store +/// view, or end the scope. Every method that takes an `Access` panics on +/// one from another scope, so a scope handle acts on its own scope alone. +/// A scope handle never keeps its scope alive. +#[derive(Clone)] +pub struct ScopeHandle(Weak); + +impl fmt::Debug for ScopeHandle { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_tuple("ScopeHandle").finish_non_exhaustive() + } +} + +impl ScopeHandle { + /// The scope handle on `scope`, for the acquisition that started it. + pub(super) fn new(scope: &Arc) -> ScopeHandle { + ScopeHandle(Arc::downgrade(scope)) + } + + /// Returns the capability for a new concurrent thread of execution, + /// forked from `parent`. + /// + /// The child gets a fresh [`ExecId`] in this scope, and the spawn is + /// the fork: the child shares a snapshot of the parent's vector clock + /// plus its own entry, while the parent's entry advances, so the + /// parent's later accesses are not ordered before the child's. + /// Everything the parent did before the spawn happens before the + /// child's first step. `origin` labels the child's operation events, + /// as in [`VfsRef::acquire`]. + /// + /// # Errors + /// Returns [`VfsError::ScopeEnded`] when the scope or `parent`'s tool + /// call has ended, or an error when the backend refuses to acquire the + /// child's identity. A refused acquire still advances `parent`'s own + /// clock entry, which orders nothing new. + /// + /// # Panics + /// Panics when `parent` belongs to another scope. + #[track_caller] + pub fn spawn(&self, parent: &Access, origin: Origin) -> Result { + self.assert_governs(parent); + parent.spawn(origin) + } + + /// Merges `child`'s final clock into `owner`'s: the join, the matching + /// edge of [`ScopeHandle::spawn`]'s fork. Everything `child` did - its + /// writes included - happens before `owner`'s next step, so the owner's + /// reads of the child's files no longer conflict. A child whose identity + /// is still live contributes its current clock; a child whose last + /// access dropped contributes its final clock, which the scope keeps for + /// exactly this. A child ended through [`ScopeHandle::end_access`] keeps + /// no seen snapshot, so joining it again contributes only its own entry + /// and fork chain. An identity this scope never held - a timer's, or + /// another scope's - joins nothing. + pub fn join(&self, owner: ExecId, child: ExecId) { + if let Some(scope) = self.0.upgrade() { + scope.join(owner, child); + } + } + + /// Ends `child`, the identity of one tool call: merges its clock into + /// `owner`'s, as [`ScopeHandle::join`] does, when `owner` is `Some`, and + /// drops its seen snapshot. Every access still holding `child` - the + /// tool's own, a store view derived from it - refuses its next + /// operation, spawn, or store view with [`VfsError::ScopeEnded`] while + /// the scope stays open. An identity this scope never held is left + /// alone, and the call is a no-op once every access in the scope has + /// dropped. + pub fn end_access(&self, owner: Option, child: ExecId) { + if let Some(scope) = self.0.upgrade() { + scope.end(owner, child); + } + } + + /// Derives the store view from `access`: an ordinary [`Access`] rooted + /// at the declared store root of the [`VfsRef`] `access` came from, + /// whose operations reach the store's own mount alone. The view keeps + /// `access`'s identity and scope - so one chain never conflicts with + /// itself through its view - applies the store's strict logical-path + /// rules, and reports error paths as the caller supplied them. + /// + /// # Errors + /// Returns [`VfsError::NoStore`] when that `VfsRef` declares no store, + /// or [`VfsError::ScopeEnded`] when the scope or `access`'s tool call + /// has ended. + /// + /// # Panics + /// Panics when `access` belongs to another scope. + #[track_caller] + pub fn store_view(&self, access: &Access) -> Result { + self.assert_governs(access); + access.store_view() + } + + /// Probes the declared store before a run starts: derives the store + /// view from `access` and stats the store root through it, which makes + /// the view's router acquire the store backend. A root the backend + /// reports [`VfsError::NotFound`] passes, so a real directory created + /// lazily still runs. + /// + /// # Errors + /// Returns [`VfsError::NoStore`] when the [`VfsRef`] `access` came from + /// declares no store, or an error when the store backend refuses the + /// session or fails the stat. + /// + /// # Panics + /// Panics when `access` belongs to another scope. + #[track_caller] + pub fn probe_store(&self, access: &Access) -> Result<(), VfsError> { + self.assert_governs(access); + access.store_view()?.probe_root() + } + + /// Ends the scope: its claims stop conflicting at once, and every + /// access still held in it - a store view the caller kept past the run, + /// a forwarded mount session - refuses its next operation, spawn, or + /// store view with [`VfsError::ScopeEnded`]. Idempotent, and a no-op + /// once every access in the scope has dropped. + pub fn end(&self) { + if let Some(scope) = self.0.upgrade() { + scope.close(); + } + } + + /// Asserts that `access` belongs to this scope handle's scope. + #[track_caller] + fn assert_governs(&self, access: &Access) { + assert!( + self.0 + .upgrade() + .is_some_and(|scope| Arc::ptr_eq(&scope, &access.scope)), + "the access belongs to another scope than this scope handle's" + ); + } +} + +#[cfg(test)] +mod tests; diff --git a/crates/runvil-internal/vfs/src/detail-tests.rs b/crates/runvil-internal/vfs/src/handle/scope_handle/tests.rs similarity index 80% rename from crates/runvil-internal/vfs/src/detail-tests.rs rename to crates/runvil-internal/vfs/src/handle/scope_handle/tests.rs index 9313ff5b4..a042b00fd 100644 --- a/crates/runvil-internal/vfs/src/detail-tests.rs +++ b/crates/runvil-internal/vfs/src/handle/scope_handle/tests.rs @@ -1,5 +1,4 @@ -use super::{access_spawn, end_scope, probe_store, scope_handle, store_view}; -use crate::test_support::{TestFs, access, handle}; +use crate::test_support::{TestFs, access, handle, test_origin}; use crate::{Access, MemoryBackend, Origin, PathReason, VfsError, VfsRef}; /// A base at `/` beside a store declared at `/my/store`: the shape @@ -14,8 +13,12 @@ fn stock() -> VfsRef { /// The chain's access and its store view, in one scope. #[track_caller] fn chain(vfs: &VfsRef) -> (Access, Access) { - let access = access(vfs).expect("the stock backend acquires"); - let view = store_view(&access).expect("the handle declares a store"); + let (access, scope) = vfs + .acquire_scope(test_origin()) + .expect("the stock backend acquires"); + let view = scope + .store_view(&access) + .expect("the handle declares a store"); (access, view) } @@ -23,17 +26,18 @@ fn chain(vfs: &VfsRef) -> (Access, Access) { fn an_ended_scope_refuses_spawns_views_and_arm_operations_and_frees_its_claims() -> Result<(), VfsError> { let vfs = stock(); - let (access, view) = chain(&vfs); + let (access, scope) = vfs.acquire_scope(test_origin())?; + let view = scope.store_view(&access)?; view.write("a.txt", b"run")?; - let arm = access_spawn(&access, Origin::new("arm"))?; - end_scope(&scope_handle(&access)); + let arm = scope.spawn(&access, Origin::new("arm"))?; + scope.end(); let ended = |result: Result| match result { Err(VfsError::ScopeEnded { path }) => assert_eq!(path, "/"), other => panic!("expected an ended-run refusal, got {other:?}"), }; - ended(access_spawn(&access, Origin::new("late arm"))); - ended(store_view(&access)); - ended(store_view(&arm)); + ended(scope.spawn(&access, Origin::new("late arm"))); + ended(scope.store_view(&access)); + ended(scope.store_view(&arm)); match arm.read("/my/store/a.txt") { Err(VfsError::ScopeEnded { path }) => assert_eq!(path, "/my/store/a.txt"), other => panic!("expected an ended-run refusal, got {other:?}"), @@ -50,6 +54,63 @@ fn an_ended_scope_refuses_spawns_views_and_arm_operations_and_frees_its_claims() Ok(()) } +#[test] +#[should_panic(expected = "the access belongs to another scope than this scope handle's")] +fn a_scope_handle_refuses_to_fork_another_scopes_access() { + let vfs = stock(); + let (_mine, scope) = vfs + .acquire_scope(test_origin()) + .expect("the stock backend acquires"); + let theirs = access(&vfs).expect("the stock backend acquires"); + let _refused = scope.spawn(&theirs, test_origin()); +} + +#[test] +#[should_panic(expected = "the access belongs to another scope than this scope handle's")] +fn a_scope_handle_refuses_a_store_view_of_another_scopes_access() { + let vfs = stock(); + let (_mine, scope) = vfs + .acquire_scope(test_origin()) + .expect("the stock backend acquires"); + let theirs = access(&vfs).expect("the stock backend acquires"); + let _refused = scope.store_view(&theirs); +} + +#[test] +#[should_panic(expected = "the access belongs to another scope than this scope handle's")] +fn a_scope_handle_refuses_to_probe_through_another_scopes_access() { + let vfs = stock(); + let (_mine, scope) = vfs + .acquire_scope(test_origin()) + .expect("the stock backend acquires"); + let theirs = access(&vfs).expect("the stock backend acquires"); + let _refused = scope.probe_store(&theirs); +} + +#[test] +fn a_scope_handle_leaves_every_other_scope_alone() -> Result<(), VfsError> { + let vfs = stock(); + let (_mine, mine) = vfs.acquire_scope(test_origin())?; + let (theirs, their_scope) = vfs.acquire_scope(test_origin())?; + let call = their_scope.spawn(&theirs, test_origin())?; + call.write("/x.txt", b"call")?; + // Another scope's handle names identities it never held: its join + // orders nothing, and its ends end nothing. + mine.join(theirs.id(), call.id()); + mine.end_access(Some(theirs.id()), call.id()); + mine.end(); + assert!(matches!( + theirs.read("/x.txt"), + Err(VfsError::Conflict { .. }) + )); + call.write("/y.txt", b"still live")?; + // The scope's own handle orders the call's writes before the root's + // next step. + their_scope.join(theirs.id(), call.id()); + assert_eq!(theirs.read("/x.txt")?, b"call"); + Ok(()) +} + #[test] fn each_strict_path_rule_is_reported_in_check_order() { let vfs = stock(); @@ -129,8 +190,7 @@ fn errors_report_paths_in_the_callers_relative_form() -> Result<(), VfsError> { view.write("f.txt", b"one")?; // A second chain's write races, and the diagnosis names the // logical path, never the canonical one. - let other_access = access(&vfs)?; - let other = store_view(&other_access).expect("the handle declares a store"); + let (_other_access, other) = chain(&vfs); match other.write("f.txt", b"two") { Err(VfsError::Conflict { path, detail }) => { assert_eq!(path, "f.txt"); @@ -204,10 +264,12 @@ fn the_store_probe_acquires_the_store_backend() { }) }); let vfs = VfsRef::builder().store("/my/store", refusing).build(); - let access = access(&vfs).expect("the router acquires lazily"); + let (access, scope) = vfs + .acquire_scope(test_origin()) + .expect("the router acquires lazily"); // Deriving the view alone never reaches the store backend. - assert!(store_view(&access).is_ok()); - match probe_store(&access) { + assert!(scope.store_view(&access).is_ok()); + match scope.probe_store(&access) { Err(VfsError::Backend { message }) => assert!(message.contains("refuses"), "{message}"), other => panic!("expected the backend's refusal, got {other:?}"), } @@ -221,21 +283,22 @@ fn the_store_probe_treats_a_missing_store_root_as_success() -> Result<(), VfsErr .mount("/data", MemoryBackend::new()) .build(); let vfs = VfsRef::builder().store("/", inner).build(); - let access = access(&vfs)?; + let (access, scope) = vfs.acquire_scope(test_origin())?; assert!(matches!(access.stat("/"), Err(VfsError::NotFound { .. }))); - probe_store(&access) + scope.probe_store(&access) } #[test] fn one_chain_never_conflicts_with_itself_through_its_view() -> Result<(), VfsError> { let vfs = stock(); - let (access, view) = chain(&vfs); + let (access, scope) = vfs.acquire_scope(test_origin())?; + let view = scope.store_view(&access)?; view.write("a.txt", b"one")?; view.append("a.txt", b"two")?; assert_eq!(view.read("a.txt")?, b"onetwo"); // A second view over the same chain shares the identity and the // scope, so its operations never race the first view's. - let also = store_view(&access).expect("the handle declares a store"); + let also = scope.store_view(&access)?; assert_eq!(also.read("a.txt")?, b"onetwo"); also.write("b.txt", b"three")?; // The chain's plain capability reads the same storage. @@ -352,10 +415,8 @@ fn the_outermost_declaration_counts() -> Result<(), VfsError> { // itself: the mounting handle declares no store, so no view. let inner = VfsRef::builder().store("/s", MemoryBackend::new()).build(); let outer = VfsRef::builder().mount("/inner", inner).build(); - assert!(matches!( - store_view(&access(&outer)?), - Err(VfsError::NoStore) - )); + let (access, scope) = outer.acquire_scope(test_origin())?; + assert!(matches!(scope.store_view(&access), Err(VfsError::NoStore))); // The outer builder's own declaration is the one the view uses. let inner = VfsRef::builder().store("/s", MemoryBackend::new()).build(); @@ -363,8 +424,10 @@ fn the_outermost_declaration_counts() -> Result<(), VfsError> { .mount("/inner", inner) .store("/outer-store", MemoryBackend::new()) .build(); - let access = access(&outer)?; - let view = store_view(&access).expect("the outer handle declares a store"); + let (access, scope) = outer.acquire_scope(test_origin())?; + let view = scope + .store_view(&access) + .expect("the outer handle declares a store"); view.write("a.txt", b"outer")?; drop(view); assert!(access.exists("/outer-store/a.txt")?); diff --git a/crates/runvil-internal/vfs/src/handle/store_view.rs b/crates/runvil-internal/vfs/src/handle/store_view.rs index 6db99cb0e..b7aea97f1 100644 --- a/crates/runvil-internal/vfs/src/handle/store_view.rs +++ b/crates/runvil-internal/vfs/src/handle/store_view.rs @@ -213,7 +213,8 @@ impl Access { /// this access's identity, scope, claims table, policy, and op /// sink. The view applies the store's strict logical-path rules /// and reports error paths in the caller's logical form. - /// Crate-internal: backs [`crate::detail::store_view`]. + /// Crate-internal: backs [`crate::ScopeHandle::store_view`] and + /// [`crate::VfsRef::acquire_store`]. /// /// # Errors /// Returns [`VfsError::NoStore`] when the handle declares no store, @@ -276,7 +277,7 @@ impl Access { /// with no policy check, claim, or event, so a lazily acquiring /// router opens the serving mount's session. A root the backend /// reports missing passes. Crate-internal: backs - /// [`crate::detail::probe_store`]. + /// [`crate::ScopeHandle::probe_store`]. pub(crate) fn probe_root(&self) -> Result<(), VfsError> { match self.inner().stat(&self.root) { Ok(_) | Err(VfsError::NotFound { .. }) => Ok(()), diff --git a/crates/runvil-internal/vfs/src/handle/tests.rs b/crates/runvil-internal/vfs/src/handle/tests.rs index 042ee8db9..9108f8a17 100644 --- a/crates/runvil-internal/vfs/src/handle/tests.rs +++ b/crates/runvil-internal/vfs/src/handle/tests.rs @@ -7,7 +7,7 @@ use std::sync::atomic::Ordering; use std::sync::{Arc, Mutex, PoisonError}; use super::claims::PRUNE_AT; -use super::{Access, VfsRef}; +use super::{Access, ScopeHandle, VfsRef}; use crate::error::VfsError; use crate::observe::{OpEvent, Origin}; use crate::path::VfsPath; diff --git a/crates/runvil-internal/vfs/src/handle/tests/happens_before.rs b/crates/runvil-internal/vfs/src/handle/tests/happens_before.rs index a6dd41b92..ae4170215 100644 --- a/crates/runvil-internal/vfs/src/handle/tests/happens_before.rs +++ b/crates/runvil-internal/vfs/src/handle/tests/happens_before.rs @@ -96,7 +96,7 @@ fn an_ended_identity_keeps_no_clock_snapshot() -> Result<(), VfsError> { arm.write("/arm.txt", b"arm")?; let arm_id = arm.id; drop(arm); - parent.join(arm_id); + parent.scope.join(parent.id, arm_id); let child = parent.spawn(test_origin())?; child.write("/child.txt", b"child")?; let child_id = child.id; @@ -140,7 +140,7 @@ fn a_join_makes_a_finished_arms_writes_readable() -> Result<(), VfsError> { arm_one.write("/evidence.md", b"one\n")?; let arm_one_id = arm_one.id; drop(arm_one); - parent.join(arm_one_id); + parent.scope.join(parent.id, arm_one_id); let arm_two = parent.spawn(test_origin())?; arm_two.append("/evidence.md", b"two\n")?; assert_eq!(arm_two.read("/evidence.md")?, b"one\ntwo\n"); diff --git a/crates/runvil-internal/vfs/src/handle/tests/operations.rs b/crates/runvil-internal/vfs/src/handle/tests/operations.rs index 7767736b5..3a03239ea 100644 --- a/crates/runvil-internal/vfs/src/handle/tests/operations.rs +++ b/crates/runvil-internal/vfs/src/handle/tests/operations.rs @@ -80,4 +80,5 @@ fn the_handle_and_capability_are_send_and_sync() { assert_send_sync::(); assert_send_sync::(); assert_send_sync::(); + assert_send_sync::(); } diff --git a/crates/runvil-internal/vfs/src/handle/tests/refusals.rs b/crates/runvil-internal/vfs/src/handle/tests/refusals.rs index 6d7ce42c7..6d0cc4390 100644 --- a/crates/runvil-internal/vfs/src/handle/tests/refusals.rs +++ b/crates/runvil-internal/vfs/src/handle/tests/refusals.rs @@ -7,7 +7,6 @@ use std::sync::atomic::AtomicUsize; use super::*; use crate::MemoryBackend; -use crate::detail::{end_access, scope_handle}; /// The error a refusing backend's acquire returns: the trait's contract /// allows refusal, so the handle must surface it as an error rather @@ -100,11 +99,11 @@ fn an_ended_identity_refuses_operations_spawns_and_views_while_the_scope_stays_o .mount("/", MemoryBackend::new()) .store("/my/store", MemoryBackend::new()) .build(); - let owner = access(&vfs)?; - let call = owner.spawn(test_origin())?; + let (owner, scope) = vfs.acquire_scope(test_origin())?; + let call = scope.spawn(&owner, test_origin())?; call.write("/a.txt", b"call")?; - let view = call.store_view()?; - end_access(&scope_handle(&owner), Some(owner.id), call.id); + let view = scope.store_view(&call)?; + scope.end_access(Some(owner.id()), call.id()); refused(call.read("/a.txt"), "/a.txt"); refused(call.write("/b.txt", b"late"), "/b.txt"); refused(view.write("b.md", b"late"), "b.md"); diff --git a/crates/runvil-internal/vfs/src/handle/tests/regions.rs b/crates/runvil-internal/vfs/src/handle/tests/regions.rs index f8e027f8d..fb3605a32 100644 --- a/crates/runvil-internal/vfs/src/handle/tests/regions.rs +++ b/crates/runvil-internal/vfs/src/handle/tests/regions.rs @@ -100,7 +100,7 @@ fn run_pair( lead(&first)?; let first_id = first.id; drop(first); - parent.join(first_id); + parent.scope.join(parent.id, first_id); let second = parent.spawn(test_origin())?; return Ok(trail(&second)); } diff --git a/crates/runvil-internal/vfs/src/lib.rs b/crates/runvil-internal/vfs/src/lib.rs index b7dcbc79b..258c8a695 100644 --- a/crates/runvil-internal/vfs/src/lib.rs +++ b/crates/runvil-internal/vfs/src/lib.rs @@ -19,7 +19,6 @@ //! `cargo test -p build-xtask` enforces the product and container //! boundaries. -pub mod detail; mod error; mod glob; mod handle; @@ -36,7 +35,7 @@ mod traits; use std::sync::{Arc, Mutex, MutexGuard, PoisonError}; pub use error::{PathReason, VfsError}; -pub use handle::{Access, VfsRef}; +pub use handle::{Access, ScopeHandle, VfsRef}; pub use memory::MemoryBackend; pub use observe::{OpEvent, OpSink, Origin}; pub use path::{VfsPath, VfsPathBuf}; diff --git a/crates/runvil-internal/vfs/src/router-tests.rs b/crates/runvil-internal/vfs/src/router-tests.rs index 1cd040358..1c811570a 100644 --- a/crates/runvil-internal/vfs/src/router-tests.rs +++ b/crates/runvil-internal/vfs/src/router-tests.rs @@ -166,7 +166,7 @@ fn closing_an_access_reports_a_mount_release_failure_that_dropping_swallows() -> let mount = TestFs::default().on_release(move |_| Err(refusal())); let vfs = VfsRef::builder().mount("/m", mount.clone()).build(); let closed = access(&vfs)?; - let id = crate::detail::access_id(&closed); + let id = closed.id(); closed.write("/m/f.txt", b"x")?; assert_eq!(closed.close(), Err(refusal())); assert!( @@ -177,7 +177,7 @@ fn closing_an_access_reports_a_mount_release_failure_that_dropping_swallows() -> access(&vfs)?.close()?; // Dropping one that did releases once and swallows the failure. let dropped = access(&vfs)?; - let id = crate::detail::access_id(&dropped); + let id = dropped.id(); dropped.write("/m/g.txt", b"x")?; drop(dropped); let releases = mount diff --git a/crates/runvil/public-api.txt b/crates/runvil/public-api.txt index 4280066ce..0b7e23146 100644 --- a/crates/runvil/public-api.txt +++ b/crates/runvil/public-api.txt @@ -133,6 +133,7 @@ auto runvil::vfs::OpEvent: Freeze, RefUnwindSafe, Send, Sync, Unpin, UnwindSafe auto runvil::vfs::Origin: Freeze, RefUnwindSafe, Send, Sync, Unpin, UnwindSafe auto runvil::vfs::PathReason: Freeze, RefUnwindSafe, Send, Sync, Unpin, UnwindSafe auto runvil::vfs::RealBackend: Freeze, RefUnwindSafe, Send, Sync, Unpin, UnwindSafe +auto runvil::vfs::ScopeHandle: Freeze, RefUnwindSafe, Send, Sync, Unpin, UnwindSafe auto runvil::vfs::Stat: Freeze, RefUnwindSafe, Send, Sync, Unpin, UnwindSafe auto runvil::vfs::Verdict: Freeze, RefUnwindSafe, Send, Sync, Unpin, UnwindSafe auto runvil::vfs::VfsError: Freeze, RefUnwindSafe, Send, Sync, Unpin, UnwindSafe @@ -226,6 +227,7 @@ derives runvil::vfs::OpEvent: Debug derives runvil::vfs::Origin: Clone, Debug, Eq, Hash, PartialEq derives runvil::vfs::PathReason: Clone, Copy, Debug, Eq, PartialEq derives runvil::vfs::RealBackend: Clone, Debug +derives runvil::vfs::ScopeHandle: Clone, Debug derives runvil::vfs::Stat: Clone, Debug, Eq, Hash, PartialEq derives runvil::vfs::Verdict: Clone, Debug, Eq, PartialEq derives runvil::vfs::VfsError: Clone, Debug, Eq, PartialEq @@ -308,6 +310,7 @@ pub const fn runvil::ids::RoundId::new(id: u64) -> Self pub const fn runvil::model::ModelRole::as_str(self) -> &'static str pub const fn runvil::timestamp::Timestamp::from_unix_millis(millis: i64) -> Self pub const fn runvil::timestamp::Timestamp::unix_millis(self) -> i64 +pub const fn runvil::vfs::Access::id(&self) -> runvil::vfs::ExecId pub const runvil::model::ModelId::GATEWAY: &'static str pub const runvil::model::ModelRole::ALL: [runvil::model::ModelRole; 5] pub const runvil::model::Role::ALL: &'static [runvil::model::Role] @@ -550,6 +553,12 @@ pub fn runvil::vfs::Policy::check(&self, op: runvil::vfs::Op, path: &runvil::vfs pub fn runvil::vfs::RealBackend::identity() -> runvil::vfs::RealBackend pub fn runvil::vfs::RealBackend::rooted(dir: impl core::convert::AsRef) -> core::result::Result pub fn runvil::vfs::RealBackend::with_writability(self, writability: runvil::vfs::Writability) -> runvil::vfs::RealBackend +pub fn runvil::vfs::ScopeHandle::end(&self) +pub fn runvil::vfs::ScopeHandle::end_access(&self, owner: core::option::Option, child: runvil::vfs::ExecId) +pub fn runvil::vfs::ScopeHandle::join(&self, owner: runvil::vfs::ExecId, child: runvil::vfs::ExecId) +pub fn runvil::vfs::ScopeHandle::probe_store(&self, access: &runvil::vfs::Access) -> core::result::Result<(), runvil::vfs::VfsError> +pub fn runvil::vfs::ScopeHandle::spawn(&self, parent: &runvil::vfs::Access, origin: runvil::vfs::Origin) -> core::result::Result +pub fn runvil::vfs::ScopeHandle::store_view(&self, access: &runvil::vfs::Access) -> core::result::Result pub fn runvil::vfs::Stat::new(file_type: runvil::vfs::FileType, size: u64) -> runvil::vfs::Stat pub fn runvil::vfs::Stat::with_created(self, created: core::option::Option) -> runvil::vfs::Stat pub fn runvil::vfs::Stat::with_mode(self, mode: core::option::Option) -> runvil::vfs::Stat @@ -580,6 +589,7 @@ pub fn runvil::vfs::VfsPath::as_str(&self) -> &str pub fn runvil::vfs::VfsPath::to_buf(&self) -> runvil::vfs::VfsPathBuf pub fn runvil::vfs::VfsPathBuf::as_str(&self) -> &str pub fn runvil::vfs::VfsRef::acquire(&self, origin: runvil::vfs::Origin) -> core::result::Result +pub fn runvil::vfs::VfsRef::acquire_scope(&self, origin: runvil::vfs::Origin) -> core::result::Result<(runvil::vfs::Access, runvil::vfs::ScopeHandle), runvil::vfs::VfsError> pub fn runvil::vfs::VfsRef::acquire_store(&self, origin: runvil::vfs::Origin) -> core::result::Result pub fn runvil::vfs::VfsRef::builder() -> runvil::vfs::VfsRefBuilder pub fn runvil::vfs::VfsRef::new(backend: impl runvil::vfs::Vfs + 'static) -> runvil::vfs::VfsRef @@ -1269,6 +1279,7 @@ pub struct runvil::vfs::ExecId(/* private fields */) pub struct runvil::vfs::ModeHandle { /* private fields */ } pub struct runvil::vfs::ModePolicy { /* private fields */ } pub struct runvil::vfs::OpEvent<'a> { /* private fields */ } +pub struct runvil::vfs::ScopeHandle(/* private fields */) pub struct runvil::vfs::VfsPath { /* private fields */ } pub struct runvil::vfs::VfsPathBuf(/* private fields */) pub struct runvil::vfs::VfsRef { /* private fields */ } diff --git a/crates/runvil/src/lib.rs b/crates/runvil/src/lib.rs index 2c79f2415..c9c04c5ce 100644 --- a/crates/runvil/src/lib.rs +++ b/crates/runvil/src/lib.rs @@ -154,6 +154,7 @@ pub mod vfs { pub use runvil_vfs::PathReason; pub use runvil_vfs::Policy; pub use runvil_vfs::RealBackend; + pub use runvil_vfs::ScopeHandle; pub use runvil_vfs::Stat; pub use runvil_vfs::Verdict; pub use runvil_vfs::Vfs; From 1b50252d4dd1117828dd8823126439b8949f82e4 Mon Sep 17 00:00:00 2001 From: Vinnie Falco Date: Fri, 6 Nov 2026 02:41:59 -0800 Subject: [PATCH 3/6] Remove the `build-ceiling` 500-line file-size check The compile-time 500-line file limit does not measure structure, and it caused splits whose only purpose was to fit the limit. This change deletes the `build-ceiling` crate, the 33 build scripts that only called `build_ceiling::check()`, and each `build-ceiling` entry in the manifests, `Cargo.lock`, and `vibelight.toml`. It also removes the call from three other build scripts and deletes `tidy-wiring.rs`, whose check required the call in each crate's `build.rs`. - This change does not merge split files back. `observation.rs` and the `__impl_tasks.lua` and `__impl_fanout.lua` chunks stay separate, and the comments on them no longer give the limit as the reason. - `desktop/build.rs`, `gateway/build.rs`, and `server/build.rs` keep their other work but no longer print `cargo::rerun-if-changed` for their crate's source trees. `desktop/build.rs` still watches `RUNVIL_GATEWAY_SIDECAR` and the sidecar marker, and `gateway/build.rs` still watches `gateway-pin/pin.json`, so both rerun less often. - `scaffold` now writes no `build.rs` and no `[build-dependencies]` table. The test `scaffold_writes_no_build_script` replaces the old build-script test and asserts both. - This change adds no other size check to replace the removed one. --- Cargo.lock | 30 -- Cargo.toml | 1 - crates/build-ceiling/Cargo.toml | 15 - crates/build-ceiling/build.rs | 13 - crates/build-ceiling/src/lib-tests.rs | 163 ---------- crates/build-ceiling/src/lib.rs | 202 ------------ crates/build-ui/Cargo.toml | 3 - crates/build-ui/build.rs | 5 - crates/build-user-guide/Cargo.toml | 3 - crates/build-user-guide/build.rs | 5 - crates/build-workshop/Cargo.toml | 3 - crates/build-workshop/build.rs | 5 - crates/build-xtask/Cargo.toml | 3 - crates/build-xtask/build.rs | 5 - crates/build-xtask/src/new_crate.rs | 32 +- crates/build-xtask/src/tidy-tests.rs | 10 - crates/build-xtask/src/tidy-wiring.rs | 305 ------------------ crates/build-xtask/src/tidy.rs | 17 +- crates/harness-gateway-client/build.rs | 13 - .../harness-internal/plugin-loader/build.rs | 13 - crates/harness-internal/runner/build.rs | 13 - .../harness-internal/test-plugin/Cargo.toml | 3 +- crates/harness-internal/test-plugin/build.rs | 13 - crates/harness/build.rs | 13 - crates/plugin-mcp/build.rs | 13 - crates/plugin-std/build.rs | 13 - crates/plugin-user-input/build.rs | 13 - crates/plugin-web/build.rs | 13 - crates/runvil-internal/engine/Cargo.toml | 3 - crates/runvil-internal/engine/build.rs | 5 - .../src/test_support/recording/observation.rs | 5 +- crates/runvil-internal/lua/Cargo.toml | 3 - crates/runvil-internal/lua/build.rs | 5 - crates/runvil-internal/lua/src/coro.rs | 8 +- .../runvil-internal/model-client/Cargo.toml | 3 - crates/runvil-internal/model-client/build.rs | 5 - crates/runvil-internal/prompt/Cargo.toml | 3 - crates/runvil-internal/prompt/build.rs | 5 - crates/runvil-internal/types/Cargo.toml | 3 - crates/runvil-internal/types/build.rs | 5 - crates/runvil-internal/vfs/build.rs | 13 - crates/runvil-plugin/Cargo.toml | 3 - crates/runvil-plugin/build.rs | 5 - crates/runvil/Cargo.toml | 3 - crates/runvil/build.rs | 5 - crates/shared-error-source/Cargo.toml | 3 - crates/shared-error-source/build.rs | 5 - crates/workshop/agents/Cargo.toml | 3 - crates/workshop/agents/build.rs | 5 - crates/workshop/desktop/Cargo.toml | 1 - crates/workshop/desktop/build.rs | 1 - crates/workshop/gateway-discovery/Cargo.toml | 3 - crates/workshop/gateway-discovery/build.rs | 5 - crates/workshop/gateway/Cargo.toml | 1 - crates/workshop/gateway/build.rs | 6 +- crates/workshop/menu/Cargo.toml | 3 - crates/workshop/menu/build.rs | 5 - crates/workshop/protocol/Cargo.toml | 3 - crates/workshop/protocol/build.rs | 5 - crates/workshop/registry/Cargo.toml | 3 - crates/workshop/registry/build.rs | 5 - crates/workshop/run-log/Cargo.toml | 3 - crates/workshop/run-log/build.rs | 5 - crates/workshop/server-api/Cargo.toml | 3 - crates/workshop/server-api/build.rs | 5 - crates/workshop/server/Cargo.toml | 1 - crates/workshop/server/build.rs | 4 - crates/workshop/status/Cargo.toml | 3 - crates/workshop/status/build.rs | 5 - crates/workshop/support/Cargo.toml | 3 - crates/workshop/support/build.rs | 5 - crates/workshop/user-state/Cargo.toml | 3 - crates/workshop/user-state/build.rs | 5 - crates/workshop/workspace/Cargo.toml | 3 - crates/workshop/workspace/build.rs | 5 - vibelight.toml | 1 - 76 files changed, 18 insertions(+), 1115 deletions(-) delete mode 100644 crates/build-ceiling/Cargo.toml delete mode 100644 crates/build-ceiling/build.rs delete mode 100644 crates/build-ceiling/src/lib-tests.rs delete mode 100644 crates/build-ceiling/src/lib.rs delete mode 100644 crates/build-ui/build.rs delete mode 100644 crates/build-user-guide/build.rs delete mode 100644 crates/build-workshop/build.rs delete mode 100644 crates/build-xtask/build.rs delete mode 100644 crates/build-xtask/src/tidy-wiring.rs delete mode 100644 crates/harness-gateway-client/build.rs delete mode 100644 crates/harness-internal/plugin-loader/build.rs delete mode 100644 crates/harness-internal/runner/build.rs delete mode 100644 crates/harness-internal/test-plugin/build.rs delete mode 100644 crates/harness/build.rs delete mode 100644 crates/plugin-mcp/build.rs delete mode 100644 crates/plugin-std/build.rs delete mode 100644 crates/plugin-user-input/build.rs delete mode 100644 crates/plugin-web/build.rs delete mode 100644 crates/runvil-internal/engine/build.rs delete mode 100644 crates/runvil-internal/lua/build.rs delete mode 100644 crates/runvil-internal/model-client/build.rs delete mode 100644 crates/runvil-internal/prompt/build.rs delete mode 100644 crates/runvil-internal/types/build.rs delete mode 100644 crates/runvil-internal/vfs/build.rs delete mode 100644 crates/runvil-plugin/build.rs delete mode 100644 crates/runvil/build.rs delete mode 100644 crates/shared-error-source/build.rs delete mode 100644 crates/workshop/agents/build.rs delete mode 100644 crates/workshop/gateway-discovery/build.rs delete mode 100644 crates/workshop/menu/build.rs delete mode 100644 crates/workshop/protocol/build.rs delete mode 100644 crates/workshop/registry/build.rs delete mode 100644 crates/workshop/run-log/build.rs delete mode 100644 crates/workshop/server-api/build.rs delete mode 100644 crates/workshop/status/build.rs delete mode 100644 crates/workshop/support/build.rs delete mode 100644 crates/workshop/user-state/build.rs delete mode 100644 crates/workshop/workspace/build.rs diff --git a/Cargo.lock b/Cargo.lock index 80ace64a0..20e234cfe 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -603,16 +603,11 @@ dependencies = [ "serde_core", ] -[[package]] -name = "build-ceiling" -version = "0.3.0" - [[package]] name = "build-ui" version = "0.3.0" dependencies = [ "anyhow", - "build-ceiling", "serde_json", "tempfile", ] @@ -621,7 +616,6 @@ dependencies = [ name = "build-user-guide" version = "0.3.0" dependencies = [ - "build-ceiling", "tempfile", ] @@ -630,7 +624,6 @@ name = "build-workshop" version = "0.3.0" dependencies = [ "anyhow", - "build-ceiling", "ctrlc", "tempfile", ] @@ -640,7 +633,6 @@ name = "build-xtask" version = "0.0.0" dependencies = [ "anyhow", - "build-ceiling", "proc-macro2", "pulldown-cmark", "rustdoc-types", @@ -4729,7 +4721,6 @@ dependencies = [ name = "runvil" version = "0.3.0" dependencies = [ - "build-ceiling", "runvil-engine", "runvil-lua", "runvil-model-client", @@ -4743,7 +4734,6 @@ dependencies = [ name = "runvil-engine" version = "0.3.0" dependencies = [ - "build-ceiling", "criterion", "mlua", "regex", @@ -4760,7 +4750,6 @@ dependencies = [ name = "runvil-lua" version = "0.3.0" dependencies = [ - "build-ceiling", "criterion", "mlua", "runvil-model-client", @@ -4774,7 +4763,6 @@ dependencies = [ name = "runvil-model-client" version = "0.3.0" dependencies = [ - "build-ceiling", "runvil-types", "serde", "serde_json", @@ -4784,7 +4772,6 @@ dependencies = [ name = "runvil-plugin" version = "0.3.0" dependencies = [ - "build-ceiling", "runvil-types", "runvil-vfs", "serde_json", @@ -4794,7 +4781,6 @@ dependencies = [ name = "runvil-prompt" version = "0.3.0" dependencies = [ - "build-ceiling", "mlua", "pulldown-cmark", "runvil-lua", @@ -4807,7 +4793,6 @@ dependencies = [ name = "runvil-types" version = "0.3.0" dependencies = [ - "build-ceiling", "serde", "serde_json", "sha2 0.11.0", @@ -5432,7 +5417,6 @@ dependencies = [ name = "shared-error-source" version = "0.3.0" dependencies = [ - "build-ceiling", "reqwest", "serde_json", "thiserror 2.0.19", @@ -7884,7 +7868,6 @@ name = "workshop" version = "0.3.0" dependencies = [ "anyhow", - "build-ceiling", "reqwest", "serde", "serde_json", @@ -7908,7 +7891,6 @@ name = "workshop-agents" version = "0.0.0" dependencies = [ "async-trait", - "build-ceiling", "harness", "plugin-user-input", "rand 0.9.5", @@ -7927,7 +7909,6 @@ version = "0.0.0" dependencies = [ "arc-swap", "axum", - "build-ceiling", "futures-util", "reqwest", "serde", @@ -7948,7 +7929,6 @@ dependencies = [ name = "workshop-gateway-discovery" version = "0.3.0" dependencies = [ - "build-ceiling", "libc", "serde", "serde_json", @@ -7963,7 +7943,6 @@ dependencies = [ name = "workshop-menu" version = "0.0.0" dependencies = [ - "build-ceiling", "serde", "serde_json", "tempfile", @@ -7979,7 +7958,6 @@ dependencies = [ name = "workshop-protocol" version = "0.0.0" dependencies = [ - "build-ceiling", "serde", "serde_json", ] @@ -7989,7 +7967,6 @@ name = "workshop-registry" version = "0.0.0" dependencies = [ "axum", - "build-ceiling", "serde_json", "tokio", "workshop-protocol", @@ -7999,7 +7976,6 @@ dependencies = [ name = "workshop-run-log" version = "0.0.0" dependencies = [ - "build-ceiling", "harness", "runvil", "serde_json", @@ -8016,7 +7992,6 @@ version = "0.3.0" dependencies = [ "async-trait", "axum", - "build-ceiling", "build-ui", "futures-util", "harness", @@ -8056,7 +8031,6 @@ dependencies = [ name = "workshop-server-api" version = "0.0.0" dependencies = [ - "build-ceiling", "workshop-server", "workshop-server-api", ] @@ -8065,7 +8039,6 @@ dependencies = [ name = "workshop-status" version = "0.0.0" dependencies = [ - "build-ceiling", "tokio", "workshop-protocol", "workshop-registry", @@ -8077,7 +8050,6 @@ name = "workshop-support" version = "0.0.0" dependencies = [ "axum", - "build-ceiling", "reqwest", "serde", "serde_json", @@ -8094,7 +8066,6 @@ name = "workshop-user-state" version = "0.0.0" dependencies = [ "axum", - "build-ceiling", "serde_json", "shared-error-source", "tempfile", @@ -8112,7 +8083,6 @@ name = "workshop-workspace" version = "0.0.0" dependencies = [ "axum", - "build-ceiling", "dunce", "humantime", "percent-encoding", diff --git a/Cargo.toml b/Cargo.toml index d08bad522..7a494df03 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -24,7 +24,6 @@ repository = "https://github.com/cppalliance/runvil" [workspace.dependencies] bytes = "1" -build-ceiling = { path = "crates/build-ceiling" } runvil = { path = "crates/runvil" } runvil-engine = { path = "crates/runvil-internal/engine" } runvil-plugin = { path = "crates/runvil-plugin" } diff --git a/crates/build-ceiling/Cargo.toml b/crates/build-ceiling/Cargo.toml deleted file mode 100644 index 7edca9fa4..000000000 --- a/crates/build-ceiling/Cargo.toml +++ /dev/null @@ -1,15 +0,0 @@ -[package] -name = "build-ceiling" -version.workspace = true -edition.workspace = true -license.workspace = true -repository.workspace = true -publish = false - -# No dependencies of any kind: the six crates that may not name a build-* -# crate include src/lib.rs with #[path] from their build scripts, so it must -# compile on std alone. -[dependencies] - -[lints] -workspace = true diff --git a/crates/build-ceiling/build.rs b/crates/build-ceiling/build.rs deleted file mode 100644 index 19c4ac472..000000000 --- a/crates/build-ceiling/build.rs +++ /dev/null @@ -1,13 +0,0 @@ -//! Runs the compile-time file-size check on this crate by including its own -//! source, because a crate cannot be its own build-dependency. - -#[expect( - unreachable_pub, - reason = "the library's public items are private to this build script" -)] -#[path = "src/lib.rs"] -mod build_ceiling; - -fn main() -> Result<(), build_ceiling::Violations> { - build_ceiling::check() -} diff --git a/crates/build-ceiling/src/lib-tests.rs b/crates/build-ceiling/src/lib-tests.rs deleted file mode 100644 index a45d39cc3..000000000 --- a/crates/build-ceiling/src/lib-tests.rs +++ /dev/null @@ -1,163 +0,0 @@ -use super::*; - -use std::fs; -use std::sync::atomic::{AtomicU64, Ordering}; - -/// A crate root under the system temporary directory that removes itself -/// on drop. -struct Fixture(PathBuf); - -impl Fixture { - fn new() -> Fixture { - static COUNTER: AtomicU64 = AtomicU64::new(0); - let root = std::env::temp_dir().join(format!( - "build-ceiling-test-{}-{}", - std::process::id(), - COUNTER.fetch_add(1, Ordering::Relaxed) - )); - fs::create_dir_all(root.join("src")).expect("the fixture's src directory creates"); - Fixture(root) - } - - /// Writes `bytes` to the `/`-separated `relative` under the root and - /// returns its path. - fn write(&self, relative: &str, bytes: impl AsRef<[u8]>) -> PathBuf { - let path = relative - .split('/') - .fold(self.0.clone(), |path, part| path.join(part)); - fs::create_dir_all(path.parent().expect("a fixture file has a parent")) - .expect("the fixture directory creates"); - fs::write(&path, bytes).expect("the fixture file writes"); - path - } -} - -impl Drop for Fixture { - fn drop(&mut self) { - let _ = fs::remove_dir_all(&self.0); - } -} - -/// `count` lines of `// line`, each ending in a newline. -fn lines(count: usize) -> String { - "// line\n".repeat(count) -} - -/// The oversized files a scan reported, as `(path, lines)`. -fn oversized(scan: &Scan) -> Vec<(PathBuf, usize)> { - scan.findings - .iter() - .filter_map(|finding| match finding { - Finding::Oversized { path, lines } => Some((path.clone(), *lines)), - Finding::Unreadable { .. } => None, - }) - .collect() -} - -#[test] -fn every_physical_line_counts_blank_comment_crlf_and_unterminated() { - let fixture = Fixture::new(); - let text = format!( - "{}{}fn last_line_without_a_newline() {{}}", - "\r\n".repeat(250), - "// comment\r\n".repeat(250) - ); - let path = fixture.write("src/lib.rs", text); - assert_eq!(oversized(&scan(&fixture.0)), vec![(path, 501)]); -} - -#[test] -fn a_file_at_the_limit_passes_and_one_line_over_fails() { - let fixture = Fixture::new(); - fixture.write("src/at.rs", lines(500)); - let over = fixture.write("src/over.rs", lines(501)); - let scan = scan(&fixture.0); - assert_eq!(oversized(&scan), vec![(over, 501)]); - assert_eq!(scan.findings.len(), 1, "only the file over the limit fails"); -} - -#[test] -fn every_counted_tree_and_the_build_script_are_scanned_and_nothing_else() { - let fixture = Fixture::new(); - let counted = [ - "src/nested/deep/mod.rs", - "tests/it/main.rs", - "benches/bench.rs", - "examples/demo.rs", - "build.rs", - ] - .map(|relative| fixture.write(relative, lines(501))); - fixture.write("ui/generated.rs", lines(501)); - fixture.write("src/notes.md", lines(501)); - let mut found: Vec = oversized(&scan(&fixture.0)) - .into_iter() - .map(|(path, _)| path) - .collect(); - found.sort(); - let mut expected = counted.to_vec(); - expected.sort(); - assert_eq!(found, expected); -} - -#[test] -fn only_paths_that_exist_are_watched() { - let fixture = Fixture::new(); - fixture.write("tests/it/main.rs", lines(1)); - let mut watched = scan(&fixture.0).watched; - watched.sort(); - assert_eq!( - watched, - vec![fixture.0.join("src"), fixture.0.join("tests")], - "a missing path makes cargo rerun the build script on every build, so only \ - existing directories and build.rs are watched" - ); - fixture.write("build.rs", lines(1)); - assert!( - scan(&fixture.0) - .watched - .contains(&fixture.0.join("build.rs")) - ); -} - -#[test] -fn display_and_debug_both_carry_the_whole_failure_message() { - let fixture = Fixture::new(); - let path = fixture.write("src/big.rs", lines(501)); - let violations = Violations { - findings: scan(&fixture.0).findings, - }; - for text in [format!("{violations}"), format!("{violations:?}")] { - for part in [ - path.display().to_string().as_str(), - "has 501 lines", - "limit of 500", - "help: split the file only into private child modules of that file", - "help: declare each child inside the file as `#[path = \"-.rs\"] mod ;`", - "help: open each child module with a `//!` line naming its one concern", - "help: move an inline `#[cfg(test)]` module to `-tests.rs` first", - "help: reach the parent's private items through `super::`", - "help: never widen anything to `pub(crate)` or `pub` to make a split compile", - ] { - assert!(text.contains(part), "missing {part:?} in:\n{text}"); - } - assert!( - text.ends_with( - "If no cohesive group splits out without widening visibility, stop and say why." - ), - "the message ends with the stop sentence:\n{text}" - ); - } -} - -#[test] -fn an_unreadable_file_is_reported_not_skipped() { - let fixture = Fixture::new(); - let path = fixture.write("src/bad.rs", [b'f', 0xff, 0xfe, b'\n']); - let findings = scan(&fixture.0).findings; - assert!( - matches!(findings.as_slice(), [Finding::Unreadable { path: reported, .. }] if *reported == path), - "the invalid UTF-8 file is reported" - ); - let text = Violations { findings }.to_string(); - assert!(text.contains(&path.display().to_string()), "{text}"); -} diff --git a/crates/build-ceiling/src/lib.rs b/crates/build-ceiling/src/lib.rs deleted file mode 100644 index 684088c93..000000000 --- a/crates/build-ceiling/src/lib.rs +++ /dev/null @@ -1,202 +0,0 @@ -//! The compile-time file-size check: every workspace crate's build script -//! calls [`check`], which fails the build when a Rust file in the crate is -//! over 500 lines. -//! -//! This file compiles on std alone, because the crates that may not name a -//! `build-*` crate include it from their build scripts with `#[path]`. - -use std::fmt; -use std::fs; -use std::io; -use std::path::{Path, PathBuf}; - -/// The most physical lines a Rust file may have. -const LIMIT: usize = 500; - -/// The directories under the crate root whose `.rs` files are counted, at -/// any depth, beside the root's own `build.rs`. -const TREES: [&str; 4] = ["src", "tests", "benches", "examples"]; - -/// The split rule, one `help:` line each. -const HELP: [&str; 6] = [ - "split the file only into private child modules of that file", - "declare each child inside the file as `#[path = \"-.rs\"] mod ;`, or \ - lay it out as `/.rs`, choosing between the two forms by the flat-directory \ - rule in AGENTS.md", - "open each child module with a `//!` line naming its one concern", - "move an inline `#[cfg(test)]` module to `-tests.rs` first, when one exists", - "reach the parent's private items through `super::`, and give a moved item `pub(super)` \ - only where the parent uses it", - "never widen anything to `pub(crate)` or `pub` to make a split compile", -]; - -/// The sentence the failure message ends with. -const STOP: &str = "If no cohesive group splits out without widening visibility, stop and say why."; - -/// Counts every physical line of every `.rs` file in the calling crate, -/// found from `CARGO_MANIFEST_DIR`, and asks Cargo to rerun the build -/// script when any of those files changes or a file is added beside them. -/// -/// A tree that does not exist yet is not watched, so the first file in a -/// new `tests/`, `benches/`, or `examples/` directory is counted only at the -/// next rerun: an edit to a watched path, or a build from a fresh checkout, -/// which is every CI build. -/// -/// # Errors -/// -/// Returns every file over 500 lines and every file or directory the scan -/// could not read. -pub fn check() -> Result<(), Violations> { - let Some(root) = std::env::var_os("CARGO_MANIFEST_DIR") else { - return Err(Violations { - findings: vec![Finding::Unreadable { - path: PathBuf::from("CARGO_MANIFEST_DIR"), - error: "the variable is not set; run the build script through cargo".to_owned(), - }], - }); - }; - let Scan { watched, findings } = scan(Path::new(&root)); - for path in watched { - println!("cargo::rerun-if-changed={}", path.display()); - } - if findings.is_empty() { - Ok(()) - } else { - Err(Violations { findings }) - } -} - -/// Every file over the limit and every file the scan could not read. Its -/// `Display` and `Debug` render the same failure message, because a `main` -/// that returns a `Result` prints its error through `Debug`. -pub struct Violations { - findings: Vec, -} - -/// One path the check failed. -enum Finding { - /// A file over the limit, with its physical line count. - Oversized { path: PathBuf, lines: usize }, - /// A file or directory the scan could not read, so it could not be - /// shown to be within the limit. - Unreadable { path: PathBuf, error: String }, -} - -/// What one scan of a crate root found. -struct Scan { - /// The trees and the build script that exist, for `rerun-if-changed`. - /// A path that does not exist is left out, because Cargo reruns a - /// build script on every build while a watched path is missing. - watched: Vec, - findings: Vec, -} - -/// Scans the crate at `root`: every `.rs` file under [`TREES`] and the -/// root's `build.rs`, in path order. -fn scan(root: &Path) -> Scan { - let mut watched = Vec::new(); - let mut files = Vec::new(); - let mut findings = Vec::new(); - for tree in TREES { - let dir = root.join(tree); - if dir.exists() { - collect(&dir, &mut files, &mut findings); - watched.push(dir); - } - } - let build = root.join("build.rs"); - if build.exists() { - files.push(build.clone()); - watched.push(build); - } - files.sort(); - for path in files { - match fs::read_to_string(&path) { - Ok(text) => { - let lines = text.lines().count(); - if lines > LIMIT { - findings.push(Finding::Oversized { path, lines }); - } - } - Err(error) => findings.push(unreadable(path, &error)), - } - } - Scan { watched, findings } -} - -/// Adds every `.rs` file under `dir`, at any depth, to `files`. -fn collect(dir: &Path, files: &mut Vec, findings: &mut Vec) { - let entries = match fs::read_dir(dir) { - Ok(entries) => entries, - Err(error) => { - findings.push(unreadable(dir.to_path_buf(), &error)); - return; - } - }; - for entry in entries { - match entry { - Ok(entry) => { - let path = entry.path(); - if path.is_dir() { - collect(&path, files, findings); - } else if path.extension().is_some_and(|extension| extension == "rs") { - files.push(path); - } - } - Err(error) => findings.push(unreadable(dir.to_path_buf(), &error)), - } - } -} - -fn unreadable(path: PathBuf, error: &io::Error) -> Finding { - Finding::Unreadable { - path, - error: error.to_string(), - } -} - -impl fmt::Display for Violations { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let mut oversized = false; - for (index, finding) in self.findings.iter().enumerate() { - if index > 0 { - writeln!(f)?; - } - match finding { - Finding::Oversized { path, lines } => { - oversized = true; - write!( - f, - "{} has {lines} lines, over the limit of {LIMIT}", - path.display() - )?; - } - Finding::Unreadable { path, error } => write!( - f, - "{} could not be read, so its lines could not be counted against the \ - limit of {LIMIT}: {error}", - path.display() - )?, - } - } - if oversized { - for help in HELP { - write!(f, "\nhelp: {help}")?; - } - write!(f, "\n{STOP}")?; - } - Ok(()) - } -} - -impl fmt::Debug for Violations { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - fmt::Display::fmt(self, f) - } -} - -impl std::error::Error for Violations {} - -#[cfg(test)] -#[path = "lib-tests.rs"] -mod tests; diff --git a/crates/build-ui/Cargo.toml b/crates/build-ui/Cargo.toml index 0b1ef5c4a..34afe7e83 100644 --- a/crates/build-ui/Cargo.toml +++ b/crates/build-ui/Cargo.toml @@ -13,8 +13,5 @@ serde_json.workspace = true [dev-dependencies] tempfile.workspace = true -[build-dependencies] -build-ceiling.workspace = true - [lints] workspace = true diff --git a/crates/build-ui/build.rs b/crates/build-ui/build.rs deleted file mode 100644 index f62713c6f..000000000 --- a/crates/build-ui/build.rs +++ /dev/null @@ -1,5 +0,0 @@ -//! Runs the compile-time file-size check on this crate. - -fn main() -> Result<(), build_ceiling::Violations> { - build_ceiling::check() -} diff --git a/crates/build-user-guide/Cargo.toml b/crates/build-user-guide/Cargo.toml index 77720a7b5..4d33c1407 100644 --- a/crates/build-user-guide/Cargo.toml +++ b/crates/build-user-guide/Cargo.toml @@ -10,8 +10,5 @@ publish = false [dev-dependencies] tempfile.workspace = true -[build-dependencies] -build-ceiling.workspace = true - [lints] workspace = true diff --git a/crates/build-user-guide/build.rs b/crates/build-user-guide/build.rs deleted file mode 100644 index f62713c6f..000000000 --- a/crates/build-user-guide/build.rs +++ /dev/null @@ -1,5 +0,0 @@ -//! Runs the compile-time file-size check on this crate. - -fn main() -> Result<(), build_ceiling::Violations> { - build_ceiling::check() -} diff --git a/crates/build-workshop/Cargo.toml b/crates/build-workshop/Cargo.toml index 06adcb9fb..3ba48ed95 100644 --- a/crates/build-workshop/Cargo.toml +++ b/crates/build-workshop/Cargo.toml @@ -17,8 +17,5 @@ ctrlc.workspace = true [dev-dependencies] tempfile.workspace = true -[build-dependencies] -build-ceiling.workspace = true - [lints] workspace = true diff --git a/crates/build-workshop/build.rs b/crates/build-workshop/build.rs deleted file mode 100644 index f62713c6f..000000000 --- a/crates/build-workshop/build.rs +++ /dev/null @@ -1,5 +0,0 @@ -//! Runs the compile-time file-size check on this crate. - -fn main() -> Result<(), build_ceiling::Violations> { - build_ceiling::check() -} diff --git a/crates/build-xtask/Cargo.toml b/crates/build-xtask/Cargo.toml index 15ab13d0f..7d0bd9b7e 100644 --- a/crates/build-xtask/Cargo.toml +++ b/crates/build-xtask/Cargo.toml @@ -19,8 +19,5 @@ toml.workspace = true tempfile.workspace = true workshop-gateway-discovery = { workspace = true, features = ["test-fixtures"] } -[build-dependencies] -build-ceiling.workspace = true - [lints] workspace = true diff --git a/crates/build-xtask/build.rs b/crates/build-xtask/build.rs deleted file mode 100644 index f62713c6f..000000000 --- a/crates/build-xtask/build.rs +++ /dev/null @@ -1,5 +0,0 @@ -//! Runs the compile-time file-size check on this crate. - -fn main() -> Result<(), build_ceiling::Violations> { - build_ceiling::check() -} diff --git a/crates/build-xtask/src/new_crate.rs b/crates/build-xtask/src/new_crate.rs index 6a55336cc..dd04fbf92 100644 --- a/crates/build-xtask/src/new_crate.rs +++ b/crates/build-xtask/src/new_crate.rs @@ -4,8 +4,7 @@ use std::fs; use std::path::{Path, PathBuf}; /// Scaffold `crates//` with a manifest, a facade `lib.rs` holding -/// the invariant docs, a build script that runs the compile-time file-size -/// check, and the crate's integration-test binary. +/// the invariant docs, and the crate's integration-test binary. /// /// # Errors /// @@ -22,7 +21,6 @@ pub(crate) fn scaffold(root: &Path, name: &str) -> anyhow::Result { fs::create_dir_all(dir.join("src"))?; fs::create_dir_all(dir.join("tests").join("it"))?; fs::write(dir.join("Cargo.toml"), manifest(name))?; - fs::write(dir.join("build.rs"), BUILD_RS)?; fs::write(dir.join("src").join("lib.rs"), lib_rs(name))?; fs::write( dir.join("tests").join("it").join("main.rs"), @@ -60,21 +58,11 @@ fn manifest(name: &str) -> String { \n\ [dependencies]\n\ \n\ - [build-dependencies]\n\ - build-ceiling.workspace = true\n\ - \n\ [lints]\n\ workspace = true\n" ) } -/// The build script: the compile-time file-size check, and nothing else. -const BUILD_RS: &str = "//! Runs the compile-time file-size check on this crate.\n\ - \n\ - fn main() -> Result<(), build_ceiling::Violations> {\n \ - build_ceiling::check()\n\ - }\n"; - fn lib_rs(name: &str) -> String { format!( "//! {name} - TODO: one-line purpose.\n\ @@ -118,24 +106,12 @@ mod tests { } #[test] - fn scaffold_wires_the_file_size_check_into_the_build_script() { + fn scaffold_writes_no_build_script() { let (_temp, _root, dir) = scaffold_scratch(); - let build = fs::read_to_string(dir.join("build.rs")).expect("build.rs"); - assert!( - build.ends_with( - "\nfn main() -> Result<(), build_ceiling::Violations> {\n \ - build_ceiling::check()\n}\n" - ), - "{build}" - ); + assert!(!dir.join("build.rs").exists()); let text = fs::read_to_string(dir.join("Cargo.toml")).expect("manifest"); let manifest: toml::Value = toml::from_str(&text).expect("valid toml"); - let ceiling = manifest - .get("build-dependencies") - .and_then(|table| table.get("build-ceiling")) - .and_then(|entry| entry.get("workspace")) - .and_then(toml::Value::as_bool); - assert_eq!(ceiling, Some(true), "{text}"); + assert!(manifest.get("build-dependencies").is_none(), "{text}"); } #[test] diff --git a/crates/build-xtask/src/tidy-tests.rs b/crates/build-xtask/src/tidy-tests.rs index 198b717f2..f17d9d338 100644 --- a/crates/build-xtask/src/tidy-tests.rs +++ b/crates/build-xtask/src/tidy-tests.rs @@ -28,16 +28,6 @@ fn every_workspace_crate_inherits_workspace_lints() { ); } -#[test] -fn every_workspace_crate_runs_the_ceiling_from_its_build_script() { - let violations = wiring::ceiling_wiring_violations(&workspace_root()); - assert!( - violations.is_empty(), - "ceiling wiring violations:\n{}", - violations.join("\n") - ); -} - #[test] fn family_crates_have_the_invariants_marker() { let violations = marker_violations(&workspace_root()); diff --git a/crates/build-xtask/src/tidy-wiring.rs b/crates/build-xtask/src/tidy-wiring.rs deleted file mode 100644 index 772d31e73..000000000 --- a/crates/build-xtask/src/tidy-wiring.rs +++ /dev/null @@ -1,305 +0,0 @@ -//! Ceiling wiring: every workspace crate's build script runs the -//! compile-time file-size check. -//! -//! The build script is read as tokens, so a comment or a string that only -//! spells the call is never read as one. - -use std::fs; -use std::path::{Component, Path, PathBuf}; - -use proc_macro2::{Delimiter, TokenStream, TokenTree}; -use syn::{Expr, ExprLit, Lit, Meta}; - -/// The ceiling's source, from the workspace root: the file the `#[path]` -/// form includes. -const CEILING_SOURCE: [&str; 4] = ["crates", "build-ceiling", "src", "lib.rs"]; - -/// Checks that every workspace crate's package-root `build.rs` calls -/// `build_ceiling::check()` and reaches it through a `build-ceiling` -/// build-dependency or a `#[path]` include of the ceiling's source. A -/// crate whose manifest could not be read is held to the `#[path]` form, -/// the one its build script alone can show, and a walk that found no -/// crate fails. -pub(super) fn ceiling_wiring_violations(root: &Path) -> Vec { - let walk = crate::product::workspace_crates(root); - if walk.crates.is_empty() && walk.unread.is_empty() { - return vec![format!( - "{}: the ceiling wiring check scanned nothing: no workspace crate was found, so \ - none can be shown to run `build_ceiling::check()`", - root.join("crates").display() - )]; - } - let mut violations = Vec::new(); - for krate in &walk.crates { - let dir = root.join(&krate.dir); - let manifest = fs::read_to_string(dir.join("Cargo.toml")) - .ok() - .and_then(|text| toml::from_str::(&text).ok()); - violations.extend(crate_violation(root, &dir, manifest.as_ref())); - } - for dir in &walk.unread { - violations.extend(crate_violation(root, &root.join(dir), None)); - } - violations -} - -/// Why the crate at `dir` is not wired, or `None` when it is. -fn crate_violation(root: &Path, dir: &Path, manifest: Option<&toml::Value>) -> Option { - let build = dir.join("build.rs"); - if let Some(target) = manifest - .and_then(|manifest| manifest.get("package")) - .and_then(|package| package.get("build")) - .filter(|target| target.as_str() != Some("build.rs")) - { - return Some(format!( - "{}: sets `package.build = {target}`, so cargo never runs the package-root \ - build.rs; remove the key and call `build_ceiling::check()` from build.rs", - dir.join("Cargo.toml").display() - )); - } - let tokens: TokenStream = match fs::read_to_string(&build) { - Ok(text) => match text.parse() { - Ok(tokens) => tokens, - Err(error) => return Some(format!("{}: unparseable source: {error}", build.display())), - }, - Err(error) => { - return Some(format!( - "{}: no readable package-root build script ({error}); every crate's build.rs \ - calls `build_ceiling::check()`, and a `src/build.rs` module is not one", - build.display() - )); - } - }; - if !calls_check(tokens.clone()) { - return Some(format!( - "{}: does not call `build_ceiling::check()`, the compile-time file-size check", - build.display() - )); - } - if manifest.is_some_and(names_ceiling) || includes_ceiling(root, dir, tokens) { - return None; - } - Some(format!( - "{}: calls `build_ceiling::check()` but reaches neither a `build-ceiling` entry under \ - [build-dependencies] nor `#[path = \"/build-ceiling/src/lib.rs\"] mod \ - build_ceiling;`", - build.display() - )) -} - -/// Whether `tokens`, at any depth, hold the call `build_ceiling::check()`. -fn calls_check(tokens: TokenStream) -> bool { - let trees: Vec = tokens.into_iter().collect(); - trees.windows(5).any(|window| { - matches!(window, [ - TokenTree::Ident(module), - TokenTree::Punct(first), - TokenTree::Punct(second), - TokenTree::Ident(function), - TokenTree::Group(arguments), - ] if module == "build_ceiling" - && first.as_char() == ':' - && second.as_char() == ':' - && function == "check" - && arguments.delimiter() == Delimiter::Parenthesis - && arguments.stream().is_empty()) - }) || trees - .iter() - .any(|tree| matches!(tree, TokenTree::Group(group) if calls_check(group.stream()))) -} - -/// Whether a `build-ceiling` entry sits in any build-dependency table. -fn names_ceiling(manifest: &toml::Value) -> bool { - crate::manifest::dependency_tables(manifest, &["build-dependencies"]) - .iter() - .any(|(_, table)| { - table.iter().any(|(key, value)| { - value - .get("package") - .and_then(toml::Value::as_str) - .unwrap_or(key) - == "build-ceiling" - }) - }) -} - -/// Whether the build script of the crate at `dir` declares -/// `mod build_ceiling;` under a `#[path]` that resolves to the ceiling's -/// source. -fn includes_ceiling(root: &Path, dir: &Path, tokens: TokenStream) -> bool { - let source = lexical( - &CEILING_SOURCE - .iter() - .fold(root.to_path_buf(), |path, part| path.join(part)), - ); - let mut paths = Vec::new(); - let mut tokens = tokens.into_iter().peekable(); - while let Some(token) = tokens.next() { - match token { - TokenTree::Punct(pound) if pound.as_char() == '#' => { - if matches!(tokens.peek(), Some(TokenTree::Punct(bang)) if bang.as_char() == '!') { - tokens.next(); - tokens.next(); - } else if let Some(TokenTree::Group(attribute)) = tokens.next() - && attribute.delimiter() == Delimiter::Bracket - && let Ok(Meta::NameValue(pair)) = syn::parse2::(attribute.stream()) - && pair.path.is_ident("path") - && let Expr::Lit(ExprLit { - lit: Lit::Str(path), - .. - }) = pair.value - { - paths.push(path.value()); - } - } - TokenTree::Ident(keyword) if keyword == "mod" => { - let named = matches!(tokens.peek(), Some(TokenTree::Ident(name)) if name == "build_ceiling"); - if named && paths.iter().any(|path| lexical(&dir.join(path)) == source) { - return true; - } - paths.clear(); - } - _ => paths.clear(), - } - } - false -} - -/// `path` with every `.` dropped and every `..` applied, without touching -/// the filesystem. -fn lexical(path: &Path) -> PathBuf { - let mut resolved = PathBuf::new(); - for component in path.components() { - match component { - Component::CurDir => {} - Component::ParentDir => { - resolved.pop(); - } - other => resolved.push(other), - } - } - resolved -} - -#[cfg(test)] -mod tests { - use super::*; - use std::path::PathBuf; - - const BUILD_DEPENDENCY: &str = "[build-dependencies]\nbuild-ceiling.workspace = true\n"; - const CALLS_CHECK: &str = - "fn main() -> Result<(), build_ceiling::Violations> { build_ceiling::check() }\n"; - - /// A build script in the `#[path]` form, including the ceiling from - /// `source`. - fn includes(source: &str) -> String { - format!( - "#[expect(unreachable_pub, reason = \"private\")]\n\ - #[path = \"{source}\"]\nmod build_ceiling;\n{CALLS_CHECK}" - ) - } - - /// Writes a crate under `crates//` named `name`, with `tail` inside - /// and after its `[package]` table and `build` as its package-root - /// `build.rs` when given. Returns the crate directory. - fn write_built(root: &Path, dir: &str, name: &str, tail: &str, build: Option<&str>) -> PathBuf { - let dir = root.join("crates").join(dir); - std::fs::create_dir_all(dir.join("src")).expect("the crate directory creates"); - let manifest = format!("[package]\nname = \"{name}\"\n{tail}"); - std::fs::write(dir.join("Cargo.toml"), manifest).expect("the manifest writes"); - if let Some(build) = build { - std::fs::write(dir.join("build.rs"), build).expect("build.rs writes"); - } - dir - } - - /// A wired workspace: a crate in each form and the ceiling's own crate. - fn write_wired(root: &Path) { - let local = Some(CALLS_CHECK); - write_built(root, "runvil", "runvil", BUILD_DEPENDENCY, local); - let web = includes("../build-ceiling/src/lib.rs"); - write_built(root, "plugin-web", "plugin-web", "", Some(&web)); - let runner = includes("../../build-ceiling/src/lib.rs"); - let dir = "harness-internal/runner"; - write_built(root, dir, "harness-runner", "", Some(&runner)); - let own = includes("src/lib.rs"); - write_built(root, "build-ceiling", "build-ceiling", "", Some(&own)); - } - - /// The wiring violations of a wired workspace plus one crate. - fn wiring_with(dir: &str, name: &str, tail: &str, build: Option<&str>) -> Vec { - let root = tempfile::TempDir::new().expect("tempdir"); - write_wired(root.path()); - write_built(root.path(), dir, name, tail, build); - ceiling_wiring_violations(root.path()) - } - - #[test] - fn dependency_wired_and_path_wired_crates_pass() { - let root = tempfile::TempDir::new().expect("tempdir"); - write_wired(root.path()); - let violations = ceiling_wiring_violations(root.path()); - assert!(violations.is_empty(), "{violations:?}"); - } - - #[test] - fn a_build_script_that_names_the_call_only_in_a_comment_fails() { - let build = Some("// build_ceiling::check()\nfn main() {}\n"); - let violations = wiring_with("plugin-mcp", "plugin-mcp", BUILD_DEPENDENCY, build); - assert_eq!(violations.len(), 1, "{violations:?}"); - assert!(violations[0].contains("plugin-mcp") && violations[0].contains("does not call")); - } - - #[test] - fn a_crate_with_no_package_root_build_script_fails_even_with_a_src_build_module() { - let root = tempfile::TempDir::new().expect("tempdir"); - write_wired(root.path()); - let name = "runvil-prompt"; - let parser = write_built(root.path(), "parser", name, BUILD_DEPENDENCY, None); - let module = parser.join("src").join("build.rs"); - std::fs::write(module, CALLS_CHECK).expect("the module writes"); - let violations = ceiling_wiring_violations(root.path()); - assert_eq!(violations.len(), 1, "{violations:?}"); - assert!(violations[0].contains("parser") && violations[0].contains("build.rs")); - } - - #[test] - fn a_path_include_of_any_file_but_the_ceiling_source_fails() { - let build = includes("../plugin-web/src/lib.rs"); - let name = "harness-gateway-client"; - let violations = wiring_with(name, name, "", Some(&build)); - assert_eq!(violations.len(), 1, "{violations:?}"); - assert!(violations[0].contains(name)); - } - - #[test] - fn a_package_build_key_fails_because_cargo_skips_the_wired_build_rs() { - let tail = format!("build = false\n{BUILD_DEPENDENCY}"); - let build = Some(CALLS_CHECK); - let violations = wiring_with("workshop/status", "workshop-status", &tail, build); - assert_eq!(violations.len(), 1, "{violations:?}"); - assert!(violations[0].contains("package.build")); - } - - #[test] - fn a_crate_whose_manifest_does_not_parse_is_held_to_the_path_form() { - let root = tempfile::TempDir::new().expect("tempdir"); - write_wired(root.path()); - let broken = root.path().join("crates").join("broken"); - std::fs::create_dir_all(&broken).expect("the crate directory creates"); - std::fs::write(broken.join("Cargo.toml"), "not [valid toml").expect("the manifest writes"); - std::fs::write(broken.join("build.rs"), CALLS_CHECK).expect("build.rs writes"); - let violations = ceiling_wiring_violations(root.path()); - assert_eq!(violations.len(), 1, "{violations:?}"); - assert!(violations[0].contains("broken")); - } - - #[test] - fn a_wiring_scan_that_finds_no_crate_fails() { - let root = tempfile::TempDir::new().expect("tempdir"); - std::fs::create_dir_all(root.path().join("crates")).expect("the crates directory creates"); - let violations = ceiling_wiring_violations(root.path()); - assert_eq!(violations.len(), 1, "{violations:?}"); - assert!(violations[0].contains("scanned nothing")); - } -} diff --git a/crates/build-xtask/src/tidy.rs b/crates/build-xtask/src/tidy.rs index 535c44167..0d4ecf2b0 100644 --- a/crates/build-xtask/src/tidy.rs +++ b/crates/build-xtask/src/tidy.rs @@ -5,25 +5,19 @@ //! `facade_shape`), the `doc(hidden)` ban over both facades and their //! containers (run from `doc_hidden`), the doctest ban over every //! workspace crate (run from `no_doctests`), the `Debug` trait's one -//! spelling over every workspace crate (run from `debug_spelling`), the -//! unsafe allowlist (run from `unsafe_allowlist`), and the ceiling wiring -//! (run from `wiring`). +//! spelling over every workspace crate (run from `debug_spelling`), and +//! the unsafe allowlist (run from `unsafe_allowlist`). //! //! Each check returns a list of human-readable violations. The `#[test]` //! wrappers assert the lists are empty, so `cargo test -p build-xtask` //! enforces the architecture; `cargo xtask tidy` prints the same report -//! on demand. The lint inheritance check binds every workspace crate, and -//! so does the ceiling wiring check, which requires every crate's build -//! script to run `build_ceiling::check()`, the compile-time 500-line file -//! limit. The shared walk's read failures are reported by -//! `marker_violations`, their one owner. +//! on demand. The lint inheritance check binds every workspace crate. The +//! shared walk's read failures are reported by `marker_violations`, their +//! one owner. use std::fs; use std::path::{Path, PathBuf}; -#[path = "tidy-wiring.rs"] -mod wiring; - /// A tiered workshop crate: its package name and its directory under the /// `crates/workshop/` container. type Tiered = (&'static str, &'static str); @@ -63,7 +57,6 @@ pub(crate) fn all_violations(root: &Path) -> Vec { let mut violations = tier_dependency_violations(root); violations.extend(marker_violations(root)); violations.extend(lint_inheritance_violations(root)); - violations.extend(wiring::ceiling_wiring_violations(root)); violations.extend(crate::unsafe_allowlist::unsafe_allowlist_violations(root)); violations.extend(crate::product::product_boundary_violations(root)); violations.extend(crate::harness_bans::harness_tokio_bans( diff --git a/crates/harness-gateway-client/build.rs b/crates/harness-gateway-client/build.rs deleted file mode 100644 index 435508aa3..000000000 --- a/crates/harness-gateway-client/build.rs +++ /dev/null @@ -1,13 +0,0 @@ -//! Includes the file-size check's source instead of depending on -//! `build-ceiling`, because Harness crates may name no `build-*` crate. - -#[expect( - unreachable_pub, - reason = "the check's public items are private to this build script" -)] -#[path = "../build-ceiling/src/lib.rs"] -mod build_ceiling; - -fn main() -> Result<(), build_ceiling::Violations> { - build_ceiling::check() -} diff --git a/crates/harness-internal/plugin-loader/build.rs b/crates/harness-internal/plugin-loader/build.rs deleted file mode 100644 index 1ae05563b..000000000 --- a/crates/harness-internal/plugin-loader/build.rs +++ /dev/null @@ -1,13 +0,0 @@ -//! Includes the file-size check's source instead of depending on -//! `build-ceiling`, because Harness crates may name no `build-*` crate. - -#[expect( - unreachable_pub, - reason = "the check's public items are private to this build script" -)] -#[path = "../../build-ceiling/src/lib.rs"] -mod build_ceiling; - -fn main() -> Result<(), build_ceiling::Violations> { - build_ceiling::check() -} diff --git a/crates/harness-internal/runner/build.rs b/crates/harness-internal/runner/build.rs deleted file mode 100644 index 1ae05563b..000000000 --- a/crates/harness-internal/runner/build.rs +++ /dev/null @@ -1,13 +0,0 @@ -//! Includes the file-size check's source instead of depending on -//! `build-ceiling`, because Harness crates may name no `build-*` crate. - -#[expect( - unreachable_pub, - reason = "the check's public items are private to this build script" -)] -#[path = "../../build-ceiling/src/lib.rs"] -mod build_ceiling; - -fn main() -> Result<(), build_ceiling::Violations> { - build_ceiling::check() -} diff --git a/crates/harness-internal/test-plugin/Cargo.toml b/crates/harness-internal/test-plugin/Cargo.toml index 1bb03bd39..76a70a26f 100644 --- a/crates/harness-internal/test-plugin/Cargo.toml +++ b/crates/harness-internal/test-plugin/Cargo.toml @@ -12,8 +12,7 @@ publish = false crate-type = ["cdylib", "rlib"] # No `[dependencies]` of any kind: a library Plugin needs nothing from Runvil, -# and this fixture proves it. The Harness may name no `build-*` crate, so -# `build.rs` includes the file-size check's source instead. +# and this fixture proves it. # Only the end-to-end tests in `tests/it` use these: the loader to insert # the built library, and the Plugin contract crate for `PackRegistry::new`. diff --git a/crates/harness-internal/test-plugin/build.rs b/crates/harness-internal/test-plugin/build.rs deleted file mode 100644 index 1ae05563b..000000000 --- a/crates/harness-internal/test-plugin/build.rs +++ /dev/null @@ -1,13 +0,0 @@ -//! Includes the file-size check's source instead of depending on -//! `build-ceiling`, because Harness crates may name no `build-*` crate. - -#[expect( - unreachable_pub, - reason = "the check's public items are private to this build script" -)] -#[path = "../../build-ceiling/src/lib.rs"] -mod build_ceiling; - -fn main() -> Result<(), build_ceiling::Violations> { - build_ceiling::check() -} diff --git a/crates/harness/build.rs b/crates/harness/build.rs deleted file mode 100644 index 435508aa3..000000000 --- a/crates/harness/build.rs +++ /dev/null @@ -1,13 +0,0 @@ -//! Includes the file-size check's source instead of depending on -//! `build-ceiling`, because Harness crates may name no `build-*` crate. - -#[expect( - unreachable_pub, - reason = "the check's public items are private to this build script" -)] -#[path = "../build-ceiling/src/lib.rs"] -mod build_ceiling; - -fn main() -> Result<(), build_ceiling::Violations> { - build_ceiling::check() -} diff --git a/crates/plugin-mcp/build.rs b/crates/plugin-mcp/build.rs deleted file mode 100644 index c03b9c50e..000000000 --- a/crates/plugin-mcp/build.rs +++ /dev/null @@ -1,13 +0,0 @@ -//! Includes the file-size check's source instead of depending on -//! `build-ceiling`, because Plugin crates may name no `build-*` crate. - -#[expect( - unreachable_pub, - reason = "the check's public items are private to this build script" -)] -#[path = "../build-ceiling/src/lib.rs"] -mod build_ceiling; - -fn main() -> Result<(), build_ceiling::Violations> { - build_ceiling::check() -} diff --git a/crates/plugin-std/build.rs b/crates/plugin-std/build.rs deleted file mode 100644 index c03b9c50e..000000000 --- a/crates/plugin-std/build.rs +++ /dev/null @@ -1,13 +0,0 @@ -//! Includes the file-size check's source instead of depending on -//! `build-ceiling`, because Plugin crates may name no `build-*` crate. - -#[expect( - unreachable_pub, - reason = "the check's public items are private to this build script" -)] -#[path = "../build-ceiling/src/lib.rs"] -mod build_ceiling; - -fn main() -> Result<(), build_ceiling::Violations> { - build_ceiling::check() -} diff --git a/crates/plugin-user-input/build.rs b/crates/plugin-user-input/build.rs deleted file mode 100644 index c03b9c50e..000000000 --- a/crates/plugin-user-input/build.rs +++ /dev/null @@ -1,13 +0,0 @@ -//! Includes the file-size check's source instead of depending on -//! `build-ceiling`, because Plugin crates may name no `build-*` crate. - -#[expect( - unreachable_pub, - reason = "the check's public items are private to this build script" -)] -#[path = "../build-ceiling/src/lib.rs"] -mod build_ceiling; - -fn main() -> Result<(), build_ceiling::Violations> { - build_ceiling::check() -} diff --git a/crates/plugin-web/build.rs b/crates/plugin-web/build.rs deleted file mode 100644 index c03b9c50e..000000000 --- a/crates/plugin-web/build.rs +++ /dev/null @@ -1,13 +0,0 @@ -//! Includes the file-size check's source instead of depending on -//! `build-ceiling`, because Plugin crates may name no `build-*` crate. - -#[expect( - unreachable_pub, - reason = "the check's public items are private to this build script" -)] -#[path = "../build-ceiling/src/lib.rs"] -mod build_ceiling; - -fn main() -> Result<(), build_ceiling::Violations> { - build_ceiling::check() -} diff --git a/crates/runvil-internal/engine/Cargo.toml b/crates/runvil-internal/engine/Cargo.toml index d7efaeeaa..c4129ed24 100644 --- a/crates/runvil-internal/engine/Cargo.toml +++ b/crates/runvil-internal/engine/Cargo.toml @@ -39,8 +39,5 @@ name = "models_loop" harness = false required-features = ["test-support"] -[build-dependencies] -build-ceiling.workspace = true - [lints] workspace = true diff --git a/crates/runvil-internal/engine/build.rs b/crates/runvil-internal/engine/build.rs deleted file mode 100644 index f62713c6f..000000000 --- a/crates/runvil-internal/engine/build.rs +++ /dev/null @@ -1,5 +0,0 @@ -//! Runs the compile-time file-size check on this crate. - -fn main() -> Result<(), build_ceiling::Violations> { - build_ceiling::check() -} diff --git a/crates/runvil-internal/engine/src/test_support/recording/observation.rs b/crates/runvil-internal/engine/src/test_support/recording/observation.rs index ab45d05a8..170baf993 100644 --- a/crates/runvil-internal/engine/src/test_support/recording/observation.rs +++ b/crates/runvil-internal/engine/src/test_support/recording/observation.rs @@ -4,9 +4,8 @@ //! expected sequences spell, and the stable trace rendering a recorder //! stores. //! -//! Kept beside the recorder rather than in it so each file stays inside -//! the repository's line ceiling; the recorder re-exports this module's -//! public items, so a suite names `recording::Observation`. +//! The recorder re-exports this module's public items, so a suite names +//! `recording::Observation`. use std::fmt; diff --git a/crates/runvil-internal/lua/Cargo.toml b/crates/runvil-internal/lua/Cargo.toml index fb6df1d29..a7fd9143e 100644 --- a/crates/runvil-internal/lua/Cargo.toml +++ b/crates/runvil-internal/lua/Cargo.toml @@ -31,8 +31,5 @@ name = "surface" harness = false required-features = ["test-support"] -[build-dependencies] -build-ceiling.workspace = true - [lints] workspace = true diff --git a/crates/runvil-internal/lua/build.rs b/crates/runvil-internal/lua/build.rs deleted file mode 100644 index f62713c6f..000000000 --- a/crates/runvil-internal/lua/build.rs +++ /dev/null @@ -1,5 +0,0 @@ -//! Runs the compile-time file-size check on this crate. - -fn main() -> Result<(), build_ceiling::Violations> { - build_ceiling::check() -} diff --git a/crates/runvil-internal/lua/src/coro.rs b/crates/runvil-internal/lua/src/coro.rs index b9330a839..3d10fd40d 100644 --- a/crates/runvil-internal/lua/src/coro.rs +++ b/crates/runvil-internal/lua/src/coro.rs @@ -50,16 +50,16 @@ const SHIM_SOURCE: &str = include_str!("__impl_coro.lua"); const TASKS_CHUNK_NAME: &str = "@crates/runvil-internal/lua/src/__impl_tasks.lua"; /// The `tasks` namespace source: spawn, the waits, the checks, note, and -/// cancel, split from the shim chunk so neither chunk outgrows the file -/// ceiling. It runs over the shim chunk's failure helpers. +/// cancel, in a chunk of its own beside the shim chunk. It runs over the +/// shim chunk's failure helpers. const TASKS_SOURCE: &str = include_str!("__impl_tasks.lua"); /// The `fanout` chunk's name. const FANOUT_CHUNK_NAME: &str = "@crates/runvil-internal/lua/src/__impl_fanout.lua"; /// The `fanout` shim source: Lua over the task protocol (`spawn`, -/// `join_any`, `cancel`), split from the shim chunk for the same -/// file-ceiling reason. It runs over the shim chunk's failure helpers plus +/// `join_any`, `cancel`), in a chunk of its own beside the shim chunk. It +/// runs over the shim chunk's failure helpers plus /// the collection enumerator and the item renderer; the scheduler's /// admission limit, not a captured cap, bounds how many arms run at once. const FANOUT_SOURCE: &str = include_str!("__impl_fanout.lua"); diff --git a/crates/runvil-internal/model-client/Cargo.toml b/crates/runvil-internal/model-client/Cargo.toml index 68dab8641..1d2057d45 100644 --- a/crates/runvil-internal/model-client/Cargo.toml +++ b/crates/runvil-internal/model-client/Cargo.toml @@ -14,8 +14,5 @@ runvil-types.workspace = true serde.workspace = true serde_json.workspace = true -[build-dependencies] -build-ceiling.workspace = true - [lints] workspace = true diff --git a/crates/runvil-internal/model-client/build.rs b/crates/runvil-internal/model-client/build.rs deleted file mode 100644 index f62713c6f..000000000 --- a/crates/runvil-internal/model-client/build.rs +++ /dev/null @@ -1,5 +0,0 @@ -//! Runs the compile-time file-size check on this crate. - -fn main() -> Result<(), build_ceiling::Violations> { - build_ceiling::check() -} diff --git a/crates/runvil-internal/prompt/Cargo.toml b/crates/runvil-internal/prompt/Cargo.toml index ca624d9c9..e06b9d525 100644 --- a/crates/runvil-internal/prompt/Cargo.toml +++ b/crates/runvil-internal/prompt/Cargo.toml @@ -24,8 +24,5 @@ runvil-lua = { workspace = true, features = ["test-support"] } # `runvil-engine` enables it from `[dev-dependencies]` only. test-support = [] -[build-dependencies] -build-ceiling.workspace = true - [lints] workspace = true diff --git a/crates/runvil-internal/prompt/build.rs b/crates/runvil-internal/prompt/build.rs deleted file mode 100644 index f62713c6f..000000000 --- a/crates/runvil-internal/prompt/build.rs +++ /dev/null @@ -1,5 +0,0 @@ -//! Runs the compile-time file-size check on this crate. - -fn main() -> Result<(), build_ceiling::Violations> { - build_ceiling::check() -} diff --git a/crates/runvil-internal/types/Cargo.toml b/crates/runvil-internal/types/Cargo.toml index 14b06f466..d1014a26d 100644 --- a/crates/runvil-internal/types/Cargo.toml +++ b/crates/runvil-internal/types/Cargo.toml @@ -18,8 +18,5 @@ sha2.workspace = true # against; the Engine itself never depends on `time`. time.workspace = true -[build-dependencies] -build-ceiling.workspace = true - [lints] workspace = true diff --git a/crates/runvil-internal/types/build.rs b/crates/runvil-internal/types/build.rs deleted file mode 100644 index f62713c6f..000000000 --- a/crates/runvil-internal/types/build.rs +++ /dev/null @@ -1,5 +0,0 @@ -//! Runs the compile-time file-size check on this crate. - -fn main() -> Result<(), build_ceiling::Violations> { - build_ceiling::check() -} diff --git a/crates/runvil-internal/vfs/build.rs b/crates/runvil-internal/vfs/build.rs deleted file mode 100644 index acd0a269b..000000000 --- a/crates/runvil-internal/vfs/build.rs +++ /dev/null @@ -1,13 +0,0 @@ -//! Includes the file-size check's source instead of depending on -//! `build-ceiling`, because this crate declares no build dependencies. - -#[expect( - unreachable_pub, - reason = "the check's public items are private to this build script" -)] -#[path = "../../build-ceiling/src/lib.rs"] -mod build_ceiling; - -fn main() -> Result<(), build_ceiling::Violations> { - build_ceiling::check() -} diff --git a/crates/runvil-plugin/Cargo.toml b/crates/runvil-plugin/Cargo.toml index e4e2e819d..e7482376c 100644 --- a/crates/runvil-plugin/Cargo.toml +++ b/crates/runvil-plugin/Cargo.toml @@ -19,8 +19,5 @@ serde_json.workspace = true # tests; Plugin crates enable it from `[dev-dependencies]` only. test-support = [] -[build-dependencies] -build-ceiling.workspace = true - [lints] workspace = true diff --git a/crates/runvil-plugin/build.rs b/crates/runvil-plugin/build.rs deleted file mode 100644 index f62713c6f..000000000 --- a/crates/runvil-plugin/build.rs +++ /dev/null @@ -1,5 +0,0 @@ -//! Runs the compile-time file-size check on this crate. - -fn main() -> Result<(), build_ceiling::Violations> { - build_ceiling::check() -} diff --git a/crates/runvil/Cargo.toml b/crates/runvil/Cargo.toml index 5ab9ab071..e97cd1b6c 100644 --- a/crates/runvil/Cargo.toml +++ b/crates/runvil/Cargo.toml @@ -23,8 +23,5 @@ runvil-vfs.workspace = true # as JSON. serde_json.workspace = true -[build-dependencies] -build-ceiling.workspace = true - [lints] workspace = true diff --git a/crates/runvil/build.rs b/crates/runvil/build.rs deleted file mode 100644 index f62713c6f..000000000 --- a/crates/runvil/build.rs +++ /dev/null @@ -1,5 +0,0 @@ -//! Runs the compile-time file-size check on this crate. - -fn main() -> Result<(), build_ceiling::Violations> { - build_ceiling::check() -} diff --git a/crates/shared-error-source/Cargo.toml b/crates/shared-error-source/Cargo.toml index 339a2e090..206e1eba4 100644 --- a/crates/shared-error-source/Cargo.toml +++ b/crates/shared-error-source/Cargo.toml @@ -20,8 +20,5 @@ json = ["dep:serde_json"] http = ["dep:reqwest"] database = ["dep:turso"] -[build-dependencies] -build-ceiling.workspace = true - [lints] workspace = true diff --git a/crates/shared-error-source/build.rs b/crates/shared-error-source/build.rs deleted file mode 100644 index f62713c6f..000000000 --- a/crates/shared-error-source/build.rs +++ /dev/null @@ -1,5 +0,0 @@ -//! Runs the compile-time file-size check on this crate. - -fn main() -> Result<(), build_ceiling::Violations> { - build_ceiling::check() -} diff --git a/crates/workshop/agents/Cargo.toml b/crates/workshop/agents/Cargo.toml index 3d900ba10..07aede2aa 100644 --- a/crates/workshop/agents/Cargo.toml +++ b/crates/workshop/agents/Cargo.toml @@ -35,8 +35,5 @@ tracing.workspace = true tempfile.workspace = true tokio = { workspace = true, features = ["macros", "rt-multi-thread"] } -[build-dependencies] -build-ceiling.workspace = true - [lints] workspace = true diff --git a/crates/workshop/agents/build.rs b/crates/workshop/agents/build.rs deleted file mode 100644 index f62713c6f..000000000 --- a/crates/workshop/agents/build.rs +++ /dev/null @@ -1,5 +0,0 @@ -//! Runs the compile-time file-size check on this crate. - -fn main() -> Result<(), build_ceiling::Violations> { - build_ceiling::check() -} diff --git a/crates/workshop/desktop/Cargo.toml b/crates/workshop/desktop/Cargo.toml index a5d295c91..beba53e74 100644 --- a/crates/workshop/desktop/Cargo.toml +++ b/crates/workshop/desktop/Cargo.toml @@ -52,7 +52,6 @@ version = "0.61" version = "2.0.2" [build-dependencies] -build-ceiling.workspace = true tauri-build.workspace = true [features] diff --git a/crates/workshop/desktop/build.rs b/crates/workshop/desktop/build.rs index 8cf15a419..8ae742a6d 100644 --- a/crates/workshop/desktop/build.rs +++ b/crates/workshop/desktop/build.rs @@ -27,7 +27,6 @@ use std::path::{Path, PathBuf}; const GATEWAY_SIDECAR_VARIABLE: &str = "RUNVIL_GATEWAY_SIDECAR"; fn main() -> Result<(), Box> { - build_ceiling::check()?; install_gateway_sidecar()?; tauri_build::try_build(tauri_build::Attributes::new().app_manifest( tauri_build::AppManifest::new().commands(&["desktop_update_supported", "quit"]), diff --git a/crates/workshop/gateway-discovery/Cargo.toml b/crates/workshop/gateway-discovery/Cargo.toml index 56d2d1a42..8a86b26a1 100644 --- a/crates/workshop/gateway-discovery/Cargo.toml +++ b/crates/workshop/gateway-discovery/Cargo.toml @@ -39,8 +39,5 @@ tempfile.workspace = true # without `--all-features`. workshop-gateway-discovery = { path = ".", features = ["test-fixtures"] } -[build-dependencies] -build-ceiling.workspace = true - [lints] workspace = true diff --git a/crates/workshop/gateway-discovery/build.rs b/crates/workshop/gateway-discovery/build.rs deleted file mode 100644 index f62713c6f..000000000 --- a/crates/workshop/gateway-discovery/build.rs +++ /dev/null @@ -1,5 +0,0 @@ -//! Runs the compile-time file-size check on this crate. - -fn main() -> Result<(), build_ceiling::Violations> { - build_ceiling::check() -} diff --git a/crates/workshop/gateway/Cargo.toml b/crates/workshop/gateway/Cargo.toml index 6cc9135e6..815d0c712 100644 --- a/crates/workshop/gateway/Cargo.toml +++ b/crates/workshop/gateway/Cargo.toml @@ -43,7 +43,6 @@ workshop-support = { workspace = true, features = ["test-fixtures"] } workshop-gateway-discovery = { workspace = true, features = ["test-fixtures"] } [build-dependencies] -build-ceiling.workspace = true serde_json.workspace = true [lints] diff --git a/crates/workshop/gateway/build.rs b/crates/workshop/gateway/build.rs index 7c8c80a14..d0e8dc8c2 100644 --- a/crates/workshop/gateway/build.rs +++ b/crates/workshop/gateway/build.rs @@ -1,6 +1,5 @@ //! Embeds the Gateway pin's `min_gateway` so Workshop can compare it with -//! the version of the Gateway it attaches to, then runs the compile-time -//! file-size check. +//! the version of the Gateway it attaches to. //! //! The pin is one file for the whole repository, and this crate is where //! the Gateway version is known, so the value is baked into this crate's @@ -8,9 +7,8 @@ use std::path::PathBuf; -fn main() -> Result<(), build_ceiling::Violations> { +fn main() { embed_min_gateway(); - build_ceiling::check() } /// Reads `gateway-pin/pin.json` and sets `RUNVIL_MIN_GATEWAY` to its diff --git a/crates/workshop/menu/Cargo.toml b/crates/workshop/menu/Cargo.toml index 836bf4233..04e7e15bc 100644 --- a/crates/workshop/menu/Cargo.toml +++ b/crates/workshop/menu/Cargo.toml @@ -20,8 +20,5 @@ workshop-support.workspace = true tempfile.workspace = true tokio = { workspace = true, features = ["macros", "rt-multi-thread"] } -[build-dependencies] -build-ceiling.workspace = true - [lints] workspace = true diff --git a/crates/workshop/menu/build.rs b/crates/workshop/menu/build.rs deleted file mode 100644 index f62713c6f..000000000 --- a/crates/workshop/menu/build.rs +++ /dev/null @@ -1,5 +0,0 @@ -//! Runs the compile-time file-size check on this crate. - -fn main() -> Result<(), build_ceiling::Violations> { - build_ceiling::check() -} diff --git a/crates/workshop/protocol/Cargo.toml b/crates/workshop/protocol/Cargo.toml index ba06381bf..87116baa4 100644 --- a/crates/workshop/protocol/Cargo.toml +++ b/crates/workshop/protocol/Cargo.toml @@ -10,8 +10,5 @@ repository.workspace = true serde.workspace = true serde_json.workspace = true -[build-dependencies] -build-ceiling.workspace = true - [lints] workspace = true diff --git a/crates/workshop/protocol/build.rs b/crates/workshop/protocol/build.rs deleted file mode 100644 index f62713c6f..000000000 --- a/crates/workshop/protocol/build.rs +++ /dev/null @@ -1,5 +0,0 @@ -//! Runs the compile-time file-size check on this crate. - -fn main() -> Result<(), build_ceiling::Violations> { - build_ceiling::check() -} diff --git a/crates/workshop/registry/Cargo.toml b/crates/workshop/registry/Cargo.toml index 8bc701c10..a5d67529d 100644 --- a/crates/workshop/registry/Cargo.toml +++ b/crates/workshop/registry/Cargo.toml @@ -15,8 +15,5 @@ workshop-protocol.workspace = true [dev-dependencies] tokio = { workspace = true, features = ["macros", "rt-multi-thread"] } -[build-dependencies] -build-ceiling.workspace = true - [lints] workspace = true diff --git a/crates/workshop/registry/build.rs b/crates/workshop/registry/build.rs deleted file mode 100644 index f62713c6f..000000000 --- a/crates/workshop/registry/build.rs +++ /dev/null @@ -1,5 +0,0 @@ -//! Runs the compile-time file-size check on this crate. - -fn main() -> Result<(), build_ceiling::Violations> { - build_ceiling::check() -} diff --git a/crates/workshop/run-log/Cargo.toml b/crates/workshop/run-log/Cargo.toml index 306b3dae5..0c840fbb7 100644 --- a/crates/workshop/run-log/Cargo.toml +++ b/crates/workshop/run-log/Cargo.toml @@ -24,8 +24,5 @@ turso.workspace = true tempfile.workspace = true tokio = { workspace = true, features = ["macros", "rt-multi-thread"] } -[build-dependencies] -build-ceiling.workspace = true - [lints] workspace = true diff --git a/crates/workshop/run-log/build.rs b/crates/workshop/run-log/build.rs deleted file mode 100644 index f62713c6f..000000000 --- a/crates/workshop/run-log/build.rs +++ /dev/null @@ -1,5 +0,0 @@ -//! Runs the compile-time file-size check on this crate. - -fn main() -> Result<(), build_ceiling::Violations> { - build_ceiling::check() -} diff --git a/crates/workshop/server-api/Cargo.toml b/crates/workshop/server-api/Cargo.toml index 6f163341f..80ac7ba4a 100644 --- a/crates/workshop/server-api/Cargo.toml +++ b/crates/workshop/server-api/Cargo.toml @@ -18,8 +18,5 @@ workshop-server.workspace = true [dev-dependencies] workshop-server-api = { path = ".", features = ["test-fixtures"] } -[build-dependencies] -build-ceiling.workspace = true - [lints] workspace = true diff --git a/crates/workshop/server-api/build.rs b/crates/workshop/server-api/build.rs deleted file mode 100644 index f62713c6f..000000000 --- a/crates/workshop/server-api/build.rs +++ /dev/null @@ -1,5 +0,0 @@ -//! Runs the compile-time file-size check on this crate. - -fn main() -> Result<(), build_ceiling::Violations> { - build_ceiling::check() -} diff --git a/crates/workshop/server/Cargo.toml b/crates/workshop/server/Cargo.toml index c0bc6705c..bd2cabef6 100644 --- a/crates/workshop/server/Cargo.toml +++ b/crates/workshop/server/Cargo.toml @@ -77,7 +77,6 @@ tower.workspace = true # The build script bundles the UI with esbuild into OUT_DIR through the # shared helper; nothing UI-built lands in the repository. [build-dependencies] -build-ceiling.workspace = true build-ui = { path = "../../build-ui" } [lints] diff --git a/crates/workshop/server/build.rs b/crates/workshop/server/build.rs index 665278767..a69db7781 100644 --- a/crates/workshop/server/build.rs +++ b/crates/workshop/server/build.rs @@ -7,10 +7,6 @@ //! crates/workshop` from the repository root). fn main() -> std::process::ExitCode { - if let Err(error) = build_ceiling::check() { - eprintln!("{error}"); - return std::process::ExitCode::FAILURE; - } match build_ui::build_sibling( "../ui", build_ui::UiBuild { diff --git a/crates/workshop/status/Cargo.toml b/crates/workshop/status/Cargo.toml index 870c081b9..6a6774012 100644 --- a/crates/workshop/status/Cargo.toml +++ b/crates/workshop/status/Cargo.toml @@ -15,8 +15,5 @@ workshop-support.workspace = true [dev-dependencies] tokio = { workspace = true, features = ["macros", "rt-multi-thread", "test-util"] } -[build-dependencies] -build-ceiling.workspace = true - [lints] workspace = true diff --git a/crates/workshop/status/build.rs b/crates/workshop/status/build.rs deleted file mode 100644 index f62713c6f..000000000 --- a/crates/workshop/status/build.rs +++ /dev/null @@ -1,5 +0,0 @@ -//! Runs the compile-time file-size check on this crate. - -fn main() -> Result<(), build_ceiling::Violations> { - build_ceiling::check() -} diff --git a/crates/workshop/support/Cargo.toml b/crates/workshop/support/Cargo.toml index 1b7d7d3ab..016f6ccca 100644 --- a/crates/workshop/support/Cargo.toml +++ b/crates/workshop/support/Cargo.toml @@ -27,8 +27,5 @@ tempfile.workspace = true tokio = { workspace = true, features = ["test-util"] } tower.workspace = true -[build-dependencies] -build-ceiling.workspace = true - [lints] workspace = true diff --git a/crates/workshop/support/build.rs b/crates/workshop/support/build.rs deleted file mode 100644 index f62713c6f..000000000 --- a/crates/workshop/support/build.rs +++ /dev/null @@ -1,5 +0,0 @@ -//! Runs the compile-time file-size check on this crate. - -fn main() -> Result<(), build_ceiling::Violations> { - build_ceiling::check() -} diff --git a/crates/workshop/user-state/Cargo.toml b/crates/workshop/user-state/Cargo.toml index edd23e216..3487a054c 100644 --- a/crates/workshop/user-state/Cargo.toml +++ b/crates/workshop/user-state/Cargo.toml @@ -25,8 +25,5 @@ tempfile.workspace = true tokio = { workspace = true, features = ["macros", "rt-multi-thread"] } tower.workspace = true -[build-dependencies] -build-ceiling.workspace = true - [lints] workspace = true diff --git a/crates/workshop/user-state/build.rs b/crates/workshop/user-state/build.rs deleted file mode 100644 index f62713c6f..000000000 --- a/crates/workshop/user-state/build.rs +++ /dev/null @@ -1,5 +0,0 @@ -//! Runs the compile-time file-size check on this crate. - -fn main() -> Result<(), build_ceiling::Violations> { - build_ceiling::check() -} diff --git a/crates/workshop/workspace/Cargo.toml b/crates/workshop/workspace/Cargo.toml index 9822bf3e0..42e20f31d 100644 --- a/crates/workshop/workspace/Cargo.toml +++ b/crates/workshop/workspace/Cargo.toml @@ -38,8 +38,5 @@ tempfile.workspace = true tokio = { workspace = true, features = ["macros", "rt-multi-thread"] } tower.workspace = true -[build-dependencies] -build-ceiling.workspace = true - [lints] workspace = true diff --git a/crates/workshop/workspace/build.rs b/crates/workshop/workspace/build.rs deleted file mode 100644 index f62713c6f..000000000 --- a/crates/workshop/workspace/build.rs +++ /dev/null @@ -1,5 +0,0 @@ -//! Runs the compile-time file-size check on this crate. - -fn main() -> Result<(), build_ceiling::Violations> { - build_ceiling::check() -} diff --git a/vibelight.toml b/vibelight.toml index f591e3779..b1d4cf98a 100644 --- a/vibelight.toml +++ b/vibelight.toml @@ -3,7 +3,6 @@ workspace = true follow = ["**"] exclude = [ - "crates/build-ceiling", # build tooling "crates/build-ui", # build tooling "crates/build-user-guide", # build tooling "crates/build-workshop", # run by the cargo alias `workshop` From a532fdf4c9c65752bfe574415212f87b28f3a45c Mon Sep 17 00:00:00 2001 From: Vinnie Falco Date: Fri, 6 Nov 2026 02:48:43 -0800 Subject: [PATCH 4/6] Set clippy's function length and argument limits This commit writes the function size limits into the clippy configuration, so that a reader can see the size rule there. A new comment gives the rule: size limits are on functions and types, and a file has no length limit. The root `clippy.toml` and the three Engine files under `crates/` each get the comment, `too-many-lines-threshold = 100`, and `too-many-arguments-threshold = 7`. - Clippy does not merge an Engine crate's `clippy.toml` with the root file, so each Engine file repeats the two root settings. The three Engine files stay byte-identical, and a `build-xtask` test checks that each one contains every root entry. - Both values are clippy's defaults, so the lint results do not change. - The new comment names a `build-xtask` type-size check. This commit does not add that check. --- clippy.toml | 5 +++++ crates/runvil-internal/clippy.toml | 5 +++++ crates/runvil-plugin/clippy.toml | 5 +++++ crates/runvil/clippy.toml | 5 +++++ 4 files changed, 20 insertions(+) diff --git a/clippy.toml b/clippy.toml index 36460a2eb..00803dd21 100644 --- a/clippy.toml +++ b/clippy.toml @@ -1,6 +1,11 @@ allow-unwrap-in-tests = true allow-expect-in-tests = true +# Size limits sit on functions (here) and on types (build-xtask's +# type-size check). A file has no length limit. +too-many-lines-threshold = 100 +too-many-arguments-threshold = 7 + # Process-global installers. Only a binary entry point may call one, under # `#[expect(clippy::disallowed_methods, reason = ...)]`. No entry names a # `replacement`, because no alternative takes the same arguments and returns diff --git a/crates/runvil-internal/clippy.toml b/crates/runvil-internal/clippy.toml index 03d12f2d4..759d96bfa 100644 --- a/crates/runvil-internal/clippy.toml +++ b/crates/runvil-internal/clippy.toml @@ -10,6 +10,11 @@ allow-unwrap-in-tests = true allow-expect-in-tests = true +# Size limits sit on functions (here) and on types (build-xtask's +# type-size check). A file has no length limit. +too-many-lines-threshold = 100 +too-many-arguments-threshold = 7 + # The first seven entries are the root file's process-global installers. The # rest are the determinism bans: no Engine code, tests included, sleeps, reads # the wall clock, waits with a timeout, or draws entropy from the OS. No entry diff --git a/crates/runvil-plugin/clippy.toml b/crates/runvil-plugin/clippy.toml index 03d12f2d4..759d96bfa 100644 --- a/crates/runvil-plugin/clippy.toml +++ b/crates/runvil-plugin/clippy.toml @@ -10,6 +10,11 @@ allow-unwrap-in-tests = true allow-expect-in-tests = true +# Size limits sit on functions (here) and on types (build-xtask's +# type-size check). A file has no length limit. +too-many-lines-threshold = 100 +too-many-arguments-threshold = 7 + # The first seven entries are the root file's process-global installers. The # rest are the determinism bans: no Engine code, tests included, sleeps, reads # the wall clock, waits with a timeout, or draws entropy from the OS. No entry diff --git a/crates/runvil/clippy.toml b/crates/runvil/clippy.toml index 03d12f2d4..759d96bfa 100644 --- a/crates/runvil/clippy.toml +++ b/crates/runvil/clippy.toml @@ -10,6 +10,11 @@ allow-unwrap-in-tests = true allow-expect-in-tests = true +# Size limits sit on functions (here) and on types (build-xtask's +# type-size check). A file has no length limit. +too-many-lines-threshold = 100 +too-many-arguments-threshold = 7 + # The first seven entries are the root file's process-global installers. The # rest are the determinism bans: no Engine code, tests included, sleeps, reads # the wall clock, waits with a timeout, or draws entropy from the OS. No entry From 5841f9df7638f773cdf7184b42aaf410fd3c8472 Mon Sep 17 00:00:00 2001 From: Vinnie Falco Date: Fri, 6 Nov 2026 03:10:17 -0800 Subject: [PATCH 5/6] Add build-xtask type-size check with an allowlist A type can spread its methods over any number of files, so this change puts size limits on types, not files. The `type_size` check fails a type with more than `MAX_METHODS` (25) inherent methods in its crate, or a struct with more than `MAX_FIELDS` (12) named fields. `ALLOWED` lifts a limit for listed types, and both `tidy.rs` and an inline `#[test]` run the check. - The two bounds keep `ALLOWED` short: it holds 12 entries across five crates. - The check sums methods by crate and by the last path segment of the `impl` self type, so the typestate impls of `SectionVm` merge into one count. The same rule sums two types of one name in one crate, and the syntax-only scan does not see an `impl` that a macro expands to. - Each `Allowed` entry stores a ceiling in `recorded`, not an exact count, so a type can shrink without an edit to `ALLOWED`. The type fails one past `recorded`, and the entry fails when its `reason` is blank or its type is gone or back within the limit. - The scan finds test code by file name and by `cfg`, not by a walk of the module tree, to keep it simple. As a result, it still reads a test-support file that only a gated `mod` line reaches, which gives the test-only `Scenario` its entry. - The check skips any item behind a `cfg` that names `test` outside `not(...)`. So it skips an item behind `cfg(any(test, feature = "..."))`, although a build with that feature compiles it. - The check reports an unreadable or unparseable file as a violation and does not skip it, and a scan that reads no `.rs` file fails. The scan also reads the crates in `walk.unread`, whose manifest the shared walk could not read. - No fixture test covers an unreadable source file or a struct behind its own test `cfg`. --- crates/build-xtask/src/main.rs | 1 + crates/build-xtask/src/tidy.rs | 6 +- crates/build-xtask/src/type_size.rs | 798 ++++++++++++++++++++++++++++ 3 files changed, 803 insertions(+), 2 deletions(-) create mode 100644 crates/build-xtask/src/type_size.rs diff --git a/crates/build-xtask/src/main.rs b/crates/build-xtask/src/main.rs index f542a61bf..6493312e3 100644 --- a/crates/build-xtask/src/main.rs +++ b/crates/build-xtask/src/main.rs @@ -24,6 +24,7 @@ mod product; mod site; mod test_support_leak; mod tidy; +mod type_size; mod unsafe_allowlist; use std::path::Path; diff --git a/crates/build-xtask/src/tidy.rs b/crates/build-xtask/src/tidy.rs index 0d4ecf2b0..8a69b8fd8 100644 --- a/crates/build-xtask/src/tidy.rs +++ b/crates/build-xtask/src/tidy.rs @@ -5,8 +5,9 @@ //! `facade_shape`), the `doc(hidden)` ban over both facades and their //! containers (run from `doc_hidden`), the doctest ban over every //! workspace crate (run from `no_doctests`), the `Debug` trait's one -//! spelling over every workspace crate (run from `debug_spelling`), and -//! the unsafe allowlist (run from `unsafe_allowlist`). +//! spelling over every workspace crate (run from `debug_spelling`), the +//! unsafe allowlist (run from `unsafe_allowlist`), and the type-size +//! limits over every workspace crate (run from `type_size`). //! //! Each check returns a list of human-readable violations. The `#[test]` //! wrappers assert the lists are empty, so `cargo test -p build-xtask` @@ -68,6 +69,7 @@ pub(crate) fn all_violations(root: &Path) -> Vec { violations.extend(crate::doc_hidden::doc_hidden_violations(root)); violations.extend(crate::no_doctests::no_doctests_violations(root)); violations.extend(crate::debug_spelling::debug_spelling_violations(root)); + violations.extend(crate::type_size::type_size_violations(root)); violations } diff --git a/crates/build-xtask/src/type_size.rs b/crates/build-xtask/src/type_size.rs new file mode 100644 index 000000000..9d7d50e7a --- /dev/null +++ b/crates/build-xtask/src/type_size.rs @@ -0,0 +1,798 @@ +//! The type-size limits: a type's inherent methods, summed across every +//! `impl` block in its crate, stay at or under [`MAX_METHODS`], and a +//! struct's named fields stay at or under [`MAX_FIELDS`], unless +//! [`ALLOWED`] records the type with a reason. Runs as part of +//! `cargo test -p build-xtask` and `cargo xtask tidy`. +//! +//! The limits sit on types because a file has no length limit: a type's +//! methods may live in any number of files, so the count sums every +//! inherent `impl` block in the crate. Trait impls are not counted, +//! because the trait fixes their methods. +//! +//! The check reads syntax, so it keys a type by its crate and the last +//! segment of the name an `impl` gives it: two types of one name in one +//! crate are summed, and an `impl` that a macro expands to is not seen. +//! It reads each crate's `src/` tree, skipping test files by name and +//! every item behind a `cfg` that names `test`; a test-support file that +//! only a gated `mod` line reaches is still read. + +use std::collections::BTreeMap; +use std::fs; +use std::path::{Path, PathBuf}; + +use proc_macro2::{TokenStream, TokenTree}; +use syn::{Attribute, Fields, Ident, ImplItem, Item, Meta, Type}; + +/// The most inherent methods one type may have, summed across every +/// `impl` block in its crate. +const MAX_METHODS: usize = 25; + +/// The most named fields one struct may have. +const MAX_FIELDS: usize = 12; + +/// The types over a limit today. A new entry is a plan decision with a +/// reason, never a way to make a change pass. +const ALLOWED: &[Allowed] = &[ + Allowed { + krate: "crates/runvil-internal/engine", + name: "ChainOps", + limit: Limit::Methods, + recorded: 34, + reason: "The one-chain request view: by design, every request that touches only its \ + own chain runs here, in an `impl` block beside the arm it serves.", + }, + Allowed { + krate: "crates/runvil-internal/engine", + name: "Scheduler", + limit: Limit::Methods, + recorded: 72, + reason: "The run's protocol orchestrator: the 2026-10-26 split moved its bookkeeping \ + into four state owners, and each remaining method runs the run's lifecycle or \ + coordinates two or more of them (12 are test hooks).", + }, + Allowed { + krate: "crates/runvil-internal/engine", + name: "Chain", + limit: Limit::Fields, + recorded: 16, + reason: "One chain's whole state, its identity, entered section, walk, `var`, and \ + per-chain handles, which the chain arena owns as one value.", + }, + Allowed { + krate: "crates/runvil-internal/engine", + name: "RunParams", + limit: Limit::Fields, + recorded: 13, + reason: "The Engine's whole input for one run, built through `new` and the `with_*` \ + methods; each field is one input the caller supplies.", + }, + Allowed { + krate: "crates/runvil-internal/engine", + name: "Scenario", + limit: Limit::Fields, + recorded: 14, + reason: "A test-only run setup as data; each field is one setting a suite chooses \ + before it runs or steps the scenario.", + }, + Allowed { + krate: "crates/runvil-internal/engine", + name: "SectionVmSetup", + limit: Limit::Fields, + recorded: 14, + reason: "A parameter object: the borrowed inputs every section VM setup shares, \ + bundled so each frame constructor passes one set of borrows instead of a \ + dozen parameters.", + }, + Allowed { + krate: "crates/runvil-internal/engine", + name: "SharedState", + limit: Limit::Fields, + recorded: 14, + reason: "The run-wide state every chain of one run shares through one `Arc`, built \ + once with the root chain and never forked; every field is the same for every \ + chain.", + }, + Allowed { + krate: "crates/runvil-internal/lua", + name: "SectionVm", + limit: Limit::Methods, + recorded: 46, + reason: "One hardened Lua VM per section, whose methods follow its typestate \ + lifecycle: construction, hardening, injection, the Engine globals' \ + installers, and the block runs.", + }, + Allowed { + krate: "crates/runvil-internal/lua", + name: "SectionVm", + limit: Limit::Fields, + recorded: 17, + reason: "One hardened Lua VM per section; each field is a per-section handle the \ + Engine globals and the executor share, such as the tool set, the offers, and \ + the jump and loop flags.", + }, + Allowed { + krate: "crates/runvil-internal/vfs", + name: "Access", + limit: Limit::Methods, + recorded: 29, + reason: "The filesystem capability one identity holds: each method is one operation \ + through the VFS with its path, policy, and claims checks, so the count \ + follows the operation set.", + }, + Allowed { + krate: "crates/workshop/gateway-discovery", + name: "ProcBsdInfo", + limit: Limit::Fields, + recorded: 22, + reason: "A `#[repr(C)]` mirror of the stable prefix of Darwin's `proc_bsdinfo`, \ + whose fields the OS fixes.", + }, + Allowed { + krate: "crates/workshop/workspace", + name: "Workspace", + limit: Limit::Methods, + recorded: 39, + reason: "The workspace every route shares: the grant set and confinement plus the \ + backing file's open, save-as, duplicate, and persistence, on one cloneable \ + state so a new grant reaches every handler at once.", + }, +]; + +/// The limit an [`Allowed`] entry lifts. +#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] +enum Limit { + /// [`MAX_METHODS`]. + Methods, + /// [`MAX_FIELDS`]. + Fields, +} + +impl Limit { + /// The count every type without an entry is held to. + fn bound(self) -> usize { + match self { + Limit::Methods => MAX_METHODS, + Limit::Fields => MAX_FIELDS, + } + } + + /// What the limit counts, as a violation names it. + fn noun(self) -> &'static str { + match self { + Limit::Methods => "inherent methods across its `impl` blocks", + Limit::Fields => "named fields", + } + } + + /// The rule a type without an entry breaks by passing the limit. + fn rule(self) -> &'static str { + match self { + Limit::Methods => "type over the inherent method limit", + Limit::Fields => "struct over the named field limit", + } + } +} + +/// A type that may pass one limit, up to the count recorded when the +/// entry was written. The type may shrink without the entry changing; it +/// fails when it grows past `recorded`, and the entry fails as stale once +/// the type is gone or back within the limit. +struct Allowed { + /// The crate directory, relative to the workspace root, with `/` + /// separators. + krate: &'static str, + /// The type's name. + name: &'static str, + /// The limit the entry lifts. + limit: Limit, + /// The most the type may count; one more fails. + recorded: usize, + /// What the type is and why its size holds, in one sentence. + reason: &'static str, +} + +/// Each counted type's count, keyed by crate directory, type name, and +/// limit, for the stale-entry pass. +type Counts = BTreeMap<(String, String, Limit), usize>; + +/// A definition or `impl` block: its file, relative to the workspace root +/// with `/` separators, and its line. +#[derive(Clone)] +struct Site { + path: String, + line: usize, +} + +/// One type's inherent methods: its first `impl` block, and how many +/// methods each file, relative to the crate, gives it. +struct Tally { + first: Site, + files: BTreeMap, +} + +impl Tally { + /// Each file's share, as `src/a.rs 8, src/b.rs 3`. + fn breakdown(&self) -> String { + self.files + .iter() + .map(|(file, count)| format!("{file} {count}")) + .collect::>() + .join(", ") + } +} + +/// What one crate's sources hold. +#[derive(Default)] +struct Census { + /// Each type's inherent methods, by type name. + methods: BTreeMap, + /// Each struct with named fields: its name, its definition, and its + /// field count. + structs: Vec<(String, Site, usize)>, + /// The first struct, enum, or union definition of each name. + defined: BTreeMap, +} + +/// Runs the limits over every workspace crate's `src/` tree, the crates +/// whose manifest the shared walk could not read included, against +/// [`ALLOWED`]. An unreadable or unparseable file is reported, not +/// skipped, and a scan that read no source at all fails: a file that was +/// never read cannot be shown within the limits. +#[must_use] +pub(crate) fn type_size_violations(root: &Path) -> Vec { + violations_against(root, ALLOWED) +} + +/// [`type_size_violations`] against any allowlist, so a fixture can pass +/// its own. +fn violations_against(root: &Path, allowed: &[Allowed]) -> Vec { + let walk = crate::product::workspace_crates(root); + let mut dirs: Vec<&PathBuf> = walk + .crates + .iter() + .map(|krate| &krate.dir) + .chain(&walk.unread) + .collect(); + dirs.sort(); + let mut violations = Vec::new(); + let mut counts = Counts::new(); + let mut read = 0; + for dir in dirs { + let crate_dir = root.join(dir); + let src = crate_dir.join("src"); + let mut files: Vec = crate::tidy::rust_files(&src) + .into_iter() + .filter(|file| !is_test_file(&crate::tidy::slash_path(&src, file))) + .collect(); + files.sort(); + read += files.len(); + let mut census = Census::default(); + for file in &files { + census.read(root, &crate_dir, file, &mut violations); + } + let krate = crate::tidy::slash_path(root, &crate_dir); + census.judge(&krate, allowed, &mut counts, &mut violations); + } + if read == 0 { + return vec![format!( + "{}: the type-size check read nothing: no workspace crate holds a `.rs` file under \ + `src/`, so none can be shown within the limits", + root.join("crates").display() + )]; + } + violations.extend(entry_violations(allowed, &counts)); + violations +} + +impl Census { + /// Parses `file` and records its types, or reports why it could not. + fn read(&mut self, root: &Path, crate_dir: &Path, file: &Path, violations: &mut Vec) { + let path = crate::tidy::slash_path(root, file); + let text = match fs::read_to_string(file) { + Ok(text) => text, + Err(error) => { + violations.push(format!("{path}: unreadable source: {error}")); + return; + } + }; + match syn::parse_file(&text) { + Ok(parsed) if gated_on_test(&parsed.attrs) => {} + Ok(parsed) => { + let local = crate::tidy::slash_path(crate_dir, file); + self.items(&parsed.items, &path, &local); + } + Err(error) => violations.push(format!( + "{path}:{}: unparseable source: required a file that parses as Rust, found {error}", + error.span().start().line + )), + } + } + + /// Records the inherent methods, structs, and definitions in `items`, + /// inline modules included, from the file at `path` (`local` within + /// its crate). + fn items(&mut self, items: &[Item], path: &str, local: &str) { + for item in items { + match item { + Item::Impl(item) if item.trait_.is_none() && !gated_on_test(&item.attrs) => { + let Some(name) = self_type_name(&item.self_ty) else { + continue; + }; + let count = item + .items + .iter() + .filter(|member| { + matches!(member, ImplItem::Fn(function) if !gated_on_test(&function.attrs)) + }) + .count(); + let tally = self.methods.entry(name).or_insert_with(|| Tally { + first: Site { + path: path.to_owned(), + line: item.impl_token.span.start().line, + }, + files: BTreeMap::new(), + }); + *tally.files.entry(local.to_owned()).or_default() += count; + } + Item::Struct(item) if !gated_on_test(&item.attrs) => { + if let Fields::Named(fields) = &item.fields { + self.structs.push(( + item.ident.to_string(), + site(&item.ident, path), + fields.named.len(), + )); + } + self.define(&item.ident, path); + } + Item::Enum(item) if !gated_on_test(&item.attrs) => self.define(&item.ident, path), + Item::Union(item) if !gated_on_test(&item.attrs) => self.define(&item.ident, path), + Item::Mod(item) if !gated_on_test(&item.attrs) => { + if let Some((_, items)) = &item.content { + self.items(items, path, local); + } + } + _ => {} + } + } + } + + /// Records `ident`'s definition in `path`, unless the name has one. + fn define(&mut self, ident: &Ident, path: &str) { + self.defined + .entry(ident.to_string()) + .or_insert_with(|| site(ident, path)); + } + + /// Judges this crate's counts against the limits and `allowed`, and + /// records each count in `counts` for [`entry_violations`]. + fn judge( + &self, + krate: &str, + allowed: &[Allowed], + counts: &mut Counts, + violations: &mut Vec, + ) { + for (name, tally) in &self.methods { + let total: usize = tally.files.values().sum(); + counts.insert((krate.to_owned(), name.clone(), Limit::Methods), total); + let site = self.defined.get(name).unwrap_or(&tally.first); + let found = format!("`{name}` with {total} ({})", tally.breakdown()); + let entry = entry(allowed, krate, name, Limit::Methods); + violations.extend(judge_one(site, entry, Limit::Methods, total, &found)); + } + for (name, site, fields) in &self.structs { + let count = counts + .entry((krate.to_owned(), name.clone(), Limit::Fields)) + .or_default(); + *count = (*count).max(*fields); + let found = format!("`{name}` with {fields}"); + let entry = entry(allowed, krate, name, Limit::Fields); + violations.extend(judge_one(site, entry, Limit::Fields, *fields, &found)); + } + } +} + +/// The site of the item `ident` names, in `path`. +fn site(ident: &Ident, path: &str) -> Site { + Site { + path: path.to_owned(), + line: ident.span().start().line, + } +} + +/// The entry in `allowed` for `name` in the crate at `krate` under `limit`. +fn entry<'a>(allowed: &'a [Allowed], krate: &str, name: &str, limit: Limit) -> Option<&'a Allowed> { + allowed + .iter() + .find(|entry| entry.krate == krate && entry.name == name && entry.limit == limit) +} + +/// The violation `count` makes against `limit` at `site`, where `entry` is +/// the allowlist's entry for the type, if it has one. +fn judge_one( + site: &Site, + entry: Option<&Allowed>, + limit: Limit, + count: usize, + found: &str, +) -> Option { + let Site { path, line } = site; + match entry { + None if count > limit.bound() => Some(format!( + "{path}:{line}: {}: required at most {} {}, or an allowlist entry with a reason, \ + found {found}", + limit.rule(), + limit.bound(), + limit.noun() + )), + Some(entry) if count > entry.recorded => Some(format!( + "{path}:{line}: allowlisted type past its recorded count: required at most {} {}, \ + the count its entry records, found {found}", + entry.recorded, + limit.noun() + )), + _ => None, + } +} + +/// The entries in `allowed` with a blank reason, and the stale ones: a type +/// the scan never counted, or one back within its limit. +fn entry_violations(allowed: &[Allowed], counts: &Counts) -> Vec { + const ALLOWLIST: &str = "crates/build-xtask/src/type_size.rs"; + let mut violations = Vec::new(); + for entry in allowed { + let Allowed { + krate, + name, + limit, + reason, + .. + } = *entry; + if reason.trim().is_empty() { + violations.push(format!( + "{ALLOWLIST}: allowlist entry without a reason: required one sentence saying what \ + `{name}` in `{krate}` is and why its size holds, found a blank reason" + )); + } + let required = format!( + "`{name}` in `{krate}` over {} {}", + limit.bound(), + limit.noun() + ); + match counts.get(&(krate.to_owned(), name.to_owned(), limit)) { + None => violations.push(format!( + "{ALLOWLIST}: stale allowlist entry: required {required}, found no such type" + )), + Some(count) if *count <= limit.bound() => violations.push(format!( + "{ALLOWLIST}: stale allowlist entry: required {required}, found {count}" + )), + Some(_) => {} + } + } + violations +} + +/// Whether `path`, relative to a crate's `src/` with `/` separators, is a +/// test file: one under a `tests/` directory, or one whose name marks it as +/// tests (`tests.rs`, `foo-tests.rs`, `foo-tests-bar.rs`, `tests-foo.rs`, +/// or `foo_tests.rs`). +fn is_test_file(path: &str) -> bool { + let mut parts: Vec<&str> = path.split('/').collect(); + let stem = parts + .pop() + .map_or("", |file| file.strip_suffix(".rs").unwrap_or(file)); + parts.contains(&"tests") + || stem == "tests" + || stem.ends_with("-tests") + || stem.contains("-tests-") + || stem.starts_with("tests-") + || stem.ends_with("_tests") +} + +/// Whether `attrs` hold a `cfg` that names `test` outside a `not`, as +/// `cfg(test)` and `cfg(any(test, feature = "..."))` do. +fn gated_on_test(attrs: &[Attribute]) -> bool { + attrs.iter().any(|attr| { + attr.path().is_ident("cfg") + && matches!(&attr.meta, Meta::List(list) if names_test(list.tokens.clone())) + }) +} + +/// Whether `tokens` name `test` at any depth outside a `not(...)`. +fn names_test(tokens: TokenStream) -> bool { + let mut tokens = tokens.into_iter(); + while let Some(token) = tokens.next() { + match token { + TokenTree::Ident(ident) if ident == "not" => { + tokens.next(); + } + TokenTree::Ident(ident) if ident == "test" => return true, + TokenTree::Group(group) if names_test(group.stream()) => return true, + _ => {} + } + } + false +} + +/// The last path segment of an `impl`'s self type, through any grouping, +/// or `None` for a self type that is not a plain path. +fn self_type_name(ty: &Type) -> Option { + match ty { + Type::Path(path) if path.qself.is_none() => path + .path + .segments + .last() + .map(|segment| segment.ident.to_string()), + Type::Group(group) => self_type_name(&group.elem), + Type::Paren(paren) => self_type_name(&paren.elem), + _ => None, + } +} + +#[cfg(test)] +mod tests { + use std::fmt::Write as _; + use std::path::Path; + + use super::*; + use crate::product::test_support::workspace_root; + + /// Writes a crate at `crates/fixture/` holding each `(path, text)` + /// under its `src/`. + fn write_fixture(root: &Path, files: &[(&str, &str)]) { + crate::product::test_support::write_crate(root, "fixture", "fixture", ""); + let src = root.join("crates").join("fixture").join("src"); + for (path, text) in files { + let file = src.join(path); + let dir = file.parent().expect("a fixture file sits in a directory"); + fs::create_dir_all(dir).expect("the fixture directory creates"); + fs::write(&file, text).expect("the fixture file writes"); + } + } + + /// An inherent `impl` of `name` holding `count` methods named after `tag`. + fn methods(name: &str, tag: &str, count: usize) -> String { + let mut text = format!("impl {name} {{\n"); + for index in 0..count { + writeln!(text, " fn {tag}_{index}(&self) {{}}").expect("a String takes every write"); + } + text + "}\n" + } + + /// A struct `name` with `count` named fields. + fn fields(name: &str, count: usize) -> String { + let mut text = format!("struct {name} {{\n"); + for index in 0..count { + writeln!(text, " f{index}: u8,").expect("a String takes every write"); + } + text + "}\n" + } + + /// An entry lifting `limit` for `name` in the fixture crate. + fn fixture_entry( + name: &'static str, + limit: Limit, + recorded: usize, + reason: &'static str, + ) -> Allowed { + Allowed { + krate: "crates/fixture", + name, + limit, + recorded, + reason, + } + } + + #[test] + fn the_workspace_keeps_every_type_within_its_size_limits() { + let violations = type_size_violations(&workspace_root()); + assert!( + violations.is_empty(), + "type-size violations:\n{}", + violations.join("\n") + ); + } + + #[test] + fn methods_are_summed_across_every_inherent_impl_block_in_the_crate() { + let root = tempfile::TempDir::new().expect("tempdir"); + let lib = format!("mod more;\npub struct Wide;\n{}", methods("Wide", "a", 13)); + let more = format!("use super::Wide;\n{}", methods("Wide", "b", 13)); + write_fixture(root.path(), &[("lib.rs", &lib), ("more.rs", &more)]); + let violations = violations_against(root.path(), &[]); + assert_eq!(violations.len(), 1, "{violations:?}"); + assert!( + violations[0].starts_with( + "crates/fixture/src/lib.rs:2: type over the inherent method limit: required at \ + most 25 inherent methods" + ) && violations[0].ends_with("found `Wide` with 26 (src/lib.rs 13, src/more.rs 13)"), + "the type is named at its definition with each file's share: {violations:?}" + ); + } + + #[test] + fn trait_impl_methods_are_not_counted() { + let root = tempfile::TempDir::new().expect("tempdir"); + let lib = format!( + "struct Wide;\n{}impl Clone for Wide {{\n fn clone(&self) -> Self {{\n \ + Wide\n }}\n}}\n", + methods("Wide", "a", 25) + ); + write_fixture(root.path(), &[("lib.rs", &lib)]); + let violations = violations_against(root.path(), &[]); + assert!(violations.is_empty(), "{violations:?}"); + } + + #[test] + fn a_struct_over_the_field_limit_is_reported_at_its_definition() { + let root = tempfile::TempDir::new().expect("tempdir"); + let lib = format!( + "//! Fixture.\n{}{}", + fields("Narrow", 12), + fields("Broad", 13) + ); + write_fixture(root.path(), &[("lib.rs", &lib)]); + let violations = violations_against(root.path(), &[]); + assert_eq!(violations.len(), 1, "{violations:?}"); + assert!( + violations[0].starts_with( + "crates/fixture/src/lib.rs:16: struct over the named field limit: required at \ + most 12 named fields" + ) && violations[0].ends_with("found `Broad` with 13"), + "{violations:?}" + ); + } + + #[test] + fn an_allowlisted_type_passes_at_its_recorded_count_and_fails_one_past_it() { + let root = tempfile::TempDir::new().expect("tempdir"); + let allowed = [fixture_entry("Wide", Limit::Methods, 27, "a fixture type")]; + let at = format!("struct Wide;\n{}", methods("Wide", "a", 27)); + write_fixture(root.path(), &[("lib.rs", &at)]); + let violations = violations_against(root.path(), &allowed); + assert!(violations.is_empty(), "{violations:?}"); + + let past = format!("struct Wide;\n{}", methods("Wide", "a", 28)); + write_fixture(root.path(), &[("lib.rs", &past)]); + let violations = violations_against(root.path(), &allowed); + assert_eq!(violations.len(), 1, "{violations:?}"); + assert!( + violations[0].contains("allowlisted type past its recorded count: required at most 27") + && violations[0].ends_with("found `Wide` with 28 (src/lib.rs 28)"), + "{violations:?}" + ); + } + + #[test] + fn stale_entries_and_a_blank_reason_are_reported() { + let root = tempfile::TempDir::new().expect("tempdir"); + let allowed = [ + fixture_entry("Gone", Limit::Methods, 30, "a type that no longer exists"), + fixture_entry("Small", Limit::Fields, 20, "a struct back within the limit"), + fixture_entry("Wide", Limit::Methods, 26, " "), + ]; + let lib = format!( + "{}struct Wide;\n{}", + fields("Small", 12), + methods("Wide", "a", 26) + ); + write_fixture(root.path(), &[("lib.rs", &lib)]); + let violations = violations_against(root.path(), &allowed); + assert_eq!(violations.len(), 3, "{violations:?}"); + assert!( + violations[0].contains("stale allowlist entry: required `Gone` in `crates/fixture`") + && violations[0].ends_with("found no such type"), + "a type the scan never counted is stale: {violations:?}" + ); + assert!( + violations[1].contains("stale allowlist entry: required `Small` in `crates/fixture`") + && violations[1].ends_with("found 12"), + "a type back within its limit is stale: {violations:?}" + ); + assert!( + violations[2].contains("allowlist entry without a reason") + && violations[2].contains("`Wide`"), + "an entry needs a reason: {violations:?}" + ); + } + + #[test] + fn test_files_and_items_gated_on_test_are_not_counted() { + let root = tempfile::TempDir::new().expect("tempdir"); + let wide = methods("Wide", "a", 30); + let lib = format!( + "pub struct Wide;\n#[cfg(test)]\nmod tests {{\n{wide}}}\n\ + #[cfg(any(test, feature = \"fixture\"))]\n{wide}" + ); + let gated = format!("#![cfg(test)]\n{wide}"); + write_fixture( + root.path(), + &[ + ("lib.rs", &lib), + ("support.rs", &gated), + ("tests.rs", &wide), + ("wide-tests.rs", &wide), + ("wide-tests-more.rs", &wide), + ("tests-wide.rs", &wide), + ("wide_tests.rs", &wide), + ("inner/tests/deep.rs", &wide), + ], + ); + let violations = violations_against(root.path(), &[]); + assert!(violations.is_empty(), "{violations:?}"); + + let ungated = format!("pub struct Wide;\n#[cfg(not(test))]\n{wide}"); + write_fixture(root.path(), &[("lib.rs", &ungated)]); + let violations = violations_against(root.path(), &[]); + assert_eq!( + violations.len(), + 1, + "a `not(test)` item is counted: {violations:?}" + ); + } + + #[test] + fn methods_in_inline_modules_count_and_test_gated_methods_do_not() { + let root = tempfile::TempDir::new().expect("tempdir"); + let lib = format!( + "pub struct Wide;\nmod inner {{\n{}}}\nimpl Wide {{\n #[cfg(test)]\n fn \ + helper(&self) {{}}\n}}\n", + methods("Wide", "a", 26) + ); + write_fixture(root.path(), &[("lib.rs", &lib)]); + let violations = violations_against(root.path(), &[]); + assert_eq!(violations.len(), 1, "{violations:?}"); + assert!( + violations[0].ends_with("found `Wide` with 26 (src/lib.rs 26)"), + "{violations:?}" + ); + } + + #[test] + fn a_crate_whose_manifest_does_not_parse_is_still_scanned() { + let root = tempfile::TempDir::new().expect("tempdir"); + let lib = format!("struct Wide;\n{}", methods("Wide", "a", 26)); + write_fixture(root.path(), &[("lib.rs", &lib)]); + fs::write( + root.path() + .join("crates") + .join("fixture") + .join("Cargo.toml"), + "not [valid toml", + ) + .expect("the manifest rewrites"); + let violations = violations_against(root.path(), &[]); + assert_eq!(violations.len(), 1, "{violations:?}"); + assert!( + violations[0].contains("type over the inherent method limit"), + "a crate whose manifest was never read cannot be shown within the limits: \ + {violations:?}" + ); + } + + #[test] + fn a_source_file_that_does_not_parse_is_reported() { + let root = tempfile::TempDir::new().expect("tempdir"); + write_fixture(root.path(), &[("lib.rs", "fn broken( {\n")]); + let violations = violations_against(root.path(), &[]); + assert_eq!(violations.len(), 1, "{violations:?}"); + assert!( + violations[0].starts_with("crates/fixture/src/lib.rs:") + && violations[0].contains("unparseable source"), + "a file that was never read cannot be shown within the limits: {violations:?}" + ); + } + + #[test] + fn a_type_size_scan_that_reads_no_source_fails() { + let root = tempfile::TempDir::new().expect("tempdir"); + fs::create_dir_all(root.path().join("crates")).expect("the crates directory creates"); + let violations = violations_against(root.path(), &[]); + assert_eq!(violations.len(), 1, "{violations:?}"); + assert!( + violations[0].contains("read nothing"), + "a check that read no source cannot show it within the limits: {violations:?}" + ); + } +} From e228e26961a43489c05bcd539d5a547f75cec3e0 Mon Sep 17 00:00:00 2001 From: Vinnie Falco Date: Fri, 6 Nov 2026 03:26:15 -0800 Subject: [PATCH 6/6] Add size limit and test placement rules to `AGENTS.md` The agent rules now limit the size of functions and types, not files: callers depend on what a type does, not on its file. A new Structural Rules bullet gives the limits and allows a file split only for a second unrelated concept. New unit tests go in a `#[cfg(test)] mod tests` block in the file under test, and the Verification bullet for `cargo test -p build-xtask` lists the type-size limits. - A new entry in the allowlist in `crates/build-xtask/src/type_size.rs` is a decision made in advance with a reason, never a way to make a change pass. The fix for an oversized type is a split by responsibility. - Existing unit-test files (`foo-tests.rs`, `foo/tests.rs`) stay as they are, so the rule change moves no file. - The commit changes prose only. The new text says every `clippy.toml` sets the function limits and `cargo test -p build-xtask` enforces the type-size limits, but this commit changes neither. --- AGENTS.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 0a0eb83fe..58b960a40 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -60,7 +60,7 @@ Seven terms have exactly one meaning each, everywhere in this repository: code c - Docs: `cargo doc --workspace --no-deps --all-features --exclude workshop --exclude workshop-server --exclude workshop-server-api` with `RUSTDOCFLAGS="-D warnings"`. Rustdoc lints are not covered by clippy; never skip the docs gate. - Facade docs: `RUSTDOCFLAGS="-D warnings" cargo doc -p runvil --no-deps`, without `--all-features`, so the facade's docs build with default features. - Facade surface: `cargo + xtask api --check`, where the pinned nightly is the one named in `crates/build-xtask/src/api/toolchain.rs`; on any other toolchain it fails at once, naming the nightly it needs. It checks that every path a surface item's signature, fields, bounds, impls, or doc links name is a facade re-export (or std, core, alloc, or an allowlisted crate), that no surface doc text names an internal crate, and that the surface listing matches the committed `crates/runvil/public-api.txt`. Run it with `CARGO_BUILD_WARNINGS` unset (in PowerShell, `Remove-Item Env:CARGO_BUILD_WARNINGS` after the clippy gate set it), because the nightly cargo turns its own warnings into errors while that variable is set. -- Boundary and structural checks: `cargo test -p build-xtask`. It enforces the product and container boundaries, the Workshop tier graph, the `## Invariants` marker, and lint inheritance. +- Boundary and structural checks: `cargo test -p build-xtask`. It enforces the product and container boundaries, the Workshop tier graph, the `## Invariants` marker, lint inheritance, and the type-size limits. - Wording and rulebook scan: `node --test crates/workshop/ui/test/docs-claims.mjs`. It enforces the Engine wording rule, the rulebook Definitions, and the Pack lifecycle wording (installed, snapshotted, or declared). It needs no `npm ci` and runs from any directory, because the file resolves the repository root from its own path. ## Structural Rules @@ -70,7 +70,8 @@ Seven terms have exactly one meaning each, everywhere in this repository: code c - No Engine test sleeps, reads the wall clock, waits with a timeout, or draws entropy from the OS. - Pseudo-random test input comes only from a fixed seed that the failure prints. - Temporary directory names come from the test's name, the process id, and a counter. -- A module with more than one file keeps its files in a directory named after it, in the standard module layout (`foo.rs` beside `foo/bar.rs`). A directory may hold any number of files, one included, and a module's unit tests are one of them (`foo/tests.rs`). Never move files between a flat sibling form and a directory because of how many files a group holds, and never change a group's layout only to bring it in line with this rule. Existing `foo-bar.rs` siblings wired with `#[path = "foo-bar.rs"] mod bar;` stay as they are, and new code adds no path attribute for layout. Top-level `tests/` and `benches/` trees follow Cargo target conventions. +- A module with more than one file keeps its files in a directory named after it, in the standard module layout (`foo.rs` beside `foo/bar.rs`). A directory may hold any number of files, one included. Never move files between a flat sibling form and a directory because of how many files a group holds, and never change a group's layout only to bring it in line with this rule. Existing `foo-bar.rs` siblings wired with `#[path = "foo-bar.rs"] mod bar;` and existing unit-test files (`foo-tests.rs`, `foo/tests.rs`) stay as they are; new code adds no path attribute for layout, and new unit tests go in a `#[cfg(test)] mod tests` block in the file under test. Top-level `tests/` and `benches/` trees follow Cargo target conventions. +- A file has no length limit. Never split a file, or move code out of one, because of its length; split a file only when it holds a second unrelated concept. Size limits sit on functions and types instead: clippy denies a function over 100 lines or 7 arguments (set in every `clippy.toml`), and `cargo test -p build-xtask` fails a type with more than 25 inherent methods across its `impl` blocks or a struct with more than 12 named fields. Split an oversized type by responsibility; a new entry in the allowlist in `crates/build-xtask/src/type_size.rs` is a plan decision with a reason, never a way to make a change pass. ## SPA and CSS Rules