Skip to content

Commit 1b28fda

Browse files
igorcostaAutohand Evolve
andcommitted
Let the reflection guard stand down instead of stranding the turn
The reflection guard escalated a second unreflected tool call to a permanent tool ban for the rest of the turn. The assistant could no longer act even after producing the reflection it was asked for, so a turn that ended in "Let me try reading those files now." stopped there, never reading anything. Replace the permanent `force_final` escalation with a single reminder: the first violation still demands reflection, the second stands down with `proceed_unreflected` so the announced work can proceed. Repetition and blind retries remain covered by ToolLoopGuard and the tool-result integrity check. Also wire `responseCompletionHooks` onto the real react-loop host so an announced-but-unexecuted action is rejected and retried instead of being presented as the final answer, and skip that policing on recovery turns where tools were deliberately withheld. Co-authored-by: Autohand Evolve <code-noreply@autohand.ai>
1 parent 95bc0a4 commit 1b28fda

6 files changed

Lines changed: 202 additions & 38 deletions

File tree

src/core/agent.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,7 @@ import {
129129
type ReactLoopControl,
130130
type ReactLoopResult,
131131
} from './agent/ReactLoopRunner.js';
132+
import { DEFAULT_RESPONSE_COMPLETION_HOOKS } from './agent/ResponseCompletionClassifier.js';
132133
import { initializeAgentDependencies, type AgentDependencyHost } from './agent/AgentDependencyComposer.js';
133134
import {
134135
InstructionRunner,
@@ -1220,6 +1221,7 @@ export class AutohandAgent {
12201221
get contextPercentLeft() { return agent.contextPercentLeft; },
12211222
conversation: agent.conversation,
12221223
get inkRenderer() { return agent.inkRenderer as AgentReactLoopHost['inkRenderer']; },
1224+
responseCompletionHooks: DEFAULT_RESPONSE_COMPLETION_HOOKS,
12231225
get lastAssistantResponseForNotification() { return agent.lastAssistantResponseForNotification; },
12241226
set lastAssistantResponseForNotification(value) { agent.lastAssistantResponseForNotification = value; },
12251227
llm: agent.llm,

src/core/agent/ReactLoopRunner.ts

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -531,8 +531,13 @@ export async function runAgentReactLoop(
531531
}
532532
}
533533

534+
// Set whenever the loop deliberately removes tools for a recovery turn:
535+
// the assistant physically cannot emit a tool call, so announcing a next
536+
// step is narration rather than a deferred action worth rejecting.
537+
let toolsWithheldForRecovery = false;
534538
if (loopGuard.isForcingFinalResponse()) {
535539
tools = [];
540+
toolsWithheldForRecovery = true;
536541
}
537542

538543
// Use ContextOrchestrator for smart auto-compaction
@@ -571,6 +576,7 @@ export async function runAgentReactLoop(
571576
if (!integrity.ok) {
572577
loopGuard.forceFinalResponse();
573578
tools = [];
579+
toolsWithheldForRecovery = true;
574580
const integrityNote =
575581
'[Tool Result Integrity] One or more prior tool results were not available in the outbound provider payload. ' +
576582
'Tools have been disabled for this recovery response. Do not retry the calls; explain the integrity failure ' +
@@ -715,7 +721,7 @@ export async function runAgentReactLoop(
715721
completion,
716722
payload,
717723
cleanupModelResponse: host.cleanupModelResponse,
718-
responseCompletionHooks: host.responseCompletionHooks,
724+
responseCompletionHooks: toolsWithheldForRecovery ? undefined : host.responseCompletionHooks,
719725
});
720726

721727
if (turnOutcome.type === 'repair') {
@@ -858,18 +864,8 @@ export async function runAgentReactLoop(
858864
continue;
859865
}
860866

861-
if (reflectionDecision.type === 'force_final' && payload.toolCalls?.length) {
862-
loopGuard.forceFinalResponse();
863-
await recordRejectedNativeToolCalls(
864-
payload.toolCalls,
865-
'Tool call not executed: repeated missing reflection forced a final response.',
866-
);
867-
host.conversation.addSystemNote(
868-
'[Critical Reflection Guard] The assistant attempted another tool call without analyzing prior results. ' +
869-
'The call was not executed. Do not call tools again; provide a finalResponse from the available evidence.'
870-
);
871-
expectedOutboundToolResultIds = currentAssistantToolCallIds;
872-
continue;
867+
if (reflectionDecision.type === 'proceed_unreflected' && debugMode) {
868+
host.writeDebugLine('[AGENT DEBUG] Reflection guard exhausted its reminder; letting the tool call proceed');
873869
}
874870

875871
if (payload.toolCalls && payload.toolCalls.length > 0) {

src/core/agent/ToolLoopPolicy.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ export interface ToolLoopResultDecision {
3030
export type ToolReflectionDecision =
3131
| { type: 'allow' }
3232
| { type: 'require_reflection' }
33-
| { type: 'force_final' }
33+
| { type: 'proceed_unreflected' }
3434
| { type: 'integrity_failure' };
3535

3636
export interface ToolLoopGuardOptions {
@@ -204,9 +204,14 @@ export class ToolReflectionGuard {
204204
return { type: 'require_reflection' };
205205
}
206206

207+
// The reminder was already delivered once. Blocking again would strand the
208+
// turn: the assistant loses tool access, cannot perform the work it just
209+
// announced, and the user gets a progress note instead of an answer.
210+
// Repetition and blind retries stay covered by ToolLoopGuard and the
211+
// tool-result integrity check, so step aside and let the work proceed.
207212
this.awaitingReflection = false;
208213
this.violationCount = 0;
209-
return { type: 'force_final' };
214+
return { type: 'proceed_unreflected' };
210215
}
211216
}
212217

src/core/agents/SubAgent.ts

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -329,19 +329,6 @@ export class SubAgent {
329329
continue;
330330
}
331331

332-
if (reflectionDecision.type === 'force_final') {
333-
loopGuard.forceFinalResponse();
334-
this.recordRejectedNativeToolCalls(
335-
payload.toolCalls,
336-
'Tool call not executed: repeated missing reflection forced a final response.',
337-
);
338-
this.conversation.addSystemNote(
339-
'[Critical Reflection Guard] Another tool call was attempted without analyzing prior results. '
340-
+ 'The call was not executed. Do not call tools again; provide the final answer.'
341-
);
342-
continue;
343-
}
344-
345332
const decision = loopGuard.observeCalls(payload.toolCalls);
346333
if (decision.type !== 'allow') {
347334
this.recordRejectedNativeToolCalls(

tests/core/agent.reflection.spec.ts

Lines changed: 172 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,10 @@ import { AutohandAgent } from '../../src/core/agent.js';
1414
import { ReactionParser } from '../../src/core/agent/ReactionParser.js';
1515
import { runAgentReactLoop } from '../../src/core/agent/ReactLoopRunner.js';
1616
import { ToolReflectionGuard } from '../../src/core/agent/ToolLoopPolicy.js';
17+
import {
18+
DEFAULT_RESPONSE_COMPLETION_HOOKS,
19+
type ResponseCompletionHook,
20+
} from '../../src/core/agent/ResponseCompletionClassifier.js';
1721
import type {
1822
AgentRuntime,
1923
AssistantReactPayload,
@@ -45,7 +49,10 @@ function createNativeToolCall(id: string, name = 'read_file', args: Record<strin
4549
};
4650
}
4751

48-
function createReactLoopHarness(completions: LLMResponse[]) {
52+
function createReactLoopHarness(
53+
completions: LLMResponse[],
54+
harnessOptions: { responseCompletionHooks?: readonly ResponseCompletionHook[] } = {},
55+
) {
4956
const parser = createParser();
5057
const messages: LLMMessage[] = [{ role: 'user', content: 'check reflection' }];
5158
const systemNotes: string[] = [];
@@ -69,6 +76,9 @@ function createReactLoopHarness(completions: LLMResponse[]) {
6976

7077
const host = {
7178
activeProvider: 'openai' as const,
79+
...(harnessOptions.responseCompletionHooks
80+
? { responseCompletionHooks: harnessOptions.responseCompletionHooks }
81+
: {}),
7282
autoReportManager: { reportError: vi.fn(async () => {}) },
7383
consecutiveCancellations: 0,
7484
contextOrchestrator: {
@@ -383,15 +393,29 @@ describe('Reflection loop guard logic', () => {
383393
expect(needsReflection).toBe(false);
384394
});
385395

386-
it('forces a final response after the reflection violation limit is exceeded', () => {
396+
it('stops blocking after one reminder instead of stranding the turn', () => {
387397
const guard = new ToolReflectionGuard();
388398
const payload: AssistantReactPayload = {
389399
toolCalls: [{ tool: 'read_file', args: { path: 'a.ts' } }]
390400
};
391401
guard.expectReflection();
392402

393403
expect(guard.evaluate(payload)).toEqual({ type: 'require_reflection' });
394-
expect(guard.evaluate(payload)).toEqual({ type: 'force_final' });
404+
expect(guard.evaluate(payload)).toEqual({ type: 'proceed_unreflected' });
405+
});
406+
407+
it('resets after standing down so a later reminder still fires once', () => {
408+
const guard = new ToolReflectionGuard();
409+
const payload: AssistantReactPayload = {
410+
toolCalls: [{ tool: 'read_file', args: { path: 'a.ts' } }]
411+
};
412+
413+
guard.expectReflection();
414+
expect(guard.evaluate(payload)).toEqual({ type: 'require_reflection' });
415+
expect(guard.evaluate(payload)).toEqual({ type: 'proceed_unreflected' });
416+
417+
guard.expectReflection();
418+
expect(guard.evaluate(payload)).toEqual({ type: 'require_reflection' });
395419
});
396420

397421
it('does not trigger guard on first iteration (no prior tool results)', () => {
@@ -505,7 +529,7 @@ describe('Reflection guard integration', () => {
505529
expect(emittedMessages).toContain('Stopped after reminder.');
506530
});
507531

508-
it('forces a tool-free response after a second unreflected follow-up attempt', async () => {
532+
it('blocks the follow-up call once and then lets the next attempt through', async () => {
509533
const { host, systemNotes, executedCalls, emittedMessages, complete } = createReactLoopHarness([
510534
{
511535
content: 'Initial lookup',
@@ -517,19 +541,20 @@ describe('Reflection guard integration', () => {
517541
},
518542
{
519543
content: 'still short',
520-
toolCalls: [createNativeToolCall('call_3', 'read_file', { path: 'blocked-twice.ts' })],
544+
toolCalls: [createNativeToolCall('call_3', 'read_file', { path: 'allowed-after-reminder.ts' })],
521545
},
522546
{
523-
content: '{"finalResponse":"Stopped after two missing reflections."}',
547+
content: '{"finalResponse":"Finished after one reflection reminder."}',
524548
},
525549
]);
526550

527551
await runAgentReactLoop(host, new AbortController());
528552

529-
expect(executedCalls.map((call) => call.id)).toEqual(['call_1']);
530-
expect(systemNotes.some((note) => note.startsWith('[Critical Reflection Guard]'))).toBe(true);
531-
expect(complete.mock.calls[3]?.[0]?.tools).toBeUndefined();
532-
expect(emittedMessages).toContain('Stopped after two missing reflections.');
553+
expect(executedCalls.map((call) => call.id)).toEqual(['call_1', 'call_3']);
554+
expect(systemNotes.some((note) => note.startsWith('[Reflection Required]'))).toBe(true);
555+
expect(systemNotes.some((note) => note.startsWith('[Critical Reflection Guard]'))).toBe(false);
556+
expect(complete.mock.calls[3]?.[0]?.tools).toBeDefined();
557+
expect(emittedMessages).toContain('Finished after one reflection reminder.');
533558
});
534559

535560
it('treats a missing-tool-output reflection as an integrity failure instead of re-running tools', async () => {
@@ -634,3 +659,140 @@ describe('System prompt includes reflection instructions', () => {
634659
expect(prompt).toContain('Reason + Reflect + Act');
635660
});
636661
});
662+
663+
/* ── Regression: reflection guard must not strand a turn ──── */
664+
665+
/**
666+
* Reported symptom: the agent printed a reflection ending in "Let me try
667+
* reading those files now." and then stopped, never reading anything.
668+
*
669+
* Two independent defects produced it:
670+
* 1. The reflection guard escalated to a permanent tool ban, so the assistant
671+
* could no longer act even after it produced the reflection it was asked
672+
* for.
673+
* 2. `responseCompletionHooks` was never wired onto the real react-loop host,
674+
* so an announced-but-unexecuted action was rendered as the final answer
675+
* instead of being rejected and retried.
676+
*/
677+
describe('Reflection guard dead-end regression', () => {
678+
it('lets the assistant keep working after a second unreflected tool call', async () => {
679+
const { host, systemNotes, executedCalls, emittedMessages, complete } = createReactLoopHarness([
680+
{
681+
content: 'Initial lookup',
682+
toolCalls: [createNativeToolCall('call_1', 'read_file', { path: 'first.ts' })],
683+
},
684+
{
685+
content: 'short',
686+
toolCalls: [createNativeToolCall('call_2', 'read_file', { path: 'reminded.ts' })],
687+
},
688+
{
689+
content: 'still short',
690+
toolCalls: [createNativeToolCall('call_3', 'read_file', { path: 'recovered.ts' })],
691+
},
692+
{
693+
content: '{"finalResponse":"Both files read."}',
694+
},
695+
]);
696+
697+
await runAgentReactLoop(host, new AbortController());
698+
699+
expect(executedCalls.map((call) => call.args?.path)).toEqual(['first.ts', 'recovered.ts']);
700+
expect(systemNotes.some((note) => note.startsWith('[Reflection Required]'))).toBe(true);
701+
expect(systemNotes.some((note) => note.startsWith('[Critical Reflection Guard]'))).toBe(false);
702+
expect(complete.mock.calls[2]?.[0]?.tools).toBeDefined();
703+
expect(emittedMessages).toContain('Both files read.');
704+
});
705+
706+
it('does not ban tools for the rest of the turn once a reminder is ignored', async () => {
707+
const { host, complete, emittedMessages } = createReactLoopHarness([
708+
{
709+
content: 'Initial lookup',
710+
toolCalls: [createNativeToolCall('call_1', 'read_file', { path: 'first.ts' })],
711+
},
712+
{
713+
content: 'short',
714+
toolCalls: [createNativeToolCall('call_2', 'read_file', { path: 'reminded.ts' })],
715+
},
716+
{
717+
content: 'still short',
718+
toolCalls: [createNativeToolCall('call_3', 'read_file', { path: 'recovered.ts' })],
719+
},
720+
{
721+
content: '{"reflection":"Both files described the delegator.","thought":"Now I can answer."}',
722+
toolCalls: [createNativeToolCall('call_4', 'read_file', { path: 'follow-up.ts' })],
723+
},
724+
{
725+
content: '{"finalResponse":"Answered after recovering."}',
726+
},
727+
]);
728+
729+
await runAgentReactLoop(host, new AbortController());
730+
731+
for (const call of complete.mock.calls) {
732+
expect(call[0]?.tools).toBeDefined();
733+
}
734+
expect(emittedMessages).toContain('Answered after recovering.');
735+
});
736+
737+
it('rejects an announced-but-unexecuted action instead of presenting it as the answer', async () => {
738+
const announcement = [
739+
"I need to stop and reflect on what I've gathered so far before proceeding.",
740+
'',
741+
'I was trying to read the actual implementation files but the tool calls were blocked.',
742+
'I need to read `src/commands/agents.ts` and `src/core/agents/AgentDelegator.ts` implementation',
743+
'before I can plan the "kill/stop" feature.',
744+
'',
745+
'Let me try reading those files now.',
746+
].join('\n');
747+
748+
const { host, systemNotes, executedCalls, emittedMessages } = createReactLoopHarness(
749+
[
750+
{ content: announcement },
751+
{
752+
content: 'Reading the delegator now.',
753+
toolCalls: [createNativeToolCall('call_1', 'read_file', { path: 'AgentDelegator.ts' })],
754+
},
755+
{ content: '{"finalResponse":"`/agents` delegates through AgentDelegator."}' },
756+
],
757+
{ responseCompletionHooks: DEFAULT_RESPONSE_COMPLETION_HOOKS },
758+
);
759+
760+
await runAgentReactLoop(host, new AbortController());
761+
762+
expect(systemNotes.some((note) => note.includes('announced an action but emitted no tool calls'))).toBe(true);
763+
expect(executedCalls.map((call) => call.args?.path)).toEqual(['AgentDelegator.ts']);
764+
expect(emittedMessages).toContain('`/agents` delegates through AgentDelegator.');
765+
expect(emittedMessages).not.toContain(announcement);
766+
});
767+
768+
it('does not police announced actions on a turn where tools were withheld', async () => {
769+
const { host, systemNotes, emittedMessages } = createReactLoopHarness(
770+
[
771+
{
772+
content: 'Initial lookup',
773+
toolCalls: [createNativeToolCall('call_1', 'read_file', { path: 'same.ts' })],
774+
},
775+
{
776+
content: '{"reflection":"The previous tool outputs weren\'t visible in my context.","thought":"I should retry."}',
777+
toolCalls: [createNativeToolCall('call_2', 'read_file', { path: 'same.ts' })],
778+
},
779+
{ content: 'I need to read the file again before I can answer.' },
780+
],
781+
{ responseCompletionHooks: DEFAULT_RESPONSE_COMPLETION_HOOKS },
782+
);
783+
784+
await runAgentReactLoop(host, new AbortController());
785+
786+
expect(systemNotes.some((note) => note.startsWith('[Tool Result Integrity]'))).toBe(true);
787+
expect(systemNotes.some((note) => note.includes('announced an action but emitted no tool calls'))).toBe(false);
788+
expect(emittedMessages).toContain('I need to read the file again before I can answer.');
789+
});
790+
791+
it('wires the response-completion hooks onto the real react-loop host', () => {
792+
const agent = createMinimalAgent();
793+
const host = agent.createReactLoopHost();
794+
795+
expect(host.responseCompletionHooks).toEqual(DEFAULT_RESPONSE_COMPLETION_HOOKS);
796+
expect(host.responseCompletionHooks.length).toBeGreaterThan(0);
797+
});
798+
});

tests/core/agent/ResponseCompletionClassifier.test.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,18 @@ describe('ResponseCompletionClassifier', () => {
8585
'Promise to answer later',
8686
'I now have a comprehensive understanding of the repository. Let me provide a clear summary to the user.',
8787
],
88+
[
89+
'Reflection that ends on an unexecuted read',
90+
[
91+
"I need to stop and reflect on what I've gathered so far before proceeding.",
92+
'',
93+
'I was trying to read the actual implementation files but the tool calls were blocked.',
94+
'I need to read `src/commands/agents.ts` and `src/core/agents/AgentDelegator.ts` implementation',
95+
'before I can plan the "kill/stop" feature.',
96+
'',
97+
'Let me try reading those files now.',
98+
].join('\n'),
99+
],
88100
])('classifies %s as invalid deferred action', (_name, response) => {
89101
const result = classifyResponseCompletion({ response });
90102

0 commit comments

Comments
 (0)