fix(persistence): legacy-mode graph WAL replay dropped raw RESP payload as cmd name (task #60)

CHANGELOG.md

@@ -6,6 +6,26 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Fixed — legacy-mode (`--disk-offload disable`) graph WAL replay silently dropped the entire graph plane on kill-9 restart (task #60)
+
+`replay_graph_wal` (`src/shard/shared_databases.rs`, the legacy-mode-only
+replay path taken when `persistence_dir` is set but disk-offload is
+disabled) passed the raw RESP-encoded `WalRecord::payload` directly as the
+`cmd` argument to `CommandReplayEngine::replay_command`, instead of parsing
+it into frames and passing the bare command name + `&[Frame]` args as the
+sibling `replay_graph_wal_v3` (disk-offload-enabled path) and
+`recovery.rs`'s KV `Command` replay both already did.
+`GraphReplayCollector::is_graph_command` compares against literal names
+like `b"GRAPH.CREATE"`, so it never matched the multi-line RESP blob —
+`graph_command_count()` stayed 0, the `replay_graph_commands` guard never
+fired, and every graph mutation was silently lost on restart. Regression
+from PR #236 (WAL v2 → v3 port). Un-gates 6 previously-RED crash-matrix
+cells (`tests/crash_matrix_cross_plane/{tests_legacy.rs,tests_spot.rs}`):
+legacy s1 `graph_isolated`, `txn_isolated_committed`,
+`txn_isolated_atomicity`, `mixed_all_planes_synced`,
+`mixed_all_planes_mid_pass_c`, and spot-check
+`cross_plane_spot_legacy_s4_graph_isolated`.
+
 ### Added — allocator-overhead and PageCache observability in INFO memory / Prometheus (task #58)
 
 `INFO memory` and `/metrics` previously had no way to see the gap between

src/shard/shared_databases.rs

@@ -1042,8 +1042,32 @@ pub fn replay_graph_wal(
         let mut dummy_dbs: Vec<Database> = (0..db_count).map(|_| Database::new()).collect();
         let mut selected_db = 0usize;
         let on_command = &mut |record: &WalRecord| {
-            if record.record_type == WalRecordType::Command {
-                engine.replay_command(&mut dummy_dbs, &record.payload, &[], &mut selected_db);
+            if record.record_type != WalRecordType::Command {
+                return;
+            }
+            // Payload is RESP-encoded (same format as AOF/WAL v2 blocks) — it
+            // must be parsed into frames before dispatch. `replay_command`
+            // expects a bare command name plus already-parsed `&[Frame]`
+            // args, never the raw multi-line RESP blob (mirrors
+            // `replay_graph_wal_v3` above and `recovery.rs`'s KV Command
+            // replay; passing the raw payload as `cmd` made
+            // `GraphReplayCollector::is_graph_command` never match, silently
+            // dropping the entire legacy-mode graph plane on restart).
+            let mut buf = bytes::BytesMut::from(&record.payload[..]);
+            let parse_cfg = crate::protocol::ParseConfig::default();
+            while let Ok(Some(frame)) = crate::protocol::parse::parse(&mut buf, &parse_cfg) {
+                let crate::protocol::Frame::Array(ref arr) = frame else {
+                    continue;
+                };
+                if arr.is_empty() {
+                    continue;
+                }
+                let cmd_name: &[u8] = match &arr[0] {
+                    crate::protocol::Frame::BulkString(s) => s.as_ref(),
+                    crate::protocol::Frame::SimpleString(s) => s.as_ref(),
+                    _ => continue,
+                };
+                engine.replay_command(&mut dummy_dbs, cmd_name, &arr[1..], &mut selected_db);
             }
         };
         let on_fpi = &mut |_record: &WalRecord| {};

tests/crash_matrix_cross_plane/tests_legacy.rs

@@ -1,28 +1,22 @@
 //! Legacy cells (`--disk-offload disable`), shards=1 only per the brief
 //! ("legacy mode has no cross-shard novelty for this suite's purpose").
 //!
-//! 5 cells here are RED, all one root cause: PR #288's own changelog states
-//! graph command replay from WAL v3 does not reconstruct the graph in
-//! legacy mode even with zero Pass C interference — a pre-existing gap
-//! unrelated to WAL recycling, deliberately not asserted by
-//! `tests/crash_recovery_wal_recycle_legacy.rs` for the same reason. This
-//! suite DOES assert it (the shared `scenarios::graph_isolated` /
-//! `mixed_synced` / `mixed_mid_pass_c` / `txn_isolated_committed` /
-//! `txn_isolated_atomicity` bodies make no exceptions), so each surfaces the
-//! same "ERR graph not found" — `harness::red_guard` skips them by default
-//! (set `MOON_CRASH_MATRIX_RED=1` to reproduce); see that function's doc for
-//! why a plain `#[ignore = "RED: ..."]` annotation does NOT achieve this
-//! (`--ignored` runs ignored tests).
+//! GREEN as of task #60: the 5 cells below were RED because
+//! `replay_graph_wal` (`src/shard/shared_databases.rs`, legacy-mode-only —
+//! the disk-offload path uses the separate `replay_graph_wal_v3`) passed
+//! the raw RESP-encoded `WalRecord::payload` blob directly as the `cmd`
+//! argument to `CommandReplayEngine::replay_command`, instead of parsing it
+//! into frames and passing the bare command name + `&[Frame]` args first
+//! (the pattern `replay_graph_wal_v3` and `recovery.rs`'s KV Command replay
+//! both already used). `GraphReplayCollector::is_graph_command` compares
+//! against literal names like `b"GRAPH.CREATE"`, so it never matched a
+//! multi-line RESP blob — `graph_command_count()` stayed 0 and the entire
+//! graph plane was silently dropped on restart. Fixed by adding the same
+//! parse-then-dispatch step; cells un-gated (no more `harness::red_guard`).
 
-use crate::harness::{self, Config};
+use crate::harness::Config;
 use crate::scenarios;
 
-const LEGACY_GRAPH_RED_REASON: &str = "legacy-mode (--disk-offload disable) graph WAL replay \
-     does not reconstruct the graph even with zero Pass C interference — \
-     PR #288 / task #43 changelog; tests/crash_recovery_wal_recycle_legacy.rs \
-     deliberately does not assert this either. Pre-existing gap, not this \
-     stage's job to fix — tracked separately.";
-
 #[test]
 #[ignore] // Requires built release binary; run explicitly.
 fn cross_plane_legacy_yes_s1_kv_isolated() {
@@ -32,9 +26,6 @@ fn cross_plane_legacy_yes_s1_kv_isolated() {
 #[test]
 #[ignore] // Requires built release binary; run explicitly.
 fn cross_plane_legacy_yes_s1_graph_isolated() {
-    if !harness::red_guard(LEGACY_GRAPH_RED_REASON) {
-        return;
-    }
     scenarios::graph_isolated(&Config::LEGACY_YES_S1);
 }
 
@@ -56,49 +47,33 @@ fn cross_plane_legacy_yes_s1_mq_isolated() {
     scenarios::mq_isolated(&Config::LEGACY_YES_S1);
 }
 
-// One root cause (legacy-mode graph WAL replay never reconstructs the
-// graph — same gap as `cross_plane_legacy_yes_s1_graph_isolated`) surfaces
-// in the 4 cells below too: each hits the same "ERR graph not found" the
-// isolated graph scenario does. This includes BOTH txn_isolated halves
-// (review round 3, P1 restructuring, task #52) — unlike prod_s1/prod_s4,
-// the atomicity half is NOT independently green here: `graph_create` itself
-// never survives a restart in legacy mode, so the atomicity scenario's
-// graph-leg assertion panics on "ERR graph not found" before it can ever
-// evaluate the atomicity claim. Confirmed on the VM, not assumed.
+// The same root cause (formerly: legacy-mode graph WAL replay never
+// reconstructed the graph — see the module doc) covered the 4 cells below
+// too, including BOTH txn_isolated halves (review round 3, P1
+// restructuring, task #52). Fixed alongside `graph_isolated` by task #60;
+// all now GREEN and un-gated.
 
 #[test]
 #[ignore] // Requires built release binary; run explicitly.
 fn cross_plane_legacy_yes_s1_txn_isolated_committed() {
-    if !harness::red_guard(LEGACY_GRAPH_RED_REASON) {
-        return;
-    }
     scenarios::txn_isolated_committed(&Config::LEGACY_YES_S1);
 }
 
 #[test]
 #[ignore] // Requires built release binary; run explicitly.
 fn cross_plane_legacy_yes_s1_txn_isolated_atomicity() {
-    if !harness::red_guard(LEGACY_GRAPH_RED_REASON) {
-        return;
-    }
     scenarios::txn_isolated_atomicity(&Config::LEGACY_YES_S1);
 }
 
 #[test]
 #[ignore] // Requires built release binary; run explicitly.
 fn cross_plane_legacy_yes_s1_mixed_all_planes_synced() {
-    if !harness::red_guard(LEGACY_GRAPH_RED_REASON) {
-        return;
-    }
     scenarios::mixed_synced(&Config::LEGACY_YES_S1);
 }
 
 #[test]
 #[ignore] // Requires built release binary; run explicitly.
 fn cross_plane_legacy_yes_s1_mixed_all_planes_mid_pass_c() {
-    if !harness::red_guard(LEGACY_GRAPH_RED_REASON) {
-        return;
-    }
     scenarios::mixed_mid_pass_c(&Config::LEGACY_YES_S1);
 }
 

tests/crash_matrix_cross_plane/tests_spot.rs

@@ -2,7 +2,7 @@
 //! production/legacy set (brief: "spot-check the remaining cells with 1-2
 //! representative scenarios each").
 
-use crate::harness::{self, Config};
+use crate::harness::Config;
 use crate::scenarios;
 
 // ---------------------------------------------------------------------------
@@ -27,25 +27,15 @@ fn cross_plane_spot_offload_no_s4_no_durability_contract_liveness() {
 // ---------------------------------------------------------------------------
 // Legacy mode at shards=4 (excluded from the full matrix — "legacy mode has
 // no cross-shard novelty" — but the brief still wants 1-2 representative
-// scenarios). graph_isolated here reproduces the SAME RED gap as
-// legacy_yes_s1 (the bug is legacy-mode WAL replay, not shard-count-specific;
-// spot-checking at shards=4 confirms the gap is not accidentally
-// shard-count-dependent). Gated by `harness::red_guard` — see that
-// function's doc for why `#[ignore = "RED: ..."]` alone does not skip it.
+// scenarios). graph_isolated here used to reproduce the SAME RED gap as
+// legacy_yes_s1 (the bug was legacy-mode WAL replay, not shard-count-
+// specific). GREEN as of task #60 (see `tests_legacy.rs` module doc for the
+// root cause); confirms the fix is not accidentally shard-count-dependent.
 // ---------------------------------------------------------------------------
 
 #[test]
 #[ignore] // Requires built release binary; run explicitly.
 fn cross_plane_spot_legacy_s4_graph_isolated() {
-    if !harness::red_guard(
-        "legacy-mode (--disk-offload disable) graph WAL replay does not \
-         reconstruct the graph — same gap as \
-         cross_plane_legacy_yes_s1_graph_isolated, confirmed shard-count- \
-         independent (PR #288 / task #43 changelog). Not this stage's job \
-         to fix — tracked separately.",
-    ) {
-        return;
-    }
     scenarios::graph_isolated(&Config::SPOT_LEGACY_S4);
 }