From 791b0880b715ffb2adf3d7f72c4805e11378e0e9 Mon Sep 17 00:00:00 2001 From: Will Anderson Date: Mon, 10 Aug 2026 08:39:36 -0500 Subject: [PATCH] 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. --- engram/dist/engram.c | 39 +++++++++++++++++++++------------------ engram/src/server.el | 43 +++++++++++++++++++++++++++++++++++++------ 2 files changed, 58 insertions(+), 24 deletions(-) diff --git a/engram/dist/engram.c b/engram/dist/engram.c index ee8b408..eebdc34 100644 --- a/engram/dist/engram.c +++ b/engram/dist/engram.c @@ -134,8 +134,7 @@ el_val_t route_text_health(el_val_t method, el_val_t path, el_val_t body) { el_val_t persist_canonical(void) { el_val_t dir_raw = env(EL_STR("ENGRAM_DATA_DIR")); el_val_t dir = ({ el_val_t _if_result_1 = 0; if (str_eq(dir_raw, EL_STR(""))) { _if_result_1 = (EL_STR("/tmp/engram")); } else { _if_result_1 = (dir_raw); } _if_result_1; }); - engram_save(el_str_concat(dir, EL_STR("/snapshot.json"))); - return 1; + return engram_save(el_str_concat(dir, EL_STR("/snapshot.json"))); return 0; } @@ -303,18 +302,22 @@ el_val_t route_save(el_val_t method, el_val_t path, el_val_t body) { el_val_t dir_raw = env(EL_STR("ENGRAM_DATA_DIR")); el_val_t dir = ({ el_val_t _if_result_19 = 0; if (str_eq(dir_raw, EL_STR(""))) { _if_result_19 = (EL_STR("/tmp/engram")); } else { _if_result_19 = (dir_raw); } _if_result_19; }); el_val_t p = ({ el_val_t _if_result_20 = 0; if (str_eq(p_raw, EL_STR(""))) { _if_result_20 = (el_str_concat(dir, EL_STR("/snapshot.json"))); } else { _if_result_20 = (p_raw); } _if_result_20; }); - engram_save(p); - return el_str_concat(el_str_concat(EL_STR("{\"ok\":true,\"path\":\""), p), EL_STR("\"}")); + el_val_t sv = engram_save(p); + el_val_t sv_ok = ({ el_val_t _if_result_21 = 0; if ((sv == 0)) { _if_result_21 = (EL_STR("false")); } else { _if_result_21 = (EL_STR("true")); } _if_result_21; }); + return el_str_concat(el_str_concat(el_str_concat(el_str_concat(el_str_concat(el_str_concat(el_str_concat(el_str_concat(EL_STR("{\"ok\":"), sv_ok), EL_STR(",\"path\":\"")), p), EL_STR("\",\"node_count\":")), int_to_str(engram_node_count())), EL_STR(",\"edge_count\":")), int_to_str(engram_edge_count())), EL_STR("}")); return 0; } el_val_t route_load(el_val_t method, el_val_t path, el_val_t body) { el_val_t p_raw = json_get_string(body, EL_STR("path")); el_val_t dir_raw = env(EL_STR("ENGRAM_DATA_DIR")); - el_val_t dir = ({ el_val_t _if_result_21 = 0; if (str_eq(dir_raw, EL_STR(""))) { _if_result_21 = (EL_STR("/tmp/engram")); } else { _if_result_21 = (dir_raw); } _if_result_21; }); - el_val_t p = ({ el_val_t _if_result_22 = 0; if (str_eq(p_raw, EL_STR(""))) { _if_result_22 = (el_str_concat(dir, EL_STR("/snapshot.json"))); } else { _if_result_22 = (p_raw); } _if_result_22; }); - engram_load(p); - return ok_json(); + el_val_t dir = ({ el_val_t _if_result_22 = 0; if (str_eq(dir_raw, EL_STR(""))) { _if_result_22 = (EL_STR("/tmp/engram")); } else { _if_result_22 = (dir_raw); } _if_result_22; }); + el_val_t p = ({ el_val_t _if_result_23 = 0; if (str_eq(p_raw, EL_STR(""))) { _if_result_23 = (el_str_concat(dir, EL_STR("/snapshot.json"))); } else { _if_result_23 = (p_raw); } _if_result_23; }); + el_val_t ld = engram_load(p); + el_val_t ld_ok = ({ el_val_t _if_result_24 = 0; if ((ld == 0)) { _if_result_24 = (EL_STR("false")); } else { _if_result_24 = (EL_STR("true")); } _if_result_24; }); + el_val_t nc_after = engram_node_count(); + el_val_t hollow = ({ el_val_t _if_result_25 = 0; if ((nc_after == 0)) { _if_result_25 = (EL_STR("true")); } else { _if_result_25 = (EL_STR("false")); } _if_result_25; }); + return el_str_concat(el_str_concat(el_str_concat(el_str_concat(el_str_concat(el_str_concat(el_str_concat(el_str_concat(el_str_concat(el_str_concat(EL_STR("{\"ok\":"), ld_ok), EL_STR(",\"path\":\"")), p), EL_STR("\",\"node_count\":")), int_to_str(nc_after)), EL_STR(",\"edge_count\":")), int_to_str(engram_edge_count())), EL_STR(",\"hollow\":")), hollow), EL_STR("}")); return 0; } @@ -336,7 +339,7 @@ el_val_t route_embed_backfill(el_val_t method, el_val_t path, el_val_t body) { el_val_t route_sync(el_val_t method, el_val_t path, el_val_t body) { el_val_t dir_raw = env(EL_STR("ENGRAM_DATA_DIR")); - el_val_t dir = ({ el_val_t _if_result_23 = 0; if (str_eq(dir_raw, EL_STR(""))) { _if_result_23 = (EL_STR("/tmp/engram")); } else { _if_result_23 = (dir_raw); } _if_result_23; }); + el_val_t dir = ({ el_val_t _if_result_26 = 0; if (str_eq(dir_raw, EL_STR(""))) { _if_result_26 = (EL_STR("/tmp/engram")); } else { _if_result_26 = (dir_raw); } _if_result_26; }); el_val_t snap_path = el_str_concat(dir, EL_STR("/.sync-export.json")); engram_save(snap_path); el_val_t snap = fs_read(snap_path); @@ -375,7 +378,7 @@ el_val_t route_emit_ise(el_val_t method, el_val_t path, el_val_t body) { el_val_t conf = el_from_float(0.8); el_val_t id = engram_node_full(content, EL_STR("InternalStateEvent"), EL_STR("state-event"), sal, imp, conf, EL_STR("Episodic"), EL_STR("[\"internal-state\",\"InternalStateEvent\"]")); el_val_t ret_raw = env(EL_STR("ENGRAM_ISE_RETENTION_MS")); - el_val_t ret_ms = ({ el_val_t _if_result_24 = 0; if (str_eq(ret_raw, EL_STR(""))) { _if_result_24 = (172800000); } else { _if_result_24 = (str_to_int(ret_raw)); } _if_result_24; }); + el_val_t ret_ms = ({ el_val_t _if_result_27 = 0; if (str_eq(ret_raw, EL_STR(""))) { _if_result_27 = (172800000); } else { _if_result_27 = (str_to_int(ret_raw)); } _if_result_27; }); el_val_t pruned = engram_prune_telemetry(ret_ms); return el_str_concat(el_str_concat(el_str_concat(el_str_concat(EL_STR("{\"ok\":true,\"id\":\""), id), EL_STR("\",\"pruned\":")), int_to_str(pruned)), EL_STR("}")); return 0; @@ -387,21 +390,21 @@ el_val_t route_capture_knowledge(el_val_t method, el_val_t path, el_val_t body) return err_json(EL_STR("missing content")); } el_val_t title = json_get_string(body, EL_STR("title")); - el_val_t label = ({ el_val_t _if_result_25 = 0; if (str_eq(title, EL_STR(""))) { _if_result_25 = (str_slice(content, 0, 60)); } else { _if_result_25 = (title); } _if_result_25; }); + el_val_t label = ({ el_val_t _if_result_28 = 0; if (str_eq(title, EL_STR(""))) { _if_result_28 = (str_slice(content, 0, 60)); } else { _if_result_28 = (title); } _if_result_28; }); el_val_t category_raw = json_get_string(body, EL_STR("category")); - el_val_t category = ({ el_val_t _if_result_26 = 0; if (str_eq(category_raw, EL_STR(""))) { _if_result_26 = (EL_STR("other")); } else { _if_result_26 = (category_raw); } _if_result_26; }); + el_val_t category = ({ el_val_t _if_result_29 = 0; if (str_eq(category_raw, EL_STR(""))) { _if_result_29 = (EL_STR("other")); } else { _if_result_29 = (category_raw); } _if_result_29; }); el_val_t ktier_raw = json_get_string(body, EL_STR("tier")); - el_val_t ktier = ({ el_val_t _if_result_27 = 0; if (str_eq(ktier_raw, EL_STR(""))) { _if_result_27 = (EL_STR("note")); } else { _if_result_27 = (ktier_raw); } _if_result_27; }); + el_val_t ktier = ({ el_val_t _if_result_30 = 0; if (str_eq(ktier_raw, EL_STR(""))) { _if_result_30 = (EL_STR("note")); } else { _if_result_30 = (ktier_raw); } _if_result_30; }); el_val_t project = json_get_string(body, EL_STR("project")); el_val_t tags_raw = json_get_raw(body, EL_STR("tags")); - el_val_t tags_base = ({ el_val_t _if_result_28 = 0; if (str_eq(tags_raw, EL_STR(""))) { _if_result_28 = (EL_STR("[]")); } else { _if_result_28 = (tags_raw); } _if_result_28; }); + el_val_t tags_base = ({ el_val_t _if_result_31 = 0; if (str_eq(tags_raw, EL_STR(""))) { _if_result_31 = (EL_STR("[]")); } else { _if_result_31 = (tags_raw); } _if_result_31; }); el_val_t base_len = str_len(tags_base); el_val_t head = str_slice(tags_base, 0, (base_len - 1)); - el_val_t sep = ({ el_val_t _if_result_29 = 0; if (str_eq(head, EL_STR("["))) { _if_result_29 = (EL_STR("")); } else { _if_result_29 = (EL_STR(",")); } _if_result_29; }); + el_val_t sep = ({ el_val_t _if_result_32 = 0; if (str_eq(head, EL_STR("["))) { _if_result_32 = (EL_STR("")); } else { _if_result_32 = (EL_STR(",")); } _if_result_32; }); el_val_t safe_cat = str_replace(category, EL_STR("\""), EL_STR("'")); el_val_t safe_tier = str_replace(ktier, EL_STR("\""), EL_STR("'")); el_val_t safe_proj = str_replace(project, EL_STR("\""), EL_STR("'")); - el_val_t proj_tag = ({ el_val_t _if_result_30 = 0; if (str_eq(safe_proj, EL_STR(""))) { _if_result_30 = (EL_STR("")); } else { _if_result_30 = (el_str_concat(el_str_concat(EL_STR(",\"project:"), safe_proj), EL_STR("\""))); } _if_result_30; }); + el_val_t proj_tag = ({ el_val_t _if_result_33 = 0; if (str_eq(safe_proj, EL_STR(""))) { _if_result_33 = (EL_STR("")); } else { _if_result_33 = (el_str_concat(el_str_concat(EL_STR(",\"project:"), safe_proj), EL_STR("\""))); } _if_result_33; }); el_val_t tags = el_str_concat(el_str_concat(el_str_concat(el_str_concat(el_str_concat(el_str_concat(el_str_concat(el_str_concat(head, sep), EL_STR("\"category:")), safe_cat), EL_STR("\",\"tier:")), safe_tier), EL_STR("\"")), proj_tag), EL_STR("]")); el_val_t sal = el_from_float(0.5); el_val_t imp = el_from_float(0.5); @@ -531,10 +534,10 @@ el_val_t handle_request(el_val_t method, el_val_t path, el_val_t body) { int main(int _argc, char** _argv) { el_runtime_init_args(_argc, _argv); bind_raw = env(EL_STR("ENGRAM_BIND")); - bind_str = ({ el_val_t _if_result_31 = 0; if (str_eq(bind_raw, EL_STR(""))) { _if_result_31 = (EL_STR(":8742")); } else { _if_result_31 = (bind_raw); } _if_result_31; }); + bind_str = ({ el_val_t _if_result_34 = 0; if (str_eq(bind_raw, EL_STR(""))) { _if_result_34 = (EL_STR(":8742")); } else { _if_result_34 = (bind_raw); } _if_result_34; }); port = parse_port(bind_str); data_dir_raw = env(EL_STR("ENGRAM_DATA_DIR")); - data_dir = ({ el_val_t _if_result_32 = 0; if (str_eq(data_dir_raw, EL_STR(""))) { _if_result_32 = (EL_STR("/tmp/engram")); } else { _if_result_32 = (data_dir_raw); } _if_result_32; }); + data_dir = ({ el_val_t _if_result_35 = 0; if (str_eq(data_dir_raw, EL_STR(""))) { _if_result_35 = (EL_STR("/tmp/engram")); } else { _if_result_35 = (data_dir_raw); } _if_result_35; }); snapshot_path = el_str_concat(data_dir, EL_STR("/snapshot.json")); engram_load(snapshot_path); boot_snap = fs_read(snapshot_path); diff --git a/engram/src/server.el b/engram/src/server.el index e5bab58..ab3b627 100644 --- a/engram/src/server.el +++ b/engram/src/server.el @@ -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 —