feat: rework manager api for cross-major calls - #9
Conversation
📝 WalkthroughWalkthroughThis change adds shared nested-execution and hashing interfaces, refactors manager lifecycle and supervision, introduces a WebSocket execution protocol, updates cross-major routing specifications, and reorganizes contributor, testing, build, release, and fuzzing documentation. ChangesManager execution and protocol modernization
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related issues
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 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
Full GenVM CI runs only when |
Linked executor PR(s)executor: genlayerlabs/genvm-executor#7 |
1e5b260 to
a43870a
Compare
|
/run-e2e core |
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
Note Superseded where it disagrees with the triage comment, which carries the current Review pass 1 — hard findingsAutomated review of Determinism-focused probing found no leader/validator/sync hash divergence, but note the 1. Cross-major calls drop the caller's custom-runner grants
Scenario: contract A registers Options: either extend the envelope with a canonical, ordered representation of the 2. Legacy calldata rewriting can replace the requested method
The manager unconditionally removes Scenario: calldata Suggested direction: reject key collisions outright, and apply the legacy translation only 3.
|
|
Note Superseded where it disagrees with the triage comment, which carries the current Review pass 2 — the route is consensus-visibleSecond pass (a code review plus two independent determinism-probing agents, one driving the 1. A same-major call hashes differently depending on the route — this is the important one
The in-process path folds Measured on the same caller, same callee, same line, same code, identical result
The route is chosen by the host, not the contract. Purity of This also sits awkwardly against the claim that the result fingerprint is identical across Suggested direction: expose a canonical small-hash computation in the shared interfaces Base comparison is moot: the nested route does not exist on 2. A contract can turn an
|
|
Note Superseded where it disagrees with the triage comment, which carries the current Review pass 3 — a contract-reachable internal errorThird pass. One contract-reachable 1. FIXED — a zero fee budget aborts the transaction with
|
|
Note Superseded where it disagrees with the triage comment, which carries the current Follow-up pass — fixes applied, and two earlier claims retractedWork done after review pass 3, driven by the todo list rather than by new fuzzing. RetractionsTwo findings in pass 3 were attributed to this branch and should not have been. Both are corrected
Common cause: treating "new in the umbrella diff" as "authored by this branch" when a submodule is Fixed
The harness now compares the real execution hash
Two things this uncovered:
v0.2's 195 goldens were regenerated against the new artifact and verified over repeated full runs: Audited and cleanSwept every Incidental: Still open from pass 3The headline finding is untouched: the call route is consensus-visible — in-process folds Also unaddressed: nested-spawn breadth, The v0.2 host-fns enum question from pass 3 is withdrawn — v0.2 is legacy and slated for Tracking: GVM-331 covers the |
github-actions
Bot
added
the
not rebased
Triage of review passes 1-3Single status table for every finding raised in the review passes above, so none of them has to be Scope disclaimer — trusted partiesThis project trusts the host, the release manifest and the executor tarballs it names, and the LLM The bar that does apply: a contract must not be able to produce an Intended, not defects
Retracted
Fixed in this branchPass 1: attach missing the terminal event (3), non-atomic idempotency (4), Open, being addressed on this branch
Tracked elsewhere
Small items still standing
|
2a1a057 to
5296764
Compare
github-actions
Bot
removed
the
not rebased
5296764 to
a9361a4
Compare
|
/run-e2e |
a9361a4 to
c4450a3
Compare
|
/run-e2e |
2 similar comments
|
/run-e2e |
|
/run-e2e |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 20
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/website/src/impl-spec/appendix/manager-api.yaml (1)
263-277: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winDocument the 404 these routes now return.
create_routeranswersnot_found()(404, JSON{"error": ...}) when the path id is not all-digits (implementation/src/manager/mod.rslines 294-296, 308-310, 327-329), but both response blocks only list200/500. Add a'404'entry so the route table matches the implementation. Also worth confirming whether the deprecatedDELETE /genvm/{genvm_id}still accepts theShutdownRequestquery parameters that the router parses — none are documented.Also applies to: 353-372
🤖 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.yaml` around lines 263 - 277, Update the response definitions for both affected GenVM routes, including the block around the GenVM status response and the additional block around lines 353-372, to add a '404' response referencing the existing ErrorResponse schema. Keep the documented 200 and 500 responses unchanged, and ensure the deprecated DELETE route’s documented parameters match the ShutdownRequest query parameters parsed by create_router.implementation/src/scripting/mod.rs (1)
160-166: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winNarrow
StdLib::OSbefore exposing it to module scripts.
StdLib::OSalso exposesos.execute,os.exit,os.getenv,os.remove,os.rename,os.tmpname, andos.setlocale; for LLM/Web modules,os.exitcan terminate the manager andos.getenvexposes provider/env state. If this is only foros.time/os.date/os.clock, nil out the others afterload_std_libs.🤖 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/scripting/mod.rs` around lines 160 - 166, Restrict the OS library exposed by the module scripting setup: after `load_std_libs`, remove `os.execute`, `os.exit`, `os.getenv`, `os.remove`, `os.rename`, `os.tmpname`, and `os.setlocale`, while preserving `os.time`, `os.date`, and `os.clock`. Update the initialization flow around `lua_libs` and `load_std_libs` so scripts retain only the required OS functions.
🧹 Nitpick comments (6)
implementation/src/manager/handlers.rs (1)
244-269: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winConsider capping
max_lenfor artifact reads.
ArtifactRequest.max_lenis unbounded on the handler side; a very large value could trigger a large allocation/response ifget_artifactdoesn't itself clamp it (not shown in this file). This overlaps with the "artifact/log allocation costs" item already noted as a remaining open item for this PR, so flagging here mainly as a pointer to add a cap (e.g. similar toSTATUS_WAIT_CAP) if not already enforced inrun.rs.🤖 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/handlers.rs` around lines 244 - 269, Cap ArtifactRequest.max_len in handle_genvm_artifact before passing it to get_artifact, reusing the established artifact-read cap if one exists in the run context (otherwise add a suitable constant analogous to STATUS_WAIT_CAP). Ensure requests above the cap are clamped or rejected consistently with existing handler behavior, preventing oversized artifact allocations and responses.implementation/src/manager/mod.rs (2)
364-369: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueConsider scoping
DefaultBodyLimit::disable()to the routes that need it.Applied as a router-wide layer, every POST (
/module/*,/log/level,/permits,/llm/check) also becomes unbounded, though only/genvm/runand/contract/detect-versioncarry large payloads. A per-routeDefaultBodyLimit::max(...)/disable()keeps the small admin endpoints bounded.🤖 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 364 - 369, Scope DefaultBodyLimit::disable() to the /genvm/run and /contract/detect-version routes instead of applying it router-wide, and preserve the default body limit for /module/*, /log/level, /permits, and /llm/check. Update the router construction around the existing fallback and with_state chain using per-route layering.
185-265: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueOptional: collapse the repeated JSON-body closures.
/module/start,/module/stop,/module/restart,/log/level,/permits(POST) and/llm/checkrepeat the same parse-then-dispatch block. A small helper taking the handler fn would remove six copies.♻️ Sketch
async fn json_route<F, Fut>(ctx: sync::DArc<AppContext>, body: Bytes, handler: F) -> Response where F: FnOnce(sync::DArc<AppContext>, serde_json::Value) -> Fut, Fut: std::future::Future<Output = anyhow::Result<Response>>, { match parse_json_body(body) { Ok(body) => unwrap_all_anyhow(handler(ctx, body).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 - 265, Optionally reduce duplication by introducing a shared JSON route helper near the route setup that parses Bytes, returns the existing parse-error response, and dispatches the handler through unwrap_all_anyhow. Update the repeated closures for handle_module_start, handle_module_stop, handle_module_restart, handle_set_log_level, the POST permits handler, and the llm/check handler to use it while preserving their current routes and behavior.implementation/src/manager/socket.rs (2)
401-421: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueOptional: avoid copying the payload out of the received frame.
body[HEADER_LEN..].to_vec()copies up tomax_message_bytes(64 MiB by default) per frame.Message::Binaryalready yieldsBytes, sobody.slice(HEADER_LEN..)would hand the payload on without the allocation (decode_payloadtakes&[u8]).🤖 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 401 - 421, Update the frame-return path in the socket message handling flow to use a zero-copy slice of the received Bytes payload instead of allocating with to_vec(). Preserve the existing HEADER_LEN offset and the returned payload type expected by decode_payload, while leaving method and request validation unchanged.
569-581: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftError classification relies on comparing
anyhowdisplay strings.
e.to_string() == "boot_id_mismatch"ande.to_string() == "unknown_id"silently reclassify tounknown_id/malformed_framethe momentrun.rsadds context or rewords a message. Since the protocol codes are part of the documented contract (manager-socket.rst), a typed error enum (ordowncast_ref) fromrun_ctx.attach/get_artifactwould make this contract compiler-checked.Also applies to: 675-678
🤖 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 569 - 581, Replace string-based anyhow error classification in the run_ctx.attach handling and the corresponding get_artifact path with typed error matching, using a shared error enum or downcast_ref. Map the typed boot_id_mismatch and unknown_id variants directly to Errors::BootIdMismatch and Errors::UnknownId, while preserving the existing fallback for other errors. Ensure added context does not change protocol classification.implementation/src/manager/run.rs (1)
922-924: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueStale, truncated doc comment above
unsafe_overrides.The preceding line still reads
/// Debug-only override: force the executor directory (a version dir under the— a sentence cut off whenreroute_tomoved intoUnsafeOverrides, which now carries its own docs. Drop it or replace it with a description of the struct field.🤖 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 922 - 924, Remove or replace the stale truncated doc comment immediately above the unsafe_overrides field in the relevant struct. Document unsafe_overrides itself accurately, without referencing the executor-directory behavior now owned by UnsafeOverrides documentation.
🤖 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/014`. manager host socket protocol.md:
- Around line 46-49: Update the fenced block containing the wire-format example
near the method_id/request_id layout to include an appropriate language
identifier, such as text, while leaving the example content unchanged.
In `@docs/contributing/howto/building/docs.md`:
- Around line 17-18: Make the shell commands in
docs/contributing/howto/building/docs.md lines 17-18 and
docs/contributing/howto/testing/integration.md line 83 shell-safe by replacing
the angle-bracket placeholders <out> and <version> with concrete values or
defined, quoted variables such as "$OUT" and "$VERSION".
In `@docs/contributing/howto/committing/git-third-party.md`:
- Around line 30-33: Update the committing instructions around the
nested-repository workflow to explicitly tell readers to return to the executor
repository root after committing inside the nested repository, before running
`git third-party save executor/third-party/wasmtime`. Preserve the existing path
and command.
In `@docs/contributing/howto/testing/python.md`:
- Around line 12-14: Update the Nix flake path in the testing command after
changing into the genlayer-py-std directory so it resolves to the
repository-root support/nix/py-test location; use the correct four-level
relative path or run the build from the repository root.
In `@docs/website/src/impl-spec/appendix/host-loop.rst`:
- Around line 131-149: Clarify the run_nested handler’s response framing around
manager_start_nested_sync: document whether failures use the standard errors/ok
prefix before the payload, matching resolve_callcontract_executor, eth_call, and
get_balance, or explicitly state that errors are encoded inside the reply
envelope. Ensure the pseudocode specifies the exact framing host implementers
must follow.
In `@docs/website/src/impl-spec/appendix/manager-api.rst`:
- Around line 9-12: Update the deprecation note in the manager API appendix to
include GET /genvm/{id}/artifact alongside the other execution endpoints,
preserving the stated one-release-train removal window and socket protocol
guidance. Also mark the corresponding artifact route as deprecated in the
manager API specification, using the existing deprecation metadata pattern.
In `@docs/website/src/impl-spec/appendix/manager-socket.rst`:
- Around line 128-135: Update the manager-socket documentation around the run
response to state that only some validation is synchronous and may cause the
request to return an internal error; do not claim the ID is always returned
immediately. Clarify that permit acquisition and process spawning continue
asynchronously, with subsequent failures reported through event notifications,
and retain the automatic event subscription behavior.
In
`@docs/website/src/spec/02-execution-environment/03-wasi_genlayer_sdk/02-gl_call.rst`:
- Around line 81-88: Update the delegation description around the “same derived
sub-VM” claim to qualify it for accepted delegations only. State that calls with
loaded custom runners are rejected, and that delegated sub-VM permissions
differ; reference the existing rules governing custom-runner rejection and
delegated permissions while preserving the equivalent observable results and
execution hashes for delegations that are accepted.
In `@executors/v0.2.x`:
- Line 1: Update the executors/v0.2.x gitlink to reference a commit reachable
from the configured remote, or fetch and publish the intended commit
2c2653992b798dd97a5362f49bb4581bd993fc18 before retaining the pointer; ensure
the final submodule reference is resolvable before merging.
In `@executors/v0.3.x`:
- Line 1: Update the gitlink for the executors/v0.3.x submodule from the
unavailable commit 96e758118e45f8edeafbe1331ef9619fe918f463 to a reachable
commit in the configured promisor remote, preserving the submodule dependency
while ensuring CI can check it out.
In `@implementation/src/manager/handlers.rs`:
- Around line 92-111: Bound the queued-execution wait in the run handler’s
`run_ctx.attach` flow with the existing `STATUS_WAIT_CAP`, applying the timeout
to each `events.changed()` await or the overall wait as consistent with
`handle_genvm_status`. Preserve normal event handling and return an appropriate
timeout error when no start event arrives within the cap.
- Around line 387-391: Build a provider-derived substitution map named
check_vars using the same config_vars context as the runtime LLM backend, then
pass check_vars to genvm_common::templater::patch_json when patching backend.
Preserve the existing DOLLAR_UNFOLDER_RE behavior and ensure config_data
placeholders are resolved before constructing the check backend.
In `@implementation/src/manager/mod.rs`:
- Around line 127-151: Introduce a bad_request response helper in the manager
module and route client-input parsing failures from parse_json_body,
parse_query, and deployment_timestamp through it instead of internal_error.
Return the appropriate 4xx response without logging these expected validation
failures at error level, while keeping internal_error for genuine handler or
server failures.
In `@implementation/src/manager/run.rs`:
- Around line 1526-1531: Update the run_nested reply-writing error path around
write_length_prefixed so that, after logging the failure, it immediately returns
from the enclosing loop/function instead of continuing to serve subsequent
requests. Preserve the existing successful-write flow and error logging.
In `@implementation/src/manager/socket.rs`:
- Around line 265-273: Update the serde_json::Value::Number handling to preserve
floating-point values instead of falling through to push_null(); encode the f64
representation using the encoder’s supported non-null representation, such as a
string, while retaining the existing i64 and u64 paths.
- Around line 50-78: Replace non-blocking control sends in async request
handling with awaited sends: add Writer::error_async using send_control_async,
and update the async reply/error call sites in
implementation/src/manager/socket.rs (lines 539, 545, 583, 603, 629, and
existing error callers) while retaining try_send only where awaiting is
impossible. No direct change is needed in
docs/website/src/impl-spec/appendix/manager-socket.rst lines 50-52 because the
documented guarantee remains correct after this code change.
- Around line 582-589: Update handle_attach so it sends the Attach response
through self.writer.send_control before subscribing with
self.subscribe(req.genvm_id, Ok(rx))?. Preserve the existing snapshot payload
and error propagation, ensuring terminal events cannot be emitted before the
attach response.
In `@install/config/genvm-manager.yaml`:
- Around line 11-12: Update the manager definition in
docs/schemas/default-config.json to declare execution_retention and
max_message_bytes alongside manifest_path and permits, using validators that
accept the configured duration and byte-limit values. Keep the schema aligned
with the defaults referenced by install/config/genvm-manager.yaml so schema
validation accepts both keys.
In `@support/scripts/check-source-text.py`:
- Around line 37-42: Update _violations() to scan the full source text rather
than only text.splitlines(), allowing the existing environment-mutation patterns
to match whitespace and newlines across multiline qualified calls, use imports,
and libc imports. Map each full-text match back to its corresponding source
line(s) so violations retain accurate reporting while continuing to detect
env::set_var, env::remove_var, libc mutation calls, and imported aliases.
In `@support/scripts/get-all-git.py`:
- Around line 155-162: Add a cross-process lock around the complete shared-cache
lifecycle in ensure_cache(), covering the existence check, repository
initialization, remote configuration, and fetch operations. Use a lock path
derived from the shared cache location returned by cache_path_for_url() or
git_common_dir(root), and ensure all callers serialize access before reading or
modifying the cache.
---
Outside diff comments:
In `@docs/website/src/impl-spec/appendix/manager-api.yaml`:
- Around line 263-277: Update the response definitions for both affected GenVM
routes, including the block around the GenVM status response and the additional
block around lines 353-372, to add a '404' response referencing the existing
ErrorResponse schema. Keep the documented 200 and 500 responses unchanged, and
ensure the deprecated DELETE route’s documented parameters match the
ShutdownRequest query parameters parsed by create_router.
In `@implementation/src/scripting/mod.rs`:
- Around line 160-166: Restrict the OS library exposed by the module scripting
setup: after `load_std_libs`, remove `os.execute`, `os.exit`, `os.getenv`,
`os.remove`, `os.rename`, `os.tmpname`, and `os.setlocale`, while preserving
`os.time`, `os.date`, and `os.clock`. Update the initialization flow around
`lua_libs` and `load_std_libs` so scripts retain only the required OS functions.
---
Nitpick comments:
In `@implementation/src/manager/handlers.rs`:
- Around line 244-269: Cap ArtifactRequest.max_len in handle_genvm_artifact
before passing it to get_artifact, reusing the established artifact-read cap if
one exists in the run context (otherwise add a suitable constant analogous to
STATUS_WAIT_CAP). Ensure requests above the cap are clamped or rejected
consistently with existing handler behavior, preventing oversized artifact
allocations and responses.
In `@implementation/src/manager/mod.rs`:
- Around line 364-369: Scope DefaultBodyLimit::disable() to the /genvm/run and
/contract/detect-version routes instead of applying it router-wide, and preserve
the default body limit for /module/*, /log/level, /permits, and /llm/check.
Update the router construction around the existing fallback and with_state chain
using per-route layering.
- Around line 185-265: Optionally reduce duplication by introducing a shared
JSON route helper near the route setup that parses Bytes, returns the existing
parse-error response, and dispatches the handler through unwrap_all_anyhow.
Update the repeated closures for handle_module_start, handle_module_stop,
handle_module_restart, handle_set_log_level, the POST permits handler, and the
llm/check handler to use it while preserving their current routes and behavior.
In `@implementation/src/manager/run.rs`:
- Around line 922-924: Remove or replace the stale truncated doc comment
immediately above the unsafe_overrides field in the relevant struct. Document
unsafe_overrides itself accurately, without referencing the executor-directory
behavior now owned by UnsafeOverrides documentation.
In `@implementation/src/manager/socket.rs`:
- Around line 401-421: Update the frame-return path in the socket message
handling flow to use a zero-copy slice of the received Bytes payload instead of
allocating with to_vec(). Preserve the existing HEADER_LEN offset and the
returned payload type expected by decode_payload, while leaving method and
request validation unchanged.
- Around line 569-581: Replace string-based anyhow error classification in the
run_ctx.attach handling and the corresponding get_artifact path with typed error
matching, using a shared error enum or downcast_ref. Map the typed
boot_id_mismatch and unknown_id variants directly to Errors::BootIdMismatch and
Errors::UnknownId, while preserving the existing fallback for other errors.
Ensure added context does not change protocol classification.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: c060afa2-a31d-471e-807d-257611a45f81
⛔ Files ignored due to path filters (23)
crates/modules-interfaces/Cargo.lockis excluded by!**/*.lock,!**/*.lockimplementation/Cargo.lockis excluded by!**/*.lock,!**/*.locksupport/tools/genvm-tool/genvm_tool/__main__.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_howto.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/rst.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/genvm_tool/tests/util/watchdog_main.pyis excluded by!support/tools/genvm-tool/**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/logger.pyis excluded by!tests/**tests/runner/origin/manager_api.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/manager-socket/test.pyis excluded by!tests/**tests/system/permits/test.pyis excluded by!tests/**tests/templates/util.jsonnetis excluded by!tests/**
📒 Files selected for processing (75)
.claude/settings.fuzzing.json.claude/skills/agentic-fuzzing/SKILL.md.genvm-tool.pyCLAUDE.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/README.mddocs/adr/014. manager host socket protocol.mddocs/adr/015. cross-major contract calls.mddocs/contributing/README.mddocs/contributing/explanation/README.mddocs/contributing/explanation/docs-layout.mddocs/contributing/explanation/executor-lines.mddocs/contributing/explanation/merge-model.mddocs/contributing/explanation/vendored-trees.mddocs/contributing/howto/README.mddocs/contributing/howto/building/build.mddocs/contributing/howto/building/docs.mddocs/contributing/howto/building/runners.mddocs/contributing/howto/committing/git-third-party.mddocs/contributing/howto/committing/runners.mddocs/contributing/howto/committing/submodules.mddocs/contributing/howto/docs/style.mddocs/contributing/howto/extending/add-host-function.mddocs/contributing/howto/extending/add-llm-provider.mddocs/contributing/howto/extending/add-wasi-function.mddocs/contributing/howto/extending/modify-runner.mddocs/contributing/howto/extending/modify-wasmtime.mddocs/contributing/howto/extending/write-a-script.mddocs/contributing/howto/genvm-tool.mddocs/contributing/howto/pr.mddocs/contributing/howto/releasing/release-build.mddocs/contributing/howto/releasing/versioning.mddocs/contributing/howto/setup.mddocs/contributing/howto/testing/README.mddocs/contributing/howto/testing/fuzzing.mddocs/contributing/howto/testing/integration.mddocs/contributing/howto/testing/python.mddocs/contributing/howto/testing/rust.mddocs/contributing/howto/write-a-script.mddocs/contributing/tutorial/README.mddocs/contributing/tutorial/first-contribution.mddocs/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/03-vm/01-startup.rstdocs/website/src/spec/03-vm/02-meta-properties.rstdocs/website/src/spec/03-vm/05-result.rstdocs/website/src/spec/04-contract-interface/04-upgradability.rstdocs/website/src/spec/appendix/constants.rstexecutors/v0.2.xexecutors/v0.3.ximplementation/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/scripting/mod.rsinstall/config/genvm-manager.yamlsupport/scripts/check-source-text.pysupport/scripts/get-all-git.py
💤 Files with no reviewable changes (2)
- docs/contributing/howto/write-a-script.md
- .claude/settings.fuzzing.json
| ``` | ||
| u16 method_id (BE) | u64 request_id (BE) | calldata(payload) | ||
| ``` | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Specify a language for the fenced block.
Use a language identifier such as text for the wire-format example so Markdown tooling can validate the fence consistently.
🧰 Tools
🪛 markdownlint-cli2 (0.23.1)
[warning] 46-46: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 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 - 49, Update
the fenced block containing the wire-format example near the
method_id/request_id layout to include an appropriate language identifier, such
as text, while leaving the example content unchanged.
Source: Linters/SAST tools
| nix develop '.?submodules=1#gen-docs' --command \ | ||
| sphinx-build -q -b dirhtml docs/website/src <out> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Make executable documentation placeholders shell-safe. Angle-bracket placeholders in bash commands are parsed as redirection operators rather than substituted values.
docs/contributing/howto/building/docs.md#L17-L18: replace<out>with a concrete directory or a defined, quoted variable such as"$OUT".docs/contributing/howto/testing/integration.md#L83-L83: replace<version>with a concrete version or a defined, quoted"$VERSION"variable.
📍 Affects 2 files
docs/contributing/howto/building/docs.md#L17-L18(this comment)docs/contributing/howto/testing/integration.md#L83-L83
🤖 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` around lines 17 - 18, Make the
shell commands in docs/contributing/howto/building/docs.md lines 17-18 and
docs/contributing/howto/testing/integration.md line 83 shell-safe by replacing
the angle-bracket placeholders <out> and <version> with concrete values or
defined, quoted variables such as "$OUT" and "$VERSION".
| 1. Edit, and **commit inside the nested repo** — patches come from the history | ||
| above the base commit | ||
| 2. `git third-party save executor/third-party/wasmtime`, path relative to the | ||
| current directory |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Clarify the working directory before git third-party save.
Step 1 tells the reader to commit inside the nested repository, but step 2 passes executor/third-party/wasmtime, which will not resolve if they remain there. Explicitly require returning to the executor repository root before running save.
Proposed clarification
-1. Edit, and **commit inside the nested repo** — patches come from the history
- above the base commit
-2. `git third-party save executor/third-party/wasmtime`, path relative to the
- current directory
+1. Edit, and **commit inside the nested repo** — patches come from the history
+ above the base commit, then return to the executor repository root
+2. From the executor repository root, run
+ `git third-party save executor/third-party/wasmtime`📝 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.
| 1. Edit, and **commit inside the nested repo** — patches come from the history | |
| above the base commit | |
| 2. `git third-party save executor/third-party/wasmtime`, path relative to the | |
| current directory | |
| 1. Edit, and **commit inside the nested repo** — patches come from the history | |
| above the base commit, then return to the executor repository root | |
| 2. From the executor repository root, run | |
| `git third-party save executor/third-party/wasmtime` |
🤖 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/committing/git-third-party.md` around lines 30 - 33,
Update the committing instructions around the nested-repository workflow to
explicitly tell readers to return to the executor repository root after
committing inside the nested repository, before running `git third-party save
executor/third-party/wasmtime`. Preserve the existing path and command.
| cd executors/v0.3.x/runners/genlayer-py-std | ||
| env_dir="$(nix build --no-link --print-out-paths path:../../support/nix/py-test)" | ||
| PYTHONPATH="$PWD/src:$PWD/src-emb" "$env_dir/bin/pytest" tests/ |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Fix the relative Nix flake path.
After cd executors/v0.3.x/runners/genlayer-py-std, ../../support/nix/py-test resolves to executors/v0.3.x/support/nix/py-test, not the repository-root support/nix/py-test. Use ../../../../support/nix/py-test or invoke the command from the repository root.
🤖 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/testing/python.md` around lines 12 - 14, Update the
Nix flake path in the testing command after changing into the genlayer-py-std
directory so it resolves to the repository-root support/nix/py-test location;
use the correct four-level relative path or run the build from the repository
root.
| execution_retention: 5m # finished executions are retained until ack or this TTL | ||
| max_message_bytes: 67108864 # 64 MiB manager websocket message cap |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd -t f 'default-config.json' -x jq '.properties.manager // .["$defs"] // .' {} | head -100
rg -n 'execution_retention|max_message_bytes' --glob '*.json'Repository: genlayerlabs/genvm-manager
Length of output: 3076
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== candidate files =="
fd -t f 'genvm-manager.yaml|default-config.json' .
echo
echo "== install/config/genvm-manager.yaml first 20 lines =="
file=$(fd -t f 'genvm-manager.yaml' . | head -n1)
if [ -n "${file:-}" ]; then
cat -n "$file" | sed -n '1,20p'
fi
echo
echo "== schema references and manager/additionalProperties occurrences =="
schema=$(fd -t f 'default-config.json' . | head -n1)
if [ -n "${schema:-}" ]; then
rg -n '"(\$id|additionalProperties|properties)"|manager|execution_retention|max_message_bytes|additionalProperties' "$schema" | sed -n '1,220p'
fi
echo
echo "== JSON path extraction if jq exists =="
if command -v jq >/dev/null 2>&1 && [ -n "${schema:-}" ]; then
jq 'def paths: path(..); def get(\$p): getpath(\$p);
{"\$id": .["$id"],
"$schema": .["$schema"],
"manager_path": paths[] as $p | select($p|map(tostring)|join(".")=="properties.manager") as $p | $p,
"manager_path_props": paths[] as $p | select($p|map(tostring)|join(".")|startswith("properties.manager.properties.")) as $p | $p,
"manager_additionalProperties": .properties.manager | getpath(paths[] | select(. as $p | ($p|map(tostring)|join("."))=="additionalProperties") // []) // null
}' "$schema" | sed -n '1,160p'
fiRepository: genlayerlabs/genvm-manager
Length of output: 2197
🏁 Script executed:
#!/bin/bash
set -euo pipefail
schema="docs/schemas/default-config.json"
echo "== manager schema lines 160-225 from $schema =="
sed -n '160,225p' "$schema" | nl -ba -v160
echo
echo "== manager properties exact names =="
python3 - <<'PY'
import json
from pathlib import Path
p = Path("docs/schemas/default-config.json")
data = json.loads(p.read_text())
manager = data.get("manager", {})
prop = manager.get("properties", {})
print(sorted(prop.keys()))
print("count:", len(prop))
keys = {"execution_retention", "max_message_bytes"}
for k in sorted(keys):
print(f"{k}:", prop.get(k))
PY
echo
echo "== remote schema existence and manager snippet =="
for url in \
"https://raw.githubusercontent.com/genlayerlabs/genvm/refs/heads/main/doc/schemas/default-config.json" \
"https://raw.githubusercontent.com/genlayerlabs/genvm/refs/heads/main/docs/schemas/default-config.json" \
"https://raw.githubusercontent.com/genlayerlabs/genvm/refs/heads/@GENVM_TAG@/doc/schemas/default-config.json" \
"https://raw.githubusercontent.com/genlayerlabs/genvm/refs/heads/@GENVM_TAG@/docs/schemas/default-config.json"
do
echo "-- $url --"
if curl -sSfI "$url" | grep -q '^HTTP/'; then
curl -sSf "$url" | sed -n '150,225p' | nl -ba -v150
else
echo "unavailable"
fi
doneRepository: genlayerlabs/genvm-manager
Length of output: 277
🏁 Script executed:
#!/bin/bash
set -euo pipefail
schema="docs/schemas/default-config.json"
echo "== manager schema lines 160-222 from $schema =="
sed -n '160,222p' "$schema"
echo
echo "== manager property names via Python =="
python3 - <<'PY'
import json
from pathlib import Path
p = Path("docs/schemas/default-config.json")
data = json.loads(p.read_text())
manager = data.get("manager", {})
prop = manager.get("properties", {})
print("manager_properties_count=", len(prop))
for k in sorted([m for m in prop if m in {"execution_retention", "max_message_bytes"} or len(m) > 20]):
print(f"{k}={prop[m] is not None}")
print("manager_additional_properties=", manager.get("additionalProperties"))
print("has_manager=", bool(manager))
PY
echo
echo "== local doc alternate file =="
ls -la doc/schemas/default-config.json 2>/dev/null || true
ls -la docs/schemas/default-config.json 2>/dev/null || true
echo
echo "== remote manifest/schema existence and manager snippets =="
python3 - <<'PY'
import json
from urllib.request import urlopen, Request
urls = [
"https://raw.githubusercontent.com/genlayerlabs/genvm/refs/heads/main/doc/schemas/default-config.json",
"https://raw.githubusercontent.com/genlayerlabs/genvm/refs/heads/main/docs/schemas/default-config.json",
"https://raw.githubusercontent.com/genlayerlabs/genvm/refs/heads/@GENVM_TAG@/doc/schemas/default-config.json",
"https://raw.githubusercontent.com/genlayerlabs/genvm/refs/heads/@GENVM_TAG@/docs/schemas/default-config.json",
]
for base in urls:
try:
req = Request(base, headers={"User-Agent": "CodeRabbit"})
with urlopen(req, timeout=15) as r:
data = json.load(r)
print(f"\n-- {base} -- status=", r.status, "size=", len(r.read(0).split()) if False else 0)
manager = data.get("manager", {})
props = manager.get("properties", {})
print("manager_exists=", bool(manager), "properties_count=", len(props), "additionalProperties=", manager.get("additionalProperties"))
for key in sorted({"threads", "execution_retention", "max_message_bytes"} & set(props.keys())):
print(f"profile_{key}=", bool(props.get(key)))
lines = base.rsplit("/", 1)[0].rsplit("/", 1)[-1].split("/", 1)[0].rsplit("/", 1)[-1]
print("path=", lines)
except Exception as e:
print(f"-- {base} -- error={type(e).__name__}: {e}")
PYRepository: genlayerlabs/genvm-manager
Length of output: 3354
Add the new manager keys to the config schema
install/config/genvm-manager.yaml references docs/schemas/default-config.json#/manager, and that manager schema only defines manifest_path/permits. Add execution_retention and max_message_bytes there, plus any schema validators required by these defaults, so schema-aware editors and CI do not reject the config.
🤖 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
definition in docs/schemas/default-config.json to declare execution_retention
and max_message_bytes alongside manifest_path and permits, using validators that
accept the configured duration and byte-limit values. Keep the schema aligned
with the defaults referenced by install/config/genvm-manager.yaml so schema
validation accepts both keys.
| def cache_path_for_url(root: Path, url: str) -> Path: | ||
| parsed = urlparse(url) | ||
| url_path = parsed.path if parsed.scheme else url.rsplit(':', 1)[-1] | ||
| name = Path(url_path).name or 'repo' | ||
| name = re.sub(r'\.git$', '', name) | ||
| name = re.sub(r'[^A-Za-z0-9._-]+', '-', name).strip('-') or 'repo' | ||
| digest = hashlib.sha256(url.encode()).hexdigest()[:16] | ||
| return root / '.git' / 'genvm-submodule-cache' / f'{name}-{digest}.git' | ||
| return git_common_dir(root) / 'genvm-submodule-cache' / f'{name}-{digest}.git' |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Serialize access to the shared submodule cache.
Using git_common_dir(root) makes one cache repository shared by linked worktrees, but ensure_cache() still performs an unlocked check/init/configure/fetch sequence. Concurrent setup can race on repository initialization, remote configuration, or fetch locks, leaving one invocation failed or consuming an incomplete cache. Add a cross-process lock around the cache lifecycle.
🤖 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/scripts/get-all-git.py` around lines 155 - 162, Add a cross-process
lock around the complete shared-cache lifecycle in ensure_cache(), covering the
existence check, repository initialization, remote configuration, and fetch
operations. Use a lock path derived from the shared cache location returned by
cache_path_for_url() or git_common_dir(root), and ensure all callers serialize
access before reading or modifying the cache.
c4450a3 to
5eaa59b
Compare
|
/run-e2e |
|
/run-e2e |
5eaa59b to
7ecb349
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
implementation/src/scripting/mod.rs (1)
160-166: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winAvoid exposing dangerous
osfunctions unless scripts need them.
StdLib::IOis already enabled, but addingStdLib::OSexposesos.execute,os.getenv, andos.exit. If only harmless time/date functions are needed, loadOSand then removeexecute,getenv, andexitfrom theostable. This is especially relevant because the manager resolves${ENV[...]}provider keys before spawning scripts.🤖 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/scripting/mod.rs` around lines 160 - 166, Update the Lua library setup around the lua_libs configuration to avoid exposing dangerous OS capabilities: retain StdLib::OS for required time/date functionality, then remove os.execute, os.getenv, and os.exit from the Lua os table before scripts run. Preserve the existing libraries and ensure the manager’s environment-provider resolution remains unaffected.
♻️ Duplicate comments (2)
support/scripts/check-source-text.py (1)
37-42: 🎯 Functional Correctness | 🟠 MajorScan the full source text for environment mutations.
These patterns are still evaluated one
text.splitlines()entry at a time, so qualified calls split across lines and multilineuse std::env::{ ... }imports bypass the guard. Scan the full text and map match offsets back to source lines while preserving the existing opt-out behavior.Proposed direction
- for lineno, line in enumerate(text.splitlines(), 1): - if any(pat.search(line) for pat in _PATTERNS): - yield lineno, line.strip() + for pat in _PATTERNS: + for match in pat.finditer(text): + lineno = text.count('\n', 0, match.start()) + 1 + yield lineno, match.group(0).strip()🤖 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/scripts/check-source-text.py` around lines 37 - 42, Update the source-scanning logic around the environment-mutation regexes to search the complete source text rather than individual text.splitlines() entries, so qualified calls and multiline use std::env imports are detected. Map each match offset back to its source line for diagnostics, and preserve the existing opt-out behavior.support/scripts/get-all-git.py (1)
149-162: 🩺 Stability & Availability | 🟠 MajorSerialize access to the shared submodule cache.
git_common_dir(root)makes linked worktrees share the same cache, butensure_cache()still performs the check/init/configure/fetch lifecycle without a cross-process lock. Concurrent setup can corrupt initialization or leave callers observing an incomplete cache.Wrap the complete
ensure_cache()lifecycle in a lock derived from the cache path, including initialization, remote configuration, and fetch.🤖 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/scripts/get-all-git.py` around lines 149 - 162, Update ensure_cache() to serialize its entire cache setup lifecycle with a cross-process lock derived from the cache path returned by cache_path_for_url(). Acquire the lock before checking or initializing the cache, and keep remote configuration and fetch operations inside the locked scope so concurrent callers cannot observe or modify an incomplete cache.
🧹 Nitpick comments (3)
docs/contributing/tutorial/README.md (1)
3-4: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winClarify the tutorial’s “throwaway change” wording.
“Does not matter” is ambiguous for contributors; explicitly describe this as a throwaway or low-risk change.
Suggested wording
-Learning-oriented walkthroughs: follow them start to finish, on a change that -does not matter, before you need the how-to guides +Learning-oriented walkthroughs: follow them start to finish on a throwaway +change before you need the how-to guides🤖 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/tutorial/README.md` around lines 3 - 4, Update the tutorial description near “Learning-oriented walkthroughs” to explicitly characterize the practice change as throwaway or low-risk, replacing the ambiguous “does not matter” wording while preserving the instruction to complete the walkthrough before using how-to guides.Source: Linters/SAST tools
implementation/src/manager/run.rs (2)
2396-2400: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueStoring the child in
process_handleonly to take it back is hard to follow.The handle is parked at Lines 2398-2400 and unconditionally reclaimed at Lines 2441-2444 with nothing awaiting in between except a mutex lock and
publish_started. If the intent is to letCtx::request_finishkill the process during that window, a comment saying so would help; otherwise the round-trip (and the"process handle missing"error path) can be dropped.Also applies to: 2440-2449
🤖 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 2396 - 2400, The top-level child process is unnecessarily stored in and reclaimed from process_handle without an explanatory purpose. Either add a concise comment around the process_handle assignment and retrieval documenting that Ctx::request_finish may terminate the child during this window, or remove the round-trip and its associated “process handle missing” error path if that behavior is not required.
605-636: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
get_artifactrebuilds the whole artifact for every chunk request.Each call copies the full
stdout/stderr(or re-serializes the entiregenvm_log) into a freshVecjust to hand back at most 256 KiB. Paging a large log through the socket becomes quadratic.stdout/stderrcould sliceas_bytes()directly, and the JSON-lines rendering could be memoized on the done struct.🤖 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 605 - 636, The get_artifact method currently materializes the entire artifact on every chunk request, causing inefficient paging. Refactor its field handling to slice stdout and stderr directly from their byte representations, and reuse a memoized genvm_log JSON-lines rendering stored on the completed result/done struct instead of rebuilding it per call; preserve total_len, offset clamping, chunk-cap, and unknown-field 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/contributing/howto/building/runners.md`:
- Around line 7-10: Update the documented genvm-post-install command to align
with the default venv creation mode: remove any create-venv false setting, or
explicitly add create-venv true. Keep the existing runner download and
missing-executor options unchanged.
In `@docs/contributing/tutorial/first-contribution.md`:
- Around line 24-26: Update the pre-commit guidance in the contribution tutorial
to use the complete manager hook command, replacing or clearly qualifying nix
fmt with nix develop '.?submodules=1#full' --command pre-commit run --all-files.
Keep the separate command for running all CI hooks if still applicable, and
ensure the instructions distinguish formatting from passing the full pre-commit
suite.
In `@implementation/src/manager/handlers.rs`:
- Around line 244-268: Clamp the client-supplied max_len in
handle_genvm_artifact to the shared ARTIFACT_CHUNK_CAP before calling
get_artifact, matching the websocket path’s behavior. Make ARTIFACT_CHUNK_CAP
visible to this handler (for example, pub(super)) and reuse that single constant
across both transports.
In `@implementation/src/manager/mod.rs`:
- Around line 364-369: Replace the unbounded DefaultBodyLimit::disable() in the
manager router construction with a limit derived from
app_ctx.config.max_message_bytes, capturing that value before with_state and
applying it through the body-limit layer so oversized HTTP bodies remain bounded
and return the expected 413 error shape.
In `@implementation/src/manager/run.rs`:
- Around line 2430-2433: Update the cleanup logic in run_genvm_process so
log_sink_guard is only forgotten for top-level executions, or explicitly removes
the nested run’s own GENVM_BY_ID_LOGGER entry before returning. Ensure nested
runs do not leave entries keyed by their nested genvm_id while preserving the
existing publish_started behavior for top-level runs.
---
Outside diff comments:
In `@implementation/src/scripting/mod.rs`:
- Around line 160-166: Update the Lua library setup around the lua_libs
configuration to avoid exposing dangerous OS capabilities: retain StdLib::OS for
required time/date functionality, then remove os.execute, os.getenv, and os.exit
from the Lua os table before scripts run. Preserve the existing libraries and
ensure the manager’s environment-provider resolution remains unaffected.
---
Duplicate comments:
In `@support/scripts/check-source-text.py`:
- Around line 37-42: Update the source-scanning logic around the
environment-mutation regexes to search the complete source text rather than
individual text.splitlines() entries, so qualified calls and multiline use
std::env imports are detected. Map each match offset back to its source line for
diagnostics, and preserve the existing opt-out behavior.
In `@support/scripts/get-all-git.py`:
- Around line 149-162: Update ensure_cache() to serialize its entire cache setup
lifecycle with a cross-process lock derived from the cache path returned by
cache_path_for_url(). Acquire the lock before checking or initializing the
cache, and keep remote configuration and fetch operations inside the locked
scope so concurrent callers cannot observe or modify an incomplete cache.
---
Nitpick comments:
In `@docs/contributing/tutorial/README.md`:
- Around line 3-4: Update the tutorial description near “Learning-oriented
walkthroughs” to explicitly characterize the practice change as throwaway or
low-risk, replacing the ambiguous “does not matter” wording while preserving the
instruction to complete the walkthrough before using how-to guides.
In `@implementation/src/manager/run.rs`:
- Around line 2396-2400: The top-level child process is unnecessarily stored in
and reclaimed from process_handle without an explanatory purpose. Either add a
concise comment around the process_handle assignment and retrieval documenting
that Ctx::request_finish may terminate the child during this window, or remove
the round-trip and its associated “process handle missing” error path if that
behavior is not required.
- Around line 605-636: The get_artifact method currently materializes the entire
artifact on every chunk request, causing inefficient paging. Refactor its field
handling to slice stdout and stderr directly from their byte representations,
and reuse a memoized genvm_log JSON-lines rendering stored on the completed
result/done struct instead of rebuilding it per call; preserve total_len, offset
clamping, chunk-cap, and unknown-field behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 1f9a45f5-9eff-45f2-a386-f83319b2e70f
⛔ Files ignored due to path filters (23)
crates/modules-interfaces/Cargo.lockis excluded by!**/*.lock,!**/*.lockimplementation/Cargo.lockis excluded by!**/*.lock,!**/*.locksupport/tools/genvm-tool/genvm_tool/__main__.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_howto.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/rst.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/genvm_tool/tests/util/watchdog_main.pyis excluded by!support/tools/genvm-tool/**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/logger.pyis excluded by!tests/**tests/runner/origin/manager_api.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/manager-socket/test.pyis excluded by!tests/**tests/system/permits/test.pyis excluded by!tests/**tests/templates/util.jsonnetis excluded by!tests/**
📒 Files selected for processing (75)
.claude/settings.fuzzing.json.claude/skills/agentic-fuzzing/SKILL.md.genvm-tool.pyCLAUDE.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/README.mddocs/adr/014. manager host socket protocol.mddocs/adr/015. cross-major contract calls.mddocs/contributing/README.mddocs/contributing/explanation/README.mddocs/contributing/explanation/docs-layout.mddocs/contributing/explanation/executor-lines.mddocs/contributing/explanation/merge-model.mddocs/contributing/explanation/vendored-trees.mddocs/contributing/howto/README.mddocs/contributing/howto/building/build.mddocs/contributing/howto/building/docs.mddocs/contributing/howto/building/runners.mddocs/contributing/howto/committing/git-third-party.mddocs/contributing/howto/committing/runners.mddocs/contributing/howto/committing/submodules.mddocs/contributing/howto/docs/style.mddocs/contributing/howto/extending/add-host-function.mddocs/contributing/howto/extending/add-llm-provider.mddocs/contributing/howto/extending/add-wasi-function.mddocs/contributing/howto/extending/modify-runner.mddocs/contributing/howto/extending/modify-wasmtime.mddocs/contributing/howto/extending/write-a-script.mddocs/contributing/howto/genvm-tool.mddocs/contributing/howto/pr.mddocs/contributing/howto/releasing/release-build.mddocs/contributing/howto/releasing/versioning.mddocs/contributing/howto/setup.mddocs/contributing/howto/testing/README.mddocs/contributing/howto/testing/fuzzing.mddocs/contributing/howto/testing/integration.mddocs/contributing/howto/testing/python.mddocs/contributing/howto/testing/rust.mddocs/contributing/howto/write-a-script.mddocs/contributing/tutorial/README.mddocs/contributing/tutorial/first-contribution.mddocs/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/03-vm/01-startup.rstdocs/website/src/spec/03-vm/02-meta-properties.rstdocs/website/src/spec/03-vm/05-result.rstdocs/website/src/spec/04-contract-interface/04-upgradability.rstdocs/website/src/spec/appendix/constants.rstexecutors/v0.2.xexecutors/v0.3.ximplementation/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/scripting/mod.rsinstall/config/genvm-manager.yamlsupport/scripts/check-source-text.pysupport/scripts/get-all-git.py
💤 Files with no reviewable changes (2)
- docs/contributing/howto/write-a-script.md
- .claude/settings.fuzzing.json
🚧 Files skipped from review as they are similar to previous changes (23)
- docs/website/src/impl-spec/appendix/index.rst
- executors/v0.2.x
- docs/website/src/spec/02-execution-environment/03-wasi_genlayer_sdk/02-gl_call.rst
- docs/website/src/spec/appendix/constants.rst
- docs/website/src/impl-spec/appendix/manager-api.rst
- docs/contributing/explanation/README.md
- docs/contributing/explanation/docs-layout.md
- docs/contributing/explanation/merge-model.md
- docs/contributing/howto/testing/fuzzing.md
- docs/website/src/impl-spec/appendix/manager-socket-consts.rst
- docs/contributing/howto/extending/modify-wasmtime.md
- docs/contributing/howto/genvm-tool.md
- docs/contributing/howto/releasing/versioning.md
- docs/contributing/howto/testing/python.md
- docs/contributing/howto/building/docs.md
- docs/contributing/howto/extending/write-a-script.md
- docs/website/src/spec/03-vm/01-startup.rst
- docs/contributing/howto/building/build.md
- docs/contributing/README.md
- docs/contributing/howto/README.md
- docs/contributing/howto/testing/rust.md
- docs/contributing/howto/extending/add-wasi-function.md
- docs/contributing/howto/releasing/release-build.md
| 5. **Pass pre-commit** — the hooks run where you commit, so never | ||
| `--no-verify`. Ahead of time: `nix fmt` for the manager, or | ||
| `./support/ci/run.sh pipeline commit-hooks` for everything, as CI does |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use the full manager pre-commit command here.
nix fmt does not clearly run the complete manager hook suite, while submodules.md specifies nix develop '.?submodules=1#full' --command pre-commit run --all-files. Replace or qualify nix fmt so contributors do not mistake formatting for passing all pre-commit checks.
🤖 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/tutorial/first-contribution.md` around lines 24 - 26,
Update the pre-commit guidance in the contribution tutorial to use the complete
manager hook command, replacing or clearly qualifying nix fmt with nix develop
'.?submodules=1#full' --command pre-commit run --all-files. Keep the separate
command for running all CI hooks if still applicable, and ensure the
instructions distinguish formatting from passing the full pre-commit suite.
| #[derive(Debug, serde::Deserialize)] | ||
| pub struct ArtifactRequest { | ||
| pub field: String, | ||
| #[serde(default)] | ||
| pub offset: u64, | ||
| pub max_len: u32, | ||
| } | ||
|
|
||
| pub async fn handle_genvm_artifact( | ||
| ctx: sync::DArc<AppContext>, | ||
| genvm_id: run::GenVMId, | ||
| req: ArtifactRequest, | ||
| ) -> Result<Json<serde_json::Value>> { | ||
| use base64::Engine; | ||
|
|
||
| let artifact = ctx | ||
| .run_ctx | ||
| .get_artifact(genvm_id, &req.field, req.offset, req.max_len)?; | ||
| Ok(Json(serde_json::json!({ | ||
| "genvm_id": genvm_id, | ||
| "field": req.field, | ||
| "total_len": artifact.total_len, | ||
| "data_base64": base64::engine::general_purpose::STANDARD.encode(&artifact.data), | ||
| }))) | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
HTTP artifact reads are unclamped, unlike the websocket path.
socket.rs clamps max_len to ARTIFACT_CHUNK_CAP (256 KiB) before calling get_artifact (Line 657), but this handler forwards the client-supplied max_len verbatim. A request with max_len=4294967295 makes the manager materialize the chunk plus a ~1.33× base64 copy, so a few concurrent calls can exhaust memory. Share one cap across both transports.
🛡️ Proposed fix
let artifact = ctx
.run_ctx
- .get_artifact(genvm_id, &req.field, req.offset, req.max_len)?;
+ .get_artifact(
+ genvm_id,
+ &req.field,
+ req.offset,
+ req.max_len.min(super::socket::ARTIFACT_CHUNK_CAP),
+ )?;(requires making ARTIFACT_CHUNK_CAP visible outside socket, e.g. pub(super))
🤖 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/handlers.rs` around lines 244 - 268, Clamp the
client-supplied max_len in handle_genvm_artifact to the shared
ARTIFACT_CHUNK_CAP before calling get_artifact, matching the websocket path’s
behavior. Make ARTIFACT_CHUNK_CAP visible to this handler (for example,
pub(super)) and reuse that single constant across both transports.
| .fallback(not_found) | ||
| .method_not_allowed_fallback(not_found) | ||
| // Contract code and calldata routinely exceed the default cap, and an | ||
| // exceeded cap answers with a plain-text 413 rather than our error shape. | ||
| .layer(axum::extract::DefaultBodyLimit::disable()) | ||
| .with_state(app_ctx) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Disabling the body limit entirely makes every POST an unbounded memory sink.
Bytes extractors buffer the whole body, so with DefaultBodyLimit::disable() a single client can drive the manager to OOM. The websocket path is bounded by config.max_message_bytes; the HTTP path should use the same knob rather than no limit, which also keeps the desired JSON error shape for oversized bodies (413 only fires above the cap).
🛡️ Proposed fix
- // Contract code and calldata routinely exceed the default cap, and an
- // exceeded cap answers with a plain-text 413 rather than our error shape.
- .layer(axum::extract::DefaultBodyLimit::disable())
+ // Contract code and calldata routinely exceed axum's 2 MiB default, so
+ // raise the cap to the configured message size instead of removing it.
+ .layer(axum::extract::DefaultBodyLimit::max(max_body_bytes))with let max_body_bytes = app_ctx.config.max_message_bytes; captured before with_state.
🤖 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 364 - 369, Replace the
unbounded DefaultBodyLimit::disable() in the manager router construction with a
limit derived from app_ctx.config.max_message_bytes, capturing that value before
with_state and applying it through the body-limit layer so oversized HTTP bodies
remain bounded and return the expected 413 error shape.
| log_sink_guard.forget(); | ||
|
|
||
| Ok((genvm_id, rx)) | ||
| } | ||
|
|
||
| #[cfg(test)] | ||
| mod tests { | ||
| use super::keep_tail; | ||
|
|
||
| #[test] | ||
| fn under_or_at_limit_is_unchanged() { | ||
| let mut s = String::from("hello"); | ||
| keep_tail(&mut s, 10); | ||
| assert_eq!(s, "hello"); | ||
| keep_tail(&mut s, 5); | ||
| assert_eq!(s, "hello"); | ||
| if is_top_level { | ||
| exec_ctx.publish_started(); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Nested runs leak their GENVM_BY_ID_LOGGER entry.
log_sink_guard.forget() is unconditional, but the map entry inserted at Line 2236 is keyed by the nested genvm_id, while finish_execution only removes exec.id (the parent). So every nested run permanently adds an entry to the global map. Forgetting the guard should be limited to the top-level case, or the nested path should remove its own key when run_genvm_process returns.
🔒️ Proposed fix
- log_sink_guard.forget();
if is_top_level {
+ log_sink_guard.forget();
exec_ctx.publish_started();
}📝 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.
| log_sink_guard.forget(); | |
| Ok((genvm_id, rx)) | |
| } | |
| #[cfg(test)] | |
| mod tests { | |
| use super::keep_tail; | |
| #[test] | |
| fn under_or_at_limit_is_unchanged() { | |
| let mut s = String::from("hello"); | |
| keep_tail(&mut s, 10); | |
| assert_eq!(s, "hello"); | |
| keep_tail(&mut s, 5); | |
| assert_eq!(s, "hello"); | |
| if is_top_level { | |
| exec_ctx.publish_started(); | |
| } | |
| if is_top_level { | |
| log_sink_guard.forget(); | |
| exec_ctx.publish_started(); | |
| } |
🤖 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 2430 - 2433, Update the
cleanup logic in run_genvm_process so log_sink_guard is only forgotten for
top-level executions, or explicitly removes the nested run’s own
GENVM_BY_ID_LOGGER entry before returning. Ensure nested runs do not leave
entries keyed by their nested genvm_id while preserving the existing
publish_started behavior for top-level runs.
* feat(manager): run cross-major contract calls as nested runs ✨ * feat(manager): make cross-contract call resolution opt-in ✨ * feat(manager): refuse nested envelopes asserting unserviceable permissions 🔒️ * fix(manager): refuse a nested reply carrying any reported effect 🔒️🐛 * fix(manager): stop the call route from being consensus-visible 🐛🔒️ * refactor(manager): serve executions over an event-driven websocket ♻️ * fix(manager): harden the run socket protocol 🔒️🐛 * fix(manager): drop runtime env mutation and open the lua stdlib 🔒️ * chore(codegen): generate host-fns and socket ids from one definition ♻️ * refactor(harness): compare the executor execution hash directly ♻️ * test(harness): route a call across a major boundary from jsonnet ✅
7ecb349 to
2e3032d
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
implementation/src/scripting/mod.rs (1)
160-166: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winAdd rationale before exposing
StdLib::OSto module Lua scripts.No shipped Lua script currently uses
os, while this exposes command execution, filesystem removal/rename, temp-file creation, and environment inspection to operator-authored module VMs. Only enable the specific utilities needed, such as injectingos.time/os.date/os.clockdirectly.🤖 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/scripting/mod.rs` around lines 160 - 166, Update the `lua_libs` configuration in the module scripting setup to remove `StdLib::OS` and avoid exposing the full OS library to module Lua scripts. Preserve only the required standard libraries, and provide narrowly scoped time utilities such as `os.time`, `os.date`, or `os.clock` through the existing Lua setup if needed; add the requested rationale at the configuration point.
🧹 Nitpick comments (7)
implementation/src/manager/run.rs (3)
2106-2110: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winPost-start failures are reported as
FailedToStart.
supervise_genvm_innercan fail afterpublish_started(e.g.child.wait()error, missing process handle), and every such error funnels intofail_to_start, so a client that already receivedStartedthen receivesFailedToStartfor the same run. Consider emitting aFinished-style terminal event (with the recorded finish cause) whenstarted_eventis already set.🤖 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 2106 - 2110, Update the error handling around supervise_genvm_inner so failures after publish_started do not call fail_to_start; when started_event is already set, emit the existing Finished-style terminal event using the recorded finish cause, while preserving fail_to_start for pre-start failures.
354-376: 🚀 Performance & Scalability | 🔵 Trivial | ⚖️ Poor tradeoffFull genvm-log re-serialization per size/chunk request.
genvm_log_json_linesrebuilds the whole log on everyartifact_sizes_forand everyget_artifactcall, so a paginated client re-serializes the entire log for each 256 KiB chunk. Caching the serialized bytes (e.g. aOnceCell<Vec<u8>>on the done result) would make chunking O(size) once instead of per request.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@implementation/src/manager/run.rs` around lines 354 - 376, Cache the serialized genvm-log bytes on the completed result and reuse them across artifact_sizes_for and get_artifact calls. Update genvm_log_json_lines and the SingleGenVMContextDone construction/access paths so serialization occurs once, while preserving the existing newline format and error handling.
540-554: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffError kinds are carried as magic strings across a module boundary.
attachsignalsboot_id_mismatch/unknown_idviaanyhow::bail!, andsocket.rs(Line 571) recovers them withe.to_string() == "boot_id_mismatch";get_artifacthas the same pattern with"unknown_id". Any context added to these errors silently reclassifies them into the wrong protocol error code. A small enum (oranyhow::Error::downcast_ref) would make the mapping compile-checked.🤖 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 540 - 554, Replace the string-based errors in the manager methods attach and get_artifact with a shared typed error kind, such as an enum, and preserve the boot_id_mismatch and unknown_id variants. Update socket.rs error mapping to downcast or otherwise match those typed variants instead of comparing e.to_string(), while retaining the existing protocol error codes.implementation/src/manager/socket_test.rs (1)
95-137: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCoverage gap: the
try_sendpath is untested.These tests exercise
forward_events, which usessend_control_asyncand correctly parks. The reply/error path fromdispatchusesWriter::send_control(try_send) and tears the connection down when the queue is full — the case flagged insocket.rs. A test asserting that a full control queue does not kill the connection on a malformed-frame reply would pin that behavior down.🤖 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 95 - 137, Add a test covering the dispatch reply/error path that uses Writer::send_control, filling the capacity-1 control queue before submitting a malformed frame. Assert the malformed-frame reply is eventually delivered once the queued frame is consumed and that the connection remains usable afterward, covering the full-queue try_send behavior in dispatch rather than only forward_events.implementation/src/manager/run_test.rs (1)
340-378: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider trimming
ROUNDSor documenting the cost.10 000 rounds each allocate a full
SingleGenVMContext(watch channel, semaphores, notifies) that stay live inknown_executionsfor the whole test, plus a rendezvous handshake per round. A smaller sweep gives essentially the same race coverage at a fraction of the CI time/memory.Also, the comment on Line 351-354 claims no panics inside the scope, but
start_tx.send(()).unwrap()on Line 378 is one (it is unreachable only because round counts match the publisher loop).🤖 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_test.rs` around lines 340 - 378, Reduce the ROUNDS count in this race test or document why 10,000 iterations are necessary, while preserving meaningful race coverage and lowering CI resource usage. In the scoped loop around start_tx.send, remove the unwrap-based panic path by handling a failed send consistently with the publisher’s early-exit behavior, and update the nearby no-panics comment to match the implementation.implementation/Cargo.toml (1)
33-33: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winPin
axumto a concrete 0.8.x version.
implementation/Cargo.lockhasaxum0.8.9, butimplementation/Cargo.tomluses the rollingcxum = { version = "0.8", features = ["ws"] }requirement, so future lockfile regenerations can change the dependency graph unexpectedly. Pin it to0.8.9here as well.🤖 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/Cargo.toml` at line 33, Update the axum dependency declaration in Cargo.toml to require the concrete 0.8.9 version while preserving the existing ws feature.docs/website/src/spec/03-vm/01-startup.rst (1)
236-240: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCross-reference
error_invalas a code symbol.This result-path text uses a bare literal for ``error_inval``` while nearby spec passages link VM error conditions with
:ref:targets. Use the matching error-code reference target so the spec remains cross-linkable if the error name is changed.🤖 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/03-vm/01-startup.rst` around lines 236 - 240, Update the result-path text in the startup specification to replace the bare ``error_inval`` literal with the matching cross-reference target used for VM error codes, while preserving the existing wording and behavior description.
🤖 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/contributing/howto/testing/integration.md`:
- Around line 59-69: Update the documentation paragraph describing --ignore-hash
to clarify that it skips comparison with stored .hash sidecars and prevents
their creation, while runtime leader-versus-validator/sync cross-mode comparison
remains enabled.
In `@docs/website/src/impl-spec/appendix/host-loop.rst`:
- Around line 40-44: Update the host-loop documentation around
resolve_callcontract_executor to state that a null reply only keeps locally
served majors in-process; callers unable to serve the advisory major must mint a
major selector and delegate the CallContract instead. Preserve the existing
hook_cross_contract_calls and host-0 routing description.
In `@docs/website/src/spec/03-vm/01-startup.rst`:
- Around line 19-21: Update the startup specification around the root-slot and
locked-slot read description to state the combined condition: the run must be
allowed to write storage and the sender must not be in upgraders. Add a
cross-reference to :ref:`gvm-def-locked-slot-nesting` so this rule remains
consistent with the upgradability specification.
In `@docs/website/src/spec/04-contract-interface/04-upgradability.rst`:
- Around line 54-59: Disambiguate the public-ABI version reference in the
upgradability documentation by linking or qualifying CURRENT_MAJOR with its
constants-appendix entry, distinct from manager_api::CURRENT_MAJOR. Update the
paragraph around the contract major-version comparison while preserving the
existing mismatch behavior and references.
In `@implementation/src/manager/handlers.rs`:
- Around line 233-236: Remove the unconditional ctx.run_ctx.ack(genvm_id) call
from handle_genvm_status so reading an available status does not consume the
execution. If legacy behavior must remain supported, only invoke ack when an
explicit legacy-query flag is present, while preserving status retrieval for
normal GET requests.
In `@implementation/src/manager/run.rs`:
- Around line 1246-1260: Replace the unbounded u32::MAX limits passed by
consume_result and run_nested with a concrete manager-side cap, preferably
derived from config.max_message_bytes, and pass that cap to
read_length_prefixed. Ensure read_length_prefixed rejects frames above the cap
before allocation, preserving normal reads for valid frames and preventing
oversized or truncated child-controlled headers from causing large manager
allocations.
---
Outside diff comments:
In `@implementation/src/scripting/mod.rs`:
- Around line 160-166: Update the `lua_libs` configuration in the module
scripting setup to remove `StdLib::OS` and avoid exposing the full OS library to
module Lua scripts. Preserve only the required standard libraries, and provide
narrowly scoped time utilities such as `os.time`, `os.date`, or `os.clock`
through the existing Lua setup if needed; add the requested rationale at the
configuration point.
---
Nitpick comments:
In `@docs/website/src/spec/03-vm/01-startup.rst`:
- Around line 236-240: Update the result-path text in the startup specification
to replace the bare ``error_inval`` literal with the matching cross-reference
target used for VM error codes, while preserving the existing wording and
behavior description.
In `@implementation/Cargo.toml`:
- Line 33: Update the axum dependency declaration in Cargo.toml to require the
concrete 0.8.9 version while preserving the existing ws feature.
In `@implementation/src/manager/run_test.rs`:
- Around line 340-378: Reduce the ROUNDS count in this race test or document why
10,000 iterations are necessary, while preserving meaningful race coverage and
lowering CI resource usage. In the scoped loop around start_tx.send, remove the
unwrap-based panic path by handling a failed send consistently with the
publisher’s early-exit behavior, and update the nearby no-panics comment to
match the implementation.
In `@implementation/src/manager/run.rs`:
- Around line 2106-2110: Update the error handling around supervise_genvm_inner
so failures after publish_started do not call fail_to_start; when started_event
is already set, emit the existing Finished-style terminal event using the
recorded finish cause, while preserving fail_to_start for pre-start failures.
- Around line 354-376: Cache the serialized genvm-log bytes on the completed
result and reuse them across artifact_sizes_for and get_artifact calls. Update
genvm_log_json_lines and the SingleGenVMContextDone construction/access paths so
serialization occurs once, while preserving the existing newline format and
error handling.
- Around line 540-554: Replace the string-based errors in the manager methods
attach and get_artifact with a shared typed error kind, such as an enum, and
preserve the boot_id_mismatch and unknown_id variants. Update socket.rs error
mapping to downcast or otherwise match those typed variants instead of comparing
e.to_string(), while retaining the existing protocol error codes.
In `@implementation/src/manager/socket_test.rs`:
- Around line 95-137: Add a test covering the dispatch reply/error path that
uses Writer::send_control, filling the capacity-1 control queue before
submitting a malformed frame. Assert the malformed-frame reply is eventually
delivered once the queued frame is consumed and that the connection remains
usable afterward, covering the full-queue try_send behavior in dispatch rather
than only forward_events.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 8faad9b9-bdd8-453c-bc07-3fb12f6caa1c
⛔ Files ignored due to path filters (23)
crates/modules-interfaces/Cargo.lockis excluded by!**/*.lock,!**/*.lockimplementation/Cargo.lockis excluded by!**/*.lock,!**/*.locksupport/tools/genvm-tool/genvm_tool/__main__.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_howto.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/rst.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/genvm_tool/tests/util/watchdog_main.pyis excluded by!support/tools/genvm-tool/**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/logger.pyis excluded by!tests/**tests/runner/origin/manager_api.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/manager-socket/test.pyis excluded by!tests/**tests/system/permits/test.pyis excluded by!tests/**tests/templates/util.jsonnetis excluded by!tests/**
📒 Files selected for processing (76)
.claude/settings.fuzzing.json.claude/skills/agentic-fuzzing/SKILL.md.genvm-tool.pyCLAUDE.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/README.mddocs/adr/014. manager host socket protocol.mddocs/adr/015. cross-major contract calls.mddocs/contributing/README.mddocs/contributing/explanation/README.mddocs/contributing/explanation/docs-layout.mddocs/contributing/explanation/executor-lines.mddocs/contributing/explanation/merge-model.mddocs/contributing/explanation/vendored-trees.mddocs/contributing/howto/README.mddocs/contributing/howto/building/build.mddocs/contributing/howto/building/docs.mddocs/contributing/howto/building/runners.mddocs/contributing/howto/committing/git-third-party.mddocs/contributing/howto/committing/runners.mddocs/contributing/howto/committing/submodules.mddocs/contributing/howto/docs/style.mddocs/contributing/howto/extending/add-host-function.mddocs/contributing/howto/extending/add-llm-provider.mddocs/contributing/howto/extending/add-wasi-function.mddocs/contributing/howto/extending/modify-runner.mddocs/contributing/howto/extending/modify-wasmtime.mddocs/contributing/howto/extending/write-a-script.mddocs/contributing/howto/genvm-tool.mddocs/contributing/howto/pr.mddocs/contributing/howto/releasing/release-build.mddocs/contributing/howto/releasing/versioning.mddocs/contributing/howto/setup.mddocs/contributing/howto/testing/README.mddocs/contributing/howto/testing/fuzzing.mddocs/contributing/howto/testing/integration.mddocs/contributing/howto/testing/python.mddocs/contributing/howto/testing/rust.mddocs/contributing/howto/write-a-script.mddocs/contributing/tutorial/README.mddocs/contributing/tutorial/first-contribution.mddocs/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/03-vm/01-startup.rstdocs/website/src/spec/03-vm/02-meta-properties.rstdocs/website/src/spec/03-vm/05-result.rstdocs/website/src/spec/04-contract-interface/04-upgradability.rstdocs/website/src/spec/appendix/constants.rstexecutors/v0.2.xexecutors/v0.3.ximplementation/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.rsinstall/config/genvm-manager.yamlsupport/scripts/check-source-text.pysupport/scripts/get-all-git.py
💤 Files with no reviewable changes (2)
- .claude/settings.fuzzing.json
- docs/contributing/howto/write-a-script.md
🚧 Files skipped from review as they are similar to previous changes (24)
- docs/website/src/impl-spec/appendix/index.rst
- docs/website/src/spec/03-vm/05-result.rst
- docs/website/src/spec/appendix/constants.rst
- docs/contributing/explanation/vendored-trees.md
- docs/website/src/spec/02-execution-environment/03-wasi_genlayer_sdk/02-gl_call.rst
- docs/website/src/impl-spec/appendix/manager-api.rst
- docs/contributing/README.md
- docs/contributing/howto/extending/modify-wasmtime.md
- docs/contributing/howto/building/build.md
- docs/contributing/howto/building/runners.md
- docs/contributing/howto/extending/add-wasi-function.md
- docs/contributing/howto/committing/runners.md
- docs/contributing/howto/README.md
- docs/contributing/howto/building/docs.md
- docs/contributing/howto/testing/rust.md
- docs/contributing/howto/genvm-tool.md
- docs/contributing/howto/testing/fuzzing.md
- docs/contributing/howto/releasing/release-build.md
- docs/contributing/howto/docs/style.md
- docs/contributing/howto/releasing/versioning.md
- docs/contributing/explanation/executor-lines.md
- docs/website/src/impl-spec/appendix/manager-socket.rst
- docs/website/src/impl-spec/appendix/manager-api.yaml
- install/config/genvm-manager.yaml
| `--ignore-hash` skips both comparison and sidecar creation — needed in runner | ||
| dev mode, where every execution hash moves | ||
| ([modify-runner.md](../extending/modify-runner.md)), not for a new case. A line | ||
| can stop tracking sidecars altogether by returning `{'save-hashes': False}` from | ||
| `integration()` in its `.genvm-tool.py`, as v0.3.x currently does while its | ||
| hashes still move | ||
|
|
||
| Neither switch turns off the leader-vs-validator/sync comparison: with sidecars | ||
| disabled a non-main mode is compared against the main mode's hash from the same | ||
| run instead. Only that comparison makes a determinism regression visible, and a | ||
| non-main step has no semantics goldens of its own, so without it the step would |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Clarify which hash comparison --ignore-hash disables.
“Skips both comparison and sidecar creation” conflicts with the following statement that leader/validator/sync comparison remains enabled. Specify that it skips the stored .hash sidecar comparison and creation, while runtime cross-mode comparison still runs.
🤖 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/testing/integration.md` around lines 59 - 69, Update
the documentation paragraph describing --ignore-hash to clarify that it skips
comparison with stored .hash sidecars and prevents their creation, while runtime
leader-versus-validator/sync cross-mode comparison remains enabled.
| ``resolve_callcontract_executor`` moves to host 0 only when the run request sets | ||
| ``hook_cross_contract_calls`` (see :doc:`manager-socket`). Otherwise the manager | ||
| answers it with a null reply, which keeps every ``CallContract`` in-process. A | ||
| host that does not route calls across major boundaries therefore need not | ||
| implement that method at all. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Correct the null-resolution behavior.
Line 42 incorrectly says a null reply keeps every CallContract in-process. A caller that cannot serve the advisory major must mint a major selector and delegate; only locally served majors stay in-process.
Proposed fix
-answers it with a null reply, which keeps every ``CallContract`` in-process. A
-host that does not route calls across major boundaries therefore need not
-implement that method at all.
+answers it with a null reply. The caller keeps the call in-process only when it
+serves the advisory major; otherwise it mints a major selector and delegates,
+where the selected line produces the canonical mismatch if no compatible line
+is installed. A host that does not route calls across major boundaries therefore
+need not implement that method at all.📝 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.
| ``resolve_callcontract_executor`` moves to host 0 only when the run request sets | |
| ``hook_cross_contract_calls`` (see :doc:`manager-socket`). Otherwise the manager | |
| answers it with a null reply, which keeps every ``CallContract`` in-process. A | |
| host that does not route calls across major boundaries therefore need not | |
| implement that method at all. | |
| ``resolve_callcontract_executor`` moves to host 0 only when the run request sets | |
| ``hook_cross_contract_calls`` (see :doc:`manager-socket`). Otherwise the manager | |
| answers it with a null reply. The caller keeps the call in-process only when it | |
| serves the advisory major; otherwise it mints a major selector and delegates, | |
| where the selected line produces the canonical mismatch if no compatible line | |
| is installed. A host that does not route calls across major boundaries therefore | |
| need not implement that method at all. |
🤖 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/host-loop.rst` around lines 40 - 44,
Update the host-loop documentation around resolve_callcontract_executor to state
that a null reply only keeps locally served majors in-process; callers unable to
serve the advisory major must mint a major selector and delegate the
CallContract instead. Preserve the existing hook_cross_contract_calls and host-0
routing description.
| #. Reads the contract's root-slot data, and its locked slots when the run may | ||
| write storage. Locked slots only constrain writes, so a run without that | ||
| permission does not read them. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Reconcile the locked-slot read condition with the upgradability page.
Here the read is gated on "the run may write storage"; docs/website/src/spec/04-contract-interface/04-upgradability.rst (Lines 16-18) gates it on the sender not being in upgraders. Both are presumably conjuncts, but as written a reader gets two different rules. Consider stating the combined condition here and linking to :ref:gvm-def-locked-slot-nesting.
🤖 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/03-vm/01-startup.rst` around lines 19 - 21, Update the
startup specification around the root-slot and locked-slot read description to
state the combined condition: the run must be allowed to write storage and the
sender must not be in upgraders. Add a cross-reference to
:ref:`gvm-def-locked-slot-nesting` so this rule remains consistent with the
upgradability specification.
| :ref:`gvm-def-consts-value-root-offsets-major` and stores the public-ABI major | ||
| version that the contract was built against. Top-level and runner loads compare | ||
| this byte to ``CURRENT_MAJOR`` and fail with | ||
| :ref:`gvm-def-str-trie-value-vm-error-invalid-contract-major-mismatch` when it | ||
| differs. :ref:`gvm-def-gl-call-call-contract` may instead delegate the callee to | ||
| another executor selected by the host. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Disambiguate CURRENT_MAJOR.
The name is also used for the manager socket protocol major (manager_api::CURRENT_MAJOR, implementation/src/manager/socket.rs Lines 306-315). Since this one is the public-ABI major, link it to its constants-appendix entry (or qualify it in prose) so the two are not read as the same 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 `@docs/website/src/spec/04-contract-interface/04-upgradability.rst` around
lines 54 - 59, Disambiguate the public-ABI version reference in the
upgradability documentation by linking or qualifying CURRENT_MAJOR with its
constants-appendix entry, distinct from manager_api::CURRENT_MAJOR. Update the
paragraph around the contract major-version comparison while preserving the
existing mismatch behavior and references.
| let status = ctx.run_ctx.status(genvm_id); | ||
| if status.is_some() { | ||
| ctx.run_ctx.ack(genvm_id); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Do callers/docs expect GET status to consume the execution?
rg -n -C3 'genvm_id.*artifact|/genvm/' docs/website/src/impl-spec/appendix/manager-api.yaml
rg -n -C3 'handle_genvm_status|\back\(' implementation/src --type=rustRepository: genlayerlabs/genvm-manager
Length of output: 5260
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## run.rs ack/status implementations"
sed -n '520,600p' implementation/src/manager/run.rs | cat -n
echo
echo "## handlers.rs status handler"
sed -n '208,280p' implementation/src/manager/handlers.rs | cat -n
echo
echo "## api spec around /genvm/{genvm_id}"
sed -n '220,235p' docs/website/src/impl-spec/appendix/manager-api.yaml | cat -n
echo
echo "## websocket ack routing"
sed -n '600,635p' implementation/src/manager/socket.rs | cat -n
echo
echo "## StatusRequest definition"
rg -n -C5 'struct StatusRequest|pub struct StatusRequest|StatusRequest' implementation/src/manager -g '*.rs'Repository: genlayerlabs/genvm-manager
Length of output: 13686
Do not auto-ack when returning status via /genvm/{id}.
handle_genvm_status currently calls ctx.run_ctx.ack(genvm_id) whenever status is present; ack() removes the execution and releases the token, so the first successful GET also makes retries/monitoring reads return null. Gate auto-ack behind an explicit legacy-query flag if needed.
🤖 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/handlers.rs` around lines 233 - 236, Remove the
unconditional ctx.run_ctx.ack(genvm_id) call from handle_genvm_status so reading
an available status does not consume the execution. If legacy behavior must
remain supported, only invoke ack when an explicit legacy-query flag is present,
while preserving status retrieval for normal GET requests.
| 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
u32::MAX frame cap lets the child force a 4 GiB allocation in the manager.
read_length_prefixed allocates vec![0; len] up-front, and both consume_result and run_nested pass u32::MAX as max_len. The comment argues the bound is the child's memory limit, but the allocation happens in the manager process, which has no such limit — and the child is the process running untrusted contract code. A single bogus length header (or a truncated frame after a large header) is enough to spike manager RSS; with several concurrent runs it is an OOM.
Consider a concrete cap (e.g. tied to config.max_message_bytes) and/or incremental reads with try_reserve instead of a single pre-sized allocation.
🛡️ Sketch
- let data = match read_length_prefixed(&mut reader, u32::MAX as usize).await {
+ let data = match read_length_prefixed(&mut reader, MAX_HOST_FRAME_LEN).await {Also applies to: 1467-1467, 1491-1491
🤖 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 1246 - 1260, Replace the
unbounded u32::MAX limits passed by consume_result and run_nested with a
concrete manager-side cap, preferably derived from config.max_message_bytes, and
pass that cap to read_length_prefixed. Ensure read_length_prefixed rejects
frames above the cap before allocation, preserving normal reads for valid frames
and preventing oversized or truncated child-controlled headers from causing
large manager allocations.
|
/run-e2e |
1 similar comment
|
/run-e2e |
github-actions
Bot
added
the
not rebased
1 participant
Changeset
Depends-On: genlayerlabs/genlayer-node#1647
Depends-On: genlayerlabs/genlayer-studio#1716
Depends-On: #9
Summary by CodeRabbit