diff --git a/frontend/src/lib/components/copilot/chat/AIChatInput.svelte b/frontend/src/lib/components/copilot/chat/AIChatInput.svelte index 072a704f66..dc73ebc615 100644 --- a/frontend/src/lib/components/copilot/chat/AIChatInput.svelte +++ b/frontend/src/lib/components/copilot/chat/AIChatInput.svelte @@ -237,7 +237,11 @@ pendingFiles + draft.blobs.length + pendingBlobs - return Math.min(laneRemaining, Math.max(0, turnCap - staged)) + // A queue counts too: what is held mid-run merges into one turn on flush, so a + // second file accepted now would be dropped there instead of refused here. + const queued = + chatHost.queuedImages.length + chatHost.queuedFiles.length + chatHost.queuedBlobs.length + return Math.min(laneRemaining, Math.max(0, turnCap - staged - queued)) } /** What to say when the host's own limit is the one that bit. */ diff --git a/frontend/src/lib/components/flows/conversations/FlowChatManager.svelte.ts b/frontend/src/lib/components/flows/conversations/FlowChatManager.svelte.ts index eec40b11d3..bd6854ed1a 100644 --- a/frontend/src/lib/components/flows/conversations/FlowChatManager.svelte.ts +++ b/frontend/src/lib/components/flows/conversations/FlowChatManager.svelte.ts @@ -331,6 +331,13 @@ export class FlowChatManager { await this.#writeConversationTitle(conversationId, trimmed) } + /** No job came of the send, so nothing is in flight and nothing is waiting on one. */ + #turnFailedToStart() { + this.isLoading = false + this.isWaitingForResponse = false + this.isDispatchingTurn = false + } + /** A turn is being dispatched or is running: nothing may move the conversation under it. */ get isTurnInFlight(): boolean { return this.isLoading || this.isWaitingForResponse || this.isDispatchingTurn @@ -588,7 +595,16 @@ export class FlowChatManager { * An empty message is allowed here: a turn can carry attachments alone, and only the * caller knows whether it does. */ - async sendMessage(additionalInputs?: Record, onUserRow?: (rowId: string) => void) { + async sendMessage( + additionalInputs?: Record, + onUserRow?: (rowId: string) => void, + /** + * The conversation the turn was started in. Passed by a caller that had to await + * something first — an attachment upload — since the reader can select another + * conversation while it runs, and the turn belongs to the one they sent it from. + */ + pinnedConversationId?: string + ) { if (this.isLoading) return const isNewConversation = this.messages.length === 0 @@ -597,8 +613,8 @@ export class FlowChatManager { this.stopPolling() // Generate a new conversation ID if we don't have one - let currentConversationId = this.selectedConversationId - if (!this.selectedConversationId) { + let currentConversationId = pinnedConversationId ?? this.selectedConversationId + if (!currentConversationId) { const newConversationId = await this.createConversation({ clearMessages: false }) currentConversationId = newConversationId } @@ -649,6 +665,10 @@ export class FlowChatManager { } catch (error) { console.error('Error running flow:', error) sendUserToast('Failed to run flow: ' + error, true) + // A turn that never started leaves nothing to wait for. Said here as well as in + // the finally because the streaming path keeps `isLoading` for its own stream, + // and without this the composer and the sidebar stay locked until a reload. + this.#turnFailedToStart() } finally { if (!this.#useStreaming) { this.isLoading = false @@ -692,6 +712,7 @@ export class FlowChatManager { const jobId = await this.#onRunFlow?.(messageContent, currentConversationId, additionalInputs) if (!jobId) { console.error('No jobId returned from onRunFlow') + this.#turnFailedToStart() return } @@ -829,6 +850,7 @@ export class FlowChatManager { const jobId = await this.#onRunFlow?.(messageContent, currentConversationId, additionalInputs) if (!jobId) { console.error('No jobId returned from onRunFlow') + this.#turnFailedToStart() return } diff --git a/frontend/src/lib/components/flows/conversations/FlowConversationsSidebar.svelte b/frontend/src/lib/components/flows/conversations/FlowConversationsSidebar.svelte index e6b8137eb7..db460cde3f 100644 --- a/frontend/src/lib/components/flows/conversations/FlowConversationsSidebar.svelte +++ b/frontend/src/lib/components/flows/conversations/FlowConversationsSidebar.svelte @@ -83,6 +83,11 @@ all: 'All' } + /** A turn is running and this is not the chat it is running in. */ + function rowLocked(conversation: ConversationWithDraft): boolean { + return manager.isTurnInFlight && manager.selectedConversationId !== conversation.id + } + function getConversationTitle(conversation: FlowConversation): string { return conversation.title || `Conversation ${conversation.created_at.slice(0, 10)}` } @@ -122,7 +127,10 @@ variant="subtle" startIcon={{ icon: Plus, classes: 'ml-[2px]' }} onClick={() => manager.createConversation({ clearMessages: true })} - title="Start new conversation" + disabled={manager.isTurnInFlight} + title={manager.isTurnInFlight + ? 'Wait for the current answer to start a new chat' + : 'Start new conversation'} iconOnly={!manager.isSidebarExpanded} wrapperClasses={manager.isSidebarExpanded ? 'grow min-w-0' : ''} btnClasses={'w-full justify-start transition-all duration-150 whitespace-nowrap'} @@ -245,6 +253,10 @@ variant="subtle" onClick={() => manager.selectConversation(conversation.id, conversation.isDraft)} selected={manager.selectedConversationId === conversation.id} + disabled={rowLocked(conversation)} + title={rowLocked(conversation) + ? 'Wait for the current answer to switch conversation' + : undefined} btnClasses="transition-all duration-150 group" > {#if conversation.is_test} @@ -255,17 +267,22 @@ {getConversationTitle(conversation)} - - -
e.stopPropagation()} - > - rowActions(conversation)} size="xs" /> -
+ + {#if !rowLocked(conversation)} + + +
e.stopPropagation()} + > + rowActions(conversation)} size="xs" /> +
+ {/if} {/if} diff --git a/frontend/src/lib/components/flows/conversations/agentChatInputs.test.ts b/frontend/src/lib/components/flows/conversations/agentChatInputs.test.ts index 3bc4c6d119..bb848c3bd2 100644 --- a/frontend/src/lib/components/flows/conversations/agentChatInputs.test.ts +++ b/frontend/src/lib/components/flows/conversations/agentChatInputs.test.ts @@ -1,7 +1,20 @@ import { describe, expect, it } from 'vitest' -import { agentModelGap, parseProviderTransform, resolveAgentModelWiring } from './agentChatInputs' +import { + agentModelGap, + parseProviderTransform, + resolveAgentChatInputs, + resolveAgentModelWiring +} from './agentChatInputs' import type { FlowModule } from '$lib/gen' +/** An agent step with the given input transforms, as the editor stores them. */ +function agentWith(input_transforms: Record): FlowModule { + return { + id: 'a', + value: { type: 'aiagent', tools: [], input_transforms } + } as unknown as FlowModule +} + function agent(expr: string): FlowModule { return { id: 'a', @@ -89,6 +102,34 @@ describe('parseProviderTransform', () => { }) }) +describe('resolveAgentChatInputs', () => { + const schema = { properties: { files: { type: 'array' } }, required: [] } + const reader = (name: string) => + agentWith({ user_attachments: { type: 'javascript', expr: `flow_input.${name}` } }) + // Every agent step carries a placeholder transform for each key of AI_AGENT_SCHEMA + // (loadSchemaFromModule writes them back onto the module), so an agent that reads + // nothing must not be mistaken for one reading a different input. + const seeded = () => agentWith({ user_attachments: { type: 'static', value: undefined } }) + + it('promotes the input one agent reads', () => { + expect(resolveAgentChatInputs([reader('files')], schema).map((i) => i.name)).toEqual(['files']) + }) + + it('still promotes it when another agent leaves the field unwired', () => { + expect(resolveAgentChatInputs([reader('files'), seeded()], schema).map((i) => i.name)).toEqual([ + 'files' + ]) + }) + + it('promotes nothing when two agents read different inputs', () => { + const twoInputs = { + properties: { files: { type: 'array' }, docs: { type: 'array' } }, + required: [] + } + expect(resolveAgentChatInputs([reader('files'), reader('docs')], twoInputs)).toEqual([]) + }) +}) + describe('resolveAgentModelWiring', () => { const fixedResource = `"kind": "anthropic", "resource": "$res:u/admin/claude"` @@ -132,7 +173,12 @@ describe('resolveAgentModelWiring', () => { } } ] - expect(resolveAgentModelWiring([parent])).toEqual({ whole: 'model', fields: {}, fixed: {} }) + expect(resolveAgentModelWiring([parent])).toEqual({ + whole: 'model', + fields: {}, + fixed: {}, + someAgentCannotRun: false + }) }) // The control writes one flow input; an agent that fixes the field instead never reads @@ -144,7 +190,6 @@ describe('resolveAgentModelWiring', () => { ]) expect(wiring?.fields.model).toBeUndefined() expect(wiring?.fixed.model).toBeUndefined() - expect(wiring?.undecided).toContain('model') }) // Disagreeing about the model is not the same as having no model: the flow runs, on a @@ -163,6 +208,29 @@ describe('resolveAgentModelWiring', () => { ).toBe('Pick a provider and model on the AI agent step to use this chat.') }) + // An expression the parser cannot account for could supply anything, so the agents it + // belongs to cannot be spoken for either. + it("offers nothing when one agent's provider cannot be read", () => { + expect( + resolveAgentModelWiring([ + agent(`({ ${fixedResource}, model: flow_input.model })`), + agent(`({ ...base, model: flow_input.model })`) + ]) + ).toBeUndefined() + }) + + // Disagreement is not the same as absence, but an agent with an empty model still + // cannot run, however well its neighbour is configured. + it('keeps warning when one agent has no model and another does', () => { + const wiring = resolveAgentModelWiring([ + agent(`({ ${fixedResource}, "model": "claude-sonnet-5" })`), + agent(`({ ${fixedResource}, "model": "" })`) + ]) + expect(agentModelGap(wiring)).toBe( + 'Pick a provider and model on the AI agent step to use this chat.' + ) + }) + it('refuses a flow mixing whole-object and field-by-field wiring', () => { expect( resolveAgentModelWiring([ diff --git a/frontend/src/lib/components/flows/conversations/agentChatInputs.ts b/frontend/src/lib/components/flows/conversations/agentChatInputs.ts index 8b3c02c9be..91f6231097 100644 --- a/frontend/src/lib/components/flows/conversations/agentChatInputs.ts +++ b/frontend/src/lib/components/flows/conversations/agentChatInputs.ts @@ -107,12 +107,11 @@ export type AgentModelWiring = { fields: Partial> fixed: Partial> /** - * Fields the agents supply differently from one another — one reading an input where - * another writes a literal, or two literals that disagree. Neither editable nor known, - * and told apart from a field nobody supplies at all: a control offered here would - * govern one agent while the rest ran on something else. + * One of the agents names no resource or no model of its own and no flow input feeds + * it, so that agent's run fails whatever the others do. Held apart from the fields, + * which describe what the composer may offer. */ - undecided?: ProviderField[] + someAgentCannotRun?: boolean } /** The flow input behind `flow_input.x`, `flow_input?.x` or `flow_input['x']`. */ @@ -200,6 +199,13 @@ type FieldSupply = | { kind: 'fixed'; value: any } | { kind: 'absent' } +/** Whether one agent supplies a field with nothing usable: no input, and no literal. */ +function agentFieldEmpty(wiring: AgentModelWiring, field: ProviderField): boolean { + if (wiring.fields[field] !== undefined) return false + const value = wiring.fixed[field] + return value === undefined || value === '' +} + function fieldSupply(wiring: AgentModelWiring, field: ProviderField): FieldSupply { const name = wiring.fields[field] if (name !== undefined) return { kind: 'wired', name } @@ -219,35 +225,44 @@ function fieldSupply(wiring: AgentModelWiring, field: ProviderField): FieldSuppl export function resolveAgentModelWiring( modules: FlowModule[] | undefined ): AgentModelWiring | undefined { - const wirings = agentSteps(modules) - .map((agent) => parseProviderTransform((agent.value as any).input_transforms?.['provider'])) - .filter((wiring): wiring is AgentModelWiring => wiring !== undefined) - if (wirings.length === 0) return undefined - if (wirings.length === 1) return wirings[0] + const agents = agentSteps(modules) + const parsed = agents.map((agent) => + parseProviderTransform((agent.value as any).input_transforms?.['provider']) + ) + if (parsed.length === 0) return undefined + // An agent whose provider cannot be read is an agent the composer cannot speak for: + // dropping it would let the rest declare a control that governs only some of them. + if (parsed.some((wiring) => wiring === undefined)) return undefined + const wirings = parsed as AgentModelWiring[] + // Whether any single agent has nothing to call, which stays true however the others + // are wired — the gap message is about that agent, not about their agreement. + const someAgentCannotRun = wirings.some( + (wiring) => + !wiring.whole && (agentFieldEmpty(wiring, 'resource') || agentFieldEmpty(wiring, 'model')) + ) + if (wirings.length === 1) return { ...wirings[0], someAgentCannotRun } const wholes = new Set(wirings.map((w) => w.whole)) if (wholes.size === 1 && !wholes.has(undefined)) { - return { whole: [...wholes][0], fields: {}, fixed: {} } + return { whole: [...wholes][0], fields: {}, fixed: {}, someAgentCannotRun } } if (wirings.some((w) => w.whole !== undefined)) return undefined const fields: AgentModelWiring['fields'] = {} const fixed: AgentModelWiring['fixed'] = {} - const undecided: ProviderField[] = [] for (const field of PROVIDER_FIELDS) { // Every agent has to supply the field the same way for the composer to speak for // them all. One wired name among agents that otherwise fix it is not agreement: // the control would move that one agent and leave the others where they are. const supplies = new Set(wirings.map((w) => JSON.stringify(fieldSupply(w, field)))) - if (supplies.size > 1) { - undecided.push(field) - continue - } + // Disagreement leaves the field neither editable nor known: a control offered here + // would govern one agent while the rest ran on something else. + if (supplies.size > 1) continue const supply: FieldSupply = JSON.parse([...supplies][0]) if (supply.kind === 'wired') fields[field] = supply.name else if (supply.kind === 'fixed') fixed[field] = supply.value } - return { fields, fixed, undecided } + return { fields, fixed, someAgentCannotRun } } /** @@ -261,15 +276,10 @@ export function resolveAgentModelWiring( export function agentModelGap(wiring: AgentModelWiring | undefined): string | undefined { // No agent, several of them, or an expression we cannot read: not ours to judge. if (!wiring || wiring.whole) return undefined - // Agents that disagree about what to call are not agents with nothing to call: the flow - // may well run, on a different model per agent, and this message would be false. - if (wiring.undecided?.some((field) => field === 'resource' || field === 'model')) { - return undefined - } - const missing = (field: ProviderField) => - wiring.fields[field] === undefined && - (wiring.fixed[field] === undefined || wiring.fixed[field] === '') - return missing('resource') || missing('model') + // Asked of each agent rather than of what they agree on: agents that merely disagree + // about the model all have one, and the message would be false — while an agent with + // an empty model still cannot run, however well the others are configured. + return wiring.someAgentCannotRun ? 'Pick a provider and model on the AI agent step to use this chat.' : undefined } @@ -300,18 +310,39 @@ export function resolveAgentChatInputs( ? additionalInputsSchema.required : [] - const keyOf = new Map() + // One input per key, and only when every agent reading that key reads the same one: + // the composer writes a single flow input, so promoting one of two would feed one + // agent and leave the other with nothing — while hiding both from the modal, where + // the reader could at least have filled them in. + const namesPerKey = new Map>() for (const module of agentSteps(modules)) { const transforms = (module.value as any).input_transforms ?? {} for (const key of AGENT_CHAT_INPUT_KEYS) { - const name = flowInputRef(transforms[key]) - // A name the schema doesn't declare has no field to promote, and `user_message` - // is already the composer itself. - if (!name || !(name in properties) || keyOf.has(name)) continue - keyOf.set(name, key) + const transform = transforms[key] + // An agent that feeds the key from anything but a flow input — a literal, another + // step's result, or the empty placeholder every agent step carries for the keys of + // AI_AGENT_SCHEMA — is not reading an input, so it has no say in which one the + // composer drives. + if (transform?.type !== 'javascript' || !transform.expr.includes('flow_input')) continue + const name = flowInputRef(transform) + // A name the schema doesn't declare has no field to promote, and one expression + // reading two inputs names none: either way this agent reads something the + // composer cannot drive, which is what disagreement means here. + const usable = name && name in properties ? name : undefined + const names = namesPerKey.get(key) ?? new Set() + names.add(usable) + namesPerKey.set(key, names) } } + const keyOf = new Map() + for (const [key, names] of namesPerKey) { + if (names.size !== 1) continue + const name = [...names][0] + if (name === undefined || keyOf.has(name)) continue + keyOf.set(name, key) + } + return [...keyOf.entries()] .map(([name, key]) => ({ name, diff --git a/frontend/src/lib/components/flows/conversations/flowChatViewHost.svelte.ts b/frontend/src/lib/components/flows/conversations/flowChatViewHost.svelte.ts index ebf972d133..81a939f72e 100644 --- a/frontend/src/lib/components/flows/conversations/flowChatViewHost.svelte.ts +++ b/frontend/src/lib/components/flows/conversations/flowChatViewHost.svelte.ts @@ -280,6 +280,9 @@ export class FlowChatViewHost implements ChatViewHost { ) } const attachments = [...images, ...blobs] + // Read before the upload below: the reader can pick another chat while it runs, and + // the turn belongs to the one they sent it from. + const conversationId = this.#manager.selectedConversationId let sentInputs: MessageInputs | undefined if (target && attachments.length > 0) { this.#uploading = true @@ -315,7 +318,8 @@ export class FlowChatViewHost implements ChatViewHost { Object.entries(this.#sentInputs).filter(([id]) => live.has(id)) ) this.#sentInputs = { ...kept, [rowId]: sentInputs } - } + }, + conversationId ) return true }