Repository navigation
fix: retain native terminal ownership across cancellation - #57
Conversation
…ure paths Add tests for shutdown behaviour when the host callback blocks, when a started shutdown is cancelled, when an initialized module is stopped before becoming ready, and when a stop wakes a backpressured delivery. A drop guard releases the blocked callback so a failed assertion cannot hang later tests. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Stopping a module now marks the transport as stopped under the pending queue lock, so sends, initialization, and the drainer all observe closure without being reported as a module fault. Queued messages are dropped and observers are woken once the opaque shutdown callback returns, keeping a caller timeout from being mistaken for physical completion. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Initialization failures were collapsed into a single string error, so a transport that was stopped mid-init surfaced as a generic transport error instead of a connection-closed one. The failure is now tracked as a distinct closed or failed variant, and callers only treat the closed case as a shutdown when deciding whether to mark initialization as failed. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add host and transport tests asserting that an ordinary stop never fabricates a faulted state, whether the module is blocked in its shutdown callback or waiting for initialization admission, while a genuine initializer failure during stop is still retained. Also reset the shared delivery code between tests and assert the pending queue drains on stop. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Delivery failures now carry the underlying error into fault_pending, which only clears queued calls when the module is stopped and the error is a connection close. Previously any failure while stopped discarded pending calls, and the stop path could finish before the module vtable was cleared. Comments were updated to describe admission rejection rather than shutdown. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Move the stop_task_spawned reset to after the lifecycle lock is acquired so a retry that fails to take the lock no longer clears the marker set by the original running stop. The flag is now only replaced by the new lock owner right before the task is spawned. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The redundant store of false before setting stop_task_spawned to true could erase the original running stop's host transition, so it has been removed. Tests now cover concurrent pending sends racing a completed drainer and a delivery failure overlapping stop. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Stopping or reinitializing a transport that has already been disposed now fails fast with ConnectionClosed instead of racing the lifecycle lock, and the host no longer clears its transition marker when a stop retry is refused. This keeps a disposed transport from being revived or from erasing the original stop's host transition. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add a regression test asserting that stop and reinitialize waiters blocked behind an in-flight shutdown fail with ConnectionClosed instead of re-entering the disposed transport, and that shutdown runs only once. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Extend the ordinary stop test to drive a real broker and observe ModuleStateChanged signals, so the retry against a closed transport is checked to fail with ConnectionClosed while the loaded entry still reports Ready. The test now also verifies the emitted stopped event carries the expected ready-to-stopped transition. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The assertion on the recorded transition origin now clones the value and matches it against Some(ModuleState::Ready) instead of comparing with assert_eq!, avoiding a borrow of the guarded field across the assertion. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The module host was overwriting transition_from with the previous state whenever a restart began, which discarded a transition already in progress. Using get_or_insert keeps the original value so the state machine reports the correct origin state. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Add tests asserting that fresh signals and calls are rejected with ConnectionClosed once shutdown has actually begun, that queued readiness and backpressure retire before stop returns, and that an in-flight signal callback is drained even when stop times out and is cancelled. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add two tests for module transport admission while an actual shutdown is in progress. One verifies that an installed vtable without the init flag closes fresh admission before teardown, and the other verifies that a backpressure retry closes without faulting the transport. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Shutdown now closes admission separately from physical completion and waits for every reserved native callback to return before tearing down the module, so a stop timeout or cancelled caller can no longer free code underneath a running callback. Readiness signals arriving after admission closes are ignored, and queued confidential bodies are released outside internal locks. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
A ready gate mutex now guards the host ready callback and the shutdown path so a late ready notification can no longer race past admission closure and mark a stopping transport as ready. The lifecycle tests were updated to drive shutdown concurrently and assert that reinitialization fails fast with ConnectionClosed instead of staying pending. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add tests asserting that a reserved native entry cannot start after admission closes even when the stop caller is cancelled, and that unanswered method calls do not hold physical callback leases. Document the shutdown barrier that waits for reserved deliver callbacks before entering the opaque shutdown callback. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Document the approach for making the native terminal the source of truth, covering the design decisions and migration steps needed before implementation begins. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Extend TbHostVtable with an optional native_terminal_state callback appended after broker_routing, leaving it unset when normalizing older hosts. Tests cover the frozen prefix and routing tail so the new field cannot be read from a truncated allocation. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The host vtable reader previously copied the native terminal state callback only when the declared size covered the whole struct, so a host that supplied that field without any later tail lost it. Each optional callback is now gated on its own complete-field size check, and the terminal state bits are documented as constants with their admission and monotonicity rules. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Adds documentation for the optional ABI capability tails, covering how optional fields are gated by size, the native query contract and its admission rules, the meaning of PhysicallyStopped, and the lifetime and thread-safety obligations a host must uphold for the query callback and context. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The short-header rejection test now allocates a properly aligned buffer instead of pointing at a stack u32, so the read is not undefined behaviour and the assertion reflects the real ABI check. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add a test that aborts the task awaiting initialization while a deferred initializer is still running, verifying the transport publishes a fault immediately and still runs the late initializer's cleanup exactly once. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Module initialization and teardown now go through transport helpers that record terminal state consistently, so a module that fails to initialize or faults is reported as failed or faulted instead of stopped. The terminal flag update was also rewritten as a compare-exchange loop to avoid losing concurrent fault or stop-intent bits. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add tests that drive the native callback admission path through setup error and unwind while an ordinary stop is in flight, asserting that a genuine setup failure still faults the transport and that the C initializer never entered. The remaining changes are rustfmt reflow and a plan note documenting the Failed versus Canceled cause split. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Rework the native terminal authority plan so task 4 records the accepted module-local lifecycle protocol instead of the earlier proposal text. The document now specifies the public API, request and finish ownership, startup resource transfer, shutdown classification, and the fixed admission cap, and adds the binding panic-payload ordering and catch-time observer corrections that the writer must follow. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add an ignored integration test that verifies the physical owner retains the native controller while the shutdown callback is still borrowing it, and that the fixture owner is reclaimed once the transport is dropped. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Native test fixtures now hand their controller to the transport so the borrowed controller stays owned for as long as the physical transport exists, including uncertain jobs. This removes the deliberate std::mem::forget leaks in the tests and lets them assert the controller is still alive through a weak reference instead. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The combined `if` with a let-chain was split into two nested `if` blocks so the transition logic no longer depends on let-chain support. Behaviour is unchanged. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
…iately The two tests that check a transport is dropped after shutdown asserted reclamation synchronously, which raced with the task's own Arc drop. They now poll the weak count under a timeout so the assertion observes actual reclamation rather than a flag that may precede task exit. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The cleanup state check now records whether cleanup was incomplete before releasing the callback and asserts it only after shutdown completes, so a regression cannot panic while the native callback still borrows the controller. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Co-authored-by: Medulla <medulla@tinyhumans.ai>
On Windows the temp directories used by these host tests inherit a private DACL, which caused the content gate assertions to fail for reasons unrelated to the behaviour under test. Securing the release cache for each directory first lets the tests exercise the intended content checks. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The native callback setup failure race test now waits up to 30 seconds instead of 2 for the attached notification. Admission hashes the real artifact twice before this hook, and instrumented CI builds can spend several seconds there, so the old bound was too tight for a fixture setup wait that is independent of the lifecycle deadline and the race assertions below. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Add a test-only hook that runs after cleanup publication so a test can observe the transport state at that point. The new test confirms that a completed shutdown has already released its native lifecycle marker. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Move the pending activity drop ahead of the state update so a completed stop cannot leave enable/disable reporting that opaque work is still running. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The test host builder now initialises the new native_terminal_state field so the execution context tests keep compiling against the updated host struct. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Tiny Sweeper reviewTiny Sweeper completed its review; deterministic results follow. State: Incomplete Review snapshot
Completeness: Incomplete Features
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_tests.rs, crates/tinybus/Cargo.toml, crates/tinybus/examples/module_controlled_init.rs, crates/tinybus/src/module/abi_tests.rs, crates/tinybus/src/module/host.rs, crates/tinybus/src/module/host_callbacks.rs, crates/tinybus/src/module/host_callbacks_tests.rs, crates/tinybus/src/module/host_tests.rs, crates/tinybus/src/module/transport.rs, crates/tinybus/src/module/transport_tests.rs, docs/modules/module/abi.md, docs/plans/native-terminal-authority.md, scripts/check-coverage.sh, scripts/test-e2e.ps1, scripts/test-e2e.sh, tinysweeper/description Before merge
How this fits togetherflowchart LR
n0["owner<br/>changed"]:::changed
n1["host<br/>changed"]:::changed
n2["..._copied_without_reading_the_additive_tail<br/>changed"]:::changed
n3["host<br/>changed"]:::changed
n4["TbHostVtable<br/>changed"]:::changed
n5["ModuleHost<br/>changed"]:::changed
n6["ModuleHostInner<br/>changed"]:::changed
n7["load_file_with_host_callbacks"]:::impacted
n8["clone"]:::impacted
n9["Err"]:::impacted
n10["abi"]:::impacted
n11["initialize"]:::impacted
n12["initialize_ok"]:::impacted
n0 -->|uses| n4
n0 -->|uses| n10
n1 -->|uses| n4
n1 -->|uses| n10
n2 -->|calls| n3
n2 -->|tests| n3
n2 -->|uses| n4
n2 -->|uses| n10
n3 -->|uses| n4
n3 -->|uses| n10
n5 -->|uses| n6
n7 -->|calls| n8
n7 -->|calls| n9
n11 -->|calls| n9
n12 -->|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
Agent review detailscritique
security
tests
commits
description
Evidence and run details
|
There was a problem hiding this comment.
tinysweeper found nothing blocking, but could not review everything, so this is not an approval: crates/tinybus/src/module/abi_tests.rs, crates/tinybus/src/module/host_callbacks_tests.rs, crates/tinybus/src/module/transport.rs, crates/tinybus/src/module/transport_tests.rs, docs/plans/native-terminal-authority.md.
$0.0176 · 2,759,618 in / 193,350 out · 387,072 cached (14%) · ladder/vectors-oai3, deepseek-v4.1-flash, · 730 embedded
critique: $0.0145 · 1,169,430 in / 169,179 out · 182,016 cached (16%) · deepseek-v4.1-flash,
security: $0.0026 · 986,298 in / 18,529 out · 107,392 cached (11%) · deepseek-v4.1-flash
tests: $0.0002 · 274,536 in / 1,749 out · 91,008 cached (33%) · deepseek-v4.1-flash
description: $0.0001 · 245,564 in / 872 out · 6,656 cached (3%) · deepseek-v4.1-flash
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_tests.rs, crates/tinybus/Cargo.toml, crates/tinybus/examples/module_controlled_init.rs, crates/tinybus/src/module/abi_tests.rs, crates/tinybus/src/module/host.rs, crates/tinybus/src/module/host_callbacks.rs, crates/tinybus/src/module/host_callbacks_tests.rs, crates/tinybus/src/module/host_tests.rs and 8 more.
$0.0090 · 1,556,713 in / 90,649 out · 327,424 cached (21%) · ladder/vectors-oai3, deepseek-v4.1-flash, · 730 embedded
critique: $0.0033 · 354,582 in / 44,902 out · 109,696 cached (31%) · deepseek-v4.1-flash
security: $0.0048 · 859,430 in / 34,615 out · 129,536 cached (15%) · deepseek-v4.1-flash,
tests: $0.0001 · 180,278 in / 656 out · 88,192 cached (49%) · deepseek-v4.1-flash
description: $0.0006 · 78,396 in / 7,450 out · 0 cached (0%) · deepseek-v4.1-flash
This cumulative SDK prerequisite for tinyhumansai/openhuman#7328 is still in progress. Keep it draft until the module-side native terminal factory, once request/finish hooks, startup ownership and their actual native proofs are implemented and independently verified. The Connection catch observer is only a prerequisite for that work.
The feature worktree is integrating the concurrently pushed PR fixes and upstream module execution/lifecycle changes through normal merges, preserving both histories. The earlier independently reviewed checkpoint includes immediate host stop fencing and retained provider drain before physical shutdown; its source/test/coverage receipts belong to that checkpoint and are not substituted for proof of the newly combined source.
Fresh full native, feature, MSRV, lint and coverage results will replace this temporary integration description when the merged checkpoint is published. The 90% file and 80% changed-line gates remain unchanged. TinySecurity's actual App adapter, release and OpenHuman migrations remain dependent on the completed SDK work.