feat(decider): compile deciders to WASM/WIT components#384
Open
yordis wants to merge 38 commits into
Open
Conversation
yordis
commented
Jun 22, 2026
yordis
commented
Member
- Decider purity is currently convention, not a guarantee — nothing prevents side effects such as reading the clock or generating ids, so a decider's logic cannot be fully trusted or safely replayed.
- Compiling deciders to zero-import WASM/WIT components makes purity enforceable by capability denial rather than by code review.
- The same compiled module can then run in any sandbox (server, browser, AI agent), and the platform — not each author — can own the event-store and infrastructure wiring.
PR SummaryMedium Risk Overview Testing and CI: Tooling: mise adds Event codec traits move from Reviewed by Cursor Bugbot for commit c017fde. Bugbot is set up for automated code reviews on this repo. Configure here. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (9)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (5)
WalkthroughAdds the WASM decider contract, guest and host bindings, scheduler domain extraction, simulator, CLI conformance runner, WASM component, and CI/build updates. ChangesWASM decider pipeline
Sequence Diagram(s)sequenceDiagram
participant deciderTest as decider-test CLI
participant simHost as SimHost
participant simScenario as SimScenario
participant guestWasm as Schedules WASM
deciderTest->>simHost: load(bytes)
deciderTest->>simHost: instantiate(host_state)
deciderTest->>simScenario: run(instance)
simScenario->>guestWasm: evolve(given events)
simScenario->>guestWasm: decide(when command)
guestWasm-->>simScenario: events / rejection / fault
simScenario-->>deciderTest: ScenarioError or success
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 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 |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
yordis
force-pushed
the
yordis/decider-wasm-wit-support
branch
from
114a0bf to
cef7f41
Compare
yordis
force-pushed
the
yordis/decider-wasm-wit-support
branch
from
cef7f41 to
858d556
Compare
yordis
force-pushed
the
yordis/decider-wasm-wit-support
branch
from
858d556 to
7a9f840
Compare
Decider purity is currently convention, not a guarantee: nothing stops a decider from performing side effects such as reading the clock or generating ids, so its logic cannot be fully trusted or replayed safely, cannot run in arbitrary sandboxes, and forces every author to own infrastructure. Compiling deciders to zero-import WASM components makes purity enforceable by capability denial, lets the same module run anywhere (server, browser, AI), and moves toward a platform that owns the event-store wiring. Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
yordis
force-pushed
the
yordis/decider-wasm-wit-support
branch
from
7a9f840 to
b552b13
Compare
…it-support Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com> # Conflicts: # rsworkspace/Cargo.lock # rsworkspace/crates/trogon-scheduler/Cargo.toml
… networks - The wasm32/native feature-build guards spawn nested `cargo check`; under `cargo llvm-cov` the injected RUSTC_WRAPPER applied `-C instrument-coverage` to the nested build, which fails for `wasm32-unknown-unknown` (no `profiler_builtins`) and no longer reflects real target behavior. Strip the coverage wrapper env so the guards exercise the actual build. - Raise `CARGO_NET_RETRY` so source-built mise tools (dylint-link, ...) tolerate transient crates.io network errors during install. Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
The wasm-tools import scan only inspected lines inside a `world root {` block,
so imports listed under a differently named world (the contract defines `world
decider`) or outside that block slipped through, diverging from the CI guard's
full-file import grep and letting a non-zero-import component pass.
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
Scenarios asserting a command is accepted (`rejected: false`) were routed through `then_rejected()`, so the runner demanded a business rejection and the boolean was effectively ignored. Add `SimScenario::then_accepted` and map the flag to it so an accepted outcome is asserted. Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
The compile-fail case for bundling commands with mismatched `Event` types only recorded the relative-WIT-path read failure trybuild hits in its copied crate tree; that fatal error masked the shared-event bound, so the guard asserted nothing. Provision the contract WIT where trybuild resolves it and add a WIT-independent shared-state/event assertion in the macro, so the mismatch is surfaced and asserted regardless of the WIT bindings. Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
…it-support Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com> # Conflicts: # rsworkspace/Cargo.toml # rsworkspace/crates/trogon-scheduler/Cargo.toml
|
Code Coverage SummaryDetailsDiff against mainResults for commit: c017fde Minimum allowed coverage is ♻️ This comment has been updated with latest results |
…tions - A corrupt or schema-mismatched snapshot frame was treated like "no snapshot" and silently initialized empty state; the WIT session constructor has no error channel, so a host applying only a post-snapshot event suffix would `decide` on incomplete state believing the snapshot loaded. Trap on a failed load so the host observes the failure instead. - The YAML conformance runner ignored `then.error`, always asserting a generic rejection, so suites could not validate specific fault/rejection outcomes. Add `SimScenario::then_error` and compare the expected value against the outcome's code or message. Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
… gate The guest SDK, `export_decider!` output, decider cdylib crates, the light proto example, and the conformance CLI execute inside the wasm32 component and are verified by the dedicated `wasm-decider` job (zero-import assertion plus sim and YAML conformance suites). Host llvm-cov cannot instrument code that runs in the guest, so exclude it from the host coverage gate, mirroring how generated proto is already ignored. Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
yordis
added
the
rust:coverage-baseline-reset
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
…host gate Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
…it-support Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com> # Conflicts: # rsworkspace/Cargo.lock
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
v1 has shipped; completed design and implementation history now lives in git, so the working notes stay focused on what is left to do. Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
Declaring the target in rust-toolchain.toml made every concurrent cargo install during mise install lazily fetch the wasm32 rust-std component; the parallel rustup downloads corrupted each other and failed setup for every CI job. Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
🧹 Nitpick comments (7)
.github/actions/setup-rust/action.yml (1)
21-29: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winMake the wasm target install opt-in.
This composite action is also used by the non-WASM
buildjob, so every run now pays an extrarustup target addnetwork hop and failure mode even when no wasm build follows. A boolean input would keep the serial-install workaround while limiting it to thetestandwasm-deciderjobs.🤖 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 @.github/actions/setup-rust/action.yml around lines 21 - 29, Make the wasm32 target installation optional in the setup-rust composite action so the non-WASM build path does not always incur the extra rustup network call. Add a boolean input to the action and gate the existing Install wasm32 target step in action.yml on that input, while keeping the serial rustup target add workaround for the jobs that actually need it. Update the callers for the test and wasm-decider jobs to enable it, and leave the build job disabled.rsworkspace/crates/trogon-decider-wit/tests/feature_build.rs (1)
53-67: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the expected wasm32 failure reason.
assert!(!output.status.success())also passes on unrelated breakages. Matchstderragainst the intentional unsupported-target diagnostic so this test only guards the host/wasm32 contract.🤖 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 `@rsworkspace/crates/trogon-decider-wit/tests/feature_build.rs` around lines 53 - 67, The host_feature_fails_on_wasm32 test only checks that cargo check fails, which can pass for unrelated errors; tighten it by asserting the stderr from cargo_check().output() contains the expected unsupported-target diagnostic for the host feature on wasm32. Update the existing host_feature_fails_on_wasm32 test to match the intentional failure reason so it specifically verifies the host/wasm32 contract instead of any build breakage.rsworkspace/crates/trogon-decider-sim/src/import_check.rs (1)
33-42: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPreserve source errors instead of flattening them to
String.These
map_err(|err| ... err.to_string())conversions discard the originalio::Error/ parser error chain, which makes failures harder to diagnose and violates the repo’s typed-error rule. Keep the source errors in dedicated variants and attach them with#[source]instead of collapsing them into text. As per coding guidelines, "Never discard error context by converting a typed error into a string; wrap the source error as a field or variant instead."Also applies to: 45-47, 70-74
🤖 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 `@rsworkspace/crates/trogon-decider-sim/src/import_check.rs` around lines 33 - 42, The error handling in ImportCheckError is flattening source failures into String, so update the error enum and the affected paths in import_check.rs to preserve typed sources instead of calling err.to_string(). Add dedicated variants with #[source] for the parser and io/command failures, then adjust the map_err calls around temp.write_all, Command::new(...).output(), and the other referenced sites to wrap the original errors directly while keeping ImportCheckError as the public wrapper.Source: Coding guidelines
rsworkspace/crates/trogon-decider-sim/src/import_check/tests.rs (1)
3-15: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a regression for the component path too.
This only exercises the raw-module fallback. The bug-prone branch here is the component flow behind
wasm-tools component wit, so a test that forcesWASM_TOOLSto an invalid executable against a component fixture would better lock in the zero-import guarantee.🤖 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 `@rsworkspace/crates/trogon-decider-sim/src/import_check/tests.rs` around lines 3 - 15, The current test only covers the raw-module fallback in `rejects_core_module_with_imports`, so add a regression that exercises the component path used by `assert_zero_imports` when `WASM_TOOLS` is unavailable or invalid. Create a component-based fixture and force `WASM_TOOLS` to point to a bogus executable so the code path behind `wasm-tools component wit` is exercised, then assert the zero-import check still fails for imports in the component flow. Use the existing `assert_zero_imports` helper and the new component test to lock in this behavior.rsworkspace/crates/trogon-decider-sim/src/scenario/tests.rs (1)
10-25: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd one test for the known-event semantic comparison path.
These cases only cover type mismatch and the raw-byte fallback. The new branch in
events_match()is thedecode_event_to_json(...)path for known event types, and that behavior is still untested here.🤖 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 `@rsworkspace/crates/trogon-decider-sim/src/scenario/tests.rs` around lines 10 - 25, Add a test that exercises the known-event semantic comparison branch in events_match(). The current tests only cover type mismatch and the raw-byte fallback; create a case using a recognized event type so events_match() goes through decode_event_to_json(...) and compares semantic JSON rather than bytes. Use a test name that clearly reflects known-event semantic matching and keep it alongside the existing tests in this scenario module.rsworkspace/crates/trogon-decider-guest-macros/src/lib.rs (1)
115-128: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the redundant
GuestSessionbundle bounds.
bundle_assertalready gives you the direct bundle-mismatch failure. Keeping the same equality constraints on the generatedimpl GuestSession for Sessionis what produces the longwit_bindgen-driven error cascade intests/ui/bundle_mismatched_event.stderr, which makes the UI snapshots much more brittle than necessary.Also applies to: 310-310
🤖 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 `@rsworkspace/crates/trogon-decider-guest-macros/src/lib.rs` around lines 115 - 128, Remove the redundant bundle equality constraints from the generated GuestSession impl in trogon-decider-guest-macros::lib.rs by updating the bundle_bounds logic used for the `impl GuestSession for Session` generation. Keep `bundle_assert` as the place that enforces bundle compatibility, and stop emitting the extra `where` bounds tying each command to the canonical decider State/Event types so the `wit_bindgen` error cascade no longer appears.rsworkspace/crates/trogon-decider-guest-macros/tests/ui/not_a_decider.stderr (1)
1-488: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCollapse this UI failure to one macro diagnostic.
This snapshot now bakes in a long cascade of secondary
E0277s from the generatedwit_bindgenlayer. That is noisy for users and brittle fortrybuildwhenever rustc or the generated bindings shift. Emitting an early trait assertion orcompile_error!for non-Deciderinputs would make both the macro UX and this snapshot much more stable.🤖 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 `@rsworkspace/crates/trogon-decider-guest-macros/tests/ui/not_a_decider.stderr` around lines 1 - 488, The `export_decider` UI snapshot is too noisy because it captures a cascade of secondary `E0277` errors from `wit_bindgen` instead of one clear macro failure. Update the macro path around `export_decider` and the generated `Session`/cabi helpers to perform an early trait check for the input type (`NotADecider` / `Decider`) and emit a single `compile_error!` or dedicated diagnostic before any `wit_bindgen::generate` expansion. Then refresh the `not_a_decider.stderr` expectation to match only that one primary diagnostic.
🤖 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 `@rsworkspace/cli/trogon-decider-test/src/codec.rs`:
- Around line 187-224: The codec helpers parse only the subject and content
fields and currently drop delivery ttl/source and message headers, so update
parse_delivery and parse_message to preserve those wire fields or reject inputs
that include them. In parse_delivery and parse_message, use the existing
schedules_v1::Delivery, schedules_v1::Message, MessageField, and
content_v1alpha1::Content types to either map ttl/source/headers through from
the serde_json value or return an error when unsupported fields are present,
instead of silently defaulting them away.
- Around line 166-177: The `codec.rs` duration parsing in the `every` schedule
branch currently casts `nanos` with `as i32`, which can silently wrap invalid
YAML values; update the `every`/`Duration` construction to validate the parsed
nanos range first and return an error for protobuf-invalid values instead of
truncating them. Use the existing `context("every schedule missing seconds")`
flow as a guide and keep the fix localized to the `value.get("every")` handling
that builds `schedules_v1::Schedule` and
`buffa_types::google::protobuf::Duration`.
In `@rsworkspace/cli/trogon-decider-test/src/main.rs`:
- Around line 78-92: The scenario loop is reusing a single SimInstance across
the whole suite, so guest-global state can leak between runs. Move the
host.instantiate(()) call inside the for scenario loop (or otherwise create a
fresh component per scenario) in the test flow around run_scenario(), so each
scenario starts from a clean instance while keeping the existing
descriptor/exercised checks intact.
In `@rsworkspace/crates/trogon-decider-guest-macros/tests/compile_fail.rs`:
- Around line 20-27: The trybuild WIT staging path in provision_trybuild_wit is
hard-coded to the default target tree and will break when CARGO_TARGET_DIR is
customized. Update the path construction to derive the trybuild destination from
Cargo’s actual target directory instead of joining a fixed "../../target/..."
path, while keeping the existing manifest-based source lookup and
fs::create_dir_all/fs::copy flow.
In `@rsworkspace/crates/trogon-decider-guest-sdk/src/bridge.rs`:
- Around line 13-16: The bridge error contract is stringly-typed and loses the
original error source chain; update DomainErrorParts and the related conversion
paths in bridge.rs to use a typed bridge error enum/struct that implements
Display and std::error::Error, with variants carrying the source error instead
of flattening it into strings. Refactor the repeated source.to_string()
conversions in the affected bridge helpers to preserve typed errors internally,
and only serialize into wire-facing fields at the final WIT boundary.
In `@rsworkspace/crates/trogon-decider-guest-sdk/src/snapshot.rs`:
- Around line 43-57: `decode_snapshot` and `snapshot_fault` are converting
snapshot failures into `DomainErrorParts` too early and stringify the underlying
decode error, which drops typed context. Introduce a local `SnapshotDecodeError`
enum in the snapshot layer, return that from
`decode_snapshot`/`decode_snapshot_frame` paths, and keep the source decode
error as a typed field or variant instead of `source.to_string()`. Then perform
the single conversion to `DomainErrorParts` only at the WIT bridge/export
boundary where the guest SDK crosses out of the typed layer.
- Around line 72-90: Reject malformed snapshot fields in decode_snapshot_frame
by validating that tags 1 and 2 have the expected length-delimited wire type
before calling read_length_delimited, and return a snapshot_fault on mismatch
instead of continuing. Also harden skip_field so its fixed32/fixed64 branches
verify enough bytes remain before advancing offset, failing closed on truncated
input. Keep the existing frame parsing flow in snapshot.rs consistent with
load_or_initial’s trap-on-bad-snapshot behavior.
In `@rsworkspace/crates/trogon-decider-sim/src/host.rs`:
- Around line 8-15: Keep Wasmtime failures typed in SimError and separate
compile from instantiate errors. In host.rs, change the Engine variant so it
stores the original typed Wasmtime/engine error instead of a String, and update
the call site that currently stringifies the failure to pass the error through
directly. Add a distinct variant for instantiation failures in SimError, and
update the code around the compile/instantiate flow so compile-time errors map
to the existing Compile path while post-compilation instantiation errors use the
new variant rather than being reported as Compile.
In `@rsworkspace/crates/trogon-decider-sim/src/import_check.rs`:
- Around line 22-27: The import validation in assert_zero_imports should not
fall back to assert_zero_imports_via_wasmparser after a wasm-tools failure for
component inputs, because that can incorrectly return Ok(()) and bypass the
zero-import check. Update assert_zero_imports to distinguish component binaries
from core wasm modules and, when the wasm-tools path fails on a component,
return the failure instead of silently retrying with the raw-module parser. Keep
the existing success path and ensure SimHost::load still enforces the
zero-import contract at load time.
In `@rsworkspace/crates/trogon-decider-sim/src/scenario.rs`:
- Around line 74-142: The `Scenario::run` API is flattening Wasmtime, domain,
and assertion failures into `String`, which loses structured context. Introduce
a typed scenario error enum/struct for `run` and replace the `Result<(),
String>` plus `map_err(|err| err.to_string())`/`format!()` paths with variants
that wrap the source errors from `open_session`, `evolve`, `decide`, and the
scenario expectation checks. Keep `Scenario::run`, `SimInstance::open_session`,
and the `events_match`/expectation branches as the main touchpoints, and
preserve original error context as fields or source errors instead of converting
to text.
In `@rsworkspace/crates/trogon-scheduler-domain/src/commands/domain/schedule.rs`:
- Around line 423-430: TzdbVersion::current() is minting an invalid sentinel
value when schedule-validation is disabled, which makes timezone.version vary by
build feature and bypasses the type’s validation. Update the current() factory
in TzdbVersion so it only returns a real, validated tzdb version (or otherwise
removes the fallback from the domain type), and keep any unknown guest/version
uncertainty outside of TzdbVersion. Ensure the constructor/factory path used by
current() preserves the type’s invariant and does not serialize "wasm-guest"
into persisted state.
- Around line 381-384: The TimeZone factory in with_tzdb_version currently only
validates the token shape, which allows nonexistent zones to be constructed when
schedule-validation is disabled. Make the constructor always enforce semantic
timezone validity by ensuring the chrono_tz::Tz parse (or equivalent lookup)
runs unconditionally, and keep returning TimeZoneError::Invalid for unknown
zones so invalid TimeZone values remain unrepresentable.
- Around line 340-347: The domain factory in schedule.rs must always validate
RRuleExpression, but the current #[cfg(feature = "schedule-validation")] guard
lets malformed RRULEs bypass parsing when the feature is off. Update the
RRuleExpression construction path to guarantee validation in all builds, using a
wasm-safe validator or a separate boundary/input type if rrule::RRuleSet cannot
run on the guest path, and keep the validation logic centered around the same
factory/conversion flow so invalid instances remain unrepresentable.
In
`@rsworkspace/crates/trogon-scheduler-domain/src/commands/domain/schedule/tests.rs`:
- Around line 238-244: The fallback in the schedule timezone tests is treating
TzdbVersion::current() and the wasm “wasm-guest” value as interchangeable, which
bypasses the validation enforced by TzdbVersion::new and leaves an invalid
domain value representable. Update the conversion path used by the schedule
validation logic and the related assertions in the
schedule_timezone/rrule_timezone tests so the fallback goes through the same
validated constructor, or change the domain model to represent the wasm sentinel
explicitly instead of storing it in TzdbVersion.
In `@rsworkspace/crates/trogon-scheduler-domain/src/commands/proto_wire.rs`:
- Around line 219-243: The proto-to-domain conversion in
message_from_proto/message_content_from_proto is lossy-decoding Content.data
with String::from_utf8_lossy, which can silently alter payload bytes before they
enter ScheduleMessage. Update message_content_from_proto to either preserve the
original bytes in MessageContent or validate UTF-8 and return a typed
CommandWireError on invalid data, using the existing message_from_proto and
ScheduleMessage conversion path to enforce domain validity.
In `@rsworkspace/crates/trogon-scheduler-domain/src/commands/state.rs`:
- Around line 54-71: The ScheduleCreated handling in state.rs currently promotes
malformed create events to a present state; update the match arm for
ScheduleEventCase::ScheduleCreated in the state transition logic to validate the
inner payload before mutating state. Reject or return an error when
inner.schedule is missing or inner.status.kind cannot be determined, rather than
falling through to STATE_VALUE_PRESENT_ENABLED, and keep the same validation
pattern used by the occurrence branches so only validated domain data is
persisted in runtime state.
In `@rsworkspace/crates/trogonai-proto/src/lib.rs`:
- Around line 57-58: The decode path in the registry lookup currently conflates
a missing registration with a failed payload decode because `json_any_by_url`
and `to_json(payload)` both collapse to `None`. Update the logic around
`json_any_by_url` and the `to_json` call so that an unregistered type still
returns `None`, but a known type whose payload cannot be decoded returns a typed
error instead of being swallowed. Keep the existing symbols
`registry.json_any_by_url`, `entry.to_json`, and the surrounding helper that
returns the optional JSON string as the place to separate these two outcomes.
In `@wasm-goals.qa.md`:
- Around line 119-123: The guest responsibilities list in the
`trogon:decider@0.1.0` documentation still mentions a guest-side `stream-id`
call, but that symbol is not part of the exposed `session` contract. Update the
description around the host/guest split to remove `stream-id` from the guest
duties and keep the guest boundary limited to `constructor`, `evolve`, `decide`,
and `snapshot`, so the documented responsibilities match the actual interface.
---
Nitpick comments:
In @.github/actions/setup-rust/action.yml:
- Around line 21-29: Make the wasm32 target installation optional in the
setup-rust composite action so the non-WASM build path does not always incur the
extra rustup network call. Add a boolean input to the action and gate the
existing Install wasm32 target step in action.yml on that input, while keeping
the serial rustup target add workaround for the jobs that actually need it.
Update the callers for the test and wasm-decider jobs to enable it, and leave
the build job disabled.
In `@rsworkspace/crates/trogon-decider-guest-macros/src/lib.rs`:
- Around line 115-128: Remove the redundant bundle equality constraints from the
generated GuestSession impl in trogon-decider-guest-macros::lib.rs by updating
the bundle_bounds logic used for the `impl GuestSession for Session` generation.
Keep `bundle_assert` as the place that enforces bundle compatibility, and stop
emitting the extra `where` bounds tying each command to the canonical decider
State/Event types so the `wit_bindgen` error cascade no longer appears.
In
`@rsworkspace/crates/trogon-decider-guest-macros/tests/ui/not_a_decider.stderr`:
- Around line 1-488: The `export_decider` UI snapshot is too noisy because it
captures a cascade of secondary `E0277` errors from `wit_bindgen` instead of one
clear macro failure. Update the macro path around `export_decider` and the
generated `Session`/cabi helpers to perform an early trait check for the input
type (`NotADecider` / `Decider`) and emit a single `compile_error!` or dedicated
diagnostic before any `wit_bindgen::generate` expansion. Then refresh the
`not_a_decider.stderr` expectation to match only that one primary diagnostic.
In `@rsworkspace/crates/trogon-decider-sim/src/import_check.rs`:
- Around line 33-42: The error handling in ImportCheckError is flattening source
failures into String, so update the error enum and the affected paths in
import_check.rs to preserve typed sources instead of calling err.to_string().
Add dedicated variants with #[source] for the parser and io/command failures,
then adjust the map_err calls around temp.write_all, Command::new(...).output(),
and the other referenced sites to wrap the original errors directly while
keeping ImportCheckError as the public wrapper.
In `@rsworkspace/crates/trogon-decider-sim/src/import_check/tests.rs`:
- Around line 3-15: The current test only covers the raw-module fallback in
`rejects_core_module_with_imports`, so add a regression that exercises the
component path used by `assert_zero_imports` when `WASM_TOOLS` is unavailable or
invalid. Create a component-based fixture and force `WASM_TOOLS` to point to a
bogus executable so the code path behind `wasm-tools component wit` is
exercised, then assert the zero-import check still fails for imports in the
component flow. Use the existing `assert_zero_imports` helper and the new
component test to lock in this behavior.
In `@rsworkspace/crates/trogon-decider-sim/src/scenario/tests.rs`:
- Around line 10-25: Add a test that exercises the known-event semantic
comparison branch in events_match(). The current tests only cover type mismatch
and the raw-byte fallback; create a case using a recognized event type so
events_match() goes through decode_event_to_json(...) and compares semantic JSON
rather than bytes. Use a test name that clearly reflects known-event semantic
matching and keep it alongside the existing tests in this scenario module.
In `@rsworkspace/crates/trogon-decider-wit/tests/feature_build.rs`:
- Around line 53-67: The host_feature_fails_on_wasm32 test only checks that
cargo check fails, which can pass for unrelated errors; tighten it by asserting
the stderr from cargo_check().output() contains the expected unsupported-target
diagnostic for the host feature on wasm32. Update the existing
host_feature_fails_on_wasm32 test to match the intentional failure reason so it
specifically verifies the host/wasm32 contract instead of any build breakage.
🪄 Autofix (Beta)
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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9bc2ee35-2b8d-4106-b449-f361bc87525d
⛔ Files ignored due to path filters (10)
rsworkspace/Cargo.lockis excluded by!**/*.lockrsworkspace/crates/trogonai-proto/src/gen/trogonai.scheduler.schedules.v1.create_schedule.__view.rsis excluded by!**/gen/**rsworkspace/crates/trogonai-proto/src/gen/trogonai.scheduler.schedules.v1.create_schedule.rsis excluded by!**/gen/**rsworkspace/crates/trogonai-proto/src/gen/trogonai.scheduler.schedules.v1.mod.rsis excluded by!**/gen/**rsworkspace/crates/trogonai-proto/src/gen/trogonai.scheduler.schedules.v1.pause_schedule.__view.rsis excluded by!**/gen/**rsworkspace/crates/trogonai-proto/src/gen/trogonai.scheduler.schedules.v1.pause_schedule.rsis excluded by!**/gen/**rsworkspace/crates/trogonai-proto/src/gen/trogonai.scheduler.schedules.v1.remove_schedule.__view.rsis excluded by!**/gen/**rsworkspace/crates/trogonai-proto/src/gen/trogonai.scheduler.schedules.v1.remove_schedule.rsis excluded by!**/gen/**rsworkspace/crates/trogonai-proto/src/gen/trogonai.scheduler.schedules.v1.resume_schedule.__view.rsis excluded by!**/gen/**rsworkspace/crates/trogonai-proto/src/gen/trogonai.scheduler.schedules.v1.resume_schedule.rsis excluded by!**/gen/**
📒 Files selected for processing (109)
.github/actions/setup-rust/action.yml.github/workflows/ci-rust.yml.mise.tomlWASM_TODO.mdproto/trogonai/scheduler/schedules/v1/create_schedule.protoproto/trogonai/scheduler/schedules/v1/pause_schedule.protoproto/trogonai/scheduler/schedules/v1/remove_schedule.protoproto/trogonai/scheduler/schedules/v1/resume_schedule.protorsworkspace/.cargo/config.tomlrsworkspace/Cargo.tomlrsworkspace/cli/trogon-decider-test/Cargo.tomlrsworkspace/cli/trogon-decider-test/schedules.yamlrsworkspace/cli/trogon-decider-test/src/codec.rsrsworkspace/cli/trogon-decider-test/src/codec/tests.rsrsworkspace/cli/trogon-decider-test/src/main.rsrsworkspace/crates/trogon-decider-guest-macros/Cargo.tomlrsworkspace/crates/trogon-decider-guest-macros/src/lib.rsrsworkspace/crates/trogon-decider-guest-macros/tests/compile_fail.rsrsworkspace/crates/trogon-decider-guest-macros/tests/ui/bundle_mismatched_event.rsrsworkspace/crates/trogon-decider-guest-macros/tests/ui/bundle_mismatched_event.stderrrsworkspace/crates/trogon-decider-guest-macros/tests/ui/bundle_mismatched_module.rsrsworkspace/crates/trogon-decider-guest-macros/tests/ui/bundle_mismatched_module.stderrrsworkspace/crates/trogon-decider-guest-macros/tests/ui/bundle_mismatched_schema.rsrsworkspace/crates/trogon-decider-guest-macros/tests/ui/bundle_mismatched_schema.stderrrsworkspace/crates/trogon-decider-guest-macros/tests/ui/bundle_mismatched_version.rsrsworkspace/crates/trogon-decider-guest-macros/tests/ui/bundle_mismatched_version.stderrrsworkspace/crates/trogon-decider-guest-macros/tests/ui/not_a_decider.rsrsworkspace/crates/trogon-decider-guest-macros/tests/ui/not_a_decider.stderrrsworkspace/crates/trogon-decider-guest-sdk/Cargo.tomlrsworkspace/crates/trogon-decider-guest-sdk/src/bridge.rsrsworkspace/crates/trogon-decider-guest-sdk/src/lib.rsrsworkspace/crates/trogon-decider-guest-sdk/src/snapshot.rsrsworkspace/crates/trogon-decider-runtime/src/event/mod.rsrsworkspace/crates/trogon-decider-runtime/src/lib.rsrsworkspace/crates/trogon-decider-sim/Cargo.tomlrsworkspace/crates/trogon-decider-sim/src/fixture.rsrsworkspace/crates/trogon-decider-sim/src/host.rsrsworkspace/crates/trogon-decider-sim/src/import_check.rsrsworkspace/crates/trogon-decider-sim/src/import_check/tests.rsrsworkspace/crates/trogon-decider-sim/src/lib.rsrsworkspace/crates/trogon-decider-sim/src/scenario.rsrsworkspace/crates/trogon-decider-sim/src/scenario/tests.rsrsworkspace/crates/trogon-decider-sim/src/session.rsrsworkspace/crates/trogon-decider-sim/tests/schedules.rsrsworkspace/crates/trogon-decider-wit/Cargo.tomlrsworkspace/crates/trogon-decider-wit/src/lib.rsrsworkspace/crates/trogon-decider-wit/tests/feature_build.rsrsworkspace/crates/trogon-decider-wit/wit/world.witrsworkspace/crates/trogon-decider/src/event/codec/event_decode.rsrsworkspace/crates/trogon-decider/src/event/codec/event_decode/tests.rsrsworkspace/crates/trogon-decider/src/event/codec/event_encode.rsrsworkspace/crates/trogon-decider/src/event/codec/event_payload_error.rsrsworkspace/crates/trogon-decider/src/event/codec/event_payload_error/tests.rsrsworkspace/crates/trogon-decider/src/event/codec/event_type.rsrsworkspace/crates/trogon-decider/src/event/codec/mod.rsrsworkspace/crates/trogon-decider/src/event/mod.rsrsworkspace/crates/trogon-decider/src/lib.rsrsworkspace/crates/trogon-decider/src/tests.rsrsworkspace/crates/trogon-scheduler-domain/Cargo.tomlrsworkspace/crates/trogon-scheduler-domain/src/commands/create_schedule.rsrsworkspace/crates/trogon-scheduler-domain/src/commands/create_schedule/tests.rsrsworkspace/crates/trogon-scheduler-domain/src/commands/domain/message.rsrsworkspace/crates/trogon-scheduler-domain/src/commands/domain/message/tests.rsrsworkspace/crates/trogon-scheduler-domain/src/commands/domain/message/tests/prop_tests.rsrsworkspace/crates/trogon-scheduler-domain/src/commands/domain/mod.rsrsworkspace/crates/trogon-scheduler-domain/src/commands/domain/schedule.rsrsworkspace/crates/trogon-scheduler-domain/src/commands/domain/schedule/tests.rsrsworkspace/crates/trogon-scheduler-domain/src/commands/domain/schedule/tests/prop_tests.rsrsworkspace/crates/trogon-scheduler-domain/src/commands/domain/schedule_event_delivery.rsrsworkspace/crates/trogon-scheduler-domain/src/commands/domain/schedule_event_delivery/tests.rsrsworkspace/crates/trogon-scheduler-domain/src/commands/domain/schedule_event_sampling_source.rsrsworkspace/crates/trogon-scheduler-domain/src/commands/domain/schedule_event_sampling_source/tests.rsrsworkspace/crates/trogon-scheduler-domain/src/commands/domain/schedule_event_schedule.rsrsworkspace/crates/trogon-scheduler-domain/src/commands/domain/schedule_event_schedule/tests.rsrsworkspace/crates/trogon-scheduler-domain/src/commands/domain/schedule_event_status.rsrsworkspace/crates/trogon-scheduler-domain/src/commands/domain/schedule_event_status/tests.rsrsworkspace/crates/trogon-scheduler-domain/src/commands/domain/schedule_id.rsrsworkspace/crates/trogon-scheduler-domain/src/commands/domain/schedule_id/tests.rsrsworkspace/crates/trogon-scheduler-domain/src/commands/domain/schedule_id/tests/prop_tests.rsrsworkspace/crates/trogon-scheduler-domain/src/commands/mod.rsrsworkspace/crates/trogon-scheduler-domain/src/commands/pause_schedule.rsrsworkspace/crates/trogon-scheduler-domain/src/commands/pause_schedule/tests.rsrsworkspace/crates/trogon-scheduler-domain/src/commands/proto_wire.rsrsworkspace/crates/trogon-scheduler-domain/src/commands/proto_wire/tests.rsrsworkspace/crates/trogon-scheduler-domain/src/commands/remove_schedule.rsrsworkspace/crates/trogon-scheduler-domain/src/commands/remove_schedule/tests.rsrsworkspace/crates/trogon-scheduler-domain/src/commands/resume_schedule.rsrsworkspace/crates/trogon-scheduler-domain/src/commands/resume_schedule/tests.rsrsworkspace/crates/trogon-scheduler-domain/src/commands/state.rsrsworkspace/crates/trogon-scheduler-domain/src/commands/state/tests.rsrsworkspace/crates/trogon-scheduler-domain/src/lib.rsrsworkspace/crates/trogon-scheduler-domain/src/snapshot_policy.rsrsworkspace/crates/trogon-scheduler-domain/src/snapshot_policy/tests.rsrsworkspace/crates/trogon-scheduler-domain/src/subject.rsrsworkspace/crates/trogon-scheduler-domain/src/subject/tests.rsrsworkspace/crates/trogon-scheduler/Cargo.tomlrsworkspace/crates/trogon-scheduler/src/commands/domain/mod.rsrsworkspace/crates/trogon-scheduler/src/commands/mod.rsrsworkspace/crates/trogon-scheduler/src/commands/state.rsrsworkspace/crates/trogonai-proto/Cargo.tomlrsworkspace/crates/trogonai-proto/src/codec.rsrsworkspace/crates/trogonai-proto/src/lib.rsrsworkspace/crates/trogonai-proto/src/scheduler/schedules/codec.rsrsworkspace/crates/trogonai-proto/src/scheduler/schedules/codec/tests.rsrsworkspace/crates/trogonai-proto/src/scheduler/schedules/mod.rsrsworkspace/crates/trogonai-proto/src/tests.rsrsworkspace/wasm-components/trogon-schedules-decider/Cargo.tomlrsworkspace/wasm-components/trogon-schedules-decider/src/lib.rswasm-goals.qa.md
💤 Files with no reviewable changes (3)
- rsworkspace/crates/trogon-scheduler-domain/src/commands/resume_schedule/tests.rs
- rsworkspace/crates/trogon-scheduler-domain/src/commands/remove_schedule/tests.rs
- rsworkspace/crates/trogon-scheduler-domain/src/commands/pause_schedule/tests.rs
Comment on lines
+238
to
+244
| assert_eq!( | ||
| schedule_timezone.tzdb_version().as_str(), | ||
| TzdbVersion::current().as_str() | ||
| ); | ||
| assert_eq!(schedule_timezone.into_string(), "UTC"); | ||
| assert_eq!(rrule_timezone.as_str(), "UTC"); | ||
| assert_eq!(rrule_timezone.tzdb_version().as_str(), chrono_tz::IANA_TZDB_VERSION); | ||
| assert_eq!(rrule_timezone.tzdb_version().as_str(), TzdbVersion::current().as_str()); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Don’t make TzdbVersion::current() a backdoor around validation.
Line 238 now accepts whatever TzdbVersion::current() returns, but the no-schedule-validation path constructs "wasm-guest" directly while TzdbVersion::new(...) rejects non-version-shaped values. That makes an invalid value object representable and likely persistable from wasm-domain state. Prefer making the fallback pass the same constructor/validation, or model the wasm sentinel as an explicit enum variant instead of a raw TzdbVersion.
As per coding guidelines, “After conversion from boundary types to domain types, domain values must be valid forever.”
🤖 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
`@rsworkspace/crates/trogon-scheduler-domain/src/commands/domain/schedule/tests.rs`
around lines 238 - 244, The fallback in the schedule timezone tests is treating
TzdbVersion::current() and the wasm “wasm-guest” value as interchangeable, which
bypasses the validation enforced by TzdbVersion::new and leaves an invalid
domain value representable. Update the conversion path used by the schedule
validation logic and the related assertions in the
schedule_timezone/rrule_timezone tests so the fallback goes through the same
validated constructor, or change the domain model to represent the wasm sentinel
explicitly instead of storing it in TzdbVersion.
Source: Coding guidelines
Comment on lines
+119
to
+123
| **Host, entirely.** The guest is passive — it only answers calls. Host does: receive | ||
| command → load snapshot bytes → read event batches → drive `session.evolve` → call | ||
| `session.decide` → append with OCC (+ retries) → optionally request + store a snapshot. | ||
| Guest does: answer `stream-id`, decode its own snapshot, fold events, decide, serialize | ||
| state on request. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Drop stream-id from the guest responsibilities list.
The current trogon:decider@0.1.0 contract only exposes constructor, evolve, decide, and snapshot on session, so documenting a guest-side stream-id call here sends implementers toward a non-existent boundary.
Suggested edit
-Guest does: answer `stream-id`, decode its own snapshot, fold events, decide, serialize
+Guest does: decode its own snapshot, fold events, decide, serialize
state on request.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| **Host, entirely.** The guest is passive — it only answers calls. Host does: receive | |
| command → load snapshot bytes → read event batches → drive `session.evolve` → call | |
| `session.decide` → append with OCC (+ retries) → optionally request + store a snapshot. | |
| Guest does: answer `stream-id`, decode its own snapshot, fold events, decide, serialize | |
| state on request. | |
| **Host, entirely.** The guest is passive — it only answers calls. Host does: receive | |
| command → load snapshot bytes → read event batches → drive `session.evolve` → call | |
| `session.decide` → append with OCC (+ retries) → optionally request + store a snapshot. | |
| Guest does: decode its own snapshot, fold events, decide, serialize state on request. |
🤖 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 `@wasm-goals.qa.md` around lines 119 - 123, The guest responsibilities list in
the `trogon:decider@0.1.0` documentation still mentions a guest-side `stream-id`
call, but that symbol is not part of the exposed `session` contract. Update the
description around the host/guest split to remove `stream-id` from the guest
duties and keep the guest boundary limited to `constructor`, `evolve`, `decide`,
and `snapshot`, so the documented responsibilities match the actual interface.
Untyped fields in a suite (structured error, out-of-range nanos, delivery ttl/source, message headers) were silently dropped or rejected at load, letting a suite exercise different commands/events than it declared and hiding regressions. Fresh component per scenario stops guest-global state from leaking between runs and making results order-dependent. Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
The guest bridge flattened every failure into code/message strings at the point of failure, discarding the source error and its causal chain before reaching the boundary that actually needs the wire fields. Typed bridge and snapshot errors carry the source until projection happens once at the edge. Also resolve the trybuild WIT staging path from Cargo's real target directory so a custom CARGO_TARGET_DIR does not fail the UI test spuriously. Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
The sim host and scenario runner flattened Wasmtime, domain, and assertion failures into strings (and reported instantiation faults as compile errors), so callers lost the source chain and diagnostics were misleading. Snapshot frame parsing and the import check could also pass malformed or component input through unchecked, weakening the trap-on-bad-snapshot and zero-import guarantees. Typed errors and wire-type/bounds validation restore both. Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
The schedule-validation gate let the wasm guest construct invalid TimeZone, RRuleExpression, and TzdbVersion values — including persisting a fake "wasm-guest" tzdb version into events — because chrono-tz and rrule were both gated off for purity. chrono-tz is import-clean, so timezone validation and the real tzdb version now run in every build. rrule must stay gated (it pulls wasm-bindgen and would break the zero-import contract), so RRULE FREQ is now validated wasm-safely to fail closed on malformed rules in the guest. Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
…dary Three boundary paths admitted invalid data into domain/runtime state: message content was lossy-decoded so non-UTF-8 bytes were silently rewritten, a ScheduleCreated event missing its schedule or status was promoted to a present state, and the event-to-json decoder collapsed a malformed known payload into the same None as an unregistered type so semantic matching silently fell back to raw bytes. Each now fails closed with a typed error. Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
…it-support Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com> # Conflicts: # rsworkspace/Cargo.lock
…ative replay The guest bridge returned an unknown-event error when EventDecode skipped an envelope, but the native runtime replay treats a skipped envelope as a no-op and keeps folding. That divergence let a shared/migrated stream replay to a different state (or fail outright) under WASM than under the host, breaking the run-anywhere identical-logic guarantee. Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
rsworkspace/crates/trogon-scheduler-domain/src/commands/domain/schedule.rs (1)
312-352: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftGuest builds can still mint invalid
RRuleExpressionvalues.With
schedule-validationdisabled, this validator only checks a small subset of RRULE semantics. Inputs likeFREQ=DAILY;COUNT=abcorFREQ=DAILY;BYMONTH=foostill reachOk(())because the fullrruleparse is compiled out and these parts are ignored here. That means malformed wire input can still cross into the domain as a supposedly-valid value object.🤖 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 `@rsworkspace/crates/trogon-scheduler-domain/src/commands/domain/schedule.rs` around lines 312 - 352, The RRULE validator in the schedule domain is too permissive when schedule-validation is disabled, so malformed guest-build inputs can still be accepted by validate_rrule. Tighten the manual parsing in the RRULE validation path by checking the syntax and values of all relevant parts, not just FREQ/COUNT/UNTIL, and reject invalid tokens like non-numeric COUNT or malformed BYMONTH before returning Ok. Use the existing validate_rrule, is_supported_frequency, and invalid_rrule helpers to keep the guest-safe validation aligned with the full RRuleSet::from_str behavior.Source: Coding guidelines
🤖 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 `@rsworkspace/crates/trogon-decider-guest-sdk/src/snapshot.rs`:
- Around line 153-166: The read_varint helper in snapshot.rs can still accept a
10-byte varint whose final payload bits overflow u64, because the last chunk is
ORed into value before overflow is rejected. Update read_varint to detect the
overflow case when shift reaches 63 and the terminal byte carries bits above
0x01, and return SnapshotDecodeError::VarintOverflow instead of truncating; keep
the fix localized to read_varint and preserve the existing
UnexpectedEof/overflow behavior.
---
Duplicate comments:
In `@rsworkspace/crates/trogon-scheduler-domain/src/commands/domain/schedule.rs`:
- Around line 312-352: The RRULE validator in the schedule domain is too
permissive when schedule-validation is disabled, so malformed guest-build inputs
can still be accepted by validate_rrule. Tighten the manual parsing in the RRULE
validation path by checking the syntax and values of all relevant parts, not
just FREQ/COUNT/UNTIL, and reject invalid tokens like non-numeric COUNT or
malformed BYMONTH before returning Ok. Use the existing validate_rrule,
is_supported_frequency, and invalid_rrule helpers to keep the guest-safe
validation aligned with the full RRuleSet::from_str behavior.
🪄 Autofix (Beta)
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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 79ad60f8-b7ff-4b95-904c-157cf0c947d2
⛔ Files ignored due to path filters (1)
rsworkspace/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (19)
rsworkspace/Cargo.tomlrsworkspace/cli/trogon-decider-test/src/main.rsrsworkspace/crates/trogon-decider-guest-macros/tests/compile_fail.rsrsworkspace/crates/trogon-decider-guest-sdk/src/bridge.rsrsworkspace/crates/trogon-decider-guest-sdk/src/lib.rsrsworkspace/crates/trogon-decider-guest-sdk/src/snapshot.rsrsworkspace/crates/trogon-decider-sim/src/host.rsrsworkspace/crates/trogon-decider-sim/src/import_check.rsrsworkspace/crates/trogon-decider-sim/src/lib.rsrsworkspace/crates/trogon-decider-sim/src/scenario.rsrsworkspace/crates/trogon-decider-sim/tests/schedules.rsrsworkspace/crates/trogon-scheduler-domain/Cargo.tomlrsworkspace/crates/trogon-scheduler-domain/src/commands/domain/schedule.rsrsworkspace/crates/trogon-scheduler-domain/src/commands/proto_wire.rsrsworkspace/crates/trogon-scheduler-domain/src/commands/state.rsrsworkspace/crates/trogon-scheduler-domain/src/commands/state/tests.rsrsworkspace/crates/trogon-scheduler/Cargo.tomlrsworkspace/crates/trogonai-proto/src/lib.rsrsworkspace/crates/trogonai-proto/src/tests.rs
💤 Files with no reviewable changes (2)
- rsworkspace/crates/trogonai-proto/src/tests.rs
- rsworkspace/crates/trogonai-proto/src/lib.rs
🚧 Files skipped from review as they are similar to previous changes (11)
- rsworkspace/crates/trogon-decider-guest-sdk/src/lib.rs
- rsworkspace/crates/trogon-decider-sim/src/lib.rs
- rsworkspace/Cargo.toml
- rsworkspace/crates/trogon-scheduler/Cargo.toml
- rsworkspace/crates/trogon-decider-guest-macros/tests/compile_fail.rs
- rsworkspace/crates/trogon-decider-sim/tests/schedules.rs
- rsworkspace/crates/trogon-scheduler-domain/src/commands/state.rs
- rsworkspace/crates/trogon-decider-sim/src/host.rs
- rsworkspace/cli/trogon-decider-test/src/main.rs
- rsworkspace/crates/trogon-decider-sim/src/import_check.rs
- rsworkspace/crates/trogon-scheduler-domain/src/commands/proto_wire.rs
The merged std_env_access dylint rule rejects reading WASM_TOOLS through std::env directly, and the host coverage gate counted guest-only code that host instrumentation can never reach. Reading via the trogon_std env abstraction and excluding the WASM guest glue, the compiled component, and the conformance CLI from the gate keep both checks measuring what host tests can actually exercise. Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
The 10th varint byte contributes a single bit to a u64, so payload bits above 0x01 shifted out and silently truncated, letting a malformed snapshot length or key decode as a different value instead of failing closed. Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
…e gate The merged dylint rules reject comparing an error's display string and inline test module bodies. Scenario assertions now match typed ScenarioError variants, the decider-test and snapshot test modules move into their own files, and added import-check host tests cover the previously-unexercised core-module paths so the host coverage gate clears its threshold. Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit c017fde. Configure here.
| pub const RESUME_SCHEDULE_TYPE_URL: &str = v1::ResumeSchedule::TYPE_URL; | ||
|
|
||
| /// Stable type URL for the [`state_v1::State`] snapshot schema version tag. | ||
| pub const SCHEDULES_STATE_SCHEMA_VERSION: &str = state_v1::State::TYPE_URL; |
There was a problem hiding this comment.
Snapshot schema tag mismatch
Medium Severity
WASM snapshot frames tag schema with SCHEDULES_STATE_SCHEMA_VERSION (State::TYPE_URL), while the native snapshot path resolves state via SnapshotType using State::FULL_NAME. A host that fills the guest snapshot frame with the runtime type name fails decode_snapshot and traps in load_or_initial instead of loading state.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit c017fde. Configure here.
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.