sledtools/pika branch #89

pika-orch-incus-cleanup-19

Generalize staged payload mounts

Target branch: master

Merge Commit: 1a5628c1baced3473f650f177ff63a00f8b43de4

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 #115 success

6 passed

head 34c17c3d18ddc41c65e409acc406b8170e2870d3 · queued 2026-03-26 01:13:50 · 6 lane(s)

queued 6s · ran 31s

check-notifications · success check-agent-contracts · success check-pikachat · success check-pikachat-typescript · success check-pikachat-openclaw-e2e · success check-fixture · success

Summary

This branch generalizes the staged payload mount system in pika's CI executor. Previously, two hard-coded optional fields (staged_linux_rust_workspace_deps_dir and staged_linux_rust_workspace_build_dir) on HostContext represented the only supported staged mounts. The change introduces a StagedPayloadMount struct carrying a local path and a device prefix, replaces the two Option<PathBuf> fields with a single Vec<StagedPayloadMount>, and updates all construction sites, device-configuration loops, path-resolution helpers, and tests to work with the new generalized collection. This makes it straightforward to add new staged payload types in the future without touching the core plumbing.

Tutorial Steps

Introduce the StagedPayloadMount struct

Intent: Define a reusable data type that pairs a local filesystem path with an Incus device-name prefix, replacing the implicit coupling between two separate Option fields and their hard-coded device names.

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

Evidence

@@ -26,6 +26,12 @@
+#[derive(Clone, Debug)]
+pub struct StagedPayloadMount {
+    pub local_mount_path: PathBuf,
+    pub device_prefix: String,
+}

A new StagedPayloadMount struct is added at the module level in executor.rs. It captures two pieces of information that were previously spread across separate fields:

local_mount_path – the host-side directory where the staged payload is materialized.
device_prefix – the string used to name Incus disk devices derived from this mount (e.g. "workspace-deps").

The struct derives Clone and Debug to match the ergonomics of the surrounding types.

Replace Option fields on HostContext with a Vec

Intent: Collapse the two independent optional path fields into a single, extensible vector so the context struct does not need modification every time a new staged payload type is introduced.

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

Evidence

@@ -39,8 +45,7 @@
-    pub staged_linux_rust_workspace_deps_dir: Option<PathBuf>,
-    pub staged_linux_rust_workspace_build_dir: Option<PathBuf>,
+    pub staged_payload_mounts: Vec<StagedPayloadMount>,

HostContext previously held two Option<PathBuf> fields:

pub staged_linux_rust_workspace_deps_dir: Option<PathBuf>,
pub staged_linux_rust_workspace_build_dir: Option<PathBuf>,

These are replaced by:

pub staged_payload_mounts: Vec<StagedPayloadMount>,

An empty Vec is semantically equivalent to both fields being None, while a two-element vector restores the previous behavior. The change is purely structural—no new capabilities are added yet, but the door is open for arbitrary payload types.

Generalize staged_payload_remote_dirs helper

Intent: Update the helper that computes remote-side paths so it iterates over the new Vec instead of manually unwrapping two Option values.

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

Evidence

@@ -1905,14 +1910,16 @@
-    [
-        ctx.staged_linux_rust_workspace_deps_dir.as_ref(),
-        ctx.staged_linux_rust_workspace_build_dir.as_ref(),
-    ]
-    .into_iter()
-    .flatten()
-    .map(|path| remote_job_path(&ctx.job_dir, &shared.remote_job_dir, path))
-    .collect()
+    ctx.staged_payload_mounts
+        .iter()
+        .map(|mount| {
+            remote_job_path(
+                &ctx.job_dir,
+                &shared.remote_job_dir,
+                &mount.local_mount_path,
+            )
+        })
+        .collect()

The staged_payload_remote_dirs function previously constructed a two-element array of Option references, flattened it, and mapped each to a remote path. Now it simply iterates ctx.staged_payload_mounts and maps each local_mount_path through remote_job_path. The logic is identical but scales to any number of mounts without code changes.

