From b5389bc07522150d55d266c9caac15e7d1eb6dcf Mon Sep 17 00:00:00 2001 From: hugocasa Date: Fri, 11 Sep 2026 16:44:08 +0200 Subject: [PATCH] fix(ai-agent): keep offline replay inert and the streamed answer to one turn Co-Authored-By: Claude Opus 5 (1M context) --- backend/windmill-worker/src/ai_executor.rs | 51 ++++++++++++++++--- .../ResourceDescriptionField.svelte | 15 +++--- .../lib/components/ResourcePathHint.svelte | 5 +- .../src/lib/components/aiAgentResult.test.ts | 29 +++++++++++ frontend/src/lib/components/aiAgentResult.ts | 8 +++ .../chat/WebSearchSourcesDisplay.svelte | 9 +++- 6 files changed, 99 insertions(+), 18 deletions(-) diff --git a/backend/windmill-worker/src/ai_executor.rs b/backend/windmill-worker/src/ai_executor.rs index 502e49452a..498f4687b5 100644 --- a/backend/windmill-worker/src/ai_executor.rs +++ b/backend/windmill-worker/src/ai_executor.rs @@ -79,6 +79,18 @@ lazy_static::lazy_static! { const DEFAULT_MAX_AGENT_ITERATIONS: usize = 10; const HARD_MAX_AGENT_ITERATIONS: usize = 1000; +/// What a run stopped by `max_iterations` reports back: the conversation it got +/// through before giving up. +/// +/// `Message` rather than `OpenAIMessage` is load-bearing. `agent_action` is +/// `skip_serializing` on `OpenAIMessage` and only reaches JSON through this +/// wrapper, so serializing these raw drops every tool name and job id and leaves +/// the partial run unreadable — which is the one thing worth having on this path. +#[derive(serde::Serialize)] +struct MaxIterPartialResult<'a> { + messages: Vec>, +} + fn strip_system_messages(messages: &[OpenAIMessage]) -> Vec { messages .iter() @@ -1614,14 +1626,6 @@ pub async fn run_agent( step_id: Option<&'a str>, result: MaxIterPartialResult<'a>, } - // Through the same wrapper the success envelope uses: `agent_action` - // is `skip_serializing` on `OpenAIMessage`, so serializing these raw - // would drop every tool name and job id and leave the partial run - // unreadable — which is the one thing worth having on this path. - #[derive(serde::Serialize)] - struct MaxIterPartialResult<'a> { - messages: Vec>, - } return Err(Error::ExecutionRawError( serde_json::value::to_raw_value(&MaxIterError { message: format!( @@ -1899,6 +1903,37 @@ mod tests { assert!(!streaming_requested(Some(false))); } + /// The frontend reads a capped run's partial actions out of this payload and + /// keys entirely off `agent_action`. That field is `skip_serializing` on + /// `OpenAIMessage`, so serializing these messages directly rather than through + /// `Message` silently empties the trace of the one run worth reading. + #[test] + fn max_iterations_partial_result_keeps_the_action_tags() { + let messages = vec![OpenAIMessage { + role: "tool".to_string(), + content: Some(OpenAIContent::Text("{\"rows\":2}".to_string())), + tool_call_id: Some("call_1".to_string()), + agent_action: Some(AgentAction::ToolCall { + job_id: uuid::Uuid::nil(), + function_name: "list_payouts".to_string(), + module_id: "b".to_string(), + }), + ..Default::default() + }]; + + let partial = MaxIterPartialResult { + messages: messages + .iter() + .map(|m| Message { message: m, agent_action: m.agent_action.as_ref() }) + .collect(), + }; + let json = serde_json::to_value(&partial).unwrap(); + + let action = &json["messages"][0]["agent_action"]; + assert_eq!(action["type"], "tool_call"); + assert_eq!(action["function_name"], "list_payouts"); + } + /// Over 64 characters OpenAI rejects the key outright, which costs a wasted round /// trip per run and silently leaves that step with no prompt caching at all. #[test] diff --git a/frontend/src/lib/components/ResourceDescriptionField.svelte b/frontend/src/lib/components/ResourceDescriptionField.svelte index 524548fa9d..2f065176dc 100644 --- a/frontend/src/lib/components/ResourceDescriptionField.svelte +++ b/frontend/src/lib/components/ResourceDescriptionField.svelte @@ -6,7 +6,7 @@ import { Button } from '$lib/components/common' import GfmMarkdown from './GfmMarkdown.svelte' import Required from './Required.svelte' - import autosize from '$lib/autosize' + import TextInput from './text_input/TextInput.svelte' interface Props { description: string @@ -35,6 +35,9 @@ unifiedSize="xs" btnClasses={editing ? 'bg-surface-hover' : ''} startIcon={{ icon: Pen }} + iconOnly + title={editing ? 'Stop editing the description' : 'Edit the description'} + aria-label={editing ? 'Stop editing the description' : 'Edit the description'} on:click={() => (editing = !editing)} /> {/if} @@ -42,13 +45,11 @@ {#if canWrite && editing}
GH Markdown
- + inputProps={{ placeholder, 'aria-label': label, disabled: !canWrite }} + />
{:else if description == undefined || description == ''}
No description provided
diff --git a/frontend/src/lib/components/ResourcePathHint.svelte b/frontend/src/lib/components/ResourcePathHint.svelte index 527a2aed2e..ba92f17d99 100644 --- a/frontend/src/lib/components/ResourcePathHint.svelte +++ b/frontend/src/lib/components/ResourcePathHint.svelte @@ -1,5 +1,6 @@ - +
The path sets who can access this resource: a u/ path is private to that user, an f/ path follows the folder's permissions — read access lets people use the resource, write diff --git a/frontend/src/lib/components/aiAgentResult.test.ts b/frontend/src/lib/components/aiAgentResult.test.ts index 22702b7456..99ce2c5a56 100644 --- a/frontend/src/lib/components/aiAgentResult.test.ts +++ b/frontend/src/lib/components/aiAgentResult.test.ts @@ -123,3 +123,32 @@ describe('formatTokenCount', () => { expect(formatTokenCount(48211)).toBe('48k') }) }) + +// The worker streams every iteration's text, and a turn may narrate and call a +// tool at once. Appending across that boundary makes the live answer read +// "I'll checkThe issue is..." and never converge on the finished output. +describe('a stream that narrates before calling a tool', () => { + it('keeps only the turn that produced the answer', () => { + const raw = [ + '{"type":"token_delta","content":"Let me check the metrics."}', + '{"type":"tool_call","call_id":"c1","function_name":"query_metrics"}', + '{"type":"tool_result","call_id":"c1","function_name":"query_metrics","result":"{}","success":true}', + '{"type":"token_delta","content":"eu-central-1 is down."}', + '' + ].join('\n') + expect(advanceAgentStream(raw, emptyAgentStreamProgress()).stream.answer).toBe( + 'eu-central-1 is down.' + ) + }) + + it('drops the narration at the boundary even across polls', () => { + const first = '{"type":"token_delta","content":"Let me check."}\n' + const afterCall = first + '{"type":"tool_call","call_id":"c1","function_name":"q"}\n' + const poll1 = advanceAgentStream(first, emptyAgentStreamProgress()) + expect(poll1.stream.answer).toBe('Let me check.') + const poll2 = advanceAgentStream(afterCall, poll1) + expect(poll2.stream.answer).toBe('') + const poll3 = advanceAgentStream(afterCall + '{"type":"token_delta","content":"Done."}\n', poll2) + expect(poll3.stream.answer).toBe('Done.') + }) +}) diff --git a/frontend/src/lib/components/aiAgentResult.ts b/frontend/src/lib/components/aiAgentResult.ts index 758e29c300..2710e5493b 100644 --- a/frontend/src/lib/components/aiAgentResult.ts +++ b/frontend/src/lib/components/aiAgentResult.ts @@ -232,6 +232,14 @@ export function advanceAgentStream( } else if (event.type === 'reasoning_token_delta' && typeof event.content === 'string') { stream.reasoning += event.content } else if (typeof event.function_name === 'string') { + if (event.type === 'tool_call') { + // A model can narrate and request a tool in the same turn, and the loop + // then runs again. That narration is not part of the answer — the + // finished result keeps only the last turn's text — so a new call + // starts the answer over rather than appending to what came before. + stream.answer = '' + stream.reasoning = '' + } stream.tool = { name: event.function_name, running: event.type !== 'tool_result', diff --git a/frontend/src/lib/components/copilot/chat/WebSearchSourcesDisplay.svelte b/frontend/src/lib/components/copilot/chat/WebSearchSourcesDisplay.svelte index ff71c44160..075fba6768 100644 --- a/frontend/src/lib/components/copilot/chat/WebSearchSourcesDisplay.svelte +++ b/frontend/src/lib/components/copilot/chat/WebSearchSourcesDisplay.svelte @@ -2,6 +2,7 @@ import { Globe } from 'lucide-svelte' import { SvelteSet } from 'svelte/reactivity' import type { WebSearchSource } from './shared' + import { isOfflineReplay } from '$lib/components/recording/offlineReplay.svelte' interface Props { sources: WebSearchSource[] @@ -38,6 +39,12 @@ const failedFavicons = new SvelteSet() + // The favicon is a request to a third party, and the public replay page promises + // to issue none — a recording comes from an arbitrary origin, so its cited + // hostnames must not leak from a viewer's browser either. Degrades to the same + // Globe the blocked/failed case already uses. + const noFavicons = $derived(isOfflineReplay()) + // Favicons come from Google's public favicon service, which discloses each // consulted hostname to a third party from the user's browser — an accepted // tradeoff for now (blocked/air-gapped environments degrade to the Globe @@ -62,7 +69,7 @@ title={source.url} class="flex items-center gap-2 py-1 px-1.5 rounded hover:bg-surface-hover min-w-0" > - {#if failedFavicons.has(hostname)} + {#if noFavicons || failedFavicons.has(hostname)} {:else}