From 7c9fdd1ee557f4df9cebeeb3069f02e679301f0e Mon Sep 17 00:00:00 2001 From: Bohdan Triapitsyn Date: Sat, 5 Sep 2026 10:53:34 +0300 Subject: [PATCH] fix: complete goal resume and comment input follow-ups (#3361) --- .../ui/src/components/browser/BrowserPane.tsx | 3 +- .../composer/ui/MobilePillComposer.test.tsx | 129 +++++++++++------- .../chat/composer/ui/MobilePillComposer.tsx | 9 +- .../src/lib/browser/annotationOverlay.test.ts | 37 +++++ .../ui/src/lib/browser/annotationOverlay.ts | 1 + packages/ui/src/sync/sync-context.tsx | 2 +- .../server/lib/session-goal/DOCUMENTATION.md | 5 +- .../web/server/lib/session-goal/runtime.js | 2 +- .../server/lib/session-goal/runtime.test.js | 46 ++++++- 9 files changed, 172 insertions(+), 62 deletions(-) diff --git a/packages/ui/src/components/browser/BrowserPane.tsx b/packages/ui/src/components/browser/BrowserPane.tsx index 3828ca2b..638b9547 100644 --- a/packages/ui/src/components/browser/BrowserPane.tsx +++ b/packages/ui/src/components/browser/BrowserPane.tsx @@ -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 = ({ 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); diff --git a/packages/ui/src/components/chat/composer/ui/MobilePillComposer.test.tsx b/packages/ui/src/components/chat/composer/ui/MobilePillComposer.test.tsx index 46a499ae..b97d22f3 100644 --- a/packages/ui/src/components/chat/composer/ui/MobilePillComposer.test.tsx +++ b/packages/ui/src/components/chat/composer/ui/MobilePillComposer.test.tsx @@ -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( - - {}} - onApplySuggestion={() => {}} - onPrimaryAction={() => {}} - onNewSession={() => {}} - onPickLocalFiles={() => {}} - onOpenIssuePicker={() => {}} - onOpenPrPicker={() => {}} - onOpenAttachSheet={() => {}} - onStartDictation={() => {}} - onAbort={() => {}} - /> - , -); +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( + new Response("[]", { headers: { "content-type": "application/json" } }) })}> + + + {}} + onApplySuggestion={() => {}} + onPrimaryAction={() => { primaryActions += 1; }} + onNewSession={() => {}} + onPickLocalFiles={() => {}} + onOpenIssuePicker={() => {}} + onOpenPrPicker={() => {}} + onOpenAttachSheet={() => {}} + onStartDictation={() => {}} + onAbort={() => {}} + /> + + + )); + const send = container.querySelector('[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"'); diff --git a/packages/ui/src/components/chat/composer/ui/MobilePillComposer.tsx b/packages/ui/src/components/chat/composer/ui/MobilePillComposer.tsx index 4221f74b..3a51c4e8 100644 --- a/packages/ui/src/components/chat/composer/ui/MobilePillComposer.tsx +++ b/packages/ui/src/components/chat/composer/ui/MobilePillComposer.tsx @@ -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) { ) : canPrimaryAction ? ( - + ) : null} {/* While running, Send moves outside because Abort owns the pill's diff --git a/packages/ui/src/lib/browser/annotationOverlay.test.ts b/packages/ui/src/lib/browser/annotationOverlay.test.ts index c1869813..de7cee94 100644 --- a/packages/ui/src/lib/browser/annotationOverlay.test.ts +++ b/packages/ui/src/lib/browser/annotationOverlay.test.ts @@ -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, diff --git a/packages/ui/src/lib/browser/annotationOverlay.ts b/packages/ui/src/lib/browser/annotationOverlay.ts index 012daf19..f3ce9cba 100644 --- a/packages/ui/src/lib/browser/annotationOverlay.ts +++ b/packages/ui/src/lib/browser/annotationOverlay.ts @@ -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(); diff --git a/packages/ui/src/sync/sync-context.tsx b/packages/ui/src/sync/sync-context.tsx index 51babed5..f27382aa 100644 --- a/packages/ui/src/sync/sync-context.tsx +++ b/packages/ui/src/sync/sync-context.tsx @@ -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(() => { diff --git a/packages/web/server/lib/session-goal/DOCUMENTATION.md b/packages/web/server/lib/session-goal/DOCUMENTATION.md index 0a5dbbd4..eb327228 100644 --- a/packages/web/server/lib/session-goal/DOCUMENTATION.md +++ b/packages/web/server/lib/session-goal/DOCUMENTATION.md @@ -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): diff --git a/packages/web/server/lib/session-goal/runtime.js b/packages/web/server/lib/session-goal/runtime.js index 3a119cb5..39e1f600 100644 --- a/packages/web/server/lib/session-goal/runtime.js +++ b/packages/web/server/lib/session-goal/runtime.js @@ -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, }); diff --git a/packages/web/server/lib/session-goal/runtime.test.js b/packages/web/server/lib/session-goal/runtime.test.js index fab9935f..ba7a8a21 100644 --- a/packages/web/server/lib/session-goal/runtime.test.js +++ b/packages/web/server/lib/session-goal/runtime.test.js @@ -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', {