Skip to content

nvx-rs: lay out execution-helper configurations with one function - #689

Merged
Pedro Henrique Penna (ppenna) merged 1 commit into
devfrom
nvx-rs-quality/exec-config-layout
Oct 22, 2026
Merged

Pedro Henrique Penna (ppenna) merged 1 commit into
devfrom
nvx-rs-quality/exec-config-layout

Conversation

@ppenna

Copy link
Copy Markdown
Contributor

Summary

The managed agent hands each workload's working directory and environment to its execution helper in a sealed configuration, which exec_config::encode in nvx-rs/src/guest/managed_agent/exec_config.rs 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 need configurations that encode never 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:

{
    let mut bytes = Vec::new();
    bytes.extend_from_slice(&EXEC_ENVIRONMENT_PRESENT.to_le_bytes());
    bytes.extend_from_slice(&1u16.to_le_bytes());
    bytes.extend_from_slice(&0u32.to_le_bytes());
    bytes.extend_from_slice(&9u32.to_le_bytes());
    bytes.extend_from_slice(b"NO_EQUALS");
    bytes
},

and both test modules defined the same file_with, which puts bytes in a file that exec_config::read reads. encode now computes the flags and leaves the layout to encode_fields(flags, cwd, environment), which the six fixtures call with the fields that they vary, as in encode_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_with moves beside read, under cfg(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 in src and tests, 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 to dev changed when the run started, among them README.md, Cargo.toml, src/lib.rs, host/mod.rs, guest/mod.rs, the managed agent's cgroup, dispatch, exec, launch, mappings, mod, and seam, 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, and tests/{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' receive and send, the two EXEC payload decoders, whose text and byte rules differ, and the Windows stat and lstat of host/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 no nvx-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_fields is pub(super), visible only in the managed agent, and keeps encode's Result<Vec<u8>, Errno> and its EOVERFLOW checks, 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.
  • It takes the working directory as a slice, empty without one, since the format encodes an absent working directory as length 0 and no bytes; the flags, which encode computes as before, say whether one is present.
  • file_with keeps using a temporary file, as both copies did, rather than the sealed memfd that create makes, and sits beside read as a cfg(test) function rather than in a new test-support module.
  • read keeps decoding the layout field by field from its descriptor, with its checks in their order.
  • The tests keep their cases, their order, and their assertion messages.

Changes

Two files, 45 insertions and 70 deletions:

  • nvx-rs/src/guest/managed_agent/exec_config.rs:
    • encode computes the flags as before and returns encode_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 that encode wrote, which now writes the working directory's bytes unconditionally, none when it is empty, rather than matching an Option.
    • file_with(bytes), cfg(test) and pub(super), with a doc comment, moved from the test module.
    • helper_defensively_rejects_unknown_flags_and_malformed_environment builds its four configurations with encode_fields.
  • nvx-rs/src/guest/managed_agent/helper.rs: helper_rejects_bad_syntax_relative_cwd_and_conflicting_cwd_flags builds its relative and conflicting configurations with exec_config::encode_fields and opens them with exec_config::file_with; the test module's file_with and its Seek and Write imports go.

The sites that now share the layout are encode, the four rejection fixtures of exec_config.rs, and the two of helper.rs; the tests of both files share file_with. No public item, message, error kind, protocol byte, fixture file, or other file changes.

Behavior

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. cwd.unwrap_or_default() is empty exactly when encode used 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:

    Fixture Flags Working directory Entries
    unknown flag 0x4000 none none
    entry without = EXEC_ENVIRONMENT_PRESENT none NO_EQUALS
    entry without a name EXEC_ENVIRONMENT_PRESENT none =value
    entered directory with a length EXEC_CWD_ENTERED / none
    relative directory EXEC_CWD_PRESENT . none
    conflicting flags EXEC_CWD_PRESENT | EXEC_CWD_ENTERED / none
  • Tests that cover it, none of them added, removed, skipped, or weakened:

    • in exec_config.rs, helper_configuration_round_trips_empty_exact_and_large_environments and helper_configuration_marks_an_entered_directory_and_is_sealed, which encode through create and read back, and helper_defensively_rejects_unknown_flags_and_malformed_environment;
    • in 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, and helper_keeps_a_directory_that_the_caller_entered;
    • the managed agent's loopback and dispatch tests, which launch workloads through 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 through encode and its helper's read, so the hypervisor tests ran the change too: the guest binaries were rebuilt from TREE into the Alpine initramfs that every host booted, and the managed-lifecycle scenario and the aci_edge_sandboxes end-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.rs and host.rs: each role's send repeats the same four lines after its state check, which check the next sequence number, encode the DATA record, and advance; encode_data could take the sequence number by reference and advance it, and a helper of that shape could serve the guest's credit too. receive repeats nine lines that need a trait over each role's parser, accept, and fail.
  • guest/ingress/encode.rs's ListenerTable::add and guest/ingress/diag.rs's note_wildcard_tcp repeat 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 Windows stat and lstat build the same Stat; a status(path, traverse) helper would mirror the POSIX implementation's status.
  • net::is_valid_guest_network and guest_subnet in host/network/mod.rs both parse an IPv4 ADDRESS/PREFIX and compute its mask, with prefix limits of 1 to 30 and 1 to 32.
  • host/sandbox/tests.rs and host/sandbox/mount/tests.rs define the same policy and native test helpers; the share tests could import the launch tests' helpers, which they can reach as a descendant of host::sandbox.
  • protocol/outer.rs's encode_outer and protocol/nvxs/record.rs's encode_record write 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.
  • The follow-ups that nvx-rs: send the guest's attachment, ACK, and CREDIT with one method #684 and nvx-rs: parse the mount mode, owner, and kind names with one function #685 list as waiting on files that open pull requests change remain.

Validation

  • BASE: fbefbfccdc341bdebc9f0aa0e742fe351846c114, origin/dev when the run started. Before this pull request was opened, origin/dev advanced to 59712ebdd by 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, and data/, none of this pull request's files or anything under nvx-rs/, aci_edge_sandboxes/, tests/, or openvmm, and no open pull request to dev changes either file.

  • TREE: 4f07eecba7cf1359cc0af7055f1e29e3ca68b80f, the tree of this commit. Every host applied the patch to BASE and checked that git write-tree equals 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 in build/openvmm.provenance.json.

  • nvx-rs: download installed the release's nvx-rs, which other sources built, so each host rebuilt it from TREE with the Cargo command with which nvx.py builds it, after its gates and before its hypervisor tests. The executable's digest changed, record-nvx-rs-provenance recorded it, and each host checked that the record names TREE's sources (source_sha256 bcff89560061ff8d8bfddcf6e12b50290defbc979f293b93ad6132ba16a35fbc) and the executable. Each host then built the openvmm_e2e and managed_lifecycle test 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 compiled nvx with the guest profile;
    • test-guest-binaries --binaries, where 34 tests passed;
    • the Alpine build-initramfs with NVX_GUEST_BINARIES, which installed the new nvx-managed-agent.

    initramfs.cpio.gz (SHA-256 18d9183a7233c813bb4210c97d72194f6a69d9f35126ea633a86f32c87b5060e), 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 guest module builds only on Linux, Clippy with -D warnings on 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 of nvx and aci_edge_sandboxes and the MSRV check of the runtime adapter's guest-only configuration, which ran as the action defines it, starting with its rustup 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.

Host profile Backend Host type check-nvx-rs and check-aci-edge-sandboxes without a hypervisor Guest artifacts test-aci-edge-sandboxes test-microvm, default suite
prometheus32 kvm baremetal Pass: every Linux step, 29 commands, including the MSRV (1.89) checks; nvx tests: 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 85 aci_edge_sandboxes tests over the two feature sets; agent bundling (4). The macOS checks and the guest-only MSRV check ran locally Copied in, digests match Pass (18/18) Pass (41 scenario runs)
prometheus30 mshv baremetal None; prometheus32 and prometheus28 ran the gates Built and tested here (34 tests) Pass (18/18) Pass (41 scenario runs)
prometheus28 whp baremetal Pass: every Windows step, 21 commands; nvx tests: 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 85 aci_edge_sandboxes tests over the two feature sets; agent bundling (4). The MSRV checks ran locally Copied in, digests match Pass (18/18) Pass (42 scenario runs)

Every 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's managed-lifecycle scenario ran the managed_lifecycle test 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, one openvmm_e2e test printed skipped: the host has no second volume for 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 with EPERM; both are properties of the hosts, not of this change.

Left to CI:

  • the OpenVMM unit and VMM suites, test-openvmm-unit and test-openvmm, since OpenVMM is unchanged;
  • the test-microvm scenarios that run only when named: smp-lapic, and managed-exec-config and sandbox-lifecycle, which need the Ubuntu scratch template that packages omit;
  • the debug-kernel scenarios, the Ubuntu and Azure Linux guests, the Docker guest build, and verify-guest-determinism;
  • the benchmarks and performance gates;
  • the virtual-machine runners;
  • check-quality and validate-nvx, which no Python, shell, or Markdown change touches.

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>
Copilot AI balanced review requested due to automatic review settings October 21, 2026 12:46

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 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_fields between production encoding and six rejection fixtures.
  • Consolidates the test-only file_with helper 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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants