fix: complete goal resume and comment input follow-ups (#3361)

This commit is contained in:
Bohdan Triapitsyn
2026-09-05 10:53:34 +03:00
committed by GitHub
parent f3f844463b
commit 7c9fdd1ee5
9 changed files with 172 additions and 62 deletions
@@ -1,3 +1,4 @@
import { isIMECompositionEvent } from '@/lib/ime';
import React from 'react';
import { toast } from '@/components/ui';
@@ -272,7 +273,7 @@ const WebviewBrowser: React.FC<BrowserPaneProps> = ({ initialUrl, directory, tab
React.useEffect(() => {
if (!isAnnotating) return;
const handler = (event: KeyboardEvent) => {
if (event.key !== 'Escape') return;
if (isIMECompositionEvent(event) || event.key !== 'Escape') return;
event.preventDefault();
event.stopImmediatePropagation();
setIsAnnotating(false);
@@ -1,61 +1,84 @@
import React from 'react';
import { describe, expect, mock, test } from 'bun:test';
import { renderToStaticMarkup } from 'react-dom/server';
import React, { act } from 'react';
import { Window } from 'happy-dom';
import { describe, expect, test } from 'bun:test';
import { createRoot } from 'react-dom/client';
import { createOpencodeClient } from '@opencode-ai/sdk/v2';
import { SyncProvider } from '@/sync/sync-context';
import { ThemeSystemProvider } from '@/contexts/ThemeSystemContext';
import { I18nProvider } from '@/lib/i18n';
import { getDefaultTheme } from '@/lib/theme/themes';
mock.module('@/components/chat/SessionGoalRow', () => ({
SessionGoalRow: () => null,
}));
mock.module('@/components/chat/SessionSuggestionChip', () => ({
SessionSuggestionChip: () => null,
}));
mock.module('./ComposerAttachmentControls', () => ({
ComposerAttachmentControls: () => null,
}));
import { MobilePillComposer } from './MobilePillComposer';
const { MobilePillComposer } = await import('./MobilePillComposer');
const renderPill = (options: { hasContent: boolean; newSessionDraftOpen: boolean; canAbort?: boolean }) => renderToStaticMarkup(
<I18nProvider>
<MobilePillComposer
message={options.hasContent ? 'Draft message' : ''}
sessionId={options.newSessionDraftOpen ? null : 'session-1'}
newSessionDraftOpen={options.newSessionDraftOpen}
hasContent={options.hasContent}
isVSCode={false}
canAbort={options.canAbort ?? false}
footerIconButtonClass="icon-button"
iconSizeClass="icon-size"
sendIconSizeClass="send-icon-size"
stopIconSizeClass="stop-icon-size"
theme={getDefaultTheme(false)}
onExpand={() => {}}
onApplySuggestion={() => {}}
onPrimaryAction={() => {}}
onNewSession={() => {}}
onPickLocalFiles={() => {}}
onOpenIssuePicker={() => {}}
onOpenPrPicker={() => {}}
onOpenAttachSheet={() => {}}
onStartDictation={() => {}}
onAbort={() => {}}
/>
</I18nProvider>,
);
const renderPill = async (options: { hasContent: boolean; newSessionDraftOpen: boolean; canAbort?: boolean }) => {
const win = new Window({ url: 'http://localhost' });
const values = { window: win, document: win.document, navigator: win.navigator, localStorage: win.localStorage, IS_REACT_ACT_ENVIRONMENT: true };
const previous = new Map(Object.keys(values).map((key) => [key, Object.getOwnPropertyDescriptor(globalThis, key)]));
for (const [key, value] of Object.entries(values)) Object.defineProperty(globalThis, key, { configurable: true, value });
const container = document.createElement('div');
const root = createRoot(container);
let primaryActions = 0;
try {
await act(async () => root.render(
<SyncProvider directory="/fixture" sdk={createOpencodeClient({ baseUrl: "http://opencode.test", fetch: async () => new Response("[]", { headers: { "content-type": "application/json" } }) })}>
<ThemeSystemProvider>
<I18nProvider>
<MobilePillComposer
directory="/fixture"
message={options.hasContent ? 'Draft message' : ''}
sessionId={options.newSessionDraftOpen ? null : 'session-1'}
newSessionDraftOpen={options.newSessionDraftOpen}
hasContent={options.hasContent}
isVSCode={false}
canAbort={options.canAbort ?? false}
footerIconButtonClass="icon-button"
iconSizeClass="icon-size"
sendIconSizeClass="send-icon-size"
stopIconSizeClass="stop-icon-size"
theme={getDefaultTheme(false)}
onExpand={() => {}}
onApplySuggestion={() => {}}
onPrimaryAction={() => { primaryActions += 1; }}
onNewSession={() => {}}
onPickLocalFiles={() => {}}
onOpenIssuePicker={() => {}}
onOpenPrPicker={() => {}}
onOpenAttachSheet={() => {}}
onStartDictation={() => {}}
onAbort={() => {}}
/>
</I18nProvider>
</ThemeSystemProvider>
</SyncProvider>));
const send = container.querySelector<HTMLButtonElement>('[aria-label="Send message"]');
if (options.hasContent) {
expect(send).not.toBeNull();
await act(async () => { send?.click(); });
expect(primaryActions).toBe(1);
}
return container.innerHTML;
} finally {
await act(async () => root.unmount());
for (const [key, descriptor] of previous) {
if (descriptor) Object.defineProperty(globalThis, key, descriptor);
else Reflect.deleteProperty(globalThis, key);
}
await win.happyDOM.close();
}
};
describe('MobilePillComposer', () => {
test('uses the inline action to send content while the session is idle', () => {
const markup = renderPill({ hasContent: true, newSessionDraftOpen: false });
test('uses the inline action to send content while the session is idle', async () => {
const markup = await renderPill({ hasContent: true, newSessionDraftOpen: false });
expect(markup).toContain('aria-label="Send message"');
expect(markup).toContain('aria-label="New chat"');
expect(markup.indexOf('aria-label="Send message"')).toBeLessThan(markup.indexOf('aria-label="New chat"'));
});
test('uses the trailing action to send content while the session is running', () => {
const markup = renderPill({ hasContent: true, newSessionDraftOpen: false, canAbort: true });
test('uses the trailing action to send content while the session is running', async () => {
const markup = await renderPill({ hasContent: true, newSessionDraftOpen: false, canAbort: true });
expect(markup).toContain('aria-label="Stop generating"');
expect(markup).toContain('aria-label="Send message"');
@@ -63,29 +86,29 @@ describe('MobilePillComposer', () => {
expect(markup.indexOf('aria-label="Stop generating"')).toBeLessThan(markup.indexOf('aria-label="Send message"'));
});
test('uses the inline send action for content in a new-session draft', () => {
const markup = renderPill({ hasContent: true, newSessionDraftOpen: true });
test('uses the inline send action for content in a new-session draft', async () => {
const markup = await renderPill({ hasContent: true, newSessionDraftOpen: true });
expect(markup).toContain('aria-label="Send message"');
expect(markup).toContain('w-0 opacity-0 overflow-hidden');
});
test('keeps the new-session action for an empty existing session', () => {
const markup = renderPill({ hasContent: false, newSessionDraftOpen: false });
test('keeps the new-session action for an empty existing session', async () => {
const markup = await renderPill({ hasContent: false, newSessionDraftOpen: false });
expect(markup).toContain('aria-label="New chat"');
expect(markup).not.toContain('aria-label="Send message"');
});
test('keeps the trailing action collapsed for an empty new-session draft', () => {
const markup = renderPill({ hasContent: false, newSessionDraftOpen: true });
test('keeps the trailing action collapsed for an empty new-session draft', async () => {
const markup = await renderPill({ hasContent: false, newSessionDraftOpen: true });
expect(markup).toContain('w-0 opacity-0 overflow-hidden');
expect(markup).not.toContain('aria-label="Send message"');
});
test('keeps abort and new-session actions while a session runs without content', () => {
const markup = renderPill({ hasContent: false, newSessionDraftOpen: false, canAbort: true });
test('keeps abort and new-session actions while a session runs without content', async () => {
const markup = await renderPill({ hasContent: false, newSessionDraftOpen: false, canAbort: true });
expect(markup).toContain('aria-label="Stop generating"');
expect(markup).toContain('aria-label="New chat"');
@@ -11,6 +11,7 @@
* is running, abort keeps that slot and the outer new-session action sends.
*/
import { Button } from '@/components/ui/button';
import { Icon } from '@/components/icon/Icon';
import { StopIcon } from '@/components/icons/StopIcon';
import { SessionGoalRow } from '@/components/chat/SessionGoalRow';
@@ -165,15 +166,17 @@ export function MobilePillComposer(props: MobilePillComposerProps) {
<StopIcon className={cn(stopIconSizeClass)} />
</button>
) : canPrimaryAction ? (
<button
<Button
type="button"
className={cn(footerIconButtonClass, 'text-primary hover:text-primary')}
variant="ghost"
size="icon"
className="text-primary hover:text-primary"
onClick={onPrimaryAction}
title={t('chat.chatInput.actions.sendMessageAria')}
aria-label={t('chat.chatInput.actions.sendMessageAria')}
>
<Icon name="send-plane-2" className={cn(sendIconSizeClass)} />
</button>
</Button>
) : null}
</div>
{/* While running, Send moves outside because Abort owns the pill's
@@ -1,3 +1,5 @@
import { readFileSync } from 'node:fs';
import { Window } from 'happy-dom';
import { describe, expect, test } from 'bun:test';
import {
@@ -80,6 +82,41 @@ describe('annotation overlay script', () => {
expect(attachCall).toBeGreaterThan(imeGuard);
});
test('preserves the overlay for composition Escape and cancels on ordinary Escape', async () => {
const win = new Window({ url: 'http://annotation.test' });
const run = new Function('window', 'document', 'requestAnimationFrame', `return ${script}`);
try {
const completion = run(win, win.document, (callback: FrameRequestCallback) => callback(0));
const hostCount = win.document.body.children.length;
expect(hostCount).toBeGreaterThan(0);
for (const options of [{ isComposing: true }, { keyCode: 229 }]) {
const event = new win.KeyboardEvent('keydown', { key: 'Escape', bubbles: true, cancelable: true, ...options });
win.dispatchEvent(event);
expect(event.defaultPrevented).toBe(false);
expect(win.document.body.children.length).toBe(hostCount);
}
const escape = new win.KeyboardEvent('keydown', { key: 'Escape', bubbles: true, cancelable: true });
win.dispatchEvent(escape);
expect(escape.defaultPrevented).toBe(true);
expect(await completion).toBeNull();
expect(win.document.body.children.length).toBe(0);
} finally {
await win.happyDOM.close();
}
});
test('guards app-side annotation Escape before cancelling the session', () => {
const source = readFileSync(new URL('../../components/browser/BrowserPane.tsx', import.meta.url), 'utf8');
const start = source.indexOf('const handler = (event: KeyboardEvent)');
const end = source.indexOf("window.addEventListener('keydown', handler, true)", start);
expect(start).toBeGreaterThan(-1);
expect(end).toBeGreaterThan(start);
const handler = source.slice(start, end);
const guard = handler.indexOf("if (isIMECompositionEvent(event) || event.key !== 'Escape') return;");
expect(guard).toBeGreaterThan(-1);
expect(handler.indexOf('cancelAnnotationSession(annotationHost)')).toBeGreaterThan(guard);
});
test('escapes a label that would otherwise close the script', () => {
const hostile = buildAnnotationOverlayScript(theme, {
...labels,
@@ -535,6 +535,7 @@ export const buildAnnotationOverlayScript = (
};
var onKeyDown = function (event) {
if (event.isComposing || event.keyCode === 229) return;
if (event.key === 'Escape') {
event.preventDefault();
event.stopImmediatePropagation();
+1 -1
View File
@@ -2635,7 +2635,7 @@ export function SyncProvider(props: {
stopped = true
clearInterval(interval)
}
}, [childStores, triggerDirectoryResync])
}, [childStores, props.sdk, triggerDirectoryResync])
// Ensure current directory's child store exists
useEffect(() => {
@@ -124,7 +124,10 @@ before touching the filesystem). Rationale: metadata rides every
The consecutive state is derived from the loaded message history, not
persisted, using `info.time.created` chronology rather than message IDs.
Summary messages are not agent turns; an ordinary completed assistant
turn naturally breaks the consecutive condition;
turn naturally breaks the consecutive condition. Explicit Resume grants
one new recovery attempt over the same transcript; the continuation
consumes that permission, so another truncation blocks again. Resume
does not bypass assistant errors or the token budget;
- otherwise, small-model audit of the objective + the last assistant turn
only — no conversation history and no continuation prompts
(`restrictToPreferredProvider`, session's own provider/model preferred):
@@ -707,7 +707,7 @@ export const createSessionGoalRuntime = ({
// A second consecutive completed, non-summary length-truncated turn is a
// bounded recovery failure. Derive this from the loaded transcript rather
// than persisting another goal counter.
if (lengthTail && hasRepeatedLengthTail(messages, lastAssistant, goal.createdAt)) {
if (lengthTail && goal.statusReason !== 'resumed' && hasRepeatedLengthTail(messages, lastAssistant, goal.createdAt)) {
await settleGoal({
sessionId, directory, goal, status: 'blocked', statusReason: 'repeated output truncation', tokensUsed, tokensBaseline, tokensCommitted, lastAccountedMessageID,
});
@@ -77,7 +77,10 @@ const createRuntimeHarness = ({ messages, messageFactory, goalOverrides = {}, ma
const fetchImpl = vi.fn(async (input, init = {}) => {
const pathname = requestPath(input);
requests.push({ pathname, method: init.method ?? 'GET', body: init.body });
if (pathname === `/session/${SESSION_ID}` && init.method === 'PATCH') return jsonResponse(activeSession);
if (pathname === `/session/${SESSION_ID}` && init.method === 'PATCH') {
activeSession.metadata = JSON.parse(init.body).metadata;
return jsonResponse(activeSession);
}
if (pathname === `/session/${SESSION_ID}`) return jsonResponse(activeSession);
if (pathname === '/session/status') return jsonResponse({});
if (pathname === `/session/${SESSION_ID}/children`) return jsonResponse([]);
@@ -98,7 +101,7 @@ const createRuntimeHarness = ({ messages, messageFactory, goalOverrides = {}, ma
idleQuietMs: 10,
maxAutoTurns,
});
return { runtime, requests, service };
return { runtime, requests, service, activeSession };
};
const runIdleTick = async (runtime) => {
@@ -420,6 +423,45 @@ describe('session goal live activity gate', () => {
runtime.stop();
});
it('allows one explicit Resume after repeated truncation, then blocks another cutoff', async () => {
const first = assistantMessage('first', { finish: 'length', time: { created: 10, completed: 11 } });
const second = assistantMessage('second', { finish: 'length', time: { created: 20, completed: 21 } });
const messages = [first, second];
const { runtime, requests, activeSession } = createRuntimeHarness({ messages });
try {
await runIdleTick(runtime);
expect(lastPatchedGoal(requests).status).toBe('blocked');
expect(requests.filter((request) => request.pathname.endsWith('/prompt_async'))).toHaveLength(0);
Object.assign(activeSession.metadata.openchamber.goal, { status: 'active', statusReason: 'resumed', turnsUsed: 0 });
await runIdleTick(runtime);
expect(lastPatchedGoal(requests)).toMatchObject({ status: 'active', statusReason: '', turnsUsed: 1 });
expect(requests.filter((request) => request.pathname.endsWith('/prompt_async'))).toHaveLength(1);
messages.push(assistantMessage('third', { finish: 'length', time: { created: 30, completed: 31 } }));
await runIdleTick(runtime);
expect(lastPatchedGoal(requests)).toMatchObject({ status: 'blocked', statusReason: 'repeated output truncation' });
expect(requests.filter((request) => request.pathname.endsWith('/prompt_async'))).toHaveLength(1);
} finally {
runtime.stop();
}
});
it.each([
{ tokenBudget: 1, error: undefined, expectedStatus: 'budgetLimited' },
{ tokenBudget: null, error: { name: 'APIError' }, expectedStatus: 'blocked' },
])('keeps $expectedStatus protection on explicit Resume', async ({ tokenBudget, error, expectedStatus }) => {
const { runtime, requests } = createRuntimeHarness({
goalOverrides: { statusReason: 'resumed', tokenBudget },
messages: [assistantMessage('resumed', { finish: 'length', error })],
});
try {
await runIdleTick(runtime);
expect(lastPatchedGoal(requests).status).toBe(expectedStatus);
expect(requests.filter((request) => request.pathname.endsWith('/prompt_async'))).toHaveLength(0);
} finally {
runtime.stop();
}
});
it('continues after a truncated agent turn followed by a length-finished summary', async () => {
const firstLength = assistantMessage('agent-length', { finish: 'length', time: { created: 10, completed: 11 } });
const summary = assistantMessage('summary', {