self-review 2026-08-10: make save/load/persist report real results
route_load was a stub response over the most destructive operation in the
server: engram_load resets the store before parsing, so a readable-but-
malformed snapshot left a hollow graph and the route answered {"ok":true}.
With 37GB of stale dated snapshots in the data dir as restore targets, that
is a live risk. Now returns the real return value plus node/edge counts and
an explicit hollow flag.
route_save discarded engram_save's return the same way; persist_canonical
returned a hardcoded 1, making 'let saved: Int = persist_canonical()' a dead
variable at six durable write paths.
This commit is contained in:
+37
-6
@@ -133,8 +133,11 @@ fn route_text_health(method: String, path: String, body: String) -> String {
|
||||
fn persist_canonical() -> Int {
|
||||
let dir_raw: String = env("ENGRAM_DATA_DIR")
|
||||
let dir: String = if str_eq(dir_raw, "") { "/tmp/engram" } else { dir_raw }
|
||||
engram_save(dir + "/snapshot.json")
|
||||
return 1
|
||||
// (2026-08-10 self-review) This returned a hardcoded 1, which made every
|
||||
// caller's `let saved: Int = persist_canonical()` a dead variable — six
|
||||
// durable write paths each believed they had confirmation of a successful
|
||||
// canonical persist and none of them had any. Propagate the real result.
|
||||
return engram_save(dir + "/snapshot.json")
|
||||
}
|
||||
|
||||
// INCOMPLETE-ROUTE FIX (2026-07-24 self-review): this route silently dropped
|
||||
@@ -329,8 +332,15 @@ fn route_save(method: String, path: String, body: String) -> String {
|
||||
let dir_raw: String = env("ENGRAM_DATA_DIR")
|
||||
let dir: String = if str_eq(dir_raw, "") { "/tmp/engram" } else { dir_raw }
|
||||
let p: String = if str_eq(p_raw, "") { dir + "/snapshot.json" } else { p_raw }
|
||||
engram_save(p)
|
||||
"{\"ok\":true,\"path\":\"" + p + "\"}"
|
||||
// (2026-08-10 self-review) engram_save returns 0 on an empty path and the
|
||||
// route discarded it, so the response was a literal "ok":true regardless
|
||||
// of whether anything was written. Report the actual result AND the counts
|
||||
// that were supposed to have been written — the same move that made
|
||||
// route_health honest on 2026-08-01. A caller can now tell "saved 13k
|
||||
// nodes" from "saved nothing and said ok".
|
||||
let sv: Int = engram_save(p)
|
||||
let sv_ok: String = if sv == 0 { "false" } else { "true" }
|
||||
"{\"ok\":" + sv_ok + ",\"path\":\"" + p + "\",\"node_count\":" + int_to_str(engram_node_count()) + ",\"edge_count\":" + int_to_str(engram_edge_count()) + "}"
|
||||
}
|
||||
|
||||
fn route_load(method: String, path: String, body: String) -> String {
|
||||
@@ -338,8 +348,29 @@ fn route_load(method: String, path: String, body: String) -> String {
|
||||
let dir_raw: String = env("ENGRAM_DATA_DIR")
|
||||
let dir: String = if str_eq(dir_raw, "") { "/tmp/engram" } else { dir_raw }
|
||||
let p: String = if str_eq(p_raw, "") { dir + "/snapshot.json" } else { p_raw }
|
||||
engram_load(p)
|
||||
ok_json()
|
||||
// (2026-08-10 self-review) This was a stub response over the single most
|
||||
// destructive operation in the server. engram_load returns 0 on an empty
|
||||
// path, an unopenable file, a zero-length file, or malloc failure — and
|
||||
// this route answered ok_json() in every one of those cases.
|
||||
//
|
||||
// Precise failure shape (el_runtime.c:9890): the fopen guard runs BEFORE
|
||||
// the store reset, so a MISSING path is genuinely safe — it returns 0 with
|
||||
// the graph intact. The dangerous case is a readable-but-malformed file:
|
||||
// the reset loop frees every node and edge FIRST, then parses, so a
|
||||
// truncated or non-snapshot JSON leaves a hollow store — and the caller
|
||||
// was told "ok":true. With 37 GB of stale dated snapshots sitting in the
|
||||
// data dir as tempting restore targets, "restore reported success and
|
||||
// silently emptied the graph" is a live risk, not a hypothetical one.
|
||||
//
|
||||
// Fix: surface the return value AND the resulting counts. node_count=0
|
||||
// after a load is the unambiguous hollow-store signal (same convention
|
||||
// route_health adopted 2026-08-01). Callers can now verify a restore
|
||||
// instead of trusting it.
|
||||
let ld: Int = engram_load(p)
|
||||
let ld_ok: String = if ld == 0 { "false" } else { "true" }
|
||||
let nc_after: Int = engram_node_count()
|
||||
let hollow: String = if nc_after == 0 { "true" } else { "false" }
|
||||
"{\"ok\":" + ld_ok + ",\"path\":\"" + p + "\",\"node_count\":" + int_to_str(nc_after) + ",\"edge_count\":" + int_to_str(engram_edge_count()) + ",\"hollow\":" + hollow + "}"
|
||||
}
|
||||
|
||||
// (2026-08-01 self-review) Health previously returned a hardcoded literal —
|
||||
|
||||
Reference in New Issue
Block a user