fix(chat): wire agentic_tools_all into both agentic loop entry points #19

2026-06-15T18:07:18Z

will.anderson commented

2026-06-15 18:07:18 +00:00

Problem

In handle_chat_agentic, tools_json was set to agentic_tools_with_web() which includes built-in tools and web_search but not MCP connector tools. Users in agentic mode could never call any mcp__* tool even when neuron-connectd is running.

handle_dharma_room_turn_agentic had the same problem (agentic_tools_literal()) plus an inline copy of the 8-iteration agentic loop that lacked bridge suspension support.

Changes

handle_chat_agentic (line 579): agentic_tools_with_web() → agentic_tools_all()
handle_dharma_room_turn_agentic: agentic_tools_literal() → agentic_tools_all(), inline loop replaced with agentic_loop() call, room_id extracted from body for dharma-scoped session IDs (dharma:<room_id>) so bridge suspension works per-room

agentic_tools_all() calls connector_tools_json() internally, which returns [] if neuron-connectd is not running, so this is safe when the bridge is down.

Test plan

Verify mcp__neuron__* tools are available in agentic chat when neuron-connectd is running
Verify agentic chat still works normally when neuron-connectd is not running (falls back to built-ins)
Verify dharma room agentic turns complete correctly and tools_used is populated in response

## Problem In `handle_chat_agentic`, `tools_json` was set to `agentic_tools_with_web()` which includes built-in tools and web_search but **not** MCP connector tools. Users in agentic mode could never call any `mcp__*` tool even when `neuron-connectd` is running. `handle_dharma_room_turn_agentic` had the same problem (`agentic_tools_literal()`) plus an inline copy of the 8-iteration agentic loop that lacked bridge suspension support. ## Changes - `handle_chat_agentic` (line 579): `agentic_tools_with_web()` → `agentic_tools_all()` - `handle_dharma_room_turn_agentic`: `agentic_tools_literal()` → `agentic_tools_all()`, inline loop replaced with `agentic_loop()` call, `room_id` extracted from body for dharma-scoped session IDs (`dharma:<room_id>`) so bridge suspension works per-room `agentic_tools_all()` calls `connector_tools_json()` internally, which returns `[]` if `neuron-connectd` is not running, so this is safe when the bridge is down. ## Test plan - [ ] Verify `mcp__neuron__*` tools are available in agentic chat when neuron-connectd is running - [ ] Verify agentic chat still works normally when neuron-connectd is not running (falls back to built-ins) - [ ] Verify dharma room agentic turns complete correctly and tools_used is populated in response

will.anderson added 1 commit 2026-06-15 18:07:21 +00:00

fix(chat): wire agentic_tools_all into agentic loop paths

Neuron Soul CI / build (pull_request) Failing after 12m20s

Details

773004f23b

handle_chat_agentic was calling agentic_tools_with_web(), which omits
MCP connector tools, so mcp__* calls were never available in agentic
mode even when neuron-connectd is running.

Switch both agentic entry points to agentic_tools_all(). For
handle_dharma_room_turn_agentic, also replace the inline 8-iteration
loop with a call to agentic_loop() so bridge suspension and the full
connector tool set work consistently. Session IDs are prefixed with
'dharma:' + room_id so suspensions stay room-scoped.

will.anderson commented

2026-06-17 17:48:55 +00:00

Review: fix(chat): wire agentic_tools_all into both agentic loop entry points

The core goal is correct and the refactor of handle_dharma_room_turn_agentic to delegate to agentic_loop is a genuine improvement — the old inline loop called dispatch_tool for any tool without the is_builtin_tool guard, so MCP connector tools would have been misrouted. Delegating to agentic_loop fixes that silently.

One issue needs attention before merge:

Missing tool_pending propagation (warning)

handle_dharma_room_turn_agentic now delegates to agentic_loop, which can return either a final reply or a bridge-suspension envelope ({"tool_pending":true,...}). The new code only checks for "error" and then reads "reply":

let result_error: String = json_get(loop_result, "error")
if !str_eq(result_error, "") {
    return "{\"error\":\"" + result_error + "\",\"response\":\"\",\"cgi_id\":\"" + cgi_id + "\"}"
}

let final_text: String = json_get(loop_result, "reply")   // "" when tool_pending
...
return "{\"response\":\"" + safe_text + "\",...}"         // silent empty response

