Back to feed

sledtools/pika branch #110

pika-cloud-path-contracts

Tighten runtime path contracts and delete stale Incus plan doc

Target branch: master

Merge Commit: d892fddb0230d3108a4b60a7c364a51f8cc016c4

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

7 passed

head 8a6eec683b349e820866f14fbff6ffea884d72b4 · queued 2026-03-26 17:10:28 · 7 lane(s)

queued 7s · ran 12m 54s

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

Summary

This branch tightens runtime path contracts in the pika-cloud crate and removes stale documentation. In the code layer, RuntimePaths is refactored to embed RuntimeArtifactPaths via serde flatten instead of duplicating its three fields (events_path, status_path, result_path), and all validation and test call-sites in spec.rs are updated to go through the new nested struct. On the documentation side, the 1000-line incus-migration-plan.md is deleted entirely as a stale artifact, the incus-dev-lane.md runbook is trimmed of legacy microVM references, hard-cut cleanup steps, and dual-stack language, and the pika-cloud todo drops its back-reference to the removed plan doc.

Tutorial Steps

Embed RuntimeArtifactPaths into RuntimePaths via serde flatten

Intent: Eliminate field duplication between RuntimeArtifactPaths and RuntimePaths. The three artifact path fields (events_path, status_path, result_path) already had a dedicated struct; RuntimePaths was carrying its own copies with independent serde defaults. This step replaces the three loose fields with a single flattened embed, enforcing a single source of truth for those paths and their defaults.

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

