diff --git a/src/chrome/src/agent/planner.js b/src/chrome/src/agent/planner.js index ec5cb5e48..3f5d2d117 100644 --- a/src/chrome/src/agent/planner.js +++ b/src/chrome/src/agent/planner.js @@ -10,6 +10,14 @@ import { sanitizeText } from './text-sanitize.js'; const UNTRUSTED_PAGE_CONTENT_TAG_RE = /<\/?untrusted_page_content\b[^>]*>/gi; const REQUEST_KINDS = new Set(['execute', 'respond', 'plan_only', 'clarify']); +function canonicalPlanRequiresDownload(_summary, _steps) { + // TODO(#2752): Derive download completion requirements from structured, + // language-neutral planner intent. Do not infer them from canonical prose; + // lookup framing such as "Find the URL to download the report" makes that + // heuristic ambiguous. Until then, preserve the planner-declared value. + return false; +} + export const PLANNER_API_REPLAY_RULE = '- Because API mutations are authorized, repeated same-kind UI mutations may include a conditional API branch: if WebBrain later reports a [BULK API MUTATION PATTERN], sample exactly one fetch_url replay with the provided replayRequestId. If that sample fails with success:false or HTTP 4xx/5xx, stop using API for that request shape and continue through the paced visible-UI loop.'; // Keep response-only routing identical across the full Plan-before-Act planner @@ -354,33 +362,48 @@ export function normalizePlan(obj, opts = {}) { const normalizedScheduling = tool === 'schedule_task' || tool === 'schedule_resume' ? { tool, hint: sanitizeText(scheduling.hint, 300) } : null; + const risks = Array.isArray(obj.risks) + ? obj.risks.map((risk) => sanitizeText(risk, 200)).filter(Boolean).slice(0, 6) + : []; const localizedInput = obj.localized && typeof obj.localized === 'object' ? obj.localized : {}; - const localizedSteps = Array.isArray(localizedInput.steps) + const providedLocalizedSteps = Array.isArray(localizedInput.steps) ? localizedInput.steps.slice(0, 12).map((step, i) => ({ id: sanitizeText(step?.id || String(i + 1), 20) || String(i + 1), action: sanitizeText(step?.action, 300), })).filter((step) => step.action) : []; const localizedSummary = sanitizeText(localizedInput.summary, 400); + const localizedStepsById = new Map(providedLocalizedSteps.map(step => [step.id, step])); + const localizedSteps = steps.map(step => ({ + id: step.id, + action: localizedStepsById.get(step.id)?.action + || step.action, + })); + const providedLocalizedRisks = Array.isArray(localizedInput.risks) + ? localizedInput.risks.slice(0, 6).map((risk) => sanitizeText(risk, 200)) + : []; const requestedLocale = normalizePlannerLocale(opts.locale || localizedInput.locale); if (opts.requireIntent) { - if (!localizedSummary) return null; - if (requestKind !== 'clarify' && requestKind !== 'respond' && (steps.length === 0 || localizedSteps.length === 0)) return null; + if (requestKind === 'clarify' && !localizedSummary) return null; + if (requestKind !== 'clarify' && requestKind !== 'respond' && steps.length === 0) return null; } const localized = { locale: requestedLocale, summary: localizedSummary || summary, steps: localizedSteps, - risks: Array.isArray(localizedInput.risks) - ? localizedInput.risks.map((risk) => sanitizeText(risk, 200)).filter(Boolean).slice(0, 6) - : [], + risks: risks.map((risk, index) => providedLocalizedRisks[index] || risk), }; const submissionBearingPlan = executablePlan || requestKind === 'clarify'; const requiresSubmission = submissionBearingPlan ? (hasRequiresSubmission ? obj.requires_submission === true : null) : false; const requiresStateChange = executablePlan - ? (!!obj.requires_state_change || requiresSubmission === true || !!normalizedScheduling) + ? ( + !!obj.requires_state_change + || requiresSubmission === true + || !!normalizedScheduling + || canonicalPlanRequiresDownload(summary, steps) + ) : false; return { request_kind: requestKind, @@ -407,9 +430,7 @@ export function normalizePlan(obj, opts = {}) { : 'auto', }, scheduling: executablePlan ? normalizedScheduling : null, - risks: Array.isArray(obj.risks) - ? obj.risks.map((r) => sanitizeText(r, 200)).filter(Boolean).slice(0, 6) - : [], + risks, localized, mode: 'act', }; diff --git a/src/firefox/src/agent/planner.js b/src/firefox/src/agent/planner.js index ec5cb5e48..3f5d2d117 100644 --- a/src/firefox/src/agent/planner.js +++ b/src/firefox/src/agent/planner.js @@ -10,6 +10,14 @@ import { sanitizeText } from './text-sanitize.js'; const UNTRUSTED_PAGE_CONTENT_TAG_RE = /<\/?untrusted_page_content\b[^>]*>/gi; const REQUEST_KINDS = new Set(['execute', 'respond', 'plan_only', 'clarify']); +function canonicalPlanRequiresDownload(_summary, _steps) { + // TODO(#2752): Derive download completion requirements from structured, + // language-neutral planner intent. Do not infer them from canonical prose; + // lookup framing such as "Find the URL to download the report" makes that + // heuristic ambiguous. Until then, preserve the planner-declared value. + return false; +} + export const PLANNER_API_REPLAY_RULE = '- Because API mutations are authorized, repeated same-kind UI mutations may include a conditional API branch: if WebBrain later reports a [BULK API MUTATION PATTERN], sample exactly one fetch_url replay with the provided replayRequestId. If that sample fails with success:false or HTTP 4xx/5xx, stop using API for that request shape and continue through the paced visible-UI loop.'; // Keep response-only routing identical across the full Plan-before-Act planner @@ -354,33 +362,48 @@ export function normalizePlan(obj, opts = {}) { const normalizedScheduling = tool === 'schedule_task' || tool === 'schedule_resume' ? { tool, hint: sanitizeText(scheduling.hint, 300) } : null; + const risks = Array.isArray(obj.risks) + ? obj.risks.map((risk) => sanitizeText(risk, 200)).filter(Boolean).slice(0, 6) + : []; const localizedInput = obj.localized && typeof obj.localized === 'object' ? obj.localized : {}; - const localizedSteps = Array.isArray(localizedInput.steps) + const providedLocalizedSteps = Array.isArray(localizedInput.steps) ? localizedInput.steps.slice(0, 12).map((step, i) => ({ id: sanitizeText(step?.id || String(i + 1), 20) || String(i + 1), action: sanitizeText(step?.action, 300), })).filter((step) => step.action) : []; const localizedSummary = sanitizeText(localizedInput.summary, 400); + const localizedStepsById = new Map(providedLocalizedSteps.map(step => [step.id, step])); + const localizedSteps = steps.map(step => ({ + id: step.id, + action: localizedStepsById.get(step.id)?.action + || step.action, + })); + const providedLocalizedRisks = Array.isArray(localizedInput.risks) + ? localizedInput.risks.slice(0, 6).map((risk) => sanitizeText(risk, 200)) + : []; const requestedLocale = normalizePlannerLocale(opts.locale || localizedInput.locale); if (opts.requireIntent) { - if (!localizedSummary) return null; - if (requestKind !== 'clarify' && requestKind !== 'respond' && (steps.length === 0 || localizedSteps.length === 0)) return null; + if (requestKind === 'clarify' && !localizedSummary) return null; + if (requestKind !== 'clarify' && requestKind !== 'respond' && steps.length === 0) return null; } const localized = { locale: requestedLocale, summary: localizedSummary || summary, steps: localizedSteps, - risks: Array.isArray(localizedInput.risks) - ? localizedInput.risks.map((risk) => sanitizeText(risk, 200)).filter(Boolean).slice(0, 6) - : [], + risks: risks.map((risk, index) => providedLocalizedRisks[index] || risk), }; const submissionBearingPlan = executablePlan || requestKind === 'clarify'; const requiresSubmission = submissionBearingPlan ? (hasRequiresSubmission ? obj.requires_submission === true : null) : false; const requiresStateChange = executablePlan - ? (!!obj.requires_state_change || requiresSubmission === true || !!normalizedScheduling) + ? ( + !!obj.requires_state_change + || requiresSubmission === true + || !!normalizedScheduling + || canonicalPlanRequiresDownload(summary, steps) + ) : false; return { request_kind: requestKind, @@ -407,9 +430,7 @@ export function normalizePlan(obj, opts = {}) { : 'auto', }, scheduling: executablePlan ? normalizedScheduling : null, - risks: Array.isArray(obj.risks) - ? obj.risks.map((r) => sanitizeText(r, 200)).filter(Boolean).slice(0, 6) - : [], + risks, localized, mode: 'act', }; diff --git a/test/run.js b/test/run.js index 68e1a8f83..f6bf5d0c4 100644 --- a/test/run.js +++ b/test/run.js @@ -54524,6 +54524,56 @@ test('planner intent degrades to a localized read-only turn after one repair', a }); }); +test('planner intent preserves Act and canonical execution fields when localized display fields are missing', async () => { + await withPlannerBrowserGlobals(async () => { + for (const [index, AgentClass] of [AgentCh, AgentFx].entries()) { + const agent = new AgentClass({ getActive: () => ({ name: 'intent-test', model: 'intent-test' }) }); + let calls = 0; + let warning = ''; + agent._chatWithCostAllowance = async () => { + calls += 1; + return { + content: JSON.stringify({ + request_kind: 'execute', + requires_state_change: false, + requires_submission: false, + allows_planner_shaped_result: false, + allows_app_state_tool_evidence: true, + read_scope: 'visible_page', + summary: 'Find and download the video from the current X tweet page', + steps: [ + { id: '1', action: 'Read the current page to find video elements or download links' }, + { id: '2', action: 'Extract the direct video URL and download it' }, + ], + memory: { use_progress_ledger: false, progress_action: null }, + scheduling: null, + risks: ['The video source may need to be resolved before downloading'], + }), + }; + }; + + const gate = await agent._runPlannerIntentGate( + 8685 + index, + { role: 'user', content: 'download this video' }, + (type, data) => { if (type === 'warning') warning = data?.message || ''; }, + null, + null, + '', + { tabUrl: 'https://x.com/example/status/1', tabTitle: 'Example post' }, + 'act', + { locale: 'tr' }, + ); + + assert.equal(calls, 1, `${AgentClass.name}: missing display localization triggered an unnecessary repair`); + assert.equal(gate.proceed, true, `${AgentClass.name}: recoverable localization blocked execution`); + assert.equal(gate.requestKind, 'execute', `${AgentClass.name}: download intent was downgraded`); + assert.equal(gate.readOnlyFallback, undefined, `${AgentClass.name}: download plan fell back to Ask`); + assert.equal(gate.requiresStateChange, false, `${AgentClass.name}: localization recovery changed canonical execution metadata`); + assert.equal(warning, '', `${AgentClass.name}: recoverable localization emitted a planner failure warning`); + } + }); +}); + test('planner intent keeps execution authorized for plan-and-act and negated approval waits', async () => { await withPlannerBrowserGlobals(async () => { const tasks = [ @@ -60582,6 +60632,68 @@ test('planner: parse and format structured plan', () => { } }); +test('planner: canonical fields recover missing and partial localization without changing execution metadata', () => { + const tracePlan = JSON.stringify({ + request_kind: 'execute', + requires_state_change: false, + requires_submission: false, + allows_planner_shaped_result: false, + allows_app_state_tool_evidence: true, + read_scope: 'visible_page', + summary: 'Find and download the video from the current X tweet page', + steps: [ + { id: '1', action: 'Read the current page to find video elements or download links' }, + { id: '2', action: 'Extract the direct video URL and download it' }, + ], + memory: { use_progress_ledger: false, progress_action: null }, + scheduling: null, + risks: ['The video source may need to be resolved before downloading'], + }); + + for (const [label, parse] of [['chrome', parsePlanFromContent], ['firefox', parsePlanFromContentFx]]) { + const plan = parse(tracePlan, { requireIntent: true, locale: 'tr' }); + assert.ok(plan, `${label}: missing localized display fields invalidated canonical intent`); + assert.equal(plan.localized.locale, 'tr', `${label}: requested display locale was lost`); + assert.equal(plan.localized.summary, plan.summary, `${label}: canonical summary did not backfill localized display text`); + assert.deepEqual( + plan.localized.steps, + plan.steps.map(step => ({ id: step.id, action: step.action })), + `${label}: canonical steps did not backfill localized display text`, + ); + assert.deepEqual(plan.localized.risks, plan.risks, `${label}: canonical risks did not backfill localized display text`); + assert.equal(plan.requires_state_change, false, `${label}: localization recovery changed canonical execution metadata`); + + const partialLocalization = JSON.parse(tracePlan); + partialLocalization.risks = ['First canonical risk', 'Second canonical risk']; + partialLocalization.localized = { + locale: 'tr', + summary: 'Geçerli videoyu indir', + steps: [{ id: '2', action: 'Doğrudan video adresini çıkar ve indir' }], + risks: ['', 'İkinci risk'], + }; + const partial = parse(JSON.stringify(partialLocalization), { requireIntent: true, locale: 'tr' }); + assert.equal(partial?.localized.steps[0]?.action, partial?.steps[0]?.action, `${label}: missing localized step did not fall back by id`); + assert.equal(partial?.localized.steps[1]?.action, 'Doğrudan video adresini çıkar ve indir', `${label}: supplied localized step was discarded`); + assert.deepEqual( + partial?.localized.risks, + ['First canonical risk', 'İkinci risk'], + `${label}: localized risk holes shifted translations away from their canonical positions`, + ); + + const malformedClarification = parse(JSON.stringify({ + request_kind: 'clarify', + requires_state_change: false, + requires_submission: false, + read_scope: 'none', + summary: 'Ask the user which account should receive the transfer.', + steps: [], + risks: [], + }), { requireIntent: true, locale: 'tr' }); + assert.equal(malformedClarification, null, `${label}: clarification without the actual localized question bypassed repair`); + + } +}); + test('planner: parse JSON inside markdown fence', () => { const fenced = 'Here is the plan:\n```json\n{"summary":"Go back","steps":[],"memory":{"use_scratchpad":false,"scratchpad_notes":[],"use_progress_ledger":false,"progress_action":null},"scheduling":null,"risks":[],"mode":"act"}\n```'; const plan = parsePlanFromContent(fenced);