Skip to content

Stop rejected actions from looping forever before dispatch #3116

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
webbrain-one merged 3 commits into from
Sep 30, 2026
Merged

Stop rejected actions from looping forever before dispatch #3116

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cbf85bc
Stop rejected actions from looping forever before dispatch
esokullu Sep 30, 2026
b90be2d
Merge pull request #386 from esokullu/fix/rejected-action-loop-recovery
webbrain-one Sep 30, 2026
fe86ee6
Canonicalize tool-arg loop keys so key order cannot dodge the limit
Sep 30, 2026
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
111 changes: 47 additions & 64 deletions src/chrome/src/agent/agent.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11828,6 +11828,46 @@ Rules: no prose intro, no conclusion, no "this screenshot shows...", no layout d
)
);

// Rejections before dispatch still consume attempts. Returning the error
// without loop accounting lets invalid coordinates/arguments retry forever.
const recordPreparationFailure = async (toolIndex, fnName, fnArgs, result, warning = '') => {
let loopCheck = { kind: 'none' };
const loopKey = this._loopCallKey(fnName, fnArgs, result);
const failedApiMutation = this._isFailedApiMutationForLoop(fnName, fnArgs, result);
if (!failedApiMutation || !failedApiMutationLoopKeysThisBatch.has(loopKey)) {
if (failedApiMutation) failedApiMutationLoopKeysThisBatch.add(loopKey);
loopCheck = this._checkLoop(tabId, fnName, fnArgs, result);
Comment thread
esokullu marked this conversation as resolved.
}
const tc = toolCalls[toolIndex];
onUpdate('tool_call', { name: fnName, args: fnArgs, outcomeUnknown: false });
onUpdate('tool_result', { name: fnName, result });
messages.push({
role: 'tool',
tool_call_id: tc.id,
content: JSON.stringify(result)
+ (loopCheck.kind === 'nudge' ? `\n${loopCheck.warning}` : ''),
});
const runId = this.currentRunId.get(tabId);
if (runId) {
try {
await trace.recordToolCall(runId, step, { name: fnName, args: fnArgs, result, latencyMs: 0 });
} catch {}
}
if (warning) onUpdate('warning', { message: warning });
if (loopCheck.kind === 'nudge') {
onUpdate('warning', { message: 'Loop detected — nudging the agent.' });
}
if (loopCheck.kind !== 'stop') return null;
this._appendSyntheticToolResults(
tabId, toolCalls, toolIndex + 1, messages, onUpdate, step,
() => ({ success: false, skipped: true, error: 'skipped: run stopped by loop detector' }),
);
if (runId) trace.recordError(runId, step, 'loop', loopCheck.message);
this._clearLoopState(tabId);
this._persist(tabId);
return { action: 'recover', value: loopCheck.message, status: 'loop_stopped' };
};