Evidence
@@ -71,12 +71,8 @@ impl RuntimeArtifactPaths {
 pub struct RuntimePaths {
     #[serde(default = "runtime_state_dir")]
     pub state_dir: String,
-    #[serde(default = "events_path")]
-    pub events_path: String,
-    #[serde(default = "status_path")]
-    pub status_path: String,
-    #[serde(default = "result_path")]
-    pub result_path: String,
+    #[serde(flatten)]
+    pub runtime_artifacts: RuntimeArtifactPaths,
@@ -91,9 +87,7 @@ impl Default for RuntimePaths {
     fn default() -> Self {
         Self {
             state_dir: runtime_state_dir(),
-            events_path: events_path(),
-            status_path: status_path(),
-            result_path: result_path(),
+            runtime_artifacts: RuntimeArtifactPaths::default(),

The RuntimePaths struct previously declared events_path, status_path, and result_path directly, each with its own #[serde(default = "…")] attribute and a matching line in Default::default(). The crate already had RuntimeArtifactPaths holding exactly these three fields with identical defaults.

This change replaces the three loose fields with:

#[serde(flatten)]
pub runtime_artifacts: RuntimeArtifactPaths,

#[serde(flatten)] ensures the JSON wire format is unchanged — the three keys still appear at the top level of the RuntimePaths object, so existing serialized specs remain compatible. The Default impl delegates to RuntimeArtifactPaths::default() instead of calling three individual default functions.

This is a mechanical but important contract tightening: any future change to the canonical artifact paths only needs to happen in one place.

Update validation call-sites in spec.rs to use the nested struct

Intent: After the struct change, every place in RuntimeSpec::validate() that accessed self.paths.events_path, self.paths.status_path, or self.paths.result_path must go through self.paths.runtime_artifacts. This step updates both the absolute-path validation block and the lifecycle-root containment check.

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

Evidence
@@ -168,9 +168,18 @@ impl RuntimeSpec {
 
         validate_absolute_normalized_path("lifecycle_root", &self.lifecycle_root)?;
         validate_absolute_normalized_path("paths.state_dir", &self.paths.state_dir)?;
-        validate_absolute_normalized_path("paths.events_path", &self.paths.events_path)?;
-        validate_absolute_normalized_path("paths.status_path", &self.paths.status_path)?;
-        validate_absolute_normalized_path("paths.result_path", &self.paths.result_path)?;
+        validate_absolute_normalized_path(
+            "paths.events_path",
+            &self.paths.runtime_artifacts.events_path,
+        )?;
+        validate_absolute_normalized_path(
+            "paths.status_path",
+            &self.paths.runtime_artifacts.status_path,
+        )?;
+        validate_absolute_normalized_path(
+            "paths.result_path",
+            &self.paths.runtime_artifacts.result_path,
+        )?;
@@ -187,9 +196,18 @@ impl RuntimeSpec {
         }
 
         for (field, path) in [
-            ("paths.events_path", self.paths.events_path.as_str()),
-            ("paths.status_path", self.paths.status_path.as_str()),
-            ("paths.result_path", self.paths.result_path.as_str()),
+            (
+                "paths.events_path",
+                self.paths.runtime_artifacts.events_path.as_str(),
+            ),
+            (
+                "paths.status_path",
+                self.paths.runtime_artifacts.status_path.as_str(),
+            ),
+            (
+                "paths.result_path",
+                self.paths.runtime_artifacts.result_path.as_str(),
+            ),

The validate() method on RuntimeSpec performs two checks on the artifact paths:

  1. Absolute-normalized-path validation — each path must be absolute and normalized.
  2. Lifecycle-root containment — each path must live under self.lifecycle_root.

Both loops previously accessed self.paths.events_path etc. directly. After the flatten refactor, they now go through self.paths.runtime_artifacts.events_path (and the same for status_path and result_path).

The human-readable field labels passed to validate_absolute_normalized_path remain unchanged (e.g. "paths.events_path"), so error messages are stable.

Update test assertions for the new nested path access

Intent: Adjust unit tests so they exercise the new RuntimeArtifactPaths embed and confirm that defaults still produce the expected canonical constants.

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

Evidence
@@ -155,7 +149,9 @@ mod tests {
 
     #[test]
     fn runtime_paths_default_to_canonical_contract() {
-        assert_eq!(RuntimePaths::default().guest_log_path, GUEST_LOG_PATH);
+        let paths = RuntimePaths::default();
+        assert_eq!(paths.guest_log_path, GUEST_LOG_PATH);
+        assert_eq!(paths.runtime_artifacts.status_path, STATUS_PATH);
     }
@@ -465,7 +483,7 @@ mod tests {
     #[test]
     fn validate_rejects_paths_outside_lifecycle_root() {
         let mut spec = sample_runtime_spec();
-        spec.paths.status_path = "/tmp/status.json".to_string();
+        spec.paths.runtime_artifacts.status_path = "/tmp/status.json".to_string();
 
         let error = spec.validate().expect_err("path outside lifecycle root");

Two test changes confirm the refactor is wired correctly:

paths.rs — runtime_paths_default_to_canonical_contract

The test previously only checked guest_log_path. It now also asserts paths.runtime_artifacts.status_path == STATUS_PATH, proving that the RuntimeArtifactPaths::default() embed produces the same canonical value that the old direct field did.

spec.rs — validate_rejects_paths_outside_lifecycle_root

This negative test mutates status_path to a path outside the lifecycle root and expects validation to fail. The mutation target changes from spec.paths.status_path to spec.paths.runtime_artifacts.status_path.

Delete the stale incus-migration-plan.md

Intent: Remove the 1000-line Incus migration plan document that is no longer actionable. The managed-agent platform has already hard-cut to Incus, the pikaci migration is tracked elsewhere, and keeping the document risks misleading future readers about the current state of the system.

Affected files: docs/incus-migration-plan.md

Evidence
@@ -1,1000 +0,0 @@
----
-summary: Holistic plan for migrating the managed agent platform and eventually `pikaci` from `microvm.nix` to Incus.
-read_when:
-  - evaluating fleet architecture for managed agents

The entire docs/incus-migration-plan.md (1000 lines) is deleted. This document was a living migration plan written when the move from microvm.nix to Incus was still in progress. Now that:

  • the managed-agent product path is Incus + OpenClaw only,
  • the pikaci Incus path is the default with no remaining automatic exclusions,
  • legacy microVM teardown has been completed,

the plan doc has served its purpose. Keeping it would create a maintenance burden and a risk of stale guidance. Operational runbook content survives in the trimmed incus-dev-lane.md.

Trim incus-dev-lane.md of legacy microVM references and completed cleanup steps

Intent: Update the surviving operational runbook to reflect the current single-backend reality. Remove dual-stack language, the one-time hard-cut cleanup section (already executed), stale diagnostic outputs, and references to vm-spawner or microVM infrastructure that no longer exist in the managed-agent path.

Affected files: docs/incus-dev-lane.md

Evidence
@@ -17,8 +17,7 @@ It is intentionally narrow:
 - the guest image is Nix-built and imported into Incus manually
 - `pika-build` is the Incus substrate for this lane, not the agent API host
 - the managed-agent product path is now Incus-only and OpenClaw-only
-- `pika-server` still shares a host with other legacy microVM infrastructure, but managed-agent
-  requests no longer provision through that substrate
+- the runbook only covers the surviving Incus-managed runtime path
@@ -81,10 +80,7 @@ nix develop .#infra -c just -f infra/justfile build-deploy
 That entrypoint now uploads a clean repo snapshot and runs `nixos-rebuild` on `pika-build`
 itself, so operators do not need local `x86_64-linux` build support to deploy the canonical host.
 
-`pika-build` now runs both host roles side by side:
-
-- the existing microVM host stack and `vm-spawner`
-- the Incus dev lane with `incusd` listening on `:8443`
+`pika-build` now runs the Incus dev lane with `incusd` listening on `:8443`.
@@ -258,85 +253,6 @@ For a repeated internal dashboard check, the shortest useful loop is now:
 3. run `scripts/incus-dogfood-check.sh ...`
 4. refresh the dashboard until `state=ready` and `startup_phase=ready`
 
-## One-Time Hard-Cut Cleanup
-
-Before deploying the managed-agent hard cut that drops legacy provider identity from
-`agent_instances`, delete the remaining managed-agent microVM rows and VMs manually.
@@ -346,7 +262,7 @@ The first Incus operational model is intentionally narrow and volume-centric.
-This differs from the old microVM model:
+This differs from the old host-local model:
@@ -370,14 +286,14 @@ Current limitations:
-- do not expect managed-agent provisioning to fall back to `microvm`
+- do not expect managed-agent provisioning to fall back to another runtime backend
@@ -441,7 +357,7 @@ These wrappers:
-- use normal `pika-server` ensure or recover semantics instead of direct `vm-spawner` deletion
+- use normal `pika-server` ensure or recover semantics

The incus-dev-lane.md runbook receives several targeted edits:

  1. Scope description — "pika-server still shares a host with other legacy microVM infrastructure" is replaced with a simple statement that the runbook only covers the Incus-managed path.

  2. Host role description — the dual-stack bullet list ("existing microVM host stack and vm-spawner" / "Incus dev lane") is collapsed to a single sentence: pika-build runs the Incus dev lane.

  3. One-Time Hard-Cut Cleanup section — the entire ~80-line section with SQL queries and vm-spawner deletion steps is removed. This cleanup has already been executed; keeping the instructions risks accidental re-runs or confusion.

  4. Diagnostic output — the "current guest ready marker" line is dropped from the check script output description, matching the current script behavior.

  5. Terminology — "old microVM model" becomes "old host-local model", "fall back to microvm" becomes "fall back to another runtime backend", and "instead of direct vm-spawner deletion" is simplified. These changes avoid referencing infrastructure that no longer exists in the product path.

Remove back-reference to deleted plan doc from pika-cloud.md todo

Intent: Keep the todo file's related-context links valid after deleting incus-migration-plan.md.

Affected files: todos/pika-cloud.md

Evidence
@@ -10,7 +10,6 @@ already decided. The goal here is to keep the next code slices coherent and smal
 
 Related context:
 
-- [`docs/incus-migration-plan.md`](../docs/incus-migration-plan.md)
 - [`todos/agents-platform.md`](./agents-platform.md)

The todos/pika-cloud.md file had a "Related context" section linking to both incus-migration-plan.md and agents-platform.md. The link to the now-deleted migration plan is removed, leaving agents-platform.md as the sole related-context pointer. This prevents broken cross-references in the repo.

Diff