pika-orch-incus-cleanup-7

The old RemoteLinuxVmContext was a flat struct with a backend discriminator field and all possible fields for both backends. This made it easy to accidentally read an Incus-only field from a microvm path or vice-versa.

What changed

1. RemoteLinuxVmSharedContext holds every field common to both backends: remote_host, directory paths for the job, snapshot, artifacts, cargo caches, and staged workspace outputs.

2. RemoteMicrovmContext wraps shared and adds remote_runtime_dir and remote_runtime_link — paths only the microvm backend needs.

3. RemoteIncusContext wraps shared and adds incus_project, incus_profile, incus_image_alias, and incus_instance_name.

4. RemoteLinuxVmContext becomes a two-variant enum. Helper methods backend() and shared() let callers that need the discriminator or shared fields access them without matching:

impl RemoteLinuxVmContext {
    fn backend(&self) -> RemoteLinuxVmBackend { ... }
    fn shared(&self) -> &RemoteLinuxVmSharedContext { ... }
}

5. The constructor remote_linux_vm_context() now builds RemoteLinuxVmSharedContext first, then wraps it in the appropriate variant based on backend.

Every downstream function that previously accepted &RemoteLinuxVmContext is updated to either accept the shared context directly (e.g. ensure_remote_linux_vm_directories), accept the specific variant (e.g. incus::prepare_runtime takes &RemoteIncusContext), or match on the enum at dispatch points.

Six dispatch functions follow the same mechanical transformation:

The key benefit: each arm destructures the enum and passes the typed inner context to the module function. For example incus::prepare_runtime now receives &RemoteIncusContext instead of &RemoteLinuxVmContext, so it can access remote.incus_project directly without going through a field that might not logically exist for that backend.

Functions that only need the shared subset (e.g. ensure_remote_linux_vm_directories, reset_remote_linux_vm_artifacts) now take &RemoteLinuxVmSharedContext directly, accessed via remote.shared().

Every public function in executor/incus.rs — roughly 15 functions including build_remote_incus_launch_command, ensure_remote_incus_image_available, prepare_runtime, collect_artifacts, cleanup_runtime, write_remote_incus_json, and others — changes its first parameter from &RemoteLinuxVmContext to &RemoteIncusContext.

Field access uniformly shifts from remote.field to remote.shared.field for shared fields, while Incus-specific fields like remote.incus_project remain direct. This is a straightforward but pervasive mechanical change that ensures type safety across the module boundary.

Functions build_remote_microvm_launch_command, prepare_remote_microvm_runtime, prepare_backend_state, collect_remote_microvm_artifacts, and cleanup_remote_microvm_runtime all change to accept &RemoteMicrovmContext.

Microvm-specific fields (remote_runtime_dir, remote_runtime_link) remain directly accessible, while shared fields go through remote.shared. This mirrors the Incus module refactor and completes the type-safe separation of backends.

Five new fields are added to IncusGuestRequest:

The corresponding constants are defined at the module level. Two previously non-test constants (WORKSPACE_DEPS_MOUNT_PATH, WORKSPACE_BUILD_MOUNT_PATH) are now gated with #[cfg(test)] since they are only referenced in tests after the payload-mount protocol takes over at runtime.

The test remote_linux_incus_build_guest_request_serializes_command_and_timeout is extended to verify all five new fields and confirms home_dir flips to /root for run_as_root jobs.

This is the most substantial new feature in the branch. It introduces a declarative mount protocol for Incus containers.

Model additions (model.rs)

struct PreparedOutputPayloadManifestRecord {
    schema_version: u32,
    kind: String,
    entrypoints: Vec<String>,
    asset_roots: Vec<String>,
    #[serde(default)]
    mounts: Vec<PreparedOutputPayloadMountRecord>,
}

struct PreparedOutputPayloadMountRecord {
    name: String,
    relative_path: String,
    guest_path: String,
    read_only: bool,
}

The mounts field defaults to empty for backward compatibility with existing manifests.

Manifest loading

load_remote_payload_manifest() SSHs to the remote host and cats <output_root>/share/pikaci/payload-manifest.json. If the file doesn't exist, it returns None.

Validation

validate_declared_payload_mount() enforces two security invariants:

• relative_path must not be absolute or contain .. components (prevents path traversal outside the payload root)
• guest_path must be an absolute normalized path

Device naming

Incus device names have length constraints. build_declared_payload_mount_device_name_impl() generates a deterministic short name by:

1. Taking the first 8 characters of the sanitized device prefix
2. Appending a truncated hex hash of "{device_prefix}:{mount_name}"
3. Joining with a hyphen, capped at 20 characters

Snapshot mount

The workspace snapshot is also expressed through this protocol via snapshot_mount_record() and add_snapshot_mount(), unifying the mount mechanism.

Integration

configure_remote_incus_devices() (updated in the branch) calls add_declared_payload_mount() for the snapshot and each prepared-output payload, replacing the previous hard-coded device add calls for workspace-deps and workspace-build.

The remote_linux_vm_context() constructor previously computed some paths relative to a shared_job_dir (remote_work_dir/jobs/<job>). Now remote_artifacts_dir, remote_workspace_deps_dir, and remote_workspace_build_dir are all relative to remote_job_dir (remote_work_dir/runs/<run>/jobs/<job>).

This means all mutable state for a run is contained under a single directory tree, making post-run cleanup a single rm -rf and preventing stale artifacts from one run from leaking into another. All test assertions are updated to reflect the new paths (e.g. /var/tmp/pikaci/runs/run/jobs/job/artifacts).

Changes in pika-news/src/web.rs

1. Removed load_pikaci_logs_metadata and load_pikaci_prepared_outputs — two helper functions that each independently resolved the forge repo and state root.

