From 6e837b21b98e8fbc5077d0f96a117cddc3466c7e Mon Sep 17 00:00:00 2001 From: Adrianno Esnarriaga Sereno Date: Sun, 13 Sep 2026 12:40:03 -0300 Subject: [PATCH 1/3] fix(agent): preserve multi-round ReAct tool results in message history Tool result messages pointed into a reusable scratch buffer that was overwritten on each iteration, corrupting prior rounds before the next provider chat. Own each result with strdup and free ReAct-owned slots on cleanup. Cast through uintptr_t so const tool_calls free cleanly under -Werror=discarded-qualifiers. Refs: #59 --- src/core/agent.c | 75 ++++++++++++++++++++------------ tests/test_agent.c | 104 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 152 insertions(+), 27 deletions(-) diff --git a/src/core/agent.c b/src/core/agent.c index f536687..2e7f45f 100644 --- a/src/core/agent.c +++ b/src/core/agent.c @@ -11,6 +11,7 @@ #include "providers/provider.h" #include "cJSON.h" #include +#include #include #include #include @@ -258,15 +259,23 @@ static const agent_tool_t *find_tool(const agent_tool_t *tools, size_t tool_coun return NULL; } -static void free_tool_calls_copy(provider_tool_call_t *copy, size_t n) +static void agent_free_owned_ptr(const void *p) { + free((void *)(uintptr_t)p); +} + +static void free_tool_calls_copy(const provider_tool_call_t *copy, size_t n) +{ + provider_tool_call_t *owned; + size_t i; if (!copy) return; - for (size_t i = 0; i < n; i++) { - free(copy[i].id); - free(copy[i].name); - free(copy[i].arguments); + owned = (provider_tool_call_t *)(uintptr_t)copy; + for (i = 0; i < n; i++) { + free(owned[i].id); + free(owned[i].name); + free(owned[i].arguments); } - free(copy); + free(owned); } /** Append user+assistant exchange to session JSON. Invalid or empty existing becomes []. */ @@ -343,6 +352,7 @@ typedef struct agent_run_ctx { char *tool_result_bufs; provider_message_t *messages; size_t total_msgs; + size_t base_msg_count; int history_count; int max_iter; int max_ctx; @@ -357,6 +367,20 @@ static void agent_oom_msg(agent_run_ctx_t *ctx) } } +/** Free ReAct-owned slots (assistant text, tool results, tool_calls copies). */ +static void agent_free_heap_messages(agent_run_ctx_t *ctx, size_t from_idx) +{ + size_t i; + if (!ctx->messages) return; + for (i = from_idx; i < ctx->total_msgs; i++) { + agent_free_owned_ptr(ctx->messages[i].content); + ctx->messages[i].content = NULL; + free_tool_calls_copy(ctx->messages[i].tool_calls, ctx->messages[i].tool_calls_count); + ctx->messages[i].tool_calls = NULL; + ctx->messages[i].tool_calls_count = 0; + } +} + /** Load skills, system prompt, memories, session history; compact if over limit. */ static int agent_prepare_context(agent_run_ctx_t *ctx) { @@ -429,6 +453,7 @@ static int agent_build_messages(agent_run_ctx_t *ctx) } ctx->messages[1 + ctx->history_count].role = "user"; ctx->messages[1 + ctx->history_count].content = ctx->user_message; + ctx->base_msg_count = ctx->total_msgs; return 0; } @@ -448,32 +473,20 @@ static int agent_react_loop(agent_run_ctx_t *ctx) { int iteration = 0; provider_response_t response = {0}; - char *prev_assistant = NULL; - provider_tool_call_t *prev_calls = NULL; - size_t prev_n = 0; for (;;) { int err = ctx->provider->chat(ctx->messages, ctx->total_msgs, ctx->tool_defs, ctx->tool_count, &response); if (err != 0) { copy_response_to_buf(response.content, ctx->response_buf, ctx->response_size); provider_response_clear(&response); - free(prev_assistant); - free_tool_calls_copy(prev_calls, prev_n); return -1; } if (response.tool_calls_count == 0 || iteration >= ctx->max_iter) { copy_response_to_buf(response.content, ctx->response_buf, ctx->response_size); provider_response_clear(&response); agent_persist_session(ctx, ctx->response_buf); - free(prev_assistant); - free_tool_calls_copy(prev_calls, prev_n); return 0; } - free(prev_assistant); - free_tool_calls_copy(prev_calls, prev_n); - prev_assistant = NULL; - prev_calls = NULL; - prev_n = 0; { size_t nc = response.tool_calls_count; char *assistant_content; @@ -482,14 +495,12 @@ static int agent_react_loop(agent_run_ctx_t *ctx) size_t new_count; if (nc > MAX_TOOL_CALLS) nc = MAX_TOOL_CALLS; - assistant_content = response.content ? strdup(response.content) : NULL; - if (!assistant_content && response.content && response.content[0] != '\0') { + assistant_content = response.content ? strdup(response.content) : strdup(""); + if (!assistant_content) { provider_response_clear(&response); agent_oom_msg(ctx); return -1; } - if (!assistant_content) - assistant_content = strdup(""); our_calls = copy_tool_calls(response.tool_calls, nc); provider_response_clear(&response); if (!our_calls) { @@ -523,24 +534,34 @@ static int agent_react_loop(agent_run_ctx_t *ctx) new_messages[ctx->total_msgs].tool_calls = our_calls; new_messages[ctx->total_msgs].tool_calls_count = nc; for (size_t k = 0; k < nc; k++) { + char *one_buf = ctx->tool_result_bufs + k * TOOL_RESULT_SIZE; + /* Scratch is reused each round; history must own a copy (Refs: #59). */ + char *tool_content = strdup(one_buf); + if (!tool_content) { + for (size_t j = 0; j < k; j++) + agent_free_owned_ptr(new_messages[ctx->total_msgs + 1 + j].content); + free(new_messages); + free_tool_calls_copy(our_calls, nc); + free(assistant_content); + agent_oom_msg(ctx); + return -1; + } new_messages[ctx->total_msgs + 1 + k].role = "user"; - new_messages[ctx->total_msgs + 1 + k].content = - ctx->tool_result_bufs + k * TOOL_RESULT_SIZE; + new_messages[ctx->total_msgs + 1 + k].content = tool_content; new_messages[ctx->total_msgs + 1 + k].tool_use_id = our_calls[k].id; } free(ctx->messages); ctx->messages = new_messages; ctx->total_msgs = new_count; iteration++; - prev_assistant = assistant_content; - prev_calls = our_calls; - prev_n = nc; } } } static void agent_run_cleanup(agent_run_ctx_t *ctx) { + if (ctx->messages && ctx->base_msg_count < ctx->total_msgs) + agent_free_heap_messages(ctx, ctx->base_msg_count); free(ctx->system_buf); free(ctx->skills_buf); free(ctx->session_buf); diff --git a/tests/test_agent.c b/tests/test_agent.c index 1ee8603..4ed22a9 100644 --- a/tests/test_agent.c +++ b/tests/test_agent.c @@ -567,6 +567,109 @@ static int test_agent_unknown_tool_continues(void) return 0; } +static int seq_tool_exec_count; +static int seq_tool_execute(const char *args_json, char *result_buf, size_t max_len) +{ + (void)args_json; + seq_tool_exec_count++; + if (max_len > 0) { + snprintf(result_buf, max_len, "tool_output_%d", seq_tool_exec_count); + result_buf[max_len - 1] = '\0'; + } + return 0; +} +static const agent_tool_t seq_echo_tool = { + .name = "echo", + .description = "Echo test with sequence counter", + .parameters_json = "{}", + .execute = seq_tool_execute, +}; + +static int multi_tool_round_call_count; +static int multi_tool_round_init(const config_t *cfg) +{ + (void)cfg; + multi_tool_round_call_count = 0; + seq_tool_exec_count = 0; + return 0; +} +static int multi_tool_round_chat(const provider_message_t *messages, size_t message_count, + const provider_tool_def_t *tools, size_t tool_count, provider_response_t *response) +{ + (void)tools; + (void)tool_count; + size_t i; + int saw_first_tool_output = 0; + response->error = 0; + response->tool_calls = NULL; + response->tool_calls_count = 0; + response->content = NULL; + multi_tool_round_call_count++; + if (multi_tool_round_call_count >= 2) { + for (i = 0; i < message_count; i++) { + if (messages[i].content && strstr(messages[i].content, "tool_output_1") != NULL) { + saw_first_tool_output = 1; + break; + } + } + if (!saw_first_tool_output) { + response->content = strdup("CORRUPTED_TOOL_HISTORY"); + return 0; + } + } + if (multi_tool_round_call_count <= 2) { + response->tool_calls = malloc(sizeof(provider_tool_call_t)); + if (!response->tool_calls) { + response->error = 1; + return -1; + } + response->tool_calls[0].id = strdup("mt1"); + response->tool_calls[0].name = strdup("echo"); + response->tool_calls[0].arguments = strdup("{}"); + response->tool_calls_count = 1; + response->content = strdup(""); + return 0; + } + response->content = strdup("multi tool done"); + return 0; +} +static void multi_tool_round_cleanup(void) {} +static const provider_t multi_tool_round_provider = { + .name = "multi_tool_round", + .init = multi_tool_round_init, + .chat = multi_tool_round_chat, + .cleanup = multi_tool_round_cleanup, +}; + +static int test_react_loop_preserves_prior_tool_results(void) +{ + int failed = 1; + const char *path = "build/test_agent_multi_tool.toml"; + FILE *f = fopen(path, "w"); + ASSERT(f); + fprintf(f, "[agent]\nmodel = \"test\"\nmax_tool_iterations = 5\n"); + fclose(f); + config_t *cfg = NULL; + char errbuf[256]; + char response_buf[4096]; + int ret; + if (config_load(path, &cfg, errbuf, sizeof(errbuf)) != 0) goto cleanup; + if (cfg == NULL) goto cleanup; + response_buf[0] = '\0'; + ret = agent_run(cfg, "cli:multitool", "hi", &multi_tool_round_provider, &seq_echo_tool, 1, + response_buf, sizeof(response_buf)); + if (ret != 0) goto cleanup; + if (strstr(response_buf, "multi tool done") == NULL) goto cleanup; + if (strstr(response_buf, "CORRUPTED_TOOL_HISTORY") != NULL) goto cleanup; + if (multi_tool_round_call_count != 3) goto cleanup; + if (seq_tool_exec_count != 2) goto cleanup; + failed = 0; +cleanup: + config_free(cfg); + remove(path); + return failed; +} + static int test_local_offline_note_skipped_for_non_local(void) { const char *path = "build/test_agent_nonlocal_note.toml"; @@ -597,6 +700,7 @@ int main(void) RUN(test_context_assembly_system_prompt_history_memories()); RUN(test_react_loop_tool_then_text()); RUN(test_react_loop_max_iterations()); + RUN(test_react_loop_preserves_prior_tool_results()); RUN(test_session_persisted_after_exchange()); RUN(test_context_compaction_when_history_exceeds_max()); RUN(test_local_offline_note_when_active_is_local()); From ccff48c9319e9d0a085b23163a607744b950cd1f Mon Sep 17 00:00:00 2001 From: Adrianno Esnarriaga Sereno Date: Sun, 13 Sep 2026 12:40:27 -0300 Subject: [PATCH 2/3] docs(agent): changelog preserve multi-round ReAct tool results Refs: #59 --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 955f8d7..bb7458f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ All notable changes to ShellClaw are documented here. Format follows [Keep a Cha ## [Unreleased] ### Fixed +- Multi-round ReAct copies tool results into the in-flight message list so a later round cannot overwrite earlier outputs. - `memory_init` no longer deletes an existing SQLite DB when `sqlite3_open` fails (permissions or transient I/O). - Anthropic `content` parse fails closed when growing the text buffer or `tool_use` array cannot `realloc`, instead of copying against an inflated cap. - HTTP 200 JSON-RPC results with a malformed ASAP envelope no longer double-free the duplicated request id. From c7a0b13d69de164e1e1e87a577962a2b5b5ec5b7 Mon Sep 17 00:00:00 2001 From: Adrianno Esnarriaga Sereno Date: Sun, 13 Sep 2026 12:52:24 -0300 Subject: [PATCH 3/3] test(agent): assert ReAct tool_calls stay live across rounds The content-only check would still pass if prev_calls were freed after chat() returned. On the third chat(), require a readable tool_calls id and a matching tool_use_id. Null tool_use_id before freeing copies. Refs: #59 --- src/core/agent.c | 3 +++ tests/test_agent.c | 45 +++++++++++++++++++++++++++++++++++---------- 2 files changed, 38 insertions(+), 10 deletions(-) diff --git a/src/core/agent.c b/src/core/agent.c index 2e7f45f..16e55d5 100644 --- a/src/core/agent.c +++ b/src/core/agent.c @@ -372,6 +372,9 @@ static void agent_free_heap_messages(agent_run_ctx_t *ctx, size_t from_idx) { size_t i; if (!ctx->messages) return; + /* tool_use_id aliases our_calls[k].id; drop it before freeing tool_calls. */ + for (i = from_idx; i < ctx->total_msgs; i++) + ctx->messages[i].tool_use_id = NULL; for (i = from_idx; i < ctx->total_msgs; i++) { agent_free_owned_ptr(ctx->messages[i].content); ctx->messages[i].content = NULL; diff --git a/tests/test_agent.c b/tests/test_agent.c index 4ed22a9..7e0a46f 100644 --- a/tests/test_agent.c +++ b/tests/test_agent.c @@ -586,36 +586,57 @@ static const agent_tool_t seq_echo_tool = { }; static int multi_tool_round_call_count; +static int multi_tool_round_saw_live_tool_calls; static int multi_tool_round_init(const config_t *cfg) { (void)cfg; multi_tool_round_call_count = 0; seq_tool_exec_count = 0; + multi_tool_round_saw_live_tool_calls = 0; return 0; } +static int multi_tool_history_is_intact(const provider_message_t *messages, size_t message_count) +{ + size_t i; + int saw_first_tool_output = 0; + const char *first_call_id = NULL; + int saw_matching_use_id = 0; + for (i = 0; i < message_count; i++) { + if (messages[i].content && strstr(messages[i].content, "tool_output_1") != NULL) + saw_first_tool_output = 1; + if (!first_call_id && messages[i].tool_calls && messages[i].tool_calls_count == 1 && + messages[i].tool_calls[0].id) { + first_call_id = messages[i].tool_calls[0].id; + if (first_call_id[0] == '\0') + return 0; + } + } + if (!saw_first_tool_output || !first_call_id) + return 0; + for (i = 0; i < message_count; i++) { + if (messages[i].tool_use_id && strcmp(messages[i].tool_use_id, first_call_id) == 0) { + saw_matching_use_id = 1; + break; + } + } + return saw_matching_use_id; +} static int multi_tool_round_chat(const provider_message_t *messages, size_t message_count, const provider_tool_def_t *tools, size_t tool_count, provider_response_t *response) { (void)tools; (void)tool_count; - size_t i; - int saw_first_tool_output = 0; response->error = 0; response->tool_calls = NULL; response->tool_calls_count = 0; response->content = NULL; multi_tool_round_call_count++; - if (multi_tool_round_call_count >= 2) { - for (i = 0; i < message_count; i++) { - if (messages[i].content && strstr(messages[i].content, "tool_output_1") != NULL) { - saw_first_tool_output = 1; - break; - } - } - if (!saw_first_tool_output) { + if (multi_tool_round_call_count >= 3) { + if (!multi_tool_history_is_intact(messages, message_count)) { response->content = strdup("CORRUPTED_TOOL_HISTORY"); return 0; } + multi_tool_round_saw_live_tool_calls = 1; } if (multi_tool_round_call_count <= 2) { response->tool_calls = malloc(sizeof(provider_tool_call_t)); @@ -655,6 +676,9 @@ static int test_react_loop_preserves_prior_tool_results(void) int ret; if (config_load(path, &cfg, errbuf, sizeof(errbuf)) != 0) goto cleanup; if (cfg == NULL) goto cleanup; + multi_tool_round_call_count = 0; + seq_tool_exec_count = 0; + multi_tool_round_saw_live_tool_calls = 0; response_buf[0] = '\0'; ret = agent_run(cfg, "cli:multitool", "hi", &multi_tool_round_provider, &seq_echo_tool, 1, response_buf, sizeof(response_buf)); @@ -663,6 +687,7 @@ static int test_react_loop_preserves_prior_tool_results(void) if (strstr(response_buf, "CORRUPTED_TOOL_HISTORY") != NULL) goto cleanup; if (multi_tool_round_call_count != 3) goto cleanup; if (seq_tool_exec_count != 2) goto cleanup; + if (!multi_tool_round_saw_live_tool_calls) goto cleanup; failed = 0; cleanup: config_free(cfg);