diff --git a/chat-sdk/src/attachments.ts b/chat-sdk/src/attachments.ts index 10d14eb6df..d89429f512 100644 --- a/chat-sdk/src/attachments.ts +++ b/chat-sdk/src/attachments.ts @@ -73,13 +73,8 @@ function dataUrlToBlob(dataUrl: string, fallbackType: string): Blob { /** * Put each attachment in the workspace's object storage and hand back what the agent reads. - * The flow runs on a worker, so the bytes have to exist somewhere the worker can fetch. - * - * A prefix per turn, and a segment per attachment inside it. The turn's prefix keeps a - * re-attached filename off the copy an earlier message still points at; the segment does the - * same within one turn, where two files can arrive under one name and would otherwise race to - * a single key and leave the agent reading one of them twice. The name itself stays the last - * segment, so anything that reads a name off the key still sees what the user attached. + * The key's turn prefix and per-file index keep two files with the same name, in this turn or + * an earlier one, from overwriting each other; the name stays the last segment. */ export async function uploadAttachments( api: WindmillChatApi, diff --git a/frontend/src/lib/components/copilot/chat/AIChatInput.svelte b/frontend/src/lib/components/copilot/chat/AIChatInput.svelte index c2be28d9c9..8682a81e2d 100644 --- a/frontend/src/lib/components/copilot/chat/AIChatInput.svelte +++ b/frontend/src/lib/components/copilot/chat/AIChatInput.svelte @@ -1380,7 +1380,7 @@ ? (pasted) => void addImages(pasted) : undefined} onTextFiles={chatHost.supportsMessageAttachments - ? (pasted) => void addTextFiles(pasted) + ? (pasted) => void addNonImageFiles(pasted) : undefined} {availableContext} {selectedContext} diff --git a/frontend/src/lib/components/copilot/chat/messageDraft.test.ts b/frontend/src/lib/components/copilot/chat/messageDraft.test.ts index a6a9bb6169..dec8e1e6b9 100644 --- a/frontend/src/lib/components/copilot/chat/messageDraft.test.ts +++ b/frontend/src/lib/components/copilot/chat/messageDraft.test.ts @@ -5,22 +5,49 @@ import { MessageDraft } from './messageDraft.svelte' // the draft-level guarantees: lanes move together, restores respect occupancy, // aggregation always applies the rules. +function blob(name: string) { + return { + name, + mediaType: 'application/pdf', + dataUrl: 'data:application/pdf;base64,JVBERg==', + size: 4 + } +} + describe('MessageDraft', () => { - it('take() snapshots and clears all four lanes atomically', () => { + it('take() snapshots and clears all five lanes atomically', () => { const d = new MessageDraft({ text: 'hello', pastes: [{ id: 'p1', content: 'x' } as any], images: [{ dataUrl: 'i1' } as any], - files: [{ name: 'a.md', content: 'a' }] + files: [{ name: 'a.md', content: 'a' }], + blobs: [blob('a.pdf')] }) const snap = d.take() expect(snap.text).toBe('hello') expect(snap.pastes).toHaveLength(1) expect(snap.images).toHaveLength(1) expect(snap.files).toHaveLength(1) + expect(snap.blobs).toHaveLength(1) expect(d.isEmpty).toBe(true) }) + it('treats a blob-only draft as occupied and caps blobs, keeping restored ones first', () => { + const d = new MessageDraft({ blobs: [blob('only.pdf')] }) + expect(d.isEmpty).toBe(false) + expect(d.hasAttachments).toBe(true) + expect(d.replaceIfEmpty({ text: 'restored' })).toBe(false) + + const dropped = d.addBlobs(Array.from({ length: 8 }, (_, i) => blob(`new${i}.pdf`))) + expect(dropped).toBe(1) + expect(d.blobs).toHaveLength(8) + + const res = d.prepend({ text: '', blobs: [blob('old.pdf')] }) + expect(res.droppedBlobs).toBe(1) + expect(d.blobs[0].name).toBe('old.pdf') + expect(d.blobs).toHaveLength(8) + }) + it('replaceIfEmpty declines when any lane is occupied', () => { const d = new MessageDraft({ files: [{ name: 'a.md', content: 'a' }] }) expect(d.replaceIfEmpty({ text: 'restored' })).toBe(false) diff --git a/frontend/src/lib/components/flows/conversations/agentAttachmentInput.ts b/frontend/src/lib/components/flows/conversations/agentAttachmentInput.ts index a4300f5534..42aaad844c 100644 --- a/frontend/src/lib/components/flows/conversations/agentAttachmentInput.ts +++ b/frontend/src/lib/components/flows/conversations/agentAttachmentInput.ts @@ -1,13 +1,9 @@ import type { FlowModule, InputTransform } from '$lib/gen' /** - * The flow's own AI agent steps, including those inside loops and branches but never one - * carried as another agent's tool. - * - * The graph walks an agent's tools as if they were child steps (flowTree.ts), which is - * right for the graph and wrong here: a tool agent's inputs belong to the agent that - * calls it, not to the chat. Counting it would let a nested agent's wiring speak for the - * step the reader is actually talking to. + * The flow's own AI agent steps, including those inside loops and branches. An agent carried + * as another agent's tool is left out, unlike in the graph (flowTree.ts): its inputs come from + * the agent calling it, not from the chat. */ export function agentSteps(modules: FlowModule[] | undefined): FlowModule[] { const found: FlowModule[] = [] @@ -83,12 +79,9 @@ function holdsS3File(property: Record | undefined): boolean { export type AttachmentsTarget = { name: string; multiple: boolean; required?: boolean } /** - * Where the composer's attachments go, or nothing when there is nowhere they fit. - * - * The agent reads `user_attachments` through a transform that may reshape what it takes, so - * the flow input feeding it is not necessarily an s3 field: an expression building the s3 - * object itself promotes a plain string. Writing `{ s3, filename }` into that input fails at - * run time, so the paperclip appears only where the schema says the value belongs. + * Where the composer's attachments go: the promoted input, only when its schema holds s3 + * objects. A transform may build the s3 object from a plain string input, and writing + * `{ s3, filename }` into that input would fail at run time. */ export function attachmentsTargetFor( input: AgentChatInput | undefined diff --git a/frontend/src/lib/components/flows/conversations/flowChatViewHost.svelte.ts b/frontend/src/lib/components/flows/conversations/flowChatViewHost.svelte.ts index faead079cc..6d8b0384d4 100644 --- a/frontend/src/lib/components/flows/conversations/flowChatViewHost.svelte.ts +++ b/frontend/src/lib/components/flows/conversations/flowChatViewHost.svelte.ts @@ -253,10 +253,8 @@ export class FlowChatViewHost implements ChatViewHost { // The attachments are this input's only editor: a value stored for it in the inputs // modal would otherwise ride along on every message. if (target) delete inputs[target.name] - // The per-turn cap again, at the place the truncation would happen: the composer - // enforces it as files are attached, but a queue built over several turns arrives - // here as one send, and a scalar input keeps the first upload — uploading the rest - // would strand them in storage while the transcript claimed they went. + // The composer caps files as they are attached, but a queue merged over several turns + // arrives here as one send, and the chat refuses more than a single-file input holds. const cap = this.maxMessageAttachments if (cap !== undefined && images.length + blobs.length > cap) { const dropped = images.length + blobs.length - cap @@ -391,10 +389,10 @@ export class FlowChatViewHost implements ChatViewHost { // Per-message actions storedImages = () => undefined - /** Send the user message at this transcript position again. */ /** The files each user message of this session went out with, for Retry. A message loaded * from history has none recorded here and retries with its text alone. */ #sentAttachments = new Map() + /** Send the user message at this transcript position again. */ retryRequest = (messageIndex: number) => { const message = this.#state.messages[messageIndex] if (!message || message.role !== 'user' || this.loading) return @@ -430,8 +428,7 @@ export class FlowChatViewHost implements ChatViewHost { // handle on the user's own disk — has no meaning here. supportsLinkedFolders = false attachmentsAsBlobs = true - // A scalar flow input holds one file; sending more would upload every one and run with - // the first, leaving the rest orphaned in storage and the transcript claiming otherwise. + // A single-file flow input takes one attachment per message. get maxMessageAttachments(): number | undefined { return this.#options.attachmentsTarget?.()?.multiple === false ? 1 : undefined } diff --git a/frontend/src/lib/components/flows/conversations/flowChatViewHost.test.ts b/frontend/src/lib/components/flows/conversations/flowChatViewHost.test.ts index 4ddeb03573..56be29bfe3 100644 --- a/frontend/src/lib/components/flows/conversations/flowChatViewHost.test.ts +++ b/frontend/src/lib/components/flows/conversations/flowChatViewHost.test.ts @@ -306,8 +306,7 @@ describe('FlowChatViewHost', () => { ]) }) - // The composer caps as files are attached, but a queue built over several turns - // arrives as one send; a scalar input would keep the first upload and strand the rest. + // A queue merged over several turns reaches the host as one send. it('re-applies a single-file cap to a merged queue', async () => { const { chat } = fakeChat() const host = new FlowChatViewHost(chat, {