Skip to content

fix: vm fatal errors - #24

Open
kp2pml30 wants to merge 5 commits into
v0.6-devfrom
fix/vm-fatal-errors
Open

fix: vm fatal errors#24
kp2pml30 wants to merge 5 commits into
v0.6-devfrom
fix/vm-fatal-errors

Conversation

@kp2pml30

@kp2pml30 kp2pml30 commented Aug 10, 2026

Copy link
Copy Markdown
Member

Closes GVM-313
Closes GVM-315
Closes GVM-336
Closes GVM-349

Summary by CodeRabbit

  • New Features

    • Added a WebSocket manager interface for execution, monitoring, cancellation, reconnection, acknowledgements, and artifact retrieval.
    • Added cross-version contract routing, nested execution, and configurable execution retention and message-size limits.
    • Added fatal versus recoverable VM errors with optional error catching.
    • Added deterministic fuzzing support and improved test tagging.
  • Bug Fixes

    • Improved runner validation, hashing, resource accounting, lifecycle handling, and error reporting.
  • Documentation

    • Expanded protocol, versioning, security, runner, testing, and internal-constant documentation.

Changeset

This PR renames the pre-finalization state to decided and changes every v0.3-line runner hash, both without a back-compat alias — so it lands together with a downstream PR in each consuming repo

Repo PR Base What it carries
genlayer-node genlayerlabs/genlayer-node#1753 v0.6-dev Hashes, regenerated ABI mirror, decided on the wire
genlayer-studio genlayerlabs/genlayer-studio#1744 v0.123-dev Hashes, re-vendored origin/, decided at the GenVM boundary
genlayer-e2e genlayerlabs/genlayer-e2e#723 main Hashes in e2e/ and tracks/v0.6/, on='decided'
genlayer-py genlayerlabs/genlayer-py#105 v0.19-dev on='decided', contract-fixture hashes
genlayer-testing-suite genlayerlabs/genlayer-testing-suite#106 v0.30-dev on='decided', .tar/.zip runner-archive loader
genlayer-dev-env genlayerlabs/genlayer-dev-env#130 main Hash in the e2e example contract

