fix(ai-chat): make an attachment's key agree with its bytes, and the test backfill find anything

Addresses the CI reviewers on f41174e, plus what the local rounds found on top.

- An image is re-encoded to PNG/JPEG on the way in but kept the picked filename,
  and the worker reads an attachment's media type from the object key alone
  (`mime_guess::from_path`), never from the stored content type — so a `.webp`
  reached the provider as PNG bytes labelled webp, which Anthropic rejects.
  `storedAttachmentName` now makes the extension match the encoding; the same
  rule covers a PDF picked without one, whose key would guess as `image/png`.
- The `is_test` backfill matched no existing conversation: no message row held
  the flow job before this release, so it now walks to the run's root, ordered
  as `get_root_job_id` is — `flow_innermost_root_job` stops at the closest flow
  scope, which would miss an agent inside a subflow.
- A stored value for the attachments input no longer rides an attachment-free
  message: the paperclip is that input's only writer wherever it is offered.
- The lowest thinking stop is now clickable from the `default` state, where the
  range input's value already read 0 and so fired no event.
- Dropped `footerControls` and the `settleStreamingRows` export, neither of
  which ever had a caller.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QN7VboDEm9HAB1t4sMxMdE

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'
 );

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"

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}
 					<div class="flex flex-row items-center gap-x-1.5 min-w-0 flex-wrap">
-						{@render footerControls?.()}
 						{#if showContextPicker && !disabled}
 							<Popover placement="bottom-start">
 								{#snippet trigger()}

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)
+	})
+})

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<Record<string, any>>(loadInputsFromStorage() ?? {})
 	let modalDraft = $state<Record<string, any>>({})
 

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.

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')
+	})
+})

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<string, string> = {
+	'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}`
+}

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()
 	}

frontend/src/lib/components/flows/conversations/turnTranscript.ts

@@ -52,7 +52,7 @@ function newRow(state: TurnState, id: string, patch: Partial<ChatMessage>): 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))
 }