2. Replaced load_pikaci_run_bundle implementation: instead of calling three separate functions, it now calls a single load_run_bundle(&state_root, run_id) from the pikaci library and destructures the result.

3. The api_forge_pikaci_prepared_outputs_handler is updated to use load_pikaci_run_bundle and pattern-match on the third tuple element.

Changes in pikaci/src/lib.rs

A new RunBundle struct and load_run_bundle function are exported. The function loads the run record and logs metadata (both required), and gracefully degrades on prepared-outputs errors — if the JSON is missing or corrupt, prepared_outputs is None rather than propagating the error.

New test

api_forge_branch_logs_keeps_run_metadata_when_prepared_outputs_are_invalid writes deliberately corrupt JSON to prepared-outputs.json, then verifies the API endpoint still returns HTTP 200 with valid run and log metadata, and pikaci_prepared_outputs as null. This covers a real-world scenario where a CI run produces valid logs but fails to write valid prepared-outputs JSON.

The new RunBundle struct groups the three pieces of run state:

pub struct RunBundle {
    pub run: RunRecord,
    pub logs: RunLogsMetadata,
    pub prepared_outputs: Option<PreparedOutputsRecord>,
}

load_run_bundle loads run and logs with normal error propagation (they must succeed), but wraps load_prepared_outputs_record in .ok().flatten() so that corrupt or missing prepared-outputs data degrades to None. This matches the existing behavior in pika-news but pushes the resilience into the library where it can be reused.

The old individual exports (load_logs_metadata, load_prepared_outputs_record) are removed from the public API of pika-news's imports, though they may still be available in the pikaci crate.

PreparedOutputPayloadManifestRecord describes a prepared output's payload manifest. The mounts field uses #[serde(default)] so that existing manifests without mounts deserialize cleanly as an empty Vec.

PreparedOutputPayloadMountRecord describes a single mount point. PartialEq is derived for test assertions. The four fields map directly to the Incus disk device configuration:

• name: logical identifier used in device naming
• relative_path: path within the payload root (. for the root itself)
• guest_path: absolute mount point inside the container
• read_only: whether to mount read-only

A round-trip test (payload_manifest_mounts_round_trip_and_stay_optional) verifies that legacy manifests without mounts decode correctly and that new manifests with mounts serialize and deserialize faithfully.

The branch title says "Keep agent-tools compatible with Python 3.9" and the changes are:

1. match/case → if/elif: Python's structural pattern matching was introduced in 3.10. All match action: / case "resolve": blocks are rewritten as if action == "resolve": / elif action == "build": chains.

2. list[str] → List[str]: The lowercase generic syntax for built-in types was introduced in 3.9 for some contexts but list[str] in function signatures requires from __future__ import annotations or Python 3.9+. The code switches to typing.List[str] (or string-quoted annotations) for broader compatibility.

3. X | Y → Union[X, Y]: The PEP 604 union syntax requires Python 3.10+. These are replaced with Union[X, Y] from the typing module.

These are purely syntactic changes with no behavioral difference, ensuring the script runs correctly on hosts that only have Python 3.9 available (e.g. older NixOS or Debian stable systems).

The Nix files receive updates to align with the new executor behavior:

• nix/incus/pikaci-image.nix: Likely updated to ensure the Incus VM image includes the pikaci-incus-run binary and has the expected mount points (/workspace/snapshot, /cargo-home, /cargo-target, /artifacts, /home/pikaci) pre-created or supported.

• nix/ci/linux-rust.nix: Updated to ensure the CI Rust toolchain environment is compatible with the new build layout where artifacts live under runs/<run>/jobs/<job>/.

A new documentation file docs/incus-migration-plan.md is added to the repository. This presumably documents the overall strategy for migrating the CI system to use Incus containers with the declared-mount protocol, including:

• The directory layout change (job dirs moving under run dirs)
• The payload manifest contract
• Device naming conventions
• Backward compatibility considerations

This serves as a reference for operators performing the migration.

Supporting changes across the pikaci crate:

• model.rs: Adds PreparedOutputPayloadManifestRecord and PreparedOutputPayloadMountRecord as described in an earlier step, plus any necessary re-exports.

• lib.rs: Exports RunBundle and load_run_bundle; may adjust which symbols are public.

• main.rs and run.rs: Updated to use the new context types and any changed function signatures in the executor module. The run orchestration layer passes the correct context variant to backend-specific functions.

The branch adds six new tests:

1. remote_linux_incus_snapshot_mount_uses_declared_mount_contract: Verifies the snapshot mount plan produces the correct device prefix, source path, mount name, relative path, guest path, and read-only flag.

2. declared_payload_mount_device_names_stay_short_and_stable: Asserts the generated device name is ≤20 characters, all lowercase alphanumeric with hyphens, and deterministic.

3. payload_manifest_mounts_round_trip_and_stay_optional: Tests backward-compatible deserialization (no mounts key → empty vec) and forward deserialization (with mounts).

4. payload_manifest_mount_validation_rejects_escaping_paths: Confirms that ../escape in relative_path and relative guest_path are both rejected with appropriate error messages.

5. api_forge_branch_logs_keeps_run_metadata_when_prepared_outputs_are_invalid: End-to-end test that writes corrupt JSON to prepared-outputs, calls the API handler, and verifies the response still contains valid run and log metadata with null prepared outputs.

6. Updated existing tests: remote_linux_incus_build_guest_request_serializes_command_and_timeout is extended to verify the five new guest-request fields. Test helper functions are refactored to use sample_remote_shared_context(), sample_microvm_context(), and sample_incus_context() instead of the old sample_remote_context(backend).