Repository navigation
nvx-rs: lay out execution-helper configurations with one function - #689
Merged
Pedro Henrique Penna (ppenna) merged 1 commit intoOct 22, 2026
Merged
Conversation
The managed agent hands a workload's working directory and environment to its execution helper in a sealed configuration, which exec_config::encode lays out: the little-endian flags, entry count, and working-directory length, then the working directory, then each entry after its little-endian length. The tests of exec_config.rs and helper.rs spelled that layout out again, byte by byte, for six configurations that encode never produces, with unknown flags, malformed entries, a relative working directory, and conflicting working-directory flags, and both test modules defined the same file_with helper. encode now computes the flags and leaves the layout to encode_fields, which takes the flags, the working directory, empty without one, and the entries. The six fixtures call encode_fields with the fields that they vary: the four configurations that exec_config's rejection test reads, and the relative and conflicting working directories of helper's. file_with moves beside read, under cfg(test), and serves both test modules. The two files lose 25 lines. Behavior is unchanged: encode_fields writes the bytes that encode wrote, in the same order, and checks the entry count, the working-directory length, and each entry's length for EOVERFLOW in the same order; a configuration without a working directory has length 0 and no working-directory bytes, as before. Each fixture encodes the bytes of its hand-written version, whose count and length were those of the entries and working directory that followed them. Only the managed agent changes, and only in this file's private layout code. The managed agent's unit tests cover the change: exec_config's round-trip and sealing tests, its rejection test, and the helper tests that read these configurations. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The narrow refactor preserves serialization, overflow checks, and test behavior, with no blocking issues identified.
No open issues 🎉
What changed in this PR
Consolidates execution-helper configuration encoding in the managed agent, preserving the byte layout and existing validation behavior.
Changes:
- Shares
encode_fieldsbetween production encoding and six rejection fixtures. - Consolidates the test-only
file_withhelper without changing assertions.
| File | Description |
|---|---|
nvx-rs/src/guest/managed_agent/helper.rs |
Uses shared helpers to construct and open rejection fixtures. |
nvx-rs/src/guest/managed_agent/exec_config.rs |
Extracts the shared encoder and test file helper. |
Review effort: Balanced
This was referenced Oct 21, 2026
Pedro Henrique Penna (ppenna)
deleted the
nvx-rs-quality/exec-config-layout
branch
October 22, 2026 03:16
This was referenced Oct 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The managed agent hands each workload's working directory and environment to its execution helper in a sealed configuration, which
exec_config::encodeinnvx-rs/src/guest/managed_agent/exec_config.rslays out: the little-endian flags, entry count, and working-directory length, then the working directory, then each entry after its little-endian length. The tests ofexec_config.rsandhelper.rsneed configurations thatencodenever produces, with unknown flags, malformed entries, a relative working directory, and conflicting working-directory flags, so they spelled that layout out again, byte by byte, six times:and both test modules defined the same
file_with, which puts bytes in a file thatexec_config::readreads.encodenow computes the flags and leaves the layout toencode_fields(flags, cwd, environment), which the six fixtures call with the fields that they vary, as inencode_fields(EXEC_ENVIRONMENT_PRESENT, b"", &[b"NO_EQUALS".to_vec()]). The layout has one definition, which the managed agent and the tests share, and each fixture names its flags, working directory, and entries rather than their encodings.file_withmoves besideread, undercfg(test), and serves both test modules. #685 listed this as a follow-up.This is the twenty-eighth
nvx-rs-quality/pull request. The 19 from #603 to #637 merged, as did #678, and #679 through #685 are open. Each had a Copilot overview review that recommended approval with no comment to follow, #636 and #637 also have approvals without comments to follow, and none was closed without merging. This run surveyed the crate by reading, and with a throwaway script, kept outside the repository, that reports every window of six or more whitespace-normalized lines that repeats insrcandtests, ignoring blank lines, comments, and lines of punctuation, exactly and with names and literals abstracted, and once more over production code alone. It skipped every file that an open pull request todevchanged when the run started, among themREADME.md,Cargo.toml,src/lib.rs,host/mod.rs,guest/mod.rs, the managed agent'scgroup,dispatch,exec,launch,mappings,mod, andseam,guest/sys/{fd,mod,port,process}.rs,guest/{spawn,status,nvxs}.rs,guest/time,guest/time_probe, the guest programs' entry points,guest/ingress/mod.rs,host/{instance,process,filesystem}.rs,host/platform/{linux,other,windows}.rs,host/sys/process.rs,host/store/{mod,tests}.rs,host/sandbox/{mod,mount,options,distro,golden}.rs,host/cli/session.rs,host/cli/sandbox/{lifecycle,mod,state}.rs,host/network/{lowering,policy/mod}.rs,host/python/golden.rs, andtests/{cli,managed_lifecycle,supervisor}.rs.Production code outside those files has little duplication left: the survey's exact repeats of five or more lines there are the NVXS session roles'
receiveandsend, the two EXEC payload decoders, whose text and byte rules differ, and the Windowsstatandlstatofhost/python/fs.rs, and its near repeats are mostly accessors, enum names, and test tables. Of the candidates, this one gives the most per changed line: 115 changed lines that remove 25, in two files that nonvx-rs-quality/pull request has changed yet, replacing six hand-written encodings and a duplicated helper with calls of the format's own layout. Follow-ups lists the others.Where the choice was open, I took the conservative option:
encode_fieldsispub(super), visible only in the managed agent, and keepsencode'sResult<Vec<u8>, Errno>and itsEOVERFLOWchecks, so the fixtures unwrap a result that cannot fail for them rather than call a test-only encoder that would spell the layout a seventh time.encodecomputes as before, say whether one is present.file_withkeeps using a temporary file, as both copies did, rather than the sealed memfd thatcreatemakes, and sits besidereadas acfg(test)function rather than in a new test-support module.readkeeps decoding the layout field by field from its descriptor, with its checks in their order.Changes
Two files, 45 insertions and 70 deletions:
nvx-rs/src/guest/managed_agent/exec_config.rs:encodecomputes the flags as before and returnsencode_fields(flags, cwd.unwrap_or_default(), execution.environment.as_deref().unwrap_or_default()).encode_fields(flags, cwd, environment),pub(super), with a doc comment: the layout thatencodewrote, which now writes the working directory's bytes unconditionally, none when it is empty, rather than matching anOption.file_with(bytes),cfg(test)andpub(super), with a doc comment, moved from the test module.helper_defensively_rejects_unknown_flags_and_malformed_environmentbuilds its four configurations withencode_fields.nvx-rs/src/guest/managed_agent/helper.rs:helper_rejects_bad_syntax_relative_cwd_and_conflicting_cwd_flagsbuilds its relative and conflicting configurations withexec_config::encode_fieldsand opens them withexec_config::file_with; the test module'sfile_withand itsSeekandWriteimports go.The sites that now share the layout are
encode, the four rejection fixtures ofexec_config.rs, and the two ofhelper.rs; the tests of both files sharefile_with. No public item, message, error kind, protocol byte, fixture file, or other file changes.Behavior
Unchanged:
encode_fieldswrites the bytes thatencodewrote, in the same order, and checks the entry count, the working-directory length, and each entry's length forEOVERFLOWin the same order.cwd.unwrap_or_default()is empty exactly whenencodeused to skip the working directory, whose length it then wrote as 0. Every sealed configuration that the managed agent hands its helper is byte-for-byte the same.Each fixture encodes the bytes of its hand-written version, whose entry count and working-directory length were those of the entries and working directory that followed them:
0x4000=EXEC_ENVIRONMENT_PRESENTNO_EQUALSEXEC_ENVIRONMENT_PRESENT=valueEXEC_CWD_ENTERED/EXEC_CWD_PRESENT.EXEC_CWD_PRESENT | EXEC_CWD_ENTERED/Tests that cover it, none of them added, removed, skipped, or weakened:
exec_config.rs,helper_configuration_round_trips_empty_exact_and_large_environmentsandhelper_configuration_marks_an_entered_directory_and_is_sealed, which encode throughcreateand read back, andhelper_defensively_rejects_unknown_flags_and_malformed_environment;helper.rs,helper_applies_exact_empty_and_layered_environments,helper_rejects_bad_syntax_relative_cwd_and_conflicting_cwd_flags,helper_does_not_exec_when_an_environment_export_fails, andhelper_keeps_a_directory_that_the_caller_entered;create.These ran in the gates of the Linux/KVM host, with glibc and, in
check-guest-binaries, with musl. Every workload that the managed agent launches passes throughencodeand its helper'sread, so the hypervisor tests ran the change too: the guest binaries were rebuilt from TREE into the Alpine initramfs that every host booted, and themanaged-lifecyclescenario and theaci_edge_sandboxesend-to-end tests run workloads with and without working directories and environments.Follow-ups
Not in this pull request, to keep it to one change. I found no suspected bug.
protocol/nvxs/guest.rsandhost.rs: each role'ssendrepeats the same four lines after its state check, which check the next sequence number, encode the DATA record, and advance;encode_datacould take the sequence number by reference and advance it, and a helper of that shape could serve the guest'scredittoo.receiverepeats nine lines that need a trait over each role's parser,accept, andfail.guest/ingress/encode.rs'sListenerTable::addandguest/ingress/diag.rs'snote_wildcard_tcprepeat a bounded find-or-insert merge; their shared parent,guest/ingress/mod.rs, is changed by nvx-rs: run the guest programs' main through one shared function #679.host/python/fs.rs: the Windowsstatandlstatbuild the sameStat; astatus(path, traverse)helper would mirror the POSIX implementation'sstatus.net::is_valid_guest_networkandguest_subnetinhost/network/mod.rsboth parse an IPv4ADDRESS/PREFIXand compute its mask, with prefix limits of 1 to 30 and 1 to 32.host/sandbox/tests.rsandhost/sandbox/mount/tests.rsdefine the samepolicyandnativetest helpers; the share tests could import the launch tests' helpers, which they can reach as a descendant ofhost::sandbox.protocol/outer.rs'sencode_outerandprotocol/nvxs/record.rs'sencode_recordwrite the same 44-byte header but validate differently, so one cannot simply call the other; one header writer would give the layout one definition, though it saves no lines.Validation
BASE:
fbefbfccdc341bdebc9f0aa0e742fe351846c114,origin/devwhen the run started. Before this pull request was opened,origin/devadvanced to59712ebddby the merges of microvm tests: stream OpenVMM transcripts to disk as they are read #659 and ci: run the OpenVMM suites on dev pushes only when their inputs change #660 and a baseline update, which change CI workflows,scripts/, documentation, anddata/, none of this pull request's files or anything undernvx-rs/,aci_edge_sandboxes/,tests/, oropenvmm, and no open pull request todevchanges either file.TREE:
4f07eecba7cf1359cc0af7055f1e29e3ca68b80f, the tree of this commit. Every host applied the patch to BASE and checked thatgit write-treeequals TREE.Release on every host:
v0.1.0-dev.c30b75afe464, the nearest first-parent ancestor of BASE that had packages, one commit earlier. Its OpenVMM,d0bdbed8f6e3b86271277dc22658109e2e633b2d, is the pinned revision, as each host checked inbuild/openvmm.provenance.json.nvx-rs:downloadinstalled the release'snvx-rs, which other sources built, so each host rebuilt it from TREE with the Cargo command with whichnvx.pybuilds it, after its gates and before its hypervisor tests. The executable's digest changed,record-nvx-rs-provenancerecorded it, and each host checked that the record names TREE's sources (source_sha256bcff89560061ff8d8bfddcf6e12b50290defbc979f293b93ad6132ba16a35fbc) and the executable. Each host then built theopenvmm_e2eandmanaged_lifecycletest binaries, whose runs in the hypervisor tests compiled nothing.Guest artifacts were rebuilt from TREE on prometheus30, whose user namespaces let Alpine's apk run its scripts (prometheus32 denies them):
build-guest-binaries --output, which compilednvxwith theguestprofile;test-guest-binaries --binaries, where 34 tests passed;build-initramfswithNVX_GUEST_BINARIES, which installed the newnvx-managed-agent.initramfs.cpio.gz(SHA-25618d9183a7233c813bb4210c97d72194f6a69d9f35126ea633a86f32c87b5060e), its packages manifest (ac7d2d3211bb368b904900f07a2bb438bf2588e3847958030f96a8aad08e68a9), and its provenance (7a8315f1c6ae7868cb771e5375d00c1f04beecdb83e0dabb62d055e3bbdd9422) went through this machine to the other two hosts, where the SHA-256 digests matched.Local checks while iterating, with Rust 1.95.0: rustfmt on Windows, and on WSL Ubuntu, since the
guestmodule builds only on Linux, Clippy with-D warningson all targets with all features and on the library without default features, and the managed agent's 53 unit tests with all features.This machine also ran the steps that no host could run, because the hosts lack the toolchain or target: on Windows, the MSRV (1.89) checks of
nvx,aci_edge_sandboxes, and the runtime adapter's protocol-only and host-only configurations; and, on WSL Ubuntu, the macOS (aarch64-apple-darwin) checks ofnvxandaci_edge_sandboxesand the MSRV check of the runtime adapter's guest-only configuration, which ran as the action defines it, starting with itsrustup target add, which gave the local 1.89 the musl target. I removed that target again afterwards. All passed.The change adds, moves, and deletes no file and changes no Markdown, so the CLI tests of validate-nvx and the link check had nothing to cover.
The hosts ran each action's steps with the toolchains and targets that they have, skipping the actions' installation commands; nothing was installed on them.
test-aci-edge-sandboxestest-microvm, default suitenvxtests: 736 in 12 test binaries with all features and 359 in 10 with the default ones, besides the runs of single tests that some tests make of their own binary; artifact bundling (15);check-guest-binaries(384); the runtime adapter's protocol-only, host-only, and musl guest-only configurations (13, 14, and 21 tests); 158 and 85aci_edge_sandboxestests over the two feature sets; agent bundling (4). The macOS checks and the guest-only MSRV check ran locallynvxtests: 429 in 12 test binaries with all features and 358 in 10 with the default ones; artifact bundling (15); the runtime adapter's protocol-only and host-only configurations (13 and 14 tests); 158 and 85aci_edge_sandboxestests over the two feature sets; agent bundling (4). The MSRV checks ran locallyEvery gate and hypervisor test passed at its first run. The Linux gates ran in the foreground of their SSH session, where no signal was ignored (
SigIgn: 0), so the CLI's Ctrl-C tests ran as intended. Each suite'smanaged-lifecyclescenario ran themanaged_lifecycletest with Cargo, which passed. No host warned of a CPU-profile fallback. The time-ABI evidence lines counted no boot, capture, or restore over its CPU-time budget on any host. On every host, oneopenvmm_e2etest printedskipped: the host has no second volumefor a check that needs one, and on the Linux hosts the filesystem caller-ownership scenario reported that OpenVMM cannot assume other host identities, so it checked that guest root and foreign guest callers fail withEPERM; both are properties of the hosts, not of this change.Left to CI:
test-openvmm-unitandtest-openvmm, since OpenVMM is unchanged;test-microvmscenarios that run only when named:smp-lapic, andmanaged-exec-configandsandbox-lifecycle, which need the Ubuntu scratch template that packages omit;verify-guest-determinism;