fix(agent): preserve multi-round ReAct tool results in message history

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.

src/core/agent.c

@@ -11,6 +11,7 @@
 #include "providers/provider.h"
 #include "cJSON.h"
 #include <pthread.h>
+#include <stdint.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -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,23 @@ 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;
+	/* 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;
+		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 +456,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 +476,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 +498,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 +537,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);

tests/test_agent.c

@@ -567,6 +567,134 @@ 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_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;
+	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 >= 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));
+		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;
+	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));
+	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;
+	if (!multi_tool_round_saw_live_tool_calls) 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 +725,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());