fix: vm fatal errors - #24
Conversation
|
👋 This PR targeted
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe change adds an Axum WebSocket manager protocol, nested cross-major execution support, generated interfaces and hashing types, expanded execution lifecycle handling, stricter runner validation, configurable runner archives, and related specifications, tests, and contributor guidance. ChangesManager execution and protocol
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related issues
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
GenVM PR actionsTick a box to run it (the box unticks itself when handled). Actions only run while the PR has the
MergeRequires, on the exact head commit:
Full CI runs only when Every repo lands ONE squashed commit, subject Commands
|
Linked executor PR(s)executor: genlayerlabs/genvm-executor#21 (v0.2) |
There was a problem hiding this comment.
Actionable comments posted: 13
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
implementation/src/manager/run.rs (1)
256-284: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
causeandexit_codeare added to the struct but not serialized.
SingleGenVMContextDonegainscauseandexit_code, and the terminalEvent::Finishedcarries both. The manualSerializeimplementation still writes eight fields and omits them. An HTTP client readingGET /genvm/{genvm_id}therefore cannot tell an exited run from a cancelled or deadline-terminated one, while a WebSocket client can.If the omission is intentional, add a comment stating it. Otherwise serialize both fields.
🔧 Proposed fix
- let mut state = serializer.serialize_struct("SingleGenVMContextDone", 8)?; + let mut state = serializer.serialize_struct("SingleGenVMContextDone", 10)?; @@ state.serialize_field("version_major", &self.version_major)?; state.serialize_field("version_minor", &self.version_minor)?; + state.serialize_field("cause", self.cause.as_str())?; + state.serialize_field("exit_code", &self.exit_code)?; state.end()🤖 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 `@implementation/src/manager/run.rs` around lines 256 - 284, Update SingleGenVMContextDone::serialize to include both cause and exit_code in the serialized output, and adjust the declared field count accordingly so GET responses expose the same termination details as Event::Finished.install/lib/python/post-install/__main__.py (1)
582-593: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winMake legacy runner integrity checking unconditional.
verify_hash=not is_legacyskips hash checks for both existing and downloaded v0.2.x runners, andmanager check-installruns only whenargs.precompileis enabled. This leaves--precompile=falsewithout an enforced integrity check. Run the legacy executor check whenever runner downloads are enabled, or add Nix-base32 verification before writing the archive.</verification,comment>
🤖 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 `@install/lib/python/post-install/__main__.py` around lines 582 - 593, Make legacy runner integrity validation unconditional in the download flow around download_runners_from_json: when runner downloads are enabled, always run the legacy executor’s check command, including when args.precompile is false, or perform equivalent Nix-base32 verification before writing the archive. Preserve the existing hash verification for non-legacy runners.Source: MCP tools
🟡 Minor comments (12)
docs/adr/014. manager host socket protocol.md-46-48 (1)
46-48: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd a language to the wire-format code fence.
The fence at Line 46 has no language.
markdownlint-cli2reports MD040. Usetextfor this protocol layout.🤖 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 `@docs/adr/014`. manager host socket protocol.md around lines 46 - 48, Update the wire-format code fence containing the method_id/request_id layout to declare the text language, resolving the MD040 language requirement without changing the protocol content.Source: Linters/SAST tools
docs/contributing/howto/building/docs.md-33-33 (1)
33-33: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winList all generated constants pages in this table.
docs/contributing/howto/genvm-tool.mdLines 33-35 listsconstants-pending.rstandinternal-constants.rstas generated outputs. Line 33 lists onlyconstants.rstandmanager-socket-consts.rst. Add the two omitted pages.🤖 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 `@docs/contributing/howto/building/docs.md` at line 33, Update the generated-output table in the documentation to include constants-pending.rst and internal-constants.rst alongside the existing constants.rst and manager-socket-consts.rst entries, preserving the codegen reference and link formatting.docs/website/src/impl-spec/01-core-architecture/01-components.rst-119-119 (1)
119-119: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winLimit the ZIP-only runner identity statement to forward-rolling lines.
Line 119 says all built-in runners use hashes of ZIP contents.
docs/contributing/howto/committing/submodules.mdLines 82-85 documents frozen v0.2.x runners as ustar TAR files with Nix base32 hashes. Qualify this sentence or document both schemes.🤖 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 `@docs/website/src/impl-spec/01-core-architecture/01-components.rst` at line 119, Update the runner identity statement near the built-in runner description to apply only to the forward-rolling format, or explicitly document both ZIP-content hashes and the frozen v0.2.x ustar TAR/Nix base32 scheme. Preserve the existing contract accessibility context while avoiding an unqualified claim that all built-in runners use ZIP hashes.docs/website/src/spec/appendix/internal-constants.rst-1-2 (1)
1-2: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUse
Internal Constantsas the page title.
docs/website/src/spec/appendix/index.rstlabels this pageInternal Constants, but the rendered page title isConstants. Rename the title to avoid confusing duplicate page headings.Suggested fix
-Constants -========= +Internal Constants +==================🤖 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 `@docs/website/src/spec/appendix/internal-constants.rst` around lines 1 - 2, Rename the document title under the “Constants” heading to “Internal Constants” so it matches the label in the appendix index and avoids duplicate or confusing headings.docs/website/src/impl-spec/appendix/manager-api.rst-9-12 (1)
9-12: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winState the lifecycle of
GET /genvm/{id}/artifact.This paragraph lists three deprecated execution endpoints.
docs/website/src/impl-spec/appendix/manager-api.yamlLines 321-327 add a fourth/genvm/*route,GET /genvm/{genvm_id}/artifact, described there as a debug convenience adapter over the socketget_artifact. That route is the only/genvm/*route withoutdeprecated: true, and this page does not mention it.A host integrator cannot tell whether the artifact route is removed with the other three. Add it to the list, or state that it survives the removal as a debug aid.
🤖 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 `@docs/website/src/impl-spec/appendix/manager-api.rst` around lines 9 - 12, Update the deprecated execution-endpoints paragraph to explicitly state the lifecycle of GET /genvm/{id}/artifact, using the API specification’s documented behavior: either include it in the one-release deprecation/removal list or clearly state that it remains afterward as a debug convenience adapter.implementation/src/manager/socket.rs-265-273 (1)
265-273: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winDo not encode non-integer JSON numbers as null.
Lines 265-273 map a
serde_json::Numbertopush_i64orpush_u64. A floating-point number matches neither, so Line 271 emitsnull. Metrics that contain durations or ratios therefore lose their value on the wire, and the client cannot tell a missing metric from a dropped float.Encode the float, or encode its decimal string, so the value survives. If calldata has no float type, state the chosen representation in
manager-socket.rst.🤖 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 `@implementation/src/manager/socket.rs` around lines 265 - 273, Update the serde_json::Value::Number handling in the socket encoder to preserve non-integer numbers instead of falling back to push_null: encode the floating-point value when supported, otherwise encode its decimal string representation. Document the chosen wire representation in manager-socket.rst so clients can distinguish it from a missing metric.docs/website/src/impl-spec/appendix/manager-socket.rst-283-289 (1)
283-289: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDo not claim
malformed_framefor invaliddeadlinevalues.
deadlineusesManagerDuration, so the wire value is decoded beforerun_ctx.startvalidates it withManagerDuration::to_std. That convertsduration number is empty,duration has more than one decimal point, and unitless strings into handler failures reported asinternal, notmalformed_frame. Update this wording to match the documented error meanings.🤖 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 `@docs/website/src/impl-spec/appendix/manager-socket.rst` around lines 283 - 289, The deadline documentation incorrectly classifies invalid ManagerDuration values as malformed_frame. Update the validation/error wording in the deadline section to state that values rejected by ManagerDuration::to_std, including empty numbers, multiple decimal points, and unitless strings, produce an internal handler failure (or the documented startup configuration error), while preserving the unit and format requirements.implementation/src/manager/run.rs-2134-2138 (1)
2134-2138: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winA failure after startup publishes
FailedToStart.
supervise_genvm_innerreturnsErrfor failures that can occur afterpublish_startedran, for example anio::Errorfromchild.wait()or the missing-process-handle path.supervise_genvmthen callsfail_to_start, so a client observesStartedfollowed byFailedToStart.FailedToStartcarries no exit code and no artifact sizes, so the client cannot reconcile the run.Select the terminal event by whether the start event was already published.
🔧 Proposed fix
if let Err(e) = supervise_genvm_inner(full_ctx, exec_ctx.clone(), req, modules_lock, permits).await { - fail_to_start(&exec_ctx, e); + if exec_ctx.started_event.get().is_some() { + log_error!(error:ah = &e; "execution failed after start"); + let cause = exec_ctx.finish_cause().unwrap_or(FinishCause::Exited); + let _ = finish_execution(&exec_ctx, None, cause).await; + } else { + fail_to_start(&exec_ctx, e); + } }Also applies to: 2162-2172
🤖 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 `@implementation/src/manager/run.rs` around lines 2134 - 2138, Update the error handling in supervise_genvm around supervise_genvm_inner so terminal failures use the start-publication state: call fail_to_start only when publish_started has not run, and emit the appropriate post-start failure/termination event when startup was already published. Ensure post-start errors such as child.wait failures or a missing process handle preserve the Started-then-terminal event contract and include the available exit code and artifact sizes.support/runner-script.py-95-100 (1)
95-100: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRename the
hashparameters to avoid Ruff A002.
support/runner-script.pyhas function arguments namedhashincheck_bytes,_object_gcs_path,_download_single,_nix_preload, and_upload_single. Rename them tohash_idorarchive_hashand update the call sites so the A002 shadowing lint does not break the build.🤖 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 `@support/runner-script.py` around lines 95 - 100, Rename the hash parameters in check_bytes, _object_gcs_path, _download_single, _nix_preload, and _upload_single to hash_id or archive_hash, and update all references and call sites consistently while preserving behavior.Source: Linters/SAST tools
SECURITY.md-43-48 (1)
43-48: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winClose the local trust boundary to an explicit assumption.
The manager default binds TCP to
127.0.0.1:3999and supports--socket, but the policy says loopback/local disk is trusted without stating the requirement on local account/socket permissions. State that the loopback-only manager and manifest/data paths must be protected by OS accounts and filesystem permission rules where the host is not trusted.🤖 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 `@SECURITY.md` around lines 43 - 48, Update the trusted-relationships section in SECURITY.md to explicitly require OS account and filesystem permission protections for the loopback-only manager, its socket, and manifest/data paths when the host is not trusted. Clarify that local disk and loopback trust depends on restricting access to authorized local users and processes.install/config/genvm-manager.yaml-11-12 (1)
11-12: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winUpdate the manager schema for the new config fields.
install/config/genvm-manager.yamlreferencesdocs/schemas/default-config.json#/manager, but that schema only definesmanifest_pathandpermits. It does not defineexecution_retentionormax_message_bytes; add both fields with their accepted type/shape or update the linked schema source.🤖 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 `@install/config/genvm-manager.yaml` around lines 11 - 12, Update the manager schema referenced by the genvm-manager configuration to define execution_retention and max_message_bytes with the accepted types and validation shape used by the configuration. Preserve the existing manifest_path and permits definitions, and ensure the schema source linked by the manager configuration is the one containing both new fields.Source: MCP tools
.claude/skills/test/SKILL.md-10-10 (1)
10-10: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd a language to the code fence.
markdownlint-cli2reports MD040 on Line 10. Change the opening fence toshell.Suggested change
-``` +```shell🤖 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 @.claude/skills/test/SKILL.md at line 10, Update the opening code fence in SKILL.md to specify the shell language, changing the unlabeled fence to a shell-labeled fence while preserving the fenced command block contents.Source: Linters/SAST tools
🧹 Nitpick comments (5)
implementation/src/manager/socket_test.rs (1)
128-239: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd tests for the frame-level protocol invariants.
The current tests cover the writer queue and the event forwarder well. Three documented invariants in
manager-socket.rsthave no test:
read_framerejects a message shorter thanHEADER_LENand an unknownmethod_idwithout closing the connection.dispatchanswersbad_request_idwhenrequest_id == 0.handle_get_artifactclampsmax_lentoARTIFACT_CHUNK_CAP.
read_frameaccepts anyStream<Item = Result<Message, axum::Error>>, so the first two are testable withfutures_util::stream::iterand atest_writerpair. Do you want me to generate these tests?🤖 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 `@implementation/src/manager/socket_test.rs` around lines 128 - 239, Add focused tests for the documented frame-level invariants using read_frame, dispatch, and handle_get_artifact. Cover rejection of messages shorter than HEADER_LEN and unknown method_id while keeping the connection usable, dispatch returning bad_request_id for request_id == 0, and handle_get_artifact clamping max_len to ARTIFACT_CHUNK_CAP. Use futures_util::stream::iter with test_writer where applicable, and assert the expected protocol responses and connection behavior.implementation/src/manager/socket.rs (2)
102-110: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winGuard the artifact branch against a closed receiver with an open control channel.
If
artifact_rxreturnsNonewhilecontrol_rxis still open, thecontinueon Line 107 restarts the loop.artifact_rx.recv()then resolves toReady(None)immediately on every iteration, and the biasedselect!keeps choosing it whilecontrol_rx.recv()stays pending. The result is a busy loop that consumes one core.Today every
Writerholds both senders, so both channels close together and the control branch breaks first. The invariant is not enforced by the types, and a future change that splits the senders reintroduces the spin. Track the closed state and stop pollingartifact_rx.♻️ Proposed fix
+ let mut artifact_open = true; loop { while let Ok(frame) = control_rx.try_recv() { write_frame(&mut sink, frame).await?; } tokio::select! { biased; frame = control_rx.recv() => { let Some(frame) = frame else { break; }; write_frame(&mut sink, frame).await?; } - frame = artifact_rx.recv() => { + frame = artifact_rx.recv(), if artifact_open => { let Some(frame) = frame else { - if control_rx.is_closed() { - break; - } - continue; + artifact_open = false; + continue; }; write_frame(&mut sink, frame).await?; } } }🤖 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 `@implementation/src/manager/socket.rs` around lines 102 - 110, Update the writer loop around the artifact_rx branch to track whether artifact_rx has closed and stop polling it afterward, while continuing to service control_rx until it closes. Preserve the existing frame-writing behavior and clean shutdown when both channels are closed, without relying on the senders closing together.
571-575: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winReplace error-string matching with typed error discrimination.
attachandget_artifactuse plainanyhow::bail!strings as discriminants, andsocket.rsmaps them by comparinge.to_string()to"boot_id_mismatch"or"unknown_id". Any context wrapped around these errors changes the mapping, sending the wrongErrorsvariant. Expose a typed error enum fromrun.rsor return a dedicated result type fromattachandget_artifactand match on that.🤖 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 `@implementation/src/manager/socket.rs` around lines 571 - 575, Replace the string-based error checks in the socket request handling with typed discrimination for failures from attach and get_artifact. Define or reuse a dedicated error enum/result type in run.rs, propagate those typed variants through both operations, and match on the variants instead of e.to_string() so BootIdMismatch and UnknownId map to the correct Errors values even when context is added.docs/website/src/impl-spec/appendix/manager-socket.rst (1)
61-63: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueLimit the payload tag claim to external variants and widen the
metricstype.Scope line 61 to request/event payloads that include a variant tag, because documented responses include non-tag-shaped payloads such as
{ "genvm_id": u64 }and{ "total_len": u64, "data": bytes }. Typemetricsas any JSON value, sinceencode_json_valuesupportsnull, booleans, numbers, strings, arrays, and objects.The
gvm-def-enum-methodsandgvm-def-enum-errorsSphinx labels are present onmanager-socket-consts.🤖 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 `@docs/website/src/impl-spec/appendix/manager-socket.rst` around lines 61 - 63, Revise the payload-format statement in the manager socket specification to apply only to request and event payloads with external variant tags, not all payloads; preserve the separate response shapes such as genvm_id and total_len/data. Widen the metrics field type to any JSON value supported by encode_json_value, including null, booleans, numbers, strings, arrays, and objects. Keep the existing Sphinx labels on manager-socket-consts and do not add duplicate labels.implementation/src/manager/mod.rs (1)
185-223: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExtract the repeated JSON-body handler shape.
Five routes repeat the same closure body: parse
Bytesintoserde_json::Value, convert a parse failure into a response, then call the handler. A small generic helper removes the duplication and keeps the error mapping consistent when it changes.♻️ Sketch
async fn json_body_route<F, Fut, R>(body: Bytes, handler: F) -> Response where F: FnOnce(serde_json::Value) -> Fut, Fut: std::future::Future<Output = anyhow::Result<R>>, R: IntoResponse, { match parse_json_body(body) { Ok(value) => unwrap_all_anyhow(handler(value).await), Err(error) => bad_request(error), } }🤖 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 `@implementation/src/manager/mod.rs` around lines 185 - 223, Extract the repeated closure logic from the module route registrations into a generic async helper, such as json_body_route, that parses Bytes, maps parse failures to the established error response, and invokes the supplied handler through unwrap_all_anyhow. Update the /module/start, /module/stop, /module/restart, and the other matching JSON routes to pass their handlers to this helper while preserving their existing response types and error 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 `@docs/adr/011`. runner ids and runtime registration.md:
- Around line 58-63: Update the earlier RegisterRunner authorization decision
near the referenced paragraph so it no longer requires the removed
register_runners permission. Align it with the later superseded statement by
documenting deterministic mode alone as the current requirement, or clearly
marking the entire old requirement as superseded.
In `@docs/adr/014`. manager host socket protocol.md:
- Around line 329-333: Update the rollout sequence description to say executors
add “hello + removal of notify_finished” instead of “hello + notify_finished,”
while preserving the surrounding staged rollout and EOF-termination requirement.
In `@docs/website/src/impl-spec/appendix/manager-api.yaml`:
- Around line 656-700: Update ExecutorSelector and its related schemas to use
the externally tagged socket shape: a single-key major or version map whose
value contains the corresponding selector fields. Remove the internal kind
discriminator and kind properties, or introduce an ExecutorSelectorRequest
wrapper that models this contract, while preserving the existing major and
version payload semantics.
In
`@docs/website/src/spec/02-execution-environment/03-wasi_genlayer_sdk/02-gl_call.rst`:
- Around line 87-93: Update the delegation rule in the call execution
description to state that a non-null host response is rejected with error_inval
when the caller has loaded custom runners; only otherwise should it execute the
derived sub-VM in the selected executor. Preserve the existing null-response
local path and observable-output behavior.
In
`@docs/website/src/spec/02-execution-environment/03-wasi_genlayer_sdk/03-schemas.rst`:
- Around line 55-58: Update the vm_error validation contract in the schema
specification so deterministic “ # ” details permitted by the VM-error result
specification are accepted, while preserving UTF-8 and trie-path validation for
the base code and parameters. Align the corresponding validator behavior and the
VM-error result specification so leaders and validators classify detailed VM
errors consistently.
In `@docs/website/src/spec/03-vm/05-result.rst`:
- Around line 65-70: Revise the “Effects of a Non-Returning Run” statement to
limit the no-effects rule to top-level execution-result reporting: only a
successful top-level Return reports storage_changes and emissions. Preserve the
clarification that recoverable VM or user errors may leave sandbox writes
unreverted, while their top-level result still reports empty effect fields and
the execution hash covers those fields.
In `@implementation/src/manager/handlers.rs`:
- Around line 92-111: Update handle_genvm_run’s queued-event wait to use a
bounded timeout around events.changed(). If the deadline expires while the run
remains queued, return an accepted response indicating the run is queued instead
of keeping the HTTP request open; preserve the existing startup failure handling
and started response when an event arrives before the deadline.
In `@implementation/src/manager/mod.rs`:
- Around line 143-165: Introduce a dedicated client-error rejection helper near
internal_error that returns HTTP 400 and logs at debug level, then update the
call sites for parse_json_body, parse_query, and deployment_timestamp to route
their failures through it instead of internal_error. Preserve internal_error for
genuine server failures.
In `@implementation/src/manager/run.rs`:
- Around line 1272-1286: Update read_length_prefixed so it does not allocate the
entire declared payload before receiving bytes: read the payload incrementally
in bounded-size chunks, or enforce a lower configured cap at both call sites
instead of passing u32::MAX. Preserve the existing length-prefix validation and
return the complete payload only after all chunks have been read.
In `@implementation/src/manager/socket.rs`:
- Around line 539-546: Update the response sends in the async handlers
handle_run, handle_attach, handle_cancel, and handle_ack to call
send_control_async instead of send_control, awaiting each call and preserving
existing error propagation and response ordering.
- Around line 582-589: In handle_attach, send the Methods::Attach response
through self.writer.send_control before calling self.subscribe(req.genvm_id,
Ok(rx))?. Preserve the existing snapshot_payload(snapshot) response contents and
subscription behavior, matching the ordering established in handle_run.
- Around line 756-770: Bound the writer shutdown wait by changing writer_task to
mutable and racing &mut writer_task.await against cancel.chan.closed() after the
connection select; if cancellation wins, abort writer_task, while preserving the
existing result/error logging for completed writer tasks.
In `@implementation/src/scripting/mod.rs`:
- Around line 165-166: Remove StdLib::OS from the UserVM constructor’s
standard-library configuration while retaining the other required libraries.
Preserve sandbox behavior by exposing only approved functionality through
Rust-wrapped globals, and add a regression test verifying Lua cannot access OS
functions such as os.execute, os.exit, os.getenv, or os.remove.
---
Outside diff comments:
In `@implementation/src/manager/run.rs`:
- Around line 256-284: Update SingleGenVMContextDone::serialize to include both
cause and exit_code in the serialized output, and adjust the declared field
count accordingly so GET responses expose the same termination details as
Event::Finished.
In `@install/lib/python/post-install/__main__.py`:
- Around line 582-593: Make legacy runner integrity validation unconditional in
the download flow around download_runners_from_json: when runner downloads are
enabled, always run the legacy executor’s check command, including when
args.precompile is false, or perform equivalent Nix-base32 verification before
writing the archive. Preserve the existing hash verification for non-legacy
runners.
---
Minor comments:
In @.claude/skills/test/SKILL.md:
- Line 10: Update the opening code fence in SKILL.md to specify the shell
language, changing the unlabeled fence to a shell-labeled fence while preserving
the fenced command block contents.
In `@docs/adr/014`. manager host socket protocol.md:
- Around line 46-48: Update the wire-format code fence containing the
method_id/request_id layout to declare the text language, resolving the MD040
language requirement without changing the protocol content.
In `@docs/contributing/howto/building/docs.md`:
- Line 33: Update the generated-output table in the documentation to include
constants-pending.rst and internal-constants.rst alongside the existing
constants.rst and manager-socket-consts.rst entries, preserving the codegen
reference and link formatting.
In `@docs/website/src/impl-spec/01-core-architecture/01-components.rst`:
- Line 119: Update the runner identity statement near the built-in runner
description to apply only to the forward-rolling format, or explicitly document
both ZIP-content hashes and the frozen v0.2.x ustar TAR/Nix base32 scheme.
Preserve the existing contract accessibility context while avoiding an
unqualified claim that all built-in runners use ZIP hashes.
In `@docs/website/src/impl-spec/appendix/manager-api.rst`:
- Around line 9-12: Update the deprecated execution-endpoints paragraph to
explicitly state the lifecycle of GET /genvm/{id}/artifact, using the API
specification’s documented behavior: either include it in the one-release
deprecation/removal list or clearly state that it remains afterward as a debug
convenience adapter.
In `@docs/website/src/impl-spec/appendix/manager-socket.rst`:
- Around line 283-289: The deadline documentation incorrectly classifies invalid
ManagerDuration values as malformed_frame. Update the validation/error wording
in the deadline section to state that values rejected by
ManagerDuration::to_std, including empty numbers, multiple decimal points, and
unitless strings, produce an internal handler failure (or the documented startup
configuration error), while preserving the unit and format requirements.
In `@docs/website/src/spec/appendix/internal-constants.rst`:
- Around line 1-2: Rename the document title under the “Constants” heading to
“Internal Constants” so it matches the label in the appendix index and avoids
duplicate or confusing headings.
In `@implementation/src/manager/run.rs`:
- Around line 2134-2138: Update the error handling in supervise_genvm around
supervise_genvm_inner so terminal failures use the start-publication state: call
fail_to_start only when publish_started has not run, and emit the appropriate
post-start failure/termination event when startup was already published. Ensure
post-start errors such as child.wait failures or a missing process handle
preserve the Started-then-terminal event contract and include the available exit
code and artifact sizes.
In `@implementation/src/manager/socket.rs`:
- Around line 265-273: Update the serde_json::Value::Number handling in the
socket encoder to preserve non-integer numbers instead of falling back to
push_null: encode the floating-point value when supported, otherwise encode its
decimal string representation. Document the chosen wire representation in
manager-socket.rst so clients can distinguish it from a missing metric.
In `@install/config/genvm-manager.yaml`:
- Around line 11-12: Update the manager schema referenced by the genvm-manager
configuration to define execution_retention and max_message_bytes with the
accepted types and validation shape used by the configuration. Preserve the
existing manifest_path and permits definitions, and ensure the schema source
linked by the manager configuration is the one containing both new fields.
In `@SECURITY.md`:
- Around line 43-48: Update the trusted-relationships section in SECURITY.md to
explicitly require OS account and filesystem permission protections for the
loopback-only manager, its socket, and manifest/data paths when the host is not
trusted. Clarify that local disk and loopback trust depends on restricting
access to authorized local users and processes.
In `@support/runner-script.py`:
- Around line 95-100: Rename the hash parameters in check_bytes,
_object_gcs_path, _download_single, _nix_preload, and _upload_single to hash_id
or archive_hash, and update all references and call sites consistently while
preserving behavior.
---
Nitpick comments:
In `@docs/website/src/impl-spec/appendix/manager-socket.rst`:
- Around line 61-63: Revise the payload-format statement in the manager socket
specification to apply only to request and event payloads with external variant
tags, not all payloads; preserve the separate response shapes such as genvm_id
and total_len/data. Widen the metrics field type to any JSON value supported by
encode_json_value, including null, booleans, numbers, strings, arrays, and
objects. Keep the existing Sphinx labels on manager-socket-consts and do not add
duplicate labels.
In `@implementation/src/manager/mod.rs`:
- Around line 185-223: Extract the repeated closure logic from the module route
registrations into a generic async helper, such as json_body_route, that parses
Bytes, maps parse failures to the established error response, and invokes the
supplied handler through unwrap_all_anyhow. Update the /module/start,
/module/stop, /module/restart, and the other matching JSON routes to pass their
handlers to this helper while preserving their existing response types and error
behavior.
In `@implementation/src/manager/socket_test.rs`:
- Around line 128-239: Add focused tests for the documented frame-level
invariants using read_frame, dispatch, and handle_get_artifact. Cover rejection
of messages shorter than HEADER_LEN and unknown method_id while keeping the
connection usable, dispatch returning bad_request_id for request_id == 0, and
handle_get_artifact clamping max_len to ARTIFACT_CHUNK_CAP. Use
futures_util::stream::iter with test_writer where applicable, and assert the
expected protocol responses and connection behavior.
In `@implementation/src/manager/socket.rs`:
- Around line 102-110: Update the writer loop around the artifact_rx branch to
track whether artifact_rx has closed and stop polling it afterward, while
continuing to service control_rx until it closes. Preserve the existing
frame-writing behavior and clean shutdown when both channels are closed, without
relying on the senders closing together.
- Around line 571-575: Replace the string-based error checks in the socket
request handling with typed discrimination for failures from attach and
get_artifact. Define or reuse a dedicated error enum/result type in run.rs,
propagate those typed variants through both operations, and match on the
variants instead of e.to_string() so BootIdMismatch and UnknownId map to the
correct Errors values even when context is added.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: f56f3a75-5b3c-4e43-af05-b200fa197268
⛔ Files ignored due to path filters (39)
crates/modules-interfaces/Cargo.lockis excluded by!**/*.lock,!**/*.lockimplementation/Cargo.lockis excluded by!**/*.lock,!**/*.locksupport/tools/genvm-tool/genvm_tool/cmd_configure.pyis excluded by!support/tools/genvm-tool/**support/tools/genvm-tool/genvm_tool/codegen/go.pyis excluded by!support/tools/genvm-tool/**support/tools/genvm-tool/genvm_tool/codegen/model.pyis excluded by!support/tools/genvm-tool/**support/tools/genvm-tool/genvm_tool/codegen/python.pyis excluded by!support/tools/genvm-tool/**support/tools/genvm-tool/genvm_tool/codegen/rst.pyis excluded by!support/tools/genvm-tool/**support/tools/genvm-tool/genvm_tool/codegen/rust.pyis excluded by!support/tools/genvm-tool/**support/tools/genvm-tool/genvm_tool/tests/__init__.pyis excluded by!support/tools/genvm-tool/**support/tools/genvm-tool/genvm_tool/tests/exec/process.pyis excluded by!support/tools/genvm-tool/**support/tools/genvm-tool/genvm_tool/tests/exec/service.pyis excluded by!support/tools/genvm-tool/**support/tools/genvm-tool/genvm_tool/tests/stage/collection.pyis excluded by!support/tools/genvm-tool/**support/tools/genvm-tool/genvm_tool/tests/stage/execution.pyis excluded by!support/tools/genvm-tool/**support/tools/genvm-tool/genvm_tool/tests/stage/filter.pyis excluded by!support/tools/genvm-tool/**support/tools/genvm-tool/genvm_tool/tests/tags.pyis excluded by!support/tools/genvm-tool/**support/tools/genvm-tool/genvm_tool/tests/util/watchdog.pyis excluded by!support/tools/genvm-tool/**support/tools/genvm-tool/unit_tests/test_codegen_detail_suffix.pyis excluded by!support/tools/genvm-tool/**support/tools/genvm-tool/unit_tests/test_process_teardown.pyis excluded by!support/tools/genvm-tool/**support/tools/genvm-tool/unit_tests/test_service_startup.pyis excluded by!support/tools/genvm-tool/**support/tools/genvm-tool/unit_tests/test_watchdog_ack.pyis excluded by!support/tools/genvm-tool/**tests/runner/genvm_tool_plugins/cargo.pyis excluded by!tests/**tests/runner/genvm_tool_plugins/docker.pyis excluded by!tests/**tests/runner/genvm_tool_plugins/genvm.pyis excluded by!tests/**tests/runner/genvm_tool_plugins/integration.pyis excluded by!tests/**tests/runner/genvm_tool_plugins/ninja.pyis excluded by!tests/**tests/runner/gvm_extra/mock_host.pyis excluded by!tests/**tests/runner/origin/base_host.pyis excluded by!tests/**tests/runner/origin/host_fns.pyis excluded by!tests/**tests/runner/origin/log_asserts.pyis excluded by!tests/**tests/runner/origin/manager_api.pyis excluded by!tests/**tests/runner/origin/public_abi.pyis excluded by!tests/**tests/system/cross-major-observability/test.pyis excluded by!tests/**tests/system/cross-major/test.pyis excluded by!tests/**tests/system/make_zip/test.pyis excluded by!tests/**tests/system/manager-socket/test.pyis excluded by!tests/**tests/system/parse_version/test.pyis excluded by!tests/**tests/system/permits/test.pyis excluded by!tests/**tests/tags.jsonis excluded by!tests/**tests/templates/simple_deploy_then_write.jsonnetis excluded by!tests/**
📒 Files selected for processing (87)
.claude/skills/agentic-fuzzing/SKILL.md.claude/skills/commit-style/SKILL.md.claude/skills/rust-test-style/SKILL.md.claude/skills/spec/SKILL.md.claude/skills/test/SKILL.md.genvm-monorepo-root.genvm-tool.pyAGENTS.mdCLAUDE.mdCLAUDE.mdSECURITY.mdcrates/modules-interfaces/Cargo.tomlcrates/modules-interfaces/codegen/data/host-fns.jsoncrates/modules-interfaces/codegen/data/manager-api.jsoncrates/modules-interfaces/src/domain.rscrates/modules-interfaces/src/host_fns.rscrates/modules-interfaces/src/lib.rscrates/modules-interfaces/src/manager_api.rscrates/modules-interfaces/src/nested.rsdocs/adr/011. runner ids and runtime registration.mddocs/adr/012. runner memory accounting.mddocs/adr/013. pre-validating untrusted decoded inputs.mddocs/adr/014. manager host socket protocol.mddocs/adr/015. cross-major contract calls.mddocs/contributing/explanation/executor-lines.mddocs/contributing/howto/building/docs.mddocs/contributing/howto/committing/submodules.mddocs/contributing/howto/extending/add-host-function.mddocs/contributing/howto/genvm-tool.mddocs/contributing/howto/testing/integration.mddocs/contributing/howto/testing/rust.mddocs/schemas/runners.jsondocs/website/src/impl-spec/01-core-architecture/01-components.rstdocs/website/src/impl-spec/02-vm/02-version-management.rstdocs/website/src/impl-spec/02-vm/03-consensus.rstdocs/website/src/impl-spec/appendix/host-loop.rstdocs/website/src/impl-spec/appendix/index.rstdocs/website/src/impl-spec/appendix/manager-api.rstdocs/website/src/impl-spec/appendix/manager-api.yamldocs/website/src/impl-spec/appendix/manager-socket-consts.rstdocs/website/src/impl-spec/appendix/manager-socket.rstdocs/website/src/spec/02-execution-environment/03-wasi_genlayer_sdk/02-gl_call.rstdocs/website/src/spec/02-execution-environment/03-wasi_genlayer_sdk/03-schemas.rstdocs/website/src/spec/02-execution-environment/04-runners.rstdocs/website/src/spec/03-vm/01-startup.rstdocs/website/src/spec/03-vm/02-meta-properties.rstdocs/website/src/spec/03-vm/03-ram-limiting.rstdocs/website/src/spec/03-vm/05-result.rstdocs/website/src/spec/04-contract-interface/02-abi.rstdocs/website/src/spec/04-contract-interface/04-upgradability.rstdocs/website/src/spec/appendix/constants-pending.rstdocs/website/src/spec/appendix/constants.rstdocs/website/src/spec/appendix/index.rstdocs/website/src/spec/appendix/internal-constants.rstdocs/website/src/spec/changelog.rstdocs/website/src/spec/index.rstexecutors/v0.2.xexecutors/v0.3.xflake.niximplementation/.ya-test-config.jsonimplementation/Cargo.tomlimplementation/src/manager/handlers.rsimplementation/src/manager/handlers_test.rsimplementation/src/manager/mod.rsimplementation/src/manager/run.rsimplementation/src/manager/run_test.rsimplementation/src/manager/socket.rsimplementation/src/manager/socket_test.rsimplementation/src/manager/versioning.rsimplementation/src/manager/versioning_test.rsimplementation/src/scripting/mod.rsimplementation/tests/dflt_requests.rsimplementation/tests/merge.rsimplementation/tests/overloaded.rsimplementation/tests/policy_dispatch.rsimplementation/tests/providers.rsimplementation/tests/request_body_limit.rsimplementation/tests/request_localhost.rsimplementation/tests/signing_server.rsimplementation/tests/timeout.rsimplementation/tests/web_render_webdriver.rsinstall/config/genvm-manager.yamlinstall/lib/python/post-install/__main__.pyrunners/default.nixrunners/views/universal.nixsupport/manifest-base.yamlsupport/runner-script.py
💤 Files with no reviewable changes (2)
- runners/default.nix
- docs/website/src/spec/appendix/constants-pending.rst
| async fn read_length_prefixed<R: tokio::io::AsyncRead + Unpin>( | ||
| reader: &mut R, | ||
| max_len: usize, | ||
| ) -> anyhow::Result<Vec<u8>> { | ||
| use tokio::io::AsyncReadExt; | ||
|
|
||
| match proc.try_wait() { | ||
| Ok(Some(status)) => { | ||
| log_debug!(id = exec.id, status = status; "genvm exited"); | ||
| let len = reader.read_u32_le().await? as usize; | ||
| anyhow::ensure!( | ||
| len <= max_len, | ||
| "manager host frame is too large: {len} > {max_len}" | ||
| ); | ||
| let mut data = vec![0; len]; | ||
| reader.read_exact(&mut data).await?; | ||
| Ok(data) | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
read_length_prefixed allocates the declared length before any payload arrives.
max_len is u32::MAX at both call sites, and vec![0; len] runs immediately after reading the header. A child that writes a 4 GiB length prefix and then stalls makes the manager allocate 4 GiB at once. The manager holds that allocation for every concurrent execution in this state. The child memory limit does not bound this allocation, because the bytes are never sent.
Read in bounded chunks, or pass a configured cap for each method.
🔧 Proposed fix
let len = reader.read_u32_le().await? as usize;
anyhow::ensure!(
len <= max_len,
"manager host frame is too large: {len} > {max_len}"
);
- let mut data = vec![0; len];
- reader.read_exact(&mut data).await?;
- Ok(data)
+ const CHUNK: usize = 1 << 20;
+ let mut data = Vec::new();
+ let mut remaining = len;
+ while remaining > 0 {
+ let take = remaining.min(CHUNK);
+ let start = data.len();
+ data.resize(start + take, 0);
+ reader.read_exact(&mut data[start..]).await?;
+ remaining -= take;
+ }
+ Ok(data)📝 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.
| async fn read_length_prefixed<R: tokio::io::AsyncRead + Unpin>( | |
| reader: &mut R, | |
| max_len: usize, | |
| ) -> anyhow::Result<Vec<u8>> { | |
| use tokio::io::AsyncReadExt; | |
| match proc.try_wait() { | |
| Ok(Some(status)) => { | |
| log_debug!(id = exec.id, status = status; "genvm exited"); | |
| let len = reader.read_u32_le().await? as usize; | |
| anyhow::ensure!( | |
| len <= max_len, | |
| "manager host frame is too large: {len} > {max_len}" | |
| ); | |
| let mut data = vec![0; len]; | |
| reader.read_exact(&mut data).await?; | |
| Ok(data) | |
| } | |
| async fn read_length_prefixed<R: tokio::io::AsyncRead + Unpin>( | |
| reader: &mut R, | |
| max_len: usize, | |
| ) -> anyhow::Result<Vec<u8>> { | |
| use tokio::io::AsyncReadExt; | |
| let len = reader.read_u32_le().await? as usize; | |
| anyhow::ensure!( | |
| len <= max_len, | |
| "manager host frame is too large: {len} > {max_len}" | |
| ); | |
| const CHUNK: usize = 1 << 20; | |
| let mut data = Vec::new(); | |
| let mut remaining = len; | |
| while remaining > 0 { | |
| let take = remaining.min(CHUNK); | |
| let start = data.len(); | |
| data.resize(start + take, 0); | |
| reader.read_exact(&mut data[start..]).await?; | |
| remaining -= take; | |
| } | |
| Ok(data) | |
| } |
🤖 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 `@implementation/src/manager/run.rs` around lines 1272 - 1286, Update
read_length_prefixed so it does not allocate the entire declared payload before
receiving bytes: read the payload incrementally in bounded-size chunks, or
enforce a lower configured cap at both call sites instead of passing u32::MAX.
Preserve the existing length-prefix validation and return the complete payload
only after all chunks have been read.
6926c73 to
bf93912
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
implementation/src/manager/mod.rs (1)
373-375: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftRestore a bounded HTTP request-body limit.
DefaultBodyLimit::disable()removes the size limit for every route that extractsBytes, including/module/*,/genvm/run,/contract/detect-version,/log/level, and/llm/check.max_message_bytescovers only the WebSocket route.
Add a configurable HTTP body-size limit and apply it withDefaultBodyLimit::max(...). If contract code or calldata need a larger limit, apply that larger bound only to the specific routes that require it.🤖 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 `@implementation/src/manager/mod.rs` around lines 373 - 375, Replace the global DefaultBodyLimit::disable() in the router setup with DefaultBodyLimit::max(...) using a configurable HTTP request-body limit. Keep max_message_bytes scoped to the WebSocket path, and apply any larger contract code or calldata limit only to the specific routes that require it rather than globally.
🤖 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.
Outside diff comments:
In `@implementation/src/manager/mod.rs`:
- Around line 373-375: Replace the global DefaultBodyLimit::disable() in the
router setup with DefaultBodyLimit::max(...) using a configurable HTTP
request-body limit. Keep max_message_bytes scoped to the WebSocket path, and
apply any larger contract code or calldata limit only to the specific routes
that require it rather than globally.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: ce0266b6-8c62-464d-a9c3-1694f60c1b84
📒 Files selected for processing (7)
.github/copilot-instructions.mddocs/website/src/impl-spec/appendix/manager-api.yamlexecutors/v0.3.ximplementation/src/manager/mod.rsimplementation/src/manager/socket.rsimplementation/src/scripting/ctx/dflt.rssupport/ci/tools/open_executor_prs.py
🚧 Files skipped from review as they are similar to previous changes (3)
- executors/v0.3.x
- implementation/src/manager/socket.rs
- docs/website/src/impl-spec/appendix/manager-api.yaml
e2ff747 to
35dd121
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
support/runner-script.py (1)
143-156: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winHandle v0.2.x hashes in runner-script.py before advertising
tar.
docs/contributing/howto/genvm-tool.mdsays v0.2.x frozen runner installers should use this script with--archive-mode tar.support/runner-script.pystill rejects legacy registry hashes by design becausecheck_bytes()only accepts Crockford-encoded hashes, while the installer bypasses check/hash validation for v0.2.x legacy lines. Add an explicit legacy hash path/bypass, or remove the v0.2.xtarmode from the documented command until it matches the installer behavior.🤖 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 `@support/runner-script.py` around lines 143 - 156, Add an explicit v0.2.x legacy-hash path in the runner flow before check_bytes() validates cur_dst, matching the installer’s bypass behavior when archive mode is tar. Preserve existing Crockford hash validation for current registry entries, and ensure legacy downloads still use _download_single() without being rejected as corrupted.
🤖 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.
Outside diff comments:
In `@support/runner-script.py`:
- Around line 143-156: Add an explicit v0.2.x legacy-hash path in the runner
flow before check_bytes() validates cur_dst, matching the installer’s bypass
behavior when archive mode is tar. Preserve existing Crockford hash validation
for current registry entries, and ensure legacy downloads still use
_download_single() without being rejected as corrupted.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 98abab33-c634-4b4b-9d94-9760c7b97148
⛔ Files ignored due to path filters (41)
support/tools/genvm-tool/genvm_tool/__init__.pyis excluded by!support/tools/genvm-tool/**support/tools/genvm-tool/genvm_tool/__main__.pyis excluded by!support/tools/genvm-tool/**support/tools/genvm-tool/genvm_tool/cmd_build_manifest.pyis excluded by!support/tools/genvm-tool/**support/tools/genvm-tool/genvm_tool/cmd_codegen.pyis excluded by!support/tools/genvm-tool/**support/tools/genvm-tool/genvm_tool/cmd_configure.pyis excluded by!support/tools/genvm-tool/**support/tools/genvm-tool/genvm_tool/cmd_docs.pyis excluded by!support/tools/genvm-tool/**support/tools/genvm-tool/genvm_tool/cmd_test.pyis excluded by!support/tools/genvm-tool/**support/tools/genvm-tool/genvm_tool/codegen/__init__.pyis excluded by!support/tools/genvm-tool/**support/tools/genvm-tool/genvm_tool/codegen/go.pyis excluded by!support/tools/genvm-tool/**support/tools/genvm-tool/genvm_tool/codegen/model.pyis excluded by!support/tools/genvm-tool/**support/tools/genvm-tool/genvm_tool/codegen/python.pyis excluded by!support/tools/genvm-tool/**support/tools/genvm-tool/genvm_tool/codegen/rst.pyis excluded by!support/tools/genvm-tool/**support/tools/genvm-tool/genvm_tool/codegen/rust.pyis excluded by!support/tools/genvm-tool/**support/tools/genvm-tool/genvm_tool/common.pyis excluded by!support/tools/genvm-tool/**support/tools/genvm-tool/genvm_tool/formatter.pyis excluded by!support/tools/genvm-tool/**support/tools/genvm-tool/genvm_tool/git/check_for_push.pyis excluded by!support/tools/genvm-tool/**support/tools/genvm-tool/genvm_tool/git/create_branches.pyis excluded by!support/tools/genvm-tool/**support/tools/genvm-tool/genvm_tool/git/verify_pushed_executors.pyis excluded by!support/tools/genvm-tool/**support/tools/genvm-tool/genvm_tool/io.pyis excluded by!support/tools/genvm-tool/**support/tools/genvm-tool/genvm_tool/manifest.pyis excluded by!support/tools/genvm-tool/**support/tools/genvm-tool/genvm_tool/tests/__init__.pyis excluded by!support/tools/genvm-tool/**support/tools/genvm-tool/genvm_tool/tests/cli.pyis excluded by!support/tools/genvm-tool/**support/tools/genvm-tool/genvm_tool/tests/stage/collection.pyis excluded by!support/tools/genvm-tool/**support/tools/genvm-tool/genvm_tool/tests/stage/configuration.pyis excluded by!support/tools/genvm-tool/**support/tools/genvm-tool/genvm_tool/tests/tags.pyis excluded by!support/tools/genvm-tool/**support/tools/genvm-tool/genvm_tool/tests/util/watchdog.pyis excluded by!support/tools/genvm-tool/**tests/runner/genvm_tool_plugins/cargo.pyis excluded by!tests/**tests/runner/genvm_tool_plugins/genvm.pyis excluded by!tests/**tests/runner/genvm_tool_plugins/integration.pyis excluded by!tests/**tests/runner/genvm_tool_plugins/ninja.pyis excluded by!tests/**tests/runner/genvm_tool_plugins/pytest.pyis excluded by!tests/**tests/runner/genvm_tool_plugins/runners.pyis excluded by!tests/**tests/runner/origin/base_host.pyis excluded by!tests/**tests/runner/origin/fees.pyis excluded by!tests/**tests/runner/origin/log_asserts.pyis excluded by!tests/**tests/system/cross-major-observability/test.pyis excluded by!tests/**tests/system/cross-major/test.pyis excluded by!tests/**tests/system/make_zip/test.pyis excluded by!tests/**tests/system/manager-socket/test.pyis excluded by!tests/**tests/system/parse_version/test.pyis excluded by!tests/**tests/system/permits/test.pyis excluded by!tests/**
📒 Files selected for processing (22)
.genvm-tool.py.github/workflows/incl_release_build_test_cell_build.yamlcrates/modules-interfaces/src/domain/fees/mod.rscrates/modules-interfaces/src/host_fns.rscrates/modules-interfaces/src/manager_api.rsdocs/website/generate.pydocs/website/merge_txts.pyexecutors/v0.2.xexecutors/v0.3.ximplementation/src/lib.rsimplementation/src/llm/handler.rsimplementation/src/manager/socket.rsimplementation/src/manager/socket_test.rsinstall/bin/genvm-post-installinstall/lib/python/post-install/__main__.pyruff.tomlsupport/runner-script.pysupport/scripts/check-commit-message.pysupport/scripts/check-source-text.pysupport/scripts/ci-changes.pysupport/scripts/get-all-git.pysupport/scripts/md-local-links.py
🚧 Files skipped from review as they are similar to previous changes (8)
- executors/v0.3.x
- executors/v0.2.x
- crates/modules-interfaces/src/host_fns.rs
- implementation/src/manager/socket_test.rs
- crates/modules-interfaces/src/manager_api.rs
- implementation/src/manager/socket.rs
- .genvm-tool.py
- install/lib/python/post-install/main.py
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (1)
support/tools/fuzz-preload/src/lib.rs (1)
50-56: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd a focused test for the intentional ABI divergence.
The fuzz-only preload documents and intentionally permits lengths above 256 bytes. Test this behavior and the successful return value.
🤖 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 `@support/tools/fuzz-preload/src/lib.rs` around lines 50 - 56, Add a focused test for the exported unsafe extern "C" function getentropy that passes a length greater than 256, verifies the buffer is filled according to fill, and asserts the function returns 0. Keep the test scoped to this intentional fuzz-preload ABI 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 @.claude/settings.local.json:
- Line 2: Remove the tracked .claude/settings.local.json file from the commit
and preserve the local-only settings convention. If “outputStyle”: “Brief Style”
should be shared by the team, add it to .claude/settings.json instead.
In `@docs/contributing/explanation/fuzz.md`:
- Around line 16-20: Update the loader/LD_PRELOAD limitation in the fuzzing
documentation to apply only to statically linked targets, not all musl builds.
Preserve that dynamically linked musl targets are preload-compatible, including
the repository’s default non-static build flags, and add documentation and tests
for any specific musl randomness path the shim cannot intercept.
In `@docs/contributing/howto/testing/fuzzing.md`:
- Around line 24-34: Update the corpus documentation to state that verification
is skipped when cargo-afl or the fuzz target is unavailable, while --no-verify
still applies the maximum-size and host-path filters. Document that
--fuzz-update-corpus runs cargo-afl afl cmin -T all and replaces the committed
corpus with the resulting minimized inputs.
In `@docs/website/src/spec/02-execution-environment/02-wasip1.rst`:
- Around line 206-208: Update the WASI metadata descriptions for
path_filestat_get and fd_filestat_get so Filestat.nlink uses the same value in
both paths. Choose a single consistent value and apply it wherever the shared
file metadata is specified, preserving the existing filetype behavior.
- Around line 164-175: Update the path-walking behavior described for ``path_*``
functions so an intermediate regular-file component returns ``Notdir`` instead
of ``Badf``. Preserve ``Badf`` specifically for a non-directory ``dirfd``.
- Around line 321-325: Update the descriptor-rights table by removing readdir,
readlink, and path_open from the regular_file rights, and removing readlink from
the directory rights; preserve the remaining rights and stdout/stderr entry
unchanged.
- Around line 265-267: Update the fd_prestat_dir_name documentation to specify
that it returns the full guest-visible path prefix supplied when the directory
was preopened, including nested paths such as /foo/bar, and that pr_name_len
reflects the full path length. Revise both affected descriptions consistently
and add a test covering a nested preopen path.
- Around line 191-196: Update the path_open documentation to describe capability
enforcement instead of stating that oflags and rights are ignored: document
returning Notdir when oflags::directory targets a regular file, and limiting
base and inheriting rights by both the requested rights and the parent
descriptor’s inheriting rights, with further reduction for unsupported
target-type rights.
In
`@docs/website/src/spec/02-execution-environment/03-wasi_genlayer_sdk/02-gl_call.rst`:
- Around line 653-655: Update the deterministic-mode return-value statement near
the gl_call documentation to preserve the documented unsafe-tracing exception:
state that it returns exactly 0 unless unsafe-tracing permits real elapsed time.
Keep the existing “call never fails” behavior and references unchanged.
- Around line 678-680: Update the `Yield` description in the GL call
documentation to remove the “no-op” wording and state that it may have the
implementation-defined yielding side effect described by
`gvm-def-gl-call-observable-discretion`, while preserving that the call returns
no value and always succeeds.
In `@executors/v0.2.x`:
- Line 1: Update the executor integration-test setup to check out and run
against pinned commit a74f7cfc179ea2d9ac66a0815deec1a6cc89ab44 instead of the
older base commit 1c8126d6efb6cab9fdce02b1745217fc571a343b, ensuring the updated
executor code and fixtures are included.
---
Nitpick comments:
In `@support/tools/fuzz-preload/src/lib.rs`:
- Around line 50-56: Add a focused test for the exported unsafe extern "C"
function getentropy that passes a length greater than 256, verifies the buffer
is filled according to fill, and asserts the function returns 0. Keep the test
scoped to this intentional fuzz-preload ABI 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 5995ee25-907e-4bbe-ae4c-db09f1af4863
⛔ Files ignored due to path filters (13)
support/tools/fuzz-preload/Cargo.lockis excluded by!**/*.lock,!**/*.locksupport/tools/genvm-tool/genvm_tool/__init__.pyis excluded by!support/tools/genvm-tool/**support/tools/genvm-tool/genvm_tool/__main__.pyis excluded by!support/tools/genvm-tool/**support/tools/genvm-tool/genvm_tool/cmd_fuzz_corpus.pyis excluded by!support/tools/genvm-tool/**support/tools/genvm-tool/genvm_tool/common.pyis excluded by!support/tools/genvm-tool/**support/tools/genvm-tool/genvm_tool/misc.pyis excluded by!support/tools/genvm-tool/**support/tools/genvm-tool/genvm_tool/tests/cli.pyis excluded by!support/tools/genvm-tool/**support/tools/genvm-tool/unit_tests/test_fuzz_corpus.pyis excluded by!support/tools/genvm-tool/**tests/runner/genvm_tool_plugins/cargo.pyis excluded by!tests/**tests/runner/genvm_tool_plugins/integration.pyis excluded by!tests/**tests/runner/genvm_tool_plugins/pytest.pyis excluded by!tests/**tests/runner/gvm_extra/case_inputs.pyis excluded by!tests/**tests/tags.jsonis excluded by!tests/**
📒 Files selected for processing (17)
.claude/settings.local.json.genvm-tool.pyAGENTS.mddocs/contributing/explanation/README.mddocs/contributing/explanation/fuzz.mddocs/contributing/howto/README.mddocs/contributing/howto/genvm-tool.mddocs/contributing/howto/testing/fuzzing.mddocs/website/src/spec/02-execution-environment/01-wasm.rstdocs/website/src/spec/02-execution-environment/02-wasip1.rstdocs/website/src/spec/02-execution-environment/03-wasi_genlayer_sdk/02-gl_call.rstdocs/website/src/spec/02-execution-environment/04-runners.rstdocs/website/src/spec/03-vm/05-result.rstexecutors/v0.2.xexecutors/v0.3.xsupport/tools/fuzz-preload/Cargo.tomlsupport/tools/fuzz-preload/src/lib.rs
🚧 Files skipped from review as they are similar to previous changes (6)
- .genvm-tool.py
- executors/v0.3.x
- AGENTS.md
- docs/contributing/howto/genvm-tool.md
- docs/website/src/spec/02-execution-environment/04-runners.rst
- docs/website/src/spec/03-vm/05-result.rst
| Writes the name reported by ``fd_prestat_get`` — the last component of the | ||
| preopened directory's path, or ``/`` for the root. A buffer shorter than that | ||
| name fails with ``Overflow``; any non-directory descriptor fails with ``Badf``. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
curl -fsSL https://docs.rs/crate/wasip1/1.0.0/source/wasi_snapshot_preview1.witx |
grep -n -E 'preopened directory name|fd_prestat_dir_name'Repository: genlayerlabs/genvm-manager
Length of output: 299
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- contributing documentation ---'
fd -i . docs/contributing 2>/dev/null | head -80
printf '%s\n' '--- target context ---'
sed -n '235,280p' docs/website/src/spec/02-execution-environment/02-wasip1.rst
printf '%s\n' '--- relevant repository references ---'
rg -n -C 3 'fd_prestat_dir_name|prestat|preopened directory|last component|path_open' docs/website/src/spec/02-execution-environment/02-wasip1.rst docs 2>/dev/null | head -240
printf '%s\n' '--- WASI Preview 1 source ---'
curl -fsSL https://raw.githubusercontent.com/WebAssembly/WASI/main/legacy/preview1/witx/wasi_snapshot_preview1.witx |
sed -n '930,975p'Repository: genlayerlabs/genvm-manager
Length of output: 10585
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- documentation contribution guidance ---'
sed -n '1,220p' docs/contributing/howto/docs/style.md
sed -n '1,180p' docs/contributing/explanation/docs-layout.md
printf '%s\n' '--- docs.rs source context ---'
curl -fsSL https://docs.rs/crate/wasip1/1.0.0/source/wasi_snapshot_preview1.witx |
sed -n '940,970p'
printf '%s\n' '--- candidate upstream sources ---'
for url in \
https://raw.githubusercontent.com/WebAssembly/WASI/main/legacy/preview1/witx/wasi_snapshot_preview1.witx \
https://raw.githubusercontent.com/WebAssembly/WASI/main/legacy/preview1/witx/wasi_snapshot_preview1.witx?raw=1 \
https://raw.githubusercontent.com/WebAssembly/wasi-libc/main/ wasi_snapshot_preview1.witx
do
printf '\nURL: %s\n' "$url"
curl -LfsS "$url" 2>/dev/null | rg -n -C 8 'fd_prestat_dir_name|pre-opened directory|preopened directory' | head -80 || true
done
printf '%s\n' '--- repository implementation and tests ---'
rg -n -C 5 'fd_prestat_dir_name|pr_name_len|preopen' --glob '!docs/website/src/spec/02-execution-environment/02-wasip1.rst' .Repository: genlayerlabs/genvm-manager
Length of output: 4968
🌐 Web query:
WASI Preview 1 fd_prestat_dir_name nested preopen path full name basename pr_name_len semantics
💡 Result:
In WASI Preview 1 (wasip1), the fd_prestat_dir_name function is used to retrieve the path prefix associated with a preopened file descriptor [1][2]. The semantics for the function parameters are as follows: 1. fd: The file descriptor to query, which must be a preopened directory [1][3]. 2. path: A buffer provided by the caller to store the resulting directory name string [1][2]. 3. path_len: The size of the buffer provided. The WASI specification requires that this matches the pr_name_len value obtained previously from the fd_prestat_get function for that specific file descriptor [4][5][3]. Regarding the path name semantics: - The path returned by fd_prestat_dir_name is the path used when the environment originally preopened that directory [5][6]. - It does not automatically include a trailing null terminator; implementations should be aware that the returned data is the raw byte sequence of the path, and it is the responsibility of the caller to handle null-termination if required (e.g., by allocating pr_name_len + 1 and manually adding a null byte) [5]. - Nested Preopens: WASI does not define a formal hierarchy or "overlay mount" table for nested preopens [7][6]. If a guest environment preopens both /host/a and /host/a/b, these are treated as two independent directory capabilities [6]. When a guest runtime (like wasi-libc) resolves an absolute path, it typically performs a prefix match against the available preopens to select the most specific one [6]. However, once a directory is opened (e.g., opening /), subsequent operations performed relative to that directory descriptor do not re-consult the preopen list, which can lead to ambiguities in POSIX path equivalence [6]. For developers implementing or interacting with these APIs, it is generally recommended to avoid deep nesting of preopens to prevent these resolution ambiguities, preferring instead to preopen the base directory and access subdirectories relative to it [8].
Citations:
- 1: https://docs.rs/wasip1/latest/wasip1/fn.fd_prestat_dir_name.html
- 2: https://wasix.org/docs/api-reference/wasi/fd_prestat_dir_name
- 3: https://github.com/WebAssembly/WASI/blob/main/legacy/preview1/docs.md
- 4: https://github.com/WebAssembly/WASI/blob/wasi-0.1/preview1/docs.md
- 5: https://github.com/WebAssembly/wasi-libc/blob/wasi-sdk-27/libc-bottom-half/sources/preopens.c
- 6: WASIp1 semantic ambiguity with overlapping preopened directories can break POSIX path equivalence bytecodealliance/wasmtime#13544
- 7: Overlapping preopens nodejs/uvwasi#111
- 8: https://hackmd.io/@q/wasm-wasi-realpath-issue
🌐 Web query:
wasi-libc fd_prestat_dir_name preopen discovery nested directory name
💡 Result:
In the WASI (specifically WASIp1) environment, the fd_prestat_dir_name function is used by libc-like layers (such as wasi-libc) during process startup to discover the paths associated with preopened file descriptors [1][2]. Regarding your query on nested directories and directory names: 1. Discovery Mechanism: At program startup, wasi-libc iterates through file descriptors starting from 3 [1][2]. It calls fd_prestat_get to determine if an FD is a preopened directory (WASI_PREOPENTYPE_DIR) [1][2]. If it is, the code then calls fd_prestat_dir_name to retrieve the guest-visible path associated with that FD [1][2]. This path is then used by the C runtime to map absolute paths (e.g., /etc/config) to the correct preopened FD [3][4][5]. 2. Nested Directory Names: The fd_prestat_dir_name function returns the path prefix that was explicitly provided to the WASI runtime when the directory was preopened [4][5]. It does not automatically discover or provide the names of subdirectories nested inside a preopened directory [3][4]. If a user preopens a host directory /host/app as the guest path /app, fd_prestat_dir_name will report /app [3]. The guest-side runtime must then use standard filesystem operations (like openat or readdir) relative to that FD to traverse or discover nested directories [4][6]. 3. Implementation Details: - The buffer returned by fd_prestat_dir_name is typically raw bytes [7][8]. - wasi-libc explicitly allocates memory for these names and adds a trailing null character (\0) to make them C-style strings [1][2]. Whether the WASI specification requires this trailing null is a known point of ambiguity; however, wasi-libc handles this by ensuring it manually adds the terminator [9]. - If multiple directories are preopened (e.g., / and /lib), the guest runtime uses these names to decide which FD to use for absolute path resolution, typically employing a longest-prefix match strategy [4][5]. In summary, fd_prestat_dir_name is limited to reporting the specific preopen path provided to the host runtime and does not facilitate the discovery of an arbitrary nested directory structure [3][4][5].
Citations:
- 1: https://github.com/WebAssembly/wasi-libc/blob/215adc8ac9f91eb055311acc72683fd2eb1ae15a/libc-bottom-half/sources/preopens.c
- 2: https://github.com/WebAssembly/wasi-libc/blob/wasi-sdk-27/libc-bottom-half/sources/preopens.c
- 3: https://hackmd.io/@q/wasm-wasi-realpath-issue
- 4: WASIp1 semantic ambiguity with overlapping preopened directories can break POSIX path equivalence bytecodealliance/wasmtime#13544
- 5: https://github.com/lbe/wasm2go-wasi-host/blob/main/docs/path-confinement.md
- 6: https://github.com/WebAssembly/wasi-filesystem/blob/main/path-resolution.md
- 7: https://docs.rs/wasip1/latest/wasip1/fn.fd_prestat_dir_name.html
- 8: Path bytes returned from
wasi::fd_prestat_dir_nameappended by0wasmerio/wasmer#3025 - 9: Does Function that return buffer with string should include trailling 0 or not WebAssembly/WASI#492
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- Preview 1 documentation ---'
for url in \
https://raw.githubusercontent.com/WebAssembly/WASI/main/legacy/preview1/docs.md \
https://raw.githubusercontent.com/WebAssembly/WASI/wasi-0.1/preview1/docs.md
do
printf '\nURL: %s\n' "$url"
curl -LfsS "$url" |
rg -n -C 12 'fd_prestat_dir_name|preopened directory name|pr_name_len|pre-opened directory' |
head -160 || true
done
printf '%s\n' '--- wasi-libc preopen discovery ---'
for url in \
https://raw.githubusercontent.com/WebAssembly/wasi-libc/main/libc-bottom-half/sources/preopens.c \
https://raw.githubusercontent.com/WebAssembly/wasi-libc/wasi-sdk-27/libc-bottom-half/sources/preopens.c
do
printf '\nURL: %s\n' "$url"
curl -LfsS "$url" |
rg -n -C 12 'fd_prestat_dir_name|pr_name_len|preopen' |
head -200 || true
doneRepository: genlayerlabs/genvm-manager
Length of output: 16360
🏁 Script executed:
#!/bin/bash
set -euo pipefail
url=https://raw.githubusercontent.com/WebAssembly/wasi-libc/wasi-sdk-27/libc-bottom-half/sources/preopens.c
tmp=$(mktemp)
trap 'rm -f "$tmp"' EXIT
curl -LfsS "$url" > "$tmp"
printf '%s\n' '--- preopen population ---'
rg -n -C 18 'fd_prestat_get|fd_prestat_dir_name|__wasilibc_register_preopened_fd|pr_name_len' "$tmp"Repository: genlayerlabs/genvm-manager
Length of output: 3467
Return the full guest-visible preopen path
fd_prestat_dir_name must return the path prefix supplied when the directory was preopened. For /foo/bar, returning only bar breaks wasi-libc preopen discovery and makes pr_name_len incorrect. Update both descriptions and add a nested-preopen test.
🤖 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 `@docs/website/src/spec/02-execution-environment/02-wasip1.rst` around lines
265 - 267, Update the fd_prestat_dir_name documentation to specify that it
returns the full guest-visible path prefix supplied when the directory was
preopened, including nested paths such as /foo/bar, and that pr_name_len
reflects the full path length. Revise both affected descriptions consistently
and add a test covering a nested preopen path.
c87b7dc to
3c34c3c
Compare
be0e15a to
30ca1ee
Compare
30ca1ee to
b8d876a
Compare
b8d876a to
80c2694
Compare
2753c4a to
c1cab0c
Compare
MuncleUscles
left a comment
There was a problem hiding this comment.
Reviewed the substantive SDK/API, runner-archive, lifecycle, and fatal-error changes. Looks good overall.
| rather than prevented, since the two share the one switch. | ||
| """ | ||
| found: list[str] = [] | ||
| for log in sorted(out_dir.glob('*.log')): |
There was a problem hiding this comment.
Non-blocking: out_dir is deliberately reused for AFL auto-resume, but this scans every *.log. If a later run uses fewer workers, stale secN.log files from the wider fleet remain and an old crashing-seed message can fail an otherwise clean rerun. Consider clearing stale logs before launch or restricting this scan to the current fleet's log names.
* chore(abi): rename the pre-finalization state to decided in v0.3 🚚💥 * fix(spec): align v0.3 WASI behavior 🐛 * refactor(codegen): scope trie details to their paths ♻️ `latest_non_final` and `accepted` named an implementation queue rather than the state-view contract: the view is the latest state-changing decided transaction, with finalized state as the fallback. Wire values change with no back-compat alias, so hosts and SDKs must move in lockstep.
c1953cc to
5a8a9d4
Compare
f9218cd to
5a8a9d4
Compare
Bump the v0.3.x gitlink: the case's `A: compared ...` prints race between overlapping nondet queue consumers and a shared stdout, and they are the only observable that a comparison stage ran.
Closes GVM-313
Closes GVM-315
Closes GVM-336
Closes GVM-349
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Changeset
This PR renames the pre-finalization state to
decidedand changes every v0.3-line runner hash, both without a back-compat alias — so it lands together with a downstream PR in each consuming repov0.6-devdecidedon the wirev0.123-devorigin/,decidedat the GenVM boundarymaine2e/andtracks/v0.6/,on='decided'v0.19-devon='decided', contract-fixture hashesv0.30-devon='decided',.tar/.ziprunner-archive loadermainNo PR needed for genlayer-js (pins no runner hashes; its
latest-final/ACCEPTEDsymbols are node-RPC and consensus-contract enums in a different namespace) or genlayer-consensus (its only pin is a pre-v0.3 deployment record of a contract live on Bradbury since 2026-03-19).genlayerlabs/genlayer-node#1753 is the hub