mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
fix(desktop): apply agent model edits on first restart
Resolve linked-agent models from the current definition before the global default, never from stale materialized instance bytes, and add a local-agent Restart action that reuses the existing stop-then-start lifecycle. Co-authored-by: Pinky <44b8e82baa6e0e254e0208d68f335c283c94e7b78dd1fa10d5a49d3f13dd0435@sprout-oss.stage.blox.sqprod.co> Signed-off-by: Wes <wesbillman@users.noreply.github.com>
This commit is contained in:
@@ -204,44 +204,42 @@ pub fn save_global_agent_config(app: &AppHandle, config: &GlobalAgentConfig) ->
|
||||
atomic_write_json_restricted(&path, &payload)
|
||||
}
|
||||
|
||||
/// Resolve the effective model and provider for an agent using the
|
||||
/// precedence chain: `agent record → linked persona → global defaults → None`.
|
||||
/// Resolve the effective model and provider for an agent.
|
||||
///
|
||||
/// This is the single source of truth used by readiness evaluation, spawn,
|
||||
/// and deploy-payload construction. All three paths must use this function so
|
||||
/// they agree on what model/provider the agent will actually run with.
|
||||
/// Definition-linked instances resolve model through `definition → global → None`.
|
||||
/// Their materialized record model is a snapshot, not an override, so consulting
|
||||
/// it would let stale bytes beat a newly edited definition for one spawn.
|
||||
/// Definition-less legacy instances resolve model through `record → global → None`.
|
||||
/// Provider retains its existing `record → definition → global` precedence; this
|
||||
/// launch fix deliberately does not change provider semantics.
|
||||
///
|
||||
/// # Arguments
|
||||
/// * `record` — the `ManagedAgentRecord` (may have `None` for model/provider)
|
||||
/// * `personas` — all current persona records (looked up by `record.persona_id`)
|
||||
/// * `global` — global agent config defaults
|
||||
///
|
||||
/// # Returns
|
||||
/// `(effective_model, effective_provider)` — both `Option<&str>`.
|
||||
/// This is the single source of truth used by readiness evaluation and spawn.
|
||||
/// Both paths must use this function so they agree on what model/provider the
|
||||
/// agent will actually run with.
|
||||
pub(crate) fn resolve_effective_model_provider<'a>(
|
||||
record: &'a ManagedAgentRecord,
|
||||
personas: &'a [AgentDefinition],
|
||||
global: &'a GlobalAgentConfig,
|
||||
) -> (Option<&'a str>, Option<&'a str>) {
|
||||
let (persona_model, persona_provider) = record
|
||||
if let Some(persona) = record
|
||||
.persona_id
|
||||
.as_deref()
|
||||
.and_then(|pid| personas.iter().find(|p| p.id == pid))
|
||||
.map(|p| (p.model.as_deref(), p.provider.as_deref()))
|
||||
.unwrap_or((None, None));
|
||||
.and_then(|pid| personas.iter().find(|persona| persona.id == pid))
|
||||
{
|
||||
return (
|
||||
persona.model.as_deref().or(global.model.as_deref()),
|
||||
record
|
||||
.provider
|
||||
.as_deref()
|
||||
.or(persona.provider.as_deref())
|
||||
.or(global.provider.as_deref()),
|
||||
);
|
||||
}
|
||||
|
||||
let effective_model = record
|
||||
.model
|
||||
.as_deref()
|
||||
.or(persona_model)
|
||||
.or(global.model.as_deref());
|
||||
let effective_provider = record
|
||||
.provider
|
||||
.as_deref()
|
||||
.or(persona_provider)
|
||||
.or(global.provider.as_deref());
|
||||
|
||||
(effective_model, effective_provider)
|
||||
(
|
||||
record.model.as_deref().or(global.model.as_deref()),
|
||||
record.provider.as_deref().or(global.provider.as_deref()),
|
||||
)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
|
||||
@@ -376,19 +376,19 @@ fn persona(id: &str, model: Option<&str>, provider: Option<&str>) -> AgentDefini
|
||||
}
|
||||
}
|
||||
|
||||
/// Tier 1 — agent record wins: record has explicit model/provider; they must
|
||||
/// outrank both the linked persona and the global defaults. Fails against any
|
||||
/// implementation that prefers global or persona over the record.
|
||||
/// Linked definitions are authoritative for model: stale materialized record
|
||||
/// bytes must not win over an edited definition on the next spawn. Provider
|
||||
/// retains the existing record-first behavior.
|
||||
#[test]
|
||||
fn resolve_agent_record_wins_over_persona_and_global() {
|
||||
fn resolve_linked_definition_model_wins_over_stale_record() {
|
||||
let mut record = bare_record();
|
||||
record.persona_id = Some("p1".to_string());
|
||||
record.model = Some("record-model".to_string());
|
||||
record.provider = Some("record-provider".to_string());
|
||||
record.model = Some("stale-record-model".to_string());
|
||||
record.provider = Some("stale-record-provider".to_string());
|
||||
let personas = vec![persona(
|
||||
"p1",
|
||||
Some("persona-model"),
|
||||
Some("persona-provider"),
|
||||
Some("edited-definition-model"),
|
||||
Some("edited-definition-provider"),
|
||||
)];
|
||||
let global = GlobalAgentConfig {
|
||||
model: Some("global-model".to_string()),
|
||||
@@ -398,12 +398,29 @@ fn resolve_agent_record_wins_over_persona_and_global() {
|
||||
|
||||
let (model, provider) = resolve_effective_model_provider(&record, &personas, &global);
|
||||
|
||||
assert_eq!(model, Some("record-model"), "record model must win");
|
||||
assert_eq!(
|
||||
provider,
|
||||
Some("record-provider"),
|
||||
"record provider must win"
|
||||
);
|
||||
assert_eq!(model, Some("edited-definition-model"));
|
||||
assert_eq!(provider, Some("stale-record-provider"));
|
||||
}
|
||||
|
||||
/// Clearing a linked definition model means inherit global; stale materialized
|
||||
/// record bytes must not survive the first restart.
|
||||
#[test]
|
||||
fn resolve_linked_definition_model_clear_uses_global_over_stale_record() {
|
||||
let mut record = bare_record();
|
||||
record.persona_id = Some("p1".to_string());
|
||||
record.model = Some("stale-record-model".to_string());
|
||||
record.provider = Some("stale-record-provider".to_string());
|
||||
let personas = vec![persona("p1", None, None)];
|
||||
let global = GlobalAgentConfig {
|
||||
model: Some("global-model".to_string()),
|
||||
provider: Some("global-provider".to_string()),
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
let (model, provider) = resolve_effective_model_provider(&record, &personas, &global);
|
||||
|
||||
assert_eq!(model, Some("global-model"));
|
||||
assert_eq!(provider, Some("stale-record-provider"));
|
||||
}
|
||||
|
||||
/// Tier 2 — persona fallback: record has no model/provider; the linked
|
||||
@@ -543,18 +560,15 @@ fn resolve_all_none_when_no_source_provides_values() {
|
||||
);
|
||||
}
|
||||
|
||||
/// Partial tier — record has model but not provider; persona has provider but
|
||||
/// not model; global has both. Each field resolves independently through the
|
||||
/// three-tier chain.
|
||||
/// Linked fields resolve independently: a blank definition model inherits the
|
||||
/// global model even when the record still carries stale bytes, while provider
|
||||
/// retains its existing record → definition → global chain.
|
||||
#[test]
|
||||
fn resolve_each_field_resolves_independently_through_tiers() {
|
||||
fn resolve_linked_fields_use_their_intended_precedence() {
|
||||
let mut record = bare_record();
|
||||
record.persona_id = Some("p1".to_string());
|
||||
record.model = Some("record-model".to_string());
|
||||
// record.provider = None → falls through to persona
|
||||
record.model = Some("stale-record-model".to_string());
|
||||
let personas = vec![persona("p1", None, Some("persona-provider"))];
|
||||
// persona.model = None → global fills model if record also had none, but
|
||||
// record has model here so global is not needed for model.
|
||||
let global = GlobalAgentConfig {
|
||||
model: Some("global-model".to_string()),
|
||||
provider: Some("global-provider".to_string()),
|
||||
@@ -563,12 +577,26 @@ fn resolve_each_field_resolves_independently_through_tiers() {
|
||||
|
||||
let (model, provider) = resolve_effective_model_provider(&record, &personas, &global);
|
||||
|
||||
assert_eq!(model, Some("record-model"), "record wins for model");
|
||||
assert_eq!(
|
||||
provider,
|
||||
Some("persona-provider"),
|
||||
"persona wins for provider when record has none"
|
||||
);
|
||||
assert_eq!(model, Some("global-model"));
|
||||
assert_eq!(provider, Some("persona-provider"));
|
||||
}
|
||||
|
||||
/// Definition-less instances retain their own explicit values over globals.
|
||||
#[test]
|
||||
fn resolve_definition_less_record_wins_over_global() {
|
||||
let mut record = bare_record();
|
||||
record.model = Some("record-model".to_string());
|
||||
record.provider = Some("record-provider".to_string());
|
||||
let global = GlobalAgentConfig {
|
||||
model: Some("global-model".to_string()),
|
||||
provider: Some("global-provider".to_string()),
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
let (model, provider) = resolve_effective_model_provider(&record, &[], &global);
|
||||
|
||||
assert_eq!(model, Some("record-model"));
|
||||
assert_eq!(provider, Some("record-provider"));
|
||||
}
|
||||
|
||||
// ── IPC serialization ─────────────────────────────────────────────────────────
|
||||
|
||||
@@ -451,12 +451,14 @@ pub fn persona_snapshot_with_agent_config_fallback(
|
||||
}
|
||||
}
|
||||
|
||||
/// Re-pin `record` to `persona`: build the snapshot (via
|
||||
/// [`persona_snapshot_with_agent_config_fallback`], so blank persona
|
||||
/// `model`/`provider` preserve the record's own values) and mirror it onto the
|
||||
/// Re-pin `record` to `persona`: build the snapshot and mirror it onto the
|
||||
/// record — the definition quad (`system_prompt`/`model`/`provider`/`runtime`),
|
||||
/// the env-override self-heal, and the `persona_source_version` drift basis.
|
||||
///
|
||||
/// Model is definition-authoritative: a blank definition model clears stale
|
||||
/// materialized bytes so spawn can resolve the current global default. Provider
|
||||
/// retains its existing blank-definition fallback behavior.
|
||||
///
|
||||
/// This is the single apply used by every snapshot-apply site: the spawn
|
||||
/// re-pin (`start_local_agent_with_preflight`), the launch backfill and
|
||||
/// restore re-snapshot (`restore.rs`), and the prospective re-snapshot inside
|
||||
@@ -469,13 +471,13 @@ pub fn persona_snapshot_with_agent_config_fallback(
|
||||
pub fn apply_persona_snapshot(record: &mut ManagedAgentRecord, persona: &AgentDefinition) {
|
||||
let snapshot = persona_snapshot_with_agent_config_fallback(
|
||||
persona,
|
||||
record.model.as_deref(), // fallback: record.model
|
||||
record.provider.as_deref(), // fallback: record.provider
|
||||
record.model.as_deref(),
|
||||
record.provider.as_deref(),
|
||||
);
|
||||
if let Some(prompt) = snapshot.system_prompt {
|
||||
record.system_prompt = Some(prompt);
|
||||
}
|
||||
record.model = snapshot.model;
|
||||
record.model = persona.model.clone();
|
||||
record.provider = snapshot.provider;
|
||||
record.runtime = snapshot.runtime;
|
||||
// env_vars stay overrides-only. Self-heal records written before the env
|
||||
|
||||
@@ -81,6 +81,33 @@ fn persona(id: &str, runtime: Option<&str>, prompt: &str) -> AgentDefinition {
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn inherited_persona_model_converges_after_one_restart() {
|
||||
let mut rec = record();
|
||||
rec.persona_id = Some("persona".into());
|
||||
rec.system_prompt = Some("prompt".into());
|
||||
rec.model = Some("stale-model".into());
|
||||
let definition = persona("persona", Some("goose"), "prompt");
|
||||
let global = GlobalAgentConfig {
|
||||
model: Some("new-default-model".into()),
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
let hash_before_restart = spawn_config_hash(
|
||||
&rec,
|
||||
std::slice::from_ref(&definition),
|
||||
&[],
|
||||
"wss://ws.example",
|
||||
&global,
|
||||
);
|
||||
apply_persona_snapshot(&mut rec, &definition);
|
||||
assert_eq!(rec.model, None);
|
||||
let hash_after_restart =
|
||||
spawn_config_hash(&rec, &[definition], &[], "wss://ws.example", &global);
|
||||
|
||||
assert_eq!(hash_before_restart, hash_after_restart);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn hash_is_deterministic() {
|
||||
let rec = record();
|
||||
|
||||
@@ -1,7 +1,10 @@
|
||||
import assert from "node:assert/strict";
|
||||
import test from "node:test";
|
||||
|
||||
import { startManagedAgentWithRules } from "./managedAgentControlActions.ts";
|
||||
import {
|
||||
respawnManagedAgentWithRules,
|
||||
startManagedAgentWithRules,
|
||||
} from "./managedAgentControlActions.ts";
|
||||
|
||||
function agent(overrides = {}) {
|
||||
return {
|
||||
@@ -77,3 +80,19 @@ test("ordinary local agents still start normally", async () => {
|
||||
});
|
||||
assert.equal(calledWith, "deadbeef".repeat(8));
|
||||
});
|
||||
|
||||
test("running local agents stop before restart", async () => {
|
||||
const calls = [];
|
||||
const runningAgent = agent({ status: "running" });
|
||||
|
||||
await respawnManagedAgentWithRules({
|
||||
agent: runningAgent,
|
||||
stopManagedAgent: async (pubkey) => calls.push(["stop", pubkey]),
|
||||
startManagedAgent: async (pubkey) => calls.push(["start", pubkey]),
|
||||
});
|
||||
|
||||
assert.deepEqual(calls, [
|
||||
["stop", runningAgent.pubkey],
|
||||
["start", runningAgent.pubkey],
|
||||
]);
|
||||
});
|
||||
|
||||
@@ -34,6 +34,7 @@ import {
|
||||
} from "@/features/agents/lib/instanceInputForDefinition";
|
||||
import {
|
||||
isManagedAgentActive,
|
||||
respawnManagedAgentWithRules,
|
||||
startManagedAgentWithRules,
|
||||
stopManagedAgentWithRules,
|
||||
} from "@/features/agents/lib/managedAgentControlActions";
|
||||
@@ -451,10 +452,19 @@ export function UserProfilePanel({
|
||||
],
|
||||
);
|
||||
|
||||
const handleAgentPrimaryAction = React.useCallback(async () => {
|
||||
const handleAgentPrimaryAction = async (restart = false) => {
|
||||
if (!managedAgent) return;
|
||||
|
||||
try {
|
||||
if (restart && managedAgent.backend.type === "local") {
|
||||
await respawnManagedAgentWithRules({
|
||||
agent: managedAgent,
|
||||
startManagedAgent: startAgentMutation.mutateAsync,
|
||||
stopManagedAgent: stopAgentMutation.mutateAsync,
|
||||
});
|
||||
toast.success(`Restarted ${managedAgent.name}.`);
|
||||
return;
|
||||
}
|
||||
if (isManagedAgentActive(managedAgent)) {
|
||||
const result = await stopManagedAgentWithRules({
|
||||
agent: managedAgent,
|
||||
@@ -480,13 +490,7 @@ export function UserProfilePanel({
|
||||
error instanceof Error ? error.message : "Agent action failed.",
|
||||
);
|
||||
}
|
||||
}, [
|
||||
channelsQuery.data,
|
||||
managedAgent,
|
||||
relayAgentsQuery.data,
|
||||
startAgentMutation.mutateAsync,
|
||||
stopAgentMutation.mutateAsync,
|
||||
]);
|
||||
};
|
||||
|
||||
const handleInstantiateAgent = React.useCallback(async () => {
|
||||
if (!resolvedPersona) return;
|
||||
@@ -828,6 +832,7 @@ export function UserProfilePanel({
|
||||
followMutation={followMutation}
|
||||
agentInstruction={agentInstruction}
|
||||
handleAgentPrimaryAction={handleAgentPrimaryAction}
|
||||
handleAgentRestart={() => void handleAgentPrimaryAction(true)}
|
||||
handleEditAgent={handleEditAgent}
|
||||
handleEditPersona={canEditPersona ? handleEditPersona : undefined}
|
||||
handleInstantiateAgent={handleInstantiateAgent}
|
||||
|
||||
@@ -8,6 +8,7 @@ import {
|
||||
MessageSquare,
|
||||
Pencil,
|
||||
Play,
|
||||
RotateCw,
|
||||
Square,
|
||||
UserMinus,
|
||||
UserPlus,
|
||||
@@ -77,6 +78,7 @@ export type ProfileSummaryViewProps = {
|
||||
canInstantiateAgent: boolean;
|
||||
agentInstruction: string | null;
|
||||
handleAgentPrimaryAction: () => void;
|
||||
handleAgentRestart: () => void;
|
||||
handleEditAgent: () => void;
|
||||
handleEditPersona?: () => void;
|
||||
handleInstantiateAgent: () => void;
|
||||
@@ -189,6 +191,7 @@ export function ProfileSummaryView({
|
||||
canInstantiateAgent,
|
||||
agentInstruction,
|
||||
handleAgentPrimaryAction,
|
||||
handleAgentRestart,
|
||||
handleEditAgent,
|
||||
handleEditPersona,
|
||||
handleInstantiateAgent,
|
||||
@@ -369,6 +372,11 @@ export function ProfileSummaryView({
|
||||
? handleAgentPrimaryAction
|
||||
: undefined
|
||||
}
|
||||
onAgentRestart={
|
||||
isOwner === true && managedAgent?.backend.type === "local"
|
||||
? handleAgentRestart
|
||||
: undefined
|
||||
}
|
||||
isFollowing={isFollowing}
|
||||
messagePending={isMessagePending}
|
||||
onMessage={onOpenDm ? handleMessage : undefined}
|
||||
@@ -629,6 +637,7 @@ function ProfilePrimaryActions({
|
||||
isFollowing,
|
||||
messagePending,
|
||||
onAgentPrimaryAction,
|
||||
onAgentRestart,
|
||||
onEditAgent,
|
||||
onMessage,
|
||||
pubkey,
|
||||
@@ -642,6 +651,7 @@ function ProfilePrimaryActions({
|
||||
isFollowing: boolean;
|
||||
messagePending?: boolean;
|
||||
onAgentPrimaryAction?: () => void;
|
||||
onAgentRestart?: () => void;
|
||||
onEditAgent: () => void;
|
||||
onMessage?: () => void;
|
||||
pubkey: string;
|
||||
@@ -698,6 +708,15 @@ function ProfilePrimaryActions({
|
||||
testId="user-profile-agent-primary-action"
|
||||
/>
|
||||
) : null}
|
||||
{onAgentRestart ? (
|
||||
<ProfileQuickAction
|
||||
disabled={agentActionDisabled}
|
||||
icon={RotateCw}
|
||||
label="Restart"
|
||||
onClick={onAgentRestart}
|
||||
testId="user-profile-agent-restart"
|
||||
/>
|
||||
) : null}
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user