Simplify staged_payload_source_root signature in incus module

Intent: Remove the Option wrapper from the local_mount_path parameter since callers now always provide a concrete path from the StagedPayloadMount struct.

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

Evidence

@@ -801,18 +801,12 @@
 fn staged_payload_source_root(
-    local_mount_path: Option<&PathBuf>,
+    local_mount_path: &Path,
     local_job_dir: &Path,
     remote_job_dir: &Path,
     remote_host: &str,
 ) -> anyhow::Result<PathBuf> {

Because every element in the staged_payload_mounts vector is guaranteed to have a local_mount_path, the staged_payload_source_root function no longer needs to accept Option<&PathBuf>. It now takes &Path directly, which eliminates two ok_or_else error branches that existed solely to handle None. This removes ~12 lines of defensive error handling that can no longer trigger.

Loop over staged mounts when configuring Incus devices

Intent: Replace the two hard-coded calls to add_declared_payload_mounts with a single loop over the staged_payload_mounts collection, using each mount's device_prefix.

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

Evidence

@@ -851,20 +839,20 @@
-        let workspace_deps_root = staged_payload_source_root(
-            ctx.staged_linux_rust_workspace_deps_dir.as_ref(),
-        ...
-        add_declared_payload_mounts(remote, &workspace_deps_root, "workspace-deps", log_path)?;
-        let workspace_build_root = staged_payload_source_root(
-            ctx.staged_linux_rust_workspace_build_dir.as_ref(),
-        ...
-        add_declared_payload_mounts(remote, &workspace_build_root, "workspace-build", log_path)?;
+        for staged_mount in &ctx.staged_payload_mounts {
+            let source_root = staged_payload_source_root(
+                &staged_mount.local_mount_path,
+                &ctx.job_dir,
+                &remote.shared.remote_job_dir,
+                &remote.shared.remote_host,
+            )?;
+            add_declared_payload_mounts(
+                remote,
+                &source_root,
+                &staged_mount.device_prefix,
+                log_path,
+            )?;
+        }

In configure_remote_incus_devices, the two back-to-back blocks that resolved a source root and attached an Incus disk device for workspace-deps and workspace-build are collapsed into a for loop over ctx.staged_payload_mounts. Each iteration reads the device_prefix from the mount itself rather than using a string literal. This is the key behavioral change: the Incus device configuration is now fully data-driven.

Build StagedPayloadMount instances in the run planner

Intent: Construct the new Vec<StagedPayloadMount> during run-plan construction, preserving the same paths and prefixes that the old Option fields carried.

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

Evidence

@@ -1466,12 +1466,25 @@
-            staged_linux_rust_workspace_deps_dir: job
-                .staged_linux_rust_lane()
-                .map(|_| job_dir.join("staged-linux-rust").join("workspace-deps")),
-            staged_linux_rust_workspace_build_dir: job
-                .staged_linux_rust_lane()
-                .map(|_| job_dir.join("staged-linux-rust").join("workspace-build")),
+            staged_payload_mounts: job
+                .staged_linux_rust_lane()
+                .map(|_| {
+                    vec![
+                        StagedPayloadMount {
+                            local_mount_path: job_dir
+                                .join("staged-linux-rust")
+                                .join("workspace-deps"),
+                            device_prefix: "workspace-deps".to_string(),
+                        },
+                        StagedPayloadMount {
+                            local_mount_path: job_dir
+                                .join("staged-linux-rust")
+                                .join("workspace-build"),
+                            device_prefix: "workspace-build".to_string(),
+                        },
+                    ]
+                })
+                .unwrap_or_default(),

In build_run_plan, the construction of HostContext now populates staged_payload_mounts with a two-element Vec when the job has a staged Linux Rust lane, or an empty Vec otherwise (via unwrap_or_default()). The paths and device prefixes are identical to what the old code produced, ensuring behavioral equivalence.

Refactor staged mount consumer wiring to use a loop

Intent: Replace the two explicit add_staged_mount_consumer calls with a loop that looks up each mount by device_prefix, improving consistency and reducing duplication.

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

Evidence

@@ -1561,26 +1574,28 @@
-            add_staged_mount_consumer(
-                &mut planned_prepares,
-                &mut prepare_nodes,
-                &deps_node_id,
-                ctx.staged_linux_rust_workspace_deps_dir
-                    .as_ref()
-                    .ok_or_else(|| anyhow!("missing staged Linux Rust workspaceDeps mount path"))?,
-                &ctx.host_log_path,
-            );
-            add_staged_mount_consumer(
-                ...
-                ctx.staged_linux_rust_workspace_build_dir
-                    .as_ref()
-                    .ok_or_else(|| {
-                        anyhow!("missing staged Linux Rust workspaceBuild mount path")
-                    })?,
-                &ctx.host_log_path,
-            );
+            for (node_id, device_prefix) in [
+                (&deps_node_id, "workspace-deps"),
+                (&build_node_id, "workspace-build"),
+            ] {
+                let mount = ctx
+                    .staged_payload_mounts
+                    .iter()
+                    .find(|mount| mount.device_prefix == device_prefix)
+                    .ok_or_else(|| {
+                        anyhow!(
+                            "missing staged Linux Rust payload mount `{device_prefix}` for job `{}`",
+                            job.id
+                        )
+                    })?;
+                add_staged_mount_consumer(
+                    &mut planned_prepares,
+                    &mut prepare_nodes,
+                    node_id,
+                    &mount.local_mount_path,
+                    &ctx.host_log_path,
+                );
+            }

The preparation-graph wiring that connects staged-mount nodes to their consumer tasks is rewritten as a loop over (node_id, device_prefix) pairs. For each pair, the corresponding StagedPayloadMount is looked up from the staged_payload_mounts vector by device_prefix. This keeps the error message precise (it includes the prefix and job id) while eliminating the duplicated add_staged_mount_consumer blocks.

Update all test HostContext constructions

Intent: Migrate every test that builds a HostContext to use the new Vec field, and update the test that exercises staged payload resolution to construct StagedPayloadMount values explicitly.

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

Evidence

@@ -2911,8 +2918,7 @@
-            staged_linux_rust_workspace_deps_dir: None,
-            staged_linux_rust_workspace_build_dir: None,
+            staged_payload_mounts: Vec::new(),

@@ -3634,12 +3632,20 @@
+            staged_payload_mounts: vec![
+                StagedPayloadMount {
+                    local_mount_path: PathBuf::from(
+                        "/tmp/run/jobs/job/staged-linux-rust/workspace-deps",
+                    ),
+                    device_prefix: "workspace-deps".to_string(),
+                },
+                StagedPayloadMount {
+                    local_mount_path: PathBuf::from(
+                        "/tmp/run/jobs/job/staged-linux-rust/workspace-build",
+                    ),
+                    device_prefix: "workspace-build".to_string(),
+                },
+            ],

@@ -3692,7 +3698,7 @@
-            Some(&local_mount),
+            &local_mount,

Nine test functions that construct HostContext with no staged mounts are updated from two None fields to staged_payload_mounts: Vec::new(). The staged_payload_remote_dirs_resolved test is the most interesting: it now builds a vec![StagedPayloadMount { .. }, StagedPayloadMount { .. }] with explicit device prefixes, validating that the generalized collection produces the same remote paths as before.

Two calls to the test helper resolve_staged_payload_source_root_for_test are updated to pass &local_mount instead of Some(&local_mount), matching the simplified function signature.

pika-orch-incus-cleanup-19

CI: success

Summary

Tutorial Steps

Introduce the StagedPayloadMount struct

Replace Option fields on HostContext with a Vec

Generalize staged_payload_remote_dirs helper

Simplify staged_payload_source_root signature in incus module

Loop over staged mounts when configuring Incus devices

Build StagedPayloadMount instances in the run planner

Refactor staged mount consumer wiring to use a loop

Update all test HostContext constructions

Diff