When a connector tool suspends the loop, json_get(loop_result, "reply") returns "" (the key doesn't exist in the pending envelope), so the function returns {"response":"","cgi_id":"...","tools_used":[]}. The old inline loop had an explicit empty-text guard that returned an error; that guard was dropped in the refactor.

The fix is to detect and pass through the pending envelope, then restore the empty-reply guard:

// Pass bridge-suspension straight through to the caller.
let is_pending: String = json_get(loop_result, "tool_pending")
if str_eq(is_pending, "true") {
    return loop_result
}

let final_text: String = json_get(loop_result, "reply")
if str_eq(final_text, "") {
    return "{\"error\":\"no response\",\"response\":\"\",\"cgi_id\":\"" + cgi_id + "\"}"
}

Note: handle_chat_agentic has the same gap (also introduced in this PR for the tool_pending case), though in practice the Studio client may never send a dharma-agentic request that hits an MCP bridge tool today. The issue is latent but will surface as soon as connector tools are actively used.

Minor notes

agentic_tools_all() makes a live 2-second curl call to the connector bridge on every invocation. For high-frequency dharma room turns this adds latency even when no connectors are configured. Worth caching in soul state with a short TTL eventually.
room_id flows directly into a state key without sanitisation — low risk given the call chain, but worth a comment on expected format.

What's good

Removing the duplicated loop logic from handle_dharma_room_turn_agentic is a clear improvement in maintainability.
The room_id-namespaced session_id ("dharma:" + room_id) is the right approach for bridge suspension in rooms.
agentic_tools_literal → agentic_tools_all in the dharma path correctly adds web search and connector tools, matching handle_chat_agentic.
Error format in the new function ("error" + "response" + "cgi_id") matches the existing dharma response contract.

## Review: fix(chat): wire agentic_tools_all into both agentic loop entry points The core goal is correct and the refactor of `handle_dharma_room_turn_agentic` to delegate to `agentic_loop` is a genuine improvement — the old inline loop called `dispatch_tool` for any tool without the `is_builtin_tool` guard, so MCP connector tools would have been misrouted. Delegating to `agentic_loop` fixes that silently. One issue needs attention before merge: ### Missing tool_pending propagation (warning) `handle_dharma_room_turn_agentic` now delegates to `agentic_loop`, which can return either a final reply or a bridge-suspension envelope (`{"tool_pending":true,...}`). The new code only checks for `"error"` and then reads `"reply"`: ```el let result_error: String = json_get(loop_result, "error") if !str_eq(result_error, "") { return "{\"error\":\"" + result_error + "\",\"response\":\"\",\"cgi_id\":\"" + cgi_id + "\"}" } let final_text: String = json_get(loop_result, "reply") // "" when tool_pending ... return "{\"response\":\"" + safe_text + "\",...}" // silent empty response ``` When a connector tool suspends the loop, `json_get(loop_result, "reply")` returns `""` (the key doesn't exist in the pending envelope), so the function returns `{"response":"","cgi_id":"...","tools_used":[]}`. The old inline loop had an explicit empty-text guard that returned an error; that guard was dropped in the refactor. The fix is to detect and pass through the pending envelope, then restore the empty-reply guard: ```el // Pass bridge-suspension straight through to the caller. let is_pending: String = json_get(loop_result, "tool_pending") if str_eq(is_pending, "true") { return loop_result } let final_text: String = json_get(loop_result, "reply") if str_eq(final_text, "") { return "{\"error\":\"no response\",\"response\":\"\",\"cgi_id\":\"" + cgi_id + "\"}" } ``` Note: `handle_chat_agentic` has the same gap (also introduced in this PR for the tool_pending case), though in practice the Studio client may never send a dharma-agentic request that hits an MCP bridge tool today. The issue is latent but will surface as soon as connector tools are actively used. ### Minor notes - `agentic_tools_all()` makes a live 2-second curl call to the connector bridge on every invocation. For high-frequency dharma room turns this adds latency even when no connectors are configured. Worth caching in soul state with a short TTL eventually. - `room_id` flows directly into a state key without sanitisation — low risk given the call chain, but worth a comment on expected format. ### What's good - Removing the duplicated loop logic from `handle_dharma_room_turn_agentic` is a clear improvement in maintainability. - The `room_id`-namespaced session_id (`"dharma:" + room_id`) is the right approach for bridge suspension in rooms. - `agentic_tools_literal` → `agentic_tools_all` in the dharma path correctly adds web search and connector tools, matching `handle_chat_agentic`. - Error format in the new function (`"error"` + `"response"` + `"cgi_id"`) matches the existing dharma response contract.

will.anderson added 1 commit 2026-06-17 18:01:37 +00:00

fix/test(chat): guard handle_dharma_room_turn_agentic against tool_pending and empty reply

Neuron Soul CI / build (pull_request) Failing after 8m0s

Details

f7ae7df9d6

When agentic_loop suspends for an MCP bridge tool it returns a
{"tool_pending":true,...} envelope with no "reply" key. Without an
explicit check, json_get(loop_result, "reply") returns "" and the
function emitted {"response":"","cgi_id":"..."} — a silent empty
response indistinguishable from a successful LLM turn with no content.

Two guards added after the existing error check:

1. tool_pending passthrough: if the loop suspended, return the pending
   envelope directly so callers (dharma room orchestrators) can
   distinguish suspension from failure and route to the approve flow.

2. Empty-reply guard: if final_text is empty after the pending check,
   return an explicit {"error":"no response",...} envelope instead of
   silently succeeding with an empty response field.

Also adds tests/test_agentic_tools.el:
- agentic_tools_all() includes all literal tool names and web_search
- connector_tools_json() returns valid JSON when bridge is down (graceful degradation)
- tool_pending envelope detection patterns (the is_pending logic)
- json_get(pending_envelope, "reply") returns "" confirming the empty-reply
  guard is load-bearing (pure string/JSON, no LLM or network required)

will.anderson merged commit e7297275a3 into main

2026-06-17 18:06:40 +00:00

will.anderson deleted branch fix/agentic-tools-all

2026-06-17 18:06:44 +00:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: neuron-technologies/neuron#19