From 281d730af48f15aef8e20a15eba70773e7257e33 Mon Sep 17 00:00:00 2001 From: Fizz <550f2cb2562fc05f87c4839dc3c9b1bfca4d68db4183f8b54d3d477f6836a91d@buzz.block.builderlab.xyz> Date: Mon, 24 Aug 2026 12:55:49 -0600 Subject: [PATCH 01/12] fix(desktop): allow a team to have zero agents so it can be deleted MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Deleting an agent team was impossible without a workaround. Three guards formed a cycle: - `delete_team` refuses while agents still reference the team ("Delete or reconfigure them first") - `validate_persona_deletion` refuses to delete a team's agent ("Delete the team to remove all team agents together") - and the team editor's submit button required at least one selected member, so the roster could not be emptied either Delete-team said "remove the agents first", delete-agent said "delete the team first", and emptying the roster — the one sequence that breaks the cycle — was blocked by the UI. The only escape was to reassign a member to another team to drain the original. The backend already supported zero-member teams: `update_team` has no minimum-member check, `apply_team_membership_delta` detaches the removed members' instances (clearing the `team_id` that `delete_team` keys on), and `persona_ids: []` is representable in `teams.json`. Only the frontend enforced the minimum, so this drops that one condition — extracted as `canSubmitTeamDialog` so the contract is unit-testable. Removing the minimum makes an empty team reachable, which would have silently exposed Deploy/Duplicate/Share on a team with nothing to deploy. Sharing one would mint a snapshot that Buzz's own importer rejects ("Team snapshot must have at least one member" — snapshot import keeps its minimum, correctly). Those three actions are now gated on `resolution.isUsable`, which was already false for an empty roster, and the team card explains the state. Edit and Delete stay enabled: emptying a team and then deleting it is the intended way out. Tests: an e2e walk of the full user sequence (edit → deselect every member → save → delete), which fails on the disabled Save button without this change; a Rust test pinning that clearing `team_id` really drops the delete guard; and unit coverage for the submit gate and the emptied-team resolution. Signed-off-by: Fizz <550f2cb2562fc05f87c4839dc3c9b1bfca4d68db4183f8b54d3d477f6836a91d@buzz.block.builderlab.xyz> Co-authored-by: Storme Briscoe Signed-off-by: Storme Briscoe --- .../src/managed_agents/teams_tests.rs | 31 ++++++++ .../features/agents/lib/teamPersonas.test.mjs | 17 +++++ .../features/agents/ui/TeamDeleteDialog.tsx | 2 +- desktop/src/features/agents/ui/TeamDialog.tsx | 7 +- .../src/features/agents/ui/TeamsSection.tsx | 21 ++++- .../agents/ui/teamDialogSelection.test.mjs | 20 +++++ .../features/agents/ui/teamDialogSelection.ts | 25 ++++++ desktop/tests/e2e/agents.spec.ts | 76 +++++++++++++++++++ 8 files changed, 190 insertions(+), 9 deletions(-) diff --git a/desktop/src-tauri/src/managed_agents/teams_tests.rs b/desktop/src-tauri/src/managed_agents/teams_tests.rs index ff7900d3923..282cb532dfe 100644 --- a/desktop/src-tauri/src/managed_agents/teams_tests.rs +++ b/desktop/src-tauri/src/managed_agents/teams_tests.rs @@ -263,6 +263,37 @@ fn agents_referencing_team_empty_when_no_matches() { assert!(agents_referencing_team(&agents, &t).is_empty()); } +/// The escape hatch from the team/agent delete deadlock, asserted end to end at +/// this layer: `delete_team_with_cascade` refuses while any agent references the +/// team, and `validate_persona_deletion` refuses to delete a team's agent — so +/// the only non-circular sequence is "empty the team's roster, then delete the +/// team". Emptying the roster clears `team_id` on the members' instances (see +/// `apply_team_membership_delta` in `commands::teams`), and once it is cleared +/// this guard must report zero referencing agents. If it still matched, the team +/// would stay permanently undeletable and the deadlock would be back. +#[test] +fn detached_agents_no_longer_reference_the_team() { + let t = team("json-team-3", "Emptied Team"); + + let mut bound = managed_agent("Bound Agent"); + bound.team_id = Some("json-team-3".to_string()); + assert_eq!( + agents_referencing_team(std::slice::from_ref(&bound), &t), + vec!["Bound Agent"], + "a bound agent blocks team deletion" + ); + + // What emptying the roster does: clear the binding, keep the agent. + let detached = ManagedAgentRecord { + team_id: None, + ..bound + }; + assert!( + agents_referencing_team(std::slice::from_ref(&detached), &t).is_empty(), + "a detached agent must not block team deletion" + ); +} + // Migration pins — exercise the real merge_teams wrapper (with production consts). #[test] diff --git a/desktop/src/features/agents/lib/teamPersonas.test.mjs b/desktop/src/features/agents/lib/teamPersonas.test.mjs index 34d2246e98e..85163e8f23f 100644 --- a/desktop/src/features/agents/lib/teamPersonas.test.mjs +++ b/desktop/src/features/agents/lib/teamPersonas.test.mjs @@ -92,3 +92,20 @@ test("getUsableTeams keeps only fully-resolved teams with at least one persona", ["team-ready"], ); }); + +// An emptied team is now reachable through the editor (removing every member is +// how you escape the team/agent delete deadlock). It must still be treated as +// unusable: a zero-member snapshot fails the importer's own +// "Team snapshot must have at least one member" check, and deploying it would +// attach no agents at all. +test("resolveTeamPersonas marks a deliberately emptied team complete but unusable", () => { + const resolution = resolveTeamPersonas(createTeam("team-empty", []), [ + createPersona("persona-1", "Solo"), + ]); + + assert.equal(resolution.hasMissingPersonas, false); + assert.equal(resolution.isComplete, true); + assert.equal(resolution.isUsable, false); + assert.equal(resolution.missingPersonaCount, 0); + assert.deepEqual(resolution.resolvedPersonaIds, []); +}); diff --git a/desktop/src/features/agents/ui/TeamDeleteDialog.tsx b/desktop/src/features/agents/ui/TeamDeleteDialog.tsx index 383f92c4a53..5477fd9a020 100644 --- a/desktop/src/features/agents/ui/TeamDeleteDialog.tsx +++ b/desktop/src/features/agents/ui/TeamDeleteDialog.tsx @@ -31,7 +31,7 @@ export function TeamDeleteDialog({ Delete team? {team - ? `Delete "${team.name}". Already-deployed agents are not affected, but this team template will no longer be available.` + ? `Delete "${team.name}". This removes the team template only — deployed agents are left in place, but deletion is blocked while any of them still reference this team.` : "Delete this team."} diff --git a/desktop/src/features/agents/ui/TeamDialog.tsx b/desktop/src/features/agents/ui/TeamDialog.tsx index 695504429dc..e4a0128e5c7 100644 --- a/desktop/src/features/agents/ui/TeamDialog.tsx +++ b/desktop/src/features/agents/ui/TeamDialog.tsx @@ -21,6 +21,7 @@ import { Textarea } from "@/shared/ui/textarea"; import { personaCatalogCopy } from "./personaLibraryCopy"; import { RemoveMembersConfirmDialog } from "./RemoveMembersConfirmDialog"; import { + canSubmitTeamDialog, copySelectedPersonaIds, countMissingPersonaIds, filterAvailablePersonaIds, @@ -325,11 +326,7 @@ export function TeamDialog({ Cancel + {/* Edit and Delete stay enabled for an unusable team on purpose. You use them + to empty a team and then delete it. */} event.preventDefault()} > onAddToChannel(team)} > @@ -141,14 +130,14 @@ export function TeamsSection({ Edit onDuplicate(team)} > Duplicate onShare(team)} > diff --git a/desktop/src/features/agents/ui/teamDialogSelection.test.mjs b/desktop/src/features/agents/ui/teamDialogSelection.test.mjs index eed58a742e2..6515ec94dc0 100644 --- a/desktop/src/features/agents/ui/teamDialogSelection.test.mjs +++ b/desktop/src/features/agents/ui/teamDialogSelection.test.mjs @@ -94,9 +94,8 @@ test("orderPersonasByInitiallySelected keeps initially selected personas at top" // ── canSubmitTeamDialog ─────────────────────────────────────────────────── // -// These tests show the correction of a defect. Before, the submit button also -// needed `selectedPersonaIds.length > 0`. Thus you could not save a team after -// you removed all the members, and you could not delete the team. +// A team with no members must be savable. If it is not, you cannot delete a +// team, because you must first remove each member. test("canSubmitTeamDialog allows saving a team with an empty roster", () => { assert.equal(canSubmitTeamDialog({ name: "Hive", isPending: false }), true); diff --git a/desktop/src/features/agents/ui/teamDialogSelection.ts b/desktop/src/features/agents/ui/teamDialogSelection.ts index 0ab4977a029..02e0417de74 100644 --- a/desktop/src/features/agents/ui/teamDialogSelection.ts +++ b/desktop/src/features/agents/ui/teamDialogSelection.ts @@ -11,21 +11,8 @@ export function copySelectedPersonaIds(personaIds: string[]): string[] { /** * Tells you if the submit button in the team dialog is enabled. * - * The team must have a name. The team does not need a member. An empty roster - * is correct. - * - * An empty roster is necessary. It is the only way to delete a team. The - * `delete_team` command stops if an agent points to the team. The agent delete - * command stops if the agent is in a team. Thus you must first remove all the - * members from the team. Then you can delete the team. - * - * The Rust code permits a team that has no members. The `update_team` command - * does not count the members. The `apply_team_membership_delta` function clears - * the `team_id` field of each member that you remove. Only the import of a - * snapshot needs one member or more. - * - * This is a separate function because a test can then read the rule. Before, - * the rule was in a JSX `disabled` expression, where a test cannot read it. + * A name is necessary. A member is not. A team with no members must be + * savable, because you must empty a team before you can delete it. */ export function canSubmitTeamDialog({ name, diff --git a/desktop/tests/e2e/agents.spec.ts b/desktop/tests/e2e/agents.spec.ts index 92f45972d2d..dbf6e57d526 100644 --- a/desktop/tests/e2e/agents.spec.ts +++ b/desktop/tests/e2e/agents.spec.ts @@ -2709,12 +2709,8 @@ test("duplicate instances move from the agents gallery into the agent profile", ).toHaveCount(0); }); -// This test shows the correction of a defect. You could not delete a team. The -// `delete_team` command stops if an agent points to the team. The agent delete -// command stops if the agent is in a team. Thus you must first remove all the -// members from the team, then delete the team. But the editor did not permit -// this, because the submit button needed one member or more. This test does the -// full sequence: edit the team, remove each member, save, then delete. +// You must be able to empty a team and then delete it. Before, the submit +// button needed one member or more, so neither step was possible. test("a team can be emptied and then deleted", async ({ page }) => { await installMockBridge(page, { personas: [ diff --git a/desktop/tests/e2e/team-snapshot.spec.ts b/desktop/tests/e2e/team-snapshot.spec.ts index 027c66d2a78..413adc0f3d5 100644 --- a/desktop/tests/e2e/team-snapshot.spec.ts +++ b/desktop/tests/e2e/team-snapshot.spec.ts @@ -49,14 +49,9 @@ const ANALYST_PERSONA_ID = "test-analyst"; const ANALYST_PUBKEY = "953d3363262e86b770419834c53d2446409db6d918a57f8f339d495d54ab001f"; -// The share tests use this team. The team has one member. -// -// You cannot share a team that has no members. A team snapshot must have one -// member or more, and the Share item is disabled for a team that has no -// members. The default mock team "Engineering" has no members. Therefore each -// share test seeds this team, and the team holds the seeded Analyst persona. -// The name must be different from "Engineering". If the names are the same, a -// locator finds two teams and the test stops. +// The share tests need a team with a member. The default mock team +// "Engineering" has none, so Share is disabled for it. Use a different name, +// or a locator matches both teams. const SHARE_TEAM_NAME = "Delivery Crew"; const SHARE_TEAM_SEED = { id: "team-share-001", From b9edbbceafb1e1ab8edfa9c33290f1e64b5d0220 Mon Sep 17 00:00:00 2001 From: Fizz <550f2cb2562fc05f87c4839dc3c9b1bfca4d68db4183f8b54d3d477f6836a91d@buzz.block.builderlab.xyz> Date: Mon, 24 Aug 2026 20:11:45 -0600 Subject: [PATCH 04/12] fix(desktop): hold the empty-team invariants at the native boundary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Carl's review of PR 6715 found two holes. The UI change was correct, but the native side did not hold the two invariants that the UI depends on. This commit closes both, and it closes the two test gaps the review named. P1 — an update must not report success while the delete guard can refuse. `commit_team_update` saved `teams.json` first, then propagated the roster to the agent store best-effort. A lost agent write left the team empty on disk while an agent kept the team id. `delete_team_with_cascade` then refuses the team. That is the exact empty-and-undeletable state this feature exists to remove, and the command reported success for it. A second save did not repair it. `apply_team_membership_delta` reads the previous-to-current delta, and after the first save both are empty, so the delta does nothing. Only boot repair could clear the binding, which needs an app restart. Two changes: - `detach_agents_outside_roster` reconciles against the *current* roster instead of a delta. An instance bound to this team whose persona is not on the roster loses the binding. The invariant is state-based, so a second save repairs a lost detach. Bindings to other teams are untouched. - `propagate_membership` returns the error. `commit_team_update` now maps it to a message that names the effect and the action. `create` keeps the best-effort policy through `propagate_membership_best_effort`, which is now a wrapper: a create has no id yet, so a retry can mint a duplicate team, and a missing backfill blocks nothing. P2 — the producer must refuse a snapshot of a team with no members. `build_team_export_snapshot` mapped an empty roster to `members = []`, and the importer rejects that artifact. The disabled menu item is not an integrity boundary: both export commands stay callable, and a share dialog can submit after the roster becomes empty, because the command reads the team by id at the time of the call. The guard is now in the producer, where both commands pass. Tests. - `update_never_reports_success_while_the_delete_guard_sees_the_agent` is the command-level regression the review asked for. It starts with a bound agent, empties the roster, and applies the real `agents_referencing_team` predicate to the store the command left. It pins the report on failure and the repair on retry. - `agents_referencing_team` becomes `pub(crate)` so that test can use the production predicate instead of a copy. - `empty_team_export_is_refused_before_any_bytes` holds the P2 guard. The importer side is already pinned by `validate_rejects_zero_members`. - Three unit tests cover the reconcile: the strict error path, the recovery save, and the scope of the reconcile. - `commit_returns_ok_when_agent_save_fails` becomes `commit_create_returns_ok_when_agent_save_fails`. Its update half is removed, because that contract changed on purpose. The e2e mock did not hold either contract, so no e2e could cover them. `e2eBridge.ts` now carries `team_id` on a mock agent, detaches on update like the native propagate, and refuses a delete while an agent still references the team. The `a team can be emptied and then deleted` spec seeds a bound agent, and it asserts the delete is refused first. Gates: cargo test 2871 pass; clippy -D warnings clean; cargo fmt clean; tsc clean; node tests 5401 pass; Playwright integration 188 pass. The 4 remaining integration failures need a local relay on port 3000 and are not related to this change. Signed-off-by: Fizz <550f2cb2562fc05f87c4839dc3c9b1bfca4d68db4183f8b54d3d477f6836a91d@buzz.block.builderlab.xyz> Co-authored-by: Storme Briscoe Signed-off-by: Storme Briscoe --- .../src-tauri/src/commands/team_snapshot.rs | 15 + .../src/commands/team_snapshot/tests.rs | 37 +++ desktop/src-tauri/src/commands/teams.rs | 304 ++++++++++++++++-- desktop/src-tauri/src/managed_agents/teams.rs | 5 +- desktop/src/testing/e2eBridge.ts | 38 +++ desktop/tests/e2e/agents.spec.ts | 28 ++ 6 files changed, 394 insertions(+), 33 deletions(-) diff --git a/desktop/src-tauri/src/commands/team_snapshot.rs b/desktop/src-tauri/src/commands/team_snapshot.rs index e4c08a14be0..f716a11b288 100644 --- a/desktop/src-tauri/src/commands/team_snapshot.rs +++ b/desktop/src-tauri/src/commands/team_snapshot.rs @@ -33,6 +33,10 @@ const PNG_MAGIC: [u8; 4] = [0x89, 0x50, 0x4e, 0x47]; const ZIP_MAGIC_PREFIX: [u8; 2] = [0x50, 0x4b]; const LEGACY_TEAM_ERROR: &str = "Legacy team files are no longer supported. Export a buzz-team-snapshot v1 .team.json or .team.png instead."; +/// Refusal for an export of a team that has no members. The importer rejects +/// such a snapshot, so the producer must not write one. +pub(crate) const EMPTY_TEAM_EXPORT_ERROR: &str = + "This team has no agents. Add at least one agent before you share or export it."; /// Decode a canonical team snapshot, rejecting retired flat team JSON and /// persona-pack ZIP files with a migration-oriented error. @@ -263,6 +267,13 @@ struct MintedMember { effective_avatar: Option, } +/// Builds the snapshot for an export. +/// +/// A team with no members must not become a snapshot. The importer rejects such +/// an artifact, because `validate_team_snapshot` needs one member or more. Both +/// export commands come through this function, so the rejection belongs here. +/// A disabled menu item is not sufficient: the commands stay callable, and a +/// share dialog can submit after the roster becomes empty. fn build_team_export_snapshot( team: &TeamRecord, personas: &[AgentDefinition], @@ -270,6 +281,10 @@ fn build_team_export_snapshot( memory_level: MemoryLevel, memory_entries_by_persona: &std::collections::HashMap>, ) -> Result { + if team.persona_ids.is_empty() { + return Err(EMPTY_TEAM_EXPORT_ERROR.to_string()); + } + let members = team .persona_ids .iter() diff --git a/desktop/src-tauri/src/commands/team_snapshot/tests.rs b/desktop/src-tauri/src/commands/team_snapshot/tests.rs index bec7f43bf8a..d096f4ac25e 100644 --- a/desktop/src-tauri/src/commands/team_snapshot/tests.rs +++ b/desktop/src-tauri/src/commands/team_snapshot/tests.rs @@ -287,6 +287,43 @@ fn team_export_with_instance_and_memory_level_uses_supplied_entries() { assert!(snap_no_instance.members[0].memory.entries.is_empty()); } +/// The producer must refuse a team that has no members. +/// +/// The importer rejects such a snapshot, so an export of it makes a file that +/// nobody can import. `managed_agents::team_snapshot` pins that rejection in +/// `validate_rejects_zero_members`. A disabled menu item does not stop the +/// export: both export commands stay callable, and they read the team from disk +/// at the time of the call. This test holds the guard in the producer, where +/// both commands pass. +#[test] +fn empty_team_export_is_refused_before_any_bytes() { + let team = TeamRecord { + id: "empty".to_string(), + name: "Emptied Team".to_string(), + description: None, + instructions: None, + persona_ids: vec![], + is_builtin: false, + source_dir: None, + is_symlink: false, + symlink_target: None, + version: None, + created_at: "now".to_string(), + updated_at: "now".to_string(), + }; + + let err = build_team_export_snapshot( + &team, + &[], + &[], + MemoryLevel::None, + &std::collections::HashMap::new(), + ) + .expect_err("an export of a team with no members must fail"); + + assert_eq!(err, EMPTY_TEAM_EXPORT_ERROR); +} + #[test] fn team_import_definitions_are_built_for_all_members() { let mut memory_bearing = member("Alice"); diff --git a/desktop/src-tauri/src/commands/teams.rs b/desktop/src-tauri/src/commands/teams.rs index e17c5bdb247..4e8408ebea2 100644 --- a/desktop/src-tauri/src/commands/teams.rs +++ b/desktop/src-tauri/src/commands/teams.rs @@ -26,14 +26,77 @@ fn trim_optional(value: Option) -> Option { }) } +/// Clear `team_id` on every instance that is bound to `team_id` but whose +/// persona is absent from `current_persona_ids`. Reports whether anything +/// changed. +/// +/// This reads the current roster, not a delta. That is what makes a failed +/// detach recoverable: after a failed agent-store write the team is already +/// saved with the new roster, so the prior→current delta is empty on the next +/// save and a delta-only pass would do nothing. The invariant here is +/// state-based — an instance bound to a team must have its persona on that +/// team's roster — so a second save still repairs the binding. +/// +/// Bindings to other teams are untouched, and an unbound instance is left to +/// the delta's backfill branch. +fn detach_agents_outside_roster( + records: &mut [crate::managed_agents::ManagedAgentRecord], + team_id: &str, + current_persona_ids: &[String], +) -> bool { + let mut changed = false; + for record in records.iter_mut() { + if record.pubkey.is_empty() || record.team_id.as_deref() != Some(team_id) { + continue; + } + let Some(persona_id) = record.persona_id.as_deref() else { + continue; + }; + if !current_persona_ids.iter().any(|id| id == persona_id) { + record.team_id = None; + changed = true; + } + } + changed +} + +/// Propagate a team's membership to its members' already-running instances. +/// Loads the agent store, applies the roster delta via +/// [`apply_team_membership_delta`], reconciles stale bindings via +/// [`detach_agents_outside_roster`], and re-saves only when something changed. +/// Reports a load/save failure to the caller, which chooses the policy. +pub(in crate::commands) fn propagate_membership( + team_id: &str, + previous_persona_ids: &[String], + current_persona_ids: &[String], + load_agents: impl FnOnce() -> Result, String>, + save_agents: impl FnOnce(&[crate::managed_agents::ManagedAgentRecord]) -> Result<(), String>, +) -> Result<(), String> { + let mut records = load_agents()?; + let delta_changed = apply_team_membership_delta( + &mut records, + team_id, + previous_persona_ids, + current_persona_ids, + ); + let detached = detach_agents_outside_roster(&mut records, team_id, current_persona_ids); + if delta_changed || detached { + save_agents(&records)?; + } + Ok(()) +} + /// Propagate a team's membership *change* to its members' already-running -/// instances, best-effort. Loads the agent store, applies the roster delta via -/// [`apply_team_membership_delta`], and re-saves only when something changed; -/// any load/save error is logged and swallowed. Called after the authoritative -/// `save_teams` succeeds — the team already exists on disk and boot repair is -/// the designed retry for a stale/unset binding, so a secondary-store hiccup -/// must not fail a command whose team write already landed (a UI retry would -/// then mint a duplicate team). +/// instances, best-effort: any load/save error is logged and swallowed. +/// +/// Used by [`commit_team_create`] and by the inbound reconcile path. For a +/// create, the team write already landed, and failing the command would make a +/// UI retry mint a duplicate team; a missing backfill only costs a member the +/// team instructions until boot repair runs, and it blocks nothing. +/// +/// [`commit_team_update`] does **not** use this policy. A removal that does not +/// reach the agent store leaves an agent bound to the team, and the delete guard +/// then refuses the team — so an update reports the failure instead. /// /// `load_agents`/`save_agents` are injected so the command wiring (prior-roster /// capture, delta direction, and this best-effort policy) is unit-testable @@ -49,19 +112,13 @@ pub(in crate::commands) fn propagate_membership_best_effort( load_agents: impl FnOnce() -> Result, String>, save_agents: impl FnOnce(&[crate::managed_agents::ManagedAgentRecord]) -> Result<(), String>, ) { - let result = (|| -> Result<(), String> { - let mut records = load_agents()?; - if apply_team_membership_delta( - &mut records, - team_id, - previous_persona_ids, - current_persona_ids, - ) { - save_agents(&records)?; - } - Ok(()) - })(); - if let Err(e) = result { + if let Err(e) = propagate_membership( + team_id, + previous_persona_ids, + current_persona_ids, + load_agents, + save_agents, + ) { eprintln!("buzz-desktop: team-membership-propagate: {e}"); } } @@ -86,11 +143,22 @@ fn commit_team_create( /// In-memory core of [`update_team`]: mutate the matching team, capturing its /// roster *before* the edit, persist teams authoritatively, then propagate the -/// prior→current delta to live instances best-effort. The prior-roster capture +/// prior→current delta to live instances. The prior-roster capture /// and its use as the delta baseline live here — not at a command call site — /// so a miswire to the wrong baseline is caught by a test. Injected persistence -/// keeps it `AppHandle`-free; a `persist_teams` error propagates, agent IO is -/// best-effort. Returns the updated team. +/// keeps it `AppHandle`-free; a `persist_teams` error propagates. Returns the +/// updated team. +/// +/// Unlike [`commit_team_create`], an update **reports** a failure of the agent +/// write. A removal clears `team_id` on the removed member. If that write does +/// not land, the agent keeps the binding and `delete_team_with_cascade` refuses +/// the team, so the user gets an empty team that they cannot delete — the exact +/// defect this command must not create. The command must not report success +/// while the delete guard can still refuse. +/// +/// Reporting is safe here because an update is idempotent: it targets an +/// existing team by id, so a retry cannot mint a duplicate team. A create has no +/// id yet, which is why it keeps the best-effort policy. #[allow(clippy::too_many_arguments)] fn commit_team_update( teams: &mut [TeamRecord], @@ -120,13 +188,19 @@ fn commit_team_update( let updated = team.clone(); persist_teams(teams)?; - propagate_membership_best_effort( + propagate_membership( &updated.id, &previous_persona_ids, &updated.persona_ids, load_agents, save_agents, - ); + ) + .map_err(|e| { + format!( + "Saved the team, but could not update its agents: {e}. The team can refuse deletion \ + until this succeeds. Save the team again." + ) + })?; Ok(updated) } @@ -395,7 +469,10 @@ pub async fn update_team(input: UpdateTeamRequest, app: AppHandle) -> Result = Vec::new(); let created = commit_team_create( &mut teams, @@ -633,9 +710,99 @@ mod tests { ) .expect("create swallows secondary-store failure"); assert_eq!(created.id, "team-a"); + } + /// `update` must NOT swallow an agent-store failure while emptying a roster. + /// + /// The removal clears `team_id` on the removed member. If that write fails + /// and the command reports success, the team is empty on disk but the agent + /// still points to it, so `delete_team_with_cascade` refuses the team. That + /// is the empty-and-undeletable state this feature exists to remove, so the + /// command must report the failure. The team write itself has landed, and an + /// update is idempotent, so a retry is safe. + #[test] + fn commit_update_reports_agent_save_failure_when_emptying_a_roster() { let mut teams = vec![team("team-a", &["duncan"])]; - let updated = commit_team_update( + let err = commit_team_update( + &mut teams, + "team-a", + "team-a".to_string(), + None, + None, + ids(&[]), + "2026-02-02T00:00:00Z".to_string(), + |_| Ok(()), + || Ok(vec![instance('a', "duncan", Some("team-a"))]), + |_| Err("disk full".to_string()), + ) + .expect_err("an update must not report success when the detach is lost"); + + assert!(err.contains("could not update its agents"), "{err}"); + assert!(err.contains("Save the team again"), "{err}"); + // The team write is authoritative and already landed. + assert!(teams[0].persona_ids.is_empty()); + } + + /// A retry after a lost detach must repair the binding. + /// + /// This is the recovery path of the test above. The team is already saved + /// empty, so the prior→current delta is empty and a delta-only pass would do + /// nothing. `detach_agents_outside_roster` reconciles against the current + /// roster instead, so saving the same empty roster again still clears the + /// stale `team_id` and makes the team deletable. + #[test] + fn resaving_an_already_empty_roster_repairs_a_lost_detach() { + let mut teams = vec![team("team-a", &[])]; + let spy = RefCell::new(StoreSpy::default()); + + commit_team_update( + &mut teams, + "team-a", + "team-a".to_string(), + None, + None, + ids(&[]), + "2026-02-03T00:00:00Z".to_string(), + |_| Ok(()), + // The agent kept its binding because the earlier write was lost. + || Ok(vec![instance('a', "duncan", Some("team-a"))]), + |records| { + spy.borrow_mut().saved = Some(records.to_vec()); + Ok(()) + }, + ) + .expect("the retry succeeds"); + + let saved = spy + .borrow() + .saved + .clone() + .expect("the retry must write the agent store"); + assert_eq!( + saved[0].team_id, None, + "a stale binding is cleared against the current roster, not a delta" + ); + } + + /// End to end at the command seam, measured by the real delete guard. + /// + /// This test starts with a bound agent and empties the roster. It applies + /// `agents_referencing_team` — the predicate `delete_team_with_cascade` uses + /// — to the agent store that the command left behind. It pins both halves of + /// the contract: + /// + /// 1. The failed save reports an error. It never claims success while the + /// delete guard still sees the agent. + /// 2. The retry succeeds, and the guard then sees no agent. Delete is + /// possible without an app restart. + #[test] + fn update_never_reports_success_while_the_delete_guard_sees_the_agent() { + let mut teams = vec![team("team-a", &["duncan"])]; + // The store on disk. A failed save leaves it as it was. + let store = RefCell::new(vec![instance('a', "duncan", Some("team-a"))]); + + // Attempt 1: the agent write fails. + let err = commit_team_update( &mut teams, "team-a", "team-a".to_string(), @@ -644,11 +811,84 @@ mod tests { ids(&[]), "2026-02-02T00:00:00Z".to_string(), |_| Ok(()), - || Err("agent store unreadable".to_string()), + || Ok(store.borrow().clone()), + |_| Err("disk full".to_string()), + ) + .expect_err("the command must report the lost detach"); + assert!(err.contains("could not update its agents"), "{err}"); + + // The team is empty on disk, but the guard still refuses deletion. The + // command reported this state instead of hiding it. + assert!(teams[0].persona_ids.is_empty()); + assert_eq!( + crate::managed_agents::agents_referencing_team(&store.borrow(), &teams[0]), + vec!["duncan"], + "the guard still sees the agent, so the report was required" + ); + + // Attempt 2: the same save, and now the write lands. + commit_team_update( + &mut teams, + "team-a", + "team-a".to_string(), + None, + None, + ids(&[]), + "2026-02-03T00:00:00Z".to_string(), |_| Ok(()), + || Ok(store.borrow().clone()), + |records| { + *store.borrow_mut() = records.to_vec(); + Ok(()) + }, ) - .expect("update swallows secondary-store failure"); - assert_eq!(updated.persona_ids, Vec::::new()); + .expect("the retry succeeds"); + + assert!( + crate::managed_agents::agents_referencing_team(&store.borrow(), &teams[0]).is_empty(), + "the retry must make the team deletable" + ); + } + + /// The reconcile is scoped: it clears a binding to *this* team only, and it + /// leaves an instance whose persona is still on the roster alone. + #[test] + fn detach_outside_roster_is_scoped_to_this_team_and_absent_personas() { + let mut records = vec![ + instance('a', "duncan", Some("team-a")), + instance('b', "paul", Some("team-b")), + instance('c', "ada", Some("team-a")), + ]; + + assert!(detach_agents_outside_roster( + &mut records, + "team-a", + &ids(&["ada"]), + )); + + assert_eq!(records[0].team_id, None, "absent from this team's roster"); + assert_eq!( + records[1].team_id.as_deref(), + Some("team-b"), + "another team's binding is untouched" + ); + assert_eq!( + records[2].team_id.as_deref(), + Some("team-a"), + "still on the roster, so the binding stays" + ); + } + + /// A roster with every binding already correct writes nothing. + #[test] + fn detach_outside_roster_is_inert_when_nothing_is_stale() { + let mut records = vec![instance('a', "duncan", Some("team-a"))]; + assert!(!detach_agents_outside_roster( + &mut records, + "team-a", + &ids(&["duncan"]), + )); + assert_eq!(records[0].team_id.as_deref(), Some("team-a")); } } diff --git a/desktop/src-tauri/src/managed_agents/teams.rs b/desktop/src-tauri/src/managed_agents/teams.rs index 937893d5316..cb5a7c04173 100644 --- a/desktop/src-tauri/src/managed_agents/teams.rs +++ b/desktop/src-tauri/src/managed_agents/teams.rs @@ -210,7 +210,10 @@ pub fn save_teams(app: &AppHandle, records: &[TeamRecord]) -> Result<(), String> /// legacy `persona_team_dir` link (directory-backed teams only) or the /// `team_id` field (every team kind, all agents created after the team_id /// seam landed). Used to block team deletion while agents still depend on it. -fn agents_referencing_team<'a>( +/// +/// Visible to the crate so a test of the update command can apply the real +/// delete guard to the agent store that the update saved. +pub(crate) fn agents_referencing_team<'a>( agents: &'a [ManagedAgentRecord], team: &TeamRecord, ) -> Vec<&'a str> { diff --git a/desktop/src/testing/e2eBridge.ts b/desktop/src/testing/e2eBridge.ts index 9002d751c8c..4e5c18c9251 100644 --- a/desktop/src/testing/e2eBridge.ts +++ b/desktop/src/testing/e2eBridge.ts @@ -99,6 +99,8 @@ export type MockManagedAgentSeed = { personaId?: string | null; /** Harness/runtime id pin; `null` = inherit from persona (native default). */ runtime?: string | null; + /** Team binding. Seed it to reproduce the native delete guard in a test. */ + teamId?: string | null; status?: RawManagedAgent["status"]; channelNames?: string[]; channelIds?: string[]; @@ -899,6 +901,9 @@ type RawManagedAgent = { persona_id: string | null; /** Record-level harness/runtime pin (`null` when inheriting from the persona). */ runtime: string | null; + /** Team this instance belongs to (`null` when unbound). The native delete + * guard refuses a team while an agent still carries its id. */ + team_id: string | null; relay_url: string; acp_command: string; agent_command: string; @@ -1748,6 +1753,7 @@ function cloneManagedAgent(agent: MockManagedAgent): RawManagedAgent { name: agent.name, persona_id: agent.persona_id, runtime: agent.runtime ?? null, + team_id: agent.team_id ?? null, relay_url: agent.relay_url, acp_command: agent.acp_command, agent_command: agent.agent_command, @@ -2309,6 +2315,7 @@ function buildSeededManagedAgent(seed: MockManagedAgentSeed): MockManagedAgent { // Native serde always emits this key (`null` when unpinned) — the bridge // must mirror the wire shape, not omit the key. runtime: seed.runtime ?? null, + team_id: seed.teamId ?? null, relay_url: DEFAULT_RELAY_WS_URL, acp_command: "buzz-acp", agent_command: agentCommand, @@ -8811,14 +8818,43 @@ async function handleUpdateTeam(args: { team.persona_ids = [...args.input.personaIds]; team.updated_at = new Date().toISOString(); + // Mirror the native `propagate_membership`: an instance bound to this team + // whose persona left the roster loses the binding. Without this, the mock + // keeps the binding and no e2e can reach the delete guard below. + const now = team.updated_at; + for (const agent of mockManagedAgents) { + if (agent.team_id !== team.id) continue; + if (agent.persona_id && team.persona_ids.includes(agent.persona_id)) { + continue; + } + agent.team_id = null; + agent.updated_at = now; + } + return { ...team, persona_ids: [...team.persona_ids] }; } +/** Mirrors the native `agents_referencing_team` guard: names of the managed + * agents that still carry this team's id. */ +function mockAgentsReferencingTeam(teamId: string): string[] { + return mockManagedAgents + .filter((agent) => agent.team_id === teamId) + .map((agent) => agent.name); +} + async function handleDeleteTeam(args: { id: string }): Promise { const team = mockTeams.find((candidate) => candidate.id === args.id); if (team?.is_builtin) { throw new Error("Built-in teams cannot be deleted."); } + // Mirror `delete_team_with_cascade`: a bound agent blocks the delete. This is + // the contract the empty-team feature depends on, so the mock must hold it. + const referencing = mockAgentsReferencingTeam(args.id); + if (referencing.length > 0) { + throw new Error( + `Cannot delete team "${args.id}": ${referencing.length} agent(s) still reference it (${referencing.join(", ")}). Delete or reconfigure them first.`, + ); + } mockTeams = mockTeams.filter((candidate) => candidate.id !== args.id); } @@ -8906,6 +8942,7 @@ async function handleCreateManagedAgent( input: { name: string; personaId?: string; + teamId?: string | null; relayUrl?: string; acpCommand?: string; agentCommand?: string; @@ -8984,6 +9021,7 @@ async function handleCreateManagedAgent( persona_id: args.input.personaId ?? null, // Create never pins a harness id — the record inherits from the persona. runtime: null, + team_id: args.input.teamId ?? null, relay_url: args.input.relayUrl ?? DEFAULT_RELAY_WS_URL, acp_command: args.input.acpCommand ?? "buzz-acp", agent_command: agentCommand, diff --git a/desktop/tests/e2e/agents.spec.ts b/desktop/tests/e2e/agents.spec.ts index dbf6e57d526..97f0d1f4bb8 100644 --- a/desktop/tests/e2e/agents.spec.ts +++ b/desktop/tests/e2e/agents.spec.ts @@ -2711,6 +2711,10 @@ test("duplicate instances move from the agents gallery into the agent profile", // You must be able to empty a team and then delete it. Before, the submit // button needed one member or more, so neither step was possible. +// +// The team here has a bound managed agent, so the test also crosses the native +// delete guard: the delete must fail while the agent carries the team id, and it +// must succeed after the save clears the binding. test("a team can be emptied and then deleted", async ({ page }) => { await installMockBridge(page, { personas: [ @@ -2732,6 +2736,15 @@ test("a team can be emptied and then deleted", async ({ page }) => { personaIds: ["custom:deadlock-a", "custom:deadlock-b"], }, ], + managedAgents: [ + { + pubkey: "de".repeat(32), + name: "Deadlock Instance", + personaId: "custom:deadlock-a", + teamId: "team-deadlock", + status: "stopped", + }, + ], }); await gotoApp(page); await page.getByTestId("open-agents-view").click(); @@ -2739,6 +2752,12 @@ test("a team can be emptied and then deleted", async ({ page }) => { const teamCard = page.getByTestId("team-card-team-deadlock"); await expect(teamCard).toBeVisible(); + // The starting state: the agent is bound, so the delete guard refuses. + const blocked = await invokeTauriExpectError(page, "delete_team", { + id: "team-deadlock", + }); + expect(blocked).toContain("still reference it (Deadlock Instance)"); + // Empty the roster via the edit dialog. await page.getByLabel("Deadlock Team team actions").click(); await page.getByRole("menuitem", { name: "Edit" }).click(); @@ -2766,6 +2785,15 @@ test("a team can be emptied and then deleted", async ({ page }) => { teams.find((team) => team.id === "team-deadlock")?.persona_ids, ).toEqual([]); + // The save also cleared the binding. This is what the delete guard reads, and + // it is the state that the save must guarantee before it reports success. + const agents = await invokeTauri< + Array<{ pubkey: string; team_id: string | null }> + >(page, "list_managed_agents"); + expect( + agents.find((agent) => agent.pubkey === "de".repeat(32))?.team_id, + ).toBe(null); + // No agent points to the team now. Thus you can delete the team. await page.getByLabel("Deadlock Team team actions").click(); await page.getByRole("menuitem", { name: "Delete" }).click(); From 772389cc936e3fcef6343391b2bbd11dfcd0a75f Mon Sep 17 00:00:00 2001 From: Storme Drone <49c46e84758b2ebff4abf5abbbd44ee4ce788fc3b55db9fa703eec124eead621@buzz.block.builderlab.xyz> Date: Mon, 24 Aug 2026 20:50:45 -0600 Subject: [PATCH 05/12] test(desktop): match empty-team export errors Co-authored-by: Storme Drone <49c46e84758b2ebff4abf5abbbd44ee4ce788fc3b55db9fa703eec124eead621@buzz.block.builderlab.xyz> Signed-off-by: Storme Drone <49c46e84758b2ebff4abf5abbbd44ee4ce788fc3b55db9fa703eec124eead621@buzz.block.builderlab.xyz> --- desktop/src/testing/e2eBridge.ts | 34 +++++++++++++--- desktop/tests/e2e/team-snapshot.spec.ts | 52 +++++++++++++++++++++++++ 2 files changed, 81 insertions(+), 5 deletions(-) diff --git a/desktop/src/testing/e2eBridge.ts b/desktop/src/testing/e2eBridge.ts index 4e5c18c9251..329bd5b02a0 100644 --- a/desktop/src/testing/e2eBridge.ts +++ b/desktop/src/testing/e2eBridge.ts @@ -8858,10 +8858,15 @@ async function handleDeleteTeam(args: { id: string }): Promise { mockTeams = mockTeams.filter((candidate) => candidate.id !== args.id); } -async function handleExportTeamToJson(args: { id: string }): Promise { - const team = mockTeams.find((candidate) => candidate.id === args.id); +function getMockTeamForExport(id: string): RawTeam { + const team = mockTeams.find((candidate) => candidate.id === id); if (!team) { - throw new Error(`Team ${args.id} not found.`); + throw new Error(`Team ${id} not found.`); + } + if (team.persona_ids.length === 0) { + throw new Error( + "This team has no agents. Add at least one agent before you share or export it.", + ); } const missingPersonaIds = team.persona_ids.filter( @@ -8874,6 +8879,18 @@ async function handleExportTeamToJson(args: { id: string }): Promise { ); } + return team; +} + +async function handleExportTeamToJson(args: { id: string }): Promise { + getMockTeamForExport(args.id); + return true; +} + +async function handleExportTeamSnapshot(args: { + id: string; +}): Promise { + getMockTeamForExport(args.id); return true; } @@ -13020,9 +13037,16 @@ export function maybeInstallE2eTauriMocks() { return importResult; } case "export_team_snapshot": - // Mimics the save-to-disk path: report success without a real dialog. - return true; + return handleExportTeamSnapshot( + payload as Parameters[0], + ); case "encode_team_snapshot_for_send": { + const input = payload as { + id: string; + memoryLevel: "none" | "core" | "everything"; + format: "json" | "png"; + }; + getMockTeamForExport(input.id); // Return a minimal PNG-shaped payload so the send flow can proceed // through upload_media_bytes without a real Rust encode step. const encodeDelayMs = activeConfig?.mock?.encodeDelayMs ?? 0; diff --git a/desktop/tests/e2e/team-snapshot.spec.ts b/desktop/tests/e2e/team-snapshot.spec.ts index 413adc0f3d5..1dbda83c9a0 100644 --- a/desktop/tests/e2e/team-snapshot.spec.ts +++ b/desktop/tests/e2e/team-snapshot.spec.ts @@ -23,6 +23,35 @@ async function readCommandLog(page: import("@playwright/test").Page) { }); } +async function invokeTauriExpectError( + page: import("@playwright/test").Page, + command: string, + payload?: Record, +) { + return page.evaluate( + async ({ targetCommand, targetPayload }) => { + const invoke = ( + window as Window & { + __BUZZ_E2E_INVOKE_MOCK_COMMAND__?: ( + command: string, + payload?: Record, + ) => Promise; + } + ).__BUZZ_E2E_INVOKE_MOCK_COMMAND__; + if (!invoke) { + throw new Error("Mock invoke bridge is unavailable."); + } + try { + await invoke(targetCommand, targetPayload); + return null; + } catch (error) { + return error instanceof Error ? error.message : String(error); + } + }, + { targetCommand: command, targetPayload: payload }, + ); +} + async function gotoAgentsPage(page: import("@playwright/test").Page) { await page.goto("/"); await page.getByTestId("open-agents-view").click(); @@ -60,6 +89,29 @@ const SHARE_TEAM_SEED = { personaIds: [ANALYST_PERSONA_ID], }; +const EMPTY_TEAM_EXPORT_ERROR = + "This team has no agents. Add at least one agent before you share or export it."; + +test("empty teams cannot export or share snapshots through the mock bridge", async ({ + page, +}) => { + await installMockBridge(page); + await gotoAgentsPage(page); + + for (const command of [ + "export_team_to_json", + "export_team_snapshot", + "encode_team_snapshot_for_send", + ]) { + const error = await invokeTauriExpectError(page, command, { + id: "team-engineering-001", + format: "png", + memoryLevel: "none", + }); + expect(error).toBe(EMPTY_TEAM_EXPORT_ERROR); + } +}); + // ── (a) Confirm-fail + retry ──────────────────────────────────────────────── test("team_snapshot_import_confirm_fail_renders_error_and_retry_succeeds", async ({ From b8a44e0bb9fa5cfdb28f5d5859d6c08082039d8f Mon Sep 17 00:00:00 2001 From: Storme Drone <49c46e84758b2ebff4abf5abbbd44ee4ce788fc3b55db9fa703eec124eead621@buzz.block.builderlab.xyz> Date: Tue, 25 Aug 2026 11:48:09 -0600 Subject: [PATCH 06/12] fix(desktop): preserve safe team updates Co-authored-by: Storme Drone <49c46e84758b2ebff4abf5abbbd44ee4ce788fc3b55db9fa703eec124eead621@buzz.block.builderlab.xyz> Signed-off-by: Storme Drone <49c46e84758b2ebff4abf5abbbd44ee4ce788fc3b55db9fa703eec124eead621@buzz.block.builderlab.xyz> --- .../src-tauri/src/commands/personas/mod.rs | 8 +- desktop/src-tauri/src/commands/teams.rs | 494 ++--------------- desktop/src-tauri/src/commands/teams_tests.rs | 522 ++++++++++++++++++ .../src-tauri/src/managed_agents/personas.rs | 14 +- .../src/managed_agents/personas/tests.rs | 76 ++- 5 files changed, 663 insertions(+), 451 deletions(-) create mode 100644 desktop/src-tauri/src/commands/teams_tests.rs diff --git a/desktop/src-tauri/src/commands/personas/mod.rs b/desktop/src-tauri/src/commands/personas/mod.rs index 3be24d04131..dd94207453f 100644 --- a/desktop/src-tauri/src/commands/personas/mod.rs +++ b/desktop/src-tauri/src/commands/personas/mod.rs @@ -4,7 +4,7 @@ use crate::{ app_state::AppState, managed_agents::{ current_instance_id, delete_agent_key, load_managed_agents, load_personas, load_teams, - save_managed_agents, save_personas, stop_managed_agent_process, + save_managed_agents, save_personas, source_team_exists, stop_managed_agent_process, sync_managed_agent_processes, try_regenerate_nest, validate_persona_activation_change, validate_persona_deletion, AgentDefinition, ManagedAgentRecord, }, @@ -129,12 +129,14 @@ pub async fn delete_persona(id: String, app: AppHandle) -> Result<(), String> { .iter() .find(|record| record.id == id) .ok_or_else(|| format!("persona {id} not found"))?; - let referenced_by_team = load_teams(&app)?.iter().any(|team| { + let teams = load_teams(&app)?; + let referenced_by_team = teams.iter().any(|team| { team.persona_ids .iter() .any(|persona_id| persona_id == id.as_str()) }); - validate_persona_deletion(persona, referenced_by_team)?; + let source_team_exists = source_team_exists(persona, &teams); + validate_persona_deletion(persona, referenced_by_team, source_team_exists)?; // Capture the coordinate before the record might leave the list. Only // reached for non-builtin, non-team personas (both rejected above), // so every deleted persona here is one this owner published. diff --git a/desktop/src-tauri/src/commands/teams.rs b/desktop/src-tauri/src/commands/teams.rs index 4e8408ebea2..39d3a0fe14d 100644 --- a/desktop/src-tauri/src/commands/teams.rs +++ b/desktop/src-tauri/src/commands/teams.rs @@ -60,19 +60,41 @@ fn detach_agents_outside_roster( changed } +/// Reports a membership propagation failure. The update command reports a +/// failure for a roster removal or a failed stale-binding detach. Other changes +/// keep the best-effort policy. +pub(in crate::commands) enum MembershipPropagationError { + Load(String), + Save { error: String, detached: bool }, +} + +impl MembershipPropagationError { + fn must_report(&self) -> bool { + matches!(self, Self::Save { detached: true, .. }) + } +} + +impl std::fmt::Display for MembershipPropagationError { + fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Load(error) | Self::Save { error, .. } => formatter.write_str(error), + } + } +} + /// Propagate a team's membership to its members' already-running instances. /// Loads the agent store, applies the roster delta via /// [`apply_team_membership_delta`], reconciles stale bindings via /// [`detach_agents_outside_roster`], and re-saves only when something changed. -/// Reports a load/save failure to the caller, which chooses the policy. +/// Reports the failure type so the caller can choose its error policy. pub(in crate::commands) fn propagate_membership( team_id: &str, previous_persona_ids: &[String], current_persona_ids: &[String], load_agents: impl FnOnce() -> Result, String>, save_agents: impl FnOnce(&[crate::managed_agents::ManagedAgentRecord]) -> Result<(), String>, -) -> Result<(), String> { - let mut records = load_agents()?; +) -> Result<(), MembershipPropagationError> { + let mut records = load_agents().map_err(MembershipPropagationError::Load)?; let delta_changed = apply_team_membership_delta( &mut records, team_id, @@ -81,7 +103,8 @@ pub(in crate::commands) fn propagate_membership( ); let detached = detach_agents_outside_roster(&mut records, team_id, current_persona_ids); if delta_changed || detached { - save_agents(&records)?; + save_agents(&records) + .map_err(|error| MembershipPropagationError::Save { error, detached })?; } Ok(()) } @@ -149,12 +172,17 @@ fn commit_team_create( /// keeps it `AppHandle`-free; a `persist_teams` error propagates. Returns the /// updated team. /// -/// Unlike [`commit_team_create`], an update **reports** a failure of the agent -/// write. A removal clears `team_id` on the removed member. If that write does -/// not land, the agent keeps the binding and `delete_team_with_cascade` refuses -/// the team, so the user gets an empty team that they cannot delete — the exact -/// defect this command must not create. The command must not report success -/// while the delete guard can still refuse. +/// Unlike [`commit_team_create`], an update reports an agent-store failure for +/// a roster removal or a failed stale-binding detach. A removal clears `team_id` +/// on the removed member. If that write does not land, the agent keeps the +/// binding and `delete_team_with_cascade` refuses the team, so the user gets an +/// empty team that they cannot delete — the exact defect this command must not +/// create. The command must not report success while the delete guard can still +/// refuse. +/// +/// When this reports an agent-store error after the team write, `update_team` +/// does not retain the team for relay sync until a retry or the next launch +/// migration runs. /// /// Reporting is safe here because an update is idempotent: it targets an /// existing team by id, so a retry cannot mint a duplicate team. A create has no @@ -188,19 +216,24 @@ fn commit_team_update( let updated = team.clone(); persist_teams(teams)?; - propagate_membership( + let has_removal = previous_persona_ids + .iter() + .any(|persona_id| !updated.persona_ids.contains(persona_id)); + if let Err(error) = propagate_membership( &updated.id, &previous_persona_ids, &updated.persona_ids, load_agents, save_agents, - ) - .map_err(|e| { - format!( - "Saved the team, but could not update its agents: {e}. The team can refuse deletion \ - until this succeeds. Save the team again." - ) - })?; + ) { + if has_removal || error.must_report() { + return Err(format!( + "Saved the team, but could not update its agents: {error}. The team can refuse deletion \ + until this succeeds. Save the team again." + )); + } + eprintln!("buzz-desktop: team-membership-propagate: {error}"); + } Ok(updated) } @@ -468,429 +501,8 @@ pub async fn update_team(input: UpdateTeamRequest, app: AppHandle) -> Result) -> ManagedAgentRecord { - let mut record = serde_json::from_value::(serde_json::json!({ - "pubkey": seed.to_string().repeat(64), - "name": persona_id, - "persona_id": persona_id, - "relay_url": "ws://localhost:3000", - "acp_command": "buzz-acp", - "agent_command": "goose", - "agent_args": [], - "mcp_command": "", - "turn_timeout_seconds": 320, - "system_prompt": "prompt", - "created_at": "2026-01-01T00:00:00Z", - "updated_at": "2026-01-01T00:00:00Z", - })) - .unwrap(); - record.team_id = team_id.map(str::to_string); - record - } - - fn ids(list: &[&str]) -> Vec { - list.iter().map(|s| s.to_string()).collect() - } - - /// A metadata-only edit (no roster change) never re-points an instance — - /// including an unbound instance of a persona this team shares with another. - #[test] - fn metadata_only_edit_leaves_bindings_untouched() { - let mut records = vec![instance('a', "duncan", None)]; - let roster = ids(&["duncan"]); - assert!(!apply_team_membership_delta( - &mut records, - "team-a", - &roster, - &roster - )); - assert_eq!(records[0].team_id, None); - } - - /// Only the *added* persona's unbound instance is bound; an untouched member - /// already present in the previous roster is not re-pointed. - #[test] - fn added_persona_backfills_only_its_unbound_instance() { - let mut records = vec![ - instance('a', "duncan", None), - instance('b', "paul", Some("team-b")), - ]; - assert!(apply_team_membership_delta( - &mut records, - "team-a", - &ids(&["paul"]), - &ids(&["paul", "duncan"]), - )); - assert_eq!(records[0].team_id.as_deref(), Some("team-a")); - // Paul was already on the team and bound elsewhere — untouched. - assert_eq!(records[1].team_id.as_deref(), Some("team-b")); - } - - /// An added persona binds even when shared across teams: an explicit add is - /// legitimate evidence (unlike the boot-repair's order-blind case). - #[test] - fn added_shared_persona_binds_to_the_edited_team() { - let mut records = vec![instance('a', "duncan", None)]; - assert!(apply_team_membership_delta( - &mut records, - "team-a", - &[], - &ids(&["duncan"]), - )); - assert_eq!(records[0].team_id.as_deref(), Some("team-a")); - } - - /// Removing a persona ("keep agents") clears its binding to *this* team so a - /// kept instance stops drawing the team's instructions at spawn. - #[test] - fn removed_persona_detaches_instance_bound_to_this_team() { - let mut records = vec![instance('a', "duncan", Some("team-a"))]; - assert!(apply_team_membership_delta( - &mut records, - "team-a", - &ids(&["duncan"]), - &[], - )); - assert_eq!(records[0].team_id, None); - } - - /// Removal only clears a binding pointing at *this* team — an instance of - /// the same persona bound to a different team is left alone. - #[test] - fn removed_persona_leaves_other_team_binding_untouched() { - let mut records = vec![instance('a', "duncan", Some("team-b"))]; - assert!(!apply_team_membership_delta( - &mut records, - "team-a", - &ids(&["duncan"]), - &[], - )); - assert_eq!(records[0].team_id.as_deref(), Some("team-b")); - } - - /// A minimal owner-authored team record for wiring tests. - fn team(id: &str, persona_ids: &[&str]) -> TeamRecord { - TeamRecord { - id: id.to_string(), - name: id.to_string(), - description: None, - instructions: None, - persona_ids: ids(persona_ids), - is_builtin: false, - source_dir: None, - is_symlink: false, - symlink_target: None, - version: None, - created_at: "2026-01-01T00:00:00Z".to_string(), - updated_at: "2026-01-01T00:00:00Z".to_string(), - } - } - - /// Records the injected store IO a commit performs, so a test can assert - /// the wiring saved (or deliberately did not) the agent store. - #[derive(Default)] - struct StoreSpy { - saved: Option>, - } - - /// Metadata-only `update_team` must pass the TRUE prior roster into the - /// delta, so an unchanged roster is an empty delta and no agent write fires. - /// The `&previous_persona_ids` → `&[]` miswire would drop the prior roster, - /// making the whole roster look "added" and re-pointing the unbound instance. - #[test] - fn commit_team_update_uses_true_prior_roster() { - let mut teams = vec![team("team-a", &["duncan"])]; - let existing = vec![instance('a', "duncan", None)]; - let spy = RefCell::new(StoreSpy::default()); - - let updated = commit_team_update( - &mut teams, - "team-a", - "Team A".to_string(), - None, - Some("new instructions".to_string()), - ids(&["duncan"]), - "2026-02-02T00:00:00Z".to_string(), - |_| Ok(()), - || Ok(existing.clone()), - |records| { - spy.borrow_mut().saved = Some(records.to_vec()); - Ok(()) - }, - ) - .expect("metadata-only update succeeds"); - - assert_eq!(updated.instructions.as_deref(), Some("new instructions")); - // Empty delta ⇒ nothing changed ⇒ no save (the true-prior-roster gate). - assert!( - spy.borrow().saved.is_none(), - "metadata-only edit must not write the agent store" - ); - } - - /// Removing a persona from the roster must reach the detach branch through - /// the command wiring: the instance bound to this team is cleared and saved. - #[test] - fn commit_team_update_removal_detaches_through_wiring() { - let mut teams = vec![team("team-a", &["duncan"])]; - let existing = vec![instance('a', "duncan", Some("team-a"))]; - let spy = RefCell::new(StoreSpy::default()); - - commit_team_update( - &mut teams, - "team-a", - "team-a".to_string(), - None, - None, - ids(&[]), - "2026-02-02T00:00:00Z".to_string(), - |_| Ok(()), - || Ok(existing.clone()), - |records| { - spy.borrow_mut().saved = Some(records.to_vec()); - Ok(()) - }, - ) - .expect("removal update succeeds"); - - let saved = spy.borrow().saved.clone().expect("detach must save"); - assert_eq!(saved[0].team_id, None, "removed persona detaches from team"); - } - - /// `create_team` has no prior roster, so its whole roster is the added delta: - /// the unbound instance of a listed persona is bound through the wiring. - #[test] - fn commit_team_create_treats_full_roster_as_added() { - let mut teams: Vec = Vec::new(); - let existing = vec![instance('a', "duncan", None)]; - let spy = RefCell::new(StoreSpy::default()); - - let created = commit_team_create( - &mut teams, - team("team-a", &["duncan"]), - |_| Ok(()), - || Ok(existing.clone()), - |records| { - spy.borrow_mut().saved = Some(records.to_vec()); - Ok(()) - }, - ) - .expect("create succeeds"); - - assert_eq!(created.id, "team-a"); - let saved = spy.borrow().saved.clone().expect("backfill must save"); - assert_eq!( - saved[0].team_id.as_deref(), - Some("team-a"), - "whole roster is the added delta on create" - ); - } - - /// A failing secondary agent write after successful `save_teams` is - /// swallowed by `create`: it still returns the persisted team. Otherwise a UI - /// retry of a create whose team already landed would mint a duplicate. - #[test] - fn commit_create_returns_ok_when_agent_save_fails() { - let mut teams: Vec = Vec::new(); - let created = commit_team_create( - &mut teams, - team("team-a", &["duncan"]), - |_| Ok(()), - || Ok(vec![instance('a', "duncan", None)]), - |_| Err("disk full".to_string()), - ) - .expect("create swallows secondary-store failure"); - assert_eq!(created.id, "team-a"); - } - - /// `update` must NOT swallow an agent-store failure while emptying a roster. - /// - /// The removal clears `team_id` on the removed member. If that write fails - /// and the command reports success, the team is empty on disk but the agent - /// still points to it, so `delete_team_with_cascade` refuses the team. That - /// is the empty-and-undeletable state this feature exists to remove, so the - /// command must report the failure. The team write itself has landed, and an - /// update is idempotent, so a retry is safe. - #[test] - fn commit_update_reports_agent_save_failure_when_emptying_a_roster() { - let mut teams = vec![team("team-a", &["duncan"])]; - let err = commit_team_update( - &mut teams, - "team-a", - "team-a".to_string(), - None, - None, - ids(&[]), - "2026-02-02T00:00:00Z".to_string(), - |_| Ok(()), - || Ok(vec![instance('a', "duncan", Some("team-a"))]), - |_| Err("disk full".to_string()), - ) - .expect_err("an update must not report success when the detach is lost"); - - assert!(err.contains("could not update its agents"), "{err}"); - assert!(err.contains("Save the team again"), "{err}"); - // The team write is authoritative and already landed. - assert!(teams[0].persona_ids.is_empty()); - } - - /// A retry after a lost detach must repair the binding. - /// - /// This is the recovery path of the test above. The team is already saved - /// empty, so the prior→current delta is empty and a delta-only pass would do - /// nothing. `detach_agents_outside_roster` reconciles against the current - /// roster instead, so saving the same empty roster again still clears the - /// stale `team_id` and makes the team deletable. - #[test] - fn resaving_an_already_empty_roster_repairs_a_lost_detach() { - let mut teams = vec![team("team-a", &[])]; - let spy = RefCell::new(StoreSpy::default()); - - commit_team_update( - &mut teams, - "team-a", - "team-a".to_string(), - None, - None, - ids(&[]), - "2026-02-03T00:00:00Z".to_string(), - |_| Ok(()), - // The agent kept its binding because the earlier write was lost. - || Ok(vec![instance('a', "duncan", Some("team-a"))]), - |records| { - spy.borrow_mut().saved = Some(records.to_vec()); - Ok(()) - }, - ) - .expect("the retry succeeds"); - - let saved = spy - .borrow() - .saved - .clone() - .expect("the retry must write the agent store"); - assert_eq!( - saved[0].team_id, None, - "a stale binding is cleared against the current roster, not a delta" - ); - } - - /// End to end at the command seam, measured by the real delete guard. - /// - /// This test starts with a bound agent and empties the roster. It applies - /// `agents_referencing_team` — the predicate `delete_team_with_cascade` uses - /// — to the agent store that the command left behind. It pins both halves of - /// the contract: - /// - /// 1. The failed save reports an error. It never claims success while the - /// delete guard still sees the agent. - /// 2. The retry succeeds, and the guard then sees no agent. Delete is - /// possible without an app restart. - #[test] - fn update_never_reports_success_while_the_delete_guard_sees_the_agent() { - let mut teams = vec![team("team-a", &["duncan"])]; - // The store on disk. A failed save leaves it as it was. - let store = RefCell::new(vec![instance('a', "duncan", Some("team-a"))]); - - // Attempt 1: the agent write fails. - let err = commit_team_update( - &mut teams, - "team-a", - "team-a".to_string(), - None, - None, - ids(&[]), - "2026-02-02T00:00:00Z".to_string(), - |_| Ok(()), - || Ok(store.borrow().clone()), - |_| Err("disk full".to_string()), - ) - .expect_err("the command must report the lost detach"); - assert!(err.contains("could not update its agents"), "{err}"); - - // The team is empty on disk, but the guard still refuses deletion. The - // command reported this state instead of hiding it. - assert!(teams[0].persona_ids.is_empty()); - assert_eq!( - crate::managed_agents::agents_referencing_team(&store.borrow(), &teams[0]), - vec!["duncan"], - "the guard still sees the agent, so the report was required" - ); - - // Attempt 2: the same save, and now the write lands. - commit_team_update( - &mut teams, - "team-a", - "team-a".to_string(), - None, - None, - ids(&[]), - "2026-02-03T00:00:00Z".to_string(), - |_| Ok(()), - || Ok(store.borrow().clone()), - |records| { - *store.borrow_mut() = records.to_vec(); - Ok(()) - }, - ) - .expect("the retry succeeds"); - - assert!( - crate::managed_agents::agents_referencing_team(&store.borrow(), &teams[0]).is_empty(), - "the retry must make the team deletable" - ); - } - - /// The reconcile is scoped: it clears a binding to *this* team only, and it - /// leaves an instance whose persona is still on the roster alone. - #[test] - fn detach_outside_roster_is_scoped_to_this_team_and_absent_personas() { - let mut records = vec![ - instance('a', "duncan", Some("team-a")), - instance('b', "paul", Some("team-b")), - instance('c', "ada", Some("team-a")), - ]; - - assert!(detach_agents_outside_roster( - &mut records, - "team-a", - &ids(&["ada"]), - )); - - assert_eq!(records[0].team_id, None, "absent from this team's roster"); - assert_eq!( - records[1].team_id.as_deref(), - Some("team-b"), - "another team's binding is untouched" - ); - assert_eq!( - records[2].team_id.as_deref(), - Some("team-a"), - "still on the roster, so the binding stays" - ); - } - - /// A roster with every binding already correct writes nothing. - #[test] - fn detach_outside_roster_is_inert_when_nothing_is_stale() { - let mut records = vec![instance('a', "duncan", Some("team-a"))]; - assert!(!detach_agents_outside_roster( - &mut records, - "team-a", - &ids(&["duncan"]), - )); - assert_eq!(records[0].team_id.as_deref(), Some("team-a")); - } -} +#[path = "teams_tests.rs"] +mod tests; #[tauri::command] pub async fn delete_team(id: String, app: AppHandle) -> Result<(), String> { diff --git a/desktop/src-tauri/src/commands/teams_tests.rs b/desktop/src-tauri/src/commands/teams_tests.rs new file mode 100644 index 00000000000..893a56c193a --- /dev/null +++ b/desktop/src-tauri/src/commands/teams_tests.rs @@ -0,0 +1,522 @@ +//! Tests for team membership updates. +//! +//! Kept in a sibling file so `teams.rs` stays under the file-size limit. + +use super::{ + apply_team_membership_delta, commit_team_create, commit_team_update, + detach_agents_outside_roster, +}; +use crate::managed_agents::{ManagedAgentRecord, TeamRecord}; +use std::cell::RefCell; + +/// A running instance: `pubkey` set, linked to a persona, optional binding. +fn instance(seed: char, persona_id: &str, team_id: Option<&str>) -> ManagedAgentRecord { + let mut record = serde_json::from_value::(serde_json::json!({ + "pubkey": seed.to_string().repeat(64), + "name": persona_id, + "persona_id": persona_id, + "relay_url": "ws://localhost:3000", + "acp_command": "buzz-acp", + "agent_command": "goose", + "agent_args": [], + "mcp_command": "", + "turn_timeout_seconds": 320, + "system_prompt": "prompt", + "created_at": "2026-01-01T00:00:00Z", + "updated_at": "2026-01-01T00:00:00Z", + })) + .unwrap(); + record.team_id = team_id.map(str::to_string); + record +} + +fn ids(list: &[&str]) -> Vec { + list.iter().map(|s| s.to_string()).collect() +} + +/// A metadata-only edit (no roster change) never re-points an instance — +/// including an unbound instance of a persona this team shares with another. +#[test] +fn metadata_only_edit_leaves_bindings_untouched() { + let mut records = vec![instance('a', "duncan", None)]; + let roster = ids(&["duncan"]); + assert!(!apply_team_membership_delta( + &mut records, + "team-a", + &roster, + &roster + )); + assert_eq!(records[0].team_id, None); +} + +/// Only the *added* persona's unbound instance is bound; an untouched member +/// already present in the previous roster is not re-pointed. +#[test] +fn added_persona_backfills_only_its_unbound_instance() { + let mut records = vec![ + instance('a', "duncan", None), + instance('b', "paul", Some("team-b")), + ]; + assert!(apply_team_membership_delta( + &mut records, + "team-a", + &ids(&["paul"]), + &ids(&["paul", "duncan"]), + )); + assert_eq!(records[0].team_id.as_deref(), Some("team-a")); + // Paul was already on the team and bound elsewhere — untouched. + assert_eq!(records[1].team_id.as_deref(), Some("team-b")); +} + +/// An added persona binds even when shared across teams: an explicit add is +/// legitimate evidence (unlike the boot-repair's order-blind case). +#[test] +fn added_shared_persona_binds_to_the_edited_team() { + let mut records = vec![instance('a', "duncan", None)]; + assert!(apply_team_membership_delta( + &mut records, + "team-a", + &[], + &ids(&["duncan"]), + )); + assert_eq!(records[0].team_id.as_deref(), Some("team-a")); +} + +/// Removing a persona ("keep agents") clears its binding to *this* team so a +/// kept instance stops drawing the team's instructions at spawn. +#[test] +fn removed_persona_detaches_instance_bound_to_this_team() { + let mut records = vec![instance('a', "duncan", Some("team-a"))]; + assert!(apply_team_membership_delta( + &mut records, + "team-a", + &ids(&["duncan"]), + &[], + )); + assert_eq!(records[0].team_id, None); +} + +/// Removal only clears a binding pointing at *this* team — an instance of +/// the same persona bound to a different team is left alone. +#[test] +fn removed_persona_leaves_other_team_binding_untouched() { + let mut records = vec![instance('a', "duncan", Some("team-b"))]; + assert!(!apply_team_membership_delta( + &mut records, + "team-a", + &ids(&["duncan"]), + &[], + )); + assert_eq!(records[0].team_id.as_deref(), Some("team-b")); +} + +/// A minimal owner-authored team record for wiring tests. +fn team(id: &str, persona_ids: &[&str]) -> TeamRecord { + TeamRecord { + id: id.to_string(), + name: id.to_string(), + description: None, + instructions: None, + persona_ids: ids(persona_ids), + is_builtin: false, + source_dir: None, + is_symlink: false, + symlink_target: None, + version: None, + created_at: "2026-01-01T00:00:00Z".to_string(), + updated_at: "2026-01-01T00:00:00Z".to_string(), + } +} + +/// Records the injected store IO a commit performs, so a test can assert +/// the wiring saved (or deliberately did not) the agent store. +#[derive(Default)] +struct StoreSpy { + saved: Option>, +} + +/// Metadata-only `update_team` must pass the TRUE prior roster into the +/// delta, so an unchanged roster is an empty delta and no agent write fires. +/// The `&previous_persona_ids` → `&[]` miswire would drop the prior roster, +/// making the whole roster look "added" and re-pointing the unbound instance. +#[test] +fn commit_team_update_uses_true_prior_roster() { + let mut teams = vec![team("team-a", &["duncan"])]; + let existing = vec![instance('a', "duncan", None)]; + let spy = RefCell::new(StoreSpy::default()); + + let updated = commit_team_update( + &mut teams, + "team-a", + "Team A".to_string(), + None, + Some("new instructions".to_string()), + ids(&["duncan"]), + "2026-02-02T00:00:00Z".to_string(), + |_| Ok(()), + || Ok(existing.clone()), + |records| { + spy.borrow_mut().saved = Some(records.to_vec()); + Ok(()) + }, + ) + .expect("metadata-only update succeeds"); + + assert_eq!(updated.instructions.as_deref(), Some("new instructions")); + // Empty delta ⇒ nothing changed ⇒ no save (the true-prior-roster gate). + assert!( + spy.borrow().saved.is_none(), + "metadata-only edit must not write the agent store" + ); +} + +/// A metadata-only update keeps its disk-authoritative result when the +/// agent store cannot load. Boot repair restores any missing backfill. +#[test] +fn commit_update_ignores_agent_load_failure_for_metadata_only_edit() { + let mut teams = vec![team("team-a", &["duncan"])]; + + let updated = commit_team_update( + &mut teams, + "team-a", + "Renamed team".to_string(), + None, + None, + ids(&["duncan"]), + "2026-02-02T00:00:00Z".to_string(), + |_| Ok(()), + || Err("corrupt managed-agents.json".to_string()), + |_| Ok(()), + ) + .expect("metadata-only update keeps the best-effort policy"); + + assert_eq!(updated.name, "Renamed team"); + assert_eq!(teams[0].name, "Renamed team"); +} + +/// An add-only update also keeps its disk-authoritative result when the +/// agent store cannot load. A missed bind does not block team deletion. +#[test] +fn commit_update_ignores_agent_load_failure_for_add_only_edit() { + let mut teams = vec![team("team-a", &["duncan"])]; + + let updated = commit_team_update( + &mut teams, + "team-a", + "team-a".to_string(), + None, + None, + ids(&["duncan", "ada"]), + "2026-02-02T00:00:00Z".to_string(), + |_| Ok(()), + || Err("corrupt managed-agents.json".to_string()), + |_| Ok(()), + ) + .expect("add-only update keeps the best-effort policy"); + + assert_eq!(updated.persona_ids, ids(&["duncan", "ada"])); + assert_eq!(teams[0].persona_ids, ids(&["duncan", "ada"])); +} + +/// A roster removal remains strict when the agent store cannot load. The +/// command cannot prove that it cleared stale bindings in that case. +#[test] +fn commit_update_reports_agent_load_failure_for_removal() { + let mut teams = vec![team("team-a", &["duncan"])]; + + let err = commit_team_update( + &mut teams, + "team-a", + "team-a".to_string(), + None, + None, + ids(&[]), + "2026-02-02T00:00:00Z".to_string(), + |_| Ok(()), + || Err("corrupt managed-agents.json".to_string()), + |_| Ok(()), + ) + .expect_err("a removal must report an agent-store load failure"); + + assert!(err.contains("could not update its agents"), "{err}"); + assert!(teams[0].persona_ids.is_empty()); +} + +/// A stale binding makes an otherwise metadata-only update strict. The +/// command must report a failed detach because the delete guard still sees +/// the agent after the team write. +#[test] +fn commit_update_reports_agent_save_failure_for_stale_detach() { + let mut teams = vec![team("team-a", &[])]; + + let err = commit_team_update( + &mut teams, + "team-a", + "Renamed team".to_string(), + None, + None, + ids(&[]), + "2026-02-02T00:00:00Z".to_string(), + |_| Ok(()), + || Ok(vec![instance('a', "duncan", Some("team-a"))]), + |_| Err("disk full".to_string()), + ) + .expect_err("a failed stale detach must not report success"); + + assert!(err.contains("could not update its agents"), "{err}"); + assert_eq!(teams[0].name, "Renamed team"); +} + +/// Removing a persona from the roster must reach the detach branch through +/// the command wiring: the instance bound to this team is cleared and saved. +#[test] +fn commit_team_update_removal_detaches_through_wiring() { + let mut teams = vec![team("team-a", &["duncan"])]; + let existing = vec![instance('a', "duncan", Some("team-a"))]; + let spy = RefCell::new(StoreSpy::default()); + + commit_team_update( + &mut teams, + "team-a", + "team-a".to_string(), + None, + None, + ids(&[]), + "2026-02-02T00:00:00Z".to_string(), + |_| Ok(()), + || Ok(existing.clone()), + |records| { + spy.borrow_mut().saved = Some(records.to_vec()); + Ok(()) + }, + ) + .expect("removal update succeeds"); + + let saved = spy.borrow().saved.clone().expect("detach must save"); + assert_eq!(saved[0].team_id, None, "removed persona detaches from team"); +} + +/// `create_team` has no prior roster, so its whole roster is the added delta: +/// the unbound instance of a listed persona is bound through the wiring. +#[test] +fn commit_team_create_treats_full_roster_as_added() { + let mut teams: Vec = Vec::new(); + let existing = vec![instance('a', "duncan", None)]; + let spy = RefCell::new(StoreSpy::default()); + + let created = commit_team_create( + &mut teams, + team("team-a", &["duncan"]), + |_| Ok(()), + || Ok(existing.clone()), + |records| { + spy.borrow_mut().saved = Some(records.to_vec()); + Ok(()) + }, + ) + .expect("create succeeds"); + + assert_eq!(created.id, "team-a"); + let saved = spy.borrow().saved.clone().expect("backfill must save"); + assert_eq!( + saved[0].team_id.as_deref(), + Some("team-a"), + "whole roster is the added delta on create" + ); +} + +/// A failing secondary agent write after successful `save_teams` is +/// swallowed by `create`: it still returns the persisted team. Otherwise a UI +/// retry of a create whose team already landed would mint a duplicate. +#[test] +fn commit_create_returns_ok_when_agent_save_fails() { + let mut teams: Vec = Vec::new(); + let created = commit_team_create( + &mut teams, + team("team-a", &["duncan"]), + |_| Ok(()), + || Ok(vec![instance('a', "duncan", None)]), + |_| Err("disk full".to_string()), + ) + .expect("create swallows secondary-store failure"); + assert_eq!(created.id, "team-a"); +} + +/// `update` must NOT swallow an agent-store failure while emptying a roster. +/// +/// The removal clears `team_id` on the removed member. If that write fails +/// and the command reports success, the team is empty on disk but the agent +/// still points to it, so `delete_team_with_cascade` refuses the team. That +/// is the empty-and-undeletable state this feature exists to remove, so the +/// command must report the failure. The team write itself has landed, and an +/// update is idempotent, so a retry is safe. +#[test] +fn commit_update_reports_agent_save_failure_when_emptying_a_roster() { + let mut teams = vec![team("team-a", &["duncan"])]; + let err = commit_team_update( + &mut teams, + "team-a", + "team-a".to_string(), + None, + None, + ids(&[]), + "2026-02-02T00:00:00Z".to_string(), + |_| Ok(()), + || Ok(vec![instance('a', "duncan", Some("team-a"))]), + |_| Err("disk full".to_string()), + ) + .expect_err("an update must not report success when the detach is lost"); + + assert!(err.contains("could not update its agents"), "{err}"); + assert!(err.contains("Save the team again"), "{err}"); + // The team write is authoritative and already landed. + assert!(teams[0].persona_ids.is_empty()); +} + +/// A retry after a lost detach must repair the binding. +/// +/// This is the recovery path of the test above. The team is already saved +/// empty, so the prior→current delta is empty and a delta-only pass would do +/// nothing. `detach_agents_outside_roster` reconciles against the current +/// roster instead, so saving the same empty roster again still clears the +/// stale `team_id` and makes the team deletable. +#[test] +fn resaving_an_already_empty_roster_repairs_a_lost_detach() { + let mut teams = vec![team("team-a", &[])]; + let spy = RefCell::new(StoreSpy::default()); + + commit_team_update( + &mut teams, + "team-a", + "team-a".to_string(), + None, + None, + ids(&[]), + "2026-02-03T00:00:00Z".to_string(), + |_| Ok(()), + // The agent kept its binding because the earlier write was lost. + || Ok(vec![instance('a', "duncan", Some("team-a"))]), + |records| { + spy.borrow_mut().saved = Some(records.to_vec()); + Ok(()) + }, + ) + .expect("the retry succeeds"); + + let saved = spy + .borrow() + .saved + .clone() + .expect("the retry must write the agent store"); + assert_eq!( + saved[0].team_id, None, + "a stale binding is cleared against the current roster, not a delta" + ); +} + +/// End to end at the command seam, measured by the real delete guard. +/// +/// This test starts with a bound agent and empties the roster. It applies +/// `agents_referencing_team` — the predicate `delete_team_with_cascade` uses +/// — to the agent store that the command left behind. It pins both halves of +/// the contract: +/// +/// 1. The failed save reports an error. It never claims success while the +/// delete guard still sees the agent. +/// 2. The retry succeeds, and the guard then sees no agent. Delete is +/// possible without an app restart. +#[test] +fn update_never_reports_success_while_the_delete_guard_sees_the_agent() { + let mut teams = vec![team("team-a", &["duncan"])]; + // The store on disk. A failed save leaves it as it was. + let store = RefCell::new(vec![instance('a', "duncan", Some("team-a"))]); + + // Attempt 1: the agent write fails. + let err = commit_team_update( + &mut teams, + "team-a", + "team-a".to_string(), + None, + None, + ids(&[]), + "2026-02-02T00:00:00Z".to_string(), + |_| Ok(()), + || Ok(store.borrow().clone()), + |_| Err("disk full".to_string()), + ) + .expect_err("the command must report the lost detach"); + assert!(err.contains("could not update its agents"), "{err}"); + + // The team is empty on disk, but the guard still refuses deletion. The + // command reported this state instead of hiding it. + assert!(teams[0].persona_ids.is_empty()); + assert_eq!( + crate::managed_agents::agents_referencing_team(&store.borrow(), &teams[0]), + vec!["duncan"], + "the guard still sees the agent, so the report was required" + ); + + // Attempt 2: the same save, and now the write lands. + commit_team_update( + &mut teams, + "team-a", + "team-a".to_string(), + None, + None, + ids(&[]), + "2026-02-03T00:00:00Z".to_string(), + |_| Ok(()), + || Ok(store.borrow().clone()), + |records| { + *store.borrow_mut() = records.to_vec(); + Ok(()) + }, + ) + .expect("the retry succeeds"); + + assert!( + crate::managed_agents::agents_referencing_team(&store.borrow(), &teams[0]).is_empty(), + "the retry must make the team deletable" + ); +} + +/// The reconcile is scoped: it clears a binding to *this* team only, and it +/// leaves an instance whose persona is still on the roster alone. +#[test] +fn detach_outside_roster_is_scoped_to_this_team_and_absent_personas() { + let mut records = vec![ + instance('a', "duncan", Some("team-a")), + instance('b', "paul", Some("team-b")), + instance('c', "ada", Some("team-a")), + ]; + + assert!(detach_agents_outside_roster( + &mut records, + "team-a", + &ids(&["ada"]), + )); + + assert_eq!(records[0].team_id, None, "absent from this team's roster"); + assert_eq!( + records[1].team_id.as_deref(), + Some("team-b"), + "another team's binding is untouched" + ); + assert_eq!( + records[2].team_id.as_deref(), + Some("team-a"), + "still on the roster, so the binding stays" + ); +} + +/// A roster with every binding already correct writes nothing. +#[test] +fn detach_outside_roster_is_inert_when_nothing_is_stale() { + let mut records = vec![instance('a', "duncan", Some("team-a"))]; + assert!(!detach_agents_outside_roster( + &mut records, + "team-a", + &ids(&["duncan"]), + )); + assert_eq!(records[0].team_id.as_deref(), Some("team-a")); +} diff --git a/desktop/src-tauri/src/managed_agents/personas.rs b/desktop/src-tauri/src/managed_agents/personas.rs index 8ff0e633dc8..9ae0cf40780 100644 --- a/desktop/src-tauri/src/managed_agents/personas.rs +++ b/desktop/src-tauri/src/managed_agents/personas.rs @@ -283,15 +283,27 @@ pub fn ensure_persona_ids_are_active( Ok(()) } +pub(crate) fn source_team_exists( + persona: &AgentDefinition, + teams: &[crate::managed_agents::TeamRecord], +) -> bool { + persona.source_team.as_deref().is_some_and(|source_team| { + teams + .iter() + .any(|team| crate::managed_agents::team_persona_key(team) == source_team) + }) +} + pub fn validate_persona_deletion( persona: &AgentDefinition, referenced_by_team: bool, + source_team_exists: bool, ) -> Result<(), String> { if persona.is_builtin { return Err("Built-in agents cannot be deleted.".to_string()); } - if persona.source_team.is_some() { + if source_team_exists { return Err(format!( "{} belongs to a team. Delete the team to remove all team agents together.", persona.display_name diff --git a/desktop/src-tauri/src/managed_agents/personas/tests.rs b/desktop/src-tauri/src/managed_agents/personas/tests.rs index cc21861a9f3..011716473a1 100644 --- a/desktop/src-tauri/src/managed_agents/personas/tests.rs +++ b/desktop/src-tauri/src/managed_agents/personas/tests.rs @@ -1,10 +1,12 @@ use super::{ built_in_persona_records, ensure_persona_ids_are_active, ensure_persona_is_active, - merge_personas, migrate_retired_personas, validate_persona_activation_change, - validate_persona_deletion, BUILT_IN_PERSONAS, RETIRED_PERSONAS, + merge_personas, migrate_retired_personas, source_team_exists, + validate_persona_activation_change, validate_persona_deletion, BUILT_IN_PERSONAS, + RETIRED_PERSONAS, }; use crate::managed_agents::discovery::{default_agent_command, effective_agent_command}; -use crate::managed_agents::AgentDefinition; +use crate::managed_agents::{AgentDefinition, TeamRecord}; +use std::path::PathBuf; fn custom_persona(id: &str, display_name: &str) -> AgentDefinition { AgentDefinition { @@ -31,6 +33,47 @@ fn custom_persona(id: &str, display_name: &str) -> AgentDefinition { } } +fn team(id: &str, source_dir: Option<&str>) -> TeamRecord { + TeamRecord { + id: id.to_string(), + name: id.to_string(), + description: None, + instructions: None, + persona_ids: Vec::new(), + is_builtin: false, + source_dir: source_dir.map(PathBuf::from), + is_symlink: false, + symlink_target: None, + version: None, + created_at: "2026-03-19T00:00:00Z".to_string(), + updated_at: "2026-03-19T00:00:00Z".to_string(), + } +} + +#[test] +fn source_team_exists_uses_the_team_persona_key() { + let mut persona = custom_persona("custom:alpha", "Alpha"); + persona.source_team = Some("com.example.alpha".to_string()); + let teams = vec![team( + "legacy-team-uuid", + Some("/managed-teams/com.example.alpha"), + )]; + + assert!(source_team_exists(&persona, &teams)); +} + +#[test] +fn source_team_exists_rejects_a_missing_team() { + let mut persona = custom_persona("custom:alpha", "Alpha"); + persona.source_team = Some("com.example.deleted".to_string()); + let teams = vec![team( + "legacy-team-uuid", + Some("/managed-teams/com.example.alpha"), + )]; + + assert!(!source_team_exists(&persona, &teams)); +} + #[test] fn merge_personas_adds_missing_built_ins() { let (records, changed) = merge_personas(Vec::new(), "2026-03-19T00:00:00Z"); @@ -242,7 +285,7 @@ fn validate_persona_deletion_rejects_builtins() { let mut persona = custom_persona("builtin:fizz", "Fizz"); persona.is_builtin = true; - let err = validate_persona_deletion(&persona, false).unwrap_err(); + let err = validate_persona_deletion(&persona, false, false).unwrap_err(); assert_eq!(err, "Built-in agents cannot be deleted."); } @@ -251,7 +294,7 @@ fn validate_persona_deletion_rejects_builtins() { fn validate_persona_deletion_rejects_team_references() { let persona = custom_persona("custom:alpha", "Alpha"); - let err = validate_persona_deletion(&persona, true).unwrap_err(); + let err = validate_persona_deletion(&persona, true, false).unwrap_err(); assert_eq!( err, @@ -263,7 +306,28 @@ fn validate_persona_deletion_rejects_team_references() { fn validate_persona_deletion_allows_safe_custom_personas() { let persona = custom_persona("custom:alpha", "Alpha"); - assert!(validate_persona_deletion(&persona, false).is_ok()); + assert!(validate_persona_deletion(&persona, false, false).is_ok()); +} + +#[test] +fn validate_persona_deletion_rejects_existing_source_team() { + let mut persona = custom_persona("custom:alpha", "Alpha"); + persona.source_team = Some("team:alpha".to_string()); + + let err = validate_persona_deletion(&persona, false, true).unwrap_err(); + + assert_eq!( + err, + "Alpha belongs to a team. Delete the team to remove all team agents together." + ); +} + +#[test] +fn validate_persona_deletion_allows_missing_source_team() { + let mut persona = custom_persona("custom:alpha", "Alpha"); + persona.source_team = Some("team:deleted".to_string()); + + assert!(validate_persona_deletion(&persona, false, false).is_ok()); } // ── migrate_retired_personas ────────────────────────────────────────────────── From 5ade68f6e1886494c601e5760ab2c9e5400f15cd Mon Sep 17 00:00:00 2001 From: Storme Drone <49c46e84758b2ebff4abf5abbbd44ee4ce788fc3b55db9fa703eec124eead621@buzz.block.builderlab.xyz> Date: Tue, 25 Aug 2026 12:30:31 -0600 Subject: [PATCH 07/12] fix(desktop): detach persona-less agents Co-authored-by: Storme Drone <49c46e84758b2ebff4abf5abbbd44ee4ce788fc3b55db9fa703eec124eead621@buzz.block.builderlab.xyz> Signed-off-by: Storme Drone <49c46e84758b2ebff4abf5abbbd44ee4ce788fc3b55db9fa703eec124eead621@buzz.block.builderlab.xyz> --- desktop/src-tauri/src/commands/teams.rs | 12 ++--- desktop/src-tauri/src/commands/teams_tests.rs | 47 ++++++++++++++++++- 2 files changed, 51 insertions(+), 8 deletions(-) diff --git a/desktop/src-tauri/src/commands/teams.rs b/desktop/src-tauri/src/commands/teams.rs index 39d3a0fe14d..dd52e934a4e 100644 --- a/desktop/src-tauri/src/commands/teams.rs +++ b/desktop/src-tauri/src/commands/teams.rs @@ -27,8 +27,8 @@ fn trim_optional(value: Option) -> Option { } /// Clear `team_id` on every instance that is bound to `team_id` but whose -/// persona is absent from `current_persona_ids`. Reports whether anything -/// changed. +/// persona is absent from `current_persona_ids` or unset. Reports whether +/// anything changed. /// /// This reads the current roster, not a delta. That is what makes a failed /// detach recoverable: after a failed agent-store write the team is already @@ -49,10 +49,10 @@ fn detach_agents_outside_roster( if record.pubkey.is_empty() || record.team_id.as_deref() != Some(team_id) { continue; } - let Some(persona_id) = record.persona_id.as_deref() else { - continue; - }; - if !current_persona_ids.iter().any(|id| id == persona_id) { + if !current_persona_ids + .iter() + .any(|id| record.persona_id.as_deref() == Some(id)) + { record.team_id = None; changed = true; } diff --git a/desktop/src-tauri/src/commands/teams_tests.rs b/desktop/src-tauri/src/commands/teams_tests.rs index 893a56c193a..f6a50a99885 100644 --- a/desktop/src-tauri/src/commands/teams_tests.rs +++ b/desktop/src-tauri/src/commands/teams_tests.rs @@ -30,6 +30,12 @@ fn instance(seed: char, persona_id: &str, team_id: Option<&str>) -> ManagedAgent record } +fn instance_without_persona(seed: char, team_id: Option<&str>) -> ManagedAgentRecord { + let mut record = instance(seed, "unassigned", team_id); + record.persona_id = None; + record +} + fn ids(list: &[&str]) -> Vec { list.iter().map(|s| s.to_string()).collect() } @@ -480,14 +486,47 @@ fn update_never_reports_success_while_the_delete_guard_sees_the_agent() { ); } -/// The reconcile is scoped: it clears a binding to *this* team only, and it -/// leaves an instance whose persona is still on the roster alone. +/// This test starts with a bound persona-less agent and empties the roster. It +/// applies `agents_referencing_team` — the predicate `delete_team_with_cascade` +/// uses — to the agent store that the update saved. The update must clear this +/// direct-command record because the delete guard does not require a persona. +#[test] +fn emptying_a_roster_detaches_a_bound_persona_less_agent() { + let mut teams = vec![team("team-a", &["duncan"])]; + let store = RefCell::new(vec![instance_without_persona('a', Some("team-a"))]); + + commit_team_update( + &mut teams, + "team-a", + "team-a".to_string(), + None, + None, + ids(&[]), + "2026-02-02T00:00:00Z".to_string(), + |_| Ok(()), + || Ok(store.borrow().clone()), + |records| { + *store.borrow_mut() = records.to_vec(); + Ok(()) + }, + ) + .expect("the update must detach a bound persona-less agent"); + + assert!( + crate::managed_agents::agents_referencing_team(&store.borrow(), &teams[0]).is_empty(), + "the delete guard must not see the detached agent" + ); +} + +/// The reconcile is scoped: it clears a binding to *this* team when the +/// persona is absent or unset, and it leaves a listed persona alone. #[test] fn detach_outside_roster_is_scoped_to_this_team_and_absent_personas() { let mut records = vec![ instance('a', "duncan", Some("team-a")), instance('b', "paul", Some("team-b")), instance('c', "ada", Some("team-a")), + instance_without_persona('d', Some("team-a")), ]; assert!(detach_agents_outside_roster( @@ -497,6 +536,10 @@ fn detach_outside_roster_is_scoped_to_this_team_and_absent_personas() { )); assert_eq!(records[0].team_id, None, "absent from this team's roster"); + assert_eq!( + records[3].team_id, None, + "an unset persona cannot remain bound to this team" + ); assert_eq!( records[1].team_id.as_deref(), Some("team-b"), From fcf83ea24de99fa86d44393c175c111f104687c9 Mon Sep 17 00:00:00 2001 From: Storme Drone <49c46e84758b2ebff4abf5abbbd44ee4ce788fc3b55db9fa703eec124eead621@buzz.block.builderlab.xyz> Date: Wed, 26 Aug 2026 15:28:58 -0600 Subject: [PATCH 08/12] fix(desktop): replay failed team membership updates Signed-off-by: Storme Drone <49c46e84758b2ebff4abf5abbbd44ee4ce788fc3b55db9fa703eec124eead621@buzz.block.builderlab.xyz> --- desktop/src-tauri/src/commands/mod.rs | 1 + desktop/src-tauri/src/commands/teams.rs | 164 ++++++++++++++---- desktop/src-tauri/src/commands/teams_tests.rs | 74 +++++++- desktop/src-tauri/src/migration.rs | 14 ++ desktop/src-tauri/src/migration_tests.rs | 15 +- 5 files changed, 223 insertions(+), 45 deletions(-) diff --git a/desktop/src-tauri/src/commands/mod.rs b/desktop/src-tauri/src/commands/mod.rs index 7cb2d8e3b83..37d5dc5a198 100644 --- a/desktop/src-tauri/src/commands/mod.rs +++ b/desktop/src-tauri/src/commands/mod.rs @@ -120,6 +120,7 @@ pub use relay_members::*; pub use relay_reconnect::*; pub use social::*; pub use team_snapshot::*; +pub(crate) use teams::replay_pending_team_membership; pub use teams::*; pub use updater::*; pub use window_chrome::*; diff --git a/desktop/src-tauri/src/commands/teams.rs b/desktop/src-tauri/src/commands/teams.rs index dd52e934a4e..fdf61fd53f7 100644 --- a/desktop/src-tauri/src/commands/teams.rs +++ b/desktop/src-tauri/src/commands/teams.rs @@ -1,3 +1,6 @@ +use std::path::PathBuf; + +use serde::{Deserialize, Serialize}; use tauri::AppHandle; use uuid::Uuid; @@ -26,6 +29,98 @@ fn trim_optional(value: Option) -> Option { }) } +/// A staged team update. The record persists before the two stores change so a +/// later save or launch can replay the original membership delta. +#[derive(Debug, Clone, Serialize, Deserialize)] +struct PendingTeamMembershipUpdate { + team_id: String, + previous_persona_ids: Vec, + current_persona_ids: Vec, +} + +fn pending_team_membership_path(app: &AppHandle) -> Result { + Ok(crate::managed_agents::managed_agents_base_dir(app)?.join("pending-team-membership.json")) +} + +fn save_pending_team_membership( + app: &AppHandle, + pending: &PendingTeamMembershipUpdate, +) -> Result<(), String> { + let path = pending_team_membership_path(app)?; + let payload = serde_json::to_vec_pretty(pending) + .map_err(|error| format!("failed to serialize pending team update: {error}"))?; + crate::managed_agents::storage::atomic_write_json(&path, &payload) +} + +fn load_pending_team_membership( + app: &AppHandle, +) -> Result, String> { + let path = pending_team_membership_path(app)?; + if !path.exists() { + return Ok(None); + } + let payload = std::fs::read_to_string(&path) + .map_err(|error| format!("failed to read pending team update: {error}"))?; + serde_json::from_str(&payload) + .map(Some) + .map_err(|error| format!("failed to parse pending team update: {error}")) +} + +fn clear_pending_team_membership(app: &AppHandle) -> Result<(), String> { + let path = pending_team_membership_path(app)?; + match std::fs::remove_file(&path) { + Ok(()) => Ok(()), + Err(error) if error.kind() == std::io::ErrorKind::NotFound => Ok(()), + Err(error) => Err(format!("failed to clear pending team update: {error}")), + } +} + +enum PendingTeamMembershipState { + Pending, + Superseded, +} + +fn pending_team_membership_state( + pending: &PendingTeamMembershipUpdate, + teams: &[TeamRecord], +) -> Result { + let team = teams + .iter() + .find(|team| team.id == pending.team_id) + .ok_or_else(|| format!("pending team {} no longer exists", pending.team_id))?; + if team.persona_ids == pending.current_persona_ids { + Ok(PendingTeamMembershipState::Pending) + } else if team.persona_ids == pending.previous_persona_ids { + Ok(PendingTeamMembershipState::Superseded) + } else { + Err(format!( + "pending team {} has an unexpected roster; save the team again", + pending.team_id + )) + } +} + +/// Replay a staged membership delta. Callers hold `managed_agents_store_lock`. +pub(crate) fn replay_pending_team_membership(app: &AppHandle) -> Result<(), String> { + let Some(pending) = load_pending_team_membership(app)? else { + return Ok(()); + }; + match pending_team_membership_state(&pending, &load_teams(app)?)? { + PendingTeamMembershipState::Pending => { + propagate_membership( + &pending.team_id, + &pending.previous_persona_ids, + &pending.current_persona_ids, + || load_managed_agents(app), + |records| save_managed_agents(app, records), + ) + .map_err(|error| format!("could not replay pending team update: {error}"))?; + } + PendingTeamMembershipState::Superseded => {} + } + clear_pending_team_membership(app) +} + /// Clear `team_id` on every instance that is bound to `team_id` but whose /// persona is absent from `current_persona_ids` or unset. Reports whether /// anything changed. @@ -60,24 +155,17 @@ fn detach_agents_outside_roster( changed } -/// Reports a membership propagation failure. The update command reports a -/// failure for a roster removal or a failed stale-binding detach. Other changes -/// keep the best-effort policy. +/// Reports a membership propagation failure. +#[derive(Debug)] pub(in crate::commands) enum MembershipPropagationError { Load(String), - Save { error: String, detached: bool }, -} - -impl MembershipPropagationError { - fn must_report(&self) -> bool { - matches!(self, Self::Save { detached: true, .. }) - } + Save(String), } impl std::fmt::Display for MembershipPropagationError { fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - Self::Load(error) | Self::Save { error, .. } => formatter.write_str(error), + Self::Load(error) | Self::Save(error) => formatter.write_str(error), } } } @@ -103,8 +191,7 @@ pub(in crate::commands) fn propagate_membership( ); let detached = detach_agents_outside_roster(&mut records, team_id, current_persona_ids); if delta_changed || detached { - save_agents(&records) - .map_err(|error| MembershipPropagationError::Save { error, detached })?; + save_agents(&records).map_err(MembershipPropagationError::Save)?; } Ok(()) } @@ -172,21 +259,13 @@ fn commit_team_create( /// keeps it `AppHandle`-free; a `persist_teams` error propagates. Returns the /// updated team. /// -/// Unlike [`commit_team_create`], an update reports an agent-store failure for -/// a roster removal or a failed stale-binding detach. A removal clears `team_id` -/// on the removed member. If that write does not land, the agent keeps the -/// binding and `delete_team_with_cascade` refuses the team, so the user gets an -/// empty team that they cannot delete — the exact defect this command must not -/// create. The command must not report success while the delete guard can still -/// refuse. -/// -/// When this reports an agent-store error after the team write, `update_team` -/// does not retain the team for relay sync until a retry or the next launch -/// migration runs. +/// Keeps the team roster and instance bindings recoverable across two stores. +/// It stages the prior→current delta before it writes either store. A failed +/// agent write leaves the stage file in place. The next update or launch replays +/// that original delta before it accepts another team edit. /// -/// Reporting is safe here because an update is idempotent: it targets an -/// existing team by id, so a retry cannot mint a duplicate team. A create has no -/// id yet, which is why it keeps the best-effort policy. +/// A create has no stable id, so it keeps a best-effort policy to avoid a +/// duplicate team on retry. #[allow(clippy::too_many_arguments)] fn commit_team_update( teams: &mut [TeamRecord], @@ -215,10 +294,8 @@ fn commit_team_update( team.updated_at = now; let updated = team.clone(); + let membership_changed = previous_persona_ids != updated.persona_ids; persist_teams(teams)?; - let has_removal = previous_persona_ids - .iter() - .any(|persona_id| !updated.persona_ids.contains(persona_id)); if let Err(error) = propagate_membership( &updated.id, &previous_persona_ids, @@ -226,10 +303,9 @@ fn commit_team_update( load_agents, save_agents, ) { - if has_removal || error.must_report() { + if membership_changed || matches!(error, MembershipPropagationError::Save(_)) { return Err(format!( - "Saved the team, but could not update its agents: {error}. The team can refuse deletion \ - until this succeeds. Save the team again." + "Saved the team, but could not update its agents: {error}. Save the team again." )); } eprintln!("buzz-desktop: team-membership-propagate: {error}"); @@ -475,9 +551,27 @@ pub async fn update_team(input: UpdateTeamRequest, app: AppHandle) -> Result Result Result<(), String> { .managed_agents_store_lock .lock() .map_err(|error| error.to_string())?; + replay_pending_team_membership(&app)?; let cascaded_persona_d_tags = delete_team_with_cascade(&app, &id)?; // delete_team_with_cascade rejects built-in teams via validate_team_deletion, // so reaching here means this team was owner-published — tombstone it. The diff --git a/desktop/src-tauri/src/commands/teams_tests.rs b/desktop/src-tauri/src/commands/teams_tests.rs index f6a50a99885..793a25a0ae5 100644 --- a/desktop/src-tauri/src/commands/teams_tests.rs +++ b/desktop/src-tauri/src/commands/teams_tests.rs @@ -4,7 +4,8 @@ use super::{ apply_team_membership_delta, commit_team_create, commit_team_update, - detach_agents_outside_roster, + detach_agents_outside_roster, pending_team_membership_state, propagate_membership, + PendingTeamMembershipState, PendingTeamMembershipUpdate, }; use crate::managed_agents::{ManagedAgentRecord, TeamRecord}; use std::cell::RefCell; @@ -200,13 +201,13 @@ fn commit_update_ignores_agent_load_failure_for_metadata_only_edit() { assert_eq!(teams[0].name, "Renamed team"); } -/// An add-only update also keeps its disk-authoritative result when the -/// agent store cannot load. A missed bind does not block team deletion. +/// An add-only update reports an agent-store load failure. The staged delta +/// remains available for replay on the next save or launch. #[test] -fn commit_update_ignores_agent_load_failure_for_add_only_edit() { +fn commit_update_reports_agent_load_failure_for_add_only_edit() { let mut teams = vec![team("team-a", &["duncan"])]; - let updated = commit_team_update( + let error = commit_team_update( &mut teams, "team-a", "team-a".to_string(), @@ -218,9 +219,9 @@ fn commit_update_ignores_agent_load_failure_for_add_only_edit() { || Err("corrupt managed-agents.json".to_string()), |_| Ok(()), ) - .expect("add-only update keeps the best-effort policy"); + .expect_err("an add-only update must report the lost binding"); - assert_eq!(updated.persona_ids, ids(&["duncan", "ada"])); + assert!(error.contains("could not update its agents"), "{error}"); assert_eq!(teams[0].persona_ids, ids(&["duncan", "ada"])); } @@ -552,6 +553,65 @@ fn detach_outside_roster_is_scoped_to_this_team_and_absent_personas() { ); } +/// A staged update only replays when the team still has the staged roster. +/// A prior roster means the team write did not land, so the stale stage is safe +/// to clear without a membership change. +#[test] +fn pending_membership_state_distinguishes_pending_and_superseded_writes() { + let pending = PendingTeamMembershipUpdate { + team_id: "team-a".to_string(), + previous_persona_ids: ids(&["duncan"]), + current_persona_ids: ids(&["ada"]), + }; + + assert!(matches!( + pending_team_membership_state(&pending, &[team("team-a", &["ada"])]), + Ok(PendingTeamMembershipState::Pending) + )); + assert!(matches!( + pending_team_membership_state(&pending, &[team("team-a", &["duncan"])]), + Ok(PendingTeamMembershipState::Superseded) + )); +} + +/// The durable replay uses the original replace delta after the first agent +/// save fails. It binds Ada on retry even though the persisted roster already +/// contains Ada and therefore supplies no new delta. +#[test] +fn replayed_replace_delta_binds_the_added_instance() { + let previous = ids(&["duncan"]); + let current = ids(&["ada"]); + let store = RefCell::new(vec![ + instance('a', "duncan", Some("team-a")), + instance('b', "ada", None), + ]); + + let error = propagate_membership( + "team-a", + &previous, + ¤t, + || Ok(store.borrow().clone()), + |_| Err("disk full".to_string()), + ) + .expect_err("the first save fails"); + assert!(error.to_string().contains("disk full")); + + propagate_membership( + "team-a", + &previous, + ¤t, + || Ok(store.borrow().clone()), + |records| { + *store.borrow_mut() = records.to_vec(); + Ok(()) + }, + ) + .expect("the durable replay succeeds"); + + assert_eq!(store.borrow()[0].team_id, None); + assert_eq!(store.borrow()[1].team_id.as_deref(), Some("team-a")); +} + /// A roster with every binding already correct writes nothing. #[test] fn detach_outside_roster_is_inert_when_nothing_is_stale() { diff --git a/desktop/src-tauri/src/migration.rs b/desktop/src-tauri/src/migration.rs index 1e22d7aaeca..e7e384972a1 100644 --- a/desktop/src-tauri/src/migration.rs +++ b/desktop/src-tauri/src/migration.rs @@ -31,6 +31,7 @@ const LEGACY_RELEASE_IDENTIFIER: &str = "xyz.block.sprout.app"; /// receive their identity via the `BUZZ_PRIVATE_KEY` env var. const SHARED_AGENT_FILES: &[&str] = &[ "agents/managed-agents.json", + "agents/pending-team-membership.json", "agents/personas.json", "agents/teams.json", ]; @@ -186,6 +187,7 @@ fn run_boot_migrations_inner(app: &tauri::AppHandle, reset_completed: bool) { // Repair dropped team↔member links, then detach directory-backed teams, // gated on a clean repair so a failure preserves `source_dir` for a retry. team_membership::repair_then_detach_teams(app); + replay_pending_team_membership_update(app); reconcile_provider_mcp_commands(app); reconcile_databricks_v1_to_v2(app); materialize_agent_runtimes(app); @@ -1367,6 +1369,18 @@ pub fn migrate_persona_provider_to_runtime(app: &tauri::AppHandle) { } mod materialize; pub use materialize::materialize_agent_runtimes; + +fn replay_pending_team_membership_update(app: &tauri::AppHandle) { + let state = app.state::(); + let Ok(_store_guard) = state.managed_agents_store_lock.lock() else { + eprintln!("buzz-desktop: pending-team-membership: cannot lock agent store"); + return; + }; + if let Err(error) = crate::commands::replay_pending_team_membership(app) { + eprintln!("buzz-desktop: pending-team-membership: {error}"); + } +} + mod fold; pub use fold::fold_personas_into_agent_store; use fold::load_persona_runtimes; diff --git a/desktop/src-tauri/src/migration_tests.rs b/desktop/src-tauri/src/migration_tests.rs index 0d49bd02aab..9b98d7a40f7 100644 --- a/desktop/src-tauri/src/migration_tests.rs +++ b/desktop/src-tauri/src/migration_tests.rs @@ -84,6 +84,11 @@ fn setup_sync_layout() -> (tempfile::TempDir, PathBuf, PathBuf) { ) .unwrap(); std::fs::write(canonical.join("agents/teams.json"), r#"[{"id":"team-1"}]"#).unwrap(); + std::fs::write( + canonical.join("agents/pending-team-membership.json"), + r#"{"team_id":"team-1","previous_persona_ids":[],"current_persona_ids":[]}"#, + ) + .unwrap(); // Teams installed from `.main` — canonical has no teams dir. let team_dir = main_instance.join("agents/teams/com.example.test-pack"); @@ -216,7 +221,7 @@ fn sync_files(canonical: &Path, worktree: &Path) -> u32 { fn sync_creates_symlinks_to_fresh_worktree() { let (_parent, canonical, worktree) = setup_sync_layout(); let synced = sync_files(&canonical, &worktree); - assert_eq!(synced, 4); + assert_eq!(synced, 5); for rel in SHARED_AGENT_FILES { let dst = worktree.join(rel); assert!(dst.is_symlink(), "{rel} should be a symlink"); @@ -244,7 +249,7 @@ fn sync_replaces_existing_files_with_symlinks() { let synced = sync_files(&canonical, &worktree); - assert_eq!(synced, 4); + assert_eq!(synced, 5); for rel in SHARED_AGENT_FILES { let dst = worktree.join(rel); assert!( @@ -263,7 +268,7 @@ fn sync_replaces_existing_files_with_symlinks() { #[test] fn sync_preserves_correct_symlinks() { let (_parent, canonical, worktree) = setup_sync_layout(); - assert_eq!(sync_files(&canonical, &worktree), 4); + assert_eq!(sync_files(&canonical, &worktree), 5); assert_eq!(sync_files(&canonical, &worktree), 0); for rel in SHARED_AGENT_FILES { let dst = worktree.join(rel); @@ -282,7 +287,7 @@ fn sync_replaces_wrong_symlinks() { std::os::unix::fs::symlink(&wrong_target, worktree.join(rel)).unwrap(); } let synced = sync_files(&canonical, &worktree); - assert_eq!(synced, 4); + assert_eq!(synced, 5); for rel in SHARED_AGENT_FILES { assert_eq!( std::fs::read_link(worktree.join(rel)).unwrap(), @@ -301,7 +306,7 @@ fn sync_handles_broken_symlinks() { std::os::unix::fs::symlink(&broken_target, worktree.join(rel)).unwrap(); } let synced = sync_files(&canonical, &worktree); - assert_eq!(synced, 4); + assert_eq!(synced, 5); for rel in SHARED_AGENT_FILES { let dst = worktree.join(rel); assert!(dst.is_symlink()); From 38cb69aeb797d562642f7c8af0351f0426532052 Mon Sep 17 00:00:00 2001 From: Storme Drone <49c46e84758b2ebff4abf5abbbd44ee4ce788fc3b55db9fa703eec124eead621@buzz.block.builderlab.xyz> Date: Wed, 26 Aug 2026 15:55:28 -0600 Subject: [PATCH 09/12] fix(desktop): discard stale team membership stages Signed-off-by: Storme Drone <49c46e84758b2ebff4abf5abbbd44ee4ce788fc3b55db9fa703eec124eead621@buzz.block.builderlab.xyz> --- desktop/src-tauri/src/commands/teams.rs | 22 +++++++++----- desktop/src-tauri/src/commands/teams_tests.rs | 15 ++++++++-- desktop/src-tauri/src/migration.rs | 30 +++++-------------- .../src/migration/team_membership.rs | 13 ++++++++ 4 files changed, 47 insertions(+), 33 deletions(-) diff --git a/desktop/src-tauri/src/commands/teams.rs b/desktop/src-tauri/src/commands/teams.rs index fdf61fd53f7..5b9fd207e6d 100644 --- a/desktop/src-tauri/src/commands/teams.rs +++ b/desktop/src-tauri/src/commands/teams.rs @@ -78,25 +78,23 @@ fn clear_pending_team_membership(app: &AppHandle) -> Result<(), String> { enum PendingTeamMembershipState { Pending, Superseded, + MissingTeam, + UnexpectedRoster, } fn pending_team_membership_state( pending: &PendingTeamMembershipUpdate, teams: &[TeamRecord], ) -> Result { - let team = teams - .iter() - .find(|team| team.id == pending.team_id) - .ok_or_else(|| format!("pending team {} no longer exists", pending.team_id))?; + let Some(team) = teams.iter().find(|team| team.id == pending.team_id) else { + return Ok(PendingTeamMembershipState::MissingTeam); + }; if team.persona_ids == pending.current_persona_ids { Ok(PendingTeamMembershipState::Pending) } else if team.persona_ids == pending.previous_persona_ids { Ok(PendingTeamMembershipState::Superseded) } else { - Err(format!( - "pending team {} has an unexpected roster; save the team again", - pending.team_id - )) + Ok(PendingTeamMembershipState::UnexpectedRoster) } } @@ -117,6 +115,14 @@ pub(crate) fn replay_pending_team_membership(app: &AppHandle) -> Result<(), Stri .map_err(|error| format!("could not replay pending team update: {error}"))?; } PendingTeamMembershipState::Superseded => {} + PendingTeamMembershipState::MissingTeam => eprintln!( + "buzz-desktop: pending-team-membership: discarding staged update for missing team {:?}", + pending.team_id + ), + PendingTeamMembershipState::UnexpectedRoster => eprintln!( + "buzz-desktop: pending-team-membership: discarding staged update for team {:?} with a different roster", + pending.team_id + ), } clear_pending_team_membership(app) } diff --git a/desktop/src-tauri/src/commands/teams_tests.rs b/desktop/src-tauri/src/commands/teams_tests.rs index 793a25a0ae5..42905e5d723 100644 --- a/desktop/src-tauri/src/commands/teams_tests.rs +++ b/desktop/src-tauri/src/commands/teams_tests.rs @@ -553,11 +553,12 @@ fn detach_outside_roster_is_scoped_to_this_team_and_absent_personas() { ); } -/// A staged update only replays when the team still has the staged roster. -/// A prior roster means the team write did not land, so the stale stage is safe +/// A staged update replays only when the team keeps the staged roster. The +/// prior roster means the team write did not land. A missing team or a different +/// roster means an inbound event superseded the stage. Each stale stage is safe /// to clear without a membership change. #[test] -fn pending_membership_state_distinguishes_pending_and_superseded_writes() { +fn pending_membership_state_distinguishes_replay_and_stale_stages() { let pending = PendingTeamMembershipUpdate { team_id: "team-a".to_string(), previous_persona_ids: ids(&["duncan"]), @@ -572,6 +573,14 @@ fn pending_membership_state_distinguishes_pending_and_superseded_writes() { pending_team_membership_state(&pending, &[team("team-a", &["duncan"])]), Ok(PendingTeamMembershipState::Superseded) )); + assert!(matches!( + pending_team_membership_state(&pending, &[]), + Ok(PendingTeamMembershipState::MissingTeam) + )); + assert!(matches!( + pending_team_membership_state(&pending, &[team("team-a", &["paul"])]), + Ok(PendingTeamMembershipState::UnexpectedRoster) + )); } /// The durable replay uses the original replace delta after the first agent diff --git a/desktop/src-tauri/src/migration.rs b/desktop/src-tauri/src/migration.rs index e7e384972a1..484837397b5 100644 --- a/desktop/src-tauri/src/migration.rs +++ b/desktop/src-tauri/src/migration.rs @@ -130,10 +130,9 @@ pub fn run_boot_migrations_after_reset(app: &tauri::AppHandle) { } fn run_boot_migrations_inner(app: &tauri::AppHandle, reset_completed: bool) { - // Initialize the process-lifetime nest directory before any filesystem - // operation that calls nest_dir(). The discriminator matches the existing - // pattern used by reconcile_target_dir: dev instances have an app-data-dir - // name starting with CANONICAL_DEV_IDENTIFIER. + // Initialize the process-lifetime nest directory before filesystem access + // that calls nest_dir(). The discriminator matches reconcile_target_dir: + // dev instances have an app-data-dir name starting with CANONICAL_DEV_IDENTIFIER. let is_dev = if let Ok(data_dir) = app.path().app_data_dir() { let dev = data_dir .file_name() @@ -145,12 +144,10 @@ fn run_boot_migrations_inner(app: &tauri::AppHandle, reset_completed: bool) { false }; - // On dev builds, copy `.repos-dir` from ~/.buzz → ~/.buzz-dev BEFORE - // control returns to lib.rs where resolve_repos_at_boot() reads it. This - // ensures the dev nest boots with the correct workspace on its first launch, - // matching what the prod nest had configured. Skip-if-dest-exists so it is - // idempotent and never clobbers a value the dev nest already set explicitly. - // Uses the composed helper so gate + migration share the tested code path. + // On dev builds, copy `.repos-dir` from ~/.buzz → ~/.buzz-dev before + // resolve_repos_at_boot() reads it. Skip-if-dest-exists so it is idempotent + // and never clobbers a value the dev nest already set explicitly. + // The composed helper keeps gate + migration on the tested code path. if let (Some(home), Some(dev_nest)) = (dirs::home_dir(), crate::managed_agents::nest_dir()) { maybe_migrate_dev_repos_dir(is_dev, reset_completed, &home, &dev_nest); } @@ -187,7 +184,7 @@ fn run_boot_migrations_inner(app: &tauri::AppHandle, reset_completed: bool) { // Repair dropped team↔member links, then detach directory-backed teams, // gated on a clean repair so a failure preserves `source_dir` for a retry. team_membership::repair_then_detach_teams(app); - replay_pending_team_membership_update(app); + team_membership::replay_pending_team_membership_update(app); reconcile_provider_mcp_commands(app); reconcile_databricks_v1_to_v2(app); materialize_agent_runtimes(app); @@ -1370,17 +1367,6 @@ pub fn migrate_persona_provider_to_runtime(app: &tauri::AppHandle) { mod materialize; pub use materialize::materialize_agent_runtimes; -fn replay_pending_team_membership_update(app: &tauri::AppHandle) { - let state = app.state::(); - let Ok(_store_guard) = state.managed_agents_store_lock.lock() else { - eprintln!("buzz-desktop: pending-team-membership: cannot lock agent store"); - return; - }; - if let Err(error) = crate::commands::replay_pending_team_membership(app) { - eprintln!("buzz-desktop: pending-team-membership: {error}"); - } -} - mod fold; pub use fold::fold_personas_into_agent_store; use fold::load_persona_runtimes; diff --git a/desktop/src-tauri/src/migration/team_membership.rs b/desktop/src-tauri/src/migration/team_membership.rs index 632714f3f91..19ba59fd4af 100644 --- a/desktop/src-tauri/src/migration/team_membership.rs +++ b/desktop/src-tauri/src/migration/team_membership.rs @@ -33,6 +33,8 @@ use std::collections::HashMap; use std::path::Path; +use tauri::Manager; + use crate::managed_agents::{team_persona_key, ManagedAgentRecord, TeamRecord}; /// Repair stale team `persona_ids`/instance `team_id`, then detach @@ -55,6 +57,17 @@ pub(super) fn repair_then_detach_teams(app: &tauri::AppHandle) { ); } +pub(super) fn replay_pending_team_membership_update(app: &tauri::AppHandle) { + let state = app.state::(); + let Ok(_store_guard) = state.managed_agents_store_lock.lock() else { + eprintln!("buzz-desktop: pending-team-membership: cannot lock agent store"); + return; + }; + if let Err(error) = crate::commands::replay_pending_team_membership(app) { + eprintln!("buzz-desktop: pending-team-membership: {error}"); + } +} + /// Gate `detach` on a successful `repair`: run detach only when repair returned /// `Ok`. Injected ops keep the gate `AppHandle`-free so a failing repair's /// skip-detach behavior is unit-testable without a filesystem fault. From 36e2f452c356e62febce3d5e8cce1650827f13e9 Mon Sep 17 00:00:00 2001 From: Storme Drone <49c46e84758b2ebff4abf5abbbd44ee4ce788fc3b55db9fa703eec124eead621@buzz.block.builderlab.xyz> Date: Thu, 27 Aug 2026 17:27:59 -0600 Subject: [PATCH 10/12] fix(desktop): replay team membership stages safely Signed-off-by: Storme Drone <49c46e84758b2ebff4abf5abbbd44ee4ce788fc3b55db9fa703eec124eead621@buzz.block.builderlab.xyz> --- desktop/src-tauri/src/commands/teams.rs | 147 +++++++++++------- desktop/src-tauri/src/commands/teams_tests.rs | 143 ++++++++++++++--- 2 files changed, 210 insertions(+), 80 deletions(-) diff --git a/desktop/src-tauri/src/commands/teams.rs b/desktop/src-tauri/src/commands/teams.rs index 5b9fd207e6d..58a1222ea5c 100644 --- a/desktop/src-tauri/src/commands/teams.rs +++ b/desktop/src-tauri/src/commands/teams.rs @@ -31,7 +31,7 @@ fn trim_optional(value: Option) -> Option { /// A staged team update. The record persists before the two stores change so a /// later save or launch can replay the original membership delta. -#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] struct PendingTeamMembershipUpdate { team_id: String, previous_persona_ids: Vec, @@ -42,60 +42,73 @@ fn pending_team_membership_path(app: &AppHandle) -> Result { Ok(crate::managed_agents::managed_agents_base_dir(app)?.join("pending-team-membership.json")) } -fn save_pending_team_membership( - app: &AppHandle, - pending: &PendingTeamMembershipUpdate, +fn save_pending_team_membership_at( + path: &std::path::Path, + pending: Option<&PendingTeamMembershipUpdate>, ) -> Result<(), String> { - let path = pending_team_membership_path(app)?; - let payload = serde_json::to_vec_pretty(pending) + let payload = serde_json::to_vec_pretty(&pending) .map_err(|error| format!("failed to serialize pending team update: {error}"))?; - crate::managed_agents::storage::atomic_write_json(&path, &payload) + crate::managed_agents::storage::atomic_write_json(path, &payload) } -fn load_pending_team_membership( - app: &AppHandle, +fn load_pending_team_membership_at( + path: &std::path::Path, ) -> Result, String> { - let path = pending_team_membership_path(app)?; if !path.exists() { return Ok(None); } - let payload = std::fs::read_to_string(&path) + let payload = std::fs::read_to_string(path) .map_err(|error| format!("failed to read pending team update: {error}"))?; serde_json::from_str(&payload) - .map(Some) .map_err(|error| format!("failed to parse pending team update: {error}")) } -fn clear_pending_team_membership(app: &AppHandle) -> Result<(), String> { - let path = pending_team_membership_path(app)?; - match std::fs::remove_file(&path) { - Ok(()) => Ok(()), - Err(error) if error.kind() == std::io::ErrorKind::NotFound => Ok(()), - Err(error) => Err(format!("failed to clear pending team update: {error}")), - } +fn save_pending_team_membership( + app: &AppHandle, + pending: &PendingTeamMembershipUpdate, +) -> Result<(), String> { + save_pending_team_membership_at(&pending_team_membership_path(app)?, Some(pending)) +} + +fn load_pending_team_membership( + app: &AppHandle, +) -> Result, String> { + load_pending_team_membership_at(&pending_team_membership_path(app)?) } -enum PendingTeamMembershipState { - Pending, - Superseded, - MissingTeam, - UnexpectedRoster, +fn clear_pending_team_membership(app: &AppHandle) -> Result<(), String> { + // Write `null` through the link instead of unlinking it. The pending file + // is shared by dev worktrees, and `atomic_write_json` preserves the link. + save_pending_team_membership_at(&pending_team_membership_path(app)?, None) } -fn pending_team_membership_state( +/// The part of a staged delta that still agrees with the current team roster. +/// +/// An inbound event can extend or reorder the roster while a local agent-store +/// write is pending. It does not erase the local add or removal evidence that +/// still holds. An inbound reversal does erase that evidence, so the replay +/// leaves that membership direction alone. +fn pending_replay_delta( pending: &PendingTeamMembershipUpdate, - teams: &[TeamRecord], -) -> Result { - let Some(team) = teams.iter().find(|team| team.id == pending.team_id) else { - return Ok(PendingTeamMembershipState::MissingTeam); - }; - if team.persona_ids == pending.current_persona_ids { - Ok(PendingTeamMembershipState::Pending) - } else if team.persona_ids == pending.previous_persona_ids { - Ok(PendingTeamMembershipState::Superseded) - } else { - Ok(PendingTeamMembershipState::UnexpectedRoster) - } + current_persona_ids: &[String], +) -> (Vec, Vec) { + let removed = pending + .previous_persona_ids + .iter() + .filter(|id| { + !pending.current_persona_ids.contains(*id) && !current_persona_ids.contains(*id) + }) + .cloned() + .collect(); + let added = pending + .current_persona_ids + .iter() + .filter(|id| { + !pending.previous_persona_ids.contains(*id) && current_persona_ids.contains(*id) + }) + .cloned() + .collect(); + (removed, added) } /// Replay a staged membership delta. Callers hold `managed_agents_store_lock`. @@ -103,27 +116,25 @@ pub(crate) fn replay_pending_team_membership(app: &AppHandle) -> Result<(), Stri let Some(pending) = load_pending_team_membership(app)? else { return Ok(()); }; - match pending_team_membership_state(&pending, &load_teams(app)?)? { - PendingTeamMembershipState::Pending => { - propagate_membership( - &pending.team_id, - &pending.previous_persona_ids, - &pending.current_persona_ids, - || load_managed_agents(app), - |records| save_managed_agents(app, records), - ) - .map_err(|error| format!("could not replay pending team update: {error}"))?; - } - PendingTeamMembershipState::Superseded => {} - PendingTeamMembershipState::MissingTeam => eprintln!( + let teams = load_teams(app)?; + let Some(team) = teams.iter().find(|team| team.id == pending.team_id) else { + eprintln!( "buzz-desktop: pending-team-membership: discarding staged update for missing team {:?}", pending.team_id - ), - PendingTeamMembershipState::UnexpectedRoster => eprintln!( - "buzz-desktop: pending-team-membership: discarding staged update for team {:?} with a different roster", - pending.team_id - ), - } + ); + return clear_pending_team_membership(app); + }; + let (previous_persona_ids, current_persona_ids) = + pending_replay_delta(&pending, &team.persona_ids); + propagate_membership_with_roster( + &pending.team_id, + &previous_persona_ids, + ¤t_persona_ids, + &team.persona_ids, + || load_managed_agents(app), + |records| save_managed_agents(app, records), + ) + .map_err(|error| format!("could not replay pending team update: {error}"))?; clear_pending_team_membership(app) } @@ -187,6 +198,27 @@ pub(in crate::commands) fn propagate_membership( current_persona_ids: &[String], load_agents: impl FnOnce() -> Result, String>, save_agents: impl FnOnce(&[crate::managed_agents::ManagedAgentRecord]) -> Result<(), String>, +) -> Result<(), MembershipPropagationError> { + propagate_membership_with_roster( + team_id, + previous_persona_ids, + current_persona_ids, + current_persona_ids, + load_agents, + save_agents, + ) +} + +/// Apply a membership delta, then reconcile bindings with the authoritative +/// roster. Replay uses a smaller delta when an inbound edit has changed the +/// roster after staging, while the reconciliation always uses that latest roster. +fn propagate_membership_with_roster( + team_id: &str, + previous_persona_ids: &[String], + current_persona_ids: &[String], + authoritative_persona_ids: &[String], + load_agents: impl FnOnce() -> Result, String>, + save_agents: impl FnOnce(&[crate::managed_agents::ManagedAgentRecord]) -> Result<(), String>, ) -> Result<(), MembershipPropagationError> { let mut records = load_agents().map_err(MembershipPropagationError::Load)?; let delta_changed = apply_team_membership_delta( @@ -195,7 +227,7 @@ pub(in crate::commands) fn propagate_membership( previous_persona_ids, current_persona_ids, ); - let detached = detach_agents_outside_roster(&mut records, team_id, current_persona_ids); + let detached = detach_agents_outside_roster(&mut records, team_id, authoritative_persona_ids); if delta_changed || detached { save_agents(&records).map_err(MembershipPropagationError::Save)?; } @@ -512,6 +544,7 @@ pub async fn create_team(input: CreateTeamRequest, app: AppHandle) -> Result Date: Mon, 31 Aug 2026 17:43:42 -0600 Subject: [PATCH 11/12] Make team membership bindings durable Signed-off-by: Storme Drone <49c46e84758b2ebff4abf5abbbd44ee4ce788fc3b55db9fa703eec124eead621@buzz.block.builderlab.xyz> --- .../src/commands/personas/inbound.rs | 67 ++++++++----- .../personas/inbound/inbound_tests.rs | 71 ++++++++++++-- desktop/src-tauri/src/commands/teams/mod.rs | 94 ++++++++++--------- desktop/src-tauri/src/commands/teams/tests.rs | 22 +++-- .../teams/tests/membership_recovery.rs | 55 +++++++++-- 5 files changed, 214 insertions(+), 95 deletions(-) diff --git a/desktop/src-tauri/src/commands/personas/inbound.rs b/desktop/src-tauri/src/commands/personas/inbound.rs index 2080630742d..8a5aa82894b 100644 --- a/desktop/src-tauri/src/commands/personas/inbound.rs +++ b/desktop/src-tauri/src/commands/personas/inbound.rs @@ -219,6 +219,10 @@ fn reconcile_inbound_persona_event_blocking( .managed_agents_store_lock .lock() .map_err(|error| error.to_string())?; + // A prior create, update, or inbound team write can have reached teams.json + // before its agent-store write failed. Replay it before accepting any next + // inbound event, so an identical retained event cannot consume that intent. + crate::commands::replay_pending_team_membership(&app)?; // Resolve inbound vs. any pending local edit before touching the store, in // the scope the event ARRIVED on. A workspace switch since arrival leaves @@ -305,9 +309,11 @@ fn reconcile_inbound_persona_event_blocking( &mut teams, d_tag, team_content_from_event(&event)?, + |pending| crate::commands::teams::save_pending_team_membership(&app, pending), |teams| save_teams(&app, teams), || load_managed_agents(&app), |records| save_managed_agents(&app, records), + || crate::commands::teams::clear_pending_team_membership(&app), ) })?; if outcome == InboundOutcome::Skipped { @@ -751,29 +757,23 @@ fn apply_inbound_managed_agent( false } -/// In-memory core of the inbound `KIND_TEAM` reconcile: capture the matched -/// team's roster *before* applying the inbound projection, apply it, persist -/// teams authoritatively, then propagate the prior→current membership delta to -/// live instances best-effort — the same binding semantics the local -/// create/update commands use. Without this, a 30176 team edit from another -/// device lands on `teams.json` but never touches `ManagedAgentRecord.team_id`: -/// an added persona's running instances stay unbound (member in roster, not in -/// behavior) and a removed persona's instances keep drawing the old team's -/// instructions at spawn until restart. -/// -/// A no-match insert has no prior roster, so its whole roster is the added -/// delta — symmetric with `commit_team_create`. Injected persistence keeps it -/// `AppHandle`-free so the prior-roster capture and delta direction are -/// unit-testable; a `persist_teams` error propagates, agent IO is best-effort -/// (mirrors the local command path: the authoritative team write already -/// landed, and boot repair is the designed retry for a stale binding). +/// In-memory core of the inbound `KIND_TEAM` reconcile. It stages a changed +/// roster before the team write, then commits the team and agent stores. It +/// clears the stage only after both writes succeed. This makes an agent-store +/// failure retryable when the team contains a persona shared by another team. +/// The caller advances relay retention only after this function succeeds. +#[allow(clippy::too_many_arguments)] fn commit_inbound_team( teams: &mut Vec, d_tag: String, inbound: TeamEventContent, + save_pending: impl FnOnce( + &crate::commands::teams::PendingTeamMembershipUpdate, + ) -> Result<(), String>, persist_teams: impl FnOnce(&[TeamRecord]) -> Result<(), String>, load_agents: impl FnOnce() -> Result, String>, save_agents: impl FnOnce(&[ManagedAgentRecord]) -> Result<(), String>, + clear_pending: impl FnOnce() -> Result<(), String>, ) -> Result<(), String> { let team_id = d_tag.clone(); let previous_persona_ids = teams @@ -787,14 +787,33 @@ fn commit_inbound_team( .find(|record| record.id == team_id) .map(|record| record.persona_ids.clone()) .unwrap_or_default(); - persist_teams(teams)?; - crate::commands::teams::propagate_membership_best_effort( - &team_id, - &previous_persona_ids, - ¤t_persona_ids, - load_agents, - save_agents, - ); + let membership_changed = previous_persona_ids != current_persona_ids; + if membership_changed { + save_pending(&crate::commands::teams::PendingTeamMembershipUpdate { + team_id: team_id.clone(), + previous_persona_ids: previous_persona_ids.clone(), + current_persona_ids: current_persona_ids.clone(), + })?; + persist_teams(teams)?; + crate::commands::teams::propagate_membership( + &team_id, + &previous_persona_ids, + ¤t_persona_ids, + load_agents, + save_agents, + ) + .map_err(|error| format!("could not update inbound team agents: {error}"))?; + clear_pending()?; + } else { + persist_teams(teams)?; + crate::commands::teams::propagate_membership_best_effort( + &team_id, + &previous_persona_ids, + ¤t_persona_ids, + load_agents, + save_agents, + ); + } Ok(()) } diff --git a/desktop/src-tauri/src/commands/personas/inbound/inbound_tests.rs b/desktop/src-tauri/src/commands/personas/inbound/inbound_tests.rs index ab932437553..c7f3ac73c2e 100644 --- a/desktop/src-tauri/src/commands/personas/inbound/inbound_tests.rs +++ b/desktop/src-tauri/src/commands/personas/inbound/inbound_tests.rs @@ -594,11 +594,13 @@ fn inbound_team_add_binds_unbound_instance_through_wiring() { persona_ids: Some(vec!["p-existing".to_string(), "p-added".to_string()]), }, |_| Ok(()), + |_| Ok(()), || Ok(existing.clone()), |records| { *saved.borrow_mut() = Some(records.to_vec()); Ok(()) }, + || Ok(()), ) .expect("inbound add succeeds"); @@ -638,11 +640,13 @@ fn inbound_team_removal_detaches_instance_through_wiring() { persona_ids: Some(vec![]), }, |_| Ok(()), + |_| Ok(()), || Ok(existing.clone()), |records| { *saved.borrow_mut() = Some(records.to_vec()); Ok(()) }, + || Ok(()), ) .expect("inbound removal succeeds"); @@ -671,11 +675,13 @@ fn inbound_team_omitted_roster_leaves_bindings_untouched() { TEAM_ID.to_string(), team_content_omitting_optional_fields("Renamed"), |_| Ok(()), + |_| Ok(()), || Ok(existing.clone()), |records| { *saved.borrow_mut() = Some(records.to_vec()); Ok(()) }, + || Ok(()), ) .expect("inbound metadata-only edit succeeds"); @@ -685,15 +691,24 @@ fn inbound_team_omitted_roster_leaves_bindings_untouched() { ); } -/// A failing agent-store write after the authoritative `save_teams` is -/// swallowed: the inbound reconcile still succeeds (boot repair is the retry), -/// so a secondary-store hiccup never aborts an inbound event whose team write -/// already landed. +/// An inbound add persists a recovery stage before the team write. A failed +/// agent write leaves that stage available to bind a shared persona on restart. +/// The error stops `commit_inbound_with_store`, so the retention head does not +/// advance until the binding becomes durable. #[test] -fn inbound_team_swallows_agent_store_failure() { - let mut teams = vec![local_team()]; +fn inbound_team_failure_keeps_a_durable_shared_persona_replay_delta() { + let pending_file = tempfile::NamedTempFile::new().expect("temporary stage"); + std::fs::write(pending_file.path(), "null").expect("initialize stage"); + let mut teams = vec![local_team(), { + let mut other = local_team(); + other.id = "other-team".to_string(); + other.persona_ids = vec!["p-added".to_string()]; + other + }]; teams[0].persona_ids = vec![]; - commit_inbound_team( + let agents = RefCell::new(vec![team_instance('a', "p-added", None)]); + + let error = commit_inbound_team( &mut teams, TEAM_ID.to_string(), TeamEventContent { @@ -702,11 +717,45 @@ fn inbound_team_swallows_agent_store_failure() { instructions: None, persona_ids: Some(vec!["p-added".to_string()]), }, + |pending| { + crate::commands::teams::save_pending_team_membership_at( + pending_file.path(), + Some(pending), + ) + }, |_| Ok(()), - || Err("agent store unreadable".to_string()), - |_| Ok(()), + || Ok(agents.borrow().clone()), + |_| Err("agent store unwritable".to_string()), + || crate::commands::teams::save_pending_team_membership_at(pending_file.path(), None), ) - .expect("inbound reconcile swallows secondary-store failure"); + .expect_err("inbound reconcile must report a lost membership binding"); + assert!( + error.contains("could not update inbound team agents"), + "{error}" + ); + assert_eq!( + teams[0].persona_ids, + vec!["p-added"], + "the team write landed" + ); + + let pending = crate::commands::teams::load_pending_team_membership_at(pending_file.path()) + .expect("read staged delta") + .expect("the failed inbound add keeps its stage"); + crate::commands::teams::propagate_membership( + &pending.team_id, + &pending.previous_persona_ids, + &pending.current_persona_ids, + || Ok(agents.borrow().clone()), + |records| { + *agents.borrow_mut() = records.to_vec(); + Ok(()) + }, + ) + .expect("a restart replay binds the explicit inbound add"); + crate::commands::teams::save_pending_team_membership_at(pending_file.path(), None) + .expect("clear replayed stage"); + assert_eq!(agents.borrow()[0].team_id.as_deref(), Some(TEAM_ID)); } /// A `persist_teams` error propagates — the authoritative team write failing is @@ -718,9 +767,11 @@ fn inbound_team_propagates_persist_teams_error() { &mut teams, TEAM_ID.to_string(), team_content("Team"), + |_| Ok(()), |_| Err("disk full".to_string()), || Ok(vec![]), |_| Ok(()), + || Ok(()), ) .expect_err("a failed team persist must propagate"); assert_eq!(err, "disk full"); diff --git a/desktop/src-tauri/src/commands/teams/mod.rs b/desktop/src-tauri/src/commands/teams/mod.rs index 5d50a2fa329..36192bcece3 100644 --- a/desktop/src-tauri/src/commands/teams/mod.rs +++ b/desktop/src-tauri/src/commands/teams/mod.rs @@ -29,20 +29,20 @@ fn trim_optional(value: Option) -> Option { }) } -/// A staged team update. The record persists before the two stores change so a -/// later save or launch can replay the original membership delta. +/// A staged team membership change. The record persists before the team and +/// agent stores change so a later save or launch can replay the original delta. #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] -struct PendingTeamMembershipUpdate { - team_id: String, - previous_persona_ids: Vec, - current_persona_ids: Vec, +pub(in crate::commands) struct PendingTeamMembershipUpdate { + pub(in crate::commands) team_id: String, + pub(in crate::commands) previous_persona_ids: Vec, + pub(in crate::commands) current_persona_ids: Vec, } -fn pending_team_membership_path(app: &AppHandle) -> Result { +fn pending_team_membership_path(app: &AppHandle) -> Result { Ok(crate::managed_agents::managed_agents_base_dir(app)?.join("pending-team-membership.json")) } -fn save_pending_team_membership_at( +pub(in crate::commands) fn save_pending_team_membership_at( path: &std::path::Path, pending: Option<&PendingTeamMembershipUpdate>, ) -> Result<(), String> { @@ -51,7 +51,7 @@ fn save_pending_team_membership_at( crate::managed_agents::storage::atomic_write_json(path, &payload) } -fn load_pending_team_membership_at( +pub(in crate::commands) fn load_pending_team_membership_at( path: &std::path::Path, ) -> Result, String> { if !path.exists() { @@ -63,20 +63,22 @@ fn load_pending_team_membership_at( .map_err(|error| format!("failed to parse pending team update: {error}")) } -fn save_pending_team_membership( - app: &AppHandle, +pub(in crate::commands) fn save_pending_team_membership( + app: &AppHandle, pending: &PendingTeamMembershipUpdate, ) -> Result<(), String> { save_pending_team_membership_at(&pending_team_membership_path(app)?, Some(pending)) } -fn load_pending_team_membership( - app: &AppHandle, +fn load_pending_team_membership( + app: &AppHandle, ) -> Result, String> { load_pending_team_membership_at(&pending_team_membership_path(app)?) } -fn clear_pending_team_membership(app: &AppHandle) -> Result<(), String> { +pub(in crate::commands) fn clear_pending_team_membership( + app: &AppHandle, +) -> Result<(), String> { // Write `null` through the link instead of unlinking it. The pending file // is shared by dev worktrees, and `atomic_write_json` preserves the link. save_pending_team_membership_at(&pending_team_membership_path(app)?, None) @@ -112,7 +114,9 @@ fn pending_replay_delta( } /// Replay a staged membership delta. Callers hold `managed_agents_store_lock`. -pub(crate) fn replay_pending_team_membership(app: &AppHandle) -> Result<(), String> { +pub(crate) fn replay_pending_team_membership( + app: &AppHandle, +) -> Result<(), String> { let Some(pending) = load_pending_team_membership(app)? else { return Ok(()); }; @@ -234,25 +238,9 @@ fn propagate_membership_with_roster( Ok(()) } -/// Propagate a team's membership *change* to its members' already-running -/// instances, best-effort: any load/save error is logged and swallowed. -/// -/// Used by [`commit_team_create`] and by the inbound reconcile path. For a -/// create, the team write already landed, and failing the command would make a -/// UI retry mint a duplicate team; a missing backfill only costs a member the -/// team instructions until boot repair runs, and it blocks nothing. -/// -/// [`commit_team_update`] does **not** use this policy. A removal that does not -/// reach the agent store leaves an agent bound to the team, and the delete guard -/// then refuses the team — so an update reports the failure instead. -/// -/// `load_agents`/`save_agents` are injected so the command wiring (prior-roster -/// capture, delta direction, and this best-effort policy) is unit-testable -/// without an `AppHandle`; the commands pass the real store IO. -/// -/// Shared with the inbound reconcile path (`commands::personas::inbound`): a -/// 30176 team edit arriving from another device must bind/detach instances the -/// same way a local edit does, so both call this one wrapper. +/// Apply a membership change without failing a metadata-only producer. The +/// caller must use [`propagate_membership`] when a roster change needs durable +/// recovery. pub(in crate::commands) fn propagate_membership_best_effort( team_id: &str, previous_persona_ids: &[String], @@ -260,32 +248,45 @@ pub(in crate::commands) fn propagate_membership_best_effort( load_agents: impl FnOnce() -> Result, String>, save_agents: impl FnOnce(&[crate::managed_agents::ManagedAgentRecord]) -> Result<(), String>, ) { - if let Err(e) = propagate_membership( + if let Err(error) = propagate_membership( team_id, previous_persona_ids, current_persona_ids, load_agents, save_agents, ) { - eprintln!("buzz-desktop: team-membership-propagate: {e}"); + eprintln!("buzz-desktop: team-membership-propagate: {error}"); } } -/// In-memory core of [`create_team`]: push the built team, persist teams -/// authoritatively, then propagate its whole roster (no prior members ⇒ the -/// whole roster is the added delta) to live instances best-effort. Decoupled -/// from the `AppHandle` shell via injected persistence so the create wiring is -/// unit-testable. A `persist_teams` error propagates; agent IO is best-effort. +/// In-memory core of [`create_team`]: stage, persist, and bind the new team's +/// full roster to already-running instances. The function clears the stage +/// only after both stores succeed. A failure returns an error because a success +/// response must mean the membership binding is durable. fn commit_team_create( teams: &mut Vec, team: TeamRecord, + save_pending: impl FnOnce(&PendingTeamMembershipUpdate) -> Result<(), String>, persist_teams: impl FnOnce(&[TeamRecord]) -> Result<(), String>, load_agents: impl FnOnce() -> Result, String>, save_agents: impl FnOnce(&[crate::managed_agents::ManagedAgentRecord]) -> Result<(), String>, + clear_pending: impl FnOnce() -> Result<(), String>, ) -> Result { + let membership_changed = !team.persona_ids.is_empty(); + if membership_changed { + save_pending(&PendingTeamMembershipUpdate { + team_id: team.id.clone(), + previous_persona_ids: Vec::new(), + current_persona_ids: team.persona_ids.clone(), + })?; + } teams.push(team.clone()); persist_teams(teams)?; - propagate_membership_best_effort(&team.id, &[], &team.persona_ids, load_agents, save_agents); + if membership_changed { + propagate_membership(&team.id, &[], &team.persona_ids, load_agents, save_agents) + .map_err(|error| format!("could not update the new team's agents: {error}"))?; + clear_pending()?; + } Ok(team) } @@ -302,8 +303,8 @@ fn commit_team_create( /// agent write leaves the stage file in place. The next update or launch replays /// that original delta before it accepts another team edit. /// -/// A create has no stable id, so it keeps a best-effort policy to avoid a -/// duplicate team on retry. +/// A create also stages its full roster before it writes the team. A retry +/// replays that stage, so the command never reports a lost member binding. #[allow(clippy::too_many_arguments)] fn commit_team_update( teams: &mut [TeamRecord], @@ -677,11 +678,14 @@ pub async fn create_team(input: CreateTeamRequest, app: AppHandle) -> Result = Vec::new(); - let created = commit_team_create( + let error = commit_team_create( &mut teams, team("team-a", &["duncan"]), |_| Ok(()), + |_| Ok(()), || Ok(vec![instance('a', "duncan", None)]), |_| Err("disk full".to_string()), + || Ok(()), ) - .expect("create swallows secondary-store failure"); - assert_eq!(created.id, "team-a"); + .expect_err("create reports a lost membership binding"); + + assert!(error.contains("could not update the new team's agents")); + assert_eq!( + teams.len(), + 1, + "the team write occurs before the agent write" + ); } } diff --git a/desktop/src-tauri/src/commands/teams/tests/membership_recovery.rs b/desktop/src-tauri/src/commands/teams/tests/membership_recovery.rs index 4860969f057..5477a4515fb 100644 --- a/desktop/src-tauri/src/commands/teams/tests/membership_recovery.rs +++ b/desktop/src-tauri/src/commands/teams/tests/membership_recovery.rs @@ -313,11 +313,13 @@ fn commit_team_create_treats_full_roster_as_added() { &mut teams, team("team-a", &["duncan"]), |_| Ok(()), + |_| Ok(()), || Ok(existing.clone()), |records| { spy.borrow_mut().saved = Some(records.to_vec()); Ok(()) }, + || Ok(()), ) .expect("create succeeds"); @@ -330,21 +332,52 @@ fn commit_team_create_treats_full_roster_as_added() { ); } -/// A failing secondary agent write after successful `save_teams` is -/// swallowed by `create`: it still returns the persisted team. Otherwise a UI -/// retry of a create whose team already landed would mint a duplicate. +/// A failed create keeps its staged delta. The retry path can bind a persona +/// even when another team also contains that persona. #[test] -fn commit_create_returns_ok_when_agent_save_fails() { - let mut teams: Vec = Vec::new(); - let created = commit_team_create( +fn failed_create_keeps_a_durable_shared_persona_replay_delta() { + let pending_file = tempfile::NamedTempFile::new().expect("temporary stage"); + std::fs::write(pending_file.path(), "null").expect("initialize stage"); + let mut teams = vec![team("team-a", &["duncan"])]; + let agents = RefCell::new(vec![instance('a', "duncan", None)]); + + let error = commit_team_create( &mut teams, - team("team-a", &["duncan"]), + team("team-b", &["duncan"]), + |pending| save_pending_team_membership_at(pending_file.path(), Some(pending)), |_| Ok(()), - || Ok(vec![instance('a', "duncan", None)]), + || Ok(agents.borrow().clone()), |_| Err("disk full".to_string()), + || save_pending_team_membership_at(pending_file.path(), None), ) - .expect("create swallows secondary-store failure"); - assert_eq!(created.id, "team-a"); + .expect_err("a create must report an undurable member binding"); + assert!( + error.contains("could not update the new team's agents"), + "{error}" + ); + assert_eq!( + teams.len(), + 2, + "the team write landed before the failed binding" + ); + + let pending = load_pending_team_membership_at(pending_file.path()) + .expect("read staged delta") + .expect("the failed create keeps its stage"); + assert_eq!(pending.team_id, "team-b"); + propagate_membership( + &pending.team_id, + &pending.previous_persona_ids, + &pending.current_persona_ids, + || Ok(agents.borrow().clone()), + |records| { + *agents.borrow_mut() = records.to_vec(); + Ok(()) + }, + ) + .expect("a launch replay binds the shared persona"); + save_pending_team_membership_at(pending_file.path(), None).expect("clear replayed stage"); + assert_eq!(agents.borrow()[0].team_id.as_deref(), Some("team-b")); } /// `update` must NOT swallow an agent-store failure while emptying a roster. @@ -689,11 +722,13 @@ fn replay_before_create_preserves_the_new_team_binding() { &mut teams, team("team-b", &["duncan"]), |_| Ok(()), + |_| Ok(()), || Ok(agents.borrow().clone()), |records| { *agents.borrow_mut() = records.to_vec(); Ok(()) }, + || Ok(()), ) .expect("the new team binds the now-unbound persona"); From 497ea6a69b9ffbd610c6fa55d960455c9e7246b0 Mon Sep 17 00:00:00 2001 From: Storme Drone <49c46e84758b2ebff4abf5abbbd44ee4ce788fc3b55db9fa703eec124eead621@buzz.block.builderlab.xyz> Date: Tue, 1 Sep 2026 10:32:00 -0600 Subject: [PATCH 12/12] fix(desktop): keep migration file size stable Signed-off-by: Storme Drone <49c46e84758b2ebff4abf5abbbd44ee4ce788fc3b55db9fa703eec124eead621@buzz.block.builderlab.xyz> --- desktop/src-tauri/src/migration.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/desktop/src-tauri/src/migration.rs b/desktop/src-tauri/src/migration.rs index dcdbca6dff2..f67f6a63b21 100644 --- a/desktop/src-tauri/src/migration.rs +++ b/desktop/src-tauri/src/migration.rs @@ -25,10 +25,8 @@ const CANONICAL_DEV_IDENTIFIER: &str = "xyz.block.buzz.app.dev"; const LEGACY_CANONICAL_DEV_IDENTIFIER: &str = "xyz.block.sprout.app.dev"; const LEGACY_RELEASE_IDENTIFIER: &str = "xyz.block.sprout.app"; -/// JSON files symlinked from worktree data directories to the canonical -/// dev data directory. Only data files — never `agent-pids/` or `logs/`. -/// `identity.key` is deliberately excluded because worktree instances -/// receive their identity via the `BUZZ_PRIVATE_KEY` env var. +/// JSON files shared through symlinks. `agent-pids/`, `logs/`, and `identity.key` stay local. +/// Worktrees receive the identity through `BUZZ_PRIVATE_KEY`. const SHARED_AGENT_FILES: &[&str] = &[ "agents/managed-agents.json", "agents/pending-team-membership.json", @@ -1368,7 +1366,6 @@ pub fn migrate_persona_provider_to_runtime(app: &tauri::AppHandle) { } mod materialize; pub use materialize::materialize_agent_runtimes; - mod fold; pub use fold::fold_personas_into_agent_store; use fold::load_persona_runtimes;