sledtools/pika branch #52

pika-orch-incus-cleanup-6

Split remote Linux VM backends

Target branch: master

Merge Commit: f33a12cee43e659be2fc891c621234cde70d6c31

branch: merged tutorial: ready ci: success

Open CI Details

Continuous Integration

CI: success

Compact status on the review page, with full logs on the CI page.

Open CI Details

Latest run #64 success

10 passed

head 418adbf844eece92c07720dc2feb46e329701db1 · queued 2026-03-25 17:40:24 · 10 lane(s)

queued 7s · ran 2m 15s

check-pika-rust · success check-pika-followup · success check-notifications · success check-agent-contracts · success check-rmp · success check-pikachat · success check-pikachat-typescript · success check-apple-host-sanity · success check-pikachat-openclaw-e2e · success check-fixture · success

Summary

This branch refactors the pikaci executor by extracting Incus and microVM backend logic from the monolithic executor.rs file into dedicated submodules (executor/incus.rs and executor/microvm.rs). Beyond the pure mechanical move, it introduces a structured guest-request protocol for Incus: instead of passing command, timeout, and root-flag as individual environment variables to incus exec, the runner now serializes an IncusGuestRequest JSON object, writes it into the guest filesystem via stdin piping, and passes only the request file path to the guest runner binary. The Incus image lookup is also hardened—switching from image show (single-object response) to image list (array response) with explicit alias matching and new unit tests covering the selection logic.

Tutorial Steps

Declare the new backend submodules

Intent: Create the module structure that will host the extracted Incus and microVM backend code, keeping the parent `executor.rs` as the orchestration layer.

Affected files: crates/pikaci/src/executor.rs

Evidence

@@ -21,6 +21,9 @@
+mod incus;
+mod microvm;

Two new child modules are declared at the top of executor.rs:

mod incus;
mod microvm;

Both modules use use super::*; to inherit the parent's imports, keeping the migration low-friction. All public items inside the submodules are scoped as pub(super) so they are visible to the parent module (and its tests) but not to the wider crate.

Introduce the IncusGuestRequest struct

Intent: Replace the previous approach of passing guest command parameters as separate environment variables with a single structured JSON payload that the host writes into the Incus guest filesystem before launch.

Affected files: crates/pikaci/src/executor.rs, crates/pikaci/src/executor/incus.rs

Evidence

@@ -63,6 +66,14 @@
+#[derive(Clone, Debug, Deserialize, Serialize, Eq, PartialEq)]
+struct IncusGuestRequest {
+    schema_version: u32,
+    command: String,
+    timeout_secs: u64,
+    run_as_root: bool,
+}

@@ -0,0 +1,586 @@
+pub(super) fn build_remote_incus_guest_request(job: &JobSpec) -> IncusGuestRequest {
+    let (command, run_as_root) = compiled_guest_command(job);
+    IncusGuestRequest {
+        schema_version: 1,
+        command,
+        timeout_secs: job.timeout_secs,
+        run_as_root,
+    }
+}

IncusGuestRequest is defined in the parent module (so both submodules can reference it) and carries four fields:

Field	Purpose
`schema_version`	Forward-compatible versioning (starts at `1`)
`command`	The compiled shell command string
`timeout_secs`	Job timeout propagated to the guest runner
`run_as_root`	Whether the guest should execute as root

build_remote_incus_guest_request in incus.rs constructs this struct from a JobSpec, reusing the existing compiled_guest_command helper.

A new constant REMOTE_LINUX_VM_INCUS_GUEST_REQUEST_PATH (/artifacts/guest-request.json) defines the well-known guest-side location for the request file.

Rewrite the Incus launch command to use the guest-request file

Intent: Simplify the Incus launch command from one that passes multiple environment variables to one that passes only the path to the pre-written JSON request file.

Affected files: crates/pikaci/src/executor/incus.rs

Evidence

@@ -0,0 +1,586 @@
+pub(super) fn build_remote_incus_launch_command(
+    remote: &RemoteLinuxVmContext,
+    request_path: &str,
+) -> String {
+    std::iter::once("sudo incus".to_string())
+        .chain(
+            [
+                "exec",
+                "--project",
+                remote.incus_project.as_str(),
+                remote.incus_instance_name.as_str(),
+                "--",
+                REMOTE_LINUX_VM_INCUS_RUN_BINARY,
+                request_path,
+            ]
+            .into_iter()
+            .map(shell_single_quote),
+        )
+        .collect::<Vec<_>>()
+        .join(" ")
+}

