mirror of
https://github.com/aaif-goose/goose.git
synced 2026-07-03 14:10:03 +02:00
Fix agent optional field persistence
Signed-off-by: morgmart <98432065+morgmart@users.noreply.github.com>
This commit is contained in:
@@ -397,10 +397,10 @@ impl PersonaStore {
|
||||
frontmatter.avatar = avatar.clone();
|
||||
}
|
||||
if let Some(provider) = &req.provider {
|
||||
frontmatter.provider = Some(provider.clone());
|
||||
frontmatter.provider = provider.clone();
|
||||
}
|
||||
if let Some(model) = &req.model {
|
||||
frontmatter.model = Some(model.clone());
|
||||
frontmatter.model = model.clone();
|
||||
}
|
||||
|
||||
let current_system_prompt = if current_body.is_empty() {
|
||||
@@ -539,10 +539,10 @@ impl PersonaStore {
|
||||
persona.system_prompt = prompt;
|
||||
}
|
||||
if let Some(provider) = req.provider {
|
||||
persona.provider = Some(provider);
|
||||
persona.provider = provider;
|
||||
}
|
||||
if let Some(model) = req.model {
|
||||
persona.model = Some(model);
|
||||
persona.model = model;
|
||||
}
|
||||
persona.updated_at = chrono::Utc::now().to_rfc3339();
|
||||
|
||||
@@ -550,6 +550,24 @@ impl PersonaStore {
|
||||
Ok(updated)
|
||||
}
|
||||
|
||||
fn is_local_avatar_referenced(filename: &str, personas: &[Persona]) -> bool {
|
||||
personas.iter().any(|persona| {
|
||||
matches!(
|
||||
&persona.avatar,
|
||||
Some(Avatar::Local(candidate)) if candidate == filename
|
||||
)
|
||||
})
|
||||
}
|
||||
|
||||
fn delete_local_avatar_if_unreferenced(filename: &str, personas: &[Persona]) {
|
||||
if Self::is_local_avatar_referenced(filename, personas) {
|
||||
return;
|
||||
}
|
||||
|
||||
let path = Self::avatars_dir().join(filename);
|
||||
let _ = std::fs::remove_file(path);
|
||||
}
|
||||
|
||||
pub fn delete(&self, id: &str) -> Result<(), String> {
|
||||
let mut personas = self.personas.lock().unwrap();
|
||||
|
||||
@@ -559,6 +577,11 @@ impl PersonaStore {
|
||||
.cloned()
|
||||
.ok_or_else(|| format!("Persona '{}' not found", id))?;
|
||||
|
||||
let local_avatar_filename = match &persona.avatar {
|
||||
Some(Avatar::Local(filename)) => Some(filename.clone()),
|
||||
_ => None,
|
||||
};
|
||||
|
||||
if persona.is_from_disk {
|
||||
let path = Self::markdown_persona_path(id)?;
|
||||
match std::fs::remove_file(&path) {
|
||||
@@ -572,18 +595,13 @@ impl PersonaStore {
|
||||
));
|
||||
}
|
||||
}
|
||||
|
||||
personas.retain(|p| p.id != id);
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
// Clean up local avatar file if present
|
||||
if let Some(Avatar::Local(filename)) = &persona.avatar {
|
||||
let path = Self::avatars_dir().join(filename);
|
||||
let _ = std::fs::remove_file(path);
|
||||
}
|
||||
|
||||
personas.retain(|p| p.id != id);
|
||||
if let Some(filename) = local_avatar_filename {
|
||||
Self::delete_local_avatar_if_unreferenced(&filename, &personas);
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
|
||||
@@ -1,4 +1,21 @@
|
||||
use super::PersonaStore;
|
||||
use crate::types::agents::{Avatar, Persona};
|
||||
|
||||
fn make_persona(id: &str, avatar: Option<Avatar>) -> Persona {
|
||||
Persona {
|
||||
id: id.to_string(),
|
||||
display_name: id.to_string(),
|
||||
avatar,
|
||||
system_prompt: "You are helpful.".to_string(),
|
||||
provider: None,
|
||||
model: None,
|
||||
is_builtin: false,
|
||||
is_from_disk: true,
|
||||
source_path: None,
|
||||
created_at: "2026-04-01T00:00:00Z".to_string(),
|
||||
updated_at: "2026-04-01T00:00:00Z".to_string(),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn markdown_persona_path_rejects_parent_segments() {
|
||||
@@ -18,3 +35,21 @@ fn markdown_persona_path_accepts_normal_slug() {
|
||||
let file_name = path.file_name().and_then(|name| name.to_str());
|
||||
assert_eq!(file_name, Some("scout.md"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn local_avatar_reference_check_counts_remaining_personas() {
|
||||
let personas = vec![
|
||||
make_persona("one", Some(Avatar::Local("shared.png".to_string()))),
|
||||
make_persona("two", Some(Avatar::Url("https://example.test/avatar.png".to_string()))),
|
||||
make_persona("three", Some(Avatar::Local("other.png".to_string()))),
|
||||
];
|
||||
|
||||
assert!(PersonaStore::is_local_avatar_referenced(
|
||||
"shared.png",
|
||||
&personas
|
||||
));
|
||||
assert!(!PersonaStore::is_local_avatar_referenced(
|
||||
"missing.png",
|
||||
&personas
|
||||
));
|
||||
}
|
||||
|
||||
@@ -70,6 +70,16 @@ where
|
||||
}
|
||||
}
|
||||
|
||||
fn deserialize_nullable_update_field<'de, D, T>(
|
||||
deserializer: D,
|
||||
) -> Result<Option<Option<T>>, D::Error>
|
||||
where
|
||||
D: Deserializer<'de>,
|
||||
T: Deserialize<'de>,
|
||||
{
|
||||
Option::<T>::deserialize(deserializer).map(Some)
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Serialize, Deserialize)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
pub struct Persona {
|
||||
@@ -126,10 +136,18 @@ pub struct UpdatePersonaRequest {
|
||||
pub avatar: Option<Option<Avatar>>,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub system_prompt: Option<String>,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub provider: Option<String>,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub model: Option<String>,
|
||||
#[serde(
|
||||
default,
|
||||
skip_serializing_if = "Option::is_none",
|
||||
deserialize_with = "deserialize_nullable_update_field"
|
||||
)]
|
||||
pub provider: Option<Option<String>>,
|
||||
#[serde(
|
||||
default,
|
||||
skip_serializing_if = "Option::is_none",
|
||||
deserialize_with = "deserialize_nullable_update_field"
|
||||
)]
|
||||
pub model: Option<Option<String>>,
|
||||
}
|
||||
|
||||
#[allow(dead_code)]
|
||||
@@ -197,4 +215,38 @@ where
|
||||
Option::<Option<T>>::deserialize(deserializer)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::UpdatePersonaRequest;
|
||||
|
||||
#[test]
|
||||
fn update_persona_request_distinguishes_missing_and_null_provider_model() {
|
||||
let missing: UpdatePersonaRequest = serde_json::from_value(serde_json::json!({
|
||||
"displayName": "Scout"
|
||||
}))
|
||||
.unwrap();
|
||||
assert_eq!(missing.provider, None);
|
||||
assert_eq!(missing.model, None);
|
||||
|
||||
let cleared: UpdatePersonaRequest = serde_json::from_value(serde_json::json!({
|
||||
"provider": null,
|
||||
"model": null
|
||||
}))
|
||||
.unwrap();
|
||||
assert_eq!(cleared.provider, Some(None));
|
||||
assert_eq!(cleared.model, Some(None));
|
||||
|
||||
let updated: UpdatePersonaRequest = serde_json::from_value(serde_json::json!({
|
||||
"provider": "goose",
|
||||
"model": "claude-sonnet-4-20250514"
|
||||
}))
|
||||
.unwrap();
|
||||
assert_eq!(updated.provider, Some(Some("goose".to_string())));
|
||||
assert_eq!(
|
||||
updated.model,
|
||||
Some(Some("claude-sonnet-4-20250514".to_string()))
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
pub use super::builtin_personas::builtin_personas;
|
||||
|
||||
@@ -153,13 +153,21 @@ export function PersonaEditor({
|
||||
e.preventDefault();
|
||||
if (!isValid || isReadOnly) return;
|
||||
|
||||
const data: CreatePersonaRequest | UpdatePersonaRequest = {
|
||||
displayName: displayName.trim(),
|
||||
avatar: avatar ?? undefined,
|
||||
systemPrompt: systemPrompt.trim(),
|
||||
provider: provider || undefined,
|
||||
model: model.trim() || undefined,
|
||||
};
|
||||
const data: CreatePersonaRequest | UpdatePersonaRequest = isEditing
|
||||
? {
|
||||
displayName: displayName.trim(),
|
||||
avatar,
|
||||
systemPrompt: systemPrompt.trim(),
|
||||
provider: provider || null,
|
||||
model: model.trim() || null,
|
||||
}
|
||||
: {
|
||||
displayName: displayName.trim(),
|
||||
avatar: avatar ?? undefined,
|
||||
systemPrompt: systemPrompt.trim(),
|
||||
provider: provider || undefined,
|
||||
model: model.trim() || undefined,
|
||||
};
|
||||
onSave(data);
|
||||
},
|
||||
[
|
||||
@@ -171,6 +179,7 @@ export function PersonaEditor({
|
||||
provider,
|
||||
model,
|
||||
onSave,
|
||||
isEditing,
|
||||
],
|
||||
);
|
||||
|
||||
|
||||
@@ -53,8 +53,8 @@ export interface UpdatePersonaRequest {
|
||||
displayName?: string;
|
||||
avatar?: Avatar | null;
|
||||
systemPrompt?: string;
|
||||
provider?: ProviderType;
|
||||
model?: string;
|
||||
provider?: ProviderType | null;
|
||||
model?: string | null;
|
||||
}
|
||||
|
||||
// Agent types
|
||||
|
||||
Reference in New Issue
Block a user