Add optional static TinyMemory module exports - #166
Conversation
Tiny Sweeper reviewTiny Sweeper reviewed this change across 6 lane(s) and found 1 active actionable finding(s). Detailed lane evidence and any incomplete work are listed below. State: Ready for maintainer review Review snapshot
Completeness: Complete 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
Resolved this pass
Before merge
How this fits togetherflowchart LR
n0["call"]:::impacted
n1["assert"]:::impacted
n2["admit_module"]:::impacted
n3["proxy"]:::impacted
n4["..._runs_over_the_bus_and_lands_in_the_store"]:::impacted
n5["Result"]:::impacted
n4 -->|calls| n0
n4 -->|tests| n0
n4 -->|calls| n1
n4 -->|calls| n2
n4 -->|calls| n3
n4 -->|uses| 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 ignored due to path filters (1)
📒 Files selected for processing (7)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe module adds an opt-in ChangesStatic-Link Module Exports
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Feature Sequence Diagram(s)sequenceDiagram
participant StaticLinkTest
participant TinyMemoryModule
participant ModuleHost
participant InMemoryBroker
participant ClientProxy
StaticLinkTest->>TinyMemoryModule: Read ABI descriptor, manifest, and initializer
StaticLinkTest->>ModuleHost: Attach linked module with workspace configuration
StaticLinkTest->>InMemoryBroker: Start broker
StaticLinkTest->>ClientProxy: Call DriverId
ClientProxy->>InMemoryBroker: Send DriverId request
InMemoryBroker->>TinyMemoryModule: Deliver DriverId request
TinyMemoryModule-->>ClientProxy: Return "tinycortex"
Suggested reviewers: Merge Risk: ⚪ Minimal · up to The supplied evidence identifies no issue that must be fixed before merging. Complete the normal build and test checks before merge. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to Static linking gives a host another way to attach the memory module without changing the default build or its declared service interface. The remaining risk is limited but not fully resolved: the new attachment path and its failure handling have not been verified against the host implementation. 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 ABI path, Comment |
There was a problem hiding this comment.
tinysweeper found nothing blocking. Approving.
$0.0083 · 331,866 in / 13,498 out · 17,071 cached (5%) · ladder/vectors, gpt-5.6-luna, deepseek-v4-flash · 382 embedded
critique: $0.0043 · 167,948 in / 3,542 out · 10,346 cached (6%) · gpt-5.6-luna
security: $0.0030 · 116,807 in / 1,765 out · 3,653 cached (3%) · gpt-5.6-luna
tests: $0.0003 · 16,405 in / 1,202 out · 1,536 cached (9%) · deepseek-v4-flash
description: $0.0002 · 7,254 in / 2,595 out · 0 cached (0%) · deepseek-v4-flash
e2e: $0.0004 · 20,188 in / 2,483 out · 1,536 cached (8%) · deepseek-v4-flash
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.0078 · 303,172 in / 20,792 out · 32,528 cached (11%) · ladder/vectors, gpt-5.6-luna, deepseek-v4-flash · 626 embedded
critique: $0.0037 · 143,928 in / 7,985 out · 18,420 cached (13%) · gpt-5.6-luna, deepseek-v4-flash
security: $0.0030 · 113,288 in / 4,401 out · 10,524 cached (9%) · gpt-5.6-luna
tests: $0.0003 · 15,574 in / 1,658 out · 1,536 cached (10%) · deepseek-v4-flash
description: $0.0002 · 6,534 in / 1,445 out · 1,024 cached (16%) · deepseek-v4-flash
e2e: $0.0004 · 19,332 in / 1,466 out · 1,024 cached (5%) · deepseek-v4-flash
There was a problem hiding this comment.
The previously-blocking findings are resolved. Clearing the changes request.
$0.0031 · 124,166 in / 9,155 out · 6,169 cached (5%) · ladder/vectors, gpt-5.6-luna, deepseek-v4-flash · 653 embedded
critique: $0.0009 · 35,449 in / 918 out · 2,073 cached (6%) · gpt-5.6-luna
security: $0.0010 · 35,017 in / 1,018 out · 0 cached (0%) · gpt-5.6-luna
tests: $0.0003 · 17,845 in / 868 out · 1,536 cached (9%) · deepseek-v4-flash
description: $0.0002 · 9,376 in / 1,474 out · 1,536 cached (16%) · deepseek-v4-flash
e2e: $0.0004 · 21,610 in / 2,063 out · 1,024 cached (5%) · deepseek-v4-flash
There was a problem hiding this comment.
tinysweeper found nothing blocking. Approving.
$0.0032 · 112,859 in / 20,142 out · 10,587 cached (9%) · ladder/vectors, gpt-5.6-luna, deepseek-v4-flash · 678 embedded
critique: $0.0014 · 51,830 in / 2,750 out · 6,491 cached (13%) · gpt-5.6-luna, deepseek-v4-flash
security: $0.0005 · 12,886 in / 1,252 out · 0 cached (0%) · gpt-5.6-luna
tests: $0.0004 · 16,034 in / 2,939 out · 1,536 cached (10%) · deepseek-v4-flash
description: $0.0003 · 7,111 in / 5,487 out · 1,024 cached (14%) · deepseek-v4-flash
e2e: $0.0005 · 19,798 in / 3,926 out · 1,536 cached (8%) · deepseek-v4-flash
There was a problem hiding this comment.
Requesting changes: 2 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.0065 · 246,202 in / 20,420 out · 31,048 cached (13%) · ladder/vectors, gpt-5.6-luna, deepseek-v4-flash · 682 embedded
critique: $0.0026 · 95,430 in / 6,219 out · 12,928 cached (14%) · gpt-5.6-luna, deepseek-v4-flash
security: $0.0028 · 102,069 in / 4,407 out · 10,952 cached (11%) · gpt-5.6-luna
tests: $0.0003 · 16,153 in / 1,730 out · 2,048 cached (13%) · deepseek-v4-flash
description: $0.0002 · 7,213 in / 3,193 out · 2,560 cached (35%) · deepseek-v4-flash
e2e: $0.0004 · 19,918 in / 1,976 out · 2,560 cached (13%) · deepseek-v4-flash
|
@tinysweeper Please re-review the current head after the resolved threads. The module CI job passed, including the static_link target and two-pass 81.77% coverage. The prior review report noted its code index was behind at 5c85aca; current head is 90a8c07, where static_link.rs and the pinned TinyBus attach_raw_with_config API are present. |
There was a problem hiding this comment.
The previously-blocking findings are resolved. Clearing the changes request.
$0.0135 · 510,306 in / 25,645 out · 32,157 cached (6%) · ladder/vectors, gpt-5.6-luna, deepseek-v4-flash · 682 embedded
critique: $0.0074 · 270,623 in / 9,682 out · 12,423 cached (5%) · gpt-5.6-luna
security: $0.0047 · 182,271 in / 4,815 out · 12,566 cached (7%) · gpt-5.6-luna
tests: $0.0005 · 19,154 in / 3,617 out · 1,536 cached (8%) · deepseek-v4-flash
description: $0.0004 · 10,214 in / 5,308 out · 1,024 cached (10%) · deepseek-v4-flash
e2e: $0.0004 · 22,911 in / 870 out · 1,536 cached (7%) · deepseek-v4-flash
Summary
Add an opt-in
static-linkfeature totinymemory-moduleso OpenHuman can link its module in-process through Rust-addressable TinyBus ABI entries. The default build retains its dynamic C exports. Both modes expand one module declaration, keeping the method manifest identical. Pin the nested TinyBus submodule to the canonical merge of tinybus#28.Related issue
None.
API or behavior changes
Additive feature and public descriptor, manifest, and initializer exports when
static-linkis enabled. Default behavior is unchanged.Validation
cargo fmt --all -- --checkcargo clippy --locked --manifest-path crates/tinymemory-module/Cargo.toml --all-targets -- -D warningscargo clippy --locked --manifest-path crates/tinymemory-module/Cargo.toml --all-targets --features static-link -- -D warningscargo build --locked --manifest-path crates/tinymemory-module/Cargo.tomlcargo test --locked --manifest-path crates/tinymemory-module/Cargo.toml --libcargo test --locked --manifest-path crates/tinymemory-module/Cargo.toml --test module_e2e(isolated dynamic loader cases)cargo test --locked --manifest-path crates/tinymemory-module/Cargo.toml --features static-link --test static_linknmconfirms the default dylib still exports TinyBus descriptor, manifest, and initializer symbolsTests
Added a static-link integration test that attaches the module through
ModuleHost::attach_raw_with_configand callsDriverIdover TinyBus. CI runs static-link clippy and the test; the existing dynamic loader E2E covers default behavior. The linked-host test uses narrowly documented unsafe calls because TinyBus requires callers to guarantee the ABI entry points remain valid for the host lifetime;unsafe_coderemains denied in the module and is allowed only on this test function. The module coverage job combines dynamic and all-feature profiles under the unchanged 80% line floor (81.77% locally).Documentation
Updated
docs/specs/tinybus-module.mdand module crate docs with the linked-mode feature and entry paths.Checklist
.envcontents in the diff or the descriptionSummary by CodeRabbit