Skip to content

feat(module): support per-instance setup factories - #48

Merged
senamakel merged 1 commit into
mainfrom
restricted-setup-factory-7433
Oct 11, 2026
Merged

senamakel merged 1 commit into
mainfrom
restricted-setup-factory-7433

Conversation

@senamakel

@senamakel senamakel commented Oct 11, 2026 •

Copy link
Copy Markdown
Member

Summary

Add an optional setup_factory = factory form to configured native, static and optional-static module exports. The safe factory runs once after complete ABI argument validation and valid initial configuration parsing. The same setup callback is shared by reference throughout the instance lifecycle and reused on reinitialization; separate ABI instances receive separate captured state.

This lets a module seal an admitted runtime mode in its own captured state and refuse incompatible reinitialization before replacing the served interface. It avoids self-calls during setup, which cannot complete before module readiness, and avoids process-wide registries. The downstream use is TinyConnectors restricted concurrent reads (tinyhumansai/openhuman#7433); this PR adds the generic owning seam only.

Existing setup = function behavior, per-instance reinitialization serialization, configuration parsing, deadlines and native ABI error mapping are preserved. Factory panic is contained as TB_PANICKED. No wire protocol, ABI layout, connection/responder/stream changes or new dependencies.

Regression evidence

Two actual compiled ModuleHost tests failed against legacy setup: the factory was never created, and changing the sealed mode during reinitialization succeeded. After the seam and review corrections, fourteen focused factory/lifecycle tests pass on current-thread/in-memory fixtures:

  • Each independently admitted ABI instance creates its own factory once; reinitialization reuses it.
  • Changed admission is refused and the live service remains unchanged.
  • Concurrent setup attempts serialize captured admission.
  • Invalid initial config/host is refused before constructing factory state.
  • Factory panic produces native initialization refusal.
  • A manually copying callback Clone cannot detach the original admission (behavioral RED preserved before the fix).
  • Null/short output and zero workers are refused without factory side effects (RED observed three unwanted constructions).
  • Deinit releases captured state while retaining the ABI tombstone; concurrent reinitialize completes before disposal, and poisoned callback locks still permit teardown (RED preserved before the fix).
  • Actual compiled instances release factory captures on successful shutdown, setup error and setup panic. The setup-error regression observed zero drops before the fix and one after it. Failed initial setup clears its callback even when the host treats the instance as terminal and does not invoke ABI shutdown.

Legacy configured callback tests stay green. Optional-static exports compile in both default and all-feature configurations.

Verification

  • All-feature workspace: 508 unit/bin tests and 3 doctests passed.
  • Default workspace: 495 unit tests and 3 doctests passed.
  • Actual dynamic loader fixtures and include-ignored coverage suite passed; existing CI per-file 90% gate passed across 37 measured source files (minimum 90.0641%, module runtime 92.1418%).
  • Locked all-target/all-feature Clippy -D warnings, full build, slim no-default check, rustdoc -D warnings, fmt and diff checks passed.
  • Full locked workspace all-feature check passed on Rust 1.85.

Canonical base is merged PR46 at 8a4d8ca; this branch is independent of PR47 and touches only module export/runtime factory delegation, sibling tests and module docs. Downstream module and host consumption will wait for upstream availability. No real services are called.

Review correction runs used serial test execution. One earlier default run hit the existing 20ms Echo timeout under machine load; its unchanged focused rerun and the complete default suite passed. One Clippy process exited137 without a diagnostic; the fresh complete strict run passed. Neither unrelated test nor resource behavior was changed.

Co-authored-by: Medulla <medulla@tinyhumans.ai>
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 11, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-11T17:03:56.696376Z 30a777c PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@coderabbitai

coderabbitai Bot commented Oct 11, 2026

Copy link
Copy Markdown

Warning

Review limit reached

  • Run on-demand review

This review includes 4 billable files and costs up to $1.00.

Or wait 31 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 9334561b-0066-423d-86a2-fe8375d971c6

📥 Commits

Reviewing files that changed from the base of the PR and between 8a4d8ca and 30a777c.


📒 Files selected for processing (4)
  • crates/tinybus-module/src/lib.rs
  • crates/tinybus-module/src/setup_factory_tests.rs
  • crates/tinybus-module/src/static_link_tests.rs
  • docs/modules/module/README.md

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 30a777cc11

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread crates/tinybus-module/src/lib.rs
@senamakel
senamakel merged commit 3740ce1 into main Oct 11, 2026
8 checks passed
@tinysweeper

tinysweeper Bot commented Oct 12, 2026 •

Copy link
Copy Markdown

Tiny Sweeper review

Tiny Sweeper completed its review; deterministic results follow.

State: Ready for maintainer review
Priority: none
Reviewed head: 30a777cc11ea
Updated: 2026-10-12T01:46:18Z

Review snapshot

Change surface Files Review signal Count
Production 1 Active findings 0
Tests 2 Noted findings 0
Documentation 1 Resolved findings 0
Configuration 0 Pending checks/questions 0

Completeness: Complete
Test assessment: Test coverage is assessed from changed tests and lane evidence; execution is not claimed without trusted check data.

What changed

The diff adds a `setup_factory` alternative to the configured `setup` form: `crates/tinybus-module/src/lib.rs#macro_rules! module_export {` gains an arm rewriting `setup_factory = $factory:path, config = $config:ty` into a configured export started by `start_reconfigurable_module_with_factory`, and `module_export_static!` gains a matching arm routing to a linked factory starter. Two new `#[doc(hidden)]` public starters (`start_reconfigurable_module_with_factory`, `start_linked_reconfigurable_module_with_factory`) delegate to a shared `start_reconfigurable_module_factory_mode` that invokes the factory under `catch_unwind(AssertUnwindSafe(factory))` after configuration parses, returning `TB_PANICKED` on panic and otherwise cloning the result into the initial setup and moving it into the reinitialize closure. A `#[cfg(test)]` module is registered for a new `setup_factory_tests.rs`, the optional static-link fixture is converted from `setup` to `setup_factory` with a new `Config` type, and `docs/modules/module/README.md` documents the per-instance state behavior.

Features

  • Added — `setup_factory` form in `module_export!`: Configured exports may write `setup_factory = $factory:path, config = $config:ty` in place of `setup = $setup:path`; the rule expands to a configured export started through `start_reconfigurable_module_with_factory`, so a safe no-argument factory runs once per ABI initialization after valid configuration is parsed and its captured state is reused for that instance's reinitializations. (crates/tinybus-module/src/lib.rs#macro_rules! module_export {)
  • Added — Per-instance factory starter entry points: New `#[doc(hidden)]` `start_reconfigurable_module_with_factory` and `start_linked_reconfigurable_module_with_factory` wrap a shared `start_reconfigurable_module_factory_mode` that calls the factory under `catch_unwind` and returns `TB_PANICKED` on panic instead of unwinding into the host; the factory result is cloned for initial setup and moved into the reinitialize closure, and the legacy starters forward `|| setup`. (crates/tinybus-module/src/lib.rs#unsafe fn start_reconfigurable_module_mode<C, F, Fut>(, crates/tinybus-module/src/lib.rs#where)
  • Added — `setup_factory` support in the static and optional-static macros: `module_export_static!` accepts the factory form and starts via the linked factory starter, and the docs state the form is available in `module_export`, `module_export_static` and `module_export_optional_static`, so linking several modules into one executable supports per-instance factory state. (crates/tinybus-module/src/lib.rs#macro_rules! module_export {, docs/modules/module/README.md)
  • Modified — Optional static-link fixture converted to a factory: The existing optional-static fixture gains a `Config` type, its setup callback now takes `(Connection, Config)`, and a no-capture closure factory is supplied that must satisfy the same `Clone + Send + Sync` and future bounds as the runtime starter. (crates/tinybus-module/src/static_link_tests.rs, crates/tinybus-module/src/static_link_tests.rs#mod configured {)
  • Added — Per-instance setup state documentation: `docs/modules/module/README.md` describes the factory signature, that the factory is called once after valid initial configuration, that state is retained for initial setup and every reinitialization, that separate ABI instances get fresh state without a global registry, that the module owns its admission logic, and that factory panics return the native initialization panic code. (docs/modules/module/README.md, docs/modules/module/README.md#corresponding `NameOwnerChanged` announcement.)
  • Internal refactor — Test module wiring for setup factories: A new `#[cfg(test)]` module registered with `#[path = "setup_factory_tests.rs"]` is added to the crate's test tree alongside the existing test module registration. (crates/tinybus-module/src/lib.rs#mod tests;, crates/tinybus-module/src/setup_factory_tests.rs)

Tests

  • unit test (crate-internal, `#[tokio::test]`) — Builds two instances with opposite `restricted` values, asserts the factory counter advances by two for the two ABI initializations, then reinitializes both and asserts the instance whose mode is unchanged succeeds while the one requesting a changed mode fails, with the served `Mode` values unchanged and the factory counter still only advanced by two.: Covers per-instance factory invocation and reuse of captured state across reinitialization; no execution evidence is present in the review evidence, so no pass or failure is asserted here. (crates/tinybus-module/src/setup_factory_tests.rs)
  • unit test (crate-internal, `#[tokio::test]`) — Reinitializes an instance with a configuration that would change the captured mode and asserts the reinitialize call errors while the live service still reports the original mode.: Exercises refusal of a mode change without replacing the live served object; no execution evidence is present, so no pass or failure is asserted here. (crates/tinybus-module/src/setup_factory_tests.rs)
  • unit test (crate-internal, `#[tokio::test]`) — Calls the returned setup closure concurrently with two different configurations and asserts exactly one of the two attempts succeeds.: Exercises serialization of concurrent setup attempts through the captured admission state; no execution evidence is present, so no pass or failure is asserted here. (crates/tinybus-module/src/setup_factory_tests.rs)
  • unit test (crate-internal, `#[test]`) — Attaches a module whose factory panics and asserts the returned error matches `tinybus::Error::ModuleRefused`.: Covers the mapping of a factory panic to a native initialization refusal rather than an unwind; no execution evidence is present, so no pass or failure is asserted here. (crates/tinybus-module/src/setup_factory_tests.rs)
  • unit test (crate-internal, `#[tokio::test]`) — Attaches a module with a type-invalid configuration, asserts the attach fails, and asserts the factory invocation counter is unchanged.: Covers that invalid initial configuration does not construct factory state; no execution evidence is present, so no pass or failure is asserted here. (crates/tinybus-module/src/setup_factory_tests.rs)
  • unit test (crate-internal, `#[test]`) — Calls `start_reconfigurable_module_with_factory` with null host and out pointers and a factory whose body is unreachable, and asserts the returned code is `TB_BAD_ARGUMENT`.: Covers host validation before factory construction on the raw starter path; no execution evidence is present, so no pass or failure is asserted here. (crates/tinybus-module/src/setup_factory_tests.rs)

Findings

No active actionable findings.

Before merge

None.

How this fits together

flowchart LR
  n0["Connection"]:::impacted
  n1["start_module_runtime"]:::impacted
  n2["Result"]:::impacted
  n3["Output"]:::impacted
  n4["TbHostVtable"]:::impacted
  n5["start_reconfigurable_module_factory_mode"]:::impacted
  n1 -->|uses| n0
  n1 -->|uses| n2
  n1 -->|uses| n3
  n1 -->|uses| n4
  n5 -->|uses| n0
  n5 -->|calls| n1
  n5 -->|uses| n2
  n5 -->|uses| n3
  n5 -->|uses| n4
  classDef changed fill:#0d4429,stroke:#238636,color:#e6edf3
  classDef impacted fill:#161b22,stroke:#6e7681,color:#c9d1d9
  classDef flagged fill:#5a1e02,stroke:#d93f0b,color:#ffffff
  classDef blocking fill:#67060c,stroke:#f85149,color:#ffffff
Loading
Agent review details

critique

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Positive: The documentation claims for the factory form were checked against the shown macro rules, the `module_export!` doc comment and the setup-factory and static-link test cases, and matched; the documented file stays well under the 500-line limit.
  • Lane summary: Reviewed 4 files; 1 finding.

security

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Positive: No untrusted-input path, authorization change, or new network, subprocess or deserialization sink was introduced; the factory is author-supplied code compiled into the module, the configuration it later receives is still parsed through `serde_json::from_slice::<C>` in the unchanged reinitialize path, and error paths still return bare codes without echoing configuration.
  • Positive: The static-link fixture's new factory captures nothing, so its `Clone`/`Send`/`Sync` surface is derivable and it satisfies the same generic bounds as the runtime starter.
  • Lane summary: This change adds factory-based configured module setups (`setup_factory`) to the module export macros and unit tests for them; no untrusted input reaches a new sink, no authentication or authorisation logic changes, and the new unsafe starters validate host/out pointers and catch factory panics before returning to the host. 1 file was not security-reviewed: docs/modules/module/README.md (prose or tabular data).

tests

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Lane summary: The change adds a `setup_factory` form to `module_export`/`module_export_static`/`module_export_optional_static`, plus three new `start_*_with_factory` starters that construct per-instance setup state inside a `catch_unwind` after config parsing. A dedicated test module (`setup_factory_tests.rs`) exercises per-instance isolation across a `tokio::join!` of two instances, reinitialization reusing captured state, concurrent setup serialization, factory panic mapping to `ModuleRefused`, invalid config not constructing factory state, and null-host validation — each assertion can fail. I found no reachable defect in the diff and no violation of the repository rules quoted.

commits

  • Conclusion: Neutral
  • Scope reviewed: all assigned evidence
  • Positive: Nothing sensitive was found in what this pull request commits.
  • Lane summary: Nothing sensitive found in what this pull request commits.

description

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Positive: The description matches the diff closely, including the claimed regression evidence; the only omission noted is the minor conversion of the existing optional-static fixture from `setup` to `setup_factory`.
  • Lane summary: The change adds `setup_factory` forms to `module_export`, `module_export_static` and (transitively) `module_export_optional_static`, threading a `FnOnce() -> F` factory through a new `start_reconfigurable_module_factory_mode` that runs the factory under `catch_unwind` after config parsing, with the legacy `start_reconfigurable_module`/`start_linked_reconfigurable_module` paths forwarding `|| setup`. I traced the macro arms, the starter functions and the tests; the legacy setup path and the new factory path are wired consistently, the factory is invoked after `parse_config` so invalid config and null host are rejected first, and the per-instance state is cloned once per ABI start. I found nothing I can substantiate as a defect, so the description matches the diff and the change looks sound to merge.
Evidence and run details
  • Models: ladder/vectors-oai3, deepseek-v4.1-flash,
  • Spend: $0.002191
  • Tokens: 305529 input · 33637 output · 54641 cached · 492 embedding
Head State Pass summary
30a777cc11ea incomplete 0 active finding(s), 0 resolved finding(s) (at 2026-10-12T00:51:09Z)
30a777cc11ea ready for maintainer review 0 active finding(s), 0 resolved finding(s) (at 2026-10-12T01:46:18Z)

tinysweeper 0.1.2

@tinysweeper tinysweeper Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tinysweeper found nothing blocking, but could not review everything, so this is not an approval: crates/tinybus-module/src/lib.rs, crates/tinybus-module/src/setup_factory_tests.rs, crates/tinybus-module/src/static_link_tests.rs, tinysweeper/tests.

             $0.0023 · 218,637 in / 30,375 out · 29,824 cached (14%) · ladder/vectors-oai3, deepseek-v4.1-flash · 492 embedded
critique:    $0.0006 · 87,944 in  / 8,540 out  · 19,584 cached (22%) · deepseek-v4.1-flash
security:    $0.0009 · 84,589 in  / 13,242 out · 6,400 cached (8%)   · deepseek-v4.1-flash
description: $0.0004 · 38,702 in  / 4,294 out  · 3,840 cached (10%)  · deepseek-v4.1-flash

@tinysweeper tinysweeper Bot added the priority: p3 Whenever. Cosmetic, a nicety, or a cleanup with no user visible effect. label Oct 12, 2026

@tinysweeper tinysweeper Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tinysweeper found nothing blocking. Approving.

             $0.0022 · 305,529 in / 33,637 out · 54,641 cached (18%) · ladder/vectors-oai3, deepseek-v4.1-flash,  · 492 embedded
critique:    $0.0019 · 144,827 in / 29,373 out · 30,961 cached (21%) · deepseek-v4.1-flash,
security:    $0.0000 · 41,672 in  / 189 out    · 3,712 cached (9%)   · deepseek-v4.1-flash
tests:       $0.0001 · 61,817 in  / 809 out    · 6,272 cached (10%)  · deepseek-v4.1-flash
description: $0.0001 · 39,093 in  / 936 out    · 13,696 cached (35%) · deepseek-v4.1-flash

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority: p3 Whenever. Cosmetic, a nicety, or a cleanup with no user visible effect.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant