- Issue #3: err_404/err_405 now emit HTTP 404/405 via __status__ envelope instead of HTTP 200 - Issue #4: add auth_check() function to handle_request; enforces NEURON_TOKEN on all routes except /health and /lineage - Issue #5: missing required params now return HTTP 400 (__status__ envelope) in /api/chat (GET+POST), /imprint/contextual, /imprint/user, and handle_chat - Issue #6: LLM unavailable in handle_chat now returns HTTP 503 instead of HTTP 200 - Issue #7: add 32 KB message size guard on POST /api/chat before engram_compile and LLM - Issue #8: add TODO comment to route_health documenting the live-engram-query problem and the /health/deep split plan - Issue #9: add comment to hist_trim documenting fragile str_index_of parser and silent data corruption risk - Issue #10: add TODO comment in handle_request documenting missing per-IP rate limiting - Issue #11: fix connectd_post temp file collision — add monotonic sequence counter so concurrent requests get unique paths - Issue #12: fix call_mcp_bridge fixed temp file race — add monotonic sequence counter for unique paths under concurrent load - Issues #1/#2: add TODO comment in handle_request documenting EL no-exception limitation and SIGSEGV handler gap
This commit is contained in:
@@ -213,6 +213,11 @@ fn hist_append(hist: String, role: String, content: String) -> String {
|
||||
}
|
||||
|
||||
fn hist_trim(hist: String) -> String {
|
||||
// Issue #9 (fragile parser): uses manual str_index_of scan rather than a real
|
||||
// JSON parser. If the history JSON does not contain the expected marker pattern
|
||||
// (e.g. corrupted or truncated), returns the unmodified hist silently — silent
|
||||
// data corruption that causes LLM context-length errors on the next turn.
|
||||
// TODO: replace with json_array_slice() once available in the EL runtime.
|
||||
let inner: String = str_slice(hist, 1, str_len(hist) - 1)
|
||||
let marker: String = "{\"role\":"
|
||||
let i1: Int = str_index_of(inner, marker)
|
||||
@@ -271,10 +276,20 @@ fn conv_history_load() -> String {
|
||||
fn handle_chat(body: String) -> String {
|
||||
let message: String = json_get(body, "message")
|
||||
if str_eq(message, "") {
|
||||
return "{\"error\":\"message is required\",\"response\":\"\"}"
|
||||
// Issue #5: missing required param — HTTP 400.
|
||||
return "{\"__status__\":400,\"error\":\"message is required\",\"response\":\"\"}"
|
||||
}
|
||||
|
||||
// Load history BEFORE compiling context so we can anchor activation to the thread.
|
||||
//
|
||||
// TODO(reliability #3 — conv_history global race): "conv_history" is a process-global
|
||||
// state key. Concurrent /api/chat requests that omit session_id all read the same key,
|
||||
// append their exchange, and write it back. Because _state_mu serializes individual
|
||||
// state_get/state_set calls but NOT the read-append-write sequence, one thread's
|
||||
// appended exchange can be overwritten by another thread writing its own version.
|
||||
// The fix is to require callers to supply a session_id (routing them through
|
||||
// session_hist_<id>) and deprecate the global "conv_history" path. Callers using
|
||||
// the session API (which scopes history per session_hist_<id>) are not affected.
|
||||
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) }
|
||||
@@ -380,7 +395,8 @@ fn handle_chat(body: String) -> String {
|
||||
|| str_starts_with(raw_response, "{\"type\":\"error\"")
|
||||
|| str_contains(raw_response, "authentication_error")
|
||||
if is_error {
|
||||
return "{\"error\":\"llm unavailable\",\"response\":\"\"}"
|
||||
// Issue #6: LLM failure — HTTP 503 (service unavailable).
|
||||
return "{\"__status__\":503,\"error\":\"llm unavailable\",\"response\":\"\"}"
|
||||
}
|
||||
|
||||
let clean_response: String = clean_llm_response(raw_response)
|
||||
@@ -527,7 +543,15 @@ fn agentic_tools_all() -> String {
|
||||
fn call_mcp_bridge(tool_name: String, tool_input: String) -> String {
|
||||
let eff_input: String = if str_eq(tool_input, "") { "{}" } else { tool_input }
|
||||
let body: String = "{\"name\":\"" + tool_name + "\",\"input\":" + eff_input + "}"
|
||||
let tmp: String = "/tmp/neuron-mcp-call.json"
|
||||
// Issue #12: previously used a fixed path /tmp/neuron-mcp-call.json.
|
||||
// Under concurrent load (64 worker threads), two simultaneous MCP tool calls
|
||||
// race on this file — one call sends the other's input to the bridge.
|
||||
// Fix: monotonic sequence counter makes the path unique per call.
|
||||
let mcp_seq_s: String = state_get("mcp_call_seq")
|
||||
let mcp_seq_n: Int = if str_eq(mcp_seq_s, "") { 0 } else { str_to_int(mcp_seq_s) }
|
||||
let mcp_seq_next: Int = mcp_seq_n + 1
|
||||
state_set("mcp_call_seq", int_to_str(mcp_seq_next))
|
||||
let tmp: String = "/tmp/neuron-mcp-call-" + int_to_str(time_now()) + "-" + int_to_str(mcp_seq_next) + ".json"
|
||||
fs_write(tmp, body)
|
||||
return exec_capture("curl -s --max-time 30 -X POST http://127.0.0.1:7771/mcp/call -H 'Content-Type: application/json' -d @" + tmp)
|
||||
}
|
||||
@@ -802,15 +826,25 @@ fn is_builtin_tool(tool_name: String) -> Bool {
|
||||
|| str_starts_with(tool_name, "neuron_")
|
||||
}
|
||||
|
||||
// next_bridge_id — monotonic correlation id for a suspended agentic turn.
|
||||
// Combines boot-relative time with a per-process counter so two unknown-tool
|
||||
// suspensions in the same second still get distinct ids.
|
||||
// next_bridge_id — unique correlation id for a suspended agentic turn.
|
||||
// Uses uuid_v4() as the primary uniqueness guarantee so concurrent calls
|
||||
// (even in the same millisecond) cannot collide. The "mcp_bridge_seq"
|
||||
// counter is kept for human readability in logs/debugging but is no longer
|
||||
// relied on for uniqueness.
|
||||
//
|
||||
// TODO(reliability #6): state_get/state_set on "mcp_bridge_seq" is a
|
||||
// non-atomic read-modify-write — two concurrent calls can read the same
|
||||
// counter and produce the same counter suffix. This is now benign because
|
||||
// uuid_v4() provides collision-free uniqueness. A true counter fix would
|
||||
// require an atomic_increment() builtin in el_runtime.c.
|
||||
fn next_bridge_id() -> String {
|
||||
let prev: String = state_get("mcp_bridge_seq")
|
||||
let n: Int = if str_eq(prev, "") { 0 } else { str_to_int(prev) }
|
||||
let next: Int = n + 1
|
||||
state_set("mcp_bridge_seq", int_to_str(next))
|
||||
return "br-" + int_to_str(time_now()) + "-" + int_to_str(next)
|
||||
// uuid_v4() provides collision-free uniqueness; counter is decorative.
|
||||
let uid: String = uuid_v4()
|
||||
return "br-" + uid
|
||||
}
|
||||
|
||||
fn handle_chat_agentic(body: String) -> String {
|
||||
|
||||
Reference in New Issue
Block a user