fix(web-shell): stabilize goal proposal approval layout (#11839)

Co-authored-by: qqqys <[email protected]>
This commit is contained in:
qqqysandqqqys authored and GitHub committed 2026-09-14 11:47:37 +00:00
1 parent 7abf922415
commit 5b7b50380a
6 files changed
+576 -37

No files matched your search

@@ -0,0 +1,96 @@
import { useState } from 'react';
import { useI18n } from '../../i18n';
import { Tabs, TabsContent, TabsList, TabsTrigger } from '../ui/tabs';
import styles from './ToolApproval.module.css';
const sectionLabels: Record<string, string> = {
Outcome: 'approval.goal.outcome',
'Done when': 'approval.goal.doneWhen',
'Must not': 'approval.goal.mustNot',
Budget: 'approval.goal.budget',
'On block': 'approval.goal.onBlock',
Context: 'approval.goal.context',
};
function splitSections(objective: string) {
const matches = [
...objective.matchAll(
/(?:^|\s)(Outcome|Done when|Must not|Budget|On block|Context):\s*/g,
),
];
// Only format the draft convention; arbitrary objectives remain verbatim.
if (
matches[0]?.[1] !== 'Outcome' ||
objective.slice(0, matches[0].index).trim()
) {
return [];
}
return matches.map((match, index) => ({
label: sectionLabels[match[1]],
text: objective
.slice(
match.index + match[0].length,
matches[index + 1]?.index ?? objective.length,
)
.trim(),
}));
}
export function GoalApprovalContent({
objective,
content,
id,
}: {
objective: string;
content: string;
id: string;
}) {
const { t } = useI18n();
const [view, setView] = useState('overview');
const sections = splitSections(objective);
const fullText = content || objective;
// Confirmation text can warn that an existing goal will be replaced. Keep
// that notice in the overview too, rather than hiding it behind a tab.
const trimmedObjective = objective.trim();
const prompt = fullText.trimEnd();
const notice =
trimmedObjective && prompt.endsWith(trimmedObjective)
? prompt.slice(0, -trimmedObjective.length).trim()
: fullText !== objective
? fullText
: '';
return (
<Tabs
value={view}
onValueChange={setView}
className={styles.goalTabs}
data-approval-shortcuts-ignore
>
<TabsList variant="line" aria-label={t('approval.goal.title')}>
<TabsTrigger value="overview">
{t('approval.goal.overview')}
</TabsTrigger>
<TabsTrigger value="full">{t('approval.goal.full')}</TabsTrigger>
</TabsList>
<div className={styles.goalBody} id={id}>
<TabsContent value="overview" className={styles.goalPanel}>
{objective && notice && <p className={styles.goalNotice}>{notice}</p>}
{sections.length ? (
sections.map((section, index) => (
<section key={index} className={styles.goalSection}>
<h3>{t(section.label)}</h3>
<p>{section.text}</p>
</section>
))
) : (
<p>{objective || fullText}</p>
)}
</TabsContent>
<TabsContent value="full" className={styles.goalPanel}>
<p>{fullText}</p>
</TabsContent>
</div>
</Tabs>
);
}
@@ -283,3 +283,93 @@
justify-content: flex-start;
}
}
.goalApproval {
display: flex;
flex-direction: column;
max-height: min(640px, 80dvh);
overflow: hidden;
}
.goalApproval > .header,
.goalApproval > .options,
.goalHint,
.goalFeedback {
flex-shrink: 0;
}
.goalTabs {
min-height: 0;
flex: 1 1 auto;
margin-top: 12px;
}
.goalBody {
height: 310px;
min-height: 0;
overflow: hidden;
flex: 1 1 auto;
}
.goalPanel {
height: 100%;
overflow-y: auto;
overscroll-behavior: contain;
scrollbar-gutter: stable;
padding: 12px 4px;
white-space: pre-wrap;
overflow-wrap: anywhere;
line-height: 1.6;
}
.goalPanel p {
margin: 0;
}
.goalPanel .goalNotice {
color: var(--muted-foreground);
margin-bottom: 16px;
}
.goalSection + .goalSection {
margin-top: 16px;
}
.goalSection h3 {
margin: 0 0 4px;
font-size: 14px;
font-weight: 500;
}
.goalHint {
margin-top: 12px;
padding-top: 12px;
border-top: 1px solid var(--border);
color: var(--muted-foreground);
font-size: 12px;
line-height: 18px;
}
.goalFeedback {
min-height: 18px;
font-size: 12px;
line-height: 18px;
}
.goalApproval > .options {
margin-top: 8px;
gap: 12px;
}
.goalApproval .option {
white-space: normal;
}
.goalApproval .option:disabled {
opacity: 0.5;
cursor: default;
}
.goalTabs > [role='tablist'] {
flex-shrink: 0;
}
@@ -1068,3 +1068,198 @@ describe('ToolApproval accessibility', () => {
expect(onConfirm).toHaveBeenCalledTimes(1);
});
});
const goalObjective =
'Outcome: Audit open PRs. Done when: Every PR has evidence. Must not: Push or comment. Budget: 20 turns. On block: Report missing access. Context: Preserve the exact budget assumption.';
const goalNotice =
'Replace the paused Goal and start working toward this objective?';
const goalRequest: PermissionRequest = {
...request,
id: 'goal-request',
toolName: 'propose_goal',
title: `Propose Goal: ${goalObjective}`,
rawInput: { objective: goalObjective },
content: [{ type: 'text', text: `${goalNotice}\n\n${goalObjective}` }],
};
function switchGoalTab(value: string) {
const tab =
container!.querySelectorAll<HTMLButtonElement>('[role="tab"]')[
value === 'full' ? 1 : 0
];
act(() => {
tab.dispatchEvent(
new MouseEvent('mousedown', { bubbles: true, button: 0 }),
);
});
}
describe('goal approval', () => {
it('formats the draft without hiding constraints or the replacement notice', () => {
render(false, goalRequest, undefined, 'zh-CN');
expect(container!.textContent).toContain('确认会话目标');
expect(container!.textContent).toContain('设置并继续');
expect(container!.textContent).toContain('暂不设置');
expect(container!.textContent).toContain(goalNotice);
expect(container!.textContent).toContain('Push or comment.');
expect(container!.textContent).toContain(
'Preserve the exact budget assumption.',
);
expect(container!.textContent).not.toContain('Propose Goal:');
expect(container!.querySelector('pre')).toBeNull();
switchGoalTab('full');
expect(
container!.querySelector('[role="tabpanel"][data-state="active"]')!
.textContent,
).toBe(`${goalNotice}\n\n${goalObjective}`);
expect(onConfirm).not.toHaveBeenCalled();
});
it('retains arbitrary objectives and fallback confirmation content', () => {
render(false, { ...goalRequest, rawInput: undefined });
expect(container!.textContent!.split(goalNotice)).toHaveLength(2);
switchGoalTab('full');
expect(
container!.querySelector('[role="tabpanel"][data-state="active"]')!
.textContent,
).toBe(`${goalNotice}\n\n${goalObjective}`);
rerender(false, {
...goalRequest,
id: 'plain',
rawInput: { objective: '原样保留\n command --flag' },
content: [],
});
expect(
container!.querySelector('[role="tabpanel"][data-state="active"]')!
.textContent,
).toBe('原样保留\n command --flag');
});
it('does not invoke approval shortcuts while reading goal tabs or text', () => {
render(false, goalRequest);
const panel = container!.querySelector(
'[role="tabpanel"][data-state="active"]',
)!;
act(() => {
for (const key of ['1', '2', 'j', 'k', 'Home', 'End', 'Escape']) {
panel.dispatchEvent(
new KeyboardEvent('keydown', { key, bubbles: true }),
);
}
});
expect(onConfirm).not.toHaveBeenCalled();
});
it('disables duplicate submissions and retains the full view when retrying', async () => {
let fail!: (reason: Error) => void;
onConfirm.mockImplementationOnce(
() =>
new Promise<void>((_, reject) => {
fail = reject;
}),
);
render(false, goalRequest);
switchGoalTab('full');
const approve = container!.querySelector<HTMLButtonElement>(
'[data-option-id="proceed"]',
)!;
act(() => {
approve.click();
approve.click();
});
expect(onConfirm).toHaveBeenCalledTimes(1);
expect(approve.disabled).toBe(true);
await act(async () => {
fail(new Error('offline'));
});
expect(approve.disabled).toBe(false);
expect(container!.querySelector('[role="alert"]')!.textContent).toContain(
'Please try again',
);
expect(
container!.querySelector('[role="tab"][data-state="active"]')!
.textContent,
).toBe('Full content');
act(() => approve.click());
expect(onConfirm).toHaveBeenCalledTimes(2);
expect(onConfirm).toHaveBeenLastCalledWith('goal-request', 'proceed');
});
it('rearms synchronous failures and ignores late rejection of an older request', async () => {
onConfirm.mockImplementationOnce(() => {
throw new Error('offline');
});
render(false, goalRequest);
act(() =>
container!
.querySelector<HTMLButtonElement>('[data-option-id="proceed"]')!
.click(),
);
expect(container!.querySelector('[role="alert"]')).not.toBeNull();
let fail!: (reason: Error) => void;
onConfirm.mockImplementationOnce(
() =>
new Promise<void>((_, reject) => {
fail = reject;
}),
);
act(() =>
container!
.querySelector<HTMLButtonElement>('[data-option-id="proceed"]')!
.click(),
);
rerender(false, { ...goalRequest, id: 'next-goal' });
act(() =>
container!
.querySelector<HTMLButtonElement>('[data-option-id="proceed"]')!
.click(),
);
await act(async () => {
fail(new Error('old request'));
});
expect(
container!.querySelector<HTMLButtonElement>('[data-option-id="proceed"]')!
.disabled,
).toBe(true);
expect(container!.querySelector('[role="alert"]')).toBeNull();
});
});
describe('successive goal approvals', () => {
it('focuses the safe default when a pending request is replaced', () => {
render(true, goalRequest);
act(() =>
container!
.querySelector<HTMLButtonElement>('[data-option-id="proceed"]')!
.click(),
);
expect(
container!.querySelector<HTMLButtonElement>('[data-option-id="proceed"]')!
.disabled,
).toBe(true);
rerender(true, { ...goalRequest, id: 'new-goal' });
expect(document.activeElement).toBe(
container!.querySelector('[data-option-id="reject"]'),
);
expect(
container!.querySelector<HTMLButtonElement>('[data-option-id="reject"]')!
.disabled,
).toBe(false);
});
});
describe('goal approval objective whitespace', () => {
it('does not repeat an objective trimmed by the confirmation producer', () => {
render(false, {
...goalRequest,
rawInput: { objective: ` ${goalObjective}\n` },
});
expect(container!.textContent!.split('Audit open PRs.')).toHaveLength(2);
expect(container!.textContent).toContain(goalNotice);
switchGoalTab('full');
expect(
container!.querySelector('[role="tabpanel"][data-state="active"]')!
.textContent,
).toBe(`${goalNotice}\n\n${goalObjective}`);
});
});
@@ -11,6 +11,7 @@ import {
import { isAgentTool } from '@qwen-code/web-shell/daemon-react-sdk';
import type { PermissionRequest, TodoItem } from '../../adapters/types';
import { useI18n } from '../../i18n';
import { GoalApprovalContent } from './GoalApprovalContent';
import { PlanExecutionView } from './PlanExecutionView';
import { isExitPlanApprovalRequest } from '../../utils/todos';
import { getShadowAwareActiveElement, isEditableTarget } from '../../utils/dom';
@@ -235,6 +236,7 @@ export function ToolApproval({
}: ToolApprovalProps) {
const { t } = useI18n();
const isAgent = isAgentTool(request.toolName);
const isGoal = request.toolName === 'propose_goal';
const isExitPlanApproval = isExitPlanApprovalRequest(request);
const hasPlanExecutionMode =
isExitPlanApproval && planExecutionMode !== undefined;
@@ -271,6 +273,10 @@ export function ToolApproval({
if (key) keyCount.set(key, (keyCount.get(key) ?? 0) + 1);
}
return (option: PermissionRequest['options'][number]) => {
if (isGoal && keyCount.get(getOptionI18nKey(option) ?? '') === 1) {
if (option.kind === 'allow_once') return t('approval.goal.confirm');
if (option.kind === 'reject_once') return t('approval.goal.reject');
}
if (hasPlanExecutionMode && option.id === 'restore_previous') {
return t('approval.option.executePlan', {
mode: t(`mode.label.${planExecutionMode}`),
@@ -299,12 +305,23 @@ export function ToolApproval({
};
}, [
displayOptions,
isGoal,
showsPlanWorkflow,
hasPlanExecutionMode,
planExecutionMode,
t,
]);
const [selected, setSelected] = useState(safeDefaultIndex);
const [submissionState, setSubmissionState] = useState<{
requestId: string;
status: 'pending' | 'failed';
} | null>(null);
const submitting =
submissionState?.requestId === request.id &&
submissionState.status === 'pending';
const submissionFailed =
submissionState?.requestId === request.id &&
submissionState.status === 'failed';
const requestRef = useRef(request);
requestRef.current = request;
const selectedRef = useRef(selected);
@@ -326,6 +343,7 @@ export function ToolApproval({
// request the user already answered.
useEffect(() => {
submittedRef.current = false;
setSubmissionState(null);
selectedRef.current = safeDefaultIndexRef.current;
setSelected(safeDefaultIndexRef.current);
}, [request.id]);
@@ -333,13 +351,18 @@ export function ToolApproval({
const parsedTitle = parseTitle(request.title);
const rawToolName =
request.toolName || parsedTitle.toolName || request.kind || 'Tool';
const toolName = showsPlanWorkflow
? t('workflow.planReview.title')
: localizeToolDisplayName(rawToolName, t);
const descriptionText = showsPlanWorkflow
? undefined
: getDescriptionText(request);
const toolName = isGoal
? t('approval.goal.title')
: showsPlanWorkflow
? t('workflow.planReview.title')
: localizeToolDisplayName(rawToolName, t);
const descriptionText =
showsPlanWorkflow || isGoal ? undefined : getDescriptionText(request);
const contentText = extractContentText(request);
const goalObjective =
typeof request.rawInput?.objective === 'string'
? request.rawInput.objective
: '';
const showsContent = Boolean(
contentText && (request.contentIsInput || contentText !== request.title),
);
@@ -349,21 +372,26 @@ export function ToolApproval({
if (disabled || submittedRef.current) return;
submittedRef.current = true;
const requestId = requestRef.current.id;
const submission = onConfirm(requestId, optionId);
if (submission) {
void submission.catch(() => {
// Re-arm only if the rejected submission still belongs to the
// current request. This instance is reused across successive
// requests (no key at the mount sites), and a submission can
// reject late (up to the action timeout); a stale rejection would
// otherwise disarm the successor's double-submit guard mid-flight.
if (requestRef.current.id === requestId) {
submittedRef.current = false;
if (isGoal) {
setSubmissionState({ requestId, status: 'pending' });
}
const onFailure = () => {
// A late rejection must not re-arm a newer request's submission.
if (requestRef.current.id === requestId) {
submittedRef.current = false;
if (isGoal) {
setSubmissionState({ requestId, status: 'failed' });
}
});
}
};
try {
const submission = onConfirm(requestId, optionId);
if (submission) void submission.catch(onFailure);
} catch {
onFailure();
}
},
[onConfirm, disabled],
[onConfirm, disabled, isGoal],
);
const focusOption = useCallback((index: number) => {
@@ -477,26 +505,32 @@ export function ToolApproval({
const isExec = isExecKind(request);
const command = getCommandFromRawInput(request);
const showsCommandBlock = Boolean((isExec && command) || showsContent);
const questionText = showsPlanWorkflow
? t('workflow.planReview.question')
: isAgent
? t('approval.launchAgentQuestion')
: isExec
? t('approval.execQuestion', { tool: toolName })
: t('approval.changeQuestion');
const showsCommandBlock =
!isGoal && Boolean((isExec && command) || showsContent);
const questionText = isGoal
? t('approval.goal.hint')
: showsPlanWorkflow
? t('workflow.planReview.question')
: isAgent
? t('approval.launchAgentQuestion')
: isExec
? t('approval.execQuestion', { tool: toolName })
: t('approval.changeQuestion');
return (
<div
ref={panelRef}
className={
variant === 'floating'
? `${styles.approval} ${styles.floating}${
isExitPlanApproval ? ` ${styles.floatingWorkflow}` : ''
}`
: styles.approval
}
className={[
styles.approval,
variant === 'floating' && styles.floating,
variant === 'floating' && isExitPlanApproval && styles.floatingWorkflow,
isGoal && styles.goalApproval,
]
.filter(Boolean)
.join(' ')}
data-web-shell-permission-panel
data-web-shell-goal-approval={isGoal || undefined}
aria-busy={isGoal ? submitting : undefined}
role="alertdialog"
aria-labelledby={headingId}
// Expose the question, the tool description, and the command/content to
@@ -507,7 +541,7 @@ export function ToolApproval({
aria-describedby={[
questionId,
descriptionText ? descId : null,
showsCommandBlock ? commandId : null,
showsCommandBlock || isGoal ? commandId : null,
]
.filter(Boolean)
.join(' ')}
@@ -528,7 +562,14 @@ export function ToolApproval({
</div>
)}
{isExec && command ? (
{isGoal ? (
<GoalApprovalContent
key={request.id}
id={commandId}
objective={goalObjective}
content={contentText || (goalObjective ? '' : request.title || '')}
/>
) : isExec && command ? (
<div className={styles.code}>
<pre className={styles.codeBlock} id={commandId} title={command}>
{command}
@@ -564,10 +605,25 @@ export function ToolApproval({
</div>
)}
<div className={styles.question} id={questionId}>
<div
className={isGoal ? styles.goalHint : styles.question}
id={questionId}
>
{questionText}
</div>
{isGoal && (
<div className={styles.goalFeedback}>
{submissionFailed ? (
<span role="alert">{t('approval.goal.failed')}</span>
) : (
<span role="status">
{submitting ? t('approval.goal.pending') : ''}
</span>
)}
</div>
)}
{/* radiogroup semantics — the approval choice is single-select. No label
on the group: the alertdialog already exposes the question via
aria-describedby, so labelling the container with the same text would
@@ -587,7 +643,7 @@ export function ToolApproval({
isSelected ? styles.optionActive : ''
}`}
data-web-shell-permission-option
disabled={disabled}
disabled={disabled || (isGoal && submitting)}
data-option-id={option.id}
tabIndex={isSelected ? 0 : -1}
role="radio"
@@ -196,3 +196,75 @@ async function capture(
): Promise<void> {
await page.screenshot({ path: testInfo.outputPath(name), fullPage: true });
}
for (const viewport of [
{ width: 1280, height: 720 },
{ width: 320, height: 568 },
{ width: 844, height: 390 },
]) {
test(`goal approval keeps actions stable while reading ${viewport.width}x${viewport.height}`, async ({
page,
}, testInfo) => {
await page.setViewportSize(viewport);
const scenario = createWebShellDaemonScenario();
const daemon = await installScenario(page, scenario, testInfo);
await gotoSession(page, scenario, daemon);
const objective = `Outcome: Review all open PRs. Done when: ${'Each PR has evidence. '.repeat(100)}Must not: Push or comment. Budget: 20 turns. On block: Report missing access. Context: Preserve this exact text.`;
const content = `Replace the paused Goal and start working toward this objective?\n\n${objective}`;
await daemon.sendEvent({
id: 10,
v: 1,
type: 'permission_request',
data: {
requestId: 'goal-review',
sessionId: scenario.sessionId,
toolCall: {
toolCallId: 'goal-review',
title: `Propose Goal: ${objective}`,
kind: 'other',
rawInput: { objective },
_meta: { toolName: 'propose_goal' },
content: [
{ type: 'content', content: { type: 'text', text: content } },
],
},
options: [
{ optionId: 'allow_once', label: 'Allow once', kind: 'allow_once' },
{ optionId: 'reject_once', label: 'Reject', kind: 'reject_once' },
],
},
});
const card = page.locator('[data-web-shell-goal-approval]');
await expect(card).toBeVisible();
const approve = card.getByRole('radio', { name: 'Set goal and continue' });
const beforeCard = await card.boundingBox();
const beforeButton = await approve.boundingBox();
expect(beforeCard).not.toBeNull();
expect(beforeButton).not.toBeNull();
expect(beforeButton!.y).toBeGreaterThanOrEqual(beforeCard!.y);
expect(beforeButton!.y + beforeButton!.height).toBeLessThanOrEqual(
Math.min(viewport.height, beforeCard!.y + beforeCard!.height),
);
await card.getByRole('tabpanel').hover();
expect(await card.boundingBox()).toEqual(beforeCard);
expect(await approve.boundingBox()).toEqual(beforeButton);
await page.mouse.move(0, 0);
expect(await card.boundingBox()).toEqual(beforeCard);
await card.getByRole('tab', { name: 'Full content' }).click();
const reading = card.getByRole('tabpanel');
expect(await reading.textContent()).toBe(content);
await reading.hover();
await page.mouse.wheel(0, 800);
await expect
.poll(() => reading.evaluate((element) => element.scrollTop))
.toBeGreaterThan(0);
expect(await card.boundingBox()).toEqual(beforeCard);
expect(await approve.boundingBox()).toEqual(beforeButton);
expect(daemon.permissionRequests()).toHaveLength(0);
await approve.click();
await expect.poll(() => daemon.permissionRequests().length).toBe(1);
expect(daemon.permissionRequests()[0].body).toEqual({
outcome: { outcome: 'selected', optionId: 'allow_once' },
});
});
}
+30
View File
@@ -539,6 +539,21 @@ const EN: Messages = {
: `Vision bridge (${v?.target ?? ''}) failed: the vision bridge could not run. The image was not interpreted.`,
'visionBridge.ok': (v) =>
`Converted ${v?.convertedCount ?? 0} image(s)${Number(v?.omittedCount ?? 0) > 0 ? ` (${v?.omittedCount ?? 0} image(s) omitted)` : ''} to text via ${v?.target ?? ''}.${v?.egressOccurred === 1 ? ' Your image and prompt/context were sent to that model.' : ''}`,
'approval.goal.title': 'Confirm session goal',
'approval.goal.overview': 'Goal overview',
'approval.goal.full': 'Full content',
'approval.goal.outcome': 'Outcome',
'approval.goal.doneWhen': 'Completion criteria',
'approval.goal.mustNot': 'Constraints',
'approval.goal.budget': 'Budget',
'approval.goal.onBlock': 'When blocked',
'approval.goal.context': 'Context',
'approval.goal.confirm': 'Set goal and continue',
'approval.goal.reject': 'Not now',
'approval.goal.hint':
'Setting this goal continues execution and checks progress after each turn. Blockers will be reported.',
'approval.goal.pending': 'Submitting…',
'approval.goal.failed': 'Could not submit your choice. Please try again.',
'approval.execQuestion': (v) => `Allow execution of: '${v?.tool ?? ''}'?`,
'approval.changeQuestion': 'Apply this change?',
'approval.launchAgentQuestion': 'Launch this agent?',
@@ -4333,6 +4348,21 @@ const ZH: Messages = {
: `视觉桥接(${v?.target ?? ''})失败:视觉桥接无法运行。图片未被解析。`,
'visionBridge.ok': (v) =>
`已通过 ${v?.target ?? ''} 将 ${v?.convertedCount ?? 0} 张图片转换为文本${Number(v?.omittedCount ?? 0) > 0 ? `(已忽略 ${v?.omittedCount ?? 0} 张图片)` : ''}。${v?.egressOccurred === 1 ? '你的图片及提示词/上下文已发送至该模型。' : ''}`,
'approval.goal.title': '确认会话目标',
'approval.goal.overview': '目标概览',
'approval.goal.full': '完整内容',
'approval.goal.outcome': '目标',
'approval.goal.doneWhen': '完成条件',
'approval.goal.mustNot': '执行边界',
'approval.goal.budget': '预算',
'approval.goal.onBlock': '遇到阻塞时',
'approval.goal.context': '上下文',
'approval.goal.confirm': '设置并继续',
'approval.goal.reject': '暂不设置',
'approval.goal.hint':
'设置后将继续执行,并在每轮结束时检查目标进展;遇到阻塞会说明原因。',
'approval.goal.pending': '正在提交…',
'approval.goal.failed': '提交失败,请重试。',
'approval.execQuestion': (v) => `允许执行:'${v?.tool ?? ''}'?`,
'approval.changeQuestion': '是否继续?',
'approval.launchAgentQuestion': '启动这个 agent?',