Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 13 additions & 3 deletions packages/cli/src/nonInteractiveCli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,12 @@ const LOOP_TYPE_LABELS: Record<LoopType, string> = {
'the model spent too many consecutive calls reading files without making progress',
[LoopType.ACTION_STAGNATION]:
'the model kept calling the same tool without making progress',
[LoopType.GLOBAL_TOOL_CALL_DUPLICATE]:
'the model repeated the same tool call across the turn, even when not back-to-back',
[LoopType.ALTERNATING_TOOL_CALL_PATTERN]:
'the model alternated between the same two tool calls in a repeating pattern',
Comment thread
wenshao marked this conversation as resolved.
[LoopType.TURN_TOOL_CALL_CAP]:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The message here is correct, but the headless caller still needs to stop the current run, not just print the message. In the tmux headless smoke test, LoopDetected was emitted, but the already-collected toolCallRequests array still flowed into processToolCallBatch(...), so the first 100 streamed read_file calls were still dispatched before the run halted.

'the model exceeded the maximum number of tool calls allowed in a single turn',
};

function emitLoopDetectedMessage(
Expand All @@ -122,9 +128,13 @@ function emitLoopDetectedMessage(
}
const reason = loopType ? LOOP_TYPE_LABELS[loopType] : undefined;
const detail = reason ? ` (${loopType}: ${reason})` : '';
process.stderr.write(
`Loop detection halted the run${detail}. Set the \`model.skipLoopDetection\` setting to true to disable.\n`,
);
// The turn cap runs before the skipLoopDetection gate, so that setting can't
// disable it — don't suggest it for TURN_TOOL_CALL_CAP.
const hint =
loopType === LoopType.TURN_TOOL_CALL_CAP
? ' This is an always-on per-turn tool-call cap and cannot be disabled via `model.skipLoopDetection`.'
: ' Set the `model.skipLoopDetection` setting to true to disable.';
process.stderr.write(`Loop detection halted the run${detail}.${hint}\n`);
}

/**
Expand Down
80 changes: 79 additions & 1 deletion packages/core/src/core/client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4286,6 +4286,82 @@ hello
expect(client['pendingMemoryPrefetch']).toBeUndefined();
});

it('should halt via the always-on turn cap before the skipLoopDetection gate', async () => {
let abortHandlerInvoked = false;
mockMemoryManager.recall.mockImplementation((_root, _query, opts) => {
opts.abortSignal?.addEventListener('abort', () => {
abortHandlerInvoked = true;
});
return new Promise(() => {});
});

const mockChat: Partial<GeminiChat> = {
addHistory: vi.fn(),
getHistory: vi.fn().mockReturnValue([]),
getHistoryLength: vi.fn().mockReturnValue(0),
};
client['chat'] = mockChat as GeminiChat;

// The always-on cap trips on the first event — it runs before (and
// independently of) the gated detectors.
const loopDetector = client['loopDetector'];
const alwaysOnSpy = vi
.spyOn(loopDetector, 'checkAlwaysOnSafeties')
.mockReturnValue(true);
const deterministicSpy = vi.spyOn(
loopDetector,
'addAndCheckDeterministicToolCallLoop',
);
vi.spyOn(loopDetector, 'getLastLoopType').mockReturnValue(
LoopType.TURN_TOOL_CALL_CAP,
);

// `run` is invoked as `turn.run(...)`, so `this` is the live Turn —
// populate pendingToolCalls the way the real Turn.run does as it streams
// ToolCallRequest chunks, so the halt's clear runs against a non-empty
// array (not a trivially-empty one).
mockTurnRunFn.mockImplementation(async function* (this: {
pendingToolCalls: unknown[];
}) {
this.pendingToolCalls.push(
{ name: 'read_file', args: { path: 'a.ts' } },
{ name: 'read_file', args: { path: 'b.ts' } },
);
yield { type: 'content', value: 'looping' };
});

const stream = client.sendMessageStream(
[{ text: 'trigger the cap' }],
new AbortController().signal,
'prompt-id-cap',
{ type: SendMessageType.UserQuery },
);
const events = [];
let result = await stream.next();
while (!result.done) {
events.push(result.value);
result = await stream.next();
}
const returnedTurn = result.value as
| { pendingToolCalls: unknown[] }
| undefined;

// Always-on cap fires and short-circuits before the gated detectors run.
expect(alwaysOnSpy).toHaveBeenCalled();
expect(deterministicSpy).not.toHaveBeenCalled();
const loopEvent = events.find(
(e) => e.type === GeminiEventType.LoopDetected,
);
expect(loopEvent?.value?.loopType).toBe(LoopType.TURN_TOOL_CALL_CAP);
// The two pending calls collected before the cap tripped are dropped, so
// the halt doesn't spawn a continuation that re-trips the cap and
// double-prints the message.
expect(returnedTurn?.pendingToolCalls).toHaveLength(0);
// The mid-stream memory prefetch is cancelled.
expect(abortHandlerInvoked).toBe(true);
expect(client['pendingMemoryPrefetch']).toBeUndefined();
});

it('should PRESERVE the pending prefetch when next-speaker continueTurn returns', async () => {
// Self-inflicted-regression guard for the round-4 finding:
// the bottom-of-try `normalCompletion = true` doesn't cover the
Expand Down Expand Up @@ -6182,6 +6258,7 @@ Other open files:

// Replace loop detector with spies
const ldMock = {
checkAlwaysOnSafeties: vi.fn().mockReturnValue(false),
Comment thread
wenshao marked this conversation as resolved.
addAndCheckDeterministicToolCallLoop: vi.fn().mockReturnValue(false),
addAndCheckHeuristicLoops: vi.fn().mockReturnValue(false),
reset: vi.fn(),
Expand Down Expand Up @@ -6211,7 +6288,8 @@ Other open files:
// consume stream
}

// Assert - neither detector path runs when skipLoopDetection is true
// Assert - always-on safeties still run, but opt-in detectors don't
expect(ldMock.checkAlwaysOnSafeties).toHaveBeenCalled();
expect(
ldMock.addAndCheckDeterministicToolCallLoop,
).not.toHaveBeenCalled();
Expand Down
25 changes: 25 additions & 0 deletions packages/core/src/core/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2123,6 +2123,31 @@ export class GeminiClient {
didUpdateIdeContextState = true;
}

// Always-on safety checks (turn tool-call cap). These fire before
// the skipLoopDetection gate so they cannot be bypassed by
// configuration.
const alwaysOnLoop = this.loopDetector.checkAlwaysOnSafeties(event);
if (alwaysOnLoop) {
// The tripping response may carry several tool calls collected
// before the cap fired. Drop them so the run halts here instead of
// executing them, spawning a continuation, and re-tripping the cap
// (which would double-print the halt message and waste a request).
turn.pendingToolCalls.length = 0;
const loopType = this.loopDetector.getLastLoopType();
yield {
type: GeminiEventType.LoopDetected,
...(loopType && { value: { loopType } }),
};
if (arenaAgentClient) {
await arenaAgentClient.reportError('Loop detected');
}
this.lastApiCompletionTimestamp = Date.now();
if (isTopLevelInteraction)
endInteractionSpan('error', { errorMessage: 'loop detected' });
this.cancelPendingMemoryPrefetch();
return turn;
Comment thread
wenshao marked this conversation as resolved.
}

// Loop detection is opt-in: `model.skipLoopDetection` defaults to true
// (see settingsSchema) to avoid false-positive interruptions. Keep BOTH
// the deterministic identical-tool-call check and the heuristic checks
Expand Down
Loading
Loading