Repository navigation
feat(module): add bounded native terminal factories and physical cleanup ownership - #61
Merged
Merged
Conversation
Add a hidden `__set_catch_observer` setter that lets the SDK register a bounded callback fired before a panicking method's payload is dropped, so callers can fence state ahead of the drop. A test confirms the observer runs before the primary payload is released. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
CatchUnwind now carries an optional observer callback that is invoked when a dispatched call panics, before the panic payload is dropped. This lets opted-in modules close native authority ahead of foreign payload destruction, since no registry or handler lock is held at that point. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The native callback setup race test now selects on the load task, the attach notification, and a deadline so a load that finishes or stalls before attach fails with a clear message instead of a bare timeout. The host helper tests create their artifacts inside a private subdirectory with a protected ACL, so files no longer inherit the checkout's DACL on Windows. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Extract the attach wait into a helper and add tests proving a valid preload outliving product deadlines is not mistaken for setup failure, and that an early load failure surfaces as a panic. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The native callback attach helper now uses a biased select, aborts the load task on timeout, and waits up to 60 seconds so directory admission and hashing cannot be mistaken for a setup failure. A new test asserts that a stuck preload is cancelled, and the early-failure test now checks the panic message contents. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add a fixture and test asserting that an early unwind wakes native entry and releases the finish and decision channels rather than disconnecting them. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
…src/module/host_callbacks_tests Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add an ignored test that exercises the controlled-release guard during an ordinary Rust unwind while native entry is blocked, asserting that stop completes and the barrier is reclaimed. Rework the native fixture attachment failure path to report the original cause without turning it into a readiness failure, and make the artificial finish gate tolerate a poisoned lock and a disconnected channel so a failed assertion cannot unwind through the C callback. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
…ll_fn The native callback tests now capture the loader's poll result and assert on it after cleanup, so a premature completion or an expired entry watchdog releases the fixture and consumes the completion before failing rather than panicking inside the poll closure. Initialization results are also asserted after the transport is released, keeping the failure path leak-free. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Rework the controlled release guard to commit and schedule synchronously via request_terminal instead of spawning on the current Tokio runtime, so the physical owner is retained even when the runtime is dropped. Add an ignored native test that cancels a stop waiter and drops the runtime to verify the owner is reclaimed only after the callback returns, and relax the early-unwind assertion to accept a not-started connection-closed stop. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The attach test helper now maps module refusals, closed connections, and other load errors to separate outcomes instead of lumping every error into LoadRefused. A new test covers a genuine admission refusal before the C entry point, asserting the module is not attached or published as Ready. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The closed-delivery fixture now releases the shared vtable lock only after the transport's physical cleanup has actually run, and a new test asserts that a fault reply does not let the next fixture start early. This keeps the global vtable state serialized through its final writes instead of freeing it as soon as the error reply is observed. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The shared vtable fixture now waits for physical cleanup before its final global writes, and the fault-reply test awaits its turn on the fixture lock instead of treating legitimate contention after release as a failure. This keeps the serialized fixture assertions from racing with other queued tests. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The pending-call test now captures the receive result and fault state before requesting terminal cleanup, then asserts afterwards. This keeps the fixture's global writes serialized so the test no longer races with cleanup. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
# Conflicts: # crates/tinybus/src/module/transport.rs
Extend the host callback lifecycle tests to exercise stop drain timeout and cancellation paths alongside the existing complete-drain case, asserting that stop intent is retained, new admission is denied, and the module owner is kept until admitted work finishes. Add a test-only accessor for the native terminal state so the tests can observe that authority closes before provider work drains. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Bind the transport to its host callback state so physical cleanup can wait for in-flight user futures to finish before releasing native exclusion. Admission is now revoked immediately on module stop, and the callback lifetime lock tolerates poisoning so a panicking callback cannot wedge shutdown. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add a test-only notification that fires when the physical drain wait begins, and use it to assert that dropping the host after a cancelled stop still waits for the provider future to be destroyed before native cleanup completes. Also add tests that a transport rejects rebinding a different callback account and that runtime drop destroys the provider future before off-runtime native cleanup. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
…e_tests.rs,crates/tinybus/src/m Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Rework the callback lifecycle validation notes to describe the single exact CallbackState retained by the loaded record and transport, the stop intent committed before draining admitted futures, and the last-host drop path that waits on the physical owner. The previous text no longer matched the implemented ordering and marker semantics. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
…es/tinybus/src/module/host_call Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Host stop now commits ordinary intent before its bounded provider drain, with drain or cancellation preserving that intent and the loaded owner while native physical handoff waits for provider completion. Direct transport stop commits after acquiring the native lifecycle lock, and canceling that wait before acquisition makes no new commitment. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
…-bus-7328 # Conflicts: # crates/tinybus/src/module/host_callbacks_tests.rs # crates/tinybus/src/module/host_tests.rs
…ispatch Add a test that captures the tracing dispatch active on the factory and configuration caller paths, then verifies that after shutdown the captured dispatch no longer forwards events and cannot select a replacement module's host callback. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
…s.rs,crates/tinybus/src/module/ Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The module start path now validates its arguments and derives the execution owner before entering the runtime, so an invalid host or worker count returns TB_BAD_ARGUMENT instead of partially starting. Reload tests were reworked to read recorded log lines through a poison-tolerant helper, and a failing-init test now takes the shared fake-module lock to avoid cross-test interference. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The shutdown, host reload, and transport tests now release their worker fixtures through guards and watchdogs so a stalled cleanup cannot hang the suite, and the transport test additionally waits for the real lifecycle lock to be released before asserting. Assertions and formatting were reflowed to keep the intent of each check explicit. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Extend the native log fixture to exercise the callback with no storage available, asserting it returns without unwinding across the ABI, and key the recorded log lookup off a stable pointer instead of a literal. Add a comment in start_module_runtime explaining that the validated entry caller's diagnostic owner must be carried into the runtime so factory, config dispatch, and shutdown share the same live bit. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The ABI callback fixture now passes the existing key variable to observe_log instead of a hardcoded pointer, keeping the test consistent with the surrounding setup. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
…ecurity-bus-7328
The terminal fixture now records whether the previous future's Drop runs on the tinybus-terminal worker thread and expects the construction Handle guard to end before its ack, so the test verifies that the owner is restored rather than merely that a configured Handle was available. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The terminal example now records an observation of mode 39 when the setup future is dropped in modes 48 or 49, keeping the example's mode bookkeeping consistent with the emitted state. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
…lidation Add tests asserting that terminal callbacks treat zero and unknown context tokens as closed, that legacy callbacks still report a null context as a bad argument, and that invalid frame and config sizes remain bad arguments. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The terminal callbacks no longer reject a null ctx up front, since TerminalState::lookup already handles that case and returns TB_CLOSED. This keeps the argument validation consistent across the three entry points and avoids returning TB_BAD_ARGUMENT for a state that is simply gone. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Clarify the obligations of the terminal setup factory and its runtime hooks, covering partial-failure cleanup, worker launch timing, and reinitialization reuse. Also document that null or unknown context tokens return TB_CLOSED for valid terminal callbacks while legacy shutdown keeps its null-context behavior, and note that the native fixture support feature is private and off by default. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The `Uncertain` terminal completion variant now states that either controller or SDK physical cleanup may be unproved, since even joined workers can leave uncertain SDK resources behind. The ABI documentation was reworded to match, without changing any behaviour. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Co-authored-by: Medulla <medulla@tinyhumans.ai>
senamakel
changed the title
senamakel
marked this pull request as ready for review
October 12, 2026 17:56
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Tiny Sweeper reviewTiny Sweeper reviewed this change across 5 lane(s) and found 0 active actionable finding(s). Detailed lane evidence and any incomplete work are listed below. State: Incomplete Review snapshot
Completeness: Incomplete FeaturesNone identified with supported citations. TestsNo supported feature-to-test mapping was produced. Test execution is not inferred.
FindingsNo active actionable findings. Could not review: crates/tinybus-module/src/lib_terminal_tests.rs, crates/tinybus-module/src/terminal_tests.rs, crates/tinybus/src/module/client_tests.rs, scripts/check-coverage.sh, scripts/test-e2e.ps1, scripts/test-e2e.sh, tinysweeper/tests Before merge
How this fits togetherflowchart LR
n0["clone"]:::impacted
n1["load_file_with_host_callbacks"]:::impacted
n2["defer_initialize"]:::impacted
n3["ensure_initialized"]:::impacted
n1 -->|calls| n0
n1 -->|calls| n2
n1 -->|calls| n3
n3 -->|calls| n0
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
Evidence and run details
|
![tinysweeper[bot]](https://avatars.githubusercontent.com/in/4526585?s=60&v=4){kind=small}
There was a problem hiding this comment.
tinysweeper found nothing blocking, but could not review everything, so this is not an approval: crates/tinybus-module/src/execution_context_tests.rs, crates/tinybus-module/src/lib.rs, crates/tinybus-module/src/lib_terminal_tests.rs, crates/tinybus-module/src/terminal_tests.rs, crates/tinybus/Cargo.toml, crates/tinybus/examples/module_terminal.rs, crates/tinybus/src/module/host.rs, crates/tinybus/src/module/host_callback_lifecycle_tests.rs and 6 more.
$0.0083 · 3,179,742 in / 82,577 out · 427,648 cached (13%) · ladder/vectors-oai3, deepseek-v4.1-flash, · 708 embedded
critique: $0.0060 · 2,235,918 in / 55,826 out · 260,864 cached (12%) · deepseek-v4.1-flash
security: $0.0017 · 378,436 in / 20,817 out · 74,112 cached (20%) · deepseek-v4.1-flash,
tests: $0.0002 · 256,357 in / 1,768 out · 85,376 cached (33%) · deepseek-v4.1-flash
description: $0.0001 · 229,959 in / 1,183 out · 7,296 cached (3%) · deepseek-v4.1-flash
tinysweeper
Bot
added
the
priority: p3
The e2e and coverage scripts now build and pin the module_terminal fixture alongside the other loader examples, and each terminal SDK scenario runs in its own process so retained fixture state cannot leak between tests. The scripts also resolve the Cargo target directory via cargo metadata instead of assuming target/debug. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The e2e and coverage scripts now list the compiled terminal scenarios up front and fail early when none are found, instead of feeding the test runner from a process substitution that could silently yield nothing. The coverage script also passes --no-clean so per-test runs accumulate into a single report. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The per-scenario test and coverage invocations for the compiled native terminal fixtures now pass the tinybus/macros feature alongside the existing flags, so those scenarios build with the macro support they require instead of failing to compile. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The per-test coverage loop invoked cargo llvm-cov without the test subcommand, which is not a valid invocation. Adding the subcommand makes the ignored terminal scenario tests actually run under coverage. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The per-test llvm-cov invocations in the terminal scenario loop passed --no-clean, which wiped the accumulated coverage data between tests and left the final report incomplete. Removing the flag lets each run append to the existing profile so the aggregate report reflects every scenario. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
…odule-coverage The private module fixtures used by the coverage check were staged under target/private-module-tests, which collides with the directory used by the test suite. They now live under target/private-module-coverage so the two runs no longer share state. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
…entity Add tests for JSON and object fixture calls preserving deadlines and confidential refusals, and for unavailable loads and explicit reports using only the registered identity. Also assert the retained terminal view reports the exact execution owner and surviving SDK resources, and that a started runtime exposes a unique connection name. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The test previously assumed host shutdown returned only after the physical worker finished, but the shutdown deadline can elapse first. It now drains the channel until the expected finish messages arrive, so the assertions run against the worker's actual completion. Auto-committed-on: dragonfly 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: crates/tinybus-module/src/lib_terminal_tests.rs, crates/tinybus-module/src/terminal_tests.rs, crates/tinybus/src/module/client_tests.rs, scripts/check-coverage.sh, scripts/test-e2e.ps1, scripts/test-e2e.sh, tinysweeper/tests.
$0.0105 · 696,141 in / 74,307 out · 99,712 cached (14%) · ladder/vectors-oai3, deepseek-v4.1-flash, , glm-5.3-flash · 732 embedded
critique: $0.0030 · 195,721 in / 43,990 out · 61,440 cached (31%) · deepseek-v4.1-flash
security: $0.0005 · 247,025 in / 5,828 out · 36,096 cached (15%) · deepseek-v4.1-flash,
description: $0.0042 · 165,859 in / 20,306 out · 1,664 cached (1%) · glm-5.3-flash, deepseek-v4.1-flash
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add opt-in native and linked terminal setup factories with one execution owner, once request/finish hooks, and a fixed 256-instance registry of checked context tokens. The SDK retains received configuration, setup futures, the initial task, runtime, connection and transport until actual settlement. Reinitialization and foreign cleanup run outside bookkeeping locks on the retained physical worker; destructor failures fault the instance and retain uncertainty rather than reporting physical completion. Legacy setup factories remain supported.
Also fixes Connection teardown ownership, panic fencing before payload destruction, zero-token Closed results, and host/provider shutdown ordering. Real loaded fixtures cover admission, retained-view capacity, startup failures, hooks, cancellation, reinitialization, nested owner restoration and blocked/panicking cleanup. Public contracts and trusted-controller obligations are documented. Latest main, including version 0.1.5 and fixture isolation, is integrated with a normal merge preserving all commits.
Validation at 5aad621: independent broad and scoped source reviews passed. Modern and Rust 1.85 workspace/feature checks, tests and doctests, formatting, strict Clippy and rustdoc passed. Both toolchains rebuilt their own eight pinned native fixtures and passed 51 host/CLI native tests plus every module native group in a fresh process. Fresh instrumented workspace and all 11 native groups also passed.
Coverage is not accepted for this head: the first raw per-file result failed; its object inventory omitted the instrumented production proc-macro DLL, and the corrected export was stopped at the user's request. Changed-line scoring was not run. Earlier passing coverage and hosted three-OS checks belong to the earlier 5fc7575 checkpoint. Fresh hosted CI for this head remains pending.
This publishes the completed lifecycle implementation for tinyhumansai/openhuman#7328. It does not implement the proposed native confidential codec, TinySecurity App/provider adapter, pairing integration, TinyBox expansion or remaining OpenHuman migrations, and does not close the tracker. Further implementation stopped at the user's request.
Summary by CodeRabbit