mirror of
https://github.com/block/buzz.git
synced 2026-11-02 15:31:27 +00:00
fix(desktop): refresh persona env vars on respawn — record.env_vars is overrides-only (#1640)
Signed-off-by: Wes <wesbillman@users.noreply.github.com> Co-authored-by: Pinky <44b8e82baa6e0e254e0208d68f335c283c94e7b78dd1fa10d5a49d3f13dd0435@sprout-oss.stage.blox.sqprod.co>
This commit is contained in:
@@ -255,19 +255,20 @@ async fn start_local_agent_with_preflight(
|
||||
return Err(format!("agent {pubkey} is no longer a local agent"));
|
||||
}
|
||||
// Re-snapshot the persona onto the record at every spawn so the agent always
|
||||
// starts with the current persona config (system_prompt, model, provider,
|
||||
// env_vars). This clears the "out of date" drift badge without requiring a
|
||||
// delete+recreate. Agent-level env_vars overrides still win (persona_snapshot
|
||||
// layers persona env under agent overrides). When the persona leaves model or
|
||||
// provider blank, the agent record's own configured values are preserved so a
|
||||
// user-set model/provider is never clobbered by an unconfigured persona.
|
||||
// starts with the current persona config (system_prompt, model, provider).
|
||||
// This clears the "out of date" drift badge without requiring a
|
||||
// delete+recreate. `record.env_vars` is NOT rewritten: it holds agent-level
|
||||
// overrides only, and spawn merges the live persona env underneath at read
|
||||
// time — so persona env edits refresh here too. When the persona leaves
|
||||
// model or provider blank, the agent record's own configured values are
|
||||
// preserved so a user-set model/provider is never clobbered by an
|
||||
// unconfigured persona.
|
||||
if let Some(persona_id) = record.persona_id.clone() {
|
||||
let personas = load_personas(app).unwrap_or_default();
|
||||
if let Some(persona) = personas.iter().find(|p| p.id == persona_id) {
|
||||
let snapshot =
|
||||
crate::managed_agents::persona_events::persona_snapshot_with_agent_config_fallback(
|
||||
persona,
|
||||
&record.env_vars,
|
||||
record.model.as_deref(), // fallback: record.model
|
||||
record.provider.as_deref(), // fallback: record.provider
|
||||
);
|
||||
@@ -276,7 +277,14 @@ async fn start_local_agent_with_preflight(
|
||||
}
|
||||
record.model = snapshot.model;
|
||||
record.provider = snapshot.provider;
|
||||
record.env_vars = snapshot.env_vars;
|
||||
// Self-heal records written before env refresh: persona env used to
|
||||
// be baked into `record.env_vars` at create/spawn, turning inherited
|
||||
// values into pseudo-overrides that shadow later persona edits. An
|
||||
// override equal to the persona's current value is indistinguishable
|
||||
// from inheritance, so drop it and let the live merge supply it.
|
||||
record
|
||||
.env_vars
|
||||
.retain(|k, v| persona.env_vars.get(k) != Some(v));
|
||||
record.persona_source_version = Some(snapshot.source_version);
|
||||
record.updated_at = crate::util::now_iso();
|
||||
}
|
||||
@@ -296,8 +304,7 @@ async fn start_local_agent_with_preflight(
|
||||
|
||||
/// Build the standard agent JSON payload for provider deploy calls.
|
||||
///
|
||||
/// Unlike local spawn (which uses only pinned `record.env_vars` for
|
||||
/// determinism), provider deploy re-reads live persona env vars and
|
||||
/// Like local spawn, provider deploy re-reads live persona env vars and
|
||||
/// structured model/provider so remote agents receive current credentials
|
||||
/// and the same authoritative values that local spawn derives from
|
||||
/// `runtime_metadata_env_vars`. The only field still pinned is
|
||||
@@ -322,11 +329,10 @@ fn build_deploy_payload(
|
||||
return Err(err);
|
||||
}
|
||||
|
||||
// Merge persona env_vars + agent env_vars for provider deploy. Provider
|
||||
// deploy re-reads live persona env vars so remote agents receive current
|
||||
// credentials; local spawn uses only pinned record.env_vars for determinism
|
||||
// across restarts. Without this, provider-backed agents wouldn't receive
|
||||
// credentials saved on the persona or the agent itself.
|
||||
// Merge persona env_vars + agent env_vars for provider deploy — the same
|
||||
// live-persona-under-overrides semantics as local spawn. Without this,
|
||||
// provider-backed agents wouldn't receive credentials saved on the persona
|
||||
// or the agent itself.
|
||||
let persona_env =
|
||||
crate::managed_agents::resolve_persona_env(app, record.persona_id.as_deref())?;
|
||||
let merged_env = crate::managed_agents::merged_user_env(&persona_env, &record.env_vars);
|
||||
@@ -733,20 +739,16 @@ pub async fn create_managed_agent(
|
||||
// and deploy read these snapshotted fields, never the live persona, so
|
||||
// the agent stays on the config it was created with across restarts;
|
||||
// delete+respawn re-runs create and rewrites the snapshot. env_vars are
|
||||
// pinned too — without that, persona credential edits would leak into a
|
||||
// running agent on restart. Agent-level env overrides (input.env_vars)
|
||||
// layer on top, matching spawn precedence (persona env < agent env).
|
||||
// NOT pinned: `record.env_vars` holds agent-level overrides only
|
||||
// (input.env_vars), and the live persona env is merged underneath at
|
||||
// read time (spawn / readiness / deploy) so persona credential edits
|
||||
// refresh on the next spawn like prompt/model/provider already do.
|
||||
let persona_snapshot = requested_persona_id.as_deref().and_then(|pid| {
|
||||
load_personas(&app)
|
||||
.ok()?
|
||||
.into_iter()
|
||||
.find(|persona| persona.id == pid)
|
||||
.map(|persona| {
|
||||
crate::managed_agents::persona_events::persona_snapshot(
|
||||
&persona,
|
||||
&input.env_vars,
|
||||
)
|
||||
})
|
||||
.map(|persona| crate::managed_agents::persona_events::persona_snapshot(&persona))
|
||||
});
|
||||
let snapshot_prompt = persona_snapshot
|
||||
.as_ref()
|
||||
@@ -754,9 +756,6 @@ pub async fn create_managed_agent(
|
||||
let snapshot_model = persona_snapshot.as_ref().and_then(|s| s.model.clone());
|
||||
let snapshot_provider = persona_snapshot.as_ref().and_then(|s| s.provider.clone());
|
||||
let snapshot_source_version = persona_snapshot.as_ref().map(|s| s.source_version.clone());
|
||||
let snapshot_env_vars = persona_snapshot
|
||||
.map(|s| s.env_vars)
|
||||
.unwrap_or_else(|| input.env_vars.clone());
|
||||
|
||||
let record = crate::managed_agents::ManagedAgentRecord {
|
||||
pubkey: pubkey.clone(),
|
||||
@@ -834,7 +833,7 @@ pub async fn create_managed_agent(
|
||||
// NOT the display_name — ACP's resolve_persona_by_name() matches slugs.
|
||||
persona_team_dir: pack_metadata.as_ref().map(|(path, _)| path.clone()),
|
||||
persona_name_in_team: pack_metadata.as_ref().map(|(_, name)| name.clone()),
|
||||
env_vars: snapshot_env_vars,
|
||||
env_vars: input.env_vars.clone(),
|
||||
created_at: now_iso(),
|
||||
updated_at: now_iso(),
|
||||
last_started_at: None,
|
||||
|
||||
@@ -300,12 +300,26 @@ pub(crate) fn merged_user_env(
|
||||
merged
|
||||
}
|
||||
|
||||
/// Resolve live env_vars for a linked persona.
|
||||
/// Look up the live env map of `persona_id` within an already-loaded persona
|
||||
/// slice. Returns an empty map for standalone agents (`None`) and for links
|
||||
/// to personas that no longer exist (an orphaned agent spawns from its own
|
||||
/// overrides alone — same fallback the prompt/model resolution uses).
|
||||
pub(crate) fn live_persona_env(
|
||||
personas: &[super::types::PersonaRecord],
|
||||
persona_id: Option<&str>,
|
||||
) -> BTreeMap<String, String> {
|
||||
persona_id
|
||||
.and_then(|pid| personas.iter().find(|p| p.id == pid))
|
||||
.map(|p| p.env_vars.clone())
|
||||
.unwrap_or_default()
|
||||
}
|
||||
|
||||
/// Resolve live env_vars for a linked persona, loading personas from disk.
|
||||
///
|
||||
/// Returns the persona's `env_vars` map if a persona_id is provided and found;
|
||||
/// returns an empty map if no persona is linked or the persona is not found.
|
||||
/// Used by the provider deploy path so remote agents receive current credentials;
|
||||
/// local spawn uses only the pinned `record.env_vars` for determinism.
|
||||
/// returns an empty map if no persona is linked. Errors if the linked persona
|
||||
/// is missing. Used by the provider deploy path, which has no pre-loaded
|
||||
/// persona slice.
|
||||
pub(crate) fn resolve_persona_env(
|
||||
app: &tauri::AppHandle,
|
||||
persona_id: Option<&str>,
|
||||
|
||||
@@ -299,9 +299,6 @@ pub struct PersonaSnapshot {
|
||||
pub system_prompt: Option<String>,
|
||||
pub model: Option<String>,
|
||||
pub provider: Option<String>,
|
||||
/// Persona env layered under the agent's own overrides (agent wins). This
|
||||
/// is the complete env map the agent spawns with — no live persona lookup.
|
||||
pub env_vars: BTreeMap<String, String>,
|
||||
/// `persona_content_hash` of the persona at snapshot time; the drift basis.
|
||||
pub source_version: String,
|
||||
}
|
||||
@@ -326,23 +323,15 @@ pub fn persona_field_with_record_fallback(
|
||||
|
||||
/// Build the pinned snapshot for an agent created from `persona`.
|
||||
///
|
||||
/// `agent_env_overrides` are the agent's own env vars (persona-independent);
|
||||
/// they win over persona env on key collision, matching spawn-time precedence
|
||||
/// (persona env < agent env). The persona's `system_prompt` is always present,
|
||||
/// so it is wrapped in `Some`.
|
||||
pub fn persona_snapshot(
|
||||
persona: &PersonaRecord,
|
||||
agent_env_overrides: &BTreeMap<String, String>,
|
||||
) -> PersonaSnapshot {
|
||||
let mut env_vars = persona.env_vars.clone();
|
||||
for (key, value) in agent_env_overrides {
|
||||
env_vars.insert(key.clone(), value.clone());
|
||||
}
|
||||
/// The persona's `system_prompt` is always present, so it is wrapped in
|
||||
/// `Some`. Env vars are deliberately absent: `record.env_vars` holds agent
|
||||
/// overrides only, and the live persona env is merged underneath at read
|
||||
/// time (spawn / readiness / deploy) — never snapshotted.
|
||||
pub fn persona_snapshot(persona: &PersonaRecord) -> PersonaSnapshot {
|
||||
PersonaSnapshot {
|
||||
system_prompt: Some(persona.system_prompt.clone()),
|
||||
model: persona.model.clone(),
|
||||
provider: persona.provider.clone(),
|
||||
env_vars,
|
||||
source_version: persona_content_hash(&persona_event_content(persona)),
|
||||
}
|
||||
}
|
||||
@@ -360,21 +349,19 @@ pub fn persona_snapshot(
|
||||
/// `source_version` is always updated to the current persona content hash so the
|
||||
/// drift badge clears correctly even when model/provider are not touched.
|
||||
///
|
||||
/// Env-var layering is unchanged: persona env < agent env (agent wins on collision).
|
||||
/// A persona with an empty env map does not wipe the agent's env vars — the agent's
|
||||
/// own overrides are merged on top and always win.
|
||||
/// Env vars are not part of the snapshot: `record.env_vars` (agent overrides)
|
||||
/// is left untouched and the live persona env is merged underneath at read time.
|
||||
///
|
||||
/// The two fields (`model`, `provider`) are independent: a persona that sets only
|
||||
/// `model` wins on `model` while the agent's `provider` is preserved, and vice versa.
|
||||
pub fn persona_snapshot_with_agent_config_fallback(
|
||||
persona: &PersonaRecord,
|
||||
agent_env_overrides: &BTreeMap<String, String>,
|
||||
current_agent_model: Option<&str>,
|
||||
current_agent_provider: Option<&str>,
|
||||
) -> PersonaSnapshot {
|
||||
// Delegate env-merge, system_prompt, and source_version to persona_snapshot
|
||||
// so future PersonaSnapshot field additions stay automatically consistent.
|
||||
let base = persona_snapshot(persona, agent_env_overrides);
|
||||
// Delegate system_prompt and source_version to persona_snapshot so future
|
||||
// PersonaSnapshot field additions stay automatically consistent.
|
||||
let base = persona_snapshot(persona);
|
||||
|
||||
// Apply the shared precedence rule: persona wins when non-blank, else
|
||||
// the agent record's value is preserved so a configured agent stays configured.
|
||||
@@ -741,12 +728,8 @@ mod tests {
|
||||
let persona = blank_model_persona();
|
||||
let expected_version = persona_content_hash(&persona_event_content(&persona));
|
||||
|
||||
let snapshot = persona_snapshot_with_agent_config_fallback(
|
||||
&persona,
|
||||
&BTreeMap::new(),
|
||||
Some("gpt-4o"),
|
||||
Some("openai"),
|
||||
);
|
||||
let snapshot =
|
||||
persona_snapshot_with_agent_config_fallback(&persona, Some("gpt-4o"), Some("openai"));
|
||||
|
||||
assert_eq!(
|
||||
snapshot.model.as_deref(),
|
||||
@@ -771,7 +754,6 @@ mod tests {
|
||||
|
||||
let snapshot = persona_snapshot_with_agent_config_fallback(
|
||||
&persona,
|
||||
&BTreeMap::new(),
|
||||
Some("gpt-4o"), // agent had a different model
|
||||
Some("openai"), // agent had a different provider
|
||||
);
|
||||
@@ -795,9 +777,7 @@ mod tests {
|
||||
let persona = blank_model_persona();
|
||||
|
||||
let snapshot = persona_snapshot_with_agent_config_fallback(
|
||||
&persona,
|
||||
&BTreeMap::new(),
|
||||
None, // agent also has no model
|
||||
&persona, None, // agent also has no model
|
||||
None, // agent also has no provider
|
||||
);
|
||||
|
||||
@@ -821,7 +801,6 @@ mod tests {
|
||||
|
||||
let snapshot = persona_snapshot_with_agent_config_fallback(
|
||||
&persona,
|
||||
&BTreeMap::new(),
|
||||
Some("claude-opus-4"),
|
||||
Some("anthropic"),
|
||||
);
|
||||
@@ -848,7 +827,6 @@ mod tests {
|
||||
|
||||
let snapshot = persona_snapshot_with_agent_config_fallback(
|
||||
&persona,
|
||||
&BTreeMap::new(),
|
||||
Some("gpt-4o"), // record model (should be overridden by persona)
|
||||
Some("openai"), // record provider (should be preserved)
|
||||
);
|
||||
@@ -874,7 +852,6 @@ mod tests {
|
||||
|
||||
let snapshot = persona_snapshot_with_agent_config_fallback(
|
||||
&persona,
|
||||
&BTreeMap::new(),
|
||||
Some("gpt-4o"), // record model (should be preserved)
|
||||
Some("openai"), // record provider (should be overridden by persona)
|
||||
);
|
||||
@@ -890,39 +867,4 @@ mod tests {
|
||||
"persona provider must win when persona has a value"
|
||||
);
|
||||
}
|
||||
|
||||
/// Env-var layering: persona env < agent env — agent overrides always win
|
||||
/// on key collision and a persona with an empty env map does not wipe the
|
||||
/// agent's env vars.
|
||||
#[test]
|
||||
fn fallback_agent_env_wins_over_persona_env() {
|
||||
let mut persona = blank_model_persona();
|
||||
persona.env_vars = BTreeMap::from([
|
||||
("SHARED_KEY".to_string(), "persona-value".to_string()),
|
||||
("PERSONA_ONLY".to_string(), "from-persona".to_string()),
|
||||
]);
|
||||
let agent_env = BTreeMap::from([
|
||||
("SHARED_KEY".to_string(), "agent-override".to_string()),
|
||||
("AGENT_ONLY".to_string(), "from-agent".to_string()),
|
||||
]);
|
||||
|
||||
let snapshot =
|
||||
persona_snapshot_with_agent_config_fallback(&persona, &agent_env, None, None);
|
||||
|
||||
assert_eq!(
|
||||
snapshot.env_vars.get("SHARED_KEY").map(String::as_str),
|
||||
Some("agent-override"),
|
||||
"agent env must win on key collision"
|
||||
);
|
||||
assert_eq!(
|
||||
snapshot.env_vars.get("PERSONA_ONLY").map(String::as_str),
|
||||
Some("from-persona"),
|
||||
"persona-only key must be present"
|
||||
);
|
||||
assert_eq!(
|
||||
snapshot.env_vars.get("AGENT_ONLY").map(String::as_str),
|
||||
Some("from-agent"),
|
||||
"agent-only key must be present"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -29,8 +29,9 @@
|
||||
//! 2. Runtime metadata env vars (`runtime_metadata_env_vars`) — provider /
|
||||
//! model env keys derived from the record's `model`/`provider` fields and
|
||||
//! the runtime's `model_env_var`/`provider_env_var`.
|
||||
//! 3. Merged user env (`merged_user_env`) — the record's `env_vars` after
|
||||
//! reserved-key and malformed-key filtering. Last-wins on collision.
|
||||
//! 3. Merged user env (`merged_user_env`) — live persona env under the
|
||||
//! record's `env_vars` overrides, after reserved-key and malformed-key
|
||||
//! filtering. Last-wins on collision.
|
||||
//!
|
||||
//! The config-file tier (Goose `~/.config/goose/config.yaml`) is tracked
|
||||
//! separately because it is not part of the process env — the harness reads
|
||||
@@ -106,9 +107,14 @@ pub(crate) fn resolve_effective_agent_env(
|
||||
}
|
||||
}
|
||||
|
||||
// Layer 3: merged user env (agent env_vars after reserved/malformed-key
|
||||
// filtering; last-wins on collision).
|
||||
let user_env = merged_user_env(&BTreeMap::new(), &record.env_vars);
|
||||
// Layer 3: merged user env — live persona env under the record's own
|
||||
// overrides (last-wins), after reserved/malformed-key filtering. Reading
|
||||
// the persona live is what makes persona credential edits refresh on the
|
||||
// next spawn instead of being frozen into the record.
|
||||
let user_env = merged_user_env(
|
||||
&super::env_vars::live_persona_env(personas, record.persona_id.as_deref()),
|
||||
&record.env_vars,
|
||||
);
|
||||
env.extend(user_env);
|
||||
|
||||
EffectiveAgentEnv {
|
||||
|
||||
@@ -58,13 +58,12 @@ pub fn backfill_persona_snapshots(app: &tauri::AppHandle) -> Result<(), String>
|
||||
);
|
||||
continue;
|
||||
};
|
||||
// Layer the agent's own env overrides over persona env, matching
|
||||
// create-time precedence (persona env < agent env). When the persona
|
||||
// leaves model/provider blank, the record's own configured values are
|
||||
// preserved — a blank persona must not clobber a user-configured agent.
|
||||
// Layer precedence at read time: persona env < agent env. When the
|
||||
// persona leaves model/provider blank, the record's own configured
|
||||
// values are preserved — a blank persona must not clobber a
|
||||
// user-configured agent.
|
||||
let snapshot = super::persona_events::persona_snapshot_with_agent_config_fallback(
|
||||
persona,
|
||||
&record.env_vars,
|
||||
record.model.as_deref(), // fallback: record.model
|
||||
record.provider.as_deref(), // fallback: record.provider
|
||||
);
|
||||
@@ -73,7 +72,11 @@ pub fn backfill_persona_snapshots(app: &tauri::AppHandle) -> Result<(), String>
|
||||
}
|
||||
record.model = snapshot.model;
|
||||
record.provider = snapshot.provider;
|
||||
record.env_vars = snapshot.env_vars;
|
||||
// env_vars stay overrides-only; see the create-path comment. Self-heal
|
||||
// pre-refresh records that baked persona env in as pseudo-overrides.
|
||||
record
|
||||
.env_vars
|
||||
.retain(|k, v| persona.env_vars.get(k) != Some(v));
|
||||
record.persona_source_version = Some(snapshot.source_version);
|
||||
record.updated_at = util::now_iso();
|
||||
changed = true;
|
||||
@@ -194,7 +197,6 @@ pub async fn restore_managed_agents_on_launch(
|
||||
};
|
||||
let snapshot = super::persona_events::persona_snapshot_with_agent_config_fallback(
|
||||
persona,
|
||||
&record.env_vars,
|
||||
record.model.as_deref(), // fallback: record.model
|
||||
record.provider.as_deref(), // fallback: record.provider
|
||||
);
|
||||
@@ -203,7 +205,12 @@ pub async fn restore_managed_agents_on_launch(
|
||||
}
|
||||
record.model = snapshot.model;
|
||||
record.provider = snapshot.provider;
|
||||
record.env_vars = snapshot.env_vars;
|
||||
// env_vars stay overrides-only; see the create-path comment.
|
||||
// Self-heal pre-refresh records that baked persona env in as
|
||||
// pseudo-overrides.
|
||||
record
|
||||
.env_vars
|
||||
.retain(|k, v| persona.env_vars.get(k) != Some(v));
|
||||
record.persona_source_version = Some(snapshot.source_version);
|
||||
record.updated_at = util::now_iso();
|
||||
changed = true;
|
||||
|
||||
@@ -1812,24 +1812,24 @@ pub fn spawn_agent_child(
|
||||
);
|
||||
}
|
||||
|
||||
// ── User env vars: the record snapshot ─────────────────────────────
|
||||
// ── User env vars: live persona env under agent overrides ──────────
|
||||
//
|
||||
// The record's `env_vars` is the complete, pinned env map — persona env
|
||||
// (snapshotted at create) already merged under the agent's own overrides.
|
||||
// We read it directly and never look up the live persona, so credential
|
||||
// edits on the persona reach the agent only via delete+respawn (which
|
||||
// rewrites the snapshot), not on a plain restart. `merged_user_env` with an
|
||||
// empty persona map still applies the reserved-key / malformed-key / NUL
|
||||
// filtering as defense-in-depth for older on-disk records.
|
||||
// The record's `env_vars` holds agent-level overrides only. The linked
|
||||
// persona's env is read live and merged underneath (agent wins on
|
||||
// collision), so persona credential edits reach the agent on the next
|
||||
// spawn — same refresh semantics as prompt/model/provider above and the
|
||||
// provider deploy path. `merged_user_env` also applies the reserved-key /
|
||||
// malformed-key / NUL filtering.
|
||||
//
|
||||
// These writes go LAST so user-provided values win over every Buzz-set env
|
||||
// above — EXCEPT reserved keys (BUZZ_PRIVATE_KEY, NOSTR_PRIVATE_KEY,
|
||||
// BUZZ_AUTH_TAG, BUZZ_API_TOKEN, BUZZ_ACP_PRIVATE_KEY, BUZZ_ACP_API_TOKEN),
|
||||
// which `merged_user_env` strips. Those carry Buzz's identity and must
|
||||
// never be GUI-overridable.
|
||||
for (key, value) in
|
||||
super::env_vars::merged_user_env(&std::collections::BTreeMap::new(), &record.env_vars)
|
||||
{
|
||||
for (key, value) in super::env_vars::merged_user_env(
|
||||
&super::env_vars::live_persona_env(&personas, record.persona_id.as_deref()),
|
||||
&record.env_vars,
|
||||
) {
|
||||
command.env(key, value);
|
||||
}
|
||||
|
||||
|
||||
@@ -290,29 +290,29 @@ fn persona_with_provider(
|
||||
}
|
||||
}
|
||||
|
||||
// ── persona pin/refresh acceptance (Phase 4) ────────────────────────────
|
||||
// ── persona env refresh acceptance ──────────────────────────────────────
|
||||
//
|
||||
// The full lifecycle Will specified: create from P0, edit P0→P1 (env_vars
|
||||
// included), restart stays pinned to P0, delete+respawn refreshes to P1. We
|
||||
// exercise it at the pure seams that `create_managed_agent` and
|
||||
// `build_managed_agent_summary` are built from: `persona_snapshot` (what create
|
||||
// writes onto the record) and `persona_drift_state` (the Agents-menu badge).
|
||||
// The env_var assertions are load-bearing — the credential pin is the field
|
||||
// that would silently leak on restart if spawn re-read the live persona.
|
||||
// The refresh lifecycle Wes decided: `record.env_vars` holds agent-level
|
||||
// overrides only, the live persona env is merged underneath at read time
|
||||
// (spawn / readiness / deploy), so persona env edits — like prompt/model/
|
||||
// provider — reach the agent on the next spawn without delete+recreate.
|
||||
// The merge assertions are load-bearing: they witness the credential refresh
|
||||
// that the old create-time env baking silently blocked.
|
||||
|
||||
use crate::managed_agents::env_vars::{live_persona_env, merged_user_env};
|
||||
use crate::managed_agents::persona_events::persona_snapshot;
|
||||
use std::collections::BTreeMap;
|
||||
|
||||
/// Apply a persona snapshot onto a record, mirroring `create_managed_agent`:
|
||||
/// snapshotted prompt/model/provider/env_vars/source_version are pinned, with
|
||||
/// the system_prompt unwrapped (the persona always carries one).
|
||||
/// snapshotted prompt/model/provider/source_version are pinned, with the
|
||||
/// system_prompt unwrapped (the persona always carries one). `env_vars` is
|
||||
/// deliberately NOT touched — it stays agent overrides only.
|
||||
fn pin_persona(record: &mut ManagedAgentRecord, persona: &crate::managed_agents::PersonaRecord) {
|
||||
let snapshot = persona_snapshot(persona, &record.env_vars);
|
||||
let snapshot = persona_snapshot(persona);
|
||||
record.persona_id = Some(persona.id.clone());
|
||||
record.system_prompt = snapshot.system_prompt;
|
||||
record.model = snapshot.model;
|
||||
record.provider = snapshot.provider;
|
||||
record.env_vars = snapshot.env_vars;
|
||||
record.persona_source_version = Some(snapshot.source_version);
|
||||
}
|
||||
|
||||
@@ -325,45 +325,63 @@ fn persona_v(id: &str, prompt: &str, env: &[(&str, &str)]) -> crate::managed_age
|
||||
p
|
||||
}
|
||||
|
||||
/// The spawn-time user env for `record` under `personas` — the same
|
||||
/// live-persona-under-overrides merge `spawn_agent_child` and
|
||||
/// `resolve_effective_agent_env` perform.
|
||||
fn spawn_user_env(
|
||||
record: &ManagedAgentRecord,
|
||||
personas: &[crate::managed_agents::PersonaRecord],
|
||||
) -> BTreeMap<String, String> {
|
||||
merged_user_env(
|
||||
&live_persona_env(personas, record.persona_id.as_deref()),
|
||||
&record.env_vars,
|
||||
)
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn create_pins_full_persona_snapshot_including_env_vars() {
|
||||
fn create_keeps_env_vars_as_overrides_only() {
|
||||
let p0 = persona_v("p", "prompt-v0", &[("ANTHROPIC_API_KEY", "key-v0")]);
|
||||
let mut record = fixture(RespondTo::Anyone, vec![], Some("tag".into()));
|
||||
pin_persona(&mut record, &p0);
|
||||
|
||||
assert_eq!(record.system_prompt.as_deref(), Some("prompt-v0"));
|
||||
assert_eq!(record.provider.as_deref(), Some("anthropic"));
|
||||
assert!(
|
||||
record.env_vars.is_empty(),
|
||||
"create must NOT bake persona env into the record — overrides only"
|
||||
);
|
||||
// The credential still reaches the spawned process via the live merge.
|
||||
assert_eq!(
|
||||
record.env_vars.get("ANTHROPIC_API_KEY").map(String::as_str),
|
||||
spawn_user_env(&record, std::slice::from_ref(&p0))
|
||||
.get("ANTHROPIC_API_KEY")
|
||||
.map(String::as_str),
|
||||
Some("key-v0"),
|
||||
"create must pin persona env_vars — the credential pin"
|
||||
"spawn env must carry the persona credential via the live merge"
|
||||
);
|
||||
assert!(record.persona_source_version.is_some());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn restart_after_persona_edit_stays_pinned_to_old_snapshot() {
|
||||
fn restart_after_persona_edit_refreshes_credential() {
|
||||
// Create from P0.
|
||||
let p0 = persona_v("p", "prompt-v0", &[("ANTHROPIC_API_KEY", "key-v0")]);
|
||||
let mut record = fixture(RespondTo::Anyone, vec![], Some("tag".into()));
|
||||
pin_persona(&mut record, &p0);
|
||||
|
||||
// Edit the persona to P1 (prompt + credential change). Restart reuses the
|
||||
// SAME record — nothing rewrites the snapshot — so spawn reads P0 fields.
|
||||
// SAME record, but spawn merges the live persona env — so the edited
|
||||
// credential reaches the next spawn without delete+recreate.
|
||||
let p1 = persona_v("p", "prompt-v1", &[("ANTHROPIC_API_KEY", "key-v1")]);
|
||||
|
||||
assert_eq!(
|
||||
record.system_prompt.as_deref(),
|
||||
Some("prompt-v0"),
|
||||
"restart must keep the pinned prompt"
|
||||
);
|
||||
assert_eq!(
|
||||
record.env_vars.get("ANTHROPIC_API_KEY").map(String::as_str),
|
||||
Some("key-v0"),
|
||||
"restart must NOT pick up the edited credential — the CRITICAL"
|
||||
spawn_user_env(&record, std::slice::from_ref(&p1))
|
||||
.get("ANTHROPIC_API_KEY")
|
||||
.map(String::as_str),
|
||||
Some("key-v1"),
|
||||
"restart must pick up the edited credential — the refresh Wes asked for"
|
||||
);
|
||||
|
||||
// The badge flips: the record's snapshot now lags the edited persona.
|
||||
// The badge flips: the record's snapshot lags the edited persona until the
|
||||
// spawn-path re-snapshot runs.
|
||||
let (out_of_date, orphaned) = super::persona_drift_state(&record, std::slice::from_ref(&p1));
|
||||
assert!(
|
||||
out_of_date,
|
||||
@@ -373,52 +391,78 @@ fn restart_after_persona_edit_stays_pinned_to_old_snapshot() {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn respawn_after_persona_edit_refreshes_to_new_snapshot() {
|
||||
let p0 = persona_v("p", "prompt-v0", &[("ANTHROPIC_API_KEY", "key-v0")]);
|
||||
let p1 = persona_v("p", "prompt-v1", &[("ANTHROPIC_API_KEY", "key-v1")]);
|
||||
|
||||
// Respawn = delete the old record + create a fresh one. create re-snapshots
|
||||
// the now-current persona (P1).
|
||||
let mut respawned = fixture(RespondTo::Anyone, vec![], Some("tag".into()));
|
||||
pin_persona(&mut respawned, &p1);
|
||||
|
||||
assert_eq!(respawned.system_prompt.as_deref(), Some("prompt-v1"));
|
||||
assert_eq!(
|
||||
respawned
|
||||
.env_vars
|
||||
.get("ANTHROPIC_API_KEY")
|
||||
.map(String::as_str),
|
||||
Some("key-v1"),
|
||||
"respawn must refresh the credential to the edited persona"
|
||||
);
|
||||
|
||||
// Now pinned to current persona → not out of date.
|
||||
let (out_of_date, orphaned) = super::persona_drift_state(&respawned, std::slice::from_ref(&p1));
|
||||
assert!(!out_of_date, "respawn pins to current persona — no drift");
|
||||
assert!(!orphaned);
|
||||
|
||||
// Sanity: P0 differs from P1, so a record still pinned to P0 would drift.
|
||||
let mut stale = fixture(RespondTo::Anyone, vec![], Some("tag".into()));
|
||||
pin_persona(&mut stale, &p0);
|
||||
assert!(super::persona_drift_state(&stale, std::slice::from_ref(&p1)).0);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn agent_env_overrides_win_over_persona_env_in_snapshot() {
|
||||
// Agent-level env_vars (input.env_vars) layer over persona env on collision,
|
||||
// matching spawn precedence (persona env < agent env).
|
||||
fn agent_env_overrides_win_over_persona_env_at_spawn() {
|
||||
// Agent-level env_vars layer over persona env on collision at read time
|
||||
// (persona env < agent env).
|
||||
let persona = persona_v("p", "prompt", &[("ANTHROPIC_API_KEY", "persona-key")]);
|
||||
let mut record = fixture(RespondTo::Anyone, vec![], Some("tag".into()));
|
||||
record.env_vars = BTreeMap::from([("ANTHROPIC_API_KEY".to_string(), "agent-key".to_string())]);
|
||||
pin_persona(&mut record, &persona);
|
||||
|
||||
assert_eq!(
|
||||
record.env_vars.get("ANTHROPIC_API_KEY").map(String::as_str),
|
||||
spawn_user_env(&record, std::slice::from_ref(&persona))
|
||||
.get("ANTHROPIC_API_KEY")
|
||||
.map(String::as_str),
|
||||
Some("agent-key"),
|
||||
"agent override must win over persona env"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn orphaned_agent_spawns_from_its_own_overrides() {
|
||||
// Persona deleted: the live merge degrades to the record's own overrides.
|
||||
let persona = persona_v("p", "prompt", &[("ANTHROPIC_API_KEY", "persona-key")]);
|
||||
let mut record = fixture(RespondTo::Anyone, vec![], Some("tag".into()));
|
||||
record.env_vars = BTreeMap::from([("EXTRA".to_string(), "agent-value".to_string())]);
|
||||
pin_persona(&mut record, &persona);
|
||||
|
||||
let env = spawn_user_env(&record, &[]);
|
||||
assert_eq!(env.get("EXTRA").map(String::as_str), Some("agent-value"));
|
||||
assert_eq!(
|
||||
env.get("ANTHROPIC_API_KEY"),
|
||||
None,
|
||||
"a deleted persona's env must not linger"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn self_heal_drops_overrides_equal_to_persona_value() {
|
||||
// Pre-refresh records baked persona env in as pseudo-overrides. The
|
||||
// spawn-path retain() treats an override equal to the persona's current
|
||||
// value as inherited, so later persona edits refresh it.
|
||||
let p0 = persona_v("p", "prompt", &[("ANTHROPIC_API_KEY", "key-v0")]);
|
||||
let mut record = fixture(RespondTo::Anyone, vec![], Some("tag".into()));
|
||||
record.env_vars = BTreeMap::from([
|
||||
("ANTHROPIC_API_KEY".to_string(), "key-v0".to_string()), // baked-in persona value
|
||||
("GENUINE".to_string(), "override".to_string()), // real override
|
||||
]);
|
||||
pin_persona(&mut record, &p0);
|
||||
|
||||
// Mirror the start/restore self-heal.
|
||||
record.env_vars.retain(|k, v| p0.env_vars.get(k) != Some(v));
|
||||
|
||||
assert_eq!(
|
||||
record.env_vars.get("ANTHROPIC_API_KEY"),
|
||||
None,
|
||||
"baked-in persona value must be reclassified as inherited"
|
||||
);
|
||||
assert_eq!(
|
||||
record.env_vars.get("GENUINE").map(String::as_str),
|
||||
Some("override"),
|
||||
"genuine overrides must survive the self-heal"
|
||||
);
|
||||
|
||||
// After the persona edits the key, the healed record refreshes.
|
||||
let p1 = persona_v("p", "prompt", &[("ANTHROPIC_API_KEY", "key-v1")]);
|
||||
assert_eq!(
|
||||
spawn_user_env(&record, std::slice::from_ref(&p1))
|
||||
.get("ANTHROPIC_API_KEY")
|
||||
.map(String::as_str),
|
||||
Some("key-v1"),
|
||||
"healed record must inherit the edited persona credential"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn deleted_persona_is_orphaned_not_out_of_date() {
|
||||
let p0 = persona_v("p", "prompt-v0", &[("KEY", "v0")]);
|
||||
|
||||
@@ -50,7 +50,6 @@ pub(crate) fn spawn_config_hash(
|
||||
if let Some(persona) = personas.iter().find(|p| p.id == persona_id) {
|
||||
let snapshot = persona_snapshot_with_agent_config_fallback(
|
||||
persona,
|
||||
&record.env_vars,
|
||||
record.model.as_deref(),
|
||||
record.provider.as_deref(),
|
||||
);
|
||||
@@ -59,7 +58,12 @@ pub(crate) fn spawn_config_hash(
|
||||
}
|
||||
record.model = snapshot.model;
|
||||
record.provider = snapshot.provider;
|
||||
record.env_vars = snapshot.env_vars;
|
||||
// Mirror the start/restore self-heal: overrides equal to the live
|
||||
// persona value are treated as inherited. The persona env itself
|
||||
// reaches the hash through `resolve_effective_agent_env` below.
|
||||
record
|
||||
.env_vars
|
||||
.retain(|k, v| persona.env_vars.get(k) != Some(v));
|
||||
}
|
||||
}
|
||||
let record = &record;
|
||||
|
||||
Reference in New Issue
Block a user