No PR needed for genlayer-js (pins no runner hashes; its latest-final / ACCEPTED symbols 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

@kp2pml30 kp2pml30 self-assigned this Aug 10, 2026
@github-actions
github-actions Bot changed the base branch from main to v0.6-dev August 10, 2026 02:35
@github-actions

Copy link
Copy Markdown

👋 This PR targeted main, so I've retargeted it to the latest dev branch v0.6-dev.

main is protected and is only an alias of the latest release branch (v0.6-dev), kept in lockstep automatically. Active v0.6 work lands on v0.6-dev, which reaches v0.6.x through the standing release-gate PR once the cross-repo E2E matrix is green.

@coderabbitai

coderabbitai Bot commented Aug 10, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

The 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.

Changes

Manager execution and protocol

Layer / File(s) Summary
Protocol contracts and nested execution
crates/modules-interfaces/..., docs/adr/015. cross-major contract calls.md
Adds generated protocol identifiers, nested execution envelopes, executor selectors, permission types, result decoding, and route-invariant result hashing.
WebSocket and HTTP integration
implementation/src/manager/..., docs/website/src/impl-spec/appendix/manager-*
Replaces Warp with Axum, adds the /ws protocol, lifecycle events, attachment, cancellation, acknowledgements, artifact retrieval, and deprecated HTTP adapters.
Execution lifecycle and version resolution
implementation/src/manager/run.rs, implementation/src/manager/versioning.rs, implementation/src/manager/*_test.rs
Adds retention, deadlines, nested execution, host routing, cancellation, event publication, supervision, and expanded version selection.
Execution and runner specifications
docs/website/src/spec/*, docs/website/src/impl-spec/*, docs/adr/*
Documents fatal VM errors, cross-major calls, result hashing, internal constants, leader-result validation, runner validation, and ABI changes.
Runner artifacts and repository support
support/*, runners/*, install/*, .genvm-tool.py, AGENTS.md, SECURITY.md
Adds ZIP/TAR archive selection, updates runner collection and installation, adds test tags and contributor guidance, and documents trust boundaries.

Estimated code review effort: 5 (Critical) | ~120 minutes

Possibly related issues

Possibly related PRs

Suggested labels: run-full-tests

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 53.63% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the VM fatal-error handling changes, which are a real part of the pull request.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/vm-fatal-errors

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Aug 10, 2026

Copy link
Copy Markdown

GenVM PR actions

Tick a box to run it (the box unticks itself when handled). Actions only run while the PR has the ci-safe label.

  • Force run full tests
  • Rerun full tests
  • Provision executor PRs
  • Merge into dev
Merge

Requires, on the exact head commit:

  • an approving review from a maintainer (any push revokes it), or the rtm label
  • linear history — 0 commits behind base
  • green full GenVM CI and green cross-repo E2E

Full CI runs only when run-full-tests or rtm is set; "Force run full tests" is a sticky toggle for the former.

Every repo lands ONE squashed commit, subject <PR title> (#N).

Commands
  • /genvm-force-mergerepo admin only: land without the review, full-CI and E2E gates, for when those signals are unobtainable, not when they are red. Base, title and 0-behind still apply, and the skip is recorded on the PR.

@github-actions

github-actions Bot commented Aug 10, 2026

Copy link
Copy Markdown

Linked executor PR(s)

executor: genlayerlabs/genvm-executor#21 (v0.2)
executor: genlayerlabs/genvm-executor#22 (v0.3)

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

cause and exit_code are added to the struct but not serialized.

SingleGenVMContextDone gains cause and exit_code, and the terminal Event::Finished carries both. The manual Serialize implementation still writes eight fields and omits them. An HTTP client reading GET /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 win

Make legacy runner integrity checking unconditional.

verify_hash=not is_legacy skips hash checks for both existing and downloaded v0.2.x runners, and manager check-install runs only when args.precompile is enabled. This leaves --precompile=false without 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 win

Add a language to the wire-format code fence.

The fence at Line 46 has no language. markdownlint-cli2 reports MD040. Use text for 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 win

List all generated constants pages in this table.

docs/contributing/howto/genvm-tool.md Lines 33-35 lists constants-pending.rst and internal-constants.rst as generated outputs. Line 33 lists only constants.rst and manager-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 win

Limit 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.md Lines 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 win

Use Internal Constants as the page title.

docs/website/src/spec/appendix/index.rst labels this page Internal Constants, but the rendered page title is Constants. 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 win

State the lifecycle of GET /genvm/{id}/artifact.

This paragraph lists three deprecated execution endpoints. docs/website/src/impl-spec/appendix/manager-api.yaml Lines 321-327 add a fourth /genvm/* route, GET /genvm/{genvm_id}/artifact, described there as a debug convenience adapter over the socket get_artifact. That route is the only /genvm/* route without deprecated: 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 win

Do not encode non-integer JSON numbers as null.

Lines 265-273 map a serde_json::Number to push_i64 or push_u64. A floating-point number matches neither, so Line 271 emits null. 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 win

Do not claim malformed_frame for invalid deadline values.

deadline uses ManagerDuration, so the wire value is decoded before run_ctx.start validates it with ManagerDuration::to_std. That converts duration number is empty, duration has more than one decimal point, and unitless strings into handler failures reported as internal, not malformed_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 win

A failure after startup publishes FailedToStart.

supervise_genvm_inner returns Err for failures that can occur after publish_started ran, for example an io::Error from child.wait() or the missing-process-handle path. supervise_genvm then calls fail_to_start, so a client observes Started followed by FailedToStart. FailedToStart carries 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 win

Rename the hash parameters to avoid Ruff A002.

support/runner-script.py has function arguments named hash in check_bytes, _object_gcs_path, _download_single, _nix_preload, and _upload_single. Rename them to hash_id or archive_hash and 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 win

Close the local trust boundary to an explicit assumption.

The manager default binds TCP to 127.0.0.1:3999 and 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 win

Update the manager schema for the new config fields.

install/config/genvm-manager.yaml references docs/schemas/default-config.json#/manager, but that schema only defines manifest_path and permits. It does not define execution_retention or max_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 win

Add a language to the code fence.

markdownlint-cli2 reports MD040 on Line 10. Change the opening fence to shell.

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 win

Add 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.rst have no test:

  • read_frame rejects a message shorter than HEADER_LEN and an unknown method_id without closing the connection.
  • dispatch answers bad_request_id when request_id == 0.
  • handle_get_artifact clamps max_len to ARTIFACT_CHUNK_CAP.

read_frame accepts any Stream<Item = Result<Message, axum::Error>>, so the first two are testable with futures_util::stream::iter and a test_writer pair. 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 win

Guard the artifact branch against a closed receiver with an open control channel.

If artifact_rx returns None while control_rx is still open, the continue on Line 107 restarts the loop. artifact_rx.recv() then resolves to Ready(None) immediately on every iteration, and the biased select! keeps choosing it while control_rx.recv() stays pending. The result is a busy loop that consumes one core.

Today every Writer holds 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 polling artifact_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 win

Replace error-string matching with typed error discrimination.

attach and get_artifact use plain anyhow::bail! strings as discriminants, and socket.rs maps them by comparing e.to_string() to "boot_id_mismatch" or "unknown_id". Any context wrapped around these errors changes the mapping, sending the wrong Errors variant. Expose a typed error enum from run.rs or return a dedicated result type from attach and get_artifact and 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 value

Limit the payload tag claim to external variants and widen the metrics type.

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 }. Type metrics as any JSON value, since encode_json_value supports null, booleans, numbers, strings, arrays, and objects.

The gvm-def-enum-methods and gvm-def-enum-errors Sphinx labels are present on manager-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 value

Extract the repeated JSON-body handler shape.

Five routes repeat the same closure body: parse Bytes into serde_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

📥 Commits

Reviewing files that changed from the base of the PR and between 70fdf78 and 6926c73.

⛔ Files ignored due to path filters (39)
  • crates/modules-interfaces/Cargo.lock is excluded by !**/*.lock, !**/*.lock
  • implementation/Cargo.lock is excluded by !**/*.lock, !**/*.lock
  • support/tools/genvm-tool/genvm_tool/cmd_configure.py is excluded by !support/tools/genvm-tool/**
  • support/tools/genvm-tool/genvm_tool/codegen/go.py is excluded by !support/tools/genvm-tool/**
  • support/tools/genvm-tool/genvm_tool/codegen/model.py is excluded by !support/tools/genvm-tool/**
  • support/tools/genvm-tool/genvm_tool/codegen/python.py is excluded by !support/tools/genvm-tool/**
  • support/tools/genvm-tool/genvm_tool/codegen/rst.py is excluded by !support/tools/genvm-tool/**
  • support/tools/genvm-tool/genvm_tool/codegen/rust.py is excluded by !support/tools/genvm-tool/**
  • support/tools/genvm-tool/genvm_tool/tests/__init__.py is excluded by !support/tools/genvm-tool/**
  • support/tools/genvm-tool/genvm_tool/tests/exec/process.py is excluded by !support/tools/genvm-tool/**
  • support/tools/genvm-tool/genvm_tool/tests/exec/service.py is excluded by !support/tools/genvm-tool/**
  • support/tools/genvm-tool/genvm_tool/tests/stage/collection.py is excluded by !support/tools/genvm-tool/**
  • support/tools/genvm-tool/genvm_tool/tests/stage/execution.py is excluded by !support/tools/genvm-tool/**
  • support/tools/genvm-tool/genvm_tool/tests/stage/filter.py is excluded by !support/tools/genvm-tool/**
  • support/tools/genvm-tool/genvm_tool/tests/tags.py is excluded by !support/tools/genvm-tool/**
  • support/tools/genvm-tool/genvm_tool/tests/util/watchdog.py is excluded by !support/tools/genvm-tool/**
  • support/tools/genvm-tool/unit_tests/test_codegen_detail_suffix.py is excluded by !support/tools/genvm-tool/**
  • support/tools/genvm-tool/unit_tests/test_process_teardown.py is excluded by !support/tools/genvm-tool/**
  • support/tools/genvm-tool/unit_tests/test_service_startup.py is excluded by !support/tools/genvm-tool/**
  • support/tools/genvm-tool/unit_tests/test_watchdog_ack.py is excluded by !support/tools/genvm-tool/**
  • tests/runner/genvm_tool_plugins/cargo.py is excluded by !tests/**
  • tests/runner/genvm_tool_plugins/docker.py is excluded by !tests/**
  • tests/runner/genvm_tool_plugins/genvm.py is excluded by !tests/**
  • tests/runner/genvm_tool_plugins/integration.py is excluded by !tests/**
  • tests/runner/genvm_tool_plugins/ninja.py is excluded by !tests/**
  • tests/runner/gvm_extra/mock_host.py is excluded by !tests/**
  • tests/runner/origin/base_host.py is excluded by !tests/**
  • tests/runner/origin/host_fns.py is excluded by !tests/**
  • tests/runner/origin/log_asserts.py is excluded by !tests/**
  • tests/runner/origin/manager_api.py is excluded by !tests/**
  • tests/runner/origin/public_abi.py is excluded by !tests/**
  • tests/system/cross-major-observability/test.py is excluded by !tests/**
  • tests/system/cross-major/test.py is excluded by !tests/**
  • tests/system/make_zip/test.py is excluded by !tests/**
  • tests/system/manager-socket/test.py is excluded by !tests/**
  • tests/system/parse_version/test.py is excluded by !tests/**
  • tests/system/permits/test.py is excluded by !tests/**
  • tests/tags.json is excluded by !tests/**
  • tests/templates/simple_deploy_then_write.jsonnet is 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.py
  • AGENTS.md
  • CLAUDE.md
  • CLAUDE.md
  • SECURITY.md
  • crates/modules-interfaces/Cargo.toml
  • crates/modules-interfaces/codegen/data/host-fns.json
  • crates/modules-interfaces/codegen/data/manager-api.json
  • crates/modules-interfaces/src/domain.rs
  • crates/modules-interfaces/src/host_fns.rs
  • crates/modules-interfaces/src/lib.rs
  • crates/modules-interfaces/src/manager_api.rs
  • crates/modules-interfaces/src/nested.rs
  • docs/adr/011. runner ids and runtime registration.md
  • docs/adr/012. runner memory accounting.md
  • docs/adr/013. pre-validating untrusted decoded inputs.md
  • docs/adr/014. manager host socket protocol.md
  • docs/adr/015. cross-major contract calls.md
  • docs/contributing/explanation/executor-lines.md
  • docs/contributing/howto/building/docs.md
  • docs/contributing/howto/committing/submodules.md
  • docs/contributing/howto/extending/add-host-function.md
  • docs/contributing/howto/genvm-tool.md
  • docs/contributing/howto/testing/integration.md
  • docs/contributing/howto/testing/rust.md
  • docs/schemas/runners.json
  • docs/website/src/impl-spec/01-core-architecture/01-components.rst
  • docs/website/src/impl-spec/02-vm/02-version-management.rst
  • docs/website/src/impl-spec/02-vm/03-consensus.rst
  • docs/website/src/impl-spec/appendix/host-loop.rst
  • docs/website/src/impl-spec/appendix/index.rst
  • docs/website/src/impl-spec/appendix/manager-api.rst
  • docs/website/src/impl-spec/appendix/manager-api.yaml
  • docs/website/src/impl-spec/appendix/manager-socket-consts.rst
  • docs/website/src/impl-spec/appendix/manager-socket.rst
  • docs/website/src/spec/02-execution-environment/03-wasi_genlayer_sdk/02-gl_call.rst
  • docs/website/src/spec/02-execution-environment/03-wasi_genlayer_sdk/03-schemas.rst
  • docs/website/src/spec/02-execution-environment/04-runners.rst
  • docs/website/src/spec/03-vm/01-startup.rst
  • docs/website/src/spec/03-vm/02-meta-properties.rst
  • docs/website/src/spec/03-vm/03-ram-limiting.rst
  • docs/website/src/spec/03-vm/05-result.rst
  • docs/website/src/spec/04-contract-interface/02-abi.rst
  • docs/website/src/spec/04-contract-interface/04-upgradability.rst
  • docs/website/src/spec/appendix/constants-pending.rst
  • docs/website/src/spec/appendix/constants.rst
  • docs/website/src/spec/appendix/index.rst
  • docs/website/src/spec/appendix/internal-constants.rst
  • docs/website/src/spec/changelog.rst
  • docs/website/src/spec/index.rst
  • executors/v0.2.x
  • executors/v0.3.x
  • flake.nix
  • implementation/.ya-test-config.json
  • implementation/Cargo.toml
  • implementation/src/manager/handlers.rs
  • implementation/src/manager/handlers_test.rs
  • implementation/src/manager/mod.rs
  • implementation/src/manager/run.rs
  • implementation/src/manager/run_test.rs
  • implementation/src/manager/socket.rs
  • implementation/src/manager/socket_test.rs
  • implementation/src/manager/versioning.rs
  • implementation/src/manager/versioning_test.rs
  • implementation/src/scripting/mod.rs
  • implementation/tests/dflt_requests.rs
  • implementation/tests/merge.rs
  • implementation/tests/overloaded.rs
  • implementation/tests/policy_dispatch.rs
  • implementation/tests/providers.rs
  • implementation/tests/request_body_limit.rs
  • implementation/tests/request_localhost.rs
  • implementation/tests/signing_server.rs
  • implementation/tests/timeout.rs
  • implementation/tests/web_render_webdriver.rs
  • install/config/genvm-manager.yaml
  • install/lib/python/post-install/__main__.py
  • runners/default.nix
  • runners/views/universal.nix
  • support/manifest-base.yaml
  • support/runner-script.py
💤 Files with no reviewable changes (2)
  • runners/default.nix
  • docs/website/src/spec/appendix/constants-pending.rst

Comment thread docs/adr/011. runner ids and runtime registration.md
Comment thread docs/adr/014. manager host socket protocol.md
Comment thread docs/website/src/impl-spec/appendix/manager-api.yaml
Comment on lines +1272 to +1286
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)
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 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.

Suggested change
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.

Comment thread implementation/src/manager/socket.rs Outdated
Comment thread implementation/src/manager/socket.rs Outdated
Comment thread implementation/src/manager/socket.rs
Comment thread implementation/src/scripting/mod.rs
@kp2pml30
kp2pml30 force-pushed the fix/vm-fatal-errors branch from 6926c73 to bf93912 Compare August 10, 2026 05:33

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 lift

Restore a bounded HTTP request-body limit.

DefaultBodyLimit::disable() removes the size limit for every route that extracts Bytes, including /module/*, /genvm/run, /contract/detect-version, /log/level, and /llm/check. max_message_bytes covers only the WebSocket route.
Add a configurable HTTP body-size limit and apply it with DefaultBodyLimit::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

📥 Commits

Reviewing files that changed from the base of the PR and between 6926c73 and bf93912.

📒 Files selected for processing (7)
  • .github/copilot-instructions.md
  • docs/website/src/impl-spec/appendix/manager-api.yaml
  • executors/v0.3.x
  • implementation/src/manager/mod.rs
  • implementation/src/manager/socket.rs
  • implementation/src/scripting/ctx/dflt.rs
  • support/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

@kp2pml30
kp2pml30 force-pushed the fix/vm-fatal-errors branch 2 times, most recently from e2ff747 to 35dd121 Compare August 10, 2026 11:14

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 win

Handle v0.2.x hashes in runner-script.py before advertising tar.

docs/contributing/howto/genvm-tool.md says v0.2.x frozen runner installers should use this script with --archive-mode tar. support/runner-script.py still rejects legacy registry hashes by design because check_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.x tar mode 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

📥 Commits

Reviewing files that changed from the base of the PR and between e2ff747 and 35dd121.

⛔ Files ignored due to path filters (41)
  • support/tools/genvm-tool/genvm_tool/__init__.py is excluded by !support/tools/genvm-tool/**
  • support/tools/genvm-tool/genvm_tool/__main__.py is excluded by !support/tools/genvm-tool/**
  • support/tools/genvm-tool/genvm_tool/cmd_build_manifest.py is excluded by !support/tools/genvm-tool/**
  • support/tools/genvm-tool/genvm_tool/cmd_codegen.py is excluded by !support/tools/genvm-tool/**
  • support/tools/genvm-tool/genvm_tool/cmd_configure.py is excluded by !support/tools/genvm-tool/**
  • support/tools/genvm-tool/genvm_tool/cmd_docs.py is excluded by !support/tools/genvm-tool/**
  • support/tools/genvm-tool/genvm_tool/cmd_test.py is excluded by !support/tools/genvm-tool/**
  • support/tools/genvm-tool/genvm_tool/codegen/__init__.py is excluded by !support/tools/genvm-tool/**
  • support/tools/genvm-tool/genvm_tool/codegen/go.py is excluded by !support/tools/genvm-tool/**
  • support/tools/genvm-tool/genvm_tool/codegen/model.py is excluded by !support/tools/genvm-tool/**
  • support/tools/genvm-tool/genvm_tool/codegen/python.py is excluded by !support/tools/genvm-tool/**
  • support/tools/genvm-tool/genvm_tool/codegen/rst.py is excluded by !support/tools/genvm-tool/**
  • support/tools/genvm-tool/genvm_tool/codegen/rust.py is excluded by !support/tools/genvm-tool/**
  • support/tools/genvm-tool/genvm_tool/common.py is excluded by !support/tools/genvm-tool/**
  • support/tools/genvm-tool/genvm_tool/formatter.py is excluded by !support/tools/genvm-tool/**
  • support/tools/genvm-tool/genvm_tool/git/check_for_push.py is excluded by !support/tools/genvm-tool/**
  • support/tools/genvm-tool/genvm_tool/git/create_branches.py is excluded by !support/tools/genvm-tool/**
  • support/tools/genvm-tool/genvm_tool/git/verify_pushed_executors.py is excluded by !support/tools/genvm-tool/**
  • support/tools/genvm-tool/genvm_tool/io.py is excluded by !support/tools/genvm-tool/**
  • support/tools/genvm-tool/genvm_tool/manifest.py is excluded by !support/tools/genvm-tool/**
  • support/tools/genvm-tool/genvm_tool/tests/__init__.py is excluded by !support/tools/genvm-tool/**
  • support/tools/genvm-tool/genvm_tool/tests/cli.py is excluded by !support/tools/genvm-tool/**
  • support/tools/genvm-tool/genvm_tool/tests/stage/collection.py is excluded by !support/tools/genvm-tool/**
  • support/tools/genvm-tool/genvm_tool/tests/stage/configuration.py is excluded by !support/tools/genvm-tool/**
  • support/tools/genvm-tool/genvm_tool/tests/tags.py is excluded by !support/tools/genvm-tool/**
  • support/tools/genvm-tool/genvm_tool/tests/util/watchdog.py is excluded by !support/tools/genvm-tool/**
  • tests/runner/genvm_tool_plugins/cargo.py is excluded by !tests/**
  • tests/runner/genvm_tool_plugins/genvm.py is excluded by !tests/**
  • tests/runner/genvm_tool_plugins/integration.py is excluded by !tests/**
  • tests/runner/genvm_tool_plugins/ninja.py is excluded by !tests/**
  • tests/runner/genvm_tool_plugins/pytest.py is excluded by !tests/**
  • tests/runner/genvm_tool_plugins/runners.py is excluded by !tests/**
  • tests/runner/origin/base_host.py is excluded by !tests/**
  • tests/runner/origin/fees.py is excluded by !tests/**
  • tests/runner/origin/log_asserts.py is excluded by !tests/**
  • tests/system/cross-major-observability/test.py is excluded by !tests/**
  • tests/system/cross-major/test.py is excluded by !tests/**
  • tests/system/make_zip/test.py is excluded by !tests/**
  • tests/system/manager-socket/test.py is excluded by !tests/**
  • tests/system/parse_version/test.py is excluded by !tests/**
  • tests/system/permits/test.py is excluded by !tests/**
📒 Files selected for processing (22)
  • .genvm-tool.py
  • .github/workflows/incl_release_build_test_cell_build.yaml
  • crates/modules-interfaces/src/domain/fees/mod.rs
  • crates/modules-interfaces/src/host_fns.rs
  • crates/modules-interfaces/src/manager_api.rs
  • docs/website/generate.py
  • docs/website/merge_txts.py
  • executors/v0.2.x
  • executors/v0.3.x
  • implementation/src/lib.rs
  • implementation/src/llm/handler.rs
  • implementation/src/manager/socket.rs
  • implementation/src/manager/socket_test.rs
  • install/bin/genvm-post-install
  • install/lib/python/post-install/__main__.py
  • ruff.toml
  • support/runner-script.py
  • support/scripts/check-commit-message.py
  • support/scripts/check-source-text.py
  • support/scripts/ci-changes.py
  • support/scripts/get-all-git.py
  • support/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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 11

🧹 Nitpick comments (1)
support/tools/fuzz-preload/src/lib.rs (1)

50-56: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Add 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

📥 Commits

Reviewing files that changed from the base of the PR and between 35dd121 and c87b7dc.

⛔ Files ignored due to path filters (13)
  • support/tools/fuzz-preload/Cargo.lock is excluded by !**/*.lock, !**/*.lock
  • support/tools/genvm-tool/genvm_tool/__init__.py is excluded by !support/tools/genvm-tool/**
  • support/tools/genvm-tool/genvm_tool/__main__.py is excluded by !support/tools/genvm-tool/**
  • support/tools/genvm-tool/genvm_tool/cmd_fuzz_corpus.py is excluded by !support/tools/genvm-tool/**
  • support/tools/genvm-tool/genvm_tool/common.py is excluded by !support/tools/genvm-tool/**
  • support/tools/genvm-tool/genvm_tool/misc.py is excluded by !support/tools/genvm-tool/**
  • support/tools/genvm-tool/genvm_tool/tests/cli.py is excluded by !support/tools/genvm-tool/**
  • support/tools/genvm-tool/unit_tests/test_fuzz_corpus.py is excluded by !support/tools/genvm-tool/**
  • tests/runner/genvm_tool_plugins/cargo.py is excluded by !tests/**
  • tests/runner/genvm_tool_plugins/integration.py is excluded by !tests/**
  • tests/runner/genvm_tool_plugins/pytest.py is excluded by !tests/**
  • tests/runner/gvm_extra/case_inputs.py is excluded by !tests/**
  • tests/tags.json is excluded by !tests/**
📒 Files selected for processing (17)
  • .claude/settings.local.json
  • .genvm-tool.py
  • AGENTS.md
  • docs/contributing/explanation/README.md
  • docs/contributing/explanation/fuzz.md
  • docs/contributing/howto/README.md
  • docs/contributing/howto/genvm-tool.md
  • docs/contributing/howto/testing/fuzzing.md
  • docs/website/src/spec/02-execution-environment/01-wasm.rst
  • docs/website/src/spec/02-execution-environment/02-wasip1.rst
  • docs/website/src/spec/02-execution-environment/03-wasi_genlayer_sdk/02-gl_call.rst
  • docs/website/src/spec/02-execution-environment/04-runners.rst
  • docs/website/src/spec/03-vm/05-result.rst
  • executors/v0.2.x
  • executors/v0.3.x
  • support/tools/fuzz-preload/Cargo.toml
  • support/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

Comment thread .claude/settings.local.json Outdated
Comment thread docs/contributing/explanation/fuzz.md
Comment thread docs/contributing/howto/testing/fuzzing.md
Comment thread docs/website/src/spec/02-execution-environment/02-wasip1.rst
Comment thread docs/website/src/spec/02-execution-environment/02-wasip1.rst Outdated
Comment on lines +265 to +267
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``.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 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:


🌐 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:


🏁 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
done

Repository: 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.

Comment thread docs/website/src/spec/02-execution-environment/02-wasip1.rst Outdated
Comment thread executors/v0.2.x Outdated
@kp2pml30
kp2pml30 force-pushed the fix/vm-fatal-errors branch 3 times, most recently from 2753c4a to c1cab0c Compare August 19, 2026 12:41

@MuncleUscles MuncleUscles left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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')):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@github-actions github-actions Bot added the not rebased branch is behind its base; rebase before it can be merged label Aug 21, 2026
* 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.
@kp2pml30
kp2pml30 force-pushed the fix/vm-fatal-errors branch from c1953cc to 5a8a9d4 Compare August 21, 2026 09:06
@github-actions github-actions Bot removed the not rebased branch is behind its base; rebase before it can be merged label Aug 21, 2026
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants