diff --git a/apps/desktop/renderer-architecture.json b/apps/desktop/renderer-architecture.json index 49bcb1032c..ddca25f7df 100644 --- a/apps/desktop/renderer-architecture.json +++ b/apps/desktop/renderer-architecture.json @@ -856,7 +856,7 @@ "react": 1 }, "importSpecifiers": 104, - "nonTriviaTokens": 13402 + "nonTriviaTokens": 13394 }, "src/renderer/use-app-shell-composer-quotes.ts": { "importDeclarations": 2, 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..d7dcff031c 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 @@ -246,6 +246,15 @@ describe('permission response IPC boundary', () => { { type: 'send', text: '' }, { type: 'send', text: 'x'.repeat(128_001) }, { type: 'send', text: 'ok', retainedAttachments: [{ name: 'broken' }] }, + // Junk attachment items must not satisfy the empty-body check + // (#4815 review reachability ③). + { type: 'send', text: '', attachmentItems: [null] }, + { type: 'send', text: '', attachmentItems: [{}] }, + { type: 'send', text: '', attachmentItems: [{ approvalId: 7 }] }, + { type: 'send', text: 'hello', attachmentItems: 'notes.txt' }, + // A raw File carrier never crosses the preload: it is encoded to inline + // base64 bytes before IPC, and main resolves only the encoded shapes. + { type: 'send', text: 'hello', attachmentItems: [{ file: {} }] }, { type: 'send', text: 'hello', turnId: 1 }, { type: 'send', text: 'hello', skillIds: ['/bad'] }, { type: 'send', text: 'hello', turnOrchestration: { mode: 'swarm', source: 'prompt' } }, @@ -286,6 +295,46 @@ 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 an inline base64 attachment as the only content', () => { + // Dragged/pasted blobs cross IPC as inline base64 bytes (the preload + // encodes the File before invoke), so an attachment-only send with no text + // is the #4804 shape at this boundary and must reach ingestion, which owns + // the byte-size and MIME checks. + const command = normalizeSessionSendCommand({ + type: 'send', + text: '', + attachmentItems: [{ name: 'pasted.png', mimeType: 'image/png', base64: 'aGVsbG8=' }], + }); + assert.deepEqual(command?.attachmentItems, [ + { name: 'pasted.png', mimeType: 'image/png', base64: 'aGVsbG8=' }, + ]); + }); + 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 d81c7faa4b..8bf452bdb4 100644 --- a/apps/desktop/src/main/__tests__/quote-companion-retry.test.ts +++ b/apps/desktop/src/main/__tests__/quote-companion-retry.test.ts @@ -43,6 +43,7 @@ import { WorkbarServicesProvider, type CompanionQuoteSnapshot, type StagedCompanionQuote, + type WorkbarIngestInput, type WorkbarServices, } from '../../renderer/features/workbar/testing.js'; @@ -60,6 +61,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]; @@ -127,7 +133,7 @@ async function renderProbe( onSend?: (send: (text: string) => Promise) => void; onProjection?: (companion: ReturnType) => void; onQueue?: (queue: (text: string) => Promise) => void; - onSteer?: (steer: (text: string) => Promise) => void; + onSteer?: (steer: SteerFn) => void; onStop?: (stop: () => Promise) => void; onDeleteQueuedEntry?: (deleteEntry: (entryId: string) => Promise) => void; onSetPermissionMode?: (set: (mode: PermissionMode) => Promise) => void; @@ -198,7 +204,7 @@ async function renderOwnershipProbe( let send!: (text: string) => Promise; let projection!: ReturnType; let queue!: (text: string) => Promise; - let steer!: (text: string) => Promise; + let steer!: SteerFn; let stop!: () => Promise; let deleteQueuedEntry!: (entryId: string) => Promise; let setPermissionMode!: (mode: PermissionMode) => Promise; @@ -238,7 +244,8 @@ async function renderOwnershipProbe( ...rendered, send: (text: string) => send(text), queue: (text: string) => queue(text), - steer: (text: string) => steer(text), + steer: (text: string, attachmentItems?: WorkbarIngestInput[], onAdmitted?: () => void) => + steer(text, attachmentItems, onAdmitted), stop: () => stop(), deleteQueuedEntry: (entryId: string) => deleteQueuedEntry(entryId), setPermissionMode: (mode: PermissionMode) => setPermissionMode(mode), @@ -1988,6 +1995,66 @@ test('recovers the Host-edited Side Conversation steer from the queue projection assert.equal(container.firstElementChild?.getAttribute('data-queue-texts'), ''); }); +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, hostTurn } = await renderOwnershipProbe({ + send: async () => ({ ok: true as const, turnId: 'old-turn' }), + submitFollowUp: async (_sessionId, placement, _text, requestedAdmissionId, payload) => { + assert.equal(placement, 'current_turn'); + admissionId = requestedAdmissionId; + steerPayload = payload; + return pendingSteer.promise; + }, + }); + + await act(async () => { + assert.equal(await send('initial prompt'), true); + hostTurn('old-turn'); + 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('retracts a queued Side Conversation message without stopping the active turn', async () => { let messageId: string | undefined; const retracted: string[] = []; @@ -3252,7 +3319,7 @@ function QuoteCompanionOwnershipProbe(props: { onSend: (send: (text: string) => Promise) => void; onProjection?: (companion: ReturnType) => void; onQueue?: (queue: (text: string) => Promise) => void; - onSteer?: (steer: (text: string) => Promise) => void; + onSteer?: (steer: SteerFn) => void; onStop?: (stop: () => Promise) => void; onDeleteQueuedEntry?: (deleteEntry: (entryId: string) => Promise) => void; onSetPermissionMode?: (set: (mode: PermissionMode) => Promise) => void; @@ -3370,3 +3437,173 @@ 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 followUpContents: Array[4]> = []; + const rendered = await renderOwnershipProbe( + { + send: async () => ({ ok: true as const, turnId: 'old-turn' }), + submitFollowUp: async (_sessionId, placement, _text, _admissionId, content) => { + assert.equal(placement, 'current_turn'); + followUpContents.push(content); + return { kind: 'queued' as const }; + }, + }, + { + pendingQuotes: [{ id: 'quote-1', value: { text: 'streaming excerpt' } }], + }, + ); + + await act(async () => { + assert.equal(await rendered.send('initial prompt'), true); + rendered.hostTurn('old-turn'); + 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(followUpContents.length, 1); + assert.deepEqual( + followUpContents[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' }), + submitFollowUp: async (_sessionId, placement, _text, admissionId) => { + assert.equal(placement, 'current_turn'); + const id = admissionId ?? ''; + admissionIds.push(id); + // The reconnect/failure path answers without an admission receipt. + return { kind: 'outcome_unknown' as const }; + }, + }, + { pendingQuotes: [] }, + ); + + await act(async () => { + assert.equal(await rendered.send('initial prompt'), true); + rendered.hostTurn('old-turn'); + 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' }), + submitFollowUp: async (_sessionId, placement, _text, admissionId) => { + assert.equal(placement, 'current_turn'); + const id = admissionId ?? ''; + admissionIds.push(id); + return { kind: 'outcome_unknown' as const }; + }, + }, + { pendingQuotes: [] }, + ); + + await act(async () => { + assert.equal(await rendered.send('initial prompt'), true); + rendered.hostTurn('old-turn'); + 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..b2390fd984 100644 --- a/apps/desktop/src/main/permission-response-guard.ts +++ b/apps/desktop/src/main/permission-response-guard.ts @@ -209,7 +209,29 @@ 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); + // attachmentItems get the same per-item normalization as the other + // structured carriers: a junk entry (`[null]`, `[{}]`) used to satisfy the + // empty-body check while nothing ingestible would arrive downstream + // (#4815 review, reachability ③). + const attachmentItems = normalizeOptionalAttachmentItems(value.attachmentItems); + const hasAttachmentItems = (attachmentItems.attachmentItems?.length ?? 0) > 0; + if ( + !text.trim() && + skillIds.length === 0 && + (quotes?.length ?? 0) === 0 && + !hasAttachmentItems && + (retainedAttachments.retainedAttachments?.length ?? 0) === 0 + ) { throw new Error('Invalid send text'); } return { @@ -219,13 +241,13 @@ export function normalizeSessionSendCommand(input: unknown): NormalizedSendSessi text, ...(displayText !== undefined ? { displayText } : {}), ...(skillIds.length > 0 ? { skillIds } : {}), - ...(value.attachmentItems !== undefined ? { attachmentItems: value.attachmentItems } : {}), - ...normalizeOptionalRetainedAttachments(value.retainedAttachments), + ...attachmentItems, + ...retainedAttachments, ...(value.turnOrchestration !== undefined ? { turnOrchestration: normalizeTurnOrchestration(value.turnOrchestration) } : {}), ...normalizeOptionalDirectoryReferences(value.directoryReferences), - ...normalizeOptionalQuotes(value.quotes), + ...(quotes !== undefined ? { quotes } : {}), ...normalizeOptionalWorkspaceFileReferences( value.workspaceFileReferences, displayText ?? text, @@ -256,6 +278,32 @@ function normalizeOptionalRetainedAttachments( : {}; } +// The wire shape is the preload's IngestPayload: an approval-backed descriptor +// (`approvalId` + `name`, optional `mimeType`) or inline `base64` bytes for a +// dragged/pasted blob — the same shapes prepareIngestItems resolves. A bare +// `{}` or `null` entry used to satisfy the empty-body check while carrying +// nothing ingestible (#4815 review). +function isComposerIngestItem(item: unknown): boolean { + if (typeof item !== 'object' || item === null) return false; + const candidate = item as Record; + if (typeof candidate.approvalId === 'string') { + return typeof candidate.name === 'string'; + } + return typeof candidate.name === 'string' && typeof candidate.base64 === 'string'; +} + +function normalizeOptionalAttachmentItems(input: unknown): { attachmentItems?: unknown[] } { + if (input === undefined) return {}; + if ( + !Array.isArray(input) || + input.length > MAX_ATTACHMENT_COUNT || + !input.every(isComposerIngestItem) + ) { + throw new Error('Invalid attachment items'); + } + return input.length > 0 ? { attachmentItems: input } : {}; +} + function normalizeOptionalWorkspaceFileReferences( input: unknown, displayText: string, diff --git a/apps/desktop/src/renderer/app-shell.tsx b/apps/desktop/src/renderer/app-shell.tsx index c6650c588f..b9f6dd34ac 100644 --- a/apps/desktop/src/renderer/app-shell.tsx +++ b/apps/desktop/src/renderer/app-shell.tsx @@ -2152,6 +2152,11 @@ function AppShellContent({ const canStageComposerContext = activeId !== undefined || taskEntry.selectors.target !== undefined; + // #4804: attachment-only sends are opt-in per host surface, and the Desktop + // host now admits them. The pickers share the same edit-mode condition. + const contextPickEnabled = + canStageComposerContext && + !(revisionDraft && activeId === revisionDraft.draftSessionId); const activeMessageLoadError = activeId ? messageLoadErrorBySession[activeId] : undefined; const activeTranscriptReadingAnchor = activeId @@ -2518,31 +2523,21 @@ function AppShellContent({ revisionNotice={ revisionDraft && activeId === revisionDraft.draftSessionId ? { - title: getDesktopConversationCopy(uiLocale).actions.revisionBannerTitle, - detail: getDesktopConversationCopy(uiLocale).actions.revisionBannerDetail, - cancelLabel: getDesktopConversationCopy(uiLocale).actions.revisionCancelLabel, + title: desktopConversationCopy.actions.revisionBannerTitle, + detail: desktopConversationCopy.actions.revisionBannerDetail, + cancelLabel: desktopConversationCopy.actions.revisionCancelLabel, onCancel: () => { void cancelRevisionDraft(); }, } : undefined } slashCommands={desktopSlashCommands} pendingAttachments={pendingAttachments} - onRemoveAttachment={removeAttachment} - pendingQuotes={pendingQuotes} + allowAttachmentOnlySend={canStageComposerContext} + onRemoveAttachment={removeAttachment} pendingQuotes={pendingQuotes} onRemoveQuote={removeQuote} onPasteAsQuote={canStageComposerContext ? addQuote : undefined} - onPickAttachments={ - !canStageComposerContext || - (revisionDraft && activeId === revisionDraft.draftSessionId) - ? undefined - : pickAttachments - } - onAttachFilePaths={ - !canStageComposerContext || - (revisionDraft && activeId === revisionDraft.draftSessionId) - ? undefined - : attachFilePaths - } + onPickAttachments={contextPickEnabled ? pickAttachments : undefined} + onAttachFilePaths={contextPickEnabled ? attachFilePaths : undefined} modelLabel={activeModelLabel ?? newChatModelLabel} activeSession={activeSessionForView} activeModelConnectionId={activeSessionForModelControls?.llmConnectionId} diff --git a/apps/desktop/src/renderer/features/workbar/ports.ts b/apps/desktop/src/renderer/features/workbar/ports.ts index e89389750a..775ae62f74 100644 --- a/apps/desktop/src/renderer/features/workbar/ports.ts +++ b/apps/desktop/src/renderer/features/workbar/ports.ts @@ -245,6 +245,7 @@ export interface SideChatSessionPort { placement: MessageQueuePlacement, text: string, admissionId: string, + content?: { quotes?: QuoteRef[]; attachmentItems?: WorkbarIngestInput[] }, ): Promise; queryMessageExecutions( sessionId: string, diff --git a/apps/desktop/src/renderer/features/workbar/testing.ts b/apps/desktop/src/renderer/features/workbar/testing.ts index 74ecf6d2db..17c789c783 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 31aa986337..fe00530282 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 @@ -306,7 +306,33 @@ export function QuoteCompanionPanel(props: { followUpMode: metadata?.followUpMode, compact: companion.compact, queue: companion.queue, - steer: companion.steer, + steer: async (text) => { + // Same staged-attachment validation as `send`: an unusable + // attachment rejects here with the localized toast instead + // of dying later on the steer path. + try { + preflightAttachmentItems(pendingAttachments); + } catch (error) { + toast.error( + copy.errors.sendRejected, + localizedShellErrorMessage(error, copy.errors.sendRejected, locale), + ); + return false; + } + // 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); @@ -317,16 +343,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; }, }) @@ -346,6 +376,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 b044414751..24a76738e0 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 @@ -199,10 +199,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; /** Queue text for the next companion turn while the current turn continues. */ queue: (text: string) => Promise; promoteQueuedEntry: (entryId: string) => Promise; @@ -1087,12 +1099,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 || @@ -1105,7 +1122,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 @@ -1123,7 +1139,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?.(); + }, }; const optimisticMessage: TransientUserMessageProjection = { id: turnId, @@ -1359,13 +1382,21 @@ export function useQuoteCompanion(input: UseQuoteCompanionInput): UseQuoteCompan const submitFollowUp = useCallback(async ( text: string, placement: MessageQueuePlacement, + structured?: { attachmentItems?: WorkbarIngestInput[]; onAdmitted?: () => void }, ): Promise => { const id = companionIdRef.current; const trimmed = text.trim(); + // Steering shares `send`'s structured-only contract: a quote or an + // attachment alone is a valid steering Message (#4804). Queued entries + // stay text-only, matching the Host queue contract. + const quoteSnapshot = + placement === 'current_turn' ? snapshotCompanionQuotes(panelId, pendingQuotes) : null; + const hasStructuredContent = + (quoteSnapshot?.quotes.length ?? 0) > 0 || (structured?.attachmentItems?.length ?? 0) > 0; if ( !mountedRef.current || !id || - !trimmed || + (!trimmed && !hasStructuredContent) || !turnInFlight || (placement === 'current_turn' && pendingAdmissionRef.current !== null) ) { @@ -1376,6 +1407,16 @@ export function useQuoteCompanion(input: UseQuoteCompanionInput): UseQuoteCompan messageId: admissionId, events: [], }; + if (placement === 'current_turn') { + // Steering 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. + admission.consumeOnAdmission = () => { + if (quoteSnapshot && quoteSnapshot.quotes.length > 0) onQuotesConsumed(quoteSnapshot); + structured?.onAdmitted?.(); + }; + } const optimisticMessage: TransientUserMessageProjection = { id: admissionId, text: trimmed, @@ -1389,7 +1430,14 @@ export function useQuoteCompanion(input: UseQuoteCompanionInput): UseQuoteCompan addPendingUserMessage(optimisticMessage); if (placement === 'current_turn') setPendingAdmission(admission); try { - const outcome = await sideChat.submitFollowUp(id, placement, trimmed, admissionId); + const outcome = await sideChat.submitFollowUp(id, placement, trimmed, admissionId, { + ...(quoteSnapshot && quoteSnapshot.quotes.length > 0 + ? { quotes: [...quoteSnapshot.quotes] } + : {}), + ...(structured?.attachmentItems?.length + ? { attachmentItems: structured.attachmentItems } + : {}), + }); if (!mountedRef.current) return false; if (placement === 'current_turn' && (await admission.stopPromise) === 'confirmed') { return false; @@ -1440,10 +1488,13 @@ export function useQuoteCompanion(input: UseQuoteCompanionInput): UseQuoteCompan }, [ addPendingUserMessage, bindAdmittedTurn, - reconcileStartedFollowUpTurn, - recordOwnedTurn, dropOptimisticUserMessage, mountedRef, + onQuotesConsumed, + panelId, + pendingQuotes, + reconcileStartedFollowUpTurn, + recordOwnedTurn, releaseAdmission, resolveAdmission, setPendingAdmission, @@ -1452,7 +1503,11 @@ export function useQuoteCompanion(input: UseQuoteCompanionInput): UseQuoteCompan ]); const steer = useCallback( - (text: string) => submitFollowUp(text, 'current_turn'), + ( + text: string, + attachmentItems?: WorkbarIngestInput[], + onAdmitted?: () => void, + ) => submitFollowUp(text, 'current_turn', { attachmentItems, onAdmitted }), [submitFollowUp], ); const queue = useCallback( 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 f205c25f94..d54063f404 100644 --- a/apps/desktop/src/renderer/platform/desktop/create-workbar-services.ts +++ b/apps/desktop/src/renderer/platform/desktop/create-workbar-services.ts @@ -62,6 +62,7 @@ export function createDesktopWorkbarServices( placement, text, admissionId, + content, ) => { const result = await bridge.sessions.submitMessage( sessionId, @@ -69,6 +70,11 @@ export function createDesktopWorkbarServices( { messageId: admissionId, text, + // A structured-only follow-up (a staged quote or a submitted + // attachment with no text) rides the one Message admission channel + // with its structured content (#4804). + ...(content?.quotes ? { quotes: content.quotes } : {}), + ...(content?.attachmentItems ? { attachmentItems: content.attachmentItems } : {}), }, { waitForHostAdmission: true }, ); diff --git a/packages/core/src/__tests__/runtime-event.test.ts b/packages/core/src/__tests__/runtime-event.test.ts index a4e8e7ea25..788bc68683 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,74 @@ 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('keeps whitespace-only persisted text model-visible, without trimming (#4815 review)', () => { + // Replay visibility must stay compatible with everything admission has + // ever accepted. Trimming here would re-read stored whitespace-only + // events as invisible and block replay on them — #4804's own failure. + // Surfaces that want the trimmed judgement trim at their own boundary. + assert.strictEqual( + runtimeEventHasModelVisibleContent(baseEvent({ content: { kind: 'text', text: ' ' } })), + true, + ); + assert.strictEqual(hasMeaningfulMessageContent({ text: ' ' }), true); + assert.strictEqual(hasMeaningfulMessageContent({ text: '' }), false); + assert.strictEqual(hasMeaningfulMessageContent({ text: '', quotes: [{ text: 'q' }] }), true); + }); + + test('counts directory references as a content carrier (#4815 review)', () => { + assert.strictEqual( + runtimeEventHasModelVisibleContent( + baseEvent({ + role: 'user', + content: { + kind: 'text', + text: '', + directoryReferences: [{ hostId: 'host-a', path: '/workspace/source' }], + }, + }), + ), + true, + ); + assert.strictEqual( + hasMeaningfulMessageContent({ + text: '', + directoryReferences: [{ hostId: 'host-a', path: '/workspace/source' }], + }), + 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..b9c25e817d 100644 --- a/packages/core/src/events.ts +++ b/packages/core/src/events.ts @@ -167,6 +167,30 @@ const MESSAGE_CONTENT_SHAPE = defineObjectShape()( ['text'], ['displayText', 'attachments', 'directoryReferences', 'quotes', 'inlineReferences'], ); + +/** + * A Turn message is meaningful when at least one of its four content carriers + * is present: inline text, an inline excerpt, an attachment reference, or a + * directory reference. Admission, compaction estimates, replay visibility, + * 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 deliberately NOT trimmed. Admission asks "is this frame + * legal"; replay visibility asks "will the model see this already-persisted + * event", and that answer must stay compatible with everything admission has + * ever accepted — trimming here retroactively re-reads stored history as + * invisible and blocks replay on it (#4815 review). Surfaces that want the + * trimmed judgement (the desktop guard) trim at their own boundary. + */ +export function hasMeaningfulMessageContent(content: MessageContent): boolean { + return ( + content.text.length > 0 || + (content.quotes?.length ?? 0) > 0 || + (content.attachments?.length ?? 0) > 0 || + (content.directoryReferences?.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__/execution-host-queue.test.ts b/packages/runtime-host/src/__tests__/execution-host-queue.test.ts index 0d62d9a3b3..5bc6d651bf 100644 --- a/packages/runtime-host/src/__tests__/execution-host-queue.test.ts +++ b/packages/runtime-host/src/__tests__/execution-host-queue.test.ts @@ -36,14 +36,17 @@ import { connect, type Socket } from 'node:net'; import { tmpdir } from 'node:os'; import { dirname, join } from 'node:path'; import { test } from 'node:test'; -import { TOOL_BOUNDARY_PROTOCOL_V1 } from '@maka/core/runtime-event'; +import { + TOOL_BOUNDARY_PROTOCOL_V1, + runtimeEventHasModelVisibleContent, +} from '@maka/core/runtime-event'; import { canonicalToolArgsHash } from '@maka/core/tool-args-identity'; import { runtimeInvocationOutcome, type RuntimeInvocationRecord, } from '@maka/core/runtime-invocation'; import { runtimeInvocationFailureClass } from '@maka/runtime/runtime-event-read-model'; -import type { MessageContent } from '@maka/core/events'; +import type { MessageContent, AttachmentRef } from '@maka/core/events'; import type { ConnectionCatalogEntry } from '@maka/core/runtime-policy'; import type { StoredMessage } from '@maka/core/session'; import { isTerminalRuntimeEvent } from '@maka/core/runtime-event'; @@ -59,6 +62,7 @@ import { FAKE_WAIT_FOR_STEERING_PROMPT, } from '@maka/runtime/test-only/fake-backend'; import { type MakaTool, type MakaToolContext } from '@maka/runtime/tool-runtime'; +import { openInteractiveArtifactStoreForWrite } from '@maka/storage/artifact-stores'; import { openInteractiveExecutionStoresForRead, openInteractiveExecutionStoresForWrite, @@ -97,8 +101,8 @@ import { PROCESS_TIMEOUT_MS, SubscriptionProbe, assertJsonLines, - attachment, connectClient, + quoteRefs, requireStartedTurn, operationError, quotedContent, @@ -231,6 +235,206 @@ test('subscribed Clients share one canonical queue and ordered root handoff', as }); }); +test('a quote-only queued message survives the wire snapshot, the admission chain, and a Host restart (#4804)', async () => { + await withExecutionRoot(async (fixture) => { + const host = await fixture.startHost(); + const client = await connectClient(fixture.root); + const probe = new SubscriptionProbe( + await client.openSessionSubscription({ + sessionId: fixture.sessionId, + transcript: { kind: 'none' }, + }), + ); + + // The root turn occupies the session so the quote-only submit queues as + // a follow-up instead of opening a successor. + const rootTurnId = randomUUID(); + requireStartedTurn( + await client.request('turn.start', { + sessionId: fixture.sessionId, + turnId: rootTurnId, + content: { text: `continuity root ${'x'.repeat(540)}` }, + }), + ); + + // ① The framed-client submit admits the quote-only Message. + const messageId = randomUUID(); + const queued = await client.request('turn.message.submit', { + originHostEpoch: host.hostEpoch, + sessionId: fixture.sessionId, + messageId, + content: quotedContent('the deploy failed at step three'), + placement: 'next_turn', + }); + assert.equal(queued.disposition, 'followup'); + + // ② The wire queue snapshot carries the entry with its excerpt — the + // read-back that a queue-snapshot decoder gap would break. + const projection = (await probe.waitFor( + (frame) => + frame.kind === 'subscription.session_projection' && + frame.snapshot.queue.followup.some((entry) => entry.messageId === messageId), + 'the queued quote-only message never reached the wire snapshot', + )) as Extract; + const wireEntry = projection.snapshot.queue.followup.find( + (entry) => entry.messageId === messageId, + ); + assert.match( + wireEntry?.content.text ?? '', + /the deploy failed at step three/, + 'the excerpt survives wire serialization', + ); + + // ③ A Host restart re-opens the stores and re-publishes the durable + // queue entry with the quote intact — close/reopen the whole chain. + await fixture.killHost(host); + await client.closed; + const secondHost = await fixture.startHost(); + const second = await connectClient(fixture.root); + const recoveredSubscription = await second.openSessionSubscription({ + sessionId: fixture.sessionId, + transcript: { kind: 'none' }, + }); + // The restart promotes the queued follow-up into a successor root Turn + // that runs to completion; the durable user message must carry the + // quote excerpt — the full submit -> admission -> wire snapshot -> + // reopen round trip me2seeks asked to pin (#5125 review, item 3). + const probe2 = new SubscriptionProbe(recoveredSubscription); + const successor = await probe2.waitFor( + (frame) => + frame.kind === 'subscription.session_projection' && + frame.snapshot.rootTurn !== null && + frame.snapshot.rootTurn.turnId !== rootTurnId, + 'no successor root was recovered after the Host restart', + ); + if (successor.kind !== 'subscription.session_projection' || !successor.snapshot.rootTurn) + return; + await waitForTerminalTurn(second, fixture.sessionId, successor.snapshot.rootTurn.turnId); + await second.close(); + await probe2.done; + await fixture.stopHost(secondHost); + assert.deepEqual( + (await fixture.readSessionUserMessages()) + .filter((message) => message.id === messageId) + .map((message) => message.id), + [messageId], + ); + }); +}); + +// The root-start admission path, end to end against the real Host and stores: +// the wire decoder, the durable admission authority and the replayed event +// must agree that structured content carries the turn, or a quote-only +// turn.start is refused (or stored invisible) one layer below any +// decoder-level assertion (#4804, #4815 review). +test('a quote-only turn.start forms a durable Turn whose user event stays model-visible (#4804)', async () => { + await withExecutionRoot(async (fixture) => { + const host = await fixture.startHost(); + const client = await connectClient(fixture.root); + const turnId = randomUUID(); + const content: MessageContent = { text: '', quotes: quoteRefs('root-start') }; + + const started = requireStartedTurn( + await client.request('turn.start', { + sessionId: fixture.sessionId, + turnId, + content, + }), + ); + await waitForTerminalTurn(client, fixture.sessionId, turnId); + assert.equal(started.turnId, turnId); + await client.close(); + await fixture.stopHost(host); + + const ledger = await fixture.readTurn(turnId); + assert.equal(ledger.runs.length, 1); + assert.deepEqual( + ledger.userMessages.map((message) => message.quotes), + [content.quotes], + ); + const userEvent = ledger.runtimeEvents.find( + (event) => event.role === 'user' && event.content?.kind === 'text', + ); + assert.ok(userEvent, 'the admitted turn persisted a user RuntimeEvent'); + if (!userEvent || userEvent.content?.kind !== 'text') return; + assert.deepEqual(userEvent.content.quotes, content.quotes); + // The persisted event passes the exact predicate that gates model replay: + // admission, durability and visibility decide by one rule. + assert.equal(runtimeEventHasModelVisibleContent(userEvent), true); + }); +}); + +test('an attachment-only turn.start forms a durable Turn whose user event stays model-visible (#4804)', async () => { + await withExecutionRoot(async (fixture) => { + // Stage the canonical Artifact first — the ingest step every real client + // performs before a hosted Turn may reference the attachment. + const owner = await tryAcquireInteractiveRootOwner(fixture.capability); + assert.ok(owner); + if (!owner) return; + let attachmentRef: AttachmentRef; + try { + const artifacts = await openInteractiveArtifactStoreForWrite(owner.lease); + try { + const artifact = await artifacts.create({ + sessionId: fixture.sessionId, + turnId: 'staging', + name: 'chart.png', + kind: 'image', + source: 'user_upload', + mimeType: 'image/png', + content: 'fake-png-bytes', + }); + attachmentRef = { + kind: 'image', + name: artifact.name, + mimeType: 'image/png', + bytes: artifact.sizeBytes, + ref: { + kind: 'session_file', + sessionId: fixture.sessionId, + relativePath: artifact.id, + }, + }; + } finally { + artifacts.close(); + } + } finally { + await owner.close(); + } + + const host = await fixture.startHost(); + const client = await connectClient(fixture.root); + const turnId = randomUUID(); + const content: MessageContent = { text: '', attachments: [attachmentRef] }; + + const started = requireStartedTurn( + await client.request('turn.start', { + sessionId: fixture.sessionId, + turnId, + content, + }), + ); + await waitForTerminalTurn(client, fixture.sessionId, turnId); + assert.equal(started.turnId, turnId); + await client.close(); + await fixture.stopHost(host); + + const ledger = await fixture.readTurn(turnId); + assert.equal(ledger.runs.length, 1); + assert.deepEqual( + ledger.userMessages.map((message) => message.attachments), + [content.attachments], + ); + const userEvent = ledger.runtimeEvents.find( + (event) => event.role === 'user' && event.content?.kind === 'text', + ); + assert.ok(userEvent, 'the admitted turn persisted a user RuntimeEvent'); + if (!userEvent || userEvent.content?.kind !== 'text') return; + assert.deepEqual(userEvent.content.attachments, content.attachments); + assert.equal(runtimeEventHasModelVisibleContent(userEvent), true); + }); +}); + test('production UDS admission commits one transcript before the root handoff', async () => { await withExecutionRoot(async (fixture) => { const host = await fixture.startHost(); diff --git a/packages/runtime-host/src/__tests__/protocol.test.ts b/packages/runtime-host/src/__tests__/protocol.test.ts index e34c371772..713722f461 100644 --- a/packages/runtime-host/src/__tests__/protocol.test.ts +++ b/packages/runtime-host/src/__tests__/protocol.test.ts @@ -1935,6 +1935,81 @@ 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. + // Whitespace-only text stays admissible: replay visibility must remain + // compatible with everything admission has ever accepted, so the + // predicate does not trim (#4815 review). + assert.throws(() => submit({ text: '' }), isInvalidFrame); + assert.doesNotThrow(() => submit({ text: ' ' })); + }); + + 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 64cec5a277..67cf74cdf3 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 = 149 as const; +export const RUNTIME_HOST_COMPATIBILITY_EPOCH = 150 as const; +// 150: 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. // 149: Connection model overrides retain disabled identities and separate capacity // from compaction. Catalog entries carry overrides; clients do not rebuild them. // 148: Model catalog entries include image support before a user override. 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 38707880db..5ed50892a6 100644 --- a/packages/runtime/src/__tests__/ai-sdk-backend.test.ts +++ b/packages/runtime/src/__tests__/ai-sdk-backend.test.ts @@ -2274,6 +2274,46 @@ describe('AiSdkBackend model history', () => { ); }); + test('a persisted quote-only user event replays its excerpt into the provider prompt (#4804)', async () => { + // The headline behaviour of #4804 measured at the production seam: a + // stored user event whose text is empty but whose quotes carry the turn + // must reach the provider prompt as the excerpt itself, not be skipped + // as invisible or summarized as a count. + const model = completionModel(); + const backend = createBackend({ + connection: connection(), + modelId: 'mock-model-id', + modelFactory: () => model, + tools: [], + } as never); + await drain( + backend.send({ + turnId: 'turn-current', + text: 'and the current ask', + context: [], + runtimeContext: [ + runtimeEvent({ + id: 'rt-quote', + turnId: 'turn-prev', + role: 'user', + author: 'user', + content: { + kind: 'text', + text: '', + quotes: [{ text: 'the deploy failed at step three' }], + }, + }), + ], + }), + ); + + const prompt = compactPrompt(model) as Array<{ role: string; content: unknown }>; + const historical = prompt[0]?.content as Array<{ type: string; text?: string }>; + const joined = JSON.stringify(historical); + assert.match(joined, /the deploy failed at step three/, 'the excerpt reaches the prompt'); + assert.match(joined, /quoted_excerpt/, 'the excerpt renders in its canonical envelope'); + }); + test('current-turn image attachment keeps its Read reference unless vision support is explicit', async () => { const pngBytes = new Uint8Array([0x89, 0x50, 0x4e, 0x47, 1, 2, 3]); const model = completionModel(); @@ -15253,6 +15293,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__/history-compact-checkpoint.test.ts b/packages/runtime/src/__tests__/history-compact-checkpoint.test.ts index db635d0fcf..05f615b6e3 100644 --- a/packages/runtime/src/__tests__/history-compact-checkpoint.test.ts +++ b/packages/runtime/src/__tests__/history-compact-checkpoint.test.ts @@ -950,6 +950,41 @@ describe('history compact checkpoint', () => { ); assert.equal(replay.diagnosticPatch.compactionDecisions?.[0]?.decision, 'replaced'); }); + + test('replay keeps a directory-only successor the checkpoint does not cover (#4804)', () => { + const events = Array.from({ length: 5 }, (_, index) => textEvent(index)); + // A directory-only user message is model-visible (#4804) and reaches the + // provider through the shared directory envelope, so the compact gate must + // not estimate it to zero and silently drop it from the successor tail — + // later provider requests would lose its directory context (#4815 review). + const directoryOnly: RuntimeEvent = { + ...textEvent(5), + id: 'event-directory-only', + role: 'user', + author: 'user', + content: { + kind: 'text', + text: '', + directoryReferences: [{ hostId: 'host-1', path: '/workspace/example' }], + }, + }; + const checkpoint = buildHistoryCompactCheckpoint({ + sessionId: 'session-1', + coveredRuntimeEvents: events.slice(0, 4), + summary: sectionedSummary('checkpoint summary'), + }); + + const replay = applyRuntimeEventHistoryCompact([...events, directoryOnly], { + enabled: true, + checkpoint, + }); + + assert.equal(replay.checkpoint?.checkpointId, checkpoint.checkpointId); + assert.deepEqual( + replay.events.map((event) => event.id), + [`history-compact:${checkpoint.checkpointId}`, 'event-4', 'event-directory-only'], + ); + }); }); function textEvent(index: number): RuntimeEvent { diff --git a/packages/runtime/src/__tests__/session-recap.test.ts b/packages/runtime/src/__tests__/session-recap.test.ts index 3759e1f344..cc2dd7bb3b 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..f16703f59f 100644 --- a/packages/runtime/src/model-history.ts +++ b/packages/runtime/src/model-history.ts @@ -195,8 +195,30 @@ 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 ?? []) { + // Weight the block the projection actually emits, not the display + // fields: name+mimeType is ~25 chars while the formatted attachment + // block with its Read guidance runs to hundreds (#4815 review). + total += formatAttachmentRefs([attachment]).length; + } + // Directory references project as one fixed envelope per message; count + // what it actually emits, or a directory-only message estimates to zero + // and the history-compact gate drops a model-visible event from the + // replay successors (#4815 review). + if (content.directoryReferences?.length) { + total += formatDirectoryReferences(content.directoryReferences).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..c09ca320c7 100644 --- a/packages/runtime/src/session-recap.ts +++ b/packages/runtime/src/session-recap.ts @@ -22,7 +22,7 @@ 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'; @@ -120,10 +120,14 @@ function projectSessionRecapMessages(events: readonly RuntimeEvent[]): ModelMess if (event.partial === true || !runtimeEventHasModelVisibleContent(event)) continue; 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 }); - } + // The gate above already decided visibility through the shared + // predicate, which is satisfied by non-empty text or by the structured + // carriers — so every event reaching here projects, with its trimmed + // text and staged refs rendered by the shared inline-ref formatter. + messages.push({ + role: event.role === 'user' ? 'user' : 'assistant', + content: formatTextWithInlineRefs({ ...content, text: content.text.trim() }), + }); continue; } if (content?.kind !== 'function_response') 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 ee62e1657e..25db1a595d 100644 --- a/packages/ui/src/__tests__/chat-turn-answer-identity.test.tsx +++ b/packages/ui/src/__tests__/chat-turn-answer-identity.test.tsx @@ -369,6 +369,69 @@ 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.ok( + container.querySelector('.maka-message-meta'), + 'a structured-only message keeps its metadata row', + ); +}); + +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..814ff6d379 100644 --- a/packages/ui/src/__tests__/composer-send-toggle.test.tsx +++ b/packages/ui/src/__tests__/composer-send-toggle.test.tsx @@ -47,6 +47,15 @@ function sendSlotControls(markup: string): string[] { return markup.match(/aria-label="(?:Send|Stop)"/g) ?? []; } +/** The Send control's own `aria-disabled` value — asserted directly, not by a + * substring that could also hit `data-disabled` or other future attributes. */ +function sendButtonAriaDisabled(markup: string): string | null { + const document = parseHTML(`${markup}`).document; + const button = document.querySelector('button[aria-label="Send"]'); + assert.ok(button, 'the send slot renders Send'); + return button.getAttribute('aria-disabled'); +} + test('an idle composer offers Send alone', () => { const controls = sendSlotControls(renderComposer(false)); assert.deepEqual(controls, ['aria-label="Send"']); @@ -64,6 +73,111 @@ test('a running composer keeps Send alone — no mode switch in the send slot', assert.doesNotMatch(markup, /SegmentedControl/); }); +// Pins the #5003 opt-in contract, not a #4815 regression: base already passed +// this exact assertion (reviewed at the #4815 head). What #4815 adds on top — +// staged quotes counting as sendable content without the flag — is covered by +// the staged-quote cases in this file. +test('an opted-in host renders Send (not Stop) for an attachment-only draft (#5003)', () => { + const attachments = [{ displayName: 'kept.png', kind: 'image' as const, size: 12 }]; + const markup = renderToStaticMarkup( + + undefined} + onStop={() => undefined} + /> + , + ); + assert.match(markup, /aria-label="Send"/); + assert.equal(sendButtonAriaDisabled(markup), null); + // 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.equal(sendButtonAriaDisabled(optedOut), 'true'); +}); + +test('a staged quote enables Send without any host opt-in (#4804)', () => { + const quotes = [ + { text: 'the deploy failed at step three', label: 'Assistant', sourceTurnId: 'turn-9' }, + ]; + const staged = renderToStaticMarkup( + + undefined} onStop={() => undefined} /> + , + ); + // The toggle and the disabled state agree: a quote-only draft is a live + // Send, and it needs no per-host decision the way attachments do. + assert.deepEqual(sendSlotControls(staged), ['aria-label="Send"']); + assert.equal(sendButtonAriaDisabled(staged), null); + // The same empty draft with nothing staged is what disabled looks like, so + // the assertion above pins the staged quote as the enabling reason. + assert.equal(sendButtonAriaDisabled(renderComposer(false)), 'true'); +}); + +test('the three send gates agree about a staged quote while streaming (#4804)', async () => { + const original = { + document: globalThis.document, + window: globalThis.window, + IS_REACT_ACT_ENVIRONMENT: (globalThis as typeof globalThis & { + IS_REACT_ACT_ENVIRONMENT?: boolean; + }).IS_REACT_ACT_ENVIRONMENT, + }; + const { document, window } = parseHTML('
'); + window.getComputedStyle = () => ({ + direction: 'ltr', + writingMode: 'horizontal-tb', + getPropertyValue: () => '', + }) as unknown as CSSStyleDeclaration; + Object.assign(globalThis, { document, window, IS_REACT_ACT_ENVIRONMENT: true }); + const container = document.querySelector('#root'); + assert.ok(container); + const root = createRoot(container); + const sends: string[] = []; + try { + await act(() => root.render( + + { + sends.push(text); + }} + onStop={() => undefined} + /> + , + )); + // Gate 1 — the send/stop toggle: mid-turn the slot stays on Send because + // the staged quote is handable content, not an empty draft. + assert.deepEqual(sendSlotControls(container.innerHTML), ['aria-label="Send"']); + const button = container.querySelector('button[aria-label="Send"]'); + assert.ok(button); + // Gate 2 — sendDisabled: the control is live. + assert.equal(button.getAttribute('aria-disabled'), null); + // Gate 3 — sendCurrent's content guard: submitting hands the empty draft + // text over, the quote travelling as the message's structured content. + // The control is type="submit", so its activation is the form's submit. + const form = container.querySelector('form'); + assert.ok(form); + await act(async () => { + form.dispatchEvent(new window.Event('submit', { bubbles: true, cancelable: true })); + await Promise.resolve(); + }); + assert.deepEqual(sends, ['']); + } finally { + await act(() => root.unmount()); + Object.assign(globalThis, original); + } +}); + 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 85ed5d091d..089b85dce4 100644 --- a/packages/ui/src/chat-turn.tsx +++ b/packages/ui/src/chat-turn.tsx @@ -260,18 +260,25 @@ 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, but the + metadata (timestamp, copy, edit entry) still belongs to the message. */} + {props.text.trim().length > 0 ? ( + + {props.inlineReferences ? ( + + ) : ( + + {props.text} + + )} + + ) : ( + userMetadata + )} ); }); diff --git a/packages/ui/src/composer.tsx b/packages/ui/src/composer.tsx index 166f786c25..7ee2a9c1c7 100644 --- a/packages/ui/src/composer.tsx +++ b/packages/ui/src/composer.tsx @@ -1262,6 +1262,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 @@ -1273,7 +1282,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(); @@ -1461,7 +1470,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. @@ -1472,7 +1481,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 ?? []);