diff --git a/backend/migrations/20260907131755_flow_conversation_is_test.up.sql b/backend/migrations/20260907131755_flow_conversation_is_test.up.sql index 5730496ba5..df4ddb8981 100644 --- a/backend/migrations/20260907131755_flow_conversation_is_test.up.sql +++ b/backend/migrations/20260907131755_flow_conversation_is_test.up.sql @@ -3,13 +3,25 @@ -- lists tell a trial apart from a real conversation. ALTER TABLE flow_conversation ADD COLUMN is_test BOOLEAN NOT NULL DEFAULT false; --- Existing rows: a conversation whose messages came from a flowpreview job was a test. +-- Existing rows: a conversation whose messages came from a flowpreview run was a test. -- Derived once here because the job is purged on retention, after which the origin of an -- old conversation is unknowable. +-- +-- Walked to the root job rather than matched directly: an existing message row never holds +-- the flow job itself. Only this migration's release starts storing it on the user row, and +-- the rows written before it point at the step that produced them — the AI agent's job for +-- an answer, the tool's own job for a tool call — whose kind is never 'flowpreview'. +-- +-- `root_job` first, matching `get_root_job_id` (windmill-worker/src/common.rs): only it +-- reaches the top of the run. `flow_innermost_root_job` stops at the closest flow scope by +-- design, so an agent inside a subflow would land on that subflow's 'flow' row and the +-- conversation would read as deployed. UPDATE flow_conversation c SET is_test = true WHERE EXISTS ( SELECT 1 FROM flow_conversation_message m JOIN v2_job j ON j.id = m.job_id - WHERE m.conversation_id = c.id AND j.kind = 'flowpreview' + JOIN v2_job root + ON root.id = coalesce(j.root_job, j.flow_innermost_root_job, j.parent_job, j.id) + WHERE m.conversation_id = c.id AND root.kind = 'flowpreview' ); diff --git a/frontend/src/lib/components/copilot/ReasoningEffortSlider.svelte b/frontend/src/lib/components/copilot/ReasoningEffortSlider.svelte index fc140588c8..6f12f630ec 100644 --- a/frontend/src/lib/components/copilot/ReasoningEffortSlider.svelte +++ b/frontend/src/lib/components/copilot/ReasoningEffortSlider.svelte @@ -33,9 +33,18 @@ overrideLabel }: Props = $props() - // A `current` that names no stop is a real state — an agent that leaves the effort unset - // sends nothing and the provider decides — and the thumb then rests at the start. Only - // `overrideLabel` tells the two apart, since a range input always has a thumb somewhere. + /** + * A `current` naming no stop is a real state, not a missing one: an agent that leaves the + * effort unset sends nothing and the provider decides. Three things follow, and each has + * been got wrong on its own — keep them together. + * + * The thumb rests at the start, because a range input always has one somewhere, and + * `overrideLabel` is what tells the reader this is not the lowest stop. The track is + * unfilled there, which index 0 gives for free. And since the input's value already reads + * 0, picking the lowest stop by pointer fires no `input` event — so a click has to be + * committed explicitly, or that stop is reachable only by keyboard. + */ + const hasPosition = $derived(stops.indexOf(current) >= 0) const stopIndex = $derived(Math.max(0, stops.indexOf(current))) // Percentage filled (accent) up to the thumb; the rest of the track stays surface-secondary. const fillPct = $derived( @@ -92,6 +101,15 @@ style="--fill: {fillPct}%" {disabled} oninput={(e) => onSelect(stops[+e.currentTarget.value])} + onclick={(e) => { + // `click`, not `pointerup`: it is the event that means pressed and released on + // the track, so a press that began on the row above cannot commit an effort + // nobody chose. Only the click that moved nothing — any other stop has already + // committed through `oninput`, and doing it again would write it twice. + if (!hasPosition && !disabled && +e.currentTarget.value === stopIndex) { + onSelect(stops[stopIndex]) + } + }} use:isolatePointer class="lean-range no-default-style w-full" aria-label="Reasoning effort" diff --git a/frontend/src/lib/components/copilot/chat/AIChatDisplay.svelte b/frontend/src/lib/components/copilot/chat/AIChatDisplay.svelte index 172ea09755..73e4dbaba5 100644 --- a/frontend/src/lib/components/copilot/chat/AIChatDisplay.svelte +++ b/frontend/src/lib/components/copilot/chat/AIChatDisplay.svelte @@ -180,7 +180,6 @@ wideLayout = false, emptyHint, inputPreface, - footerControls, footerSettings, initialInstructions = undefined, onDraftChange = undefined, @@ -213,8 +212,6 @@ wideLayout?: boolean emptyHint?: Snippet inputPreface?: Snippet - /** Extra controls at the head of the composer's footer row. */ - footerControls?: Snippet /** The settings control at the footer's right edge, where the copilot puts its * model picker. A host that configures its turn elsewhere replaces it here. */ footerSettings?: Snippet @@ -648,8 +645,7 @@ // nothing else would lose the group and the `+` with it. const showFooterLeftControls = $derived( !footerMessageShown && - (footerControls !== undefined || - canAttachFiles || + (canAttachFiles || showContextPicker || showAutonomyModeSelector || (chatHost.mode === AIMode.SCRIPT && hasDiff)) @@ -973,7 +969,6 @@ the panel, or the Escape-to-stop focus check would wrongly reject them. --> > {#if showFooterLeftControls}
- {@render footerControls?.()} {#if showContextPicker && !disabled} {#snippet trigger()} diff --git a/frontend/src/lib/components/copilot/chat/blobUtils.test.ts b/frontend/src/lib/components/copilot/chat/blobUtils.test.ts new file mode 100644 index 0000000000..2c3b50a64a --- /dev/null +++ b/frontend/src/lib/components/copilot/chat/blobUtils.test.ts @@ -0,0 +1,42 @@ +import { describe, expect, it } from 'vitest' +import { dataUrlToBlob, matchesAccept } from './blobUtils' + +function file(name: string, type: string): File { + return new File(['x'], name, { type }) +} + +describe('dataUrlToBlob', () => { + // The bytes are re-uploaded verbatim, so a decode that drops or shifts one is a + // corrupted file the reader only discovers downstream. + it('decodes base64 back to the exact bytes', async () => { + const bytes = new Uint8Array([0x00, 0xff, 0x10, 0x89, 0x50]) + const b64 = btoa(String.fromCharCode(...bytes)) + const blob = dataUrlToBlob(`data:application/pdf;base64,${b64}`) + expect(blob.type).toBe('application/pdf') + expect(new Uint8Array(await blob.arrayBuffer())).toEqual(bytes) + }) + + it('percent-decodes a url that is not base64', async () => { + const blob = dataUrlToBlob('data:text/plain,hello%20world') + expect(blob.type).toBe('text/plain') + expect(await blob.text()).toBe('hello world') + }) + + it('falls back to a media type when the url names none', async () => { + expect(dataUrlToBlob('data:;base64,QQ==').type).toBe('application/octet-stream') + expect(dataUrlToBlob('data:;base64,QQ==', 'image/png').type).toBe('image/png') + }) +}) + +describe('matchesAccept', () => { + it('matches an extension, a type wildcard and an exact media type', () => { + expect(matchesAccept(file('report.PDF', ''), '.pdf')).toBe(true) + expect(matchesAccept(file('shot.png', 'image/png'), 'image/*')).toBe(true) + expect(matchesAccept(file('shot.png', 'image/png'), 'image/png')).toBe(true) + }) + + it('refuses a file no pattern covers, and allows everything when the list is empty', () => { + expect(matchesAccept(file('notes.txt', 'text/plain'), '.pdf, image/*')).toBe(false) + expect(matchesAccept(file('notes.txt', 'text/plain'), '')).toBe(true) + }) +}) diff --git a/frontend/src/lib/components/flows/conversations/FlowChatInterface.svelte b/frontend/src/lib/components/flows/conversations/FlowChatInterface.svelte index 9a0cf9131f..cc3ffd935f 100644 --- a/frontend/src/lib/components/flows/conversations/FlowChatInterface.svelte +++ b/frontend/src/lib/components/flows/conversations/FlowChatInterface.svelte @@ -78,8 +78,9 @@ const STORAGE_KEY_PREFIX = 'windmill_flow_chat_inputs_' let showInputsModal = $state(false) - // Conversation settings, persisted per flow. Attachments are absent by construction: - // they ride the composer's own draft and are cleared with it on send. + // Conversation settings, persisted per flow. These can include a value for the + // attachments input, saved while the modal was its editor; `sendRequest` drops that one + // once the paperclip takes over, so a stored file never rides a later message. let inputValues = $state>(loadInputsFromStorage() ?? {}) let modalDraft = $state>({}) diff --git a/frontend/src/lib/components/flows/conversations/agentChatInputs.ts b/frontend/src/lib/components/flows/conversations/agentChatInputs.ts index 9a7f81bdc6..12ebbc581c 100644 --- a/frontend/src/lib/components/flows/conversations/agentChatInputs.ts +++ b/frontend/src/lib/components/flows/conversations/agentChatInputs.ts @@ -1,4 +1,6 @@ import type { FlowModule, InputTransform } from '$lib/gen' +import { parseExpressionAt } from 'acorn' + /** * The flow's own AI agent steps, including those inside loops and branches but never one * carried as another agent's tool. @@ -30,7 +32,6 @@ function agentSteps(modules: FlowModule[] | undefined): FlowModule[] { walk(modules ?? []) return found } -import { parseExpressionAt } from 'acorn' /** * AI agent inputs the chat composer can drive, in footer display order. diff --git a/frontend/src/lib/components/flows/conversations/attachmentNames.test.ts b/frontend/src/lib/components/flows/conversations/attachmentNames.test.ts new file mode 100644 index 0000000000..3204e57237 --- /dev/null +++ b/frontend/src/lib/components/flows/conversations/attachmentNames.test.ts @@ -0,0 +1,31 @@ +import { describe, expect, it } from 'vitest' +import { storedAttachmentName } from './attachmentNames' + +/** + * The worker reads an attachment's media type from the object key and nothing else, so a + * key whose extension disagrees with the bytes reaches the provider mislabelled. + */ +describe('storedAttachmentName', () => { + // The composer re-encodes images, so the picked extension is the one that lies. + it('renames a re-encoded image to the type it was encoded as', () => { + expect(storedAttachmentName('photo.webp', 'image/png')).toBe('photo.png') + expect(storedAttachmentName('holiday.png', 'image/jpeg')).toBe('holiday.jpg') + }) + + it('gives an extension to a name that has none', () => { + expect(storedAttachmentName('attachment-1', 'image/png')).toBe('attachment-1.png') + expect(storedAttachmentName('contract', 'application/pdf')).toBe('contract.pdf') + }) + + it('replaces only the last extension', () => { + expect(storedAttachmentName('report.2026.final.webp', 'image/png')).toBe( + 'report.2026.final.png' + ) + }) + + // Blobs upload byte for byte, so a type we do not re-encode keeps the name as picked. + it('leaves a type it does not re-encode alone', () => { + expect(storedAttachmentName('notes.csv', 'text/csv')).toBe('notes.csv') + expect(storedAttachmentName('archive.zip', 'application/zip')).toBe('archive.zip') + }) +}) diff --git a/frontend/src/lib/components/flows/conversations/attachmentNames.ts b/frontend/src/lib/components/flows/conversations/attachmentNames.ts new file mode 100644 index 0000000000..05ecc68fda --- /dev/null +++ b/frontend/src/lib/components/flows/conversations/attachmentNames.ts @@ -0,0 +1,28 @@ +/** + * What a chat attachment is stored under in object storage. + * + * The worker reads an attachment's media type from the object key and nothing else — + * `mime_guess::from_path` in `windmill-ai/src/image_handler.rs`, falling back to + * `image/png` when it can read no extension — and never from the content type stored + * beside it. So the key's extension is a claim about the bytes, and it has to be true. + */ + +/** The extension each type the composer can send must be stored under. */ +const EXTENSION_BY_MEDIA_TYPE: Record = { + 'image/png': 'png', + 'image/jpeg': 'jpg', + 'application/pdf': 'pdf' +} + +/** + * The composer re-encodes every image to PNG or JPEG, so keeping the picked `photo.webp` + * would hand the provider PNG bytes labelled webp, which Anthropic rejects outright; and a + * PDF picked without an extension would be read back as the `image/png` fallback. A type + * not listed is left as picked — blobs upload byte for byte, so their name is already true. + */ +export function storedAttachmentName(filename: string, mediaType: string): string { + const extension = EXTENSION_BY_MEDIA_TYPE[mediaType] + if (!extension) return filename + const stem = filename.replace(/\.[^./]+$/, '') + return `${stem || filename}.${extension}` +} diff --git a/frontend/src/lib/components/flows/conversations/flowChatViewHost.svelte.ts b/frontend/src/lib/components/flows/conversations/flowChatViewHost.svelte.ts index adf3988e26..e84a50372a 100644 --- a/frontend/src/lib/components/flows/conversations/flowChatViewHost.svelte.ts +++ b/frontend/src/lib/components/flows/conversations/flowChatViewHost.svelte.ts @@ -10,6 +10,7 @@ import { ToolCallStore, type ToolCallDetails } from './toolCallContext.svelte' import { AttachedFilesStore } from '$lib/components/copilot/chat/files/attachedFiles.svelte' import { SessionArtifactsStore } from '$lib/components/copilot/chat/artifacts/artifactsState.svelte' import { dataUrlToBlob, type AttachedBlob } from '$lib/components/copilot/chat/blobUtils' +import { storedAttachmentName } from './attachmentNames' import type { AttachedImage } from '$lib/components/copilot/chat/imageUtils' import type { AttachedTextFile } from '$lib/components/copilot/chat/textFileUtils' import { HelpersService } from '$lib/gen' @@ -259,6 +260,13 @@ export class FlowChatViewHost implements ChatViewHost { const text = options.instructions?.trim() ?? '' const args = { ...(this.#options.additionalInputs?.() ?? {}) } const target = this.#options.attachmentsTarget?.() + // Where the paperclip is the input's editor, the stored settings have no say over it: + // a value saved while the modal owned it — before this workspace had object storage — + // would otherwise ride along on every later message. Attachments are set below or not + // at all. Where the modal still owns it, what the reader typed there stands. + if (target && this.supportsMessageAttachments) { + delete args[target.name] + } let images = options.images ?? [] let blobs = options.blobs ?? [] // The composer refuses an attachment-only send (requiresMessageText), so this is @@ -354,8 +362,11 @@ export class FlowChatViewHost implements ChatViewHost { const prefix = `windmill_chat_uploads/${randomUUID()}` return Promise.all( attachments.map(async (attachment, index) => { - const filename = attachment.name ?? `attachment-${index + 1}` const blob = dataUrlToBlob(attachment.dataUrl, attachment.mediaType) + const filename = storedAttachmentName( + attachment.name ?? `attachment-${index + 1}`, + blob.type + ) const { file_key } = await HelpersService.fileUpload({ workspace, fileKey: `${prefix}/${filename}`, @@ -366,6 +377,7 @@ export class FlowChatViewHost implements ChatViewHost { }) ) } + cancel = () => { void this.#manager.cancelCurrentJob() } diff --git a/frontend/src/lib/components/flows/conversations/turnTranscript.ts b/frontend/src/lib/components/flows/conversations/turnTranscript.ts index 7a44833446..547672fbf1 100644 --- a/frontend/src/lib/components/flows/conversations/turnTranscript.ts +++ b/frontend/src/lib/components/flows/conversations/turnTranscript.ts @@ -52,7 +52,7 @@ function newRow(state: TurnState, id: string, patch: Partial): Chat } /** Everything still streaming is finished: nothing further will be appended to it. */ -export function settleStreamingRows(rows: ChatMessage[]): ChatMessage[] { +function settleStreamingRows(rows: ChatMessage[]): ChatMessage[] { return rows.map((row) => (row.streaming ? { ...row, streaming: false } : row)) }