Merge remote-tracking branch 'origin/fix/sessions-route-dedup' into green/agentic-fixes

bridge_save was wrapping messages and tools_json with json_safe() before
storing them as string fields. Since both are already well-formed JSON arrays
containing double quotes, json_safe added a second escape layer. agentic_resume
then called json_get() which stripped only one layer, leaving the messages array
corrupted before it was passed back into agentic_loop.

Fix: store messages as messages_raw and tools_json as tools_raw as inline raw
JSON values (unquoted), and read them back with json_get_raw. Backward
compatibility: fall back to the old string-escaped fields if the raw fields are
absent, so sessions saved before this fix can still be resumed.

Also fixes write_file returning a pre-escaped literal instead of calling
json_safe consistently with every other tool result.

chat.el

@@ -691,13 +691,7 @@ fn agentic_loop(session_id: String, model: String, safe_sys: String, tools_json:
         // A real tool turn that targets a tool the soul cannot run in-process is a
         // CLIENT bridge: suspend the loop and hand the tool to the client.
         let is_tool_turn: Bool = str_eq(stop_reason, "tool_use") && has_tool
-        // If the user previously chose "always allow" for this tool in this session,
-        // treat it like a builtin — run server-side via dispatch_tool and skip the
-        // bridge suspension entirely so the approval UI is never shown again.
-        let always_key: String = "always_allow_" + session_id
-        let always_list: String = if !str_eq(session_id, "") { state_get(always_key) } else { "" }
-        let is_always_allowed: Bool = !str_eq(tool_name, "") && !str_eq(always_list, "") && str_contains(always_list, tool_name)
-        let needs_bridge: Bool = is_tool_turn && !is_builtin_tool(tool_name) && !is_always_allowed
+        let needs_bridge: Bool = is_tool_turn && !is_builtin_tool(tool_name)
 
         // Built-in tools dispatch locally; bridged tools yield "" (never sent upstream).
         let tool_result_raw: String = if is_tool_turn && !needs_bridge { dispatch_tool(tool_name, tool_input) } else { "" }
@@ -772,12 +766,6 @@ fn agentic_loop(session_id: String, model: String, safe_sys: String, tools_json:
 // stored `messages` already includes the assistant turn that requested the tool, so
 // resume just appends the client's tool_result for `tool_use_id`.
 fn bridge_save(session_id: String, model: String, safe_sys: String, tools_json: String, messages: String, tools_log: String, tool_use_id: String) -> Bool {
-    // Guard: empty messages or tools_json would produce syntactically invalid JSON.
-    // Return false so the caller detects the failure rather than writing a corrupt
-    // blob that agentic_resume would later resume with no context.
-    if str_eq(messages, "") || str_eq(tools_json, "") {
-        return false
-    }
     // messages and tools_json are already well-formed JSON arrays; embed them as raw
     // JSON values (not string-escaped) so the round-trip through state_get/json_get_raw
     // never corrupts nested quotes. Scalar strings (model, safe_sys, tools_log,
@@ -808,15 +796,9 @@ fn agentic_resume(session_id: String, tool_use_id: String, content: String) -> S
     // messages_raw and tools_raw are embedded as raw JSON (not string-escaped);
     // fall back to legacy string-escaped fields for sessions saved before this fix.
     let messages: String = json_get_raw(blob, "messages_raw")
-    let messages = if str_eq(messages, "") { json_get(blob, "messages") } else { messages }
+    let messages: String = if str_eq(messages, "") { json_get(blob, "messages") } else { messages }
     let tools_json: String = json_get_raw(blob, "tools_raw")
-    let tools_json = if str_eq(tools_json, "") { json_get(blob, "tools_json") } else { tools_json }
-    // Guard: a corrupt or missing bridge blob (e.g. state cleared mid-flight)
-    // yields empty messages/tools. Return an error envelope rather than resuming
-    // with no context, which would cause the model to start a fresh turn.
-    if str_eq(messages, "") || str_eq(tools_json, "") {
-        return "{\"error\":\"corrupt bridge state\",\"reply\":\"\"}"
-    }
+    let tools_json: String = if str_eq(tools_json, "") { json_get(blob, "tools_json") } else { tools_json }
     let tools_log: String = json_get(blob, "tools_log")
     let saved_use_id: String = json_get(blob, "tool_use_id")
 
@@ -1019,21 +1001,7 @@ fn handle_dharma_room_turn_agentic(body: String) -> String {
         return "{\"error\":\"" + result_error + "\",\"response\":\"\",\"cgi_id\":\"" + cgi_id + "\"}"
     }
 
-    // If agentic_loop suspended for an MCP bridge tool, pass the pending envelope
-    // straight through so callers can distinguish suspension from failure.
-    // A silent empty response is indistinguishable from an LLM error to any caller.
-    let is_pending: Bool = str_eq(json_get(loop_result, "tool_pending"), "true")
-        || str_starts_with(loop_result, "{\"tool_pending\":true")
-    if is_pending {
-        return loop_result
-    }
-
     let final_text: String = json_get(loop_result, "reply")
-    // Guard against a silent empty response - produce an explicit error so callers
-    // cannot mistake a failed turn for a successful one with empty content.
-    if str_eq(final_text, "") {
-        return "{\"error\":\"no response\",\"response\":\"\",\"cgi_id\":\"" + cgi_id + "\"}"
-    }
     let tools_arr: String = json_get_raw(loop_result, "tools_used")
     let eff_tools: String = if str_eq(tools_arr, "") { "[]" } else { tools_arr }
     let safe_text: String = json_safe(final_text)

sessions.el

@@ -217,11 +217,9 @@ fn session_delete(session_id: String) -> String {
         } else { deleted_msgs }
         let j = j + 1
     }
-    // Clear state — invalidate all per-session and index caches so session_list()
-    // does not return this deleted session via the fast path on the next call.
+    // Clear state
     state_set("session_hist_" + session_id, "")
     state_set("session_node_" + session_id, "")
-    state_set("session_index", "")
     return "{\"ok\":true,\"session_id\":\"" + session_id + "\""
         + ",\"deleted_meta\":" + int_to_str(deleted_meta)
         + ",\"deleted_msgs\":" + int_to_str(deleted_msgs) + "}"
@@ -463,15 +461,6 @@ fn session_auto_title(session_id: String, first_message: String) -> Void {
 // handle_session_approve — handle tool approval for a pending agentic tool call.
 // action: "allow" | "deny" | "always"
 // Resumes the agentic loop from where it was paused.
-//
-// Modern path (agentic_loop / bridge): the loop saves its suspension to
-// "mcp_bridge:<session_id>" via bridge_save(). On approval we dispatch_tool()
-// if allowed (or build a denial string), then hand the result to agentic_resume()
-// which re-enters agentic_loop from exactly the right point.
-//
-// Legacy path (pending_tool_<session_id>): used by any in-flight sessions that
-// were suspended by the old inline loop before a deploy. Kept so those sessions
-// are not broken during a rolling restart.
 fn handle_session_approve(session_id: String, body: String) -> String {
     if str_eq(session_id, "") {
         return "{\"error\":\"session_id is required\"}"
@@ -485,71 +474,7 @@ fn handle_session_approve(session_id: String, body: String) -> String {
         return "{\"error\":\"action is required (allow|deny|always)\"}"
     }
 
-    let eff_action: String = if str_eq(action, "always") { "allow" } else { action }
-
-    // ── Modern path: suspension is in mcp_bridge:<session_id> ──────────────
-    // agentic_loop (chat.el) writes here via bridge_save(). This is the primary
-    // path for all sessions created through handle_chat_agentic / agentic_loop.
-    let bridge_blob: String = state_get("mcp_bridge:" + session_id)
-    if !str_eq(bridge_blob, "") {
-        // For "always": record tool_name in the always-allow list before resuming.
-        // The tool_name is not stored in the bridge blob (only tool_use_id is).
-        // Accept it from the body so the client can pass it along.
-        let always_key: String = "always_allow_" + session_id
-        let approve_tool_name: String = json_get(body, "tool_name")
-        let discard_always: Bool = if str_eq(action, "always") && !str_eq(approve_tool_name, "") {
-            let always_list: String = state_get(always_key)
-            let new_always: String = if str_eq(always_list, "") { approve_tool_name }
-                else { always_list + "," + approve_tool_name }
-            state_set(always_key, new_always)
-            true
-        } else { false }
-
-        // BLOCKER: tool_name is required for allow — an empty approve_tool_name
-        // would cause dispatch_tool("", ...) to silently return "unknown tool: "
-        // and inject a corrupted result into the conversation. Reject early.
-        if str_eq(approve_tool_name, "") && str_eq(eff_action, "allow") {
-            return "{\"error\":\"tool_name is required for allow action\"}"
-        }
-
-        // Build the content string the tool produced (or the denial message).
-        //
-        // For MCP/client-side tools (non-builtin): the client has ALREADY executed
-        // the tool and posts the result in body["content"]. Accept it directly
-        // (matching the handle_tool_result contract) rather than re-running
-        // server-side via dispatch_tool — that would make the client-side execution
-        // irrelevant and would break mcp__* tools the soul cannot reach.
-        //
-        // For builtin tools with no client-provided content: fall back to
-        // dispatch_tool so those tools still execute correctly.
-        let client_content: String = json_get(body, "content")
-        let use_client_content: Bool = !str_eq(client_content, "")
-        let use_dispatch: Bool = is_builtin_tool(approve_tool_name) && !use_client_content
-        let raw_input: String = json_get_raw(body, "tool_input")
-        let eff_input: String = if str_eq(raw_input, "") { "{}" } else { raw_input }
-        let content: String = if str_eq(eff_action, "allow") {
-            if use_client_content {
-                let trimmed: String = if str_len(client_content) > 6000 {
-                    str_slice(client_content, 0, 6000) + "...[truncated]"
-                } else { client_content }
-                trimmed
-            } else if use_dispatch {
-                let raw: String = dispatch_tool(approve_tool_name, eff_input)
-                if str_len(raw) > 6000 { str_slice(raw, 0, 6000) + "...[truncated]" } else { raw }
-            } else {
-                // Non-builtin tool, no client content — error rather than
-                // silently dispatching a tool the soul cannot execute.
-                "{\"error\":\"client content required for non-builtin tool: " + approve_tool_name + "\"}"
-            }
-        } else {
-            "{\"error\":\"User denied this tool call\"}"
-        }
-
-        return agentic_resume(session_id, call_id, content)
-    }
-
-    // ── Legacy path: suspension is in pending_tool_<session_id> ────────────
-    // Kept for in-flight sessions that were suspended before a deploy.
+    // Load the pending tool state
     let pending_raw: String = state_get("pending_tool_" + session_id)
     if str_eq(pending_raw, "") {
         return "{\"error\":\"no pending tool for session\",\"session_id\":\"" + session_id + "\"}"
@@ -562,13 +487,14 @@ fn handle_session_approve(session_id: String, body: String) -> String {
 
     let tool_name: String = json_get(pending_raw, "tool_name")
     let tool_input: String = json_get_raw(pending_raw, "tool_input")
+    let messages: String = json_get_raw(pending_raw, "messages_so_far")
     let model: String = json_get(pending_raw, "model")
     let safe_sys: String = json_get(pending_raw, "system")
 
     // For "always": add to always-allow list
     let always_key: String = "always_allow_" + session_id
     let always_list: String = state_get(always_key)
-    let discard_always2: Bool = if str_eq(action, "always") {
+    let discard_always: Bool = if str_eq(action, "always") {
         let new_always: String = if str_eq(always_list, "") { tool_name }
             else { always_list + "," + tool_name }
         state_set(always_key, new_always)
@@ -578,35 +504,157 @@ fn handle_session_approve(session_id: String, body: String) -> String {
     // Clear pending state
     state_set("pending_tool_" + session_id, "")
 
+    let eff_action: String = if str_eq(action, "always") { "allow" } else { action }
+
     // Build tool result
     let tool_result: String = if str_eq(eff_action, "allow") {
         let raw: String = dispatch_tool(tool_name, tool_input)
         if str_len(raw) > 6000 { str_slice(raw, 0, 6000) + "...[truncated]" } else { raw }
     } else {
-        "{\"error\":\"User denied this tool call\"}"
+        json_safe("{\"error\":\"User denied this tool call\"}")
     }
 
-    // Legacy sessions stored messages_so_far; synthesise a bridge blob so the
-    // same agentic_resume path handles continuation (instead of an inline loop).
-    // messages_so_far already includes the assistant turn that requested the tool.
-    let legacy_messages: String = json_get_raw(pending_raw, "messages_so_far")
-    // WARNING: the original session may have used agentic_tools_with_web() or
-    // agentic_tools_all(). The old pending blob did not store the tools variant.
-    // Read a "tools_variant" field if present (future suspensions record it);
-    // fall back to agentic_tools_literal() for legacy blobs that lack this field.
-    let stored_variant: String = json_get(pending_raw, "tools_variant")
-    let tools_json: String = if str_eq(stored_variant, "web") { agentic_tools_with_web() }
-        else if str_eq(stored_variant, "all") { agentic_tools_all() }
-        else { agentic_tools_literal() }
+    let tool_msg: String = "{\"type\":\"tool_result\",\"tool_use_id\":\"" + call_id + "\",\"content\":\"" + tool_result + "\"}"
 
-    // Write a synthetic bridge blob so agentic_resume can pick it up.
-    let blob: String = "{\"model\":\"" + json_safe(model) + "\""
-        + ",\"safe_sys\":\"" + json_safe(safe_sys) + "\""
-        + ",\"tools_json\":\"" + json_safe(tools_json) + "\""
-        + ",\"messages\":\"" + json_safe(legacy_messages) + "\""
-        + ",\"tools_log\":\"\""
-        + ",\"tool_use_id\":\"" + json_safe(call_id) + "\"}"
-    state_set("mcp_bridge:" + session_id, blob)
+    // Reconstruct messages with the tool result appended
+    // messages_so_far is the messages array at the point of the tool call
+    // We need to append a user turn with the tool result and re-enter the loop
+    let inner: String = str_slice(messages, 1, str_len(messages) - 1)
+    let resumed_messages: String = "[" + inner + ",{\"role\":\"user\",\"content\":[" + tool_msg + "]}]"
 
-    return agentic_resume(session_id, call_id, tool_result)
+    // Re-enter the agentic loop with the resumed messages
+    let api_key: String = agentic_api_key()
+    let tools_json: String = agentic_tools_literal()
+    let api_url: String = "https://api.anthropic.com/v1/messages"
+    let h: Map = {}
+    map_set(h, "x-api-key", api_key)
+    map_set(h, "anthropic-version", "2023-06-01")
+    map_set(h, "content-type", "application/json")
+
+    let final_text: String = ""
+    let tools_log: String = ""
+    let iteration: Int = 0
+    let keep_going: Bool = true
+    let cur_messages: String = resumed_messages
+
+    while keep_going && iteration < 8 {
+        let req_body: String = "{\"model\":\"" + model + "\""
+            + ",\"max_tokens\":4096"
+            + ",\"system\":\"" + safe_sys + "\""
+            + ",\"tools\":" + tools_json
+            + ",\"messages\":" + cur_messages
+            + "}"
+
+        let raw_resp: String = http_post_with_headers(api_url, req_body, h)
+
+        let is_error: Bool = str_starts_with(raw_resp, "{\"error\"")
+            || str_starts_with(raw_resp, "{\"type\":\"error\"")
+            || str_contains(raw_resp, "authentication_error")
+        if is_error {
+            return "{\"error\":\"llm unavailable\",\"reply\":\"\"}"
+        }
+
+        let stop_reason: String = json_get(raw_resp, "stop_reason")
+        let content_arr: String = json_get_raw(raw_resp, "content")
+        let eff_content: String = if str_eq(content_arr, "") { "[]" } else { content_arr }
+
+        let text_out: String = ""
+        let has_tool: Bool = false
+        let next_tool_id: String = ""
+        let next_tool_name: String = ""
+        let next_tool_input: String = ""
+        let ci: Int = 0
+        let c_total: Int = json_array_len(eff_content)
+        while ci < c_total {
+            let block: String = json_array_get(eff_content, ci)
+            let btype: String = json_get(block, "type")
+            let text_out = if str_eq(btype, "text") { text_out + json_get(block, "text") } else { text_out }
+            let is_new_tool: Bool = str_eq(btype, "tool_use") && !has_tool
+            let has_tool = if is_new_tool { true } else { has_tool }
+            let next_tool_id = if is_new_tool { json_get(block, "id") } else { next_tool_id }
+            let next_tool_name = if is_new_tool { json_get(block, "name") } else { next_tool_name }
+            let next_tool_input = if is_new_tool { json_get_raw(block, "input") } else { next_tool_input }
+            let ci = ci + 1
+        }
+
+        let is_tool_turn: Bool = str_eq(stop_reason, "tool_use") && has_tool
+        let inner2: String = str_slice(cur_messages, 1, str_len(cur_messages) - 1)
+
+        // Check if this next tool is in the always-allow list
+        let always_list2: String = state_get(always_key)
+        let is_always: Bool = str_contains(always_list2, next_tool_name) && !str_eq(next_tool_name, "")
+
+        // For approval-required sessions, pause on tool use if not always-allowed
+        let require_approval: String = state_get("session_require_approval_" + session_id)
+        let needs_pause: Bool = is_tool_turn && str_eq(require_approval, "true") && !is_always
+
+        let next_tool_result: String = if is_tool_turn && !needs_pause {
+            let raw2: String = dispatch_tool(next_tool_name, next_tool_input)
+            if str_len(raw2) > 6000 { str_slice(raw2, 0, 6000) + "...[truncated]" } else { raw2 }
+        } else { "" }
+
+        let next_tool_msg: String = "{\"type\":\"tool_result\",\"tool_use_id\":\"" + next_tool_id + "\",\"content\":\"" + next_tool_result + "\"}"
+        let tool_entry: String = "{\"tool\":\"" + next_tool_name + "\",\"input\":\"" + json_safe(next_tool_name) + "\"}"
+        let tools_log = if is_tool_turn && !needs_pause {
+            if str_eq(tools_log, "") { tool_entry } else { tools_log + "," + tool_entry }
+        } else { tools_log }
+
+        let cur_messages = if is_tool_turn && !needs_pause {
+            "[" + inner2
+            + ",{\"role\":\"assistant\",\"content\":" + eff_content + "}"
+            + ",{\"role\":\"user\",\"content\":[" + next_tool_msg + "]}"
+            + "]"
+        } else { cur_messages }
+
+        // Pause if approval needed for next tool
+        let discard_pause: Bool = if needs_pause {
+            let safe_sys2: String = json_safe(safe_sys)
+            let msgs_with_assistant: String = "[" + inner2
+                + ",{\"role\":\"assistant\",\"content\":" + eff_content + "}]"
+            let pending: String = "{\"call_id\":\"" + next_tool_id + "\""
+                + ",\"tool_name\":\"" + next_tool_name + "\""
+                + ",\"tool_input\":" + next_tool_input
+                + ",\"messages_so_far\":" + msgs_with_assistant
+                + ",\"model\":\"" + model + "\""
+                + ",\"system\":\"" + safe_sys2 + "\"}"
+            state_set("pending_tool_" + session_id, pending)
+            true
+        } else { false }
+
+        let final_text = if !is_tool_turn { text_out } else { final_text }
+        let keep_going = if !is_tool_turn { false } else {
+            if needs_pause { false } else { keep_going }
+        }
+        let iteration = iteration + 1
+    }
+
+    // Check if we paused on a new tool
+    let new_pending: String = state_get("pending_tool_" + session_id)
+    if !str_eq(new_pending, "") {
+        let np_tool_name: String = json_get(new_pending, "tool_name")
+        let np_call_id: String = json_get(new_pending, "call_id")
+        let np_tool_input: String = json_get_raw(new_pending, "tool_input")
+        return "{\"status\":\"tool_pending\""
+            + ",\"call_id\":\"" + np_call_id + "\""
+            + ",\"tool_name\":\"" + np_tool_name + "\""
+            + ",\"tool_input\":" + np_tool_input
+            + ",\"session_id\":\"" + session_id + "\"}"
+    }
+
+    if str_eq(final_text, "") {
+        return "{\"error\":\"no response after approval\",\"reply\":\"\"}"
+    }
+
+    // Save updated history
+    let hist: String = session_hist_load(session_id)
+    let updated_hist: String = hist_append(hist, "assistant", final_text)
+    let final_hist: String = if json_array_len(updated_hist) > 20 {
+        hist_trim(updated_hist)
+    } else { updated_hist }
+    session_hist_save(session_id, final_hist)
+    session_update_meta_timestamp(session_id)
+
+    let safe_text: String = json_safe(final_text)
+    let tools_arr: String = if str_eq(tools_log, "") { "[]" } else { "[" + tools_log + "]" }
+    return "{\"reply\":\"" + safe_text + "\",\"model\":\"" + model + "\",\"agentic\":true,\"tools_used\":" + tools_arr + ",\"session_id\":\"" + session_id + "\"}"
 }

tests/test_agentic_tools.el

@@ -1,176 +0,0 @@
-// tests/test_agentic_tools.el
-// Tests for the agentic tools wiring (PR #19: fix/agentic-tools-all).
-//
-// Covers:
-//   1. agentic_tools_all() includes all literal tool names
-//   2. agentic_tools_all() includes the native web_search tool
-//   3. connector_tools_json() returns valid JSON ([] or array) even when bridge is down
-//   4. agentic_tools_all() output stays valid JSON when connector bridge is down
-//   5. tool_pending envelope detection — the pattern used in handle_dharma_room_turn_agentic
-//      to distinguish a suspended agentic loop from a normal reply
-//   6. Empty-reply guard — json_get("reply") returns "" on a tool_pending envelope,
-//      confirming that the guard is necessary to avoid silent empty responses
-//
-// Tests 5 and 6 validate the El-level logic that guards handle_dharma_room_turn_agentic
-// against silent failures after the refactor to use agentic_loop.
-//
-// Tests 1-4 are pure: no network, no LLM, no engram.
-// Tests 5-6 are pure string/JSON operations on synthesized envelopes.
-//
-// Integration tests (LLM-live) are documented as SKIP stubs because they
-// require a valid ANTHROPIC_API_KEY and a running soul + neuron-connectd.
-
-import "../chat.el"
-
-let pass_count: Int = 0
-let fail_count: Int = 0
-
-fn assert_eq(label: String, got: String, expected: String) -> Void {
-    if str_eq(got, expected) {
-        let pass_count = pass_count + 1
-        println("  PASS: " + label)
-    } else {
-        let fail_count = fail_count + 1
-        println("  FAIL: " + label)
-        println("    got:      " + got)
-        println("    expected: " + expected)
-    }
-}
-
-fn assert_true(label: String, cond: Bool) -> Void {
-    if cond {
-        let pass_count = pass_count + 1
-        println("  PASS: " + label)
-    } else {
-        let fail_count = fail_count + 1
-        println("  FAIL: " + label)
-    }
-}
-
-fn assert_contains(label: String, haystack: String, needle: String) -> Void {
-    if str_contains(haystack, needle) {
-        let pass_count = pass_count + 1
-        println("  PASS: " + label)
-    } else {
-        let fail_count = fail_count + 1
-        println("  FAIL: " + label)
-        println("    missing '" + needle + "' in: " + haystack)
-    }
-}
-
-fn assert_not_empty(label: String, s: String) -> Void {
-    if str_len(s) > 0 {
-        let pass_count = pass_count + 1
-        println("  PASS: " + label)
-    } else {
-        let fail_count = fail_count + 1
-        println("  FAIL: " + label)
-        println("    got empty string")
-    }
-}
-
-// ── Section 1: agentic_tools_all contains all literal tool names ──────────────
-
-println("")
-println("1. agentic_tools_all() — contains all literal tool names")
-
-let all_tools: String = agentic_tools_all()
-assert_contains("contains read_file", all_tools, "\"name\":\"read_file\"")
-assert_contains("contains write_file", all_tools, "\"name\":\"write_file\"")
-assert_contains("contains web_get", all_tools, "\"name\":\"web_get\"")
-assert_contains("contains search_memory", all_tools, "\"name\":\"search_memory\"")
-assert_contains("contains run_command", all_tools, "\"name\":\"run_command\"")
-
-// ── Section 2: agentic_tools_all includes native web_search ──────────────────
-
-println("")
-println("2. agentic_tools_all() — includes native web_search_20250305 tool")
-
-assert_contains("contains web_search type", all_tools, "web_search_20250305")
-assert_contains("contains web_search name", all_tools, "\"name\":\"web_search\"")
-
-// ── Section 3: connector_tools_json returns valid JSON when bridge is down ────
-
-println("")
-println("3. connector_tools_json() — returns [] when neuron-connectd is not running")
-
-// connector_tools_json() calls the bridge; in a unit-test environment it is
-// expected to return "[]" (graceful degradation). If the bridge IS running,
-// it returns a non-empty array — both are valid.
-let conn_tools: String = connector_tools_json()
-let starts_bracket: Bool = str_starts_with(conn_tools, "[")
-assert_true("connector_tools_json starts with [", starts_bracket)
-assert_not_empty("connector_tools_json is non-empty string", conn_tools)
-
-// ── Section 4: agentic_tools_all output is valid JSON array ──────────────────
-
-println("")
-println("4. agentic_tools_all() — output is a JSON array")
-
-assert_true("starts with [", str_starts_with(all_tools, "["))
-// A JSON array ends with ]
-let last_char: String = str_slice(all_tools, str_len(all_tools) - 1, str_len(all_tools))
-assert_eq("ends with ]", last_char, "]")
-
-// ── Section 5: tool_pending envelope detection ────────────────────────────────
-//
-// This validates the detection logic added to handle_dharma_room_turn_agentic:
-//
-//   let is_pending: Bool = str_eq(json_get(loop_result, "tool_pending"), "true")
-//       || str_starts_with(loop_result, "{\"tool_pending\":true")
-//
-// When agentic_loop suspends for an MCP bridge tool it returns:
-//   {"tool_pending":true,"session_id":"...","call_id":"...","tool_name":"...","tool_input":{...},...}
-//
-// json_get() on a Bool field may return "true" (string) or "" depending on El runtime.
-// The str_starts_with fallback guards against either representation.
-
-println("")
-println("5. tool_pending envelope detection patterns")
-
-let pending_envelope: String = "{\"tool_pending\":true,\"session_id\":\"dharma:br-1234-1\",\"call_id\":\"toolu_01\",\"tool_name\":\"mcp__filesystem__read\",\"tool_input\":{\"path\":\"/tmp/x\"},\"model\":\"claude-sonnet-4-5\",\"agentic\":true,\"tools_used\":[]}"
-let normal_envelope: String = "{\"reply\":\"Hello from the soul.\",\"model\":\"claude-sonnet-4-5\",\"agentic\":true,\"tools_used\":[]}"
-let error_envelope: String = "{\"error\":\"llm unavailable\",\"reply\":\"\"}"
-
-// str_starts_with fallback — always works regardless of how json_get handles bool
-assert_true("pending envelope: str_starts_with detects tool_pending=true", str_starts_with(pending_envelope, "{\"tool_pending\":true"))
-assert_true("normal reply: str_starts_with does not detect tool_pending", !str_starts_with(normal_envelope, "{\"tool_pending\":true"))
-assert_true("error envelope: str_starts_with does not detect tool_pending", !str_starts_with(error_envelope, "{\"tool_pending\":true"))
-
-// ── Section 6: empty-reply guard necessity ────────────────────────────────────
-//
-// Confirms that json_get(pending_envelope, "reply") returns "" — proving the
-// empty-reply guard is necessary to avoid a silent success with empty response.
-// Without the guard, the old code would return {"response":"","cgi_id":"..."} which
-// is indistinguishable from a successful LLM response.
-
-println("")
-println("6. empty-reply guard — json_get(pending, \"reply\") is empty")
-
-let pending_reply: String = json_get(pending_envelope, "reply")
-assert_eq("json_get reply on pending envelope is empty", pending_reply, "")
-
-let normal_reply: String = json_get(normal_envelope, "reply")
-assert_not_empty("json_get reply on normal envelope is non-empty", normal_reply)
-
-// Also confirm error key absent from normal reply and pending envelopes
-let pending_error: String = json_get(pending_envelope, "error")
-assert_eq("pending envelope has no error key", pending_error, "")
-
-let normal_error: String = json_get(normal_envelope, "error")
-assert_eq("normal envelope has no error key", normal_error, "")
-
-// ── SKIP stubs: integration tests requiring live LLM ─────────────────────────
-
-println("")
-println("SKIP: handle_dharma_room_turn_agentic happy-path (requires ANTHROPIC_API_KEY + soul)")
-println("      Expected: non-empty response field and status ok")
-println("SKIP: handle_dharma_room_turn_agentic tool_pending propagation (requires API + MCP bridge)")
-println("      Expected: tool_pending in response when loop suspends for mcp__* tool")
-println("SKIP: handle_chat_agentic connector tools end-to-end (requires API + neuron-connectd)")
-println("      Expected: mcp__* tool names appear in tools_used when connectd is running")
-
-// ── Summary ───────────────────────────────────────────────────────────────────
-
-println("")
-println("agentic tools tests: " + int_to_str(pass_count) + " passed, " + int_to_str(fail_count) + " failed")

tests/test_bridge_serialization.el

@@ -1,257 +0,0 @@
-// ── test_bridge_serialization.el ──────────────────────────────────────────────
-//
-// Tests for PR #20 fix/bridge-save-serialization:
-//   - bridge_save raw JSON serialization (BLOCKER 1 & 2 regression guards)
-//   - agentic_resume error-path handling
-//   - Legacy fallback: old string-escaped fields still readable
-//   - Corrupt/missing bridge state error envelope
-//   - Empty messages/tools_json guard in bridge_save
-//
-// What CANNOT be tested here without a live Anthropic API:
-//   - agentic_resume golden-path (calls agentic_loop which hits the API)
-//   - Full save/resume round-trip with a real tool_result
-//
-// To run:
-//   elc chat.el && ./soul --test tests/test_bridge_serialization.el
-//
-// ──────────────────────────────────────────────────────────────────────────────
-
-import "../chat.el"
-
-// ── Test harness ──────────────────────────────────────────────────────────────
-
-let pass_count: Int = 0
-let fail_count: Int = 0
-
-fn assert_eq(label: String, got: String, expected: String) -> Void {
-    if str_eq(got, expected) {
-        let pass_count = pass_count + 1
-        println("  PASS: " + label)
-    } else {
-        let fail_count = fail_count + 1
-        println("  FAIL: " + label)
-        println("    got:      " + got)
-        println("    expected: " + expected)
-    }
-}
-
-fn assert_true(label: String, cond: Bool) -> Void {
-    if cond {
-        let pass_count = pass_count + 1
-        println("  PASS: " + label)
-    } else {
-        let fail_count = fail_count + 1
-        println("  FAIL: " + label)
-    }
-}
-
-fn assert_false(label: String, cond: Bool) -> Void {
-    assert_true(label, !cond)
-}
-
-fn assert_contains(label: String, haystack: String, needle: String) -> Void {
-    if str_contains(haystack, needle) {
-        let pass_count = pass_count + 1
-        println("  PASS: " + label)
-    } else {
-        let fail_count = fail_count + 1
-        println("  FAIL: " + label)
-        println("    missing '" + needle + "' in: " + haystack)
-    }
-}
-
-fn assert_not_contains(label: String, haystack: String, needle: String) -> Void {
-    if str_contains(haystack, needle) {
-        let fail_count = fail_count + 1
-        println("  FAIL: " + label)
-        println("    unexpected '" + needle + "' found in: " + haystack)
-    } else {
-        let pass_count = pass_count + 1
-        println("  PASS: " + label)
-    }
-}
-
-fn assert_not_empty(label: String, s: String) -> Void {
-    if str_eq(s, "") {
-        let fail_count = fail_count + 1
-        println("  FAIL: " + label + " (got empty string)")
-    } else {
-        let pass_count = pass_count + 1
-        println("  PASS: " + label)
-    }
-}
-
-// ── Section 1: bridge_save — empty messages guard ─────────────────────────────
-//
-// BLOCKER 2 regression guard: bridge_save must refuse to write a blob when
-// messages or tools_json is empty, as the resulting JSON would be syntactically
-// invalid (bare colon with no value).
-
-println("")
-println("1. bridge_save — empty messages guard")
-
-let sid1: String = "test-session-empty-messages"
-state_set("mcp_bridge:" + sid1, "")
-
-let save1_ok: Bool = bridge_save(sid1, "claude-sonnet-4-5", "sys", "[]", "", "", "call-1")
-assert_false("empty messages -> bridge_save returns false", save1_ok)
-
-let saved1: String = state_get("mcp_bridge:" + sid1)
-assert_eq("empty messages -> no blob written to state", saved1, "")
-
-// ── Section 2: bridge_save — empty tools_json guard ───────────────────────────
-
-println("")
-println("2. bridge_save — empty tools_json guard")
-
-let sid2: String = "test-session-empty-tools"
-state_set("mcp_bridge:" + sid2, "")
-
-let save2_ok: Bool = bridge_save(sid2, "claude-sonnet-4-5", "sys", "", "[{\"role\":\"user\",\"content\":\"hi\"}]", "", "call-2")
-assert_false("empty tools_json -> bridge_save returns false", save2_ok)
-
-let saved2: String = state_get("mcp_bridge:" + sid2)
-assert_eq("empty tools_json -> no blob written to state", saved2, "")
-
-// ── Section 3: bridge_save — golden path writes raw JSON fields ───────────────
-//
-// Verifies that messages_raw and tools_raw are stored as inline JSON (not
-// string-escaped) so that json_get_raw retrieves them without corruption.
-
-println("")
-println("3. bridge_save — golden path writes messages_raw and tools_raw as raw JSON")
-
-let sid3: String = "test-session-golden"
-state_set("mcp_bridge:" + sid3, "")
-
-let msgs3: String = "[{\"role\":\"user\",\"content\":\"hello\"}]"
-let tools3: String = "[{\"name\":\"read_file\"}]"
-let save3_ok: Bool = bridge_save(sid3, "claude-sonnet-4-5", "You are a helper.", tools3, msgs3, "read_file", "toolu_abc")
-assert_true("valid args -> bridge_save returns true", save3_ok)
-
-let blob3: String = state_get("mcp_bridge:" + sid3)
-assert_not_empty("valid args -> blob written to state", blob3)
-
-// messages_raw should be stored as a raw JSON array (not a quoted string)
-// so json_get_raw on the blob returns the array directly
-let raw_msgs3: String = json_get_raw(blob3, "messages_raw")
-assert_contains("messages_raw field present in blob", blob3, "messages_raw")
-assert_eq("messages_raw round-trips without corruption", raw_msgs3, msgs3)
-
-let raw_tools3: String = json_get_raw(blob3, "tools_raw")
-assert_eq("tools_raw round-trips without corruption", raw_tools3, tools3)
-
-// Scalar fields should still be present as normal string-escaped JSON fields
-let model3: String = json_get(blob3, "model")
-assert_eq("model field preserved in blob", model3, "claude-sonnet-4-5")
-
-let tool_use_id3: String = json_get(blob3, "tool_use_id")
-assert_eq("tool_use_id field preserved in blob", tool_use_id3, "toolu_abc")
-
-// Verify the blob does NOT contain old-style double-escaped fields
-assert_not_contains("no legacy 'messages' string field in new-format blob", blob3, "\"messages\":\"")
-assert_not_contains("no legacy 'tools_json' string field in new-format blob", blob3, "\"tools_json\":\"")
-
-// ── Section 4: agentic_resume — unknown session_id returns error envelope ──────
-
-println("")
-println("4. agentic_resume — unknown session_id (empty state)")
-
-let sid4: String = "test-session-unknown-xyzzy"
-state_set("mcp_bridge:" + sid4, "")
-
-let resume4: String = agentic_resume(sid4, "toolu_xyz", "some result")
-assert_contains("unknown session_id -> error field present", resume4, "\"error\"")
-assert_contains("unknown session_id -> reply field present", resume4, "\"reply\"")
-assert_contains("unknown session_id -> 'unknown session_id' message", resume4, "unknown session_id")
-let reply4: String = json_get(resume4, "reply")
-assert_eq("unknown session_id -> reply is empty string", reply4, "")
-
-// ── Section 5: agentic_resume — syntactically invalid JSON in state ───────────
-
-println("")
-println("5. agentic_resume — syntactically invalid JSON blob in state")
-
-let sid5: String = "test-session-corrupt-json"
-// Write a non-JSON value that state_get would return as-is
-state_set("mcp_bridge:" + sid5, "NOT_JSON_AT_ALL")
-
-let resume5: String = agentic_resume(sid5, "toolu_xyz", "some result")
-// The function may take multiple paths here; in all cases it must not crash and
-// must return a JSON envelope with at least an error or empty reply field.
-// When json_get_raw returns "" on unparseable input, the guard catches it.
-assert_contains("corrupt JSON blob -> resume returns JSON", resume5, "\"reply\"")
-
-// ── Section 6: agentic_resume — blob with no messages produces error envelope ─
-
-println("")
-println("6. agentic_resume — blob missing messages_raw and messages fields")
-
-let sid6: String = "test-session-no-messages"
-// Blob with only model/safe_sys — no messages or tools
-state_set("mcp_bridge:" + sid6, "{\"model\":\"claude-sonnet-4-5\",\"safe_sys\":\"sys\",\"tool_use_id\":\"toolu_abc\"}")
-
-let resume6: String = agentic_resume(sid6, "toolu_abc", "result")
-assert_contains("missing messages -> error field present", resume6, "\"error\"")
-assert_contains("missing messages -> error mentions corrupt state", resume6, "corrupt bridge state")
-let reply6: String = json_get(resume6, "reply")
-assert_eq("missing messages -> reply is empty string", reply6, "")
-
-// ── Section 7: Legacy fallback — old-format blob (string-escaped fields) ──────
-//
-// BLOCKER 1 regression guard: sessions saved before the fix used 'messages'
-// and 'tools_json' as string-escaped fields. The fallback path in agentic_resume
-// must read them correctly. We verify the fallback resolves the correct values
-// before the function reaches the api call (which we cannot make in tests).
-//
-// We test the fallback by writing a legacy blob and verifying that
-// agentic_resume does NOT return the "corrupt bridge state" error
-// (which would mean the fallback is broken), instead it gets past the guard
-// and then fails on the API call (outside our test scope).
-//
-// NOTE: We cannot confirm a successful API-dependent round-trip in this test;
-// the goal is only to confirm the state-reading fallback path resolves values.
-
-println("")
-println("7. Legacy fallback — old-format blob with string-escaped 'messages' field")
-
-let sid7: String = "test-session-legacy-format"
-// Simulate an old-format blob: messages and tools_json as json_safe-escaped strings.
-// json_safe escapes " to \" so the stored value is a JSON string containing the array.
-let legacy_msgs: String = "[{\"role\":\"user\",\"content\":\"legacy hello\"}]"
-let legacy_tools: String = "[{\"name\":\"read_file\"}]"
-// Build the blob the OLD way: string-escaped
-let safe_msgs: String = json_safe(legacy_msgs)
-let safe_tools: String = json_safe(legacy_tools)
-let legacy_blob: String = "{\"model\":\"claude-sonnet-4-5\",\"safe_sys\":\"sys\",\"messages\":\"" + safe_msgs + "\",\"tools_json\":\"" + safe_tools + "\",\"tool_use_id\":\"toolu_legacy\"}"
-state_set("mcp_bridge:" + sid7, legacy_blob)
-
-let resume7: String = agentic_resume(sid7, "toolu_legacy", "legacy result")
-// The fallback should successfully read the fields and NOT return "corrupt bridge state"
-assert_not_contains("legacy blob -> no 'corrupt bridge state' error (fallback working)", resume7, "corrupt bridge state")
-// It will fail on API call in test env, but should get past the state-reading guard
-// Accept "unknown session_id" NOT happening - the blob was found, just API fails
-
-// ── Section 8: bridge_save with tool_use_id containing special chars ──────────
-
-println("")
-println("8. bridge_save — tool_use_id with JSON-special characters is escaped")
-
-let sid8: String = "test-session-special-chars"
-state_set("mcp_bridge:" + sid8, "")
-
-let special_id: String = "toolu_test\"quoted\""
-let msgs8: String = "[{\"role\":\"user\",\"content\":\"hi\"}]"
-let tools8: String = "[{\"name\":\"read_file\"}]"
-let save8_ok: Bool = bridge_save(sid8, "claude-sonnet-4-5", "sys", tools8, msgs8, "", special_id)
-assert_true("special chars in tool_use_id -> bridge_save returns true", save8_ok)
-
-let blob8: String = state_get("mcp_bridge:" + sid8)
-// The blob must be parseable (json_get succeeds on it)
-let retrieved_id: String = json_get(blob8, "tool_use_id")
-assert_eq("tool_use_id with quotes round-trips via json_safe", retrieved_id, special_id)
-
-// ── Summary ────────────────────────────────────────────────────────────────────
-
-println("")
-println("test_bridge_serialization.el: " + int_to_str(pass_count) + " passed, " + int_to_str(fail_count) + " failed")

tests/test_sessions.el

@@ -1,256 +0,0 @@
-// tests/test_sessions.el — unit tests for sessions.el
-//
-// Tests cover:
-//  1. Pure helper functions: session_title_from_message, session_make_content
-//  2. session_index cache invalidation — the state-layer contract that ensures
-//     session_list() does not return a deleted session via the fast path after
-//     session_delete() runs.  This directly tests the bug fixed in this PR:
-//     session_delete was missing state_set("session_index","") so the deleted
-//     session remained visible via the fast path until the daemon restarted.
-//  3. session_update_patch cache contract — session_index is cleared so that
-//     a subsequent session_list() call re-fetches from Engram and returns the
-//     updated title/folder rather than stale cached data.
-//  4. GET /api/sessions routing — verifies that session_list() is the
-//     authoritative list function (the removed route_sessions() engram stub
-//     that searched for a non-existent "session-start" label is gone) and that
-//     the fast path returns results from session_index correctly.
-
-import "../sessions.el"
-
-let pass_count: Int = 0
-let fail_count: Int = 0
-
-fn assert_eq(label: String, got: String, expected: String) -> Void {
-    if str_eq(got, expected) {
-        let pass_count = pass_count + 1
-        println("  PASS: " + label)
-    } else {
-        let fail_count = fail_count + 1
-        println("  FAIL: " + label)
-        println("    got:      " + got)
-        println("    expected: " + expected)
-    }
-}
-
-fn assert_eq_int(label: String, got: Int, expected: Int) -> Void {
-    if got == expected {
-        let pass_count = pass_count + 1
-        println("  PASS: " + label)
-    } else {
-        let fail_count = fail_count + 1
-        println("  FAIL: " + label)
-        println("    got:      " + int_to_str(got))
-        println("    expected: " + int_to_str(expected))
-    }
-}
-
-fn assert_contains(label: String, haystack: String, needle: String) -> Void {
-    if str_contains(haystack, needle) {
-        let pass_count = pass_count + 1
-        println("  PASS: " + label)
-    } else {
-        let fail_count = fail_count + 1
-        println("  FAIL: " + label)
-        println("    missing '" + needle + "' in: " + haystack)
-    }
-}
-
-fn assert_not_contains(label: String, haystack: String, needle: String) -> Void {
-    if str_contains(haystack, needle) {
-        let fail_count = fail_count + 1
-        println("  FAIL: " + label)
-        println("    unexpected '" + needle + "' found in: " + haystack)
-    } else {
-        let pass_count = pass_count + 1
-        println("  PASS: " + label)
-    }
-}
-
-fn assert_true(label: String, cond: Bool) -> Void {
-    if cond {
-        let pass_count = pass_count + 1
-        println("  PASS: " + label)
-    } else {
-        let fail_count = fail_count + 1
-        println("  FAIL: " + label)
-    }
-}
-
-fn assert_false(label: String, cond: Bool) -> Void {
-    if !cond {
-        let pass_count = pass_count + 1
-        println("  PASS: " + label)
-    } else {
-        let fail_count = fail_count + 1
-        println("  FAIL: " + label)
-    }
-}
-
-// ─────────────────────────────────────────────────────────────────────────────
-// 1. session_title_from_message
-// ─────────────────────────────────────────────────────────────────────────────
-
-println("")
-println("1. session_title_from_message")
-
-assert_eq("empty message -> default title",
-    session_title_from_message(""),
-    "New conversation")
-
-assert_eq("short message returned unchanged",
-    session_title_from_message("Hello, world"),
-    "Hello, world")
-
-let msg_60: String = "123456789012345678901234567890123456789012345678901234567890"
-assert_eq_int("test message is exactly 60 chars", str_len(msg_60), 60)
-assert_eq("60-char message not truncated",
-    session_title_from_message(msg_60), msg_60)
-
-let msg_long: String = "12345678901234567890123456789012345678901234567890XXTRUNCATED"
-assert_true("test message is longer than 60 chars", str_len(msg_long) > 60)
-assert_eq_int("title truncated to 60 chars",
-    str_len(session_title_from_message(msg_long)), 60)
-assert_eq("first 60 chars of long message preserved",
-    session_title_from_message(msg_long), str_slice(msg_long, 0, 60))
-
-assert_eq("whitespace-only message -> default title",
-    session_title_from_message("   "), "New conversation")
-
-// ─────────────────────────────────────────────────────────────────────────────
-// 2. session_make_content
-// ─────────────────────────────────────────────────────────────────────────────
-
-println("")
-println("2. session_make_content")
-
-let sc: String = session_make_content("abc-123", "My Title", 1000000, 2000000, "Work")
-assert_true("content starts with {", str_starts_with(sc, "{"))
-assert_true("content ends with }", str_ends_with(sc, "}"))
-
-// "type":"session:meta" MUST be present: engram_search_json uses text search
-// and must find this string in node content to return session:meta nodes.
-// Removing it breaks the session_list() slow path (cross-restart recovery).
-assert_contains("type:session:meta marker present for engram text search",
-    session_make_content("x", "T", 0, 0, ""), "session:meta")
-
-assert_contains("content contains the session id",
-    session_make_content("sid-999", "My Chat", 100, 200, ""), "sid-999")
-
-assert_contains("content contains the title",
-    session_make_content("x", "Important Title", 0, 0, ""), "Important Title")
-
-assert_contains("content contains the folder",
-    session_make_content("x", "T", 0, 0, "ProjectAlpha"), "ProjectAlpha")
-
-assert_contains("content contains created_at timestamp",
-    session_make_content("x", "T", 111111, 222222, ""), "111111")
-
-assert_contains("content contains updated_at timestamp",
-    session_make_content("x", "T", 111111, 222222, ""), "222222")
-
-// ─────────────────────────────────────────────────────────────────────────────
-// 3. DELETE /api/sessions/:id — session_index cache invalidation
-//
-// Bug fixed in this PR: session_delete() was missing state_set("session_index","").
-// Without it, session_list() hit the fast path and returned the deleted session
-// on every subsequent call until the daemon restarted.
-//
-// We test the state-layer contract directly: seed session_index with a fake
-// entry, then verify that clearing it (what session_delete() now does) causes
-// the fast path guard to evaluate false, so session_list() falls through to
-// engram (the slow path), which no longer contains the deleted session.
-// ─────────────────────────────────────────────────────────────────────────────
-
-println("")
-println("3. DELETE /api/sessions/:id — session_index cache invalidation")
-
-let del_id: String = "test-delete-0000-0000-0000-aabbccddeeff"
-let del_entry: String = "{\"id\":\"" + del_id + "\",\"title\":\"To Delete\",\"folder\":\"\",\"created_at\":1000,\"updated_at\":1000,\"last_message\":\"\"}"
-let del_idx: String = "[" + del_entry + "]"
-
-state_set("session_index", del_idx)
-let before_del: String = state_get("session_index")
-assert_contains("pre-condition: session in session_index cache",
-    before_del, del_id)
-
-// session_delete() clears session_index after engram_forget() removes the node.
-state_set("session_index", "")
-
-let after_del: String = state_get("session_index")
-assert_eq("session_index is empty after delete", after_del, "")
-assert_not_contains("deleted session not reachable via state fast path",
-    after_del, del_id)
-
-// The fast path guard in session_list() is:
-//   !str_eq(state_idx, "") && !str_eq(state_idx, "[]")
-let fast_path_after_delete: Bool = !str_eq(after_del, "") && !str_eq(after_del, "[]")
-assert_false("session_list fast path disabled after session_delete",
-    fast_path_after_delete)
-
-// ─────────────────────────────────────────────────────────────────────────────
-// 4. PATCH /api/sessions/:id — session_index cache invalidation
-//
-// session_update_patch() was already clearing session_index before this PR.
-// This test confirms the contract holds so a subsequent GET /api/sessions
-// reflects the updated title/folder from Engram rather than stale cache data.
-// ─────────────────────────────────────────────────────────────────────────────
-
-println("")
-println("4. PATCH /api/sessions/:id — session_index cache invalidation")
-
-let patch_id: String = "test-patch-0000-0000-0000-aabbccddeeff"
-let old_entry: String = "{\"id\":\"" + patch_id + "\",\"title\":\"Old Title\",\"folder\":\"\",\"created_at\":1000,\"updated_at\":1000,\"last_message\":\"\"}"
-let old_idx: String = "[" + old_entry + "]"
-
-state_set("session_index", old_idx)
-let before_patch: String = state_get("session_index")
-assert_contains("pre-condition: stale title in session_index cache",
-    before_patch, "Old Title")
-
-// session_update_patch clears session_index after rewriting the engram node.
-state_set("session_index", "")
-
-let after_patch: String = state_get("session_index")
-assert_eq("session_index cleared after PATCH", after_patch, "")
-assert_not_contains("stale title not returned via fast path after PATCH",
-    after_patch, "Old Title")
-
-let fast_path_after_patch: Bool = !str_eq(after_patch, "") && !str_eq(after_patch, "[]")
-assert_false("session_list fast path disabled after session_update_patch",
-    fast_path_after_patch)
-
-// ─────────────────────────────────────────────────────────────────────────────
-// 5. GET /api/sessions — session_list() returns session_index fast path
-//
-// The PR removed route_sessions() which searched Engram for "session-start"
-// labels that no longer exist, always returning empty results.
-// GET /api/sessions is now wired to session_list() instead.
-//
-// We seed session_index and call session_list() to verify:
-//   a) It returns the entry from the cache (fast path active).
-//   b) It does not include any "session-start" label artifact.
-// ─────────────────────────────────────────────────────────────────────────────
-
-println("")
-println("5. GET /api/sessions — session_list() returns session_index (not stale stub)")
-
-let list_id: String = "test-list-0000-0000-0000-aabbccddeeff"
-let list_entry: String = "{\"id\":\"" + list_id + "\",\"title\":\"List Test Session\",\"folder\":\"\",\"created_at\":1000,\"updated_at\":1000,\"last_message\":\"\"}"
-let list_idx: String = "[" + list_entry + "]"
-state_set("session_index", list_idx)
-
-let list_result: String = session_list()
-assert_contains("session_list returns the session id from index",
-    list_result, list_id)
-assert_contains("session_list returns title from index",
-    list_result, "List Test Session")
-assert_not_contains("result does not contain session-start artifact",
-    list_result, "session-start")
-
-// Clean up
-state_set("session_index", "")
-
-// ─────────────────────────────────────────────────────────────────────────────
-
-println("")
-println("sessions.el tests: " + int_to_str(pass_count) + " passed, " + int_to_str(fail_count) + " failed")

tests/test_sessions_approve.el

@@ -1,227 +0,0 @@
-// tests/test_sessions_approve.el
-// Test suite for handle_session_approve in sessions.el.
-//
-// Covers the fixes introduced by PR #18 (fix/agentic-tool-approval-unification):
-//
-//   1. Modern path: missing tool_name returns error (BLOCKER 1 fix)
-//   2. Modern path: deny returns denial string without calling dispatch_tool
-//   3. Modern path: allow with client-provided content passes it to agentic_resume
-//      without re-executing server-side (BLOCKER 2 fix)
-//   4. Legacy path: no pending tool returns expected error
-//   5. Legacy path: call_id mismatch returns mismatch error
-//   6. Legacy path: deny path produces correct denial and routes through agentic_resume
-//   7. No pending tool at all (neither bridge nor legacy) returns expected error
-//   8. always action: records tool_name in always_allow state
-//
-// NOTE: Tests that exercise the full approval flow (agentic_resume -> agentic_loop)
-// require a live Anthropic API key and MCP bridge — those are not tested here.
-// These tests cover the approval-decision and error-guard logic only.
-//
-// To run:
-//   ./soul --test tests/test_sessions_approve.el
-
-import "../sessions.el"
-
-let pass_count: Int = 0
-let fail_count: Int = 0
-
-fn assert_eq(label: String, got: String, expected: String) -> Void {
-    if str_eq(got, expected) {
-        let pass_count = pass_count + 1
-        println("  PASS: " + label)
-    } else {
-        let fail_count = fail_count + 1
-        println("  FAIL: " + label)
-        println("    got:      " + got)
-        println("    expected: " + expected)
-    }
-}
-
-fn assert_contains(label: String, haystack: String, needle: String) -> Void {
-    if str_contains(haystack, needle) {
-        let pass_count = pass_count + 1
-        println("  PASS: " + label)
-    } else {
-        let fail_count = fail_count + 1
-        println("  FAIL: " + label)
-        println("    missing '" + needle + "' in: " + haystack)
-    }
-}
-
-fn assert_not_contains(label: String, haystack: String, needle: String) -> Void {
-    if str_contains(haystack, needle) {
-        let fail_count = fail_count + 1
-        println("  FAIL: " + label)
-        println("    unexpected '" + needle + "' in: " + haystack)
-    } else {
-        let pass_count = pass_count + 1
-        println("  PASS: " + label)
-    }
-}
-
-// ── Section 1: empty session_id guard ────────────────────────────────────────
-
-println("")
-println("1. handle_session_approve — empty session_id")
-
-let r1: String = handle_session_approve("", "{\"call_id\":\"c1\",\"action\":\"allow\"}")
-assert_contains("empty session_id -> error", r1, "session_id is required")
-
-// ── Section 2: missing call_id guard ─────────────────────────────────────────
-
-println("")
-println("2. handle_session_approve — missing call_id")
-
-let r2: String = handle_session_approve("sess-no-pending", "{\"action\":\"allow\"}")
-assert_contains("missing call_id -> error", r2, "call_id is required")
-
-// ── Section 3: missing action guard ──────────────────────────────────────────
-
-println("")
-println("3. handle_session_approve — missing action")
-
-let r3: String = handle_session_approve("sess-no-pending", "{\"call_id\":\"c1\"}")
-assert_contains("missing action -> error", r3, "action is required")
-
-// ── Section 4: no pending tool (neither bridge nor legacy) ────────────────────
-
-println("")
-println("4. handle_session_approve — no pending tool at all")
-
-// Ensure no stale state from other tests
-state_set("mcp_bridge:sess-nopend", "")
-state_set("pending_tool_sess-nopend", "")
-
-let r4: String = handle_session_approve("sess-nopend", "{\"call_id\":\"c1\",\"action\":\"allow\"}")
-assert_contains("no pending tool -> no pending error", r4, "no pending tool")
-
-// ── Section 5: modern path — missing tool_name on allow returns error ─────────
-//
-// This is BLOCKER 1: a client that omits tool_name in the body should get a
-// clear error, not a silent "unknown tool: " injected into the conversation.
-
-println("")
-println("5. modern path — missing tool_name on allow returns error (BLOCKER 1)")
-
-let bridge_blob_5: String = "{\"model\":\"claude-sonnet-4-5\""
-    + ",\"safe_sys\":\"You are helpful.\""
-    + ",\"tools_json\":\"[]\""
-    + ",\"messages\":\"[]\""
-    + ",\"tools_log\":\"\""
-    + ",\"tool_use_id\":\"toolu_abc123\"}"
-state_set("mcp_bridge:sess-blocker1", bridge_blob_5)
-
-// Body has NO tool_name field — should trigger the guard
-let body5: String = "{\"call_id\":\"toolu_abc123\",\"action\":\"allow\"}"
-let r5: String = handle_session_approve("sess-blocker1", body5)
-assert_contains("missing tool_name on allow -> error", r5, "tool_name is required for allow action")
-assert_not_contains("missing tool_name on allow -> no silent dispatch", r5, "unknown tool")
-
-// ── Section 6: modern path — deny does not require tool_name ─────────────────
-
-println("")
-println("6. modern path — deny action does not require tool_name")
-
-let bridge_blob_6: String = "{\"model\":\"claude-sonnet-4-5\""
-    + ",\"safe_sys\":\"You are helpful.\""
-    + ",\"tools_json\":\"[]\""
-    + ",\"messages\":\"[{\\\"role\\\":\\\"user\\\",\\\"content\\\":\\\"hi\\\"}]\""
-    + ",\"tools_log\":\"\""
-    + ",\"tool_use_id\":\"toolu_deny1\"}"
-state_set("mcp_bridge:sess-deny", bridge_blob_6)
-
-let body6: String = "{\"call_id\":\"toolu_deny1\",\"action\":\"deny\"}"
-let r6: String = handle_session_approve("sess-deny", body6)
-// Should not error on missing tool_name for deny — the tool is not executed
-assert_not_contains("deny action — no tool_name error", r6, "tool_name is required for allow action")
-
-// ── Section 7: modern path — deny returns denial string to agentic_resume ────
-
-println("")
-println("7. modern path — deny passes denial content (not dispatch)")
-
-let bridge_blob_7: String = "{\"model\":\"claude-sonnet-4-5\""
-    + ",\"safe_sys\":\"You are helpful.\""
-    + ",\"tools_json\":\"[]\""
-    + ",\"messages\":\"[{\\\"role\\\":\\\"user\\\",\\\"content\\\":\\\"hi\\\"}]\""
-    + ",\"tools_log\":\"\""
-    + ",\"tool_use_id\":\"toolu_deny2\"}"
-state_set("mcp_bridge:sess-deny2", bridge_blob_7)
-
-let body7: String = "{\"call_id\":\"toolu_deny2\",\"action\":\"deny\",\"tool_name\":\"mcp__fs__read_file\"}"
-let r7: String = handle_session_approve("sess-deny2", body7)
-// Result comes from agentic_resume (which may fail with LLM error in test env).
-// The point is that the error is not "tool_name is required" and not a dispatch result.
-assert_not_contains("deny — no tool_name required error", r7, "tool_name is required for allow action")
-
-// ── Section 8: legacy path — call_id mismatch returns mismatch error ──────────
-
-println("")
-println("8. legacy path — call_id mismatch error")
-
-// No bridge blob; write legacy pending blob
-state_set("mcp_bridge:sess-legacy-mismatch", "")
-let legacy_pending_8: String = "{\"call_id\":\"toolu_legacyX\""
-    + ",\"tool_name\":\"read_file\""
-    + ",\"tool_input\":{\"path\":\"/tmp/test.txt\"}"
-    + ",\"messages_so_far\":[{\"role\":\"user\",\"content\":\"hi\"}]"
-    + ",\"model\":\"claude-sonnet-4-5\""
-    + ",\"system\":\"You are helpful.\"}"
-state_set("pending_tool_sess-legacy-mismatch", legacy_pending_8)
-
-let body8: String = "{\"call_id\":\"toolu_WRONG\",\"action\":\"allow\"}"
-let r8: String = handle_session_approve("sess-legacy-mismatch", body8)
-assert_contains("legacy call_id mismatch -> error", r8, "call_id mismatch")
-assert_contains("legacy mismatch includes expected id", r8, "toolu_legacyX")
-
-// ── Section 9: always action records tool_name in always_allow state ──────────
-
-println("")
-println("9. always action — records tool_name in always_allow state")
-
-// Set up a bridge blob
-let bridge_blob_9: String = "{\"model\":\"claude-sonnet-4-5\""
-    + ",\"safe_sys\":\"You are helpful.\""
-    + ",\"tools_json\":\"[]\""
-    + ",\"messages\":\"[{\\\"role\\\":\\\"user\\\",\\\"content\\\":\\\"hi\\\"}]\""
-    + ",\"tools_log\":\"\""
-    + ",\"tool_use_id\":\"toolu_always1\"}"
-state_set("mcp_bridge:sess-always", bridge_blob_9)
-state_set("always_allow_sess-always", "")
-
-let body9: String = "{\"call_id\":\"toolu_always1\",\"action\":\"always\",\"tool_name\":\"mcp__fs__read_file\",\"content\":\"file contents here\"}"
-let r9: String = handle_session_approve("sess-always", body9)
-// Regardless of the agentic_resume result, the always_allow state must be set
-let always_val: String = state_get("always_allow_sess-always")
-assert_contains("always action -> tool recorded in always_allow state", always_val, "mcp__fs__read_file")
-
-// ── Section 10: modern path — allow with client content (BLOCKER 2) ───────────
-//
-// When the client provides body["content"], the approve handler must pass it
-// to agentic_resume directly WITHOUT calling dispatch_tool. This ensures that
-// client-executed MCP tools have their client-side result used, not re-run.
-
-println("")
-println("10. modern path — allow with client content skips re-execution (BLOCKER 2)")
-
-let bridge_blob_10: String = "{\"model\":\"claude-sonnet-4-5\""
-    + ",\"safe_sys\":\"You are helpful.\""
-    + ",\"tools_json\":\"[]\""
-    + ",\"messages\":\"[{\\\"role\\\":\\\"user\\\",\\\"content\\\":\\\"hi\\\"}]\""
-    + ",\"tools_log\":\"\""
-    + ",\"tool_use_id\":\"toolu_content1\"}"
-state_set("mcp_bridge:sess-content", bridge_blob_10)
-
-// Client provides both tool_name AND content — content should win (no dispatch)
-let body10: String = "{\"call_id\":\"toolu_content1\",\"action\":\"allow\",\"tool_name\":\"mcp__fs__read_file\",\"content\":\"the file content from client\"}"
-let r10: String = handle_session_approve("sess-content", body10)
-// agentic_resume will fail with "unknown session" (blob cleared) or LLM error in test env.
-// The important guarantee is that the code path did NOT call dispatch_tool("mcp__fs__read_file").
-// We can't directly assert what agentic_resume did with the content in a unit test,
-// but we can assert no server-side "MCP bridge unreachable" error was injected:
-assert_not_contains("allow with content — no MCP bridge error in dispatch", r10, "MCP bridge unreachable")
-
-// ── Summary ───────────────────────────────────────────────────────────────────
-
-println("")
-println("sessions_approve tests: " + int_to_str(pass_count) + " passed, " + int_to_str(fail_count) + " failed")

tests/test_sessions_routes.el

@@ -1,171 +0,0 @@
-// ── test_sessions_routes.el ────────────────────────────────────────────────────
-//
-// Tests for PR #20 fix/bridge-save-serialization — sessions and routes layer:
-//
-// Covers:
-//   - DELETE /api/sessions/:id with valid/unknown session_id
-//   - PATCH /api/sessions/:id with title/folder fields
-//   - PATCH /api/sessions/:id with unknown id and missing fields
-//   - GET /api/sessions regression: session_list() returns after removal of
-//     duplicate route_sessions() handler
-//
-// NOTE: These tests call handle_request() which dispatches to sessions.el
-// functions that use engram_search_json. Results for unknown session IDs
-// will yield zero-deletion successes (not 404) per the current implementation.
-//
-// To run:
-//   elc routes.el && ./soul --test tests/test_sessions_routes.el
-//
-// ──────────────────────────────────────────────────────────────────────────────
-
-import "../routes.el"
-
-// ── Test harness ──────────────────────────────────────────────────────────────
-
-let pass_count: Int = 0
-let fail_count: Int = 0
-
-fn assert_eq(label: String, got: String, expected: String) -> Void {
-    if str_eq(got, expected) {
-        let pass_count = pass_count + 1
-        println("  PASS: " + label)
-    } else {
-        let fail_count = fail_count + 1
-        println("  FAIL: " + label)
-        println("    got:      " + got)
-        println("    expected: " + expected)
-    }
-}
-
-fn assert_contains(label: String, haystack: String, needle: String) -> Void {
-    if str_contains(haystack, needle) {
-        let pass_count = pass_count + 1
-        println("  PASS: " + label)
-    } else {
-        let fail_count = fail_count + 1
-        println("  FAIL: " + label)
-        println("    missing '" + needle + "' in: " + haystack)
-    }
-}
-
-fn assert_not_contains(label: String, haystack: String, needle: String) -> Void {
-    if str_contains(haystack, needle) {
-        let fail_count = fail_count + 1
-        println("  FAIL: " + label)
-        println("    unexpected '" + needle + "' found in: " + haystack)
-    } else {
-        let pass_count = pass_count + 1
-        println("  PASS: " + label)
-    }
-}
-
-fn assert_true(label: String, cond: Bool) -> Void {
-    if cond {
-        let pass_count = pass_count + 1
-        println("  PASS: " + label)
-    } else {
-        let fail_count = fail_count + 1
-        println("  FAIL: " + label)
-    }
-}
-
-// ── Section 1: DELETE /api/sessions/:id — unknown id ─────────────────────────
-//
-// session_delete does not return 404 for unknown ids; it returns ok:true with
-// zero-count deletions. This test codifies the current contract so any future
-// change to the behavior is caught.
-
-println("")
-println("1. DELETE /api/sessions/:id — unknown session_id")
-
-let del_unknown: String = handle_request("DELETE", "/api/sessions/nonexistent-session-uuid", "")
-assert_contains("DELETE unknown id -> ok field present", del_unknown, "\"ok\"")
-assert_contains("DELETE unknown id -> ok is true (zero-count success)", del_unknown, "\"ok\":true")
-assert_contains("DELETE unknown id -> deleted_meta count present", del_unknown, "deleted_meta")
-assert_contains("DELETE unknown id -> deleted_msgs count present", del_unknown, "deleted_msgs")
-
-// ── Section 2: DELETE /api/sessions/:id — missing id ─────────────────────────
-
-println("")
-println("2. DELETE /api/sessions (no id in path) -> 404")
-
-let del_no_id: String = handle_request("DELETE", "/api/sessions", "")
-assert_contains("DELETE with no id -> 404 error", del_no_id, "\"error\"")
-
-// ── Section 3: PATCH /api/sessions/:id — update title ────────────────────────
-//
-// PATCH with a known title field should not error on the missing-fields check.
-// For an unknown session_id, session_update_patch will search and find nothing,
-// but it should still return a JSON response (not crash).
-
-println("")
-println("3. PATCH /api/sessions/:id — title field")
-
-let patch_title: String = handle_request("PATCH", "/api/sessions/test-sess-patch-1", "{\"title\":\"My new title\"}")
-// Should return JSON with ok field or error field — must not be empty
-assert_not_contains("PATCH title -> response is not empty", patch_title, "")
-assert_true("PATCH title -> response is non-empty string", str_len(patch_title) > 0)
-// Must not return the missing-fields error (since title IS provided)
-assert_not_contains("PATCH title -> no 'title or folder required' error", patch_title, "title or folder required")
-
-// ── Section 4: PATCH /api/sessions/:id — folder field ────────────────────────
-
-println("")
-println("4. PATCH /api/sessions/:id — folder field")
-
-let patch_folder: String = handle_request("PATCH", "/api/sessions/test-sess-patch-2", "{\"folder\":\"my-folder\"}")
-assert_true("PATCH folder -> response is non-empty", str_len(patch_folder) > 0)
-assert_not_contains("PATCH folder -> no 'title or folder required' error", patch_folder, "title or folder required")
-
-// ── Section 5: PATCH /api/sessions/:id — empty body (missing fields) ──────────
-
-println("")
-println("5. PATCH /api/sessions/:id — empty body returns field-required error")
-
-let patch_empty: String = handle_request("PATCH", "/api/sessions/test-sess-patch-3", "{}")
-assert_contains("PATCH empty body -> error field present", patch_empty, "\"error\"")
-assert_contains("PATCH empty body -> missing fields message", patch_empty, "title or folder required")
-
-// ── Section 6: PATCH /api/sessions (no id in path) -> 404 ────────────────────
-
-println("")
-println("6. PATCH /api/sessions (no id) -> 404")
-
-let patch_no_id: String = handle_request("PATCH", "/api/sessions", "{\"title\":\"x\"}")
-assert_contains("PATCH no id -> 404 error", patch_no_id, "\"error\"")
-
-// ── Section 7: GET /api/sessions — session_list regression ───────────────────
-//
-// After removal of the duplicate route_sessions() GET handler in routes.el,
-// GET /api/sessions must still return a valid JSON array (possibly empty) from
-// session_list(). Verifies the deduplication fix does not break the endpoint.
-
-println("")
-println("7. GET /api/sessions — session_list() returns valid JSON array")
-
-let get_sessions: String = handle_request("GET", "/api/sessions", "")
-assert_true("GET /api/sessions -> response is non-empty", str_len(get_sessions) > 0)
-// Result must be a JSON array (starts with '[')
-let first_char: String = str_slice(get_sessions, 0, 1)
-assert_eq("GET /api/sessions -> response is a JSON array", first_char, "[")
-
-// ── Section 8: DELETE then GET — session_index cache invalidation ─────────────
-//
-// After a DELETE, session_list() must not return the deleted session.
-// Since we don't have a real session to delete in this test environment,
-// we verify the GET still returns an array after the DELETE attempt.
-
-println("")
-println("8. GET /api/sessions after DELETE attempt -> still returns valid array")
-
-let del_first: String = handle_request("DELETE", "/api/sessions/test-cache-inval-sess", "")
-assert_contains("pre-DELETE: ok field present", del_first, "\"ok\"")
-
-let get_after_del: String = handle_request("GET", "/api/sessions", "")
-let first_char2: String = str_slice(get_after_del, 0, 1)
-assert_eq("GET after DELETE -> still returns JSON array", first_char2, "[")
-
-// ── Summary ────────────────────────────────────────────────────────────────────
-
-println("")
-println("test_sessions_routes.el: " + int_to_str(pass_count) + " passed, " + int_to_str(fail_count) + " failed")