From 4641b18ab305247f607050dd0ab6571f4903e4f9 Mon Sep 17 00:00:00 2001 From: Vasanthdev2004 Date: Tue, 4 Aug 2026 14:39:55 +0530 Subject: [PATCH 1/2] fix(agent): stop a denied tool looping past the repeated-failure halt MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The repeated-failure guard keys its streak on the first 80 characters of the error text. A permission denial reads "Error: Permission denied for : ", and reason names the path or command that was refused, so the text differs on every call while describing the same unchanging refusal. Each call therefore rebuilt the record at count 1 and toolFailureStopAt was never reached. Not hypothetical. A headless run made 384 denied calls over 26 minutes under a halt set to 6, produced no files, and reported nothing. #702 already hit this shape once and fixed it by making one error message id-invariant; that works per message and needs every future message to remember. Denials now key on their DenialCategory instead, which is a small closed enum the loop already sets on the result, so the class is fixed rather than one instance of it. Adds a second, content-blind counter beside the streak. The signature-keyed one cannot by construction see a tool that fails with a genuinely different error every time, and that is still a tool that is not working. It counts consecutive failures regardless of the error and is cleared only by a success of that same tool, so changing how a tool fails is not progress and neither is some other tool succeeding. It stops at 12 rather than 6 on purpose: a model iterating on a tricky edit legitimately fails a few times with different errors while converging, which is the same reasoning that moved toolFailureStopAt from 4 to 6. Two counters, tripping on either, is what both of the agent CLIs I compared against arrived at independently after hitting this bug — a tight bound on identical failures ORed with a looser bound that no amount of varying the error can reset. Every guard is mutation-checked. Reverting the denial re-key fails TestPermissionDenialStreakSurvivesVaryingReasonText; deleting the content-blind bound, or letting a signature change reset it, each fail TestToolFailingWithDifferentErrorsEveryTimeStillStops and TestSuccessResetsBothFailureCounters. One existing test call site gains the new parameter. --- internal/agent/guardrails.go | 48 +++++++++++-- internal/agent/guardrails_test.go | 109 +++++++++++++++++++++++++++++- internal/agent/loop.go | 2 +- 3 files changed, 152 insertions(+), 7 deletions(-) diff --git a/internal/agent/guardrails.go b/internal/agent/guardrails.go index ea179b459..dfa8e9a85 100644 --- a/internal/agent/guardrails.go +++ b/internal/agent/guardrails.go @@ -55,6 +55,22 @@ const ( // error, so this only affects true same-error loops. toolFailureStopAt = 6 + // toolFailureAnyErrorStopAt halts a tool that keeps failing with DIFFERENT + // errors. The streak above cannot see that case by construction: a changed + // signature rebuilds the record at 1, so a tool whose error text varies every + // call never reaches toolFailureStopAt however long it loops. + // + // That is not hypothetical. A headless run made 384 denied calls over 26 + // minutes without tripping a halt set to 6, because each denial named a + // different path. Keying denials on their category (see observeToolResult) + // fixes that case; this bound covers every error with no category to key on. + // + // Deliberately well above toolFailureStopAt. A model iterating on a genuinely + // tricky edit legitimately fails several times with different errors while it + // converges, and this must not cut those runs short — the same reasoning that + // moved toolFailureStopAt from 4 to 6. Both counters reset on success. + toolFailureAnyErrorStopAt = 12 + // maxContinueNudges bounds how many times the headless completion gate // (Options.RequireCompletionSignal) re-prompts a model that stopped without a // tool call while work clearly remained. Once spent, the run finalizes as @@ -327,6 +343,11 @@ type toolFailureRecord struct { count int errSig string hintShown bool + // anyErrorCount counts consecutive failures of this tool REGARDLESS of the + // error, and is cleared only by a success. count above restarts whenever the + // signature changes, which is exactly what a varying error message defeats; + // this one cannot be reset by changing the text. + anyErrorCount int } type toolFailureOutcome struct { @@ -472,24 +493,41 @@ func newGuardState() *guardState { // observeToolResult tracks repeated identical failures of a tool. A successful // result clears that tool's failure streak. Returns whether to inject a one-shot // corrective hint and/or stop the run. -func (state *guardState) observeToolResult(name string, failed bool, output string) toolFailureOutcome { +func (state *guardState) observeToolResult(name string, failed bool, output string, denial DenialCategory) toolFailureOutcome { if state.toolFailures == nil { state.toolFailures = map[string]*toolFailureRecord{} } if !failed { - delete(state.toolFailures, name) // success resets the streak + delete(state.toolFailures, name) // success resets both counters return toolFailureOutcome{} } + // A denial keys on its CATEGORY, not its prose. The message embeds the path + // or command that was refused, so it differs on every call while describing + // the same unchanging refusal — which rebuilt the record at 1 each time and + // let a denied tool loop indefinitely under a halt set to 6. The category is + // a small closed enum the loop already sets on the result. sig := errorSignature(output) + if denial != "" { + sig = "denial:" + string(denial) + } record := state.toolFailures[name] - if record == nil || record.errSig != sig { - record = &toolFailureRecord{count: 1, errSig: sig} + if record == nil { + record = &toolFailureRecord{errSig: sig} state.toolFailures[name] = record + } + if record.errSig != sig { + // A different error restarts the same-error streak but NOT the + // content-blind one: changing how a tool fails is not progress. + record.count = 1 + record.errSig = sig + record.hintShown = false } else { record.count++ } + record.anyErrorCount++ + outcome := toolFailureOutcome{Count: record.count} - if record.count >= toolFailureStopAt { + if record.count >= toolFailureStopAt || record.anyErrorCount >= toolFailureAnyErrorStopAt { outcome.Stop = true return outcome } diff --git a/internal/agent/guardrails_test.go b/internal/agent/guardrails_test.go index fc8079f13..8c7ef47b6 100644 --- a/internal/agent/guardrails_test.go +++ b/internal/agent/guardrails_test.go @@ -2,6 +2,8 @@ package agent import ( "context" + "path/filepath" + "strconv" "strings" "testing" @@ -189,7 +191,7 @@ func TestUnknownExecSessionProbingTripsFailureHalt(t *testing.T) { var state guardState var stoppedAt int for i := 1; i <= toolFailureStopAt; i++ { - out := state.observeToolResult(tools.WriteStdinToolName, true, tools.UnknownExecSessionError(i)) + out := state.observeToolResult(tools.WriteStdinToolName, true, tools.UnknownExecSessionError(i), "") if out.Stop { stoppedAt = i break @@ -200,6 +202,111 @@ func TestUnknownExecSessionProbingTripsFailureHalt(t *testing.T) { } } +// A permission denial repeats forever when the streak is keyed on the error +// TEXT, because the denial message embeds the path or command that varies per +// call. +// +// Observed, not theorised: a headless run made 384 denied calls over 26 minutes +// without tripping a halt that stops at 6. Every denial carried the same typed +// category and a different reason string, so errorSignature differed each time +// and the record was rebuilt at count 1 on every call. +// +// TestUnknownExecSessionErrorSignatureIsIDInvariant fixed one message this way. +// Keying on the category fixes the class, without needing every future denial +// message to remember to be invariant. +func TestPermissionDenialStreakSurvivesVaryingReasonText(t *testing.T) { + var state guardState + stoppedAt := 0 + for i := 1; i <= toolFailureStopAt; i++ { + // The shape the loop actually produces: same tool, same category, a + // different path every time. + output := "Error: Permission denied for write_file: cannot write " + + filepath.Join("C:", "ws", "pkg", "file"+strconv.Itoa(i)+".go") + out := state.observeToolResult("write_file", true, output, DenialPermissionDenied) + if out.Stop { + stoppedAt = i + break + } + } + if stoppedAt != toolFailureStopAt { + t.Fatalf("denials with varying reason text stopped at %d, want %d", stoppedAt, toolFailureStopAt) + } +} + +// The content-blind bound. A tool failing over and over with genuinely +// DIFFERENT errors and no denial category is still a tool that is not working, +// and the same-signature streak can never see it. +// +// Bounded well above toolFailureStopAt on purpose: a model iterating on a +// tricky edit legitimately fails a few times with different errors while it +// converges, and cutting that short is the regression +// TestSuccessResetsBothFailureCounters guards. +func TestToolFailingWithDifferentErrorsEveryTimeStillStops(t *testing.T) { + var state guardState + stoppedAt := 0 + for i := 1; i <= toolFailureAnyErrorStopAt; i++ { + out := state.observeToolResult("bash", true, "distinct failure "+strconv.Itoa(i), "") + if out.Stop { + stoppedAt = i + break + } + } + if stoppedAt != toolFailureAnyErrorStopAt { + t.Fatalf("a tool failing with a new error each call stopped at %d, want %d", stoppedAt, toolFailureAnyErrorStopAt) + } +} + +// Neither counter may outlive a success, or a long run that fails occasionally +// and recovers would eventually halt for no reason. This is the property that +// keeps the content-blind bound safe to add. +func TestSuccessResetsBothFailureCounters(t *testing.T) { + var state guardState + for i := 1; i < toolFailureAnyErrorStopAt; i++ { + if out := state.observeToolResult("bash", true, "distinct failure "+strconv.Itoa(i), ""); out.Stop { + t.Fatalf("stopped at %d before the success that should reset it", i) + } + } + state.observeToolResult("bash", false, "ok", "") + + // Same again from zero. Reaching the bound a second time proves the counter + // restarted rather than merely paused. + stoppedAt := 0 + for i := 1; i <= toolFailureAnyErrorStopAt; i++ { + if out := state.observeToolResult("bash", true, "later failure "+strconv.Itoa(i), ""); out.Stop { + stoppedAt = i + break + } + } + if stoppedAt != toolFailureAnyErrorStopAt { + t.Fatalf("after a success the tool stopped at %d, want a full fresh %d", stoppedAt, toolFailureAnyErrorStopAt) + } +} + +// Records are keyed per tool, so ANOTHER tool succeeding in between must not +// clear the failing tool's streak. +// +// This is the realistic shape of the run that motivated the fix: the model kept +// making progress elsewhere — reading files, updating its plan — while one tool +// was refused over and over. A reset keyed on "something succeeded" rather than +// "this tool succeeded" would make the halt unreachable in exactly the runs that +// need it. +func TestAnotherToolSucceedingDoesNotClearAFailingToolsStreak(t *testing.T) { + var state guardState + stoppedAt := 0 + for i := 1; i <= toolFailureStopAt; i++ { + state.observeToolResult("read_file", false, "ok", "") + out := state.observeToolResult("write_file", true, + "Error: Permission denied for write_file: "+strconv.Itoa(i), DenialPermissionDenied) + if out.Stop { + stoppedAt = i + break + } + } + if stoppedAt != toolFailureStopAt { + t.Fatalf("denials interleaved with another tool's successes stopped at %d, want %d", stoppedAt, toolFailureStopAt) + } +} + func TestGuardStateResetsToolOnlyStreakOnEmptyNonToolTurn(t *testing.T) { var state guardState toolOnly := zeroruntime.CollectedStream{ diff --git a/internal/agent/loop.go b/internal/agent/loop.go index 05691295a..960190693 100644 --- a/internal/agent/loop.go +++ b/internal/agent/loop.go @@ -740,7 +740,7 @@ func Run(ctx context.Context, prompt string, provider Provider, options Options) // aren't fixed by reformatting the call, so a "match this schema" hint // would misdirect the model toward JSON shape or blocked behavior. retriableFailure := isRetriableToolError(toolResult) - outcome := guards.observeToolResult(call.Name, retriableFailure, toolResult.Output) + outcome := guards.observeToolResult(call.Name, retriableFailure, toolResult.Output, toolResult.DenialReason) posture.observeToolOutcome(outcome, toolResult) if outcome.Stop { // The assistant message advertised EVERY collected tool call, but From c519809539a981a38acac3bddf50baadaadebabc Mon Sep 17 00:00:00 2001 From: Vasanthdev2004 Date: Tue, 4 Aug 2026 19:10:34 +0530 Subject: [PATCH 2/2] fix(agent): count denials as failures and report the bound that tripped MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses both blocking findings from @anandh8x's review. He was right on both, and the first was fatal: the previous commit was a no-op in production. loop.go passed isRetriableToolError as the guard's `failed` flag, and that returns false for any categorized denial (a policy refusal is deliberately not retriable). observeToolResult therefore took its success branch and DELETED the record before it could key on DenialReason, so a denied tool still looped to the turn limit. The re-key was correct and unreachable. The flag is now split. `failed` counts a denial toward the streaks; `hintable` stays retriable-only, because a schema hint is the wrong response to a refusal — the call shape is fine, the answer was no. Collapsing the two is what made a caller unable to express "count this but do not coach the model about it". Second finding: outcome.Count returned the signature-keyed record.count even when the content-blind counter was what tripped the stop. With twelve distinct errors that count is 1, so the final answer told the user a tool "failed 1 time in a row with the same error". The outcome now carries the counter that actually fired plus a Varied flag, and the stop answer says "each with a different error" in that case. Every earlier test passed while the production path was broken, because they called observeToolResult directly with failed=true. So the important addition here is TestRunStopsARepeatedlyDeniedToolAtTheFailureBound, which drives Run itself: a tool that always prompts, an approver that always denies, and a different command per turn so the denial reason varies as it does in a real run. Verified by reverting the fix: the run makes 10 denied calls instead of halting at 6 and dies on the no-output guard 13 turns later, while the helper-level test stays green — which is precisely why this shipped in the first place. --- internal/agent/guardrails.go | 35 ++++++-- internal/agent/guardrails_test.go | 128 ++++++++++++++++++++++++++++-- internal/agent/loop.go | 10 ++- 3 files changed, 157 insertions(+), 16 deletions(-) diff --git a/internal/agent/guardrails.go b/internal/agent/guardrails.go index dfa8e9a85..a6d157bff 100644 --- a/internal/agent/guardrails.go +++ b/internal/agent/guardrails.go @@ -354,6 +354,11 @@ type toolFailureOutcome struct { InjectHint bool Stop bool Count int + // Varied reports that the stop came from the content-blind bound, i.e. Count + // failures with DIFFERENT errors rather than the same one repeated. The final + // answer says so, because "failed 12 times with the same error" would be a + // plainly false description of a tool that failed 12 different ways. + Varied bool } // errorSignature normalizes a tool error to a short, comparable signature so @@ -379,9 +384,13 @@ func toolFailureHint(toolName, schemaJSON, errOutput string) string { // toolFailureStopAnswer is the final answer when the repeated-failure guard halts // a run. -func toolFailureStopAnswer(toolName string, count int) string { +func toolFailureStopAnswer(toolName string, count int, varied bool) string { + cause := " times in a row with the same error, " + if varied { + cause = " times in a row, each with a different error, " + } return "Agent stopped: the `" + toolName + "` tool failed " + strconv.Itoa(count) + - " times in a row with the same error, so I halted instead of looping further. " + + cause + "so I halted instead of looping further. " + "Please check the request or adjust the tool arguments." } @@ -493,7 +502,12 @@ func newGuardState() *guardState { // observeToolResult tracks repeated identical failures of a tool. A successful // result clears that tool's failure streak. Returns whether to inject a one-shot // corrective hint and/or stop the run. -func (state *guardState) observeToolResult(name string, failed bool, output string, denial DenialCategory) toolFailureOutcome { +// hintable is separate from failed on purpose. A categorized denial MUST count +// toward the streaks — that is the whole point of keying on the category — but a +// schema hint is the wrong response to a policy refusal: the call shape is fine, +// the answer was no. Collapsing the two is what made the earlier version of this +// fix a no-op, since the caller could only pass a flag that excluded denials. +func (state *guardState) observeToolResult(name string, failed bool, hintable bool, output string, denial DenialCategory) toolFailureOutcome { if state.toolFailures == nil { state.toolFailures = map[string]*toolFailureRecord{} } @@ -507,7 +521,7 @@ func (state *guardState) observeToolResult(name string, failed bool, output stri // let a denied tool loop indefinitely under a halt set to 6. The category is // a small closed enum the loop already sets on the result. sig := errorSignature(output) - if denial != "" { + if denial != DenialNone { sig = "denial:" + string(denial) } record := state.toolFailures[name] @@ -527,11 +541,20 @@ func (state *guardState) observeToolResult(name string, failed bool, output stri record.anyErrorCount++ outcome := toolFailureOutcome{Count: record.count} - if record.count >= toolFailureStopAt || record.anyErrorCount >= toolFailureAnyErrorStopAt { + switch { + case record.count >= toolFailureStopAt: + outcome.Stop = true + return outcome + case record.anyErrorCount >= toolFailureAnyErrorStopAt: + // Report the counter that actually tripped. record.count is the + // same-signature streak and is often 1 here, which would describe a tool + // that failed a dozen different ways as having failed once. outcome.Stop = true + outcome.Count = record.anyErrorCount + outcome.Varied = true return outcome } - if record.count >= toolFailureHintAt && !record.hintShown { + if hintable && record.count >= toolFailureHintAt && !record.hintShown { record.hintShown = true outcome.InjectHint = true } diff --git a/internal/agent/guardrails_test.go b/internal/agent/guardrails_test.go index 8c7ef47b6..859275120 100644 --- a/internal/agent/guardrails_test.go +++ b/internal/agent/guardrails_test.go @@ -191,7 +191,7 @@ func TestUnknownExecSessionProbingTripsFailureHalt(t *testing.T) { var state guardState var stoppedAt int for i := 1; i <= toolFailureStopAt; i++ { - out := state.observeToolResult(tools.WriteStdinToolName, true, tools.UnknownExecSessionError(i), "") + out := state.observeToolResult(tools.WriteStdinToolName, true, true, tools.UnknownExecSessionError(i), "") if out.Stop { stoppedAt = i break @@ -222,7 +222,7 @@ func TestPermissionDenialStreakSurvivesVaryingReasonText(t *testing.T) { // different path every time. output := "Error: Permission denied for write_file: cannot write " + filepath.Join("C:", "ws", "pkg", "file"+strconv.Itoa(i)+".go") - out := state.observeToolResult("write_file", true, output, DenialPermissionDenied) + out := state.observeToolResult("write_file", true, true, output, DenialPermissionDenied) if out.Stop { stoppedAt = i break @@ -245,7 +245,7 @@ func TestToolFailingWithDifferentErrorsEveryTimeStillStops(t *testing.T) { var state guardState stoppedAt := 0 for i := 1; i <= toolFailureAnyErrorStopAt; i++ { - out := state.observeToolResult("bash", true, "distinct failure "+strconv.Itoa(i), "") + out := state.observeToolResult("bash", true, true, "distinct failure "+strconv.Itoa(i), "") if out.Stop { stoppedAt = i break @@ -262,17 +262,17 @@ func TestToolFailingWithDifferentErrorsEveryTimeStillStops(t *testing.T) { func TestSuccessResetsBothFailureCounters(t *testing.T) { var state guardState for i := 1; i < toolFailureAnyErrorStopAt; i++ { - if out := state.observeToolResult("bash", true, "distinct failure "+strconv.Itoa(i), ""); out.Stop { + if out := state.observeToolResult("bash", true, true, "distinct failure "+strconv.Itoa(i), ""); out.Stop { t.Fatalf("stopped at %d before the success that should reset it", i) } } - state.observeToolResult("bash", false, "ok", "") + state.observeToolResult("bash", false, false, "ok", "") // Same again from zero. Reaching the bound a second time proves the counter // restarted rather than merely paused. stoppedAt := 0 for i := 1; i <= toolFailureAnyErrorStopAt; i++ { - if out := state.observeToolResult("bash", true, "later failure "+strconv.Itoa(i), ""); out.Stop { + if out := state.observeToolResult("bash", true, true, "later failure "+strconv.Itoa(i), ""); out.Stop { stoppedAt = i break } @@ -294,8 +294,8 @@ func TestAnotherToolSucceedingDoesNotClearAFailingToolsStreak(t *testing.T) { var state guardState stoppedAt := 0 for i := 1; i <= toolFailureStopAt; i++ { - state.observeToolResult("read_file", false, "ok", "") - out := state.observeToolResult("write_file", true, + state.observeToolResult("read_file", false, false, "ok", "") + out := state.observeToolResult("write_file", true, false, "Error: Permission denied for write_file: "+strconv.Itoa(i), DenialPermissionDenied) if out.Stop { stoppedAt = i @@ -307,6 +307,118 @@ func TestAnotherToolSucceedingDoesNotClearAFailingToolsStreak(t *testing.T) { } } +// alwaysPromptingTool is never allowed to run: it exists so a Run-level test can +// drive real permission denials through the loop. +type alwaysPromptingTool struct{ ran int } + +func (tool *alwaysPromptingTool) Name() string { return "bash" } +func (tool *alwaysPromptingTool) Description() string { return "test shell tool" } +func (tool *alwaysPromptingTool) Parameters() tools.Schema { + return tools.Schema{ + Type: "object", + Properties: map[string]tools.PropertySchema{"command": {Type: "string"}}, + Required: []string{"command"}, + AdditionalProperties: false, + } +} +func (tool *alwaysPromptingTool) Safety() tools.Safety { + return tools.Safety{SideEffect: tools.SideEffectShell, Permission: tools.PermissionPrompt, Reason: "runs shell commands"} +} +func (tool *alwaysPromptingTool) Run(context.Context, map[string]any) tools.Result { + tool.ran++ + return tools.Result{Status: tools.StatusOK, Output: "should never run"} +} + +// The regression for the gap this whole change existed to close, driven through +// Run rather than through the guard helper. +// +// The first version of this fix was a no-op in production and every one of its +// unit tests passed, because they called observeToolResult directly with +// failed=true. The loop passes isRetriableToolError, which returns false for a +// categorized denial, so the guard took its success branch and deleted the +// record before it could key on the category. A denied tool still looped to the +// turn limit. +// +// This drives the real path: a tool that always prompts, an approver that always +// denies, and a different command each turn so the denial REASON — and therefore +// the error text — varies exactly as it does in a real run. +func TestRunStopsARepeatedlyDeniedToolAtTheFailureBound(t *testing.T) { + tool := &alwaysPromptingTool{} + registry := tools.NewRegistry() + registry.Register(tool) + + // Comfortably more turns than the bound, so reaching the bound is what stops + // the run rather than exhausting the provider or MaxTurns. + turns := make([][]zeroruntime.StreamEvent, 0, toolFailureStopAt+4) + for i := range toolFailureStopAt + 4 { + turns = append(turns, toolTurn("call-"+strconv.Itoa(i), "bash", + `{"command":"touch /etc/file`+strconv.Itoa(i)+`"}`)) + } + provider := &mockProvider{turns: turns} + + denials := 0 + result, err := Run(context.Background(), "do the thing", provider, Options{ + Registry: registry, + PermissionMode: PermissionModeAsk, + MaxTurns: len(turns) + 5, + OnPermissionRequest: func(_ context.Context, request PermissionRequest) (PermissionDecision, error) { + denials++ + // The varying half. In production this is the refused path or command; + // here it is the command, which lands in the denial message the guard + // used to key on. + return PermissionDecision{ + Action: PermissionDecisionDeny, + Reason: "refused " + request.ToolName + " call " + strconv.Itoa(denials), + }, nil + }, + }) + if err != nil { + t.Fatal(err) + } + + if denials != toolFailureStopAt { + t.Errorf("the run made %d denied calls, want it halted at %d", denials, toolFailureStopAt) + } + if tool.ran != 0 { + t.Errorf("the denied tool executed %d times; the denial must precede execution", tool.ran) + } + want := toolFailureStopAnswer("bash", toolFailureStopAt, false) + if result.FinalAnswer != want { + t.Errorf("final answer =\n %q\nwant\n %q", result.FinalAnswer, want) + } +} + +// The stop message must describe the bound that actually tripped. When the +// content-blind counter is what halts the run, the same-signature streak is +// usually 1, and reporting that would tell the user a tool failed once after it +// failed a dozen different ways. +func TestVariedFailureStopAnswerReportsTheRightCounter(t *testing.T) { + var state guardState + var outcome toolFailureOutcome + for i := 1; i <= toolFailureAnyErrorStopAt; i++ { + outcome = state.observeToolResult("bash", true, true, "distinct failure "+strconv.Itoa(i), "") + if outcome.Stop { + break + } + } + if !outcome.Stop { + t.Fatal("never stopped") + } + if !outcome.Varied { + t.Error("Varied = false for a stop driven by the content-blind counter") + } + if outcome.Count != toolFailureAnyErrorStopAt { + t.Errorf("Count = %d, want the counter that tripped (%d)", outcome.Count, toolFailureAnyErrorStopAt) + } + answer := toolFailureStopAnswer("bash", outcome.Count, outcome.Varied) + if !strings.Contains(answer, "each with a different error") { + t.Errorf("stop answer describes the wrong cause: %q", answer) + } + if strings.Contains(answer, "with the same error") { + t.Errorf("stop answer claims a same-error loop after distinct failures: %q", answer) + } +} + func TestGuardStateResetsToolOnlyStreakOnEmptyNonToolTurn(t *testing.T) { var state guardState toolOnly := zeroruntime.CollectedStream{ diff --git a/internal/agent/loop.go b/internal/agent/loop.go index 960190693..5e4245350 100644 --- a/internal/agent/loop.go +++ b/internal/agent/loop.go @@ -740,7 +740,13 @@ func Run(ctx context.Context, prompt string, provider Provider, options Options) // aren't fixed by reformatting the call, so a "match this schema" hint // would misdirect the model toward JSON shape or blocked behavior. retriableFailure := isRetriableToolError(toolResult) - outcome := guards.observeToolResult(call.Name, retriableFailure, toolResult.Output, toolResult.DenialReason) + // A categorized denial is NOT retriable — retrying it verbatim is + // pointless — but it is still a failure the streaks must count, or a + // refused tool loops until the turn limit. Passing retriableFailure for + // both is what let that happen: observeToolResult took its success + // branch and deleted the record before it could key on the category. + countedFailure := retriableFailure || toolResult.DenialReason != DenialNone + outcome := guards.observeToolResult(call.Name, countedFailure, retriableFailure, toolResult.Output, toolResult.DenialReason) posture.observeToolOutcome(outcome, toolResult) if outcome.Stop { // The assistant message advertised EVERY collected tool call, but @@ -750,7 +756,7 @@ func Run(ctx context.Context, prompt string, provider Provider, options Options) // messages stay valid for a strict provider replay (Anthropic // rejects a tool_use with no answering tool_result). messages = appendAbortedToolResults(messages, collected.ToolCalls[index+1:]) - result.FinalAnswer = toolFailureStopAnswer(call.Name, outcome.Count) + result.FinalAnswer = toolFailureStopAnswer(call.Name, outcome.Count, outcome.Varied) result.Messages = copyMessages(messages) return result, nil }