From 978e66790cfbcc8239ae57a1c3d8d57653d5d9f3 Mon Sep 17 00:00:00 2001
From: atlas <atlas@hyperhive.local>
Date: Tue, 1 Sep 2026 13:25:42 +0200
Subject: [PATCH] fix(#3892): a corrupt power-intent row is an error, not an
 absent one
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

`PowerStore::get` ended `Ok(row.and_then(|s| Wanted::parse(&s)))`, so a
row holding an unparseable value collapsed into the same `Ok(None)` as
no row at all. Its doc comment stated the narrower case as the whole one
("`None` when the agent has no row yet"), which is what kept the gap
invisible.

The damage is not that a caller reads the wrong value. `get_or_seed`
takes `None` to mean "never seeded" and writes
`Wanted::from_running(running)` — so a corrupted row silently
**overwrote itself** with whatever the container happened to be doing at
that moment, which is precisely the inference an authoritative intent
store exists to prevent. Its own comment says the DB is authoritative
once seeded.

The error arm was already in the signature; this makes it reachable.
Every caller already handles `Result`: the two `get_or_seed` paths
propagate, so a corrupt row now fails that agent's boot reconcile
instead of erasing itself, and `workers::wanted` already skips an agent
whose intent could not be read.

The second test is the one that matters — it asserts the row still holds
its original bytes afterwards, because the overwrite, not the read, was
the bug. Both tests carry a valid and an absent row alongside, so
neither can pass by breaking `get` for everything.

Found by argus reviewing #3891, which reads this store but does not
write it.
---
 hive-c0re/src/stores/power.rs | 68 ++++++++++++++++++++++++++++++++++-
 1 file changed, 67 insertions(+), 1 deletion(-)

diff --git a/hive-c0re/src/stores/power.rs b/hive-c0re/src/stores/power.rs
index e34beb3d..d19fbbeb 100644
--- a/hive-c0re/src/stores/power.rs
+++ b/hive-c0re/src/stores/power.rs
@@ -109,6 +109,13 @@ impl PowerStore {
 
     /// Read an agent's intent. `None` when the agent has no row yet
     /// (callers seed from observed state via [`Self::get_or_seed`]).
+    ///
+    /// A row that exists but holds an unparseable value is an **error**, not
+    /// `None`. The two used to collapse, and the damage was not that a caller
+    /// read the wrong value: [`Self::get_or_seed`] takes `None` to mean "never
+    /// seeded" and writes what the container is *currently doing*, so a
+    /// corrupted row silently overwrote itself with the observed state this
+    /// store exists to overrule.
     pub fn get(&self, agent: &str) -> Result<Option<Wanted>> {
         let conn = self.conn.lock().expect("agent_power mutex poisoned");
         let row: Option<String> = conn
@@ -119,7 +126,12 @@ impl PowerStore {
             )
             .optional()
             .context("select agent_power")?;
-        Ok(row.and_then(|s| Wanted::parse(&s)))
+        match row {
+            None => Ok(None),
+            Some(stored) => Wanted::parse(&stored).map(Some).with_context(|| {
+                format!("agent_power row for {agent} holds an unknown intent {stored:?}")
+            }),
+        }
     }
 
     /// Write an agent's intent (last-writer-wins, synchronous at
@@ -178,6 +190,60 @@ mod tests {
         );
     }
 
+    /// Write a value `set` cannot produce, the way corruption would.
+    fn write_raw(store: &PowerStore, agent: &str, wanted: &str) {
+        let conn = store.conn.lock().expect("agent_power mutex poisoned");
+        conn.execute(
+            "INSERT INTO agent_power (agent, wanted, updated_at) VALUES (?1, ?2, 0)",
+            params![agent, wanted],
+        )
+        .expect("insert raw row");
+    }
+
+    /// A row that exists but does not parse is an error, not an absent one.
+    /// The two rows beside it are the control: this must not pass by breaking
+    /// `get` for everything.
+    #[test]
+    fn an_unparseable_row_is_an_error_not_an_absent_one() {
+        let store = PowerStore::open_in_memory().expect("open");
+        store.set("valid", Wanted::Up).expect("set");
+        write_raw(&store, "corrupt", "sideways");
+
+        assert_eq!(store.get("valid").expect("valid reads"), Some(Wanted::Up));
+        assert_eq!(store.get("missing").expect("absent reads"), None);
+
+        let err = store
+            .get("corrupt")
+            .expect_err("corrupt must not read as absent");
+        assert!(
+            format!("{err:#}").contains("sideways"),
+            "the error should name the offending value: {err:#}"
+        );
+    }
+
+    /// The damage the error arm exists to prevent: `get_or_seed` treats
+    /// `None` as "never seeded" and writes observed state, so a corrupt row
+    /// used to overwrite itself with whatever the container was doing.
+    #[test]
+    fn a_corrupt_row_is_not_reseeded_from_observed_state() {
+        let store = PowerStore::open_in_memory().expect("open");
+        write_raw(&store, "corrupt", "sideways");
+
+        assert!(store.get_or_seed("corrupt", true).is_err());
+
+        let stored: String = store
+            .conn
+            .lock()
+            .expect("agent_power mutex poisoned")
+            .query_row(
+                "SELECT wanted FROM agent_power WHERE agent = ?1",
+                params!["corrupt"],
+                |r| r.get(0),
+            )
+            .expect("row still there");
+        assert_eq!(stored, "sideways", "the corrupt row must be left alone");
+    }
+
     #[test]
     fn get_set_roundtrip_and_seed() {
         let store = PowerStore::open_in_memory().expect("open");