Embedded Support - #80
Open
NellowTCS wants to merge 41 commits into
Open
Conversation
|
Important Review skippedToo many files! This PR contains 269 files, which is 169 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (6)
📒 Files selected for processing (269)
You can disable this status message by setting the 📝 WalkthroughWalkthroughThe pull request adds configurable no_std randomness, bounded core data structures, shared MessagePack and synchronization APIs, Embassy runtime support, unified schema capacity errors, portable registries, and native framed transport streams. ChangesPlatform portability and bounded runtime foundations
Estimated code review effort: 5 (Critical) | ~90 minutes Sequence Diagram(s)sequenceDiagram
participant InvocationId
participant saikuro_random
participant EntropyBackend
participant Drbg
InvocationId->>saikuro_random: uuid_v4()
alt DRBG enabled
saikuro_random->>Drbg: fill UUID bytes
Drbg-->>saikuro_random: deterministic bytes
else platform backend selected
saikuro_random->>EntropyBackend: fill UUID bytes
EntropyBackend-->>saikuro_random: entropy bytes
end
saikuro_random-->>InvocationId: UUID v4
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Member
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Build/adapters/rust/src/schema.rs`:
- Line 87: Update the insertion handling in to_core and build_schema so
FunctionMap and namespace-map capacity errors are propagated instead of
discarded, causing schema registration or announcement to fail rather than
publish partial definitions. Add tests covering both function and namespace
overflow paths.
In `@Build/Cargo.toml`:
- Around line 29-30: Update the workspace dependency configuration for serde and
serde_bytes, using their workspace dependency declarations rather than only the
local manifest: disable default features, enable alloc for both, and enable
derive for serde. Add std explicitly only to workspace members that require it,
while keeping saikuro-core on the no_std-compatible configuration.
In `@Build/crates/saikuro-exec/Cargo.toml`:
- Around line 12-26: Update the embassy-runtime feature declaration to include
the futures/async-await feature alongside its existing dependencies, ensuring
$crate::_futures::select! resolves for the Embassy backend. Leave the other
runtime feature definitions unchanged.
In `@Build/crates/saikuro-exec/src/embassy_backend.rs`:
- Around line 594-623: Update the RwLock documentation to explicitly state that
readers are serialized and a task must not hold one read guard while awaiting
another read guard on the same lock; retain the current single-Mutex
implementation and clarify that this restriction differs from Tokio RwLock
reentrancy.
- Around line 545-567: Update the State::Ready(value) arm in the oneshot
Receiver poll implementation to restore data.channel to State::Closed before
returning Poll::Ready(Ok(value)). Preserve the existing delivery result while
ensuring subsequent polls return Poll::Ready(Err(RecvError)) instead of parking
a waker indefinitely.
- Around line 873-891: Update ChangedFuture::poll to compare
this.receiver.version with state.version before checking state.senders == 0,
returning the pending change when versions differ. Only return Err(RecvError)
after confirming no unread version remains, while preserving Poll::Pending for
an open receiver without a new value.
- Around line 286-295: Store the requested capacity value in ChannelState and
enforce it during enqueue operations. In the channel() constructor, capture the
capacity parameter and store it in ChannelState. Update both the try_send method
and the send method to check the current queue length against the stored
capacity before allowing a message to be enqueued; if the queue length has
reached or exceeded capacity, return the Full error instead of allowing the
enqueue to proceed. This ensures that a channel created with channel(1) respects
the 1-slot limit rather than allowing 256 messages as it currently does.
- Around line 48-65: Update the duration conversion used by both sleep and
timeout to preserve microsecond resolution and saturate at the target integer
range instead of using dur.as_millis() as u64. Apply the corrected conversion
when constructing EmbDuration for Timer::after in the sleep and timeout
functions, preserving their existing completion and timeout behavior.
- Around line 686-723: Update Barrier::wait so the generation value used by the
waiter is captured within the same self.inner.state.lock critical section that
increments arrived and handles barrier release. Preserve the immediate return
for the releasing task, and ensure non-releasing tasks wait against the
pre-arrival generation so a concurrent release cannot be missed.
- Around line 591-592: Update the embassy backend’s `sync::Mutex` re-export to
bind `embassy_sync::mutex::Mutex` to `CriticalSectionRawMutex`, matching the
existing `RwLock` alias and preserving the facade’s single-parameter `Mutex<T>`
signature across backends.
In `@Build/crates/saikuro-exec/src/lib.rs`:
- Around line 59-78: Update the embassy-runtime select_impl! macro so facade
call sites passing unfused futures, including listener.accept(),
forward_rx.recv(), and adapter.recv(), satisfy futures::select! by fusing each
branch before delegation. Preserve the existing tokio/wasm macro behavior and
the public saikuro_exec::select! call-site contract.
In `@Build/crates/saikuro-schema/src/registry.rs`:
- Around line 212-219: Update SchemaRegistry::snapshot so failed inserts into
schema.namespace and schema.types are propagated as a capacity error instead of
discarding the Result with .ok(). Preserve successful entries, and update every
snapshot caller to handle the returned error explicitly.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 38f43176-8848-49b1-9d32-0ee1c2f89f69
⛔ Files ignored due to path filters (1)
Build/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (32)
Build/Cargo.tomlBuild/adapters/c/tests/c_api_runtime.rsBuild/adapters/c/tests/cpp_wrapper_runtime.rsBuild/adapters/rust/src/schema.rsBuild/crates/saikuro-codegen/src/generator.rsBuild/crates/saikuro-codegen/tests/c_cpp_codegen.rsBuild/crates/saikuro-core/Cargo.tomlBuild/crates/saikuro-core/src/capability.rsBuild/crates/saikuro-core/src/envelope.rsBuild/crates/saikuro-core/src/error.rsBuild/crates/saikuro-core/src/invocation.rsBuild/crates/saikuro-core/src/lib.rsBuild/crates/saikuro-core/src/log.rsBuild/crates/saikuro-core/src/resource.rsBuild/crates/saikuro-core/src/schema.rsBuild/crates/saikuro-core/src/value.rsBuild/crates/saikuro-exec/Cargo.tomlBuild/crates/saikuro-exec/src/embassy_backend.rsBuild/crates/saikuro-exec/src/lib.rsBuild/crates/saikuro-random/src/drbg.rsBuild/crates/saikuro-runtime/src/connection.rsBuild/crates/saikuro-runtime/src/lib.rsBuild/crates/saikuro-schema/src/registry.rsBuild/tests/tests/capability_enforcement.rsBuild/tests/tests/codegen_output.rsBuild/tests/tests/common/mod.rsBuild/tests/tests/cross_language_wire.rsBuild/tests/tests/envelope_roundtrip.rsBuild/tests/tests/error_propagation.rsBuild/tests/tests/resource_dispatch.rsBuild/tests/tests/sandbox_dispatch.rsBuild/tests/tests/schema_validation.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- Build/crates/saikuro-core/src/invocation.rs
- Build/crates/saikuro-random/src/drbg.rs
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Build/crates/saikuro-core/src/envelope.rs (1)
100-100: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftPreserve canonical metadata ordering before serialising envelopes.
Envelope::metais nowheapless::FnvIndexMap, whose Serde iteration order follows insertion order, while the previous unbounded map type iterated keys in order.to_msgpack()/from_msgpack()use thismetafield through Serde, so equivalent metadata inserted in different orders can produce different MessagePack bytes.Make metadata serde output key-order canonical, or use a bounded map type that serialises in a fixed order.
Also applies to lines 123-134 via
to_msgpack()/from_msgpack().🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Build/crates/saikuro-core/src/envelope.rs` at line 100, Update Envelope metadata serialization around the meta field and the to_msgpack()/from_msgpack() methods so equivalent metadata always produces canonical key-ordered MessagePack bytes regardless of insertion order. Use a bounded map or custom serde serialization that preserves the existing deterministic key ordering while retaining the current metadata behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Build/crates/saikuro-core/Cargo.toml`:
- Around line 15-20: Add a direct compile-time feature conflict guard in
saikuro-random covering incompatible combinations of os, custom, or wasm with
drbg, so enabling saikuro-core/drbg cannot coexist with the forwarded os
backend. Place the guard in saikuro-random’s crate-level initialization and
preserve valid single-backend configurations.
In `@Build/crates/saikuro-core/src/sync.rs`:
- Around line 14-16: Replace each affected poisoned-lock recovery call using
unwrap_or_else(|poison| poison.into_inner()) with expect(...) so poisoning
immediately panics as documented. Update the lock acquisition call sites in
sync.rs while preserving normal guard access when the lock is not poisoned.
In `@Build/crates/saikuro-router/src/provider.rs`:
- Around line 122-131: Update ProviderRegistry to guard by_namespace and
by_provider with a single shared lock, then revise register replacement logic to
remove the namespace from the old provider’s record before adding it to the new
one. In deregister, remove each namespace from by_namespace only when its
current ProviderHandle still has the deregistering provider_id, keeping both
indexes consistent atomically.
---
Outside diff comments:
In `@Build/crates/saikuro-core/src/envelope.rs`:
- Line 100: Update Envelope metadata serialization around the meta field and the
to_msgpack()/from_msgpack() methods so equivalent metadata always produces
canonical key-ordered MessagePack bytes regardless of insertion order. Use a
bounded map or custom serde serialization that preserves the existing
deterministic key ordering while retaining the current metadata behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7abf91c6-461c-4d5c-b041-661475d22c23
⛔ Files ignored due to path filters (1)
Build/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (32)
.cargo/config.tomlBuild/Cargo.tomlBuild/crates/saikuro-core/Cargo.tomlBuild/crates/saikuro-core/src/envelope.rsBuild/crates/saikuro-core/src/error.rsBuild/crates/saikuro-core/src/invocation.rsBuild/crates/saikuro-core/src/lib.rsBuild/crates/saikuro-core/src/msgpack.rsBuild/crates/saikuro-core/src/sync.rsBuild/crates/saikuro-core/src/value.rsBuild/crates/saikuro-exec/src/embassy_backend.rsBuild/crates/saikuro-random/src/drbg.rsBuild/crates/saikuro-random/src/lib.rsBuild/crates/saikuro-router/Cargo.tomlBuild/crates/saikuro-router/src/error.rsBuild/crates/saikuro-router/src/lib.rsBuild/crates/saikuro-router/src/provider.rsBuild/crates/saikuro-router/src/router.rsBuild/crates/saikuro-router/src/stream_state.rsBuild/crates/saikuro-runtime/Cargo.tomlBuild/crates/saikuro-runtime/src/connection.rsBuild/crates/saikuro-schema/Cargo.tomlBuild/crates/saikuro-schema/src/capability_engine.rsBuild/crates/saikuro-schema/src/lib.rsBuild/crates/saikuro-schema/src/registry.rsBuild/crates/saikuro-schema/src/validator.rsBuild/crates/saikuro-storage/Cargo.tomlBuild/crates/saikuro-storage/src/error.rsBuild/crates/saikuro-storage/src/traits.rsBuild/crates/saikuro-transport/Cargo.tomlBuild/crates/saikuro-transport/src/error.rsBuild/tests/Cargo.toml
💤 Files with no reviewable changes (3)
- Build/crates/saikuro-storage/Cargo.toml
- Build/crates/saikuro-transport/Cargo.toml
- Build/crates/saikuro-runtime/Cargo.toml
🚧 Files skipped from review as they are similar to previous changes (8)
- .cargo/config.toml
- Build/crates/saikuro-core/src/invocation.rs
- Build/crates/saikuro-random/src/drbg.rs
- Build/crates/saikuro-random/src/lib.rs
- Build/tests/Cargo.toml
- Build/crates/saikuro-core/src/value.rs
- Build/crates/saikuro-core/src/error.rs
- Build/crates/saikuro-exec/src/embassy_backend.rs
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Build/crates/saikuro-exec/src/embassy_backend.rs (1)
478-485: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winReject zero-capacity channels.
Line 485 stores zero as the effective limit. Lines 321 and 335 then classify the empty queue as full forever.
try_sendalways returnsFull, andsendremains pending even when a receiver awaitsrecv(). Reject zero capacity or implement rendezvous behavior.Proposed fix
pub fn channel<T>(capacity: usize) -> (Sender<T>, Receiver<T>) { + assert!( + capacity > 0, + "saikuro-exec: mpsc capacity must be greater than zero" + ); assert!( capacity <= CHANNEL_CAPACITY, "saikuro-exec: mpsc capacity {capacity} exceeds the fixed \🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Build/crates/saikuro-exec/src/embassy_backend.rs` around lines 478 - 485, Update the channel function to reject capacity == 0 before constructing ChannelInner, while preserving the existing upper-bound validation and nonzero channel behavior. Ensure the assertion clearly reports that zero capacity is unsupported.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Build/adapters/rust/src/provider.rs`:
- Around line 206-214: Update announce to return Result<()> and propagate the
build_schema failure instead of logging and returning successfully. In serve_on,
handle the announce result and return the error before entering the serve loop,
while preserving successful announcement behavior.
In `@Build/crates/saikuro-core/src/sync.rs`:
- Around line 14-16: Update the module-level documentation in sync.rs to
describe lock poisoning by backend: std::sync::Mutex and RwLock write guards
become poisoned after a panic, std::sync::RwLockReadGuard does not, and spin
no_std guards expose no poison state. Remove the blanket claim that every
poisoned lock causes the next acquisition to panic.
In `@Build/crates/saikuro-core/tests/invocation.rs`:
- Around line 5-9: Update msgpack_roundtrip_uses_binary_uuid to inspect the
encoded MessagePack representation in addition to the round-trip assertion.
Verify that the UUID is encoded as a binary payload with exactly 16 bytes,
rather than only relying on decoding agreement.
In `@Build/crates/saikuro-core/tests/value.rs`:
- Around line 41-50: The round-trip test only verifies partial schema content
and the byte variant, allowing data loss or byte changes. Strengthen the
assertions around the schema and byte test values to compare them with their
original values; if Schema or the relevant type lacks PartialEq, assert every
field and exact byte contents individually.
In `@Build/crates/saikuro-exec/src/lib.rs`:
- Around line 67-90: Document the restricted syntax exposed by the Embassy
select_impl! macro: branches must use pattern = future => { block } and do not
support guards, else, biased;, or expression handlers. Either update the macro
to support the full Tokio select! syntax or clearly state this contract in the
public saikuro_exec::select! documentation, preserving portability expectations
for cross-runtime callers.
In `@Build/crates/saikuro-router/src/provider.rs`:
- Around line 151-171: Update ProviderRegistry::register to remove namespaces
from the provider’s existing by_provider entry when they are absent from the new
namespaces list, deleting each corresponding by_namespace route only if it still
maps to provider_id. Perform this cleanup before overwriting state.by_provider,
and add a regression test covering re-registering one provider with fewer
namespaces and verifying omitted routes are removed.
---
Outside diff comments:
In `@Build/crates/saikuro-exec/src/embassy_backend.rs`:
- Around line 478-485: Update the channel function to reject capacity == 0
before constructing ChannelInner, while preserving the existing upper-bound
validation and nonzero channel behavior. Ensure the assertion clearly reports
that zero capacity is unsupported.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 9eab3bbc-0ce2-412e-b251-10f6965a4a09
📒 Files selected for processing (30)
Build/Cargo.tomlBuild/adapters/rust/src/error.rsBuild/adapters/rust/src/provider.rsBuild/adapters/rust/src/schema.rsBuild/adapters/rust/tests/integration.rsBuild/adapters/rust/tests/schema_capacity.rsBuild/crates/saikuro-core/Cargo.tomlBuild/crates/saikuro-core/src/envelope.rsBuild/crates/saikuro-core/src/invocation.rsBuild/crates/saikuro-core/src/resource.rsBuild/crates/saikuro-core/src/sync.rsBuild/crates/saikuro-core/src/value.rsBuild/crates/saikuro-core/tests/invocation.rsBuild/crates/saikuro-core/tests/resource.rsBuild/crates/saikuro-core/tests/value.rsBuild/crates/saikuro-exec/Cargo.tomlBuild/crates/saikuro-exec/src/embassy_backend.rsBuild/crates/saikuro-exec/src/lib.rsBuild/crates/saikuro-random/src/lib.rsBuild/crates/saikuro-router/src/provider.rsBuild/crates/saikuro-runtime/src/connection.rsBuild/crates/saikuro-runtime/src/handle.rsBuild/crates/saikuro-runtime/src/lib.rsBuild/crates/saikuro-runtime/tests/schema_registration.rsBuild/crates/saikuro-schema/src/registry.rsBuild/crates/saikuro-schema/src/validator.rsBuild/crates/saikuro-schema/tests/validator.rsBuild/crates/saikuro-storage/src/util.rsBuild/crates/saikuro-storage/tests/util.rsBuild/tests/tests/envelope_roundtrip.rs
💤 Files with no reviewable changes (4)
- Build/crates/saikuro-runtime/src/lib.rs
- Build/crates/saikuro-core/src/resource.rs
- Build/crates/saikuro-core/src/invocation.rs
- Build/crates/saikuro-core/src/value.rs
🚧 Files skipped from review as they are similar to previous changes (6)
- Build/crates/saikuro-core/Cargo.toml
- Build/tests/tests/envelope_roundtrip.rs
- Build/crates/saikuro-exec/Cargo.toml
- Build/crates/saikuro-runtime/src/connection.rs
- Build/Cargo.toml
- Build/crates/saikuro-random/src/lib.rs
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
Build/crates/saikuro-exec/src/tokio_backend.rs (1)
37-39: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrefer an explicit re-export list over
tokio::io::*.The glob makes the whole Tokio I/O surface part of the
saikuro-execpublic API. Two consequences follow. A Tokio minor release can add items and change what downstream code resolves. The Embassy backend cannot supply a matchingiomodule, so the abstraction does not hold across backends.Re-export only the items the workspace uses:
AsyncRead,AsyncWrite,AsyncReadExt,AsyncWriteExt,ReadBuf,split, andduplex.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Build/crates/saikuro-exec/src/tokio_backend.rs` around lines 37 - 39, Replace the wildcard re-export in the `io` module with an explicit list containing only `AsyncRead`, `AsyncWrite`, `AsyncReadExt`, `AsyncWriteExt`, `ReadBuf`, `split`, and `duplex`, keeping the public I/O abstraction limited to these workspace-used symbols.Build/crates/saikuro-transport/src/framing.rs (1)
180-186: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winEvery read copies through a 4 KiB stack buffer.
For a 300 KB frame this performs about 75
poll_readcalls plus 75 copies intoread_buf. The comment explains why you avoid reading into a zero-filledread_buf, butBytesMut::chunk_mutwithReadBuf::uninitgives the same safety without the intermediate copy, and the read size can track the pending frame length.This is a hot path for large payloads. Consider sizing the read against
read_buf.capacity()afterdecodereserves the remaining frame bytes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Build/crates/saikuro-transport/src/framing.rs` around lines 180 - 186, Update the framing read path around the poll_read call to read directly into the writable uninitialized region of this.read_buf using BytesMut::chunk_mut and ReadBuf::uninit, eliminating the 4 KiB stack buffer and copy through read_buf. Size the writable region to the pending frame capacity after decode reserves the remaining bytes, then advance this.read_buf by the number of bytes read while preserving the existing TransportError handling and EOF behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Build/crates/saikuro-transport/Cargo.toml`:
- Line 33: Update the feature definitions around saikuro-core and
saikuro-transport’s wasm-runtime so WASM uses a new core standard-library
convenience feature that excludes saikuro-random/os. Add that core feature,
preserve existing non-WASM std behavior, and replace the wasm-runtime dependency
on saikuro-core/std with the new feature while retaining saikuro-random/wasm.
- Around line 26-29: Update the WASM-target dependency declaration for
saikuro-transport to set default-features = false while retaining the explicit
wasm-runtime feature, preventing the default native transport and Tokio runtime
features from being enabled.
In `@Build/crates/saikuro-transport/src/framing.rs`:
- Around line 56-64: The oversized-frame path in decode leaves rejected payload
bytes to be interpreted as a new header. Update the framing state around
pending_len and the MessageTooLarge error so subsequent input is discarded until
the declared frame payload is consumed, or explicitly document and enforce that
callers must clear the buffer before retrying; preserve normal decoding for
valid frames and prevent arbitrary resynchronization.
- Around line 163-201: Make the framing stream terminal after any decoding,
read, or truncated-frame error by adding a failed state to the stream type and
checking it at the start of poll_next. Set this state before returning
Some(Err(...)) in each error path, and return Poll::Ready(None) on subsequent
polls; preserve normal frame delivery and clean EOF behavior.
- Around line 147-154: Enable the futures sink feature required by
FramedStream::split by updating the workspace futures dependency to include both
sink and alloc, or by adding sink to the saikuro-transport futures dependency
while preserving existing feature settings. Do not change the StreamExt::split
turbofish or pin_project usage.
In `@Build/tests/tests/transport_framing.rs`:
- Around line 72-89: Update codec_rejects_oversized_frame_then_recovers so wire
retains trailing payload bytes after the forged oversized header before
appending the valid frame, ensuring decode actually resynchronizes on buffered
data. Add a separate test covering FramedStream polling after it yields a
framing error, and assert the defined post-error behavior on the subsequent
poll.
---
Nitpick comments:
In `@Build/crates/saikuro-exec/src/tokio_backend.rs`:
- Around line 37-39: Replace the wildcard re-export in the `io` module with an
explicit list containing only `AsyncRead`, `AsyncWrite`, `AsyncReadExt`,
`AsyncWriteExt`, `ReadBuf`, `split`, and `duplex`, keeping the public I/O
abstraction limited to these workspace-used symbols.
In `@Build/crates/saikuro-transport/src/framing.rs`:
- Around line 180-186: Update the framing read path around the poll_read call to
read directly into the writable uninitialized region of this.read_buf using
BytesMut::chunk_mut and ReadBuf::uninit, eliminating the 4 KiB stack buffer and
copy through read_buf. Size the writable region to the pending frame capacity
after decode reserves the remaining bytes, then advance this.read_buf by the
number of bytes read while preserving the existing TransportError handling and
EOF behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 78077156-a3d6-42d0-b98e-703082552247
⛔ Files ignored due to path filters (5)
Build/Cargo.lockis excluded by!**/*.lockDemo/wasm/c/Cargo.lockis excluded by!**/*.lockDemo/wasm/cpp/Cargo.lockis excluded by!**/*.lockDemo/wasm/runtime/Cargo.lockis excluded by!**/*.lockDemo/wasm/rust/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (16)
Build/Cargo.tomlBuild/crates/saikuro-exec/src/tokio_backend.rsBuild/crates/saikuro-router/Cargo.tomlBuild/crates/saikuro-schema/Cargo.tomlBuild/crates/saikuro-transport/Cargo.tomlBuild/crates/saikuro-transport/src/error.rsBuild/crates/saikuro-transport/src/framing.rsBuild/crates/saikuro-transport/src/lib.rsBuild/crates/saikuro-transport/src/memory.rsBuild/crates/saikuro-transport/src/selector.rsBuild/crates/saikuro-transport/src/tcp.rsBuild/crates/saikuro-transport/src/traits.rsBuild/crates/saikuro-transport/src/unix.rsBuild/tests/Cargo.tomlBuild/tests/tests/transport_framing.rsBuild/tests/tests/transport_memory_stress.rs
🚧 Files skipped from review as they are similar to previous changes (5)
- Build/tests/Cargo.toml
- Build/crates/saikuro-schema/Cargo.toml
- Build/crates/saikuro-router/Cargo.toml
- Build/Cargo.toml
- Build/crates/saikuro-transport/src/error.rs
serves me right for writing bad code before :/
| seed[i * 8..i * 8 + 8].copy_from_slice(&word.load(Ordering::Acquire).to_ne_bytes()); | ||
| } | ||
| let mut key = [0u8; KEY_LEN]; | ||
| let mut nonce = [0u8; NONCE_LEN]; |
i mean we moved it but we need to use a thing for this Askama
| return Err(SaikuroError::Entropy(format!("DRBG seed must be at least {SEED_LEN} bytes"))); | ||
| } | ||
| let mut key = [0u8; KEY_LEN]; | ||
| let mut nonce = [0u8; NONCE_LEN]; |
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
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.
Summary by CodeRabbit
New Features
Bug Fixes
Tests