refactor: read agent memory as either a legacy shape or the current one

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
hugocasa
2026-09-16 09:30:13 +02:00
co-authored by Claude Opus 5
parent 7bbd0b65de
commit 4e2eec4c64
15 changed files with 195 additions and 172 deletions
+4 -12
View File
@@ -88,7 +88,7 @@ pub enum Memory {
#[serde(default, deserialize_with = "deserialize_blank_as_none")]
memory_id: Option<Uuid>,
},
/// Written before the step's `previous_messages` input, which it is equivalent to.
/// Written before a step had history inputs of its own, and read on its own where it remains.
Manual {
messages: Vec<OpenAIMessage>,
},
@@ -107,12 +107,6 @@ fn deserialize_blank_as_none<'de, D: serde::Deserializer<'de>>(
}
}
fn deserialize_present_messages<'de, D: serde::Deserializer<'de>>(
deserializer: D,
) -> Result<Option<Option<Vec<OpenAIMessage>>>, D::Error> {
<Option<Vec<OpenAIMessage>> as serde::Deserialize>::deserialize(deserializer).map(Some)
}
fn deserialize_present<'de, D: serde::Deserializer<'de>>(
deserializer: D,
) -> Result<Option<serde_json::Value>, D::Error> {
@@ -137,10 +131,8 @@ struct AIAgentArgsRaw {
// nothing runs stateless instead of falling back to the run's memory id.
#[serde(default, deserialize_with = "deserialize_present")]
memory_id: Option<serde_json::Value>,
// Same distinction for an authored previous messages expression: null replaces a legacy manual
// list with no history, where an absent key keeps that list.
#[serde(default, deserialize_with = "deserialize_present_messages")]
previous_messages: Option<Option<Vec<OpenAIMessage>>>,
#[serde(default)]
previous_messages: Option<Vec<OpenAIMessage>>,
enabled_tools: Option<Vec<String>>,
// Legacy field for backward compatibility
messages_context_length: Option<usize>,
@@ -207,7 +199,7 @@ impl From<AIAgentArgsRaw> for AIAgentArgs {
max_iterations: raw.max_iterations,
memory,
memory_id,
previous_messages: raw.previous_messages.map(Option::unwrap_or_default),
previous_messages: raw.previous_messages,
enabled_tools: raw.enabled_tools,
credentials_check: raw.credentials_check.unwrap_or(false),
}
+121 -107
View File
@@ -119,10 +119,10 @@ enum HistorySource<'a> {
Stateless,
}
/// The step's history inputs count only as the step authored them. A static empty value is a form
/// A step's memory id counts only as the step authored it. A static empty value is a form
/// placeholder, so it reads as unset rather than as an expression that evaluated to nothing, which
/// runs without memory; an AI-filled value would let the model choose which memory the agent reads.
fn keep_authored_history_args(
fn keep_authored_memory_id(
args: &mut AIAgentArgs,
step_input_transforms: &HashMap<String, InputTransform>,
) {
@@ -131,21 +131,12 @@ fn keep_authored_history_args(
Some(InputTransform::Static { .. }) if args.memory_id.as_deref() != Some("") => {}
_ => args.memory_id = None,
}
match step_input_transforms.get("previous_messages") {
Some(InputTransform::Javascript { .. }) => {}
Some(InputTransform::Static { .. })
if args
.previous_messages
.as_ref()
.is_some_and(|m| !m.is_empty()) => {}
_ => args.previous_messages = None,
}
}
/// Reconciles the step's history inputs, the agent's memory policy and the run's memory id, for
/// every shape a flow or agent resource may still carry. Managed memory reads only a memory id, and
/// memory that is off reads only previous messages. Also returns lines for the job log: a step
/// input that went unused, or a policy that remembers ending up stateless.
/// Reconciles the step's history inputs, the agent's memory policy and the run's memory id. A step
/// holds one of two shapes: an older `auto` or `manual` memory, read as the editor that wrote it
/// meant it, or the current setting plus the step's own history inputs. Also returns lines for the
/// job log: an input that went unused, or a policy that remembers ending up stateless.
fn resolve_history_source<'a>(
args: &'a AIAgentArgs,
run_memory_id: Option<Uuid>,
@@ -153,57 +144,97 @@ fn resolve_history_source<'a>(
flow_path: &str,
) -> (HistorySource<'a>, Vec<&'static str>) {
let mut notes = Vec::new();
let (context_length, legacy_memory_id) = match &args.memory {
Some(Memory::Window { context_length }) => (*context_length, None),
// An id baked in at save time only ever applied when the run carried none.
Some(Memory::Auto { context_length, memory_id }) => (*context_length, *memory_id),
Some(Memory::Manual { .. } | Memory::Off) | None => {
let no_memory_id = "No memory id was passed to this run, so the agent runs without memory.";
match &args.memory {
// The step's own history inputs came after these, so a step that still holds one reads it
// alone: what it did before the editor offered them is what it keeps doing.
Some(Memory::Manual { messages }) => {
note_unread_step_inputs(&mut notes, args);
(HistorySource::Messages(messages), notes)
}
Some(Memory::Auto { context_length, memory_id }) => {
note_unread_step_inputs(&mut notes, args);
// An id baked in at save time only ever applied when the run carried none.
match run_memory_id.or(*memory_id) {
Some(memory_id) => (
HistorySource::Window { memory_id, context_length: *context_length },
notes,
),
None => {
notes.push(no_memory_id);
(HistorySource::Stateless, notes)
}
}
}
Some(Memory::Window { context_length }) => {
if args
.previous_messages
.as_ref()
.is_some_and(|messages| !messages.is_empty())
{
notes.push("Managed memory is on, so this step's previous messages are ignored.");
}
let memory_id = match args.memory_id.as_deref() {
Some("") => {
notes.push(
"This step's memory id evaluated to an empty value, so the agent runs without memory.",
);
return (HistorySource::Stateless, notes);
}
Some(step_memory_id) => memory_key(workspace_id, flow_path, step_memory_id),
None => match run_memory_id {
Some(memory_id) => memory_id,
None => {
notes.push(no_memory_id);
return (HistorySource::Stateless, notes);
}
},
};
(
HistorySource::Window { memory_id, context_length: *context_length },
notes,
)
}
Some(Memory::Off) | None => {
if args.memory_id.as_deref().is_some_and(|id| !id.is_empty()) {
notes.push("Managed memory is off, so this step's memory id is ignored.");
}
let history = match (&args.previous_messages, &args.memory) {
(Some(messages), _) => HistorySource::Messages(messages),
// The fixed list an older editor stored in `memory`, which the step's previous messages
// replace.
(None, Some(Memory::Manual { messages })) => HistorySource::Messages(messages),
_ => HistorySource::Stateless,
};
return (history, notes);
match &args.previous_messages {
Some(messages) => (HistorySource::Messages(messages), notes),
None => (HistorySource::Stateless, notes),
}
}
};
}
}
/// An older memory setting reads neither history input, which is only visible in the job log: the
/// editor offers them on a step that has been moved to the current settings.
fn note_unread_step_inputs(notes: &mut Vec<&'static str>, args: &AIAgentArgs) {
if args.memory_id.as_deref().is_some_and(|id| !id.is_empty()) {
notes.push("This step uses an older memory setting, so its memory id is not read.");
}
if args
.previous_messages
.as_ref()
.is_some_and(|messages| !messages.is_empty())
{
notes.push("Managed memory is on, so this step's previous messages are ignored.");
notes
.push("This step uses an older memory setting, so its previous messages are not read.");
}
let memory_id = match args.memory_id.as_deref() {
Some("") => {
notes.push(
"This step's memory id evaluated to an empty value, so the agent runs without memory.",
);
return (HistorySource::Stateless, notes);
}
Some(step_memory_id) => memory_key(workspace_id, flow_path, step_memory_id),
None => match run_memory_id.or(legacy_memory_id) {
Some(memory_id) => memory_id,
None => {
notes
.push("No memory id was passed to this run, so the agent runs without memory.");
return (HistorySource::Stateless, notes);
}
},
};
(HistorySource::Window { memory_id, context_length }, notes)
}
/// Whether a request has something to ask the model. Only text output sends previous messages, so an
/// image prompt comes from the user message alone. An empty list, as an expression evaluating to
/// null gives, is no conversation.
fn has_prompt(history: &HistorySource, has_user_message: bool, is_text_output: bool) -> bool {
/// Whether a request has something to ask the model. Only text output sends previous messages, so
/// an image prompt comes from the user message alone. An empty list is no conversation, except
/// under a legacy `manual` memory, which ran on whatever list it held.
fn has_prompt(
history: &HistorySource,
has_user_message: bool,
is_text_output: bool,
legacy_list: bool,
) -> bool {
has_user_message
|| (is_text_output && matches!(history, HistorySource::Messages(m) if !m.is_empty()))
|| (is_text_output
&& (legacy_list || matches!(history, HistorySource::Messages(m) if !m.is_empty())))
}
fn find_module_by_id(
@@ -655,7 +686,7 @@ pub async fn handle_ai_agent_job(
(args, tools)
};
keep_authored_history_args(&mut args, &step_input_transforms);
keep_authored_memory_id(&mut args, &step_input_transforms);
// Nesting is capped at flow → agent → nested agent. When this job is itself a nested tool,
// a linked resource's tool set may still contain AIAgent tools (the editor can't constrain a
@@ -1149,7 +1180,10 @@ pub async fn run_agent(
}
}
if !has_prompt(&history, has_user_message, is_text_output) {
// A `manual` memory sent whatever list it held, an empty one included, so a step that still has
// one keeps running without a user message.
let legacy_list = matches!(args.memory, Some(Memory::Manual { .. }));
if !has_prompt(&history, has_user_message, is_text_output, legacy_list) {
let missing = if !is_text_output {
"'user_message' must be provided for image output"
} else if matches!(
@@ -2074,10 +2108,22 @@ mod tests {
Resolved::Messages(1),
),
(
"the step's previous messages replace a legacy manual list",
"a previous messages expression that evaluated to null is no history",
json!({ "previous_messages": null }),
Some(run),
Resolved::Stateless { noted: false },
),
(
"a legacy manual list ignores the step's previous messages",
json!({ "memory": { "kind": "manual", "messages": message }, "previous_messages": two_messages }),
Some(run),
Resolved::Messages(2),
Resolved::Messages(1),
),
(
"legacy auto ignores a step memory id",
json!({ "memory": { "kind": "auto", "context_length": 4, "memory_id": baked }, "memory_id": "cust_1" }),
None,
Resolved::Window(baked, 4),
),
];
for (name, history, run_memory_id, expected) in cases {
@@ -2113,7 +2159,6 @@ mod tests {
serde_json::from_value(serde_json::json!({
"provider": { "kind": "openai", "resource": {}, "model": "m" },
"memory_id": null,
"previous_messages": [],
}))
.unwrap()
};
@@ -2127,52 +2172,8 @@ mod tests {
),
] {
let mut args = args();
keep_authored_history_args(&mut args, &transforms(transform));
keep_authored_memory_id(&mut args, &transforms(transform));
assert_eq!(args.memory_id.as_deref(), expected, "{transform}");
assert!(args.previous_messages.is_none(), "{transform}");
}
}
/// Empty previous messages a form leaves on a step never replace a legacy list; an expression
/// does, even when it evaluates to null.
#[test]
fn only_an_expression_can_empty_a_legacy_message_list() {
let run = Uuid::from_u128(1);
for (transform, expected) in [
(
r#"{ "type": "javascript", "expr": "flow_input.history" }"#,
Resolved::Messages(0),
),
(
r#"{ "type": "static", "value": null }"#,
Resolved::Messages(1),
),
(
r#"{ "type": "static", "value": [] }"#,
Resolved::Messages(1),
),
] {
let mut args: AIAgentArgs = serde_json::from_value(serde_json::json!({
"provider": { "kind": "openai", "resource": {}, "model": "m" },
"memory": { "kind": "manual", "messages": [{ "role": "user", "content": "earlier" }] },
"previous_messages": if transform.contains("[]") { serde_json::json!([]) } else { serde_json::Value::Null },
}))
.unwrap();
let transforms = HashMap::from([(
"previous_messages".to_string(),
serde_json::from_str(transform).unwrap(),
)]);
keep_authored_history_args(&mut args, &transforms);
let resolved = match resolve_history_source(&args, Some(run), "ws", "f/flow") {
(HistorySource::Messages(m), _) => Resolved::Messages(m.len()),
(HistorySource::Window { memory_id, context_length }, _) => {
Resolved::Window(memory_id, context_length)
}
(HistorySource::Stateless, notes) => {
Resolved::Stateless { noted: !notes.is_empty() }
}
};
assert_eq!(resolved, expected, "{transform}");
}
}
@@ -2185,10 +2186,23 @@ mod tests {
}))
.unwrap();
let (history, _) = resolve_history_source(&args, None, "ws", "f/flow");
assert!(has_prompt(&history, false, true));
assert!(!has_prompt(&history, false, false));
assert!(has_prompt(&history, true, false));
assert!(!has_prompt(&HistorySource::Messages(&[]), false, true));
assert!(has_prompt(&history, false, true, false));
assert!(!has_prompt(&history, false, false, false));
assert!(has_prompt(&history, true, false, false));
assert!(!has_prompt(
&HistorySource::Messages(&[]),
false,
true,
false
));
// A legacy `manual` memory ran on an empty list alone, and still does for text output.
assert!(has_prompt(&HistorySource::Messages(&[]), false, true, true));
assert!(!has_prompt(
&HistorySource::Messages(&[]),
false,
false,
true
));
}
#[test]
+1 -1
View File
File diff suppressed because one or more lines are too long
+11 -7
View File
@@ -58,17 +58,21 @@ which memory it is:
hashed the same way: a fixed value is one memory shared by every run, an expression such as
`flow_input.customer_id` one memory per key, and an expression that evaluates to nothing runs
stateless rather than falling back to the run's id. With memory off, `previous_messages` supplies
the history itself. The editor never seeds a placeholder for either, because a present key is the
step's choice, and a static empty value reads as unset.
the history itself. An older `auto` or `manual` memory reads neither, so the editor offers them
only once the step is moved to the current settings, which the alert's button does. The editor
never seeds a placeholder for either, because a present key is the step's choice, and a static
empty value reads as unset.
The worker reconciles them once per agent invocation, nested agent tools included, in
`resolve_history_source` (`windmill-worker/src/ai_executor.rs`):
1. Memory off, or a legacy `manual` memory: the history is `previous_messages`, else the `manual`
list, else nothing. Memory is neither read nor written, and a step `memory_id` is ignored.
2. Managed memory: a step `previous_messages` is ignored. The memory id is the step's, else the
run's, else a legacy id baked into the `auto` object. With no memory id the agent runs
stateless.
1. A legacy `auto` or `manual` memory: read as the editor that wrote it ran it. `manual` replays
its list; `auto` uses the run's memory id, else the id baked into it, else runs stateless.
Neither history input is read.
2. Managed memory: the memory id is the step's, else the run's. With no memory id the agent runs
stateless, and a step `previous_messages` is ignored.
3. Memory off: the history is `previous_messages`, else nothing. Memory is neither read nor
written, and a step `memory_id` is ignored.
Each ignored input and each stateless fallback is written to the job log.
File diff suppressed because one or more lines are too long
File diff suppressed because one or more lines are too long
@@ -3,8 +3,8 @@ import { AI_AGENT_SCHEMA, memoryPropertyFor } from './flowInfers'
import {
AGENT_FIELD_BY_KEY,
AGENT_FIELDS,
agentMemoryMode,
historyInputApplies,
keepsManagedMemory,
agentFieldIsSet,
initialVisibleAgentFields
} from './agentFormFields'
@@ -88,15 +88,18 @@ describe('initialVisibleAgentFields', () => {
describe('historyInputApplies', () => {
// Mirrors the worker: offering a step input a run would ignore misleads the author.
it('offers a memory id only with managed memory, and previous messages only without', () => {
expect(keepsManagedMemory(undefined)).toBe(false)
expect(keepsManagedMemory({ kind: 'window', context_length: 0 })).toBe(false)
expect(keepsManagedMemory({ kind: 'manual', messages: [] })).toBe(false)
expect(keepsManagedMemory({ kind: 'auto', context_length: 4, memory_id: 'x' })).toBe(true)
expect(historyInputApplies('memory_id', true)).toBe(true)
expect(historyInputApplies('previous_messages', true)).toBe(false)
expect(historyInputApplies('memory_id', false)).toBe(false)
expect(historyInputApplies('previous_messages', false)).toBe(true)
it('offers each history input in its own memory mode, and neither on an older setting', () => {
expect(agentMemoryMode(undefined)).toBe('off')
expect(agentMemoryMode({ kind: 'window', context_length: 0 })).toBe('off')
expect(agentMemoryMode({ kind: 'window', context_length: 10 })).toBe('managed')
expect(agentMemoryMode({ kind: 'manual', messages: [] })).toBe('legacy')
expect(agentMemoryMode({ kind: 'auto', context_length: 4, memory_id: 'x' })).toBe('legacy')
expect(historyInputApplies('memory_id', 'managed')).toBe(true)
expect(historyInputApplies('previous_messages', 'managed')).toBe(false)
expect(historyInputApplies('memory_id', 'off')).toBe(false)
expect(historyInputApplies('previous_messages', 'off')).toBe(true)
expect(historyInputApplies('memory_id', 'legacy')).toBe(false)
expect(historyInputApplies('previous_messages', 'legacy')).toBe(false)
expect(historyInputApplies('previous_messages', undefined)).toBe(true)
})
})
@@ -43,14 +43,24 @@ export function keepsManagedMemory(memory: any): boolean {
return (memory?.kind === 'window' || memory?.kind === 'auto') && Boolean(memory.context_length)
}
/** Whether a run reads this step input, mirroring the worker: managed memory reads only a memory id,
* memory that is off only previous messages. A setting the form cannot read yet leaves both open. */
export type AgentMemoryMode = 'legacy' | 'managed' | 'off'
/** Which shape the step's memory holds: an older `auto`/`manual` setting, or the current one. */
export function agentMemoryMode(memory: any): AgentMemoryMode {
if (memory?.kind === 'auto' || memory?.kind === 'manual') return 'legacy'
return keepsManagedMemory(memory) ? 'managed' : 'off'
}
/** Whether a run reads this step input, mirroring the worker: managed memory reads only a memory
* id, memory that is off only previous messages, and an older setting neither. A setting the form
* cannot read yet leaves both open. */
export function historyInputApplies(
key: AgentHistoryKey,
managedMemory: boolean | undefined
mode: AgentMemoryMode | undefined
): boolean {
if (managedMemory === undefined) return true
return (key === 'memory_id') === managedMemory
if (mode === undefined) return true
if (mode === 'legacy') return false
return (key === 'memory_id') === (mode === 'managed')
}
/** A memory setting in words, for a linked agent's summary. */
@@ -31,14 +31,9 @@
let legacyMessages = $derived(
memory?.kind === 'manual' ? ((memory.messages ?? []) as unknown[]) : undefined
)
// A chat run always carries the conversation's memory id, and a step's own memory id replaces the
// baked one, so in either case the baked id is never read. A blank static id reads as unset.
let stepMemoryId = $derived(
args?.memory_id?.type === 'javascript' ||
(args?.memory_id?.type === 'static' && String(args.memory_id.value ?? '').trim() !== '')
)
// A chat run always carries the conversation's memory id, so there a baked id was never read.
let legacyMemoryId = $derived(
on && memory?.kind === 'auto' && memory.memory_id && !chatInputEnabled && !stepMemoryId
on && memory?.kind === 'auto' && memory.memory_id && !chatInputEnabled
? String(memory.memory_id)
: undefined
)
@@ -58,9 +58,10 @@
AGENT_TOOLS_ROW,
AGENT_FIELD_GROUPS,
agentFieldAppliesTo,
agentMemoryMode,
historyInputApplies,
type AgentMemoryMode,
initialVisibleAgentFields,
keepsManagedMemory,
type AgentFieldGroup,
type AgentFieldSpec,
type AgentHistoryKey
@@ -179,16 +180,16 @@
let schemaProperties = $derived((schema?.properties ?? {}) as Record<string, any>)
// Whether the brain edited here, or the linked agent's, keeps managed memory. Unknown for an
// Which memory shape the brain edited here, or the linked agent's, holds. Unknown for an
// expression or a linked agent that has not loaded, which keeps previous messages addable.
let managedMemory = $derived.by((): boolean | undefined => {
let memoryMode = $derived.by((): AgentMemoryMode | undefined => {
if ('memory' in schemaProperties) {
const transform = args?.memory
return transform == undefined || transform.type === 'static'
? keepsManagedMemory(transform?.value)
? agentMemoryMode(transform?.value)
: undefined
}
return linkedMemory ? keepsManagedMemory(linkedMemory.memory) : undefined
return linkedMemory ? agentMemoryMode(linkedMemory.memory) : undefined
})
// The one-of field rewrites a value that matches none of its options, so a legacy kind the step
@@ -241,7 +242,7 @@
(args?.memory?.type === 'javascript' || args?.memory?.type === 'ai')
)
let memoryIdOffered = $derived(
(managedMemory === true || memoryIsExpression) &&
(memoryMode === 'managed' || memoryIsExpression) &&
scopedFields.some((spec) => spec.key === 'memory_id')
)
@@ -328,7 +329,7 @@
!(imageOutput && spec.textOnly) &&
// Memory id's row appears on its own when it is offered, so the menu never adds it.
spec.key !== 'memory_id' &&
!(isHistoryKey(spec.key) && !historyInputApplies(spec.key, managedMemory))
!(isHistoryKey(spec.key) && !historyInputApplies(spec.key, memoryMode))
)
}
@@ -578,9 +579,11 @@
{/if}
{:else}
{@render transformField(spec.key, spec.label, spec.tooltip, spec)}
{#if isHistoryKey(spec.key) && !historyInputApplies(spec.key, managedMemory)}
{#if isHistoryKey(spec.key) && !historyInputApplies(spec.key, memoryMode)}
<p class="mt-1 text-2xs text-hint">
Ignored while managed memory is {managedMemory ? 'on' : 'off'}.
{memoryMode === 'legacy'
? 'Not read by the older memory setting on this step.'
: `Ignored while managed memory is ${memoryMode === 'managed' ? 'on' : 'off'}.`}
</p>
{/if}
{#if spec.key === 'enabled_tools' && noToolsEnabled}
@@ -76,13 +76,13 @@ export const AI_AGENT_SCHEMA: Schema = {
memory_id: {
type: 'string',
description:
'Names the memory this step reads and writes, overriding the memory id the run was started with. Read only while managed memory is on.',
'Names the memory this step reads and writes, overriding the memory id the run was started with. Read only while managed memory is on, and not at all by an older auto or manual memory.',
showExpr: "fields.output_type !== 'image'"
},
previous_messages: {
type: 'array',
description:
'History the flow supplies, sent before the user message. Read only while managed memory is off.',
'History the flow supplies, sent before the user message. Read only while managed memory is off, and not at all by an older auto or manual memory.',
items: {
type: 'object',
properties: {
+8 -6
View File
@@ -554,8 +554,9 @@ components:
type: object
deprecated: true
description: |
Deprecated, still read: equivalent to `window`. A `memory_id` here is only used when the run
passes no memory id; set `memory_id` on the step instead.
Deprecated, still read as it was written: the run's memory id, else the `memory_id` here.
The step's own `memory_id` is not read while this kind is set; switch the kind to `window`
to use it.
properties:
kind:
type: string
@@ -589,7 +590,7 @@ components:
MemoryManual:
type: object
deprecated: true
description: Deprecated, still read. Use the step's `previous_messages` input instead.
description: Deprecated, still read as it was written. Move the step to `off` with `previous_messages` instead.
properties:
kind:
type: string
@@ -1086,14 +1087,15 @@ components:
parameter). Leave unset to use the run's memory id. A fixed value shares one memory
across every run; an expression such as `flow_input.customer_id` keeps one memory per
key. When it evaluates to an empty value the agent runs without memory. Read only
while `memory` keeps messages; ignored when it is off.
while `memory` is `window`: it is ignored when memory is off, and an older `auto` or
`manual` memory reads neither history input.
previous_messages:
allOf:
- $ref: '#/components/schemas/InputTransform'
description: |
Array of MemoryMessage. History supplied by the flow, sent between the system prompt
and the user message. Read only while `memory` is off; ignored while it keeps
messages. Replaces the list of a deprecated `manual` memory.
and the user message. Read only while `memory` is off or absent: managed memory
ignores it, and an older `auto` or `manual` memory reads neither history input.
output_schema:
allOf:
- $ref: '#/components/schemas/InputTransform'
File diff suppressed because one or more lines are too long
File diff suppressed because one or more lines are too long
File diff suppressed because one or more lines are too long