diff --git a/chat.el b/chat.el index 913259d..ef8086e 100644 --- a/chat.el +++ b/chat.el @@ -94,18 +94,39 @@ fn hist_append(hist: String, role: String, content: String) -> String { return "[" + inner + "," + entry + "]" } +// hist_trim — drop the oldest two entries from a history JSON array. +// +// Issue #5 (BROKEN 20-TURN TRIM) + Issue #10 (OFF-BY-ONE): the original code uses +// str_index_of to find '{"role":' markers by raw string scanning. If any message content +// contains the literal string '{"role":' (e.g. the LLM quoted JSON), the marker search +// lands inside a content value and the resulting slice is malformed. Additionally, the +// function had no minimum-retained-count guard. +// +// Fix: use json_array_len / json_array_get to work at the structural level, immune to +// content containing marker strings. Drop entries 0 and 1 (oldest user+assistant pair) +// and rebuild from entry 2 onward. Minimum retained count: 2 entries (never over-trim). fn hist_trim(hist: String) -> String { - let inner: String = str_slice(hist, 1, str_len(hist) - 1) - let marker: String = "{\"role\":" - let i1: Int = str_index_of(inner, marker) - let tail1: String = str_slice(inner, i1 + 1, str_len(inner)) - let i2: Int = str_index_of(tail1, marker) - let tail2: String = str_slice(tail1, i2 + 1, str_len(tail1)) - let i3: Int = str_index_of(tail2, marker) - if i3 >= 0 { - return "[" + str_slice(tail2, i3, str_len(tail2)) + "]" + let total: Int = json_array_len(hist) + // Safety: never trim below 2 entries. If already at or below the minimum, return unchanged. + if total <= 2 { + return hist } - return hist + // Drop entry 0 and entry 1 (oldest user+assistant pair). Rebuild from entry 2 onward. + let result: String = "" + let i: Int = 2 + while i < total { + let entry: String = json_array_get(hist, i) + let result = if str_eq(result, "") { + entry + } else { + result + "," + entry + } + let i = i + 1 + } + if str_eq(result, "") { + return hist + } + return "[" + result + "]" } // clean_llm_response — strips GPT-2 BPE byte-to-unicode artifacts that vLLM @@ -124,29 +145,72 @@ fn clean_llm_response(s: String) -> String { } // conv_history_persist — save conversation history to engram for cross-restart continuity. -// Stores as a Conversation node. Overwrites by using consistent label "conv:history". +// Stores as a Conversation node with label "conv:history". +// +// Issue #4 (OVERWRITE WITHOUT DELETE): engram_node_full behaviour on duplicate labels is +// implementation-defined. If it appends rather than upserts, stale older nodes accumulate. +// TODO: replace with explicit delete-then-create once engram exposes a label-scoped delete API. +// +// Issue #7 (DUAL STORAGE): auto_persist() also writes a per-turn Conversation node per turn. +// Both run every turn for different purposes (rolling array vs. Q&A snapshot). Documented here. fn conv_history_persist(hist: String) -> Void { if str_eq(hist, "") { return "" } if str_eq(hist, "[]") { return "" } - let ts: Int = time_now() + // Issue #6 (PARTIAL-WRITE GUARD): refuse to persist a blob that is not a complete JSON + // array. A truncated write starting with '[' but missing ']' passes the old + // str_starts_with check and would overwrite a good node with a corrupt one. + if !str_starts_with(hist, "[") { return "" } + if !str_contains(hist, "]") { return "" } let tags: String = "[\"conv-history\",\"persistent\"]" - let discard: String = engram_node_full( + let node_id: String = engram_node_full( hist, "Conversation", "conv:history", el_from_float(0.7), el_from_float(0.8), el_from_float(0.9), "Episodic", tags ) + // Issue #2 (SILENT FAILURE): surface write failures in logs rather than dropping silently. + if str_eq(node_id, "") { + println("[chat] conv_history_persist: engram_node_full returned empty — history node may be lost") + } } // conv_history_load — restore conversation history from engram on first access. -// Returns the most recent "conv:history" node content, or "" if none found. +// +// Issue #1 (ASYMMETRIC PERSIST/LOAD): original code loaded only via vector search, which +// is not symmetric with the label-based write in conv_history_persist. A cold or corrupt +// vector index returns [] even when the node exists on disk. Fixed by trying a label-based +// fetch (engram_get_node_by_label) first, falling back to vector search only when that fails. +// +// Issue #2 (SILENT LOAD FAILURE): all failure paths now emit a log line so history loss +// is visible rather than silently treated as a first-turn conversation. +// +// Issue #6 (PARTIAL-WRITE GUARD): content must start with '[' AND contain ']' before +// being accepted — a truncated write that starts with '[' but has no ']' would pass the +// old str_starts_with check and cause downstream json_array_len to malfunction. fn conv_history_load() -> String { + // Primary: label-based fetch — symmetric with persist, immune to vector index drift. + let label_node: String = engram_get_node_by_label("conv:history") + let label_ok: Bool = !str_eq(label_node, "") && !str_eq(label_node, "null") + if label_ok { + let label_content: String = json_get(label_node, "content") + let label_valid: Bool = str_starts_with(label_content, "[") && str_contains(label_content, "]") + if label_valid { + return label_content + } + // Label node exists but content is invalid — partial write or corruption. + println("[chat] conv_history_load: label node found but content invalid — falling back to vector search") + } + + // Fallback: vector search — covers nodes indexed before this fix, or on cold index. let results: String = engram_search_json("conv:history", 3) if str_eq(results, "") { return "" } if str_eq(results, "[]") { return "" } let node: String = json_array_get(results, 0) let content: String = json_get(node, "content") - // Validate it looks like a JSON array - if !str_starts_with(content, "[") { return "" } + // Issue #6: full partial-write guard — require both '[' prefix AND ']' presence. + if !str_starts_with(content, "[") || !str_contains(content, "]") { + println("[chat] conv_history_load: vector search result content invalid — treating as first turn") + return "" + } return content } @@ -157,6 +221,13 @@ fn handle_chat(body: String) -> String { } // Load history BEFORE compiling context so we can anchor activation to the thread. + // Issue #3 (NO RECOVERY PATH): when conv_history_load() returns "" (corrupted node, + // missing embeddings, search failure), handle_chat treats it identically to a genuine + // first-turn conversation — no retry, no ID fallback, no caller signal. The old history + // node also sits as an orphaned entry in engram and is never cleaned up. The improvements + // in conv_history_load() (Issues #1, #2) reduce false negatives, but a full recovery path + // requires caller-level state changes too invasive for a targeted fix. + // TODO: add a load-failure signal to the response envelope so callers can surface it. let state_hist: String = state_get("conv_history") let stored_hist: String = if str_eq(state_hist, "") { conv_history_load() } else { state_hist } let hist_len: Int = if str_eq(stored_hist, "") { 0 } else { json_array_len(stored_hist) } @@ -186,6 +257,13 @@ fn handle_chat(body: String) -> String { let req_model: String = json_get(body, "model") let model: String = if str_eq(req_model, "") { chat_default_model() } else { req_model } + // Safety augmentation on the main chat path. Previously only applied on the + // handle_chat_as_soul / handle_dharma_room_turn paths. The phrase-list bell + // detector (safety_augment_system) was absent from handle_chat, so a user + // expressing crisis in the primary conversational UI bypassed soft/hard + // directive injection entirely. Applying it here before every llm_call_system. + let full_system = safety_augment_system(full_system, message) + let raw_response: String = llm_call_system(model, full_system, message) let is_error: Bool = str_starts_with(raw_response, "{\"error\"") @@ -200,6 +278,11 @@ fn handle_chat(body: String) -> String { let updated_hist: String = hist_append(stored_hist, "user", message) let updated_hist2: String = hist_append(updated_hist, "assistant", raw_response) + // Issue #8 (NO MAX SIZE GUARD): the 20-turn count limit bounds entry count, but individual + // messages can be arbitrarily large (up to max_tokens = 4096 tokens each). At 20 turns the + // history blob can reach ~80KB before trim fires. engram_node_full has no apparent size cap. + // A byte-length cap would require truncating or summarising entries — too invasive here. + // TODO: add a byte-length cap (e.g. 32KB) that drops oldest entries until under limit. let final_hist: String = if json_array_len(updated_hist2) > 20 { hist_trim(updated_hist2) } else { @@ -509,12 +592,17 @@ fn dispatch_tool(tool_name: String, tool_input: String) -> String { let path: String = json_get(tool_input, "path") let old_text: String = json_get(tool_input, "old_text") let new_text: String = json_get(tool_input, "new_text") - let content: String = fs_read(path) + let root: String = agent_workspace_root() + if !path_within_root(path, root) { + return json_safe("denied: path is outside the agent workspace root") + } + let resolved: String = resolve_in_root(path, root) + let content: String = fs_read(resolved) if str_eq(content, "") { return json_safe("{\"error\":\"file not found\"}") } let updated: String = str_replace(content, old_text, new_text) - fs_write(path, updated) + fs_write(resolved, updated) return json_safe("{\"ok\":true}") } if str_eq(tool_name, "remember") { @@ -675,12 +763,23 @@ fn handle_chat_agentic(body: String) -> String { // Persist the exchange to session/global history for thread continuity on next turn. // Only save when the loop completed (reply present), not when tool_pending. + // + // Issue #9 (AGENTIC HISTORY NOT PERSISTED): the agentic path previously only saved + // history to in-process state (state_set), which is lost on restart. We now also call + // conv_history_persist() for the default session (hist_key == "conv_history") so agentic + // history survives restarts the same way non-agentic history does. Per-session histories + // (session_hist_) are still in-process only — persisting all named sessions would + // require per-session engram labels, a larger change tracked separately. let reply_text: String = json_get(result, "reply") let discard_hist: Bool = if !str_eq(reply_text, "") { let updated: String = hist_append(agentic_hist, "user", message) let updated2: String = hist_append(updated, "assistant", reply_text) let trimmed: String = if json_array_len(updated2) > 20 { hist_trim(updated2) } else { updated2 } state_set(hist_key, trimmed) + // Only persist the default global session to engram — named sessions are ephemeral. + if str_eq(hist_key, "conv_history") { + conv_history_persist(trimmed) + } true } else { false } @@ -1054,13 +1153,19 @@ fn handle_dharma_room_turn(body: String) -> String { // engram_node(content, "episodic", ...) which wrongly put a TIER into the node_type // slot — that's why nodes showed node_type="episodic". Use the full, correct contract.) let utterance_tags: String = "[\"soul-utterance\",\"episodic\"]" - let discard_id: String = engram_node_full( + let utterance_id: String = engram_node_full( clean_response, "Conversation", "soul:utterance", el_from_float(0.6), el_from_float(0.6), el_from_float(0.8), "Episodic", utterance_tags ) + if str_eq(utterance_id, "") { + println("[chat] handle_dharma_room_turn: utterance engram write failed — node lost") + } if !str_eq(snap_path, "") { - let discard_save: String = engram_save(snap_path) + let save_result: String = engram_save(snap_path) + if str_eq(save_result, "") { + println("[chat] handle_dharma_room_turn: engram_save failed for " + snap_path) + } } let safe_response: String = json_safe(clean_response)