for (let toolIndex = 0; toolIndex < toolCalls.length; toolIndex++) {
const tc = toolCalls[toolIndex];
const callState = { index: toolIndex, invoked: false, consequential: false, result: null, dispatchState: null };
Expand Down Expand Up @@ -11865,21 +11905,8 @@ Rules: no prose intro, no conclusion, no "this screenshot shows...", no layout d
const parsedArgs = this._parseToolCallArgs(tc);
if (parsedArgs.error) {
const result = this._invalidToolArgumentsResult(fnName, parsedArgs);
messages.push({
role: 'tool',
tool_call_id: tc.id,
content: JSON.stringify(result),
});
onUpdate('warning', { message: result.error });
const runId = this.currentRunId.get(tabId);
if (runId) {
trace.recordToolCall(runId, step, {
name: fnName,
args: {},
result,
latencyMs: 0,
});
}
const recovery = await recordPreparationFailure(toolIndex, fnName, {}, result, result.error);
if (recovery) return recovery;
if (interruptFailedBrowserAction(toolIndex, fnName)) { navNotices.length = 0; break; }
continue;
}
Expand Down Expand Up @@ -11911,12 +11938,8 @@ Rules: no prose intro, no conclusion, no "this screenshot shows...", no layout d
: null;
const preparationFailure = argumentValidation.ok ? coordinates.block : argumentValidation.result;
if (preparationFailure) {
const result = preparationFailure;
onUpdate('tool_call', { name: fnName, args: fnArgs, outcomeUnknown: false });
onUpdate('tool_result', { name: fnName, result });
messages.push({ role: 'tool', tool_call_id: tc.id, content: JSON.stringify(result) });
const runId = this.currentRunId.get(tabId);
if (runId) trace.recordToolCall(runId, step, { name: fnName, args: fnArgs, result, latencyMs: 0 });
const recovery = await recordPreparationFailure(toolIndex, fnName, fnArgs, preparationFailure);
if (recovery) return recovery;
if (interruptFailedBrowserAction(toolIndex, fnName)) { navNotices.length = 0; break; }
continue;
}
Expand All @@ -11938,50 +11961,10 @@ Rules: no prose intro, no conclusion, no "this screenshot shows...", no layout d

const recordPagePreparationTimeout = async (error, stage) => {
const timeoutResult = this._contentActionPreparationTimeoutResult(fnName, error, stage);
let loopCheck = { kind: 'none' };
const loopKey = this._loopCallKey(fnName, fnArgs, timeoutResult);
const failedApiMutation = this._isFailedApiMutationForLoop(fnName, fnArgs, timeoutResult);
if (!failedApiMutation || !failedApiMutationLoopKeysThisBatch.has(loopKey)) {
if (failedApiMutation) failedApiMutationLoopKeysThisBatch.add(loopKey);
loopCheck = this._checkLoop(tabId, fnName, fnArgs, timeoutResult);
}
onUpdate('tool_call', { name: fnName, args: fnArgs, outcomeUnknown: false });
onUpdate('tool_result', { name: fnName, result: timeoutResult });
messages.push({
role: 'tool',
tool_call_id: tc.id,
content: JSON.stringify(timeoutResult)
+ (loopCheck.kind === 'nudge' ? `\n${loopCheck.warning}` : ''),
});
const runId = this.currentRunId.get(tabId);
if (runId) {
try {
await trace.recordToolCall(runId, step, {
name: fnName,
args: fnArgs,
result: timeoutResult,
latencyMs: 0,
});
} catch {}
}
onUpdate('warning', { message: 'Page action preparation timed out before dispatch.' });
if (loopCheck.kind === 'nudge') {
onUpdate('warning', { message: 'Loop detected — nudging the agent.' });
}
if (loopCheck.kind !== 'stop') return null;
this._appendSyntheticToolResults(
tabId,
toolCalls,
toolIndex + 1,
messages,
onUpdate,
step,
() => ({ success: false, skipped: true, error: 'skipped: run stopped by loop detector' }),
return recordPreparationFailure(
toolIndex, fnName, fnArgs, timeoutResult,
'Page action preparation timed out before dispatch.',
);
if (runId) trace.recordError(runId, step, 'loop', loopCheck.message);
this._clearLoopState(tabId);
this._persist(tabId);
return { action: 'recover', value: loopCheck.message, status: 'loop_stopped' };
};
const detectSubmitWithDeadline = () => this._withContentActionDeadline(
async abortSignal => {
Expand Down
49 changes: 47 additions & 2 deletions src/chrome/src/agent/loop-bucket.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,9 +157,54 @@ function _ghResourceBucket(host, path) {
return null; // not a GitHub host
}

/**
* Deterministic JSON with object keys in sorted order.
*
* `JSON.stringify` preserves insertion order, so `{selector:"#a",text:"x"}`
* and `{text:"x",selector:"#a"}` — the same rejected call — hash to two
* different loop keys. A model that keeps re-emitting the same invalid
* argument object can then permute key order on every attempt and never
* reach the rejection limit. Sorting keys collapses those variants into one
* identity. Array order is preserved because element order is meaningful.
*
* `undefined`/function/symbol values are dropped from objects and become
* `null` inside arrays, exactly as `JSON.stringify` does, so an absent
* argument and an explicitly undefined one stay one identity.
*/
function canonicalJson(value, seen = new Set()) {
if (value === null) return 'null';
if (typeof value !== 'object') return _canonicalScalar(value);
if (seen.has(value)) return '"[circular]"';
seen.add(value);
let encoded;
if (Array.isArray(value)) {
encoded = `[${value.map(entry => canonicalJson(entry, seen)).join(',')}]`;
} else {
const members = [];
for (const key of Object.keys(value).sort()) {
if (_isOmittable(value[key])) continue;
members.push(`${JSON.stringify(key)}:${canonicalJson(value[key], seen)}`);
}
encoded = `{${members.join(',')}}`;
}
seen.delete(value);
return encoded;
}

function _isOmittable(value) {
return value === undefined || typeof value === 'function' || typeof value === 'symbol';
}

function _canonicalScalar(value) {
if (typeof value === 'bigint') return JSON.stringify(value.toString());
if (typeof value === 'number') return Number.isFinite(value) ? JSON.stringify(value) : 'null';
return JSON.stringify(value) ?? 'null';
}

/**
* Build the loop-detector key for a tool call. URL-family tools bucket
* by resource + method; other tools fall back to exact JSON args.
* by resource + method; other tools fall back to key-order-independent
* JSON args.
*
* Returns the args-portion of the loop key. Caller appends `|name|errored`.
*/
Expand All @@ -171,7 +216,7 @@ export function bucketArgsKey(name, args) {
const fetchTextWindow = name === 'fetch_url' ? _fetchTextWindowKey(args) : '';
return `url:${bucket}|${method}${pageSourceRange}${fetchTextWindow}`;
}
return JSON.stringify(args || {});
return canonicalJson(args || {});
}

function _pageSourceRangeKey(args) {
Expand Down
16 changes: 15 additions & 1 deletion src/chrome/src/agent/loop-detector.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ export class LoopDetector {
// interleaving reads cannot evade the generic loop detector.
this.noProgressScrolls = new Map(); // tabId -> { key, count }
// Separate buffer for coordinate-based click attempts. The general loop
// detector keys on JSON.stringify(args), so when the model interleaves
// detector keys on the exact argument bucket, so when the model interleaves
// execute_js with different code strings between clicks, the same
// (x,y) click never accumulates to the threshold inside its window.
// This buffer tracks ONLY coord clicks and survives any amount of
Expand Down Expand Up @@ -646,6 +646,20 @@ export class LoopDetector {
};
}
} else if (toolResult?.success === true && toolResult?.verified !== false) {
// A verified click can recover via coordinates, a selector, or an AX
// target. Its success must retire the shared preparation failures;
// reads, fresh captures, and dispatch-only success are not progress.
if (
['click', 'click_ax', 'iframe_click'].includes(toolName)
&& toolResult.verified === true
&& toolResult.noDispatch !== true
&& toolResult.dispatched !== false
&& toolResult.outcomeUnknown !== true
&& toolResult.inconclusive !== true
) {
failures.delete('screenshot-coordinate-capture');
failures.delete('coordinate-provenance');
}
for (const scope of equivalentFailureScopes) failures.delete(scope);
if (failures.size) this.failedActionLoops.set(tabId, failures);
else this.failedActionLoops.delete(tabId);
Expand Down
111 changes: 47 additions & 64 deletions src/firefox/src/agent/agent.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10494,6 +10494,46 @@ Rules: no prose intro, no conclusion, no "this screenshot shows...", no layout d
)
);

// Rejections before dispatch still consume attempts. Returning the error
// without loop accounting lets invalid coordinates/arguments retry forever.
const recordPreparationFailure = async (toolIndex, fnName, fnArgs, result, warning = '') => {
let loopCheck = { kind: 'none' };
const loopKey = this._loopCallKey(fnName, fnArgs, result);
const failedApiMutation = this._isFailedApiMutationForLoop(fnName, fnArgs, result);
if (!failedApiMutation || !failedApiMutationLoopKeysThisBatch.has(loopKey)) {
if (failedApiMutation) failedApiMutationLoopKeysThisBatch.add(loopKey);
loopCheck = this._checkLoop(tabId, fnName, fnArgs, result);
Comment thread
esokullu marked this conversation as resolved.
}
const tc = toolCalls[toolIndex];
onUpdate('tool_call', { name: fnName, args: fnArgs, outcomeUnknown: false });
onUpdate('tool_result', { name: fnName, result });
messages.push({
role: 'tool',
tool_call_id: tc.id,
content: JSON.stringify(result)
+ (loopCheck.kind === 'nudge' ? `\n${loopCheck.warning}` : ''),
});
const runId = this.currentRunId.get(tabId);
if (runId) {
try {
await trace.recordToolCall(runId, step, { name: fnName, args: fnArgs, result, latencyMs: 0 });
} catch {}
}
if (warning) onUpdate('warning', { message: warning });
if (loopCheck.kind === 'nudge') {
onUpdate('warning', { message: 'Loop detected — nudging the agent.' });
}
if (loopCheck.kind !== 'stop') return null;
this._appendSyntheticToolResults(
tabId, toolCalls, toolIndex + 1, messages, onUpdate, step,
() => ({ success: false, skipped: true, error: 'skipped: run stopped by loop detector' }),
);
if (runId) trace.recordError(runId, step, 'loop', loopCheck.message);
this._clearLoopState(tabId);
this._persist(tabId);
return { action: 'recover', value: loopCheck.message, status: 'loop_stopped' };
};

for (let toolIndex = 0; toolIndex < toolCalls.length; toolIndex++) {
const tc = toolCalls[toolIndex];
const callState = { index: toolIndex, invoked: false, consequential: false, result: null, dispatchState: null };
Expand Down Expand Up @@ -10529,21 +10569,8 @@ Rules: no prose intro, no conclusion, no "this screenshot shows...", no layout d
const parsedArgs = this._parseToolCallArgs(tc);
if (parsedArgs.error) {
const result = this._invalidToolArgumentsResult(fnName, parsedArgs);
messages.push({
role: 'tool',
tool_call_id: tc.id,
content: JSON.stringify(result),
});
onUpdate('warning', { message: result.error });
const runId = this.currentRunId.get(tabId);
if (runId) {
trace.recordToolCall(runId, step, {
name: fnName,
args: {},
result,
latencyMs: 0,
});
}
const recovery = await recordPreparationFailure(toolIndex, fnName, {}, result, result.error);
if (recovery) return recovery;
if (interruptFailedBrowserAction(toolIndex, fnName)) { navNotices.length = 0; break; }
continue;
}
Expand Down Expand Up @@ -10575,12 +10602,8 @@ Rules: no prose intro, no conclusion, no "this screenshot shows...", no layout d
: null;
const preparationFailure = argumentValidation.ok ? coordinates.block : argumentValidation.result;
if (preparationFailure) {
const result = preparationFailure;
onUpdate('tool_call', { name: fnName, args: fnArgs, outcomeUnknown: false });
onUpdate('tool_result', { name: fnName, result });
messages.push({ role: 'tool', tool_call_id: tc.id, content: JSON.stringify(result) });
const runId = this.currentRunId.get(tabId);
if (runId) trace.recordToolCall(runId, step, { name: fnName, args: fnArgs, result, latencyMs: 0 });
const recovery = await recordPreparationFailure(toolIndex, fnName, fnArgs, preparationFailure);
if (recovery) return recovery;
if (interruptFailedBrowserAction(toolIndex, fnName)) { navNotices.length = 0; break; }
continue;
}
Expand All @@ -10593,50 +10616,10 @@ Rules: no prose intro, no conclusion, no "this screenshot shows...", no layout d

const recordPagePreparationTimeout = async (error, stage) => {
const timeoutResult = this._contentActionPreparationTimeoutResult(fnName, error, stage);
let loopCheck = { kind: 'none' };
const loopKey = this._loopCallKey(fnName, fnArgs, timeoutResult);
const failedApiMutation = this._isFailedApiMutationForLoop(fnName, fnArgs, timeoutResult);
if (!failedApiMutation || !failedApiMutationLoopKeysThisBatch.has(loopKey)) {
if (failedApiMutation) failedApiMutationLoopKeysThisBatch.add(loopKey);
loopCheck = this._checkLoop(tabId, fnName, fnArgs, timeoutResult);
}
onUpdate('tool_call', { name: fnName, args: fnArgs, outcomeUnknown: false });
onUpdate('tool_result', { name: fnName, result: timeoutResult });
messages.push({
role: 'tool',
tool_call_id: tc.id,
content: JSON.stringify(timeoutResult)
+ (loopCheck.kind === 'nudge' ? `\n${loopCheck.warning}` : ''),
});
const runId = this.currentRunId.get(tabId);
if (runId) {
try {
await trace.recordToolCall(runId, step, {
name: fnName,
args: fnArgs,
result: timeoutResult,
latencyMs: 0,
});
} catch {}
}
onUpdate('warning', { message: 'Page action preparation timed out before dispatch.' });
if (loopCheck.kind === 'nudge') {
onUpdate('warning', { message: 'Loop detected — nudging the agent.' });
}
if (loopCheck.kind !== 'stop') return null;
this._appendSyntheticToolResults(
tabId,
toolCalls,
toolIndex + 1,
messages,
onUpdate,
step,
() => ({ success: false, skipped: true, error: 'skipped: run stopped by loop detector' }),
return recordPreparationFailure(
toolIndex, fnName, fnArgs, timeoutResult,
'Page action preparation timed out before dispatch.',
);
if (runId) trace.recordError(runId, step, 'loop', loopCheck.message);
this._clearLoopState(tabId);
this._persist(tabId);
return { action: 'recover', value: loopCheck.message, status: 'loop_stopped' };
};
const detectSubmitWithDeadline = () => this._withContentActionDeadline(
async abortSignal => {
Expand Down
Loading
Loading