refactor(executor/core): dyn-safe RuntimeOps + de-erased builder & registrar lifetimes (034 Phase 2, #130)

CHANGELOG.md

@@ -31,6 +31,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Added
 
+- **Design 034 Phase 2 — dyn-safe `RuntimeOps` capability trait (Issue #130, [review doc](docs/design/034-technical-debt-review.md)).** New object-safe trait in `aimdb-executor` (`name` / `now_nanos` / `unix_time` / boxed `sleep` / `log(LogLevel, …)`) so a runtime adapter can travel as `Arc<dyn RuntimeOps>` instead of a generic parameter — the groundwork for removing `R` from the record object graph (#131). Implemented by `TokioAdapter`, `EmbassyAdapter`, and `WasmAdapter`, each covered by a shared behavioral contract test. `BoxFuture`'s canonical definition moves to `aimdb-executor` (re-exported unchanged from `aimdb-core`). ([aimdb-executor](aimdb-executor/CHANGELOG.md), [aimdb-tokio-adapter](aimdb-tokio-adapter/CHANGELOG.md), [aimdb-embassy-adapter](aimdb-embassy-adapter/CHANGELOG.md), [aimdb-wasm-adapter](aimdb-wasm-adapter/CHANGELOG.md))
+
 - **M17 — centralized Embassy connector spine: one audited home for the single-core `unsafe` ([Design 033](docs/design/033-M17-unify-connectors-drop-send.md)).** New `aimdb-embassy-adapter::connectors` module (features `connectors` / `connector-io`) collects the force-`Send` plumbing every Embassy connector used to hand-roll: session transports get `EmbassySessionClient`/`EmbassySessionServer`, `OneShotDialer`/`OneShotListener`/`OneShotCell`, and the framed `EmbassyConnection` + `Framer`; data-plane transports get the `EmbassySink`/`EmbassySource` bridges (over `EmbassySinkRaw`/`EmbassySourceRaw`) that ride core's existing `pump_sink`/`pump_source`, plus `into_box_future` for protocol tasks. The serial Embassy half is now thin sugar (just a COBS `Framer`) with **zero `unsafe`** (down from a 407-line hand-roll with 7 `unsafe impl`s); the MQTT and KNX Embassy halves dropped their hand-rolled publisher/router loops and `SendFutureWrapper` use to ride core's pumps (KNX inbound telegrams now flow through `pump_source`). All connector-crate `unsafe`/`SendFutureWrapper` is gone — confined to the adapter. The std/Tokio side, `aimdb-client`, the WebSocket server, examples, and tests are unchanged. (Chosen over Design 033's original "drop `Send` from the contract", which would have pushed `!Send` onto the std side; see the doc's Implementation Decision.) ([aimdb-embassy-adapter](aimdb-embassy-adapter/CHANGELOG.md), [aimdb-serial-connector](aimdb-serial-connector/CHANGELOG.md), [aimdb-mqtt-connector](aimdb-mqtt-connector/CHANGELOG.md), [aimdb-knx-connector](aimdb-knx-connector/CHANGELOG.md))
 
 - **Remote access via connectors — Phases 0–6: converge four hand-rolled networking stacks onto two shared engines (Issue #39, [design doc](docs/design/remote-access-via-connectors.md)).** AimX remote access (and any future transport) now rides the connector layer instead of a bespoke I/O abstraction. New, runtime-neutral `aimdb-core::session` module (feature `connector-session`, `no_std + alloc`): the three-layer substrate (`Connection`/`Listener`/`Dialer` + `EnvelopeCodec` + `Dispatch`/`Session`), the reactive **server** engine (`serve`/`run_session`) and proactive **client** engine (`run_client`/`pump_client`), the `pump_sink`/`pump_source` data-plane toolkit, and the transport-agnostic `SessionClientConnector`/`SessionServerConnector`. The AimX-v2 NDJSON protocol (`session::aimx`: `AimxCodec` + `AimxDispatch`) and the WebSocket connector are ports onto this substrate, so the AimX server/client and WS server/client stacks collapse onto the two engines. New **`aimdb-uds-connector`** crate carries the UDS transport (`UdsClient`/`UdsServer`). ([aimdb-core](aimdb-core/CHANGELOG.md), [aimdb-uds-connector](aimdb-uds-connector/CHANGELOG.md), [aimdb-websocket-connector](aimdb-websocket-connector/CHANGELOG.md), [aimdb-client](aimdb-client/CHANGELOG.md), [aimdb-mqtt-connector](aimdb-mqtt-connector/CHANGELOG.md), [aimdb-knx-connector](aimdb-knx-connector/CHANGELOG.md), [aimdb-embassy-adapter](aimdb-embassy-adapter/CHANGELOG.md))
@@ -42,6 +44,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed (breaking)
 
+- **Design 034 Phase 2 — registrar lifetime fix + de-erased builder internals (Issue #130, [review doc](docs/design/034-technical-debt-review.md)).** `RecordRegistrar`'s fluent methods now take fresh borrows (`&mut self -> &mut Self`) instead of borrowing the registrar for its entire lifetime — a `configure` closure can finally use separate statements (`reg.source_raw(…); reg.tap_raw(…);`) and reuse the registrar after a chain. `configure`'s closure bound drops its HRTB; `OutboundConnectorBuilder`/`InboundConnectorBuilder` gain a second lifetime parameter (`<'r, 'a, T, R>`); `RecordT::register` and the adapter/persistence extension traits follow. Internally, `AimDbBuilder` stores its spawn/start functions typed (`SpawnFnType<R>`/`StartFnType<R>`) instead of `Box<dyn Any>` — the panicking downcasts in `build()` are gone, and `AimDb<R>`'s struct-level bound moved to its impls. Closure-based user code compiles unchanged. ([aimdb-core](aimdb-core/CHANGELOG.md), [aimdb-embassy-adapter](aimdb-embassy-adapter/CHANGELOG.md), [aimdb-persistence](aimdb-persistence/CHANGELOG.md))
+
 - **Design 034 Phase 1 — mechanical debt cleanup (Issues #129, #132, [review doc](docs/design/034-technical-debt-review.md)).** `DbError` is unified on `alloc::string::String`: every variant has one shape on all targets, `thiserror` derives `Display`/`Error` unconditionally (now a mandatory no_std dependency of `aimdb-core`), and no_std builds produce the same error messages as std builds instead of `Error 0xNNNN` codes — on no_std the `_field: ()` placeholders become the real `String` fields. The dead `Database<A>` wrapper, the `TokioDatabase`/`EmbassyDatabase` aliases, and the deprecated `RecordRegistrar::link()` are removed. `ConsumerTrait::subscribe_any` is infallible (returns `Box<dyn AnyReader>`), `OutboundRoute` is a struct with named fields, and `aimdb-core`'s `std` feature no longer pulls a `tokio` dependency (tests cover it via dev-deps). Internally: all dual std/alloc import pairs and the duplicated `RuntimeContext` impl blocks are collapsed, and crate-private `log_*!` macros replace the 62 per-call-site `#[cfg(feature = "tracing")]` gates. The `aimdb-wasm-adapter` ring-lag error now carries a `buffer_name` like the other adapters. ([aimdb-core](aimdb-core/CHANGELOG.md), [aimdb-tokio-adapter](aimdb-tokio-adapter/CHANGELOG.md), [aimdb-embassy-adapter](aimdb-embassy-adapter/CHANGELOG.md), [aimdb-mqtt-connector](aimdb-mqtt-connector/CHANGELOG.md), [aimdb-knx-connector](aimdb-knx-connector/CHANGELOG.md))
 
 - **CLI/MCP endpoint surface reworked to `--connect <url>` (Issue #123).** `aimdb`'s per-command `--socket <path>` is replaced by a global `--connect <endpoint>` (`AIMDB_CONNECT` env; bare paths still work). `aimdb-mcp` renames the tools' `socket_path` param to `endpoint`, the startup `--socket` to `--connect`, and `AIMDB_SOCKET` to `AIMDB_CONNECT`; the pool is keyed by endpoint URL. `aimdb-client`'s `AimxConnection::connect` takes a `&str` endpoint (was a path) and `ClientError::ConnectionFailed.socket` is renamed `endpoint`. Serial endpoints (`serial://…`) require building the CLI/MCP with `--features transport-serial`. ([aimdb-client](aimdb-client/CHANGELOG.md), [tools/aimdb-cli](tools/aimdb-cli/CHANGELOG.md), [tools/aimdb-mcp](tools/aimdb-mcp/CHANGELOG.md))

aimdb-core/CHANGELOG.md

@@ -25,6 +25,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Internal refactors
 
+- **Phase 2 — builder internals de-erased (Issue #130, [design doc §3.2](../docs/design/034-technical-debt-review.md)).** No behavior change: `AimDbBuilder` stores its per-record future collectors and `on_start` tasks as their typed forms (`Vec<(StringKey, SpawnFnType<R>)>` / `Vec<StartFnType<R>>`) instead of `Box<dyn Any + Send>` — the builder is already generic over `R`, so the erasure bought nothing. The panicking downcasts in `build()` (`"spawn function type mismatch"`, `"on_start fn type mismatch"`) are deleted. To keep the field types well-formed on the `NoRuntime` typestate, `AimDb<R>`'s struct-level `R: RuntimeAdapter + 'static` bound moved to its impl blocks (strictly more permissive). `BoxFuture` is now a re-export of the canonical alias in `aimdb-executor` (same type).
+
 - **Phase 1 mechanical cleanup (Issue #132, [design doc](../docs/design/034-technical-debt-review.md)).** No behavior change:
   - All dual `#[cfg(feature = "std")] use std::… / #[cfg(not(…))] use alloc::…` import pairs replaced by single unconditional `use alloc::…` imports; redundant per-module `extern crate alloc;` declarations dropped (the crate root has one). The duplicated std/no_std `RuntimeContext` impl blocks in `context.rs` (character-identical except for the `Arc` path) are merged into one.
   - New crate-private `log_debug!`/`log_info!`/`log_warn!`/`log_error!` macros (`src/log.rs`) forward to `tracing` when the feature is on and expand to an argument-borrowing no-op otherwise — deleting all 62 per-call-site `#[cfg(feature = "tracing")]` gates. `defmt` is deliberately not folded in (most sites use `{:?}` with non-`defmt::Format` types); router.rs keeps its paired explicit defmt gates.
@@ -41,6 +43,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed (breaking)
 
+- **Phase 2 — `RecordRegistrar` fluent methods take fresh borrows (Issue #130, [design doc §3.5](../docs/design/034-technical-debt-review.md)).** The methods previously took `&'a mut self` and returned `&'a mut Self` with `'a` = the struct's own lifetime parameter, so the first call borrowed the registrar for its entire remaining lifetime and only one unbroken chain per `configure` closure was possible. Now:
+  - All fluent methods are `&mut self -> &mut Self`; separate statements (`reg.source_raw(…); reg.tap_raw(…);`) and registrar reuse after a chain work.
+  - `configure`'s closure bound is `FnOnce(&mut RecordRegistrar<'_, T, R>)` (HRTB dropped) — existing closures compile unchanged.
+  - `OutboundConnectorBuilder` / `InboundConnectorBuilder` are now `<'r, 'a, T, R>` (`'r` borrows the registrar, `'a` is the registrar's record borrow) instead of the double-`'a`; `finish()` returns `&'r mut RecordRegistrar<'a, T, R>`, so `.finish().link_from(…)` chains keep working.
+  - `RecordT::register` is `fn register(reg: &mut RecordRegistrar<'_, Self, R>, cfg: &Self::Config)` (was `fn register<'a>(reg: &'a mut RecordRegistrar<'a, …>)`); the `impl_record_registrar_ext!` macro and the adapter/persistence extension traits follow the same shape. Code that *names* the old signatures (custom `RecordT` impls, custom extension traits) must update; closure-based configuration is source-compatible.
+
 - **`DbError` unified on `alloc::string::String` — one shape per variant on every target (Issue #129, [design doc §3.1.1](../docs/design/034-technical-debt-review.md)).** The std/no_std dual-field design (`key: String` under `std`, `_key: ()` otherwise) is gone: every context field is a `String` unconditionally (the crate already requires `alloc` everywhere), and `Display`/`Error` derive from `thiserror` on every target (thiserror 2.x is no_std-capable, now a mandatory `default-features = false` dependency; the `remote-access` feature no longer lists it). Consequences:
   - **no_std only:** field renames (`_endpoint`/`_reason`/`_buffer_name`/`_key`/… `: ()` → the real `String` fields); `Display` now prints the same full messages as std builds instead of the `Error 0xNNNN` numeric-code table (which is deleted); `DbError` now implements `core::error::Error`. `error_code()` / `error_category()` are unchanged.
   - **std:** no change to variant shapes or messages.

aimdb-core/src/builder.rs

@@ -5,7 +5,6 @@
 
 use core::any::TypeId;
 use core::fmt::Debug;
-use core::marker::PhantomData;
 
 use alloc::{
     boxed::Box,
@@ -19,17 +18,13 @@ use crate::extensions::Extensions;
 use crate::graph::DependencyGraph;
 
 /// Shorthand for a heap-pinned, `Send`, `'static` future — the unit of work
-/// the `AimDbRunner` drives.
-pub type BoxFuture = core::pin::Pin<Box<dyn core::future::Future<Output = ()> + Send + 'static>>;
+/// the `AimDbRunner` drives. Canonical definition lives in `aimdb-executor`.
+pub type BoxFuture = aimdb_executor::BoxFuture;
 
-/// Type-erased on_start function stored in `AimDbBuilder::start_fns`.
-///
-/// Defined once here so `on_start()` (which stores) and `build()` (which
-/// downcasts) share the *exact same* type and a silent type mismatch cannot
-/// cause a runtime panic. Single alias regardless of `std`/`no_std`.
+/// `on_start` task stored in `AimDbBuilder::start_fns`, invoked at `build()`.
 type StartFnType<R> = Box<dyn FnOnce(Arc<R>) -> BoxFuture + Send>;
 
-/// Type-erased per-record future collector stored in `AimDbBuilder::spawn_fns`.
+/// Per-record future collector stored in `AimDbBuilder::spawn_fns`.
 ///
 /// At `build()` time each is invoked in topological order; the returned
 /// `Vec<BoxFuture>` is appended to the runner's accumulator.
@@ -294,20 +289,18 @@ pub struct AimDbBuilder<R = NoRuntime> {
     /// Connector builders that will be invoked during build()
     connector_builders: Vec<Box<dyn crate::connector::ConnectorBuilder<R>>>,
 
-    /// Spawn functions with their keys
-    spawn_fns: Vec<(StringKey, Box<dyn core::any::Any + Send>)>,
+    /// Per-record future collectors with their keys. Always empty on the
+    /// `NoRuntime` typestate — `configure()` only exists once `R` is fixed.
+    spawn_fns: Vec<(StringKey, SpawnFnType<R>)>,
 
     /// Startup tasks registered via on_start() — spawned after build() completes.
-    /// Stored type-erased (`Box<Box<dyn FnOnce(Arc<R>) -> BoxFuture<…>>>`) to allow
-    /// the field to exist on the unparameterised NoRuntime builder too.
-    start_fns: Vec<Box<dyn core::any::Any + Send>>,
+    /// Always empty on the `NoRuntime` typestate — `on_start()` only exists
+    /// once `R` is fixed.
+    start_fns: Vec<StartFnType<R>>,
 
     /// Generic extension storage for external crates (e.g., persistence, metrics).
     /// Moved into AimDbInner during build() so it can be read on the live AimDb handle.
     extensions: Extensions,
-
-    /// PhantomData to track the runtime type parameter
-    _phantom: PhantomData<R>,
 }
 
 impl AimDbBuilder<NoRuntime> {
@@ -322,7 +315,6 @@ impl AimDbBuilder<NoRuntime> {
             spawn_fns: Vec::new(),
             start_fns: Vec::new(),
             extensions: Extensions::new(),
-            _phantom: PhantomData,
         }
     }
 
@@ -332,15 +324,14 @@ impl AimDbBuilder<NoRuntime> {
     ///
     /// # Type Safety Note
     ///
-    /// The `connector_builders` field is intentionally reset to `Vec::new()` during this
-    /// transition because connectors are parameterized by the runtime type:
-    ///
-    /// - Before: `Vec<Box<dyn ConnectorBuilder<NoRuntime>>>`
-    /// - After: `Vec<Box<dyn ConnectorBuilder<R>>>`
-    ///
-    /// These types are incompatible and cannot be transferred. However, this is not a bug
-    /// because `.with_connector()` is only available AFTER calling `.runtime()` (it's defined
-    /// in the `impl<R> where R: RuntimeAdapter` block, not in `impl AimDbBuilder<NoRuntime>`).
+    /// The `connector_builders`, `spawn_fns` and `start_fns` fields are
+    /// intentionally reset to `Vec::new()` during this transition: all three
+    /// are parameterized by the runtime type (`ConnectorBuilder<NoRuntime>` →
+    /// `ConnectorBuilder<R>`, etc.), and the `NoRuntime` instantiations are
+    /// incompatible with — and provably empty before — the typed ones, because
+    /// `.with_connector()`, `.configure()` and `.on_start()` are only available
+    /// AFTER calling `.runtime()` (they're defined in the
+    /// `impl<R> where R: RuntimeAdapter` block, not in `impl AimDbBuilder<NoRuntime>`).
     ///
     /// This means the type system **enforces** the correct call order:
     /// ```rust,ignore
@@ -360,9 +351,8 @@ impl AimDbBuilder<NoRuntime> {
             runtime: Some(rt),
             connector_builders: Vec::new(),
             spawn_fns: Vec::new(),
-            start_fns: self.start_fns,
+            start_fns: Vec::new(),
             extensions: self.extensions,
-            _phantom: PhantomData,
         }
     }
 }
@@ -407,11 +397,8 @@ where
         F: FnOnce(Arc<R>) -> Fut + Send + 'static,
         Fut: core::future::Future<Output = ()> + Send + 'static,
     {
-        // Type-erase so the field can be shared with the `NoRuntime` builder struct.
-        // Uses the module-level `StartFnType<R>` alias — must stay in sync with
-        // the downcast in `build()`.
-        let boxed: StartFnType<R> = Box::new(move |runtime| Box::pin(f(runtime)));
-        self.start_fns.push(Box::new(boxed));
+        self.start_fns
+            .push(Box::new(move |runtime| Box::pin(f(runtime))));
         self
     }
 
@@ -494,7 +481,7 @@ where
     pub fn configure<T>(
         &mut self,
         key: impl RecordKey,
-        f: impl for<'a> FnOnce(&'a mut RecordRegistrar<'a, T, R>),
+        f: impl FnOnce(&mut RecordRegistrar<'_, T, R>),
     ) -> &mut Self
     where
         T: Send + Sync + 'static + Debug + Clone,
@@ -566,8 +553,7 @@ where
                     )
                 });
 
-            // Store the spawn function (type-erased in Box<dyn Any>)
-            self.spawn_fns.push((spawn_key, Box::new(spawn_fn)));
+            self.spawn_fns.push((spawn_key, spawn_fn));
         }
 
         self
@@ -776,7 +762,7 @@ where
         log_info!("Collecting futures for {} records", self.spawn_fns.len());
 
         // Build a lookup map from spawn_fns for topological ordering
-        let mut spawn_fn_map: HashMap<StringKey, Box<dyn core::any::Any + Send>> =
+        let mut spawn_fn_map: HashMap<StringKey, SpawnFnType<R>> =
             self.spawn_fns.into_iter().collect();
 
         // Execute collectors in topological order — transforms collect after their inputs.
@@ -787,15 +773,11 @@ where
                 continue;
             };
 
-            let Some(spawn_fn_any) = spawn_fn_map.remove(&key) else {
+            let Some(spawn_fn) = spawn_fn_map.remove(&key) else {
                 continue;
             };
 
-            let spawn_fn = spawn_fn_any
-                .downcast::<SpawnFnType<R>>()
-                .expect("spawn function type mismatch");
-
-            futures_acc.extend((*spawn_fn)(&runtime, &db, id)?);
+            futures_acc.extend(spawn_fn(&runtime, &db, id)?);
         }
 
         log_info!("Record future collection complete");
@@ -827,13 +809,8 @@ where
         if !self.start_fns.is_empty() {
             log_debug!("Collecting {} on_start future(s)", self.start_fns.len());
 
-            for (idx, start_fn_any) in self.start_fns.into_iter().enumerate() {
-                let start_fn = start_fn_any
-                    .downcast::<StartFnType<R>>()
-                    .unwrap_or_else(|_| {
-                        panic!("on_start fn[{idx}] type mismatch — this is a bug in aimdb-core")
-                    });
-                futures_acc.push((*start_fn)(runtime.clone()));
+            for start_fn in self.start_fns {
+                futures_acc.push(start_fn(runtime.clone()));
             }
         }
 
@@ -868,7 +845,10 @@ impl Default for AimDbBuilder<NoRuntime> {
 ///     .register_record::<Temperature>(&TemperatureConfig)
 ///     .build()?;
 /// ```
-pub struct AimDb<R: aimdb_executor::RuntimeAdapter + 'static> {
+// No struct-level bound: `SpawnFnType<R>` must be a well-formed type even for
+// the builder's `NoRuntime` typestate (where it is never instantiated). All
+// functionality lives on `R: RuntimeAdapter` impls.
+pub struct AimDb<R> {
     /// Internal state
     inner: Arc<AimDbInner>,
 
@@ -880,7 +860,7 @@ pub struct AimDb<R: aimdb_executor::RuntimeAdapter + 'static> {
     profiling_clock: crate::profiling::Clock,
 }
 
-impl<R: aimdb_executor::RuntimeAdapter + 'static> Clone for AimDb<R> {
+impl<R> Clone for AimDb<R> {
     fn clone(&self) -> Self {
         Self {
             inner: self.inner.clone(),