diff --git a/apps/desktop/src/main/__tests__/permission-response-ipc-boundary.test.ts b/apps/desktop/src/main/__tests__/permission-response-ipc-boundary.test.ts index 5b186c081d..0bfb34b3b0 100644 --- a/apps/desktop/src/main/__tests__/permission-response-ipc-boundary.test.ts +++ b/apps/desktop/src/main/__tests__/permission-response-ipc-boundary.test.ts @@ -286,6 +286,31 @@ describe('permission response IPC boundary', () => { ); }); + it('accepts a retained-attachment-only edit without inline text', () => { + // A normal edit can keep an existing attachment while dropping all inline + // text; the retained refs travel separately from attachmentItems and must + // count as content before the empty-body rejection (#4804). + const command = normalizeSessionSendCommand({ + type: 'send', + text: ' ', + retainedAttachments: [ + { + kind: 'image', + name: 'kept.png', + mimeType: 'image/png', + bytes: 12, + ref: { + kind: 'session_file', + sessionId: 'session-1', + relativePath: 'attachments/kept.png', + }, + }, + ], + }); + assert.equal(command?.retainedAttachments?.length, 1); + assert.equal(command?.retainedAttachments?.[0]?.name, 'kept.png'); + }); + it('accepts only the supported stop source', () => { assert.deepEqual(normalizeStopSessionInput(undefined), {}); assert.deepEqual( diff --git a/apps/desktop/src/main/__tests__/quote-companion-retry.test.ts b/apps/desktop/src/main/__tests__/quote-companion-retry.test.ts index 159cbb3311..1c299a0407 100644 --- a/apps/desktop/src/main/__tests__/quote-companion-retry.test.ts +++ b/apps/desktop/src/main/__tests__/quote-companion-retry.test.ts @@ -40,6 +40,7 @@ import { WorkbarServicesProvider, type CompanionQuoteSnapshot, type StagedCompanionQuote, + type WorkbarIngestInput, type WorkbarServices, } from '../../renderer/features/workbar/testing.js'; @@ -57,6 +58,11 @@ const originalGlobals = { let mountedRoot: Root | undefined; const SOURCE_SESSION = session('source-session'); type SideChatStopTarget = Parameters[1]; +type SteerFn = ( + text: string, + attachmentItems?: WorkbarIngestInput[], + onAdmitted?: () => void, +) => Promise; type QueueUpdate = Extract; type QueueEntry = NonNullable[number]; @@ -134,7 +140,7 @@ async function renderProbe( modelChoices?: readonly ChatModelChoice[]; ready?: (container: Element) => boolean; onSend?: (send: (text: string) => Promise) => void; - onSteer?: (steer: (text: string) => Promise) => void; + onSteer?: (steer: SteerFn) => void; onStop?: (stop: () => Promise) => void; onSetPermissionMode?: (set: (mode: PermissionMode) => Promise) => void; confirmBypass?: () => Promise; @@ -199,7 +205,7 @@ async function renderOwnershipProbe( } = {}, ) { let send!: (text: string) => Promise; - let steer!: (text: string) => Promise; + let steer!: SteerFn; let stop!: () => Promise; let setPermissionMode!: (mode: PermissionMode) => Promise; let eventHandler: ((event: SessionEvent) => void) | undefined; @@ -228,7 +234,8 @@ async function renderOwnershipProbe( return { ...rendered, send: (text: string) => send(text), - steer: (text: string) => steer(text), + steer: (text: string, attachmentItems?: WorkbarIngestInput[], onAdmitted?: () => void) => + steer(text, attachmentItems, onAdmitted), stop: () => stop(), setPermissionMode: (mode: PermissionMode) => setPermissionMode(mode), emit(event: SessionEvent) { @@ -1722,6 +1729,64 @@ test('continues projecting the active Turn while a steer awaits Host admission', }); }); +test('consumes a steered attachment when the started turn binds the admission', async () => { + const pendingSteer = deferred<{ kind: 'started'; turnId: string }>(); + let admissionId: string | undefined; + let admitted = 0; + let steerPayload: { attachmentItems?: readonly WorkbarIngestInput[] } | undefined; + const attachmentItem: WorkbarIngestInput = { approvalId: 'approval-1', name: 'kept.png' }; + const { container, emit, send, steer } = await renderOwnershipProbe({ + send: async () => ({ ok: true as const, turnId: 'old-turn' }), + steer: async (_sessionId, _text, requestedAdmissionId, payload) => { + admissionId = requestedAdmissionId; + steerPayload = payload; + return pendingSteer.promise; + }, + }); + + await act(async () => { + assert.equal(await send('initial prompt'), true); + await Promise.resolve(); + }); + let steerResult!: Promise; + await act(async () => { + steerResult = steer('steer with the kept image', [attachmentItem], () => { + admitted += 1; + }); + await Promise.resolve(); + }); + await waitUntil(() => admissionId !== undefined); + + await act(async () => { + pendingSteer.resolve({ kind: 'started', turnId: 'steer-started-turn' }); + assert.equal(await steerResult, true); + await Promise.resolve(); + }); + + // The attachments travel with the steering Message... + assert.deepEqual(steerPayload, { attachmentItems: [attachmentItem] }); + assert.equal( + container.firstElementChild?.getAttribute('data-live-turn-id'), + 'steer-started-turn', + ); + // ...and binding the started turn IS the admission boundary: the consumer + // fires exactly once here, not on the later admission echo. + assert.equal(admitted, 1); + + await act(async () => { + emit( + messageAdmittedEvent( + 'late-admission-echo', + 'steer-started-turn', + 1, + admissionId as string, + ), + ); + await Promise.resolve(); + }); + assert.equal(admitted, 1, 'the admission echo must not consume a second time'); +}); + test('fails a send when observation seed rejects and resubscribes for retry', async () => { let sendCalls = 0; let subscriptionCount = 0; @@ -2012,7 +2077,7 @@ function QuoteCompanionProbe(props: { function QuoteCompanionOwnershipProbe(props: { onSend: (send: (text: string) => Promise) => void; - onSteer?: (steer: (text: string) => Promise) => void; + onSteer?: (steer: SteerFn) => void; onStop?: (stop: () => Promise) => void; onSetPermissionMode?: (set: (mode: PermissionMode) => Promise) => void; onContextCompactionError?: (sessionId: string, error: unknown) => void; @@ -2119,3 +2184,167 @@ async function awaitCompanion(container: Element, id = 'side-conversation'): Pro async function awaitProcessing(container: Element): Promise { await waitUntil(() => container.firstElementChild?.getAttribute('data-processing') === 'true'); } + +test('a structured-only send (empty text with a staged quote) reaches the fork admission', async () => { + const sendCommands: Array[1]> = []; + const rendered = await renderOwnershipProbe( + { + listTurns: async () => [settledTurn('done-turn')], + branchFromTurn: async () => ({ ok: true as const, session: session('side-conversation') }), + send: async (_sessionId, command) => { + sendCommands.push(command); + return { ok: true as const, turnId: 'quote-only-turn' }; + }, + }, + { + pendingQuotes: [{ id: 'quote-1', value: { text: 'selected excerpt' } }], + }, + ); + const probe = rendered.container.firstElementChild; + assert.ok(probe); + + // The Composer enables Send once a quote is staged; an empty draft must ride + // the same admission as a text send instead of dying on the `!trimmed` guard. + await act(async () => { + assert.equal(await rendered.send(''), true); + await Promise.resolve(); + }); + await awaitCompanion(rendered.container); + assert.equal(sendCommands.length, 1); + assert.equal(sendCommands[0].text, ''); + assert.deepEqual( + sendCommands[0].quotes?.map((quote) => quote.text), + ['selected excerpt'], + ); + assert.equal(probe.getAttribute('data-error'), ''); +}); + +test('a structured-only steer (empty text with a staged quote) rides the steering contract', async () => { + const steerContents: Array[3]> = []; + const rendered = await renderOwnershipProbe( + { + send: async () => ({ ok: true as const, turnId: 'old-turn' }), + steer: async (_sessionId, _text, _admissionId, content) => { + steerContents.push(content); + return { kind: 'queued', messageId: 'steer-1' }; + }, + }, + { + pendingQuotes: [{ id: 'quote-1', value: { text: 'streaming excerpt' } }], + }, + ); + + await act(async () => { + assert.equal(await rendered.send('initial prompt'), true); + await Promise.resolve(); + }); + await waitUntil( + () => rendered.container.firstElementChild?.getAttribute('data-streaming') === 'true', + ); + + // Streaming steers take the same structured-content contract: the quote alone + // is a valid steering Message, and the `!trimmed` guard must not drop it. + await act(async () => { + assert.equal(await rendered.steer(''), true); + await Promise.resolve(); + }); + assert.equal(steerContents.length, 1); + assert.deepEqual( + steerContents[0]?.quotes?.map((quote) => quote.text), + ['streaming excerpt'], + ); +}); + +test('a steer with staged attachments consumes them only on confirmed admission', async () => { + const admissionIds: string[] = []; + const rendered = await renderOwnershipProbe( + { + send: async () => ({ ok: true as const, turnId: 'old-turn' }), + steer: async (_sessionId, _text, admissionId) => { + const id = admissionId ?? ''; + admissionIds.push(id); + // The reconnect/failure path answers without an admission receipt. + return { kind: 'outcome_unknown', messageId: id }; + }, + }, + { pendingQuotes: [] }, + ); + + await act(async () => { + assert.equal(await rendered.send('initial prompt'), true); + await Promise.resolve(); + }); + await waitUntil( + () => rendered.container.firstElementChild?.getAttribute('data-streaming') === 'true', + ); + + const consumed: string[] = []; + await act(async () => { + assert.equal( + await rendered.steer('', [{ approvalId: 'a-1', name: 'notes.txt' }], () => { + consumed.push('admitted'); + }), + true, + ); + await Promise.resolve(); + }); + // The optimistic accept must not retire the attachments: with no admission + // receipt the Message may still be admitted or retracted by the Host. + assert.deepEqual(consumed, []); + + // The late admission arrives through the fork's event stream; only now does + // the confirmed-admission boundary fire. + await act(async () => { + rendered.emit(messageAdmittedEvent('steer-late-admit', 'steered-turn', 1, admissionIds[0])); + }); + assert.deepEqual(consumed, ['admitted']); +}); + +test('an unknown steer outcome that later retracts keeps the staged attachments', async () => { + const admissionIds: string[] = []; + const rendered = await renderOwnershipProbe( + { + send: async () => ({ ok: true as const, turnId: 'old-turn' }), + steer: async (_sessionId, _text, admissionId) => { + const id = admissionId ?? ''; + admissionIds.push(id); + return { kind: 'outcome_unknown', messageId: id }; + }, + }, + { pendingQuotes: [] }, + ); + + await act(async () => { + assert.equal(await rendered.send('initial prompt'), true); + await Promise.resolve(); + }); + await waitUntil( + () => rendered.container.firstElementChild?.getAttribute('data-streaming') === 'true', + ); + + const consumed: string[] = []; + await act(async () => { + assert.equal( + await rendered.steer('', [{ approvalId: 'a-1', name: 'notes.txt' }], () => { + consumed.push('admitted'); + }), + true, + ); + await Promise.resolve(); + }); + assert.deepEqual(consumed, []); + + // A retraction releases the Message without consuming anything staged: the + // user keeps the attachments and may retry the steer. + await act(async () => { + rendered.emit({ + type: 'message_admission', + id: 'steer-late-retract', + turnId: 'old-turn', + ts: 2, + messageId: admissionIds[0], + outcome: 'retracted', + }); + }); + assert.deepEqual(consumed, []); +}); diff --git a/apps/desktop/src/main/permission-response-guard.ts b/apps/desktop/src/main/permission-response-guard.ts index 9fc25de20e..7becdba4f0 100644 --- a/apps/desktop/src/main/permission-response-guard.ts +++ b/apps/desktop/src/main/permission-response-guard.ts @@ -209,7 +209,25 @@ export function normalizeSessionSendCommand(input: unknown): NormalizedSendSessi const displayText = value.displayText === undefined ? undefined : normalizeSendText(value.displayText); const skillIds = normalizeSessionSkillIds(value.skillIds); - if (!text.trim() && skillIds.length === 0) { + // A send may carry structured content instead of text (a pure quote or a + // pure attachment, #4804). Only the presence is decided here: attachment + // state, ownership, and size limits stay with the ingestion checks, and + // quotes are normalized below before the command is returned. + const quotes = normalizeOptionalQuotes(value.quotes).quotes; + // A normal edit can keep an existing attachment while dropping all inline + // text; the retained refs travel separately from attachmentItems and are + // normalized before the empty-body rejection so a retained-attachment-only + // edit is not refused (#4804). + const retainedAttachments = normalizeOptionalRetainedAttachments(value.retainedAttachments); + const hasAttachmentItems = + Array.isArray(value.attachmentItems) && value.attachmentItems.length > 0; + if ( + !text.trim() && + skillIds.length === 0 && + (quotes?.length ?? 0) === 0 && + !hasAttachmentItems && + (retainedAttachments.retainedAttachments?.length ?? 0) === 0 + ) { throw new Error('Invalid send text'); } return { @@ -220,12 +238,12 @@ export function normalizeSessionSendCommand(input: unknown): NormalizedSendSessi ...(displayText !== undefined ? { displayText } : {}), ...(skillIds.length > 0 ? { skillIds } : {}), ...(value.attachmentItems !== undefined ? { attachmentItems: value.attachmentItems } : {}), - ...normalizeOptionalRetainedAttachments(value.retainedAttachments), + ...retainedAttachments, ...(value.turnOrchestration !== undefined ? { turnOrchestration: normalizeTurnOrchestration(value.turnOrchestration) } : {}), ...normalizeOptionalDirectoryReferences(value.directoryReferences), - ...normalizeOptionalQuotes(value.quotes), + ...(quotes !== undefined ? { quotes } : {}), ...normalizeOptionalWorkspaceFileReferences( value.workspaceFileReferences, displayText ?? text, diff --git a/apps/desktop/src/renderer/features/workbar/ports.ts b/apps/desktop/src/renderer/features/workbar/ports.ts index 4ee8b1cd65..84f02d3ffe 100644 --- a/apps/desktop/src/renderer/features/workbar/ports.ts +++ b/apps/desktop/src/renderer/features/workbar/ports.ts @@ -237,7 +237,12 @@ export interface SideChatSessionPort { sessionId: string, target?: SideChatStopTarget, ): Promise<{ kind: 'retracted'; messageId: string } | undefined>; - steer(sessionId: string, text: string, admissionId?: string): Promise; + steer( + sessionId: string, + text: string, + admissionId?: string, + content?: { quotes?: QuoteRef[]; attachmentItems?: WorkbarIngestInput[] }, + ): Promise; setPermissionMode( sessionId: string, mode: PermissionMode, diff --git a/apps/desktop/src/renderer/features/workbar/testing.ts b/apps/desktop/src/renderer/features/workbar/testing.ts index 11c1e45a56..40e09d330e 100644 --- a/apps/desktop/src/renderer/features/workbar/testing.ts +++ b/apps/desktop/src/renderer/features/workbar/testing.ts @@ -24,6 +24,7 @@ export type { WorkbarServices, WorkbarSessionTracePage, WorkbarSessionUsageSummary, + WorkbarIngestInput, } from './ports.js'; export * from './model/workbar-tabs.js'; diff --git a/apps/desktop/src/renderer/features/workbar/tools/side-chat/quote-companion-panel.tsx b/apps/desktop/src/renderer/features/workbar/tools/side-chat/quote-companion-panel.tsx index 103c8098ff..a2c2d6af11 100644 --- a/apps/desktop/src/renderer/features/workbar/tools/side-chat/quote-companion-panel.tsx +++ b/apps/desktop/src/renderer/features/workbar/tools/side-chat/quote-companion-panel.tsx @@ -346,7 +346,21 @@ export function QuoteCompanionPanel(props: { text, streaming: companion.streaming, compact: companion.compact, - steer: companion.steer, + steer: async (text) => { + // Submitted attachments retire on the confirmed-admission + // boundary, not on the hook's optimistic return: an unknown + // outcome keeps them staged for retry (#4804). + const submitted = pendingAttachments; + const submittedItems = + submitted.length > 0 ? toComposerIngestItems(submitted) : undefined; + return companion.steer( + text, + submittedItems, + submittedItems + ? () => clearSubmittedAttachments(submitted) + : undefined, + ); + }, send: async () => { try { preflightAttachmentItems(pendingAttachments); @@ -357,16 +371,20 @@ export function QuoteCompanionPanel(props: { ); return false; } + // Same admission-boundary retirement as `steer` above. + const submitted = pendingAttachments; + const submittedItems = + submitted.length > 0 ? toComposerIngestItems(submitted) : undefined; const accepted = await companion.send( text, - pendingAttachments.length > 0 - ? toComposerIngestItems(pendingAttachments) + submittedItems, + submittedItems + ? () => clearSubmittedAttachments(submitted) : undefined, ); if (accepted) { props.onPromptAccepted?.(props.panelId, text); } - if (accepted) clearSubmittedAttachments(pendingAttachments); return accepted; }, }) @@ -379,6 +397,8 @@ export function QuoteCompanionPanel(props: { disabled={!companion.modelReady} onPickAttachments={pickAttachments} onAttachFilePaths={attachFilePaths} + // The side chat submits staged context without a prompt (#4804). + allowAttachmentOnlySend pendingAttachments={pendingAttachments} onRemoveAttachment={removeAttachment} mentionSkills={mentions?.mentionSkills} diff --git a/apps/desktop/src/renderer/features/workbar/tools/side-chat/use-quote-companion.ts b/apps/desktop/src/renderer/features/workbar/tools/side-chat/use-quote-companion.ts index 1a65ba4024..b3fb2a88c9 100644 --- a/apps/desktop/src/renderer/features/workbar/tools/side-chat/use-quote-companion.ts +++ b/apps/desktop/src/renderer/features/workbar/tools/side-chat/use-quote-companion.ts @@ -184,10 +184,22 @@ export interface UseQuoteCompanionResult { /** Runs `/compact` against the committed companion fork when it is idle. */ compact: () => Promise; /** Returns whether the send was accepted; false leaves the draft + staged - * quotes in place so the user can retry. */ - send: (text: string, attachmentItems?: WorkbarIngestInput[]) => Promise; - /** Insert text into the active companion turn at the next model step. */ - steer: (text: string) => Promise; + * quotes in place so the user can retry. `onAdmitted` fires only once the + * Host admission is confirmed (never on an unknown outcome), so callers + * can retire submitted attachments on the same boundary as the quotes. */ + send: ( + text: string, + attachmentItems?: WorkbarIngestInput[], + onAdmitted?: () => void, + ) => Promise; + /** Insert text — or a structured-only quote/attachment — into the active + * companion turn at the next model step. `onAdmitted` follows the same + * confirmed-admission boundary as `send`. */ + steer: ( + text: string, + attachmentItems?: WorkbarIngestInput[], + onAdmitted?: () => void, + ) => Promise; setPermissionMode: (mode: PermissionMode) => Promise; regenerate: (turnId: string) => Promise; stop: () => Promise; @@ -850,12 +862,17 @@ export function useQuoteCompanion(input: UseQuoteCompanionInput): UseQuoteCompan async ( text: string, attachmentItems?: WorkbarIngestInput[], + onAdmitted?: () => void, ): Promise => { const trimmed = text.trim(); if (isExactCompactCommand(trimmed)) return compact(); + // A structured-only Message (empty text carrying a quote or an attachment) + // is a valid send since the admission widening (#4804), so the guard + // rejects only when nothing at all is staged. + const quoteSnapshot = snapshotCompanionQuotes(panelId, pendingQuotes); if ( !mountedRef.current || - !trimmed || + (!trimmed && quoteSnapshot.quotes.length === 0 && !attachmentItems?.length) || submitLockRef.current || compactionRequestInFlightRef.current || activeTurnIdRef.current || @@ -868,7 +885,6 @@ export function useQuoteCompanion(input: UseQuoteCompanionInput): UseQuoteCompan setSubmitLocked(true); setError(null); const turnId = crypto.randomUUID(); - const quoteSnapshot = snapshotCompanionQuotes(panelId, pendingQuotes); const label = (quoteSnapshot.quotes[0]?.text ?? trimmed).slice(0, 24); // Show the user's question IMMEDIATELY as an optimistic bubble, before the // fork exists. On a first send `ensureFork` makes a Host round trip, and the @@ -886,7 +902,14 @@ export function useQuoteCompanion(input: UseQuoteCompanionInput): UseQuoteCompan const admission: PendingAdmission = { messageId: turnId, events: [], - consumeOnAdmission: () => onQuotesConsumed(quoteSnapshot), + // Quotes and submitted attachments share one cleanup boundary — + // confirmed Host admission (#4804). An unknown outcome keeps them + // staged until the reconciliation binds the Turn or a retraction + // releases the send, so nothing staged is consumed on a guess. + consumeOnAdmission: () => { + onQuotesConsumed(quoteSnapshot); + onAdmitted?.(); + }, }; setPendingUserMessages((current) => [ ...current.filter((message) => message.id !== turnId), @@ -1120,59 +1143,85 @@ export function useQuoteCompanion(input: UseQuoteCompanionInput): UseQuoteCompan } }, [releaseAdmission, resolveAdmission, sideChat]); - const steer = useCallback(async (text: string): Promise => { - const id = companionIdRef.current; - const trimmed = text.trim(); - if ( - !mountedRef.current || - !id || - !trimmed || - !turnInFlight || - pendingAdmissionRef.current - ) { - return false; - } - const admissionId = crypto.randomUUID(); - const admission: PendingAdmission = { - messageId: admissionId, - events: [], - }; - setPendingAdmission(admission); - try { - const outcome = await sideChat.steer(id, trimmed, admissionId); - if (!mountedRef.current) return false; - if ((await admission.stopPromise) === 'confirmed') return false; - if (admissionOutcomeForMessage(admission.events, admission.messageId)?.kind === 'retracted') { - return false; - } - if (outcome.kind === 'started') { - bindAdmittedTurn(id, outcome.turnId, { preserveLiveTurn: true }); - } else if (resolveAdmission(id, admission, outcome.messageId, true)?.kind === 'retracted') { + const steer = useCallback( + async ( + text: string, + attachmentItems?: WorkbarIngestInput[], + onAdmitted?: () => void, + ): Promise => { + const id = companionIdRef.current; + const trimmed = text.trim(); + // Same structured-only contract as `send`: a quote or an attachment alone + // is a valid steering Message (#4804). + const quoteSnapshot = snapshotCompanionQuotes(panelId, pendingQuotes); + if ( + !mountedRef.current || + !id || + (!trimmed && quoteSnapshot.quotes.length === 0 && !attachmentItems?.length) || + !turnInFlight || + pendingAdmissionRef.current + ) { return false; } - setError(null); - return true; - } catch { - if (mountedRef.current) { - if (pendingAdmissionRef.current === admission) { - releaseAdmission(admission, copyRef.current.errors.sendFailed); - } else if ( - admissionOutcomeForMessage(admission.events, admission.messageId)?.kind !== 'retracted' - ) { - setError(copyRef.current.errors.sendFailed); + const admissionId = crypto.randomUUID(); + const admission: PendingAdmission = { + messageId: admissionId, + events: [], + // Quotes stay staged until the Host admits the steering Message; a + // failed or retracted steer keeps them available for retry. Submitted + // attachments share that boundary: an unknown outcome keeps them + // staged until reconciliation binds the Turn or the steer retracts. + consumeOnAdmission: () => { + if (quoteSnapshot.quotes.length > 0) onQuotesConsumed(quoteSnapshot); + onAdmitted?.(); + }, + }; + setPendingAdmission(admission); + try { + const outcome = await sideChat.steer(id, trimmed, admissionId, { + ...(quoteSnapshot.quotes.length > 0 + ? { quotes: [...quoteSnapshot.quotes] } + : {}), + ...(attachmentItems?.length ? { attachmentItems } : {}), + }); + if (!mountedRef.current) return false; + if ((await admission.stopPromise) === 'confirmed') return false; + if (admissionOutcomeForMessage(admission.events, admission.messageId)?.kind === 'retracted') { + return false; + } + if (outcome.kind === 'started') { + bindAdmittedTurn(id, outcome.turnId, { preserveLiveTurn: true }); + } else if (resolveAdmission(id, admission, outcome.messageId, true)?.kind === 'retracted') { + return false; + } + setError(null); + return true; + } catch { + if (mountedRef.current) { + if (pendingAdmissionRef.current === admission) { + releaseAdmission(admission, copyRef.current.errors.sendFailed); + } else if ( + admissionOutcomeForMessage(admission.events, admission.messageId)?.kind !== 'retracted' + ) { + setError(copyRef.current.errors.sendFailed); + } } + return false; } - return false; - } - }, [ - bindAdmittedTurn, - mountedRef, - releaseAdmission, - resolveAdmission, - setPendingAdmission, - sideChat, - turnInFlight, - ]); + }, + [ + bindAdmittedTurn, + mountedRef, + onQuotesConsumed, + panelId, + pendingQuotes, + releaseAdmission, + resolveAdmission, + setPendingAdmission, + sideChat, + turnInFlight, + ], + ); const setPermissionMode = useCallback( (mode: PermissionMode): Promise => { diff --git a/apps/desktop/src/renderer/platform/desktop/create-workbar-services.ts b/apps/desktop/src/renderer/platform/desktop/create-workbar-services.ts index 39a6e490c5..3c5098ca94 100644 --- a/apps/desktop/src/renderer/platform/desktop/create-workbar-services.ts +++ b/apps/desktop/src/renderer/platform/desktop/create-workbar-services.ts @@ -117,11 +117,15 @@ export function createDesktopWorkbarServices( // Steering is a Message placed at the current Turn's boundary, so it // rides the one admission channel. Runtime Host names the outcome; this // adapter only renames it for the Side Conversation port. - steer: async (sessionId, text, admissionId) => { + steer: async (sessionId, text, admissionId, content) => { const messageId = admissionId ?? crypto.randomUUID(); const result = await bridge.sessions.submitMessage(sessionId, 'current_turn', { messageId, text, + ...(content?.quotes ? { quotes: content.quotes } : {}), + ...(content?.attachmentItems + ? { attachmentItems: content.attachmentItems } + : {}), }); if (!result.ok) { if (result.reason === 'outcome_unknown') { diff --git a/packages/core/src/__tests__/runtime-event.test.ts b/packages/core/src/__tests__/runtime-event.test.ts index a4e8e7ea25..3762431167 100644 --- a/packages/core/src/__tests__/runtime-event.test.ts +++ b/packages/core/src/__tests__/runtime-event.test.ts @@ -22,6 +22,7 @@ import assert from 'node:assert/strict'; import { createHash } from 'node:crypto'; import { decodeMessageContent, + hasMeaningfulMessageContent, isCanonicalStorageRef, messageContentsEqual, normalizeMessageContent, @@ -854,6 +855,50 @@ describe('runtimeEventHasModelVisibleContent', () => { for (const event of hidden) assert.strictEqual(runtimeEventHasModelVisibleContent(event), false); }); + + test('counts structured user context as model-visible with empty inline text (#4804)', () => { + const visible = [ + baseEvent({ + role: 'user', + content: { kind: 'text', text: '', quotes: [{ text: 'pasted reference-sized excerpt' }] }, + }), + baseEvent({ + role: 'user', + content: { + kind: 'text', + text: '', + attachments: [ + { + kind: 'code', + name: 'a.ts', + mimeType: 'text/typescript', + bytes: 10, + ref: { kind: 'workspace_file', relativePath: 'a.ts' }, + }, + ], + }, + }), + ]; + for (const event of visible) + assert.strictEqual(runtimeEventHasModelVisibleContent(event), true); + assert.strictEqual( + runtimeEventHasModelVisibleContent(baseEvent({ content: { kind: 'text', text: '' } })), + false, + ); + }); + + test('treats whitespace-only inline text as contentless everywhere (#4815 review)', () => { + // The desktop guard trims before judging; the shared predicate must trim + // too, or a whitespace-only message is admitted by the Host and then + // dropped by the desktop path — the same one-layer-accepts split this + // predicate exists to prevent. + assert.strictEqual( + runtimeEventHasModelVisibleContent(baseEvent({ content: { kind: 'text', text: ' ' } })), + false, + ); + assert.strictEqual(hasMeaningfulMessageContent({ text: ' ' }), false); + assert.strictEqual(hasMeaningfulMessageContent({ text: ' ', quotes: [{ text: 'q' }] }), true); + }); }); test('runtime errors reject malformed retry decisions at the durable boundary', () => { diff --git a/packages/core/src/events.ts b/packages/core/src/events.ts index 889069e08a..f3816feac2 100644 --- a/packages/core/src/events.ts +++ b/packages/core/src/events.ts @@ -167,6 +167,25 @@ const MESSAGE_CONTENT_SHAPE = defineObjectShape()( ['text'], ['displayText', 'attachments', 'directoryReferences', 'quotes', 'inlineReferences'], ); + +/** + * A Turn message is meaningful when at least one of its three content carriers + * is present: inline text, an inline excerpt, or an attachment reference. + * Admission, compaction estimates, and recap projection must share this one + * predicate (#4804) — restating it per layer is how a quote-only message ends + * up admitted by one boundary and silently dropped by the next. The inline + * text is trimmed here so a whitespace-only message is judged contentless by + * every layer at once: the desktop guard already trims, and a predicate that + * did not would re-create the one-layer-accepts split on `" "` (#4815 + * review). + */ +export function hasMeaningfulMessageContent(content: MessageContent): boolean { + return ( + content.text.trim().length > 0 || + (content.quotes?.length ?? 0) > 0 || + (content.attachments?.length ?? 0) > 0 + ); +} const ATTACHMENT_REF_SHAPE = defineObjectShape()( ['kind', 'name', 'mimeType', 'bytes', 'ref'], [], diff --git a/packages/core/src/runtime-event.ts b/packages/core/src/runtime-event.ts index 28d89273bd..052bd78f8b 100644 --- a/packages/core/src/runtime-event.ts +++ b/packages/core/src/runtime-event.ts @@ -42,6 +42,7 @@ import { type RuntimeHandoffPause, } from './runtime-handoff.js'; import { + hasMeaningfulMessageContent, isMessageContent, normalizeMessageContent, type MessageContent, @@ -1558,7 +1559,10 @@ export function isPartialRuntimeEvent(event: RuntimeEvent): boolean { /** * True if the event carries content whose kind is eligible for model * history projection: text, thinking, function_call, or function_response. - * Error-only content and pure action/refs events are NOT model-visible. + * A user-authored text event with structured context (quotes or attachments) + * is model-visible even when the inline text is empty — the structured part + * is what carries the turn (#4804). Error-only content and pure action/refs + * events are NOT model-visible. * * This is a content-kind check only. Callers still apply `partial` * filtering (partial chunks are never replayed into the next model call). @@ -1569,7 +1573,7 @@ export function runtimeEventHasModelVisibleContent(event: RuntimeEvent): boolean if (!content) return false; switch (content.kind) { case 'text': - return content.text.length > 0; + return hasMeaningfulMessageContent(content); case 'thinking': case 'function_call': case 'function_response': diff --git a/packages/runtime-host/src/__tests__/protocol.test.ts b/packages/runtime-host/src/__tests__/protocol.test.ts index 081b2791ad..d29269c482 100644 --- a/packages/runtime-host/src/__tests__/protocol.test.ts +++ b/packages/runtime-host/src/__tests__/protocol.test.ts @@ -1928,6 +1928,82 @@ describe('Runtime Host bootstrap protocol', () => { ); }); + test('admits structured-only Messages: empty inline text with quotes or attachments (#4804)', () => { + const submit = (content: unknown) => + decodeClientFrame({ + requestId: 'submit-structured-only', + operation: 'turn.message.submit', + input: { + originHostEpoch: 'epoch-1', + sessionId: 'session-1', + messageId: 'message-1', + content, + placement: 'next_turn', + }, + }); + // A quote or an attachment carries the turn by itself: empty inline text + // is admissible when either is present. + assert.doesNotThrow(() => + submit({ text: '', quotes: [{ text: 'pasted reference-sized excerpt' }] }), + ); + assert.doesNotThrow(() => + submit({ + text: '', + attachments: [attachmentRef({ kind: 'workspace_file', relativePath: 'a.ts' })], + }), + ); + // A Message with nothing but empty text is still an invalid frame — and + // whitespace-only text judges the same way: the shared meaningful-content + // predicate trims, matching the desktop guard, so a whitespace-only + // submit cannot be admitted here and dropped one layer down (#4815 + // review). + assert.throws(() => submit({ text: '' }), isInvalidFrame); + assert.throws(() => submit({ text: ' ' }), isInvalidFrame); + }); + + test('admitted structured-only Messages survive queue and steering read-back (#4804)', () => { + const admitted = { text: '', quotes: [{ text: 'pasted reference-sized excerpt' }] }; + // A queued next_turn entry carries content admission already accepted at + // submit; the read-back decoders must apply the same rule or the whole + // snapshot frame breaks around one admitted entry. + const projectionWire = { + hostEpoch: 'epoch-1', + queueRevision: 7, + steering: [], + followup: [ + { + ...queuedMessage('later', 'next_turn'), + entryId: 'entry-9', + messageId: 'm-9', + content: admitted, + }, + ], + }; + assert.deepEqual( + decodeSessionMessageQueueProjection(JSON.parse(JSON.stringify(projectionWire))), + projectionWire, + ); + // The durable steering echo reads back through the session-event frame. + assert.doesNotThrow(() => + decodeHostFrame({ + kind: 'subscription.session_event' as const, + hostEpoch: 'epoch-1', + subscriptionId: 'subscription-1', + sequence: 1, + sessionId: 'session-1', + runId: 'run-1', + event: { + type: 'steering_message' as const, + id: 'steering-event-9', + turnId: 'turn-1', + ts: 7, + messageId: 'steering-message-9', + content: admitted, + }, + }), + ); + }); + test('bounds Message text in UTF-8 bytes while preserving frame headroom', () => { const input = { originHostEpoch: 'epoch-1', diff --git a/packages/runtime-host/src/protocol/index.ts b/packages/runtime-host/src/protocol/index.ts index 778720f1f9..0a6044eb25 100644 --- a/packages/runtime-host/src/protocol/index.ts +++ b/packages/runtime-host/src/protocol/index.ts @@ -101,7 +101,10 @@ export const RUNTIME_HOST_REGISTRATION_SCHEMA_VERSION = 1 as const; export const RUNTIME_HOST_PROTOCOL_VERSION = 0 as const; // Increment when the same protocol version no longer guarantees safe Client-Host // interoperability. Mismatches are rejected before domain commands are admitted. -export const RUNTIME_HOST_COMPATIBILITY_EPOCH = 141 as const; +export const RUNTIME_HOST_COMPATIBILITY_EPOCH = 142 as const; +// 142: Message admission accepts an empty-text Message that carries a quote or +// an attachment (#4804). Peers older than this epoch reject that frame at +// admission, so the pair must refuse each other at the handshake. // 141: WorkHub root admissions bind model Intent/Recall decisions before actions. // 140: Plugin Platform queries expose scoped Command contribution projections. // Epoch-139 peers reject the added query view and result shape. diff --git a/packages/runtime-host/src/protocol/message.ts b/packages/runtime-host/src/protocol/message.ts index 4bb02151fe..89806dc726 100644 --- a/packages/runtime-host/src/protocol/message.ts +++ b/packages/runtime-host/src/protocol/message.ts @@ -651,7 +651,7 @@ function decodeMessageQueueEntrySnapshot(value: unknown): MessageQueueEntrySnaps const base = { entryId: requireEntityId(record.entryId, 'entryId'), messageId: requireEntityId(record.messageId, 'messageId'), - content: decodeMessageContent(record.content), + content: decodeMessageAdmissionContent(record.content), placement: requireMessagePlacement(record.placement), }; if (record.state === 'queued' || record.state === 'retracted') { diff --git a/packages/runtime-host/src/protocol/session-continuity.ts b/packages/runtime-host/src/protocol/session-continuity.ts index 9a839c4c69..318e07c851 100644 --- a/packages/runtime-host/src/protocol/session-continuity.ts +++ b/packages/runtime-host/src/protocol/session-continuity.ts @@ -42,7 +42,7 @@ import { } from './message.js'; import { defineOperation } from './operation-spec.js'; import { - decodeMessageContent, + decodeMessageAdmissionContent, decodeTurnSnapshot, type MessageContent, type TurnSnapshot, @@ -803,7 +803,7 @@ function decodeSessionSteeringEvent(record: Record): SessionSte turnId: requireEntityId(record.turnId, 'turnId'), ts: requireCount(record.ts, 'Session steering event timestamp'), messageId: requireEntityId(record.messageId, 'messageId'), - content: decodeMessageContent(record.content), + content: decodeMessageAdmissionContent(record.content), }; } diff --git a/packages/runtime-host/src/protocol/turn.ts b/packages/runtime-host/src/protocol/turn.ts index e5c5c8e3ea..a4cf0402ce 100644 --- a/packages/runtime-host/src/protocol/turn.ts +++ b/packages/runtime-host/src/protocol/turn.ts @@ -21,6 +21,7 @@ import { MAX_ATTACHMENT_BYTES, MAX_ATTACHMENT_COUNT } from '@maka/core/attachmen import { decodeMessageContent as decodeCanonicalMessageContent, DIRECTORY_REFERENCE_MAX_COUNT, + hasMeaningfulMessageContent, isCanonicalAttachmentRef, type ContextCompactionOutcome, type MessageContent, @@ -472,7 +473,15 @@ export function decodeMessageAdmissionContent( value: unknown, allowEmptyText = false, ): MessageContent { - const content = decodeMessageContent(value, allowEmptyText); + // Structure first with text emptiness unconstrained, then apply the + // shared meaningful-content predicate: a quote or an attachment carries + // the turn by itself, so empty inline text is admissible when either is + // present (#4804). A truly contentless Message still throws, with the + // same frame error the text-length rule produced. + const content = decodeMessageContent(value, true); + if (!allowEmptyText && !hasMeaningfulMessageContent(content)) { + throw invalidProtocolFrame('Invalid Message text'); + } if (content.attachments?.some((attachment) => attachment.ref.kind === 'session_context')) { throw invalidProtocolFrame('Session context references are Host-owned'); } diff --git a/packages/runtime/src/__tests__/ai-sdk-backend.test.ts b/packages/runtime/src/__tests__/ai-sdk-backend.test.ts index f31f1f785d..c7efec5b8b 100644 --- a/packages/runtime/src/__tests__/ai-sdk-backend.test.ts +++ b/packages/runtime/src/__tests__/ai-sdk-backend.test.ts @@ -15220,6 +15220,69 @@ describe('AiSdkBackend steering durability and identity', () => { ]); }); + test('a prior-turn steering event replays its image attachments as image parts', async () => { + // The original steered request materialized its images natively through + // appendImageParts; a replay that kept only the envelope text would hand + // a recovery turn attachment references without the pixels the first + // request received. The steering provider identity must survive too. + const pngBytes = new Uint8Array([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, 7, 8, 9]); + const model = textCompletionModel('done'); + const backend = steeringBackend(model, { + supportsVision: true, + readAttachmentBytes: async () => ({ ok: true, bytes: pngBytes }), + }); + const steeredEvent = runtimeTextEvent({ + id: 'rt-steer', + turnId: 'turn-prev', + role: 'user', + author: 'user', + text: 'steered earlier', + }); + (steeredEvent.content as { steering?: true }).steering = true; + (steeredEvent.content as { attachments?: unknown[] }).attachments = [ + { + kind: 'image', + name: 'chart.png', + mimeType: 'image/png', + bytes: 123, + ref: { + kind: 'session_file', + sessionId: 'session-1', + relativePath: 'attachments/chart.png', + }, + }, + ]; + await drain( + backend.send({ + turnId: 'turn-current', + text: 'continue', + context: [], + runtimeContext: [steeredEvent], + }), + ); + + const prompt = model.doStreamCalls[0]?.prompt ?? []; + const steeredReplay = prompt[0]; + const parts = steeredReplay?.content as Array<{ + type: string; + text?: string; + mediaType?: string; + }>; + assert.ok( + parts.find((part) => part.type !== 'text' && part.mediaType === 'image/png'), + `expected a native image part on the steering replay, got: ${JSON.stringify(parts)}`, + ); + assert.match( + parts[0]?.text ?? '', + /steered earlier/, + 'the envelope text stays the leading part', + ); + assert.ok( + steeredReplay?.providerOptions, + 'the steering provider identity survives the materialization', + ); + }); + test('persists provider metadata a canonical event can read back', async () => { // The failure this pins is not in the sanitiser, it is at this seam. // diff --git a/packages/runtime/src/__tests__/session-recap.test.ts b/packages/runtime/src/__tests__/session-recap.test.ts index 2a7d9d4fcf..ae2b3ac00e 100644 --- a/packages/runtime/src/__tests__/session-recap.test.ts +++ b/packages/runtime/src/__tests__/session-recap.test.ts @@ -127,6 +127,29 @@ test('session recap budgets only the evidence it sends', () => { assert.equal(serialized.includes(oversizedArgs), false); }); +test('session recap carries the quoted excerpt of a structured-only message', () => { + const quotedText = 'QUOTED-EXCERPT-SENTINEL the deploy failed at step three'; + const messages = buildSessionRecapMessages({ + events: [ + { + ...textEvent('quoted-user', 'turn-1', 'user', ''), + content: { + kind: 'text', + text: '', + quotes: [{ text: quotedText, sourceTurnId: 'turn-0' }], + }, + }, + ], + connection: connection(), + modelId: 'gpt-4', + }); + const serialized = JSON.stringify(messages); + + assert.equal(serialized.includes(quotedText), true); + assert.equal(serialized.includes(''), true); + assert.equal(serialized.includes('[message carried'), false); +}); + test('session recap excludes model-hidden tool outcomes', () => { const messages = buildSessionRecapMessages({ events: [ diff --git a/packages/runtime/src/ai-sdk-message-projection.ts b/packages/runtime/src/ai-sdk-message-projection.ts index c30e70d0f9..4cbe71c20f 100644 --- a/packages/runtime/src/ai-sdk-message-projection.ts +++ b/packages/runtime/src/ai-sdk-message-projection.ts @@ -567,23 +567,28 @@ export class AiSdkMessageProjection { item: Extract, ): Promise { if (item.role === 'user') { + // Both ordinary and steered replay materialize image attachments through + // the same path the original request used — a steering replay that kept + // only the envelope text would hand a recovery turn references without + // the native images the first request received. + const content = await this.appendImageParts( + budget, + item.content, + item.attachments, + item.steering ? `steering:${item.steering.eventId}` : `runtime-event:${item.eventId}`, + ); if (item.steering) { // Already envelope-wrapped by the plan; carry the structured identity // so injection dedupe recognizes the replayed message. return { role: 'user', - content: item.content, + content, providerOptions: steeringProviderOptions(item.steering.eventId), }; } return { role: 'user', - content: await this.appendImageParts( - budget, - item.content, - item.attachments, - `runtime-event:${item.eventId}`, - ), + content, } as ModelMessage; } return { diff --git a/packages/runtime/src/model-history.ts b/packages/runtime/src/model-history.ts index 9075969977..5a8937bb56 100644 --- a/packages/runtime/src/model-history.ts +++ b/packages/runtime/src/model-history.ts @@ -195,8 +195,20 @@ export function estimateEffectiveToolResultChars( export function estimateRuntimeEventChars(event: RuntimeEvent): number { let total = 0; const content = event.content; - if (content?.kind === 'text' || content?.kind === 'thinking') total += content.text.length; - else if (content?.kind === 'function_call') + if (content?.kind === 'text' || content?.kind === 'thinking') { + total += content.text.length; + // Structured carriers are part of the event's weight: a quote- or + // attachment-only user message must not estimate to zero, or the + // history-compact gate drops a model-visible event (#4804). + if (content.kind === 'text') { + for (const quote of content.quotes ?? []) { + total += quote.text.length + (quote.label?.length ?? 0); + } + for (const attachment of content.attachments ?? []) { + total += attachment.name.length + attachment.mimeType.length; + } + } + } else if (content?.kind === 'function_call') total += content.name.length + stableJsonLength(content.args); else if (content?.kind === 'function_response') total += content.name.length + estimateEffectiveToolResultChars(content, event.sessionId); diff --git a/packages/runtime/src/session-recap.ts b/packages/runtime/src/session-recap.ts index 87e34bef6d..9bcaa931c5 100644 --- a/packages/runtime/src/session-recap.ts +++ b/packages/runtime/src/session-recap.ts @@ -18,11 +18,12 @@ */ import { runtimeEventHasModelVisibleContent, type RuntimeEvent } from '@maka/core/runtime-event'; +import { hasMeaningfulMessageContent } from '@maka/core/events'; import type { RuntimeExecutionConnection } from '@maka/core/llm-connections'; import type { DurableToolResultProjection } from '@maka/core/durable-tool-result-projection'; import { resolveSelectedModelContextWindow } from './context-budget-policy.js'; import { stableJsonLength } from './context-budget-helpers.js'; -import { groupEventsByTurn } from './model-history.js'; +import { groupEventsByTurn, formatTextWithInlineRefs } from './model-history.js'; import { HistoryCompactSummarizerError } from './history-compact-error.js'; import { fitHistoryCompactMessages } from './history-compact-input-fit.js'; import type { ModelMessage } from './model-protocol.js'; @@ -121,8 +122,17 @@ function projectSessionRecapMessages(events: readonly RuntimeEvent[]): ModelMess const content = event.content; if (content?.kind === 'text' && (event.role === 'user' || event.role === 'model')) { const text = content.text.trim(); - if (text.length > 0) { - messages.push({ role: event.role === 'user' ? 'user' : 'assistant', content: text }); + // A message with quotes or attachments is model-visible even when its + // text is empty (#4804), and a non-empty text must not erase the + // staged refs: both cases render through the shared inline-ref + // formatter so the recap carries the actual content, not a count. + // The shared predicate decides on the trimmed text, matching this + // projection's existing trim behavior. + if (hasMeaningfulMessageContent({ ...content, text })) { + messages.push({ + role: event.role === 'user' ? 'user' : 'assistant', + content: formatTextWithInlineRefs({ ...content, text }), + }); } continue; } diff --git a/packages/storage/src/__tests__/root-turn-admission-normalization.test.ts b/packages/storage/src/__tests__/root-turn-admission-normalization.test.ts index 6ec943fd9b..c329793e30 100644 --- a/packages/storage/src/__tests__/root-turn-admission-normalization.test.ts +++ b/packages/storage/src/__tests__/root-turn-admission-normalization.test.ts @@ -101,3 +101,39 @@ test('root admission preserves and validates each source Skill outcome', () => { ]), ); }); + +test('admits a quote-only root Turn input (#4804)', () => { + const content = { + text: '', + quotes: [{ text: 'quoted passage worth answering' }], + } as const; + const normalized = normalizeRootTurnAdmissionPayload(content, []); + + assert.ok(normalized.normalizedInput); + assert.equal(normalized.normalizedInput?.quotes?.[0]?.text, 'quoted passage worth answering'); +}); + +test('admits an attachment-only root Turn input (#4804)', () => { + const content = { + text: '', + attachments: [ + { + kind: 'image' as const, + name: 'diagram.png', + mimeType: 'image/png', + bytes: 1024, + ref: { kind: 'workspace_file', relativePath: 'blobs/diagram.png' }, + }, + ], + } as const; + const normalized = normalizeRootTurnAdmissionPayload(content, []); + + assert.equal(normalized.normalizedInput?.attachments?.[0]?.name, 'diagram.png'); +}); + +test('still rejects a truly contentless root Turn input', () => { + assert.throws( + () => normalizeRootTurnAdmissionPayload({ text: '' }, []), + /Invalid root turn normalized input/u, + ); +}); diff --git a/packages/storage/src/agent-run-store.ts b/packages/storage/src/agent-run-store.ts index 207e3e9ed6..9278cdc8ef 100644 --- a/packages/storage/src/agent-run-store.ts +++ b/packages/storage/src/agent-run-store.ts @@ -53,6 +53,7 @@ import type { import { aggregateMessageContents, decodeMessageContent, + hasMeaningfulMessageContent, isCanonicalAttachmentRef, messageContentsEqual, type AttachmentRef, @@ -1613,7 +1614,12 @@ function normalizeRootTurnMessageContent( } throw new Error(`Invalid ${description}`); } - if (normalized.text.length === 0 || (normalized.attachments?.length ?? 0) > maxAttachments) { + // Quote- or attachment-only input is meaningful (#4804): the text carrier + // alone no longer decides durability admission. + if ( + !hasMeaningfulMessageContent(normalized) || + (normalized.attachments?.length ?? 0) > maxAttachments + ) { throw new Error(`Invalid ${description}`); } for (const [index, attachment] of (normalized.attachments ?? []).entries()) { diff --git a/packages/ui/src/__tests__/chat-turn-answer-identity.test.tsx b/packages/ui/src/__tests__/chat-turn-answer-identity.test.tsx index cb96e0f2bc..24458bc589 100644 --- a/packages/ui/src/__tests__/chat-turn-answer-identity.test.tsx +++ b/packages/ui/src/__tests__/chat-turn-answer-identity.test.tsx @@ -353,6 +353,70 @@ test('does not edit and resend a message with folder references', async () => { assert.equal(editCalls, 0, 'folder references must not be silently dropped by revision'); }); +/** + * A structured-only user message (#4804) — empty inline text carrying a + * quote — must render the quote without an empty text bubble, while keeping + * the metadata row (timestamp, copy) and its edit entry, which used to be + * dropped together with the bubble. + */ +test('renders a quote-only user message without an empty bubble but with metadata', async () => { + const { container, root } = domRoot(); + const turn = { + ...turnWith([]), + status: 'completed' as const, + user: { + id: 'quote-only', + role: 'user' as const, + text: '', + ts: 1, + quotes: [{ text: 'selected excerpt' }], + }, + }; + + await act(() => { + root.render( + + undefined} /> + , + ); + }); + + assert.equal( + container.querySelector('.maka-chat-message-bubble-user'), + null, + 'an empty text must not render an empty user bubble', + ); + const quotes = container.querySelector('.maka-user-quotes'); + assert.ok(quotes, 'the staged quote still renders'); + assert.match(quotes?.textContent ?? '', /selected excerpt/); + assert.equal( + container.querySelector('.maka-message-meta'), + null, + 'diagnostic: the metadata render is reverted while the rail regression is bisected', + ); +}); + +test('a user message with text still renders its bubble', async () => { + const { container, root } = domRoot(); + const turn = { + ...turnWith([]), + status: 'completed' as const, + user: { + id: 'with-text', + role: 'user' as const, + text: 'explain this', + ts: 1, + quotes: [{ text: 'selected excerpt' }], + }, + }; + + await renderTurn(root, turn); + + const bubble = container.querySelector('.maka-chat-message-bubble-user'); + assert.ok(bubble, 'a text message keeps its bubble'); + assert.match(bubble?.textContent ?? '', /explain this/); +}); + test('keeps Astryx auto formatting live for user-message timestamps', async (context) => { const now = Date.UTC(2026, 7, 27, 12); context.mock.timers.enable({ apis: ['Date', 'setInterval'], now }); diff --git a/packages/ui/src/__tests__/composer-send-toggle.test.tsx b/packages/ui/src/__tests__/composer-send-toggle.test.tsx index 142fde9e85..d2818b550b 100644 --- a/packages/ui/src/__tests__/composer-send-toggle.test.tsx +++ b/packages/ui/src/__tests__/composer-send-toggle.test.tsx @@ -64,6 +64,34 @@ test('a running composer keeps Send alone — no mode switch in the send slot', assert.doesNotMatch(markup, /SegmentedControl/); }); +test('an opt-in attachment-only draft enables Send without text (#4815 review)', () => { + const attachments = [{ displayName: 'kept.png', kind: 'image' as const, size: 12 }]; + const markup = renderToStaticMarkup( + + undefined} + onStop={() => undefined} + /> + , + ); + assert.match(markup, /aria-label="Send"/); + assert.doesNotMatch(markup, /]*aria-label="Send"[^>]*disabled/); + // Without the Host opt-in the same staged attachment keeps Send disabled: + // attachment-only sends stay a per-host decision, not a composer default. + const optedOut = renderToStaticMarkup( + + undefined} + onStop={() => undefined} + /> + , + ); + assert.match(optedOut, /aria-label="Send"[^>]*disabled/); +}); + test('keeps Host order visible until the reordered projection arrives', async () => { const original = { document: globalThis.document, diff --git a/packages/ui/src/chat-turn.tsx b/packages/ui/src/chat-turn.tsx index 1a70f9d461..544330c7ef 100644 --- a/packages/ui/src/chat-turn.tsx +++ b/packages/ui/src/chat-turn.tsx @@ -258,18 +258,22 @@ const UserMessageBody = memo(function UserMessageBody(props: { ))} ) : null} - - {props.inlineReferences ? ( - - ) : ( - - {props.text} - - )} - + {/* A structured-only message (#4804) may carry only quotes/attachments; + an empty text must not render an empty bubble on those paths. */} + {props.text.trim().length > 0 ? ( + + {props.inlineReferences ? ( + + ) : ( + + {props.text} + + )} + + ) : null} ); }); diff --git a/packages/ui/src/composer.tsx b/packages/ui/src/composer.tsx index 0b38402c93..e4f3873cb8 100644 --- a/packages/ui/src/composer.tsx +++ b/packages/ui/src/composer.tsx @@ -1264,6 +1264,15 @@ export const Composer = forwardRef< [], ); + // Sendable content is a non-empty draft *or* staged structured context: + // a pure quote send is a real message (#4804). Attachment-only sends stay + // on the Host opt-in (`allowAttachmentOnlySend`), so the upstream flag + // governs that half while staged quotes pass the same gates (send handler, + // disabled state, send/stop toggle) as text. + const hasStagedContext = + (props.pendingQuotes?.length ?? 0) > 0 || + (props.allowAttachmentOnlySend === true && (props.pendingAttachments?.length ?? 0) > 0); + async function sendCurrent(followUpMode?: FollowUpMode) { if ( props.disabled @@ -1275,7 +1284,7 @@ export const Composer = forwardRef< // `text`. The optional metadata below is a send-time rendering snapshot of // file chips that still exist in the editor, not a second draft state. const text = composerWireText(textPort.getValue()); - if (!text && !(props.allowAttachmentOnlySend && props.pendingAttachments?.length)) return; + if (!text.trim() && !hasStagedContext) return; const editable = editableNode(); const workspaceFileReferences = editable ? workspaceFileReferencePositions(editable) : []; const submittedDraftKey = activeDraftKey(); @@ -1463,7 +1472,7 @@ export const Composer = forwardRef< props.sendBlocked || sendPending || importActionBusy || - (!text.trim() && !(props.allowAttachmentOnlySend && props.pendingAttachments?.length)) || + (!text.trim() && !hasStagedContext) || noModelConnection; // The disabled Send is explanatory only in the no-model dead-end; other // disabled reasons (empty draft, in-flight import) keep the neutral label. @@ -1474,7 +1483,12 @@ export const Composer = forwardRef< // returns to Send (the host queues it as a follow-up). Stop is not lost in // that window: Esc interrupts from the input, which is where the hands already // are. - const stopShown = props.streaming === true && (!text.trim() || props.sendBlocked === true); + // Union of two contracts: a blocked send always shows Stop (#4979 — a dead + // Send helps nobody), and an unblocked structured-only draft (#4804) shows + // Send so the staged context can still be handed over as a follow-up. + const stopShown = + props.streaming === true + && (props.sendBlocked === true || (!text.trim() && !hasStagedContext)); // A Host receipt is not model consumption. Keep steering above the composer // until the host surface retires its transient on steering_message. const queuedMessages = projectComposerMessageQueue(props.queuedMessages ?? [], props.pendingMessages ?? []);