From 5723b80a9b56123ef0de7db820802dc6ec4a57d7 Mon Sep 17 00:00:00 2001 From: Wolfy-J Date: Mon, 10 Aug 2026 13:28:09 -0400 Subject: [PATCH] fix: answer sibling tool calls when exit validator rejects finish process_tool_results returned as soon as a rejected finish set skip_call, before the loop that records tool_result observations for any other tool calls made in the same assistant turn. Those siblings had already run (their output sat in tool_results), so the next prompt rebuild carried tool_use ids the API never saw answered, and Anthropic's messages API rejected the whole request. Only a genuine completion (task_complete) should skip the loop; a rejected finish must fall through so siblings still get recorded. --- src/node/agent/_index.yaml | 19 +++ src/node/agent/node.lua | 10 +- src/node/agent/process_tool_results_test.lua | 131 ++++++++++++++++++ src/node/agent/stub/_index.yaml | 7 + src/node/agent/stub/exit_reject_validator.lua | 7 + 5 files changed, 170 insertions(+), 4 deletions(-) create mode 100644 src/node/agent/process_tool_results_test.lua create mode 100644 src/node/agent/stub/exit_reject_validator.lua diff --git a/src/node/agent/_index.yaml b/src/node/agent/_index.yaml index b5121d1..db06aa8 100644 --- a/src/node/agent/_index.yaml +++ b/src/node/agent/_index.yaml @@ -331,6 +331,25 @@ entries: test: wippy.test:test method: run_tests + - name: process_tool_results_test + kind: function.lua + meta: + name: Agent Node process_tool_results Test + type: test + comment: Unit tests proving a rejected finish still records observations for sibling tool calls in the same turn. + group: Workflow / Agent Node + tags: + - dataflow + - agent + - exit-validation + - test + source: file://process_tool_results_test.lua + imports: + agent_node: userspace.dataflow.node.agent:node + agent_consts: userspace.dataflow.node.agent:consts + test: wippy.test:test + method: run_tests + - name: agent_checkpoint_test kind: function.lua meta: diff --git a/src/node/agent/node.lua b/src/node/agent/node.lua index 902504f..dce24b7 100644 --- a/src/node/agent/node.lua +++ b/src/node/agent/node.lua @@ -1428,7 +1428,6 @@ local function process_tool_results(n, tool_results, iteration, exit_tool_name, local control_delegations = {} local task_complete = false local final_result = nil - local skip_call = false if exit_tool_name and agent_result.tool_calls then for _, original_tool_call in ipairs(agent_result.tool_calls) do @@ -1454,8 +1453,6 @@ local function process_tool_results(n, tool_results, iteration, exit_tool_name, exit_validation = true } }) - task_complete = false - skip_call = true else task_complete = true final_result = validated_result @@ -1469,7 +1466,11 @@ local function process_tool_results(n, tool_results, iteration, exit_tool_name, end end - if task_complete or skip_call then + -- A rejected finish must not short-circuit here: any sibling tool calls in the + -- same turn already ran (tool_results holds their output) and each still needs a + -- recorded observation, or the next request carries tool_use ids the API never + -- sees answered. Only a genuine completion ends the loop early. + if task_complete then return control_responses, control_delegations, task_complete, final_result end @@ -2388,5 +2389,6 @@ return { _test = { build_agent_context_config = build_agent_context_config, process_multiple_inputs = process_multiple_inputs, + process_tool_results = process_tool_results, } } diff --git a/src/node/agent/process_tool_results_test.lua b/src/node/agent/process_tool_results_test.lua new file mode 100644 index 0000000..af19642 --- /dev/null +++ b/src/node/agent/process_tool_results_test.lua @@ -0,0 +1,131 @@ +local test = require("test") +local agent_node = require("agent_node") +local agent_consts = require("agent_consts") + +-- A minimal node_sdk double: records every n:data() call so tests can assert +-- on which observations process_tool_results actually wrote. +local function make_recording_node() + local recorded = {} + local n + n = { + node_id = "test-node", + data = function(_self, data_type, content, opts) + table.insert(recorded, { + data_type = data_type, + content = content, + opts = opts + }) + end + } + return n, recorded +end + +local function find_by_tool_call_id(recorded, tool_call_id) + for _, row in ipairs(recorded) do + local meta = row.opts and row.opts.metadata or {} + if meta.tool_call_id == tool_call_id then + return row + end + end + return nil +end + +local function define_tests() + describe("process_tool_results: exit validator rejection", function() + it("still records observations for sibling tool calls in the same turn", function() + local process_tool_results = agent_node._test.process_tool_results + test.not_nil(process_tool_results, "process_tool_results exported for testing") + + local n, recorded = make_recording_node() + + local agent_result = { + tool_calls = { + { id = "call_finish", name = "finish", arguments = { answer = "done" } }, + { id = "call_search_1", name = "kb_search", arguments = { query = "a" } }, + { id = "call_search_2", name = "kb_search", arguments = { query = "b" } }, + } + } + + -- Only the sibling calls were actually executed by execute_tools; the + -- exit call never appears in tool_results (matches production: split_exit_tool_calls + -- routes it away from the executable set). + local tool_results = { + call_search_1 = { result = { hits = 1 } }, + call_search_2 = { result = { hits = 2 } }, + } + + local arena_config = { + exit_func_id = "userspace.dataflow.node.agent.stub:exit_reject_validator" + } + + local control_responses, control_delegations, task_complete, final_result = process_tool_results( + n, + tool_results, + 1, + "finish", + agent_result, + arena_config, + {}, + {} + ) + + test.eq(task_complete, false, "rejected finish does not complete the task") + test.is_nil(final_result, "no final result on rejection") + test.eq(#control_responses, 0, "no control responses for plain tool results") + test.eq(#control_delegations, 0, "no delegations for plain tool results") + + local rejection = find_by_tool_call_id(recorded, "call_finish") + test.not_nil(rejection, "rejection observation recorded for the finish call") + test.eq(rejection.data_type, agent_consts.DATA_TYPE.AGENT_OBSERVATION, "rejection is an observation") + test.eq((rejection.opts.metadata or {}).is_error, true, "rejection observation flagged as error") + test.eq((rejection.opts.metadata or {}).exit_validation, true, "rejection observation flagged as exit validation") + + local sibling_1 = find_by_tool_call_id(recorded, "call_search_1") + test.not_nil(sibling_1, "sibling call_search_1 got a recorded observation") + test.eq((sibling_1.opts.metadata or {}).tool_name, "kb_search", "sibling observation carries tool name") + test.eq((sibling_1.opts.metadata or {}).is_error, false, "sibling observation is not an error") + + local sibling_2 = find_by_tool_call_id(recorded, "call_search_2") + test.not_nil(sibling_2, "sibling call_search_2 got a recorded observation") + + test.eq(#recorded, 3, "exactly one observation per tool_use id: rejection plus both siblings") + end) + + it("a genuine completion still returns early without touching sibling results", function() + local process_tool_results = agent_node._test.process_tool_results + + local n, recorded = make_recording_node() + + local agent_result = { + tool_calls = { + { id = "call_finish", name = "finish", arguments = { answer = "done" } }, + { id = "call_search_1", name = "kb_search", arguments = { query = "a" } }, + } + } + + local tool_results = { + call_search_1 = { result = { hits = 1 } }, + } + + -- No exit_func_id: the exit tool call is accepted unconditionally. + local arena_config = {} + + local _control_responses, _control_delegations, task_complete, final_result = process_tool_results( + n, + tool_results, + 1, + "finish", + agent_result, + arena_config, + {}, + {} + ) + + test.eq(task_complete, true, "unconditional finish completes the task") + test.eq((final_result :: any).answer, "done", "final result carries the finish arguments") + test.eq(#recorded, 0, "success path does not record sibling tool observations") + end) + end) +end + +return { run_tests = test.run_cases(define_tests) } diff --git a/src/node/agent/stub/_index.yaml b/src/node/agent/stub/_index.yaml index d7c4480..ecd6446 100644 --- a/src/node/agent/stub/_index.yaml +++ b/src/node/agent/stub/_index.yaml @@ -98,6 +98,13 @@ entries: imports: helpers: userspace.dataflow.node.agent.stub:recovery_helpers method: handler +- name: exit_reject_validator + kind: function.lua + meta: + test_only: true + comment: Exit validator stub that always rejects, for exit-validation-rejection tests + source: file://exit_reject_validator.lua + method: handler - name: lifecycle_recorder kind: function.lua meta: diff --git a/src/node/agent/stub/exit_reject_validator.lua b/src/node/agent/stub/exit_reject_validator.lua new file mode 100644 index 0000000..1c49672 --- /dev/null +++ b/src/node/agent/stub/exit_reject_validator.lua @@ -0,0 +1,7 @@ +-- Deterministic exit validator stub that always rejects, for exercising the +-- arena_config.exit_func_id rejection path in process_tool_results tests. +local function handler(_input) + return nil, "required output missing" +end + +return { handler = handler }