diff --git a/src/chrome/src/agent/agent.js b/src/chrome/src/agent/agent.js index 9feb48deb..991b32eaf 100644 --- a/src/chrome/src/agent/agent.js +++ b/src/chrome/src/agent/agent.js @@ -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); + } + 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 }; @@ -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; } @@ -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; } @@ -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 => { diff --git a/src/chrome/src/agent/loop-bucket.js b/src/chrome/src/agent/loop-bucket.js index ba382c928..3230b4ba5 100644 --- a/src/chrome/src/agent/loop-bucket.js +++ b/src/chrome/src/agent/loop-bucket.js @@ -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`. */ @@ -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) { diff --git a/src/chrome/src/agent/loop-detector.js b/src/chrome/src/agent/loop-detector.js index bd93586ca..fd8353062 100644 --- a/src/chrome/src/agent/loop-detector.js +++ b/src/chrome/src/agent/loop-detector.js @@ -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 @@ -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); diff --git a/src/firefox/src/agent/agent.js b/src/firefox/src/agent/agent.js index 53223990f..90778f869 100644 --- a/src/firefox/src/agent/agent.js +++ b/src/firefox/src/agent/agent.js @@ -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); + } + 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 }; @@ -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; } @@ -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; } @@ -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 => { diff --git a/src/firefox/src/agent/loop-bucket.js b/src/firefox/src/agent/loop-bucket.js index ba382c928..3230b4ba5 100644 --- a/src/firefox/src/agent/loop-bucket.js +++ b/src/firefox/src/agent/loop-bucket.js @@ -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`. */ @@ -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) { diff --git a/src/firefox/src/agent/loop-detector.js b/src/firefox/src/agent/loop-detector.js index bd93586ca..fd8353062 100644 --- a/src/firefox/src/agent/loop-detector.js +++ b/src/firefox/src/agent/loop-detector.js @@ -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 @@ -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); diff --git a/test/agent-lifecycle.mjs b/test/agent-lifecycle.mjs index baaa48d27..15885a4ab 100644 --- a/test/agent-lifecycle.mjs +++ b/test/agent-lifecycle.mjs @@ -35,6 +35,132 @@ function setup(Agent, provider = {}) { const deferred = () => { let resolve; const promise = new Promise(r => { resolve = r; }); return { promise, resolve }; }; for (const [browser, Agent] of variants) { + for (const failure of ['out-of-bounds', 'stale capture', 'missing capture', 'invalid schema', 'malformed JSON']) { + test(`${browser}: repeated ${failure} calls enter loop recovery before dispatch`, async () => { + const agent = setup(Agent); + const tabId = 71; + agent.maxSteps = Infinity; + agent._resolvePromptTier = () => 'full'; + agent._resolveVisionRoute = async () => ({ provider: null }); + agent.executeTool = async () => assert.fail('Rejected action or queued fallback was dispatched'); + agent._captchaMutationPreflight = async () => assert.fail('Rejected action reached page preflight'); + const messages = []; + const updates = []; + const results = []; + for (let attempt = 1; attempt <= 3; attempt++) { + const capture = agent._registerScreenshotCapture(tabId, { + imageWidth: 1102, imageHeight: 746, cssWidth: 1102, cssHeight: 746, + }); + // Capture churn and JSON property order must not disguise one failure. + // Coordinate failures carry a constant failureScope, so only the + // invalid-schema case can be hidden by reordering — it has to build + // its own key-ordered args to exercise the default scope. + const entries = [ + ['x', 800], ['y', failure === 'out-of-bounds' ? 780 : 400], + ['coordinate_space', 'screenshot'], + ...(failure === 'missing capture' ? [] : [['capture_id', failure === 'stale capture' ? `old-${attempt}` : capture.captureId]]), + ]; + const schemaEntries = [['text', '02'], ['selector', '#jj']]; + if (attempt % 2 === 0) { + entries.reverse(); + schemaEntries.reverse(); + } + const args = failure === 'malformed JSON' ? '{"x":' + : failure === 'invalid schema' ? JSON.stringify(Object.fromEntries(schemaEntries)) + : JSON.stringify(Object.fromEntries(entries)); + const name = failure === 'invalid schema' ? 'set_field' : 'click'; + results.push(await agent._executeToolBatch( + tabId, + [ + { id: `rejected-${attempt}`, function: { name, arguments: args } }, + { id: `fallback-${attempt}`, function: { name: 'click', arguments: '{"text":"Save"}' } }, + ], + messages, (type, data) => updates.push({ type, data }), + { supportsVision: false }, null, new Set(['click', 'set_field']), attempt, + )); + } + assert.deepEqual(results.map(result => result.action), ['continue', 'continue', 'recover']); + assert.equal(results[2].status, 'loop_stopped'); + assert.match(results[2].value, /three times/i); + assert.equal(messages.length, 6, 'Every rejected and skipped call needs a result'); + for (let attempt = 1; attempt <= 3; attempt++) { + const rejected = messages.find(message => message.tool_call_id === `rejected-${attempt}`); + const result = JSON.parse(rejected.content.split('\n')[0]); + assert.equal(result.success, false); + assert.equal(result.dispatched, false); + assert.equal(result.noDispatch, true); + const fallback = messages.find(message => message.tool_call_id === `fallback-${attempt}`); + assert.equal(JSON.parse(fallback.content).skipped, true); + } + assert.match(messages.find(message => message.tool_call_id === 'rejected-2').content, /FAILED ACTION LOOP/); + assert.ok(updates.some(update => update.type === 'warning' && /Loop detected/.test(update.data.message))); + }); + } + + test(`${browser}: verified screenshot corrections reset failures before later mistakes`, async () => { + const agent = setup(Agent); + allowBatchPreparation(agent); + agent._resolveVisionRoute = async () => ({ provider: null }); + const tabId = 73; + const dispatched = []; + agent.executeTool = async (_tabId, name, args) => { + dispatched.push({ name, args }); + return { success: true, verified: true, dispatched: true }; + }; + const capture = agent._registerScreenshotCapture(tabId, { + imageWidth: 1102, imageHeight: 746, cssWidth: 1102, cssHeight: 746, + }); + const messages = []; + const expectedCounts = [1, 2, 0, 1, 2, 0, 1]; + for (const [index, y] of [780, 780, 400, 790, 790, 410, 800].entries()) { + const result = await agent._executeToolBatch( + tabId, + [{ id: `correction-${index}`, function: { + name: 'click', + arguments: JSON.stringify({ x: 800, y, coordinate_space: 'screenshot', capture_id: capture.captureId }), + } }], + messages, () => {}, { supportsVision: false }, null, new Set(['click']), index + 1, + ); + assert.equal(result.action, 'continue'); + assert.equal(agent.failedActionLoops.get(tabId)?.get('screenshot-coordinate-capture') || 0, expectedCounts[index]); + } + assert.match(messages[1].content, /FAILED ACTION LOOP/); + assert.equal(dispatched.length, 2); + assert.equal(dispatched[0].args.y, 400); + assert.equal(dispatched[0].args.coordinate_space, 'css'); + }); + + test(`${browser}: only verified click progress clears coordinate preparation failures`, () => { + const tabId = 74; + const scope = 'screenshot-coordinate-capture'; + const failure = { success: false, noDispatch: true, dispatched: false, failureScope: scope, error: 'Stale capture' }; + for (const [name, result] of [ + ['get_accessibility_tree', { success: true, verified: true }], + ['inspect_viewport', { success: true }], + ['set_field', { success: true, verified: true }], + ['click', { success: true }], + ['click', { success: true, verified: false }], + ['click', { success: true, verified: true, noDispatch: true }], + ['click', { success: true, verified: true, dispatched: false }], + ['click', { success: true, verified: true, outcomeUnknown: true }], + ['click', { success: true, verified: true, inconclusive: true }], + ['click', { success: true, verified: true, noProgress: true }], + ]) { + const agent = setup(Agent); + agent._checkLoop(tabId, 'click', { x: 800, y: 780 }, failure); + agent._checkLoop(tabId, name, { ref_id: 'ref_recovery' }, result); + assert.equal(agent.failedActionLoops.get(tabId)?.get(scope), 1, `${name}: ${JSON.stringify(result)}`); + assert.equal(agent._checkLoop(tabId, 'click', { x: 800, y: 780 }, failure).kind, 'nudge'); + assert.equal(agent._checkLoop(tabId, 'click', { x: 800, y: 780 }, failure).kind, 'stop'); + } + for (const name of ['click', 'click_ax', 'iframe_click']) { + const agent = setup(Agent); + agent.failedActionLoops.set(tabId, new Map([[scope, 2], ['coordinate-provenance', 2], ['unrelated-target', 1]])); + agent._checkLoop(tabId, name, { ref_id: 'ref_recovery' }, { success: true, verified: true, dispatched: true }); + assert.deepEqual([...agent.failedActionLoops.get(tabId)], [['unrelated-target', 1]], `${name}: keep unrelated failure counts`); + } + }); + test(`${browser}: cancellation survives nested readers and resets only on a fresh run`, async () => { const agent = setup(Agent); await agent._claimRunEntry(1, 'interactive'); @@ -287,6 +413,61 @@ function assertCompleteToolHistory(messages) { for (const [browser, Agent] of variants) { for (const streaming of [false, true]) { + test(`${browser}: unlimited ${streaming ? 'stream' : 'chat'} run recovers from repeated rejected clicks`, async () => { + const tabId = 72; + let agent; + let attempts = 0; + let recoveries = 0; + const nextCall = () => { + attempts++; + const capture = agent._registerScreenshotCapture(tabId, { + imageWidth: 1102, imageHeight: 746, cssWidth: 1102, cssHeight: 746, + }); + return { + id: `outside-${attempts}`, type: 'function', + function: { + name: 'click', + arguments: JSON.stringify({ x: 800, y: 780, coordinate_space: 'screenshot', capture_id: capture.captureId }), + }, + }; + }; + const provider = { + supportsTools: true, + chat: async (messages, options) => { + assertCompleteToolHistory(messages); + if (!options.tools?.length) { + recoveries++; + return { content: 'The month could not be selected. The second post is incomplete.' }; + } + // Bound the test even if a regression lets the production loop run on. + if (attempts >= 3) return { content: 'Unexpected extra browser step.' }; + return { toolCalls: [nextCall()] }; + }, + async *chatStream(messages) { + assertCompleteToolHistory(messages); + if (attempts >= 3) yield { type: 'text', content: 'Unexpected extra browser step.' }; + else yield { type: 'tool_call', content: [{ ...nextCall(), index: 0 }] }; + yield { type: 'done' }; + }, + }; + agent = setup(Agent, provider); + agent.maxSteps = Infinity; + agent._resolveVisionRoute = async () => ({ provider: null }); + agent._beginReadCompleteness = async () => null; + agent._maybeRunPlannerGate = async () => ({ proceed: true, requiresStateChange: true }); + agent.executeTool = async () => assert.fail('Out-of-bounds click was dispatched'); + const update = () => {}; + const result = streaming + ? await agent.processMessageStream(tabId, 'Set the month to October.', update, 'act', { askStreamingEnabled: false }) + : await agent.processMessage(tabId, 'Set the month to October.', update, 'act', [], { askStreamingEnabled: false }); + assert.equal(attempts, 3); + assert.equal(recoveries, 1, 'Run must enter one tool-free partial-result recovery'); + assert.equal(agent.testStatus, 'loop_stopped'); + assert.match(result, /second post is incomplete/); + assert.equal(agent.isRunning(tabId), false); + assertCompleteToolHistory(agent.getConversation(tabId, 'act')); + }); + for (const phase of ['checkpoint', 'preflight', 'execution', 'after-result']) { test(`${browser}: ${streaming ? 'stream' : 'chat'} Stop during ${phase} persists paired tools for the next request`, async () => { const tabId = 41; diff --git a/test/run.js b/test/run.js index 8b518d1fe..5abc95376 100644 --- a/test/run.js +++ b/test/run.js @@ -818,6 +818,11 @@ class ConfiguredLoopDetector extends LoopDetectorCh { return MUTATION_TOOLS_CH.has(toolName); } } +class ConfiguredLoopDetectorFx extends LoopDetectorFx { + _isBrowserMutationTool(toolName) { + return MUTATION_TOOLS_FX.has(toolName); + } +} const { detectProgressAction, isValidLedgerStatus, @@ -23028,7 +23033,7 @@ test('fetch_url loop buckets allow semantic pages/searches but collapse guessed assert.equal(rangeA, bucketArgsKeyFx('fetch_url', { url, headers: { Range: 'bytes=0-9999' } }), 'firefox Range bucket drift'); }); -test('bucketArgsKey: non-URL tools fall back to exact JSON args', () => { +test('bucketArgsKey: non-URL tools fall back to key-order-independent JSON args', () => { // click_ax with the same ref_id should match itself assert.equal( bucketArgsKey('click_ax', { ref_id: 'ref_42' }), @@ -23041,6 +23046,58 @@ test('bucketArgsKey: non-URL tools fall back to exact JSON args', () => { ); }); +test('bucketArgsKey: reordered object keys cannot disguise one rejected call', () => { + // JSON.stringify keeps insertion order, so a model that re-emits the same + // invalid argument object with its keys permuted would hash to a fresh key + // every attempt and never hit the rejection limit. + for (const key of [bucketArgsKey, bucketArgsKeyFx]) { + assert.equal( + key('set_field', { text: '02', selector: '#jj' }), + key('set_field', { selector: '#jj', text: '02' }), + 'top-level key order must not change the loop identity', + ); + assert.equal( + key('click', { x: 10, y: 20, meta: { a: 1, b: 2 } }), + key('click', { meta: { b: 2, a: 1 }, y: 20, x: 10 }), + 'nested key order must not change the loop identity', + ); + assert.notEqual( + key('click', { steps: [1, 2] }), + key('click', { steps: [2, 1] }), + 'array order is semantic and must stay distinct', + ); + assert.notEqual( + key('set_field', { text: '02', selector: '#jj' }), + key('set_field', { text: '03', selector: '#jj' }), + 'different values are different calls', + ); + } + // Degenerate values must not throw or collapse into one identity. + const cyclic = { text: '02' }; + cyclic.self = cyclic; + assert.equal(bucketArgsKey('set_field', cyclic), bucketArgsKey('set_field', cyclic)); + assert.equal(bucketArgsKey('set_field'), bucketArgsKey('set_field', null)); + assert.equal( + bucketArgsKey('set_field', { text: undefined, extra: 1 }), + bucketArgsKey('set_field', { extra: 1 }), + 'undefined values must not split an identity', + ); +}); + +test('rejected invalid-schema calls share one failure scope across key orders', () => { + for (const [label, Detector] of [['chrome', ConfiguredLoopDetector], ['firefox', ConfiguredLoopDetectorFx]]) { + const detector = new Detector(); + const tabId = 1; + const rejection = { success: false, invalidArguments: true, noDispatch: true, dispatched: false }; + const outcomes = [ + { text: '02', selector: '#jj' }, + { selector: '#jj', text: '02' }, + { selector: '#jj', text: '02' }, + ].map(args => detector._checkLoop(tabId, 'set_field', args, rejection).kind); + assert.deepEqual(outcomes, ['none', 'nudge', 'stop'], `${label}: key order must not dodge the limit`); + } +}); + test('URL_FAMILY_TOOLS contains the expected tool names', () => { // Lock the membership so a future contributor doesn't accidentally // remove fetch_url and silently regress the loop detector. @@ -23079,6 +23136,8 @@ test('firefox loop-bucket matches chrome', () => { ['fetch_url', { url: 'https://api.github.com/repos/o/r/contents/foo.json', method: 'POST' }], ['click_ax', { ref_id: 'ref_42' }], ['fetch_url', { url: 'not a url' }], + ['set_field', { selector: '#jj', nested: { b: 2, a: [1, 2] }, text: '02' }], + ['set_field', { text: undefined, extra: 1 }], ]; for (const [name, args] of samples) { assert.equal(bucketArgsKeyFx(name, args), bucketArgsKey(name, args), `mismatch on ${name} ${JSON.stringify(args)}`);