The old build_remote_incus_launch_command accepted a &JobSpec and embedded PIKACI_INCUS_GUEST_COMMAND, PIKACI_INCUS_TIMEOUT_SECS, and PIKACI_INCUS_RUN_AS_ROOT as env arguments to incus exec. The new version takes only the request_path string and passes it as a positional argument to the guest runner binary.

The actual job data is delivered separately via write_remote_incus_json, which pipes the serialized IncusGuestRequest into the guest through incus exec -- sh -lc 'cat > /artifacts/guest-request.json'. This two-step approach avoids shell-escaping issues with complex commands and keeps the launch invocation deterministic.

Write the guest-request JSON into the Incus VM via stdin piping

Intent: Provide a reliable mechanism for transferring structured data into the Incus guest without relying on shared filesystem mounts or environment variable length limits.

Affected files: crates/pikaci/src/executor/incus.rs

Evidence

@@ -0,0 +1,586 @@
+fn write_remote_incus_json<T: Serialize>(
+    remote: &RemoteLinuxVmContext,
+    guest_path: &str,
+    value: &T,
+    log_path: &Path,
+    label: &str,
+) -> anyhow::Result<()> {
+    let payload = serde_json::to_vec_pretty(value).context("encode Incus guest json payload")?;
+    let guest_parent = Path::new(guest_path)
+        .parent()
+        .ok_or_else(|| anyhow!("Incus guest path `{guest_path}` has no parent"))?;
+    let guest_command = format!(
+        "set -euo pipefail; mkdir -p {}; cat > {}",
+        shell_single_quote(&guest_parent.display().to_string()),
+        shell_single_quote(guest_path),
+    );
+    let mut child = run_remote_incus_command(...)

write_remote_incus_json is a generic helper that:

Serializes the value to pretty-printed JSON bytes.
Derives the parent directory from guest_path and creates it inside the guest.
Spawns incus exec ... -- sh -lc 'mkdir -p <parent>; cat > <path>' with Stdio::piped() stdin.
Writes the payload into stdin, then waits for the child process.
Logs stdout/stderr and fails on non-zero exit.

This replaces the need for environment variables and supports arbitrarily large payloads without shell quoting concerns.

Harden Incus image lookup with alias-aware list parsing

Intent: Fix a fragility in image fingerprint resolution: the previous code used `incus image show` which returns a single object and does not verify the alias matches. The new code uses `incus image list` which returns an array, and explicitly filters by alias name.

Affected files: crates/pikaci/src/executor/incus.rs

Evidence

@@ -0,0 +1,586 @@
+#[derive(Deserialize)]
+struct RemoteIncusImageShowRecord {
+    fingerprint: String,
+    #[serde(default)]
+    aliases: Vec<RemoteIncusImageAliasRecord>,
+}
+
+#[derive(Deserialize)]
+struct RemoteIncusImageAliasRecord {
+    name: String,
+}
+
+fn select_remote_incus_image_record(
+    records: Vec<RemoteIncusImageShowRecord>,
+    image_alias: &str,
+) -> anyhow::Result<RemoteIncusImageShowRecord> {
+    let mut matches = records
+        .into_iter()
+        .filter(|record| record.aliases.iter().any(|alias| alias.name == image_alias))
+        .collect::<Vec<_>>();
+    match matches.len() {
+        1 => Ok(matches.remove(0)),
+        0 => bail!("returned no matching alias record"),
+        _ => bail!("returned multiple matching alias records"),
+    }
+}

The image metadata struct now includes an aliases array. load_remote_incus_image_record switches from image show (single-record response) to image list (array response), then calls select_remote_incus_image_record which:

Filters records where any alias .name matches the expected image alias.
Returns the single match, or errors with a clear message for zero or multiple matches.

This prevents silent mismatches when the Incus server has multiple images or when an alias points to an unexpected image.

Extract all Incus backend functions into executor/incus.rs

Intent: Move every Incus-specific function out of the 3800-line executor.rs monolith into a focused submodule, reducing cognitive load and enabling independent review of each backend.

Affected files: crates/pikaci/src/executor.rs, crates/pikaci/src/executor/incus.rs

Evidence

@@ -2842,11 +2763,8 @@
-        RemoteLinuxVmBackend::Incus => ensure_remote_incus_runtime(job, remote, log_path),
+        RemoteLinuxVmBackend::Incus => incus::prepare_runtime(job, remote, log_path),

@@ -2857,458 +2775,19 @@
-        RemoteLinuxVmBackend::Incus => ensure_remote_incus_image_available(remote, log_path),
+        RemoteLinuxVmBackend::Incus => incus::prepare_backend_state(remote, log_path),

@@ -3394,26 +2873,8 @@
-        RemoteLinuxVmBackend::Incus => {
-            copy_remote_incus_file_to_local(remote, "/artifacts/guest.log", &ctx.guest_log_path)?;
+        RemoteLinuxVmBackend::Incus => incus::collect_artifacts(remote, ctx),

@@ -3422,8 +2883,8 @@
-        RemoteLinuxVmBackend::Incus => delete_remote_incus_instance(remote, log_path),
+        RemoteLinuxVmBackend::Incus => incus::cleanup_runtime(remote, log_path),

The following functions are relocated from executor.rs into executor/incus.rs, each exposed through a thin pub(super) facade:

Parent call site	New delegation
`ensure_remote_incus_image_available`	`incus::prepare_backend_state`
`load_remote_incus_image_record`	`incus::load_image_record`
`ensure_remote_incus_runtime`	`incus::prepare_runtime`
`delete_remote_incus_instance`	`incus::cleanup_runtime`
`copy_remote_incus_file_to_local` (artifacts)	`incus::collect_artifacts`
`build_remote_incus_launch_command`	`incus::build_spawn_command`
`build_remote_incus_device_add_args`	`incus::build_device_add_args`
`run_remote_incus_command`	private inside `incus`
`run_remote_incus_to_log`	private inside `incus`
`remote_realpath`	private inside `incus`
`configure_remote_incus_devices`	private inside `incus`
`wait_for_remote_incus_instance`	private inside `incus`

The parent executor.rs now contains only backend-agnostic orchestration and delegates to the appropriate submodule via match remote.backend.

Extract all microVM backend functions into executor/microvm.rs

Intent: Apply the same separation to the microVM backend, moving its launch-command builder, runtime provisioning, artifact collection, and cleanup into a dedicated submodule.

Affected files: crates/pikaci/src/executor.rs, crates/pikaci/src/executor/microvm.rs

Evidence

@@ -2121,11 +2133,7 @@
-        RemoteLinuxVmBackend::Microvm => GuestRunnerConfig {
-            guest_system: REMOTE_MICROVM_GUEST_SYSTEM,
-            host_pkgs_expr: "nixpkgs.legacyPackages.x86_64-linux",
-            hypervisor: "cloud-hypervisor",
-        },
+        RemoteLinuxVmBackend::Microvm => microvm::guest_runner_config(),

@@ -2842,11 +2763,8 @@
-        RemoteLinuxVmBackend::Microvm => {
-            ensure_remote_microvm_runtime(job, ctx, remote, log_path)?;
-            reset_remote_linux_vm_artifacts(remote, log_path)
-        }
+        RemoteLinuxVmBackend::Microvm => microvm::prepare_runtime(job, ctx, remote, log_path),

The microVM submodule (executor/microvm.rs) receives:

guest_runner_config() — returns the GuestRunnerConfig for microVM jobs.
build_launch_command() / build_remote_microvm_launch_command() — the complex shell script that starts virtiofsd processes, waits for sockets, and launches microvm-run.
prepare_runtime() — combines ensure_remote_microvm_runtime + reset_remote_linux_vm_artifacts.
prepare_backend_state() — delegates to prepare_runtime for microVM (idempotent).
ensure_remote_microvm_runtime() — the full flake-based remote Nix build and symlink workflow.
collect_artifacts() — copies guest.log and result.json from the remote artifacts directory.
cleanup_runtime() — currently a no-op (Ok(())) for microVM, but now explicitly stated.

The microVM cleanup was previously an inline Ok(()) in the match arm; it is now a named function, making future cleanup logic easier to add.

Update the Nix guest image to read from the request file

Intent: Align the NixOS-based Incus guest image with the new guest-request protocol so the runner binary reads its parameters from the JSON file instead of environment variables.

Affected files: nix/incus/pikaci-image.nix

Evidence

(no diff hunk shown in truncated payload — file listed as changed)

The pikaci-image.nix file is listed as changed in this branch. While the diff was truncated, the expected change aligns the guest-side pikaci-incus-run script/binary to accept a single positional argument (the path to guest-request.json), parse the JSON, and extract command, timeout_secs, and run_as_root from it—replacing the previous PIKACI_INCUS_GUEST_COMMAND, PIKACI_INCUS_TIMEOUT_SECS, and PIKACI_INCUS_RUN_AS_ROOT environment variable reads.

Update and add unit tests for the new module structure

Intent: Verify the behavioral changes (guest-request serialization, alias-based image selection, launch command format) and ensure test imports reference the new submodule paths.

Affected files: crates/pikaci/src/executor.rs

Evidence

@@ -3760,19 +3183,19 @@
+        HostLocalEnvironmentRefresh, REMOTE_LINUX_VM_INCUS_GUEST_REQUEST_PATH,
+        REMOTE_LINUX_VM_INCUS_WORKSPACE_DEPS_MOUNT_PATH, REMOTE_MICROVM_VIRTIOFS_SOCKETS,
+        RemoteLinuxVmContext, attach_remote_linux_vm_execution,
+        build_sync_directory_finalize_command, ... incus, microvm, ...

@@ -4030,21 +3452,75 @@
+    fn remote_linux_incus_launch_uses_incus_exec_runner() {
+        let command = incus::build_launch_command(
+            &sample_remote_context(RemoteLinuxVmBackend::Incus),
+            REMOTE_LINUX_VM_INCUS_GUEST_REQUEST_PATH,
+        );
+        assert!(command.contains("sudo incus"));
+        assert!(command.contains("'/artifacts/guest-request.json'"));
+        assert!(!command.contains("PIKACI_INCUS_GUEST_COMMAND"));
+        assert!(!command.contains("PIKACI_INCUS_TIMEOUT_SECS"));
+        assert!(!command.contains("PIKACI_INCUS_RUN_AS_ROOT"));

@@ -4030,21 +3452,75 @@
+    fn remote_linux_incus_guest_request_captures_command_timeout_and_user() {
+        let request = incus::build_guest_request(&sample_shell_job("actionlint"));
+        assert_eq!(request.schema_version, 1);
+        assert_eq!(request.command, "bash --noprofile --norc -lc 'actionlint'");
+        assert_eq!(request.timeout_secs, 120);
+        assert!(!request.run_as_root);

@@ -4030,21 +3452,75 @@
+    fn remote_linux_incus_image_record_selection_uses_matching_alias() {
+        let fingerprint = incus::select_image_fingerprint_from_json(...
+        assert_eq!(fingerprint, "right");

@@ -4030,21 +3452,75 @@
+    fn remote_linux_incus_image_record_selection_rejects_missing_alias() {
+        let err = incus::select_image_fingerprint_from_json(...
+        assert!(err.to_string().contains("returned no matching alias record"));

Four test changes validate the refactored code:

remote_linux_incus_launch_uses_incus_exec_runner — Updated to call incus::build_launch_command with the request path. Now asserts the path /artifacts/guest-request.json is present and that the old environment variables (PIKACI_INCUS_GUEST_COMMAND, etc.) are absent.
remote_linux_incus_guest_request_captures_command_timeout_and_user (new) — Verifies IncusGuestRequest serialization for both normal and root jobs, checking schema_version, command, timeout_secs, and run_as_root.
remote_linux_incus_image_record_selection_uses_matching_alias (new) — Feeds a two-element JSON array into select_image_fingerprint_from_json and confirms the record with the matching alias is selected.
remote_linux_incus_image_record_selection_rejects_missing_alias (new) — Confirms that an array with no matching alias produces a clear error message.

The existing remote_linux_microvm_launch_starts_virtiofsd_and_waits_for_sockets test is updated to call microvm::build_launch_command and the remote_linux_incus_read_only_disk_device_uses_virtiofs_bus test now calls incus::build_device_add_args.

pika-orch-incus-cleanup-6

CI: success

Summary

Tutorial Steps

Declare the new backend submodules

Introduce the IncusGuestRequest struct

Rewrite the Incus launch command to use the guest-request file

Write the guest-request JSON into the Incus VM via stdin piping

Harden Incus image lookup with alias-aware list parsing

Extract all Incus backend functions into executor/incus.rs

Extract all microVM backend functions into executor/microvm.rs

Update the Nix guest image to read from the request file

Update and add unit tests for the new module structure

Diff