sledtools/pika branch #146

pika-cloud-substrate-polish

Polish pika-cloud Incus substrate

Target branch: master

Merge Commit: b131b2ef52ccbd5008add165f894e6434f893396

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

5 passed

head 91deb743484ee8be833da2c5756dbc1eb5839127 · queued 2026-03-27 14:38:51 · 5 lane(s)

queued 3m 12s · ran 3m 16s

check-notifications · success check-agent-contracts · success check-pikachat · success check-apple-host-sanity · success check-fixture · success

Summary

This branch relocates Incus-specific types and constants from the top-level pika-cloud crate root (lib.rs) into the dedicated pika-cloud::incus submodule, and removes the now-unnecessary ProviderKind enum and its provider field from RuntimeSpec. The IncusGuestRunRequest struct and INCUS_GUEST_RUN_REQUEST_SCHEMA_VERSION constant move to crates/pika-cloud/src/incus.rs, all downstream consumers in jerichoci update their import paths accordingly, and the RuntimeSpec struct drops the provider: ProviderKind field since Incus is the only supported provider. Associated tests are relocated or updated to reflect the new layout.

Tutorial Steps

Move IncusGuestRunRequest and schema version constant into the incus submodule

Intent: Colocate the Incus guest-run request type and its schema version constant with the rest of the Incus-specific code in `crates/pika-cloud/src/incus.rs`, rather than leaving them in the crate root.

Affected files: crates/pika-cloud/src/incus.rs, crates/pika-cloud/src/lib.rs

Evidence

@@ -8,6 +8,7 @@ use crate::spec::{IncusRuntimeConfig, RuntimeIdentity, RuntimeResources, Runtim
 
+pub const INCUS_GUEST_RUN_REQUEST_SCHEMA_VERSION: u32 = 2;
 pub const INCUS_READ_ONLY_DISK_IO_BUS: &str = "virtiofs";

@@ -18,6 +19,15 @@ pub const INCUS_DEVICE_IO_BUS_KEY: &str = "io.bus";
 
+#[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)]
+#[serde(deny_unknown_fields)]
+pub struct IncusGuestRunRequest {
+    pub schema_version: u32,
+    pub command: String,
+    pub timeout_secs: u64,
+    pub run_as_root: bool,
+}

@@ -25,58 +25,3 @@ pub use spec::{
 };
-
-use serde::{Deserialize, Serialize};
-
-pub const INCUS_GUEST_RUN_REQUEST_SCHEMA_VERSION: u32 = 2;
-
-#[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)]
-#[serde(deny_unknown_fields)]
-pub struct IncusGuestRunRequest {

The IncusGuestRunRequest struct and its companion constant INCUS_GUEST_RUN_REQUEST_SCHEMA_VERSION previously lived in crates/pika-cloud/src/lib.rs, the crate root. This branch moves both into crates/pika-cloud/src/incus.rs where all other Incus device constants, mount planning logic, and runtime plan types already reside.

In incus.rs, the constant is added at the top of the public constant block:

pub const INCUS_GUEST_RUN_REQUEST_SCHEMA_VERSION: u32 = 2;

The struct definition follows the existing constant block, keeping its #[serde(deny_unknown_fields)] attribute and all four public fields (schema_version, command, timeout_secs, run_as_root). Note that the derive list changes from Clone, Copy, Debug (the old location mistakenly had Copy on the enum, not the struct) to Clone, Debug — matching what the struct actually needs.

The corresponding declarations and the use serde::{Deserialize, Serialize} import are deleted from lib.rs, which no longer re-exports these items from the crate root.

Relocate the round-trip test for IncusGuestRunRequest

Intent: Move the serialization round-trip test alongside the struct it validates, keeping test coverage intact after the relocation.

Affected files: crates/pika-cloud/src/incus.rs, crates/pika-cloud/src/lib.rs

Evidence

@@ -263,4 +273,17 @@ mod tests {
+    #[test]
+    fn incus_guest_run_request_round_trips() {
+        let request = IncusGuestRunRequest {
+            schema_version: INCUS_GUEST_RUN_REQUEST_SCHEMA_VERSION,
+            command: "bash --noprofile --norc -lc 'cargo test -p pika-cloud'".to_string(),
+            timeout_secs: 120,
+            run_as_root: false,
+        };
+        let encoded = serde_json::to_string(&request).expect("encode request");
+        let decoded: IncusGuestRunRequest = serde_json::from_str(&encoded).expect("decode request");
+        assert_eq!(decoded, request);
+    }

The incus_guest_run_request_round_trips test is moved verbatim from the tests module in lib.rs into the existing tests module in incus.rs. The test body is identical — it constructs an IncusGuestRunRequest, encodes it to JSON, decodes it back, and asserts equality. Because the struct and constant now live in the same file, the test no longer requires a cross-module import.

Remove ProviderKind enum and its provider field from RuntimeSpec

Intent: Eliminate dead abstraction: since Incus is the only supported provider, the `ProviderKind` enum and the `provider` field on `RuntimeSpec` add indirection without value.

Affected files: crates/pika-cloud/src/lib.rs, crates/pika-cloud/src/spec.rs

Evidence

@@ -25,58 +25,3 @@ pub use spec::{
-#[derive(Clone, Copy, Debug, Deserialize, Eq, PartialEq, Serialize)]
-#[serde(rename_all = "snake_case")]
-pub enum ProviderKind {
-    Incus,
-}

@@ -37,7 +36,6 @@ pub struct IncusRuntimeConfig {
 pub struct RuntimeSpec {
     pub identity: RuntimeIdentity,
-    pub provider: ProviderKind,
     pub incus: IncusRuntimeConfig,

@@ -105,7 +103,6 @@ impl RuntimeSpec {
         Self {
             identity,
-            provider: ProviderKind::Incus,
             incus,

@@ -189,7 +186,6 @@ impl RuntimeSpec {
-        let _ = self.provider;
         Ok(())

The ProviderKind enum (with its single Incus variant) is deleted from lib.rs, along with the two serde tests (provider_kind_serde_uses_snake_case and unknown_provider_kind_rejected).

In spec.rs:

The use crate::ProviderKind import is removed.
The provider: ProviderKind field is removed from the RuntimeSpec struct definition.
The constructor no longer sets provider: ProviderKind::Incus.
The validation method drops the let _ = self.provider; line that previously existed solely to ensure the field was referenced.

Since the struct uses serde::Deserialize without deny_unknown_fields, any existing serialized RuntimeSpec payloads that still contain a "provider": "incus" key will be silently tolerated during deserialization — this backward-compatibility behavior is verified by the renamed test in the next step.

Update spec tests for provider field removal

Intent: Ensure test coverage reflects the structural change and explicitly verifies that legacy payloads containing the removed `provider` field still deserialize correctly.

Affected files: crates/pika-cloud/src/spec.rs

Evidence

@@ -325,7 +321,6 @@ mod tests {
-        assert_eq!(spec.provider, ProviderKind::Incus);
         assert_eq!(spec.paths, RuntimePaths::default());

@@ -334,7 +329,7 @@ mod tests {
-    fn runtime_spec_defaults_paths_to_runtime_root() {
+    fn runtime_spec_defaults_paths_to_runtime_root_and_tolerates_legacy_provider_field() {

Two changes in the spec.rs test module:

The constructor test removes the assert_eq!(spec.provider, ProviderKind::Incus) assertion, since the field no longer exists.
The deserialization test is renamed from runtime_spec_defaults_paths_to_runtime_root to runtime_spec_defaults_paths_to_runtime_root_and_tolerates_legacy_provider_field. The test JSON fixture already includes a "provider": "incus" key, so this test now explicitly documents and verifies that old serialized specs containing the removed field are still accepted without error — a deliberate backward-compatibility guarantee.

Update import paths in jerichoci consumers

Intent: Fix all downstream references in the `jerichoci` crate so they import `IncusGuestRunRequest` and `INCUS_GUEST_RUN_REQUEST_SCHEMA_VERSION` from `pika_cloud::incus` instead of from the crate root.

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

Evidence

@@ -11,9 +11,8 @@ use pika_cloud::{
-    CLOUD_GUEST_LOG_PATH, EVENTS_PATH, IncusGuestRunRequest, LIFECYCLE_SCHEMA_VERSION, RESULT_PATH,
-    RuntimeArtifacts, RuntimeResultStatus, RuntimeTerminalResult, STATUS_PATH,
-    runtime_terminal_result_for_exit_code,
+    CLOUD_GUEST_LOG_PATH, EVENTS_PATH, LIFECYCLE_SCHEMA_VERSION, RESULT_PATH, RuntimeArtifacts,
+    RuntimeResultStatus, RuntimeTerminalResult, STATUS_PATH, runtime_terminal_result_for_exit_code,

@@ -3,7 +3,10 @@ use pika_cloud::incus::{
-use pika_cloud::incus::{INCUS_DEVICE_TYPE_KEY, INCUS_DISK_DEVICE_TYPE};
+use pika_cloud::incus::{
+    INCUS_DEVICE_TYPE_KEY, INCUS_DISK_DEVICE_TYPE, INCUS_GUEST_RUN_REQUEST_SCHEMA_VERSION,
+    IncusGuestRunRequest,
+};

@@ -2453,10 +2452,10 @@ mod tests {
+    use pika_cloud::incus::INCUS_GUEST_RUN_REQUEST_SCHEMA_VERSION;
     use pika_cloud::{
-        CLOUD_GUEST_LOG_PATH, EVENTS_PATH, GUEST_REQUEST_PATH,
-        INCUS_GUEST_RUN_REQUEST_SCHEMA_VERSION, LIFECYCLE_SCHEMA_VERSION, RESULT_PATH, STATUS_PATH,
-        runtime_terminal_result_for_exit_code,
+        CLOUD_GUEST_LOG_PATH, EVENTS_PATH, GUEST_REQUEST_PATH, LIFECYCLE_SCHEMA_VERSION,
+        RESULT_PATH, STATUS_PATH, runtime_terminal_result_for_exit_code,

With IncusGuestRunRequest and INCUS_GUEST_RUN_REQUEST_SCHEMA_VERSION no longer re-exported from the pika_cloud crate root, the two consuming files in jerichoci need updated imports.

executor.rs (main module):

Removes IncusGuestRunRequest from the use pika_cloud::{...} block (it was only needed in the incus submodule, not here).
In the test module, moves INCUS_GUEST_RUN_REQUEST_SCHEMA_VERSION to a separate use pika_cloud::incus::... import.

executor/incus.rs (Incus-specific executor logic):

Expands the existing use pika_cloud::incus::{...} import to include both INCUS_GUEST_RUN_REQUEST_SCHEMA_VERSION and IncusGuestRunRequest.
Updates the call site in build_remote_incus_guest_request from pika_cloud::INCUS_GUEST_RUN_REQUEST_SCHEMA_VERSION to the now-imported bare name INCUS_GUEST_RUN_REQUEST_SCHEMA_VERSION.

These are purely mechanical import-path changes with no behavioral impact.

pika-cloud-substrate-polish

CI: success

Summary

Tutorial Steps

Move IncusGuestRunRequest and schema version constant into the incus submodule

Relocate the round-trip test for IncusGuestRunRequest

Remove ProviderKind enum and its provider field from RuntimeSpec

Update spec tests for provider field removal

Update import paths in jerichoci consumers

Diff