Expose a statically linked TinyDocs module entry - #18
Conversation
Tiny Sweeper reviewTiny Sweeper reviewed this change across 6 lane(s) and found 5 active actionable finding(s). Detailed lane evidence and any incomplete work are listed below. State: Incomplete Review snapshot
Completeness: Incomplete What changedThe review could not produce a supported behavioral summary; inspect the cited changed surface and lane details below. FeaturesNone identified with supported citations. TestsNo supported feature-to-test mapping was produced. Test execution is not inferred.
FindingsPreviously reported and still active
Pending checks: TinyBus module E2E Could not review: Cargo.toml, crates/tinydocs-module/Cargo.toml, crates/tinydocs-module/src/lib.rs, crates/tinydocs-module/src/service/mod.rs, tinysweeper/description, tinysweeper/e2e, tinysweeper/tests Before merge
How this fits togetherflowchart LR
n0["Documents<br/>changed"]:::changed
n1["setup<br/>changed"]:::changed
n2["service"]:::impacted
n3["generate_pptx"]:::impacted
n4["read_output"]:::impacted
n5["generate_docx"]:::impacted
n6["hold"]:::impacted
n7["generate_docx_holds_a_readable_document"]:::impacted
n1 -->|uses| n0
n2 -->|uses| n0
n3 -->|calls| n6
n5 -->|calls| n6
n7 -->|calls| n2
n7 -->|tests| n2
n7 -->|calls| n4
n7 -->|tests| n4
n7 -->|calls| n5
n7 -->|tests| n5
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
Agent review detailscritique
security
tests
commits
description
e2e
Evidence and run details
|
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (5)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe ChangesStatic linking support
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Feature Merge Risk: ⚪ Minimal · up to No confirmed defect in the supplied evidence blocks merging. The linked-host path still needs its planned integration check. Security Architecture ReviewSecurity architecture risk: 🟡 Moderate · up to The linked entry points retain the existing service methods, but the host-side admission and failure behavior of this new integration path has not been verified. No security exploit is established. Retained concerns
Security review detailsSecurity Blast Radius
Security Findings and Attack Paths
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
A rabbit checks the linking trail, Comment |
![tinysweeper[bot]](https://avatars.githubusercontent.com/in/4526585?s=60&v=4){kind=small}
There was a problem hiding this comment.
Requesting changes: 1 lane(s) blocking, worst finding is high.
Fix or reply to the findings below and push. The next review clears this automatically once they are gone — you should not need to dismiss anything by hand.
$0.0049 · 199,038 in / 21,948 out · 39,668 cached (20%) · ladder/vectors, gpt-5.6-luna, deepseek-v4-flash · 228 embedded
critique: $0.0017 · 66,082 in / 3,172 out · 8,796 cached (13%) · gpt-5.6-luna, deepseek-v4-flash
security: $0.0016 · 60,057 in / 1,658 out · 3,736 cached (6%) · gpt-5.6-luna
tests: $0.0009 · 46,755 in / 8,781 out · 19,968 cached (43%) · deepseek-v4-flash
description: $0.0002 · 7,508 in / 4,006 out · 2,560 cached (34%) · deepseek-v4-flash
e2e: $0.0003 · 16,171 in / 1,530 out · 4,608 cached (28%) · deepseek-v4-flash
| mod service; | ||
|
|
||
| /// Entry points for hosts that link `TinyDocs` into their own executable. | ||
| #[cfg(feature = "static-link")] |
There was a problem hiding this comment.
Exercise the static-link export path
The new static-link feature is only compiled when explicitly enabled, and the existing loader test exercises the dynamic-library path instead. No test builds this crate with static-link or verifies that the re-exported ABI symbols are present and callable, so errors in module_export_static!, the generated ABI, or the public re-exports will not be detected before a host tries to link against it. Add a dedicated static-link integration/compile test that enables the feature and calls or links the exported symbols.
[RULE] e2e-uncovered ·
|
|
||
| /// Entry points for hosts that link `TinyDocs` into their own executable. | ||
| #[cfg(feature = "static-link")] | ||
| pub mod linked { |
There was a problem hiding this comment.
Cover the static-linking path with an end-to-end test
The new static-link feature and linked module provide public ABI symbols for hosts that want to link TinyDocs directly rather than loading it as a dynamic library. The only end-to-end test (module_e2e) builds and loads the dynamic library; it never enables the static-link feature or verifies that the exported symbols are present and usable. A bug in the static-linking path (e.g. a missing import, a mismatched macro) would not be caught until a host tries to use it.
Add a test that compiles the crate with --features static-link and verifies the re-exported symbols are callable, or exercise the path through a minimal host link.
Additional tests observation
Add tests for the static-link feature path
[RULE] missing-test-coverage
The new pub mod linked and the conditional module_export_static! macro inside service/mod.rs are gated behind the static-link feature and have no test coverage. An earlier finding already flagged this gap. A compilation test or an integration test that builds with --features static-link and verifies the exported symbols are callable would prevent silent regressions.
[RULE] e2e-uncovered ·
tinysweeper
Bot
added
the
priority: p1
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Requesting changes: 4 lane(s) blocking, worst finding is critical.
Fix or reply to the findings below and push. The next review clears this automatically once they are gone — you should not need to dismiss anything by hand.
$0.0078 · 268,968 in / 34,511 out · 30,298 cached (11%) · ladder/vectors, gpt-5.6-luna, deepseek-v4-flash · 281 embedded
critique: $0.0039 · 130,965 in / 9,517 out · 6,164 cached (5%) · gpt-5.6-luna, deepseek-v4-flash
security: $0.0023 · 75,399 in / 5,153 out · 3,654 cached (5%) · gpt-5.6-luna
tests: $0.0009 · 39,470 in / 11,487 out · 14,336 cached (36%) · deepseek-v4-flash
description: $0.0002 · 5,098 in / 3,606 out · 1,024 cached (20%) · deepseek-v4-flash
e2e: $0.0003 · 14,051 in / 2,647 out · 1,536 cached (11%) · deepseek-v4-flash
| codegen-units = 1 | ||
| strip = "debuginfo" | ||
|
|
||
| [patch.crates-io] |
There was a problem hiding this comment.
Update Cargo.lock for the vendored TinyBus patches
These patches replace the registry resolutions recorded in the existing Cargo.lock with local path packages. Because the repository's CI and release commands use --locked, Cargo will reject the unchanged lockfile with “lock file needs to be updated but --locked was passed.” Regenerate and commit Cargo.lock after applying these patches.
[RULE] lockfile-sync ·
| tinybus = { version = "=0.1.2", default-features = false, features = ["macros", "modules"] } | ||
| # The module-side SDK owns its runtime and exports the stable C entrypoints. | ||
| tinybus-module = { version = "0.1.0", path = "../../vendor/tinybus/crates/tinybus-module" } | ||
| tinybus-module = "=0.1.2" |
There was a problem hiding this comment.
Update vendor/tinybus to provide module_export_static! macro
The workspace patches tinybus-module to the vendored source, so this registry-looking dependency still resolves to the checked-out vendor implementation during workspace builds. The static-link implementation requires the vendor's static-export macro/API, but the vendor submodule was not available to verify that it provides the required macro. If it does not, enabling static-link fails to compile; update the vendored TinyBus revision/package together with this manifest change and verify the feature build.
[RULE] dependency-api-mismatch ·
|
|
||
| [features] | ||
| # Link this adapter into a host binary without exporting the shared C names. | ||
| static-link = [] |
There was a problem hiding this comment.
Add tests for the static-link feature path
The feature declaration changes the crate's supported build matrix, but no feature-specific test is present for it. At minimum, CI should compile and test the crate with --features static-link; preferably it should also exercise the exported entry points so this configuration cannot silently drift.
Additional critique observation
Exercise the static-link export path
[RULE] missing-feature-coverage
This feature introduces a separate public linking mode, but the repository context shows no test exercising the exported static-link symbols. A build or API change in this path can therefore pass CI while producing unusable host entry points. Add coverage that enables static-link and verifies the exported entry points can be linked and called.
Additional security observation
Exercise the static-link export path
[RULE] missing-feature-coverage
This pull request introduces the static-link feature, but the repository still has no test that builds or loads the static-linked adapter. A feature-specific export or ABI regression can therefore pass CI unnoticed. Add coverage that enables this feature and verifies the resulting entry points.
Additional critique observation
Cover the static-linking path with an end-to-end test
[RULE] missing-integration-test
The new feature is intended for hosts that link the adapter into their executable, yet the available test graph contains no caller for these symbols. Add an end-to-end host test that builds with static-link, links the adapter, and reaches the module initialization/manifest path rather than only testing the dynamic library.
Additional security observation
Cover the static-linking path with an end-to-end test
[RULE] missing-feature-coverage
The static-linking path is a distinct integration mode from the cdylib path and is not exercised by the existing dynamic-module loader test. Add an end-to-end test that links the adapter into a host and invokes its exported service.
Additional security observation
Add tests for the static-link feature path
[RULE] missing-feature-coverage
Enabling static-link currently adds an untested build configuration. Add a deterministic feature-enabled test or CI job so compilation and the static export behavior are checked before regressions reach users.
Additional security observation
Add a test for the static-linking entry points
[RULE] missing-feature-coverage
The newly declared feature changes how the module's entry points are provided, yet no test covers those entry points in static-link mode. Add a test that calls the statically linked interface rather than only testing the separately loaded artifact.
[RULE] missing-feature-test ·
| tinydocs-bus = { path = "../tinydocs-bus" } | ||
| # TinyBus provides the typed service interface and dynamic module host ABI. | ||
| tinybus = { version = "0.1.0", path = "../../vendor/tinybus/crates/tinybus", default-features = false, features = ["macros", "modules"] } | ||
| tinybus = { version = "=0.1.2", default-features = false, features = ["macros", "modules"] } |
There was a problem hiding this comment.
Add a test for the static-linking entry points
The manifest now exposes a static-link configuration, while the repository context explicitly shows no test calling the resulting entry points. This leaves ABI initialization, manifest export, and symbol availability unchecked for the new caller mode. Add a dedicated test target or host fixture that enables the feature and invokes those entry points.
[RULE] missing-static-link-test ·
| }; | ||
| } | ||
|
|
||
| #[cfg(feature = "static-link")] |
There was a problem hiding this comment.
Cover the static-linking path with an end-to-end test
The new linked_module entry point has no end-to-end coverage. Nothing verifies that a statically linked host can initialize the module and route a request through the generated exports, so this path can regress independently of the existing dynamic-loader tests. Add an end-to-end test or CI target that enables static-link and invokes the linked module.
Additional security observation
Add tests for the static-link feature path
[RULE] missing-feature-test
There is no feature-enabled test covering compilation and behavior of the newly added static-link surface. Without it, changes to the conditional exports or macro expansion can silently break static consumers while the default feature tests continue to pass.
Additional tests observation
Cover the static-linking path with an end-to-end test
[RULE] no-test-coverage
The static-linking path is used when a host links tinydocs-module as an rlib and calls linked_module to register the service. No test verifies that this works end-to-end when the feature is enabled. Add a test (possibly integration) that enables the feature and verifies the module can be initialised.
[RULE] missing-test ·
| mod service; | ||
|
|
||
| /// Entry points for hosts that link `TinyDocs` into their own executable. | ||
| #[cfg(feature = "static-link")] |
There was a problem hiding this comment.
Cover the static-linking path with an end-to-end test
The static-link feature introduces a distinct host-integration path, but the existing loader coverage only exercises the dynamic module artifact. Add an end-to-end test that links the module into a host and reaches the service through that path.
Additional tests observation
Exercise the static-link export path
[RULE] no-test-coverage
The static-link feature adds publicly exported entry points (linked_module, ABI symbols) but no test exercises them. A unit test that builds with the feature and calls one of these symbols would catch regressions (e.g., a missing re-export or a renamed macro). Add a test under #[cfg(feature = "static-link")] in a test module or in the existing test file.
Additional tests observation
Add a test for the static-linking entry points
[RULE] no-test-coverage
The linked module re-exports ABI symbols like tinybus_module_init_v1. There is no test ensuring these symbols are actually reachable under the static-link feature. Add a test that enables the feature and calls one of them (or at least checks that the module compiles and the symbols are present).
[RULE] missing-integration-test ·
| /// Entry points for hosts that link `TinyDocs` into their own executable. | ||
| #[cfg(feature = "static-link")] | ||
| pub mod linked { | ||
| pub use crate::service::exports::{ |
There was a problem hiding this comment.
Add a test for the static-linking entry points
The ABI, init, and manifest symbols newly exposed through linked are not exercised by any test. Add a static-link test that resolves these entry points and verifies the manifest and initialization behavior expected by a linked host.
[RULE] missing-entrypoint-test ·
| pub mod outputs; | ||
| mod service; | ||
|
|
||
| /// Entry points for hosts that link `TinyDocs` into their own executable. |
There was a problem hiding this comment.
Exercise the static-link export path
The static-link feature adds new public symbols (linked_module, TINYBUS_MODULE_ABI_V1, tinybus_module_init_v1, tinybus_module_manifest_v1) that are reachable by a host binary linking the crate as an rlib. No end-to-end test loads or calls these symbols. A test should build a small host program that depends on tinydocs-module with features = ["static-link"] and calls each exported function, verifying that the module initializes and reports a correct manifest. This finding was raised in earlier cycles and remains unaddressed.
[RULE] e2e-uncovered ·
| repository = "https://github.com/tinyhumansai/tinydocs" | ||
| publish = false | ||
|
|
||
| [features] |
There was a problem hiding this comment.
Add tests for the static-link feature path
The static-link feature is defined but not tested. The absence of an end-to-end test means any regression in the static-linking entry points will go undetected. A test should be added (e.g., in tests/ as an integration test or via a #[cfg(feature = "static-link")] test) that exercises the feature. This finding was raised in earlier cycles and remains unaddressed.
Additional tests observation
Add tests for the static-link feature path
[RULE] no-test-coverage
The new static-link feature flag is added with no corresponding test that exercises the code it gates. A test in the crate (e.g., a local unit test) should build with features = ["static-link"] and assert that the exported items exist and are callable.
[RULE] e2e-uncovered ·
| )] | ||
| mod exports { | ||
| tinybus_module::module_export! { | ||
| pub(crate) mod exports { |
There was a problem hiding this comment.
Add a test for the static-linking entry points
The change uses module_export_optional_static! instead of module_export!. The exported symbols (ABI version, init, manifest) are now gated behind pub(crate) visibility and the macro may behave differently when the module is statically linked. No test verifies that linked_module returns a properly configured module host or that the ABI functions are callable. This finding was raised in earlier cycles and remains unaddressed.
[RULE] e2e-uncovered ·
Co-authored-by: Medulla <medulla@tinyhumans.ai>
There was a problem hiding this comment.
tinysweeper found nothing blocking, but could not review everything, so this is not an approval: Cargo.toml, crates/tinydocs-module/Cargo.toml, crates/tinydocs-module/src/lib.rs, crates/tinydocs-module/src/service/mod.rs, tinysweeper/description, tinysweeper/e2e, tinysweeper/tests.
$0.0000 · 0 in / 0 out · 281 embedded · ladder/vectors
1 participant
Summary
tinydocs-module/static-linkusing TinyBusmodule_export_optional_static!, which selects dynamic exports by default and a Rust-addressable module in static builds.tinydocs_module::linked_module()for direct host registration. The earlierlinkedraw ABI entry points remain available.433d9edand use=0.1.2registry dependency identities, with local Cargo patches for standalone builds.Related issue
Depends on tinyhumansai/tinybus#29, which builds on merged #28. Ready condition: #29 is merged, this gitlink is reachable from upstream TinyBus, and CI passes. This PR remains a draft until then.
API or behavior changes
Additive public
linked_module()understatic-link. The default loadable module path and wire contract are unchanged.Validation
cargo fmt --manifest-path Cargo.toml --all -- --checkcargo check --manifest-path Cargo.toml -p tinydocs-module --offlinecargo check --manifest-path Cargo.toml -p tinydocs-module --features static-link --offlineTests
The two feature builds verify both TinyBus macro expansions and the public root reexport. No runtime behavior changes were added in this package.
Documentation
The feature and root entry point are documented in this PR and by TinyBus #29.
Checklist
Summary by CodeRabbit