mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
fix(desktop): repair dropped team membership links at boot and on edit (#5904)
Two membership-propagation defects let an agent team silently lose members — both observed live on Will's store (Sietch Tabr), not hypothetical. **Stale `persona_ids` dropped on save.** Team records written before persona ids were namespaced hold bare slugs (`thufir`) instead of the namespaced id (`sietch-tabr:thufir`). Nothing rewrites them, and the interactive save path (`ensure_persona_ids_are_active`) *drops* any id it cannot resolve — so the next in-app save shrinks the team. This nuked four of five Sietch Tabr members. **`team_id` drifts from team membership.** Team instructions are injected at spawn by matching `record.team_id` (`spawn_snapshot::effective_team_instructions`), so an instance's binding must track its persona's membership. It drifts two ways: adding a persona to a team leaves the persona's already-running instances at `team_id: null` (a member in the roster but not in behavior — seen twice, Gurney and Hayt), and removing a persona while keeping its agents leaves the kept instance bound to a team that no longer lists it (still drawing that team's instructions at spawn). ## Fix A boot migration (`migration/team_membership.rs`) heals existing stores in one pass over `teams.json` + `managed-agents.json`: - **Rewrite stale ids.** A stale id is one no definition slug resolves. Its target is the definition whose `source_team_persona_slug` equals the bare slug, scoped to the team's source team (via `source_dir` for a directory-backed team, or the unique `source_team` among resolvable members for a detached one). Rewrite only when exactly one candidate matches; zero or many leave the id in place — strictly safer than the save path, which drops it. - **Repair `team_id`.** Backfill an instance whose persona is a team member but whose own binding is unset, and heal a stale binding whose team no longer lists the persona (re-point when exactly one *other* team claims it, otherwise unbind). Both directions gate on single-team evidence — a persona spanning several teams has none (JSON team order is not ownership), so it is left as-is and logged. A binding whose team still lists the persona is authoritative and never touched. Runs BEFORE `detach_directory_backed_teams` (so a not-yet-detached team can still be scoped by its `source_dir`) and before any UI save can drop an id. Rewrite-or-leave converges to a fixed point, so a second boot is a no-op; the store is backed up once before either write. The edit path (`commands/teams.rs`) propagates a membership change to live instances immediately, without waiting for the next boot, scoped to the delta between the pre-edit and post-edit rosters: - **Added personas** (on the team now, not before) backfill `team_id` on their unbound instances. An explicit add is legitimate binding evidence even for a persona shared across teams — unlike the order-blind boot case. - **Removed personas** (on the team before, not now) clear `team_id` on instances bound to *this* team (bindings to other teams are untouched), so a "keep agents" removal stops feeding a kept instance the old team's instructions. - **Delta-scoping keeps a metadata-only edit inert:** with no roster change, no instance is re-pointed — a shared unbound persona is never silently bound to whichever team was edited last. Propagation is best-effort after the authoritative `save_teams` (mirroring `retain_team_pending`): the team already exists on disk, and boot repair is the designed retry for a stale/unset binding, so a secondary `managed-agents.json` write failure no longer fails a command whose team write succeeded — which would otherwise let a UI retry mint a duplicate team. --------- Signed-off-by: Will Pfleger <pfleger.will@gmail.com> Co-authored-by: Duncan <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz>
This commit is contained in:
Will Pfleger
@@ -255,8 +255,14 @@ fn reconcile_inbound_persona_event_blocking(
|
||||
}
|
||||
KIND_TEAM => {
|
||||
let mut teams = load_teams(&app)?;
|
||||
apply_inbound_team(&mut teams, d_tag, team_content_from_event(&event)?);
|
||||
save_teams(&app, &teams)?;
|
||||
commit_inbound_team(
|
||||
&mut teams,
|
||||
d_tag,
|
||||
team_content_from_event(&event)?,
|
||||
|teams| save_teams(&app, teams),
|
||||
|| load_managed_agents(&app),
|
||||
|records| save_managed_agents(&app, records),
|
||||
)?;
|
||||
}
|
||||
KIND_MANAGED_AGENT => {
|
||||
let mut agents = load_managed_agents(&app)?;
|
||||
@@ -584,6 +590,53 @@ 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).
|
||||
fn commit_inbound_team(
|
||||
teams: &mut Vec<TeamRecord>,
|
||||
d_tag: String,
|
||||
inbound: TeamEventContent,
|
||||
persist_teams: impl FnOnce(&[TeamRecord]) -> Result<(), String>,
|
||||
load_agents: impl FnOnce() -> Result<Vec<ManagedAgentRecord>, String>,
|
||||
save_agents: impl FnOnce(&[ManagedAgentRecord]) -> Result<(), String>,
|
||||
) -> Result<(), String> {
|
||||
let team_id = d_tag.clone();
|
||||
let previous_persona_ids = teams
|
||||
.iter()
|
||||
.find(|record| record.id == team_id)
|
||||
.map(|record| record.persona_ids.clone())
|
||||
.unwrap_or_default();
|
||||
apply_inbound_team(teams, d_tag, inbound);
|
||||
let current_persona_ids = teams
|
||||
.iter()
|
||||
.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,
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Merge an inbound kind:30176 team projection into the local set.
|
||||
///
|
||||
/// Matches the local record whose `id` equals the event's d-tag (the d-tag IS
|
||||
|
||||
@@ -546,6 +546,176 @@ fn inbound_team_no_match_inserts_idempotently() {
|
||||
assert_eq!(teams.len(), 2, "re-receive of inserted team no-ops");
|
||||
}
|
||||
|
||||
// ── Inbound team → membership propagation (commit_inbound_team wiring) ─────
|
||||
|
||||
use std::cell::RefCell;
|
||||
|
||||
/// A running instance of `persona_id`, optionally bound to a team.
|
||||
fn team_instance(seed: char, persona_id: &str, team_id: Option<&str>) -> ManagedAgentRecord {
|
||||
let mut record = local_agent();
|
||||
record.pubkey = seed.to_string().repeat(64);
|
||||
record.name = persona_id.to_string();
|
||||
record.persona_id = Some(persona_id.to_string());
|
||||
record.team_id = team_id.map(str::to_string);
|
||||
record
|
||||
}
|
||||
|
||||
/// An inbound team edit that ADDS a persona must bind that persona's unbound
|
||||
/// running instances to the team — exactly like a local `update_team`. Without
|
||||
/// the propagation wiring the instance stays unbound (member in roster, not in
|
||||
/// behavior) until restart.
|
||||
#[test]
|
||||
fn inbound_team_add_binds_unbound_instance_through_wiring() {
|
||||
let mut teams = vec![local_team()];
|
||||
teams[0].persona_ids = vec!["p-existing".to_string()];
|
||||
let existing = vec![
|
||||
team_instance('a', "p-added", None),
|
||||
team_instance('b', "p-existing", Some(TEAM_ID)),
|
||||
];
|
||||
let saved = RefCell::new(None);
|
||||
|
||||
commit_inbound_team(
|
||||
&mut teams,
|
||||
TEAM_ID.to_string(),
|
||||
TeamEventContent {
|
||||
name: "Team".to_string(),
|
||||
description: None,
|
||||
instructions: None,
|
||||
persona_ids: Some(vec!["p-existing".to_string(), "p-added".to_string()]),
|
||||
},
|
||||
|_| Ok(()),
|
||||
|| Ok(existing.clone()),
|
||||
|records| {
|
||||
*saved.borrow_mut() = Some(records.to_vec());
|
||||
Ok(())
|
||||
},
|
||||
)
|
||||
.expect("inbound add succeeds");
|
||||
|
||||
let saved = saved
|
||||
.borrow()
|
||||
.clone()
|
||||
.expect("add must save the agent store");
|
||||
assert_eq!(
|
||||
saved[0].team_id.as_deref(),
|
||||
Some(TEAM_ID),
|
||||
"the added persona's unbound instance is bound to the team"
|
||||
);
|
||||
assert_eq!(
|
||||
saved[1].team_id.as_deref(),
|
||||
Some(TEAM_ID),
|
||||
"an instance already on the team is untouched"
|
||||
);
|
||||
}
|
||||
|
||||
/// An inbound team edit that REMOVES a persona ("keep agents") must detach that
|
||||
/// persona's instances bound to this team, so a kept instance stops drawing the
|
||||
/// team's instructions at spawn.
|
||||
#[test]
|
||||
fn inbound_team_removal_detaches_instance_through_wiring() {
|
||||
let mut teams = vec![local_team()];
|
||||
teams[0].persona_ids = vec!["p-removed".to_string()];
|
||||
let existing = vec![team_instance('a', "p-removed", Some(TEAM_ID))];
|
||||
let saved = RefCell::new(None);
|
||||
|
||||
commit_inbound_team(
|
||||
&mut teams,
|
||||
TEAM_ID.to_string(),
|
||||
TeamEventContent {
|
||||
name: "Team".to_string(),
|
||||
description: None,
|
||||
instructions: None,
|
||||
persona_ids: Some(vec![]),
|
||||
},
|
||||
|_| Ok(()),
|
||||
|| Ok(existing.clone()),
|
||||
|records| {
|
||||
*saved.borrow_mut() = Some(records.to_vec());
|
||||
Ok(())
|
||||
},
|
||||
)
|
||||
.expect("inbound removal succeeds");
|
||||
|
||||
let saved = saved
|
||||
.borrow()
|
||||
.clone()
|
||||
.expect("removal must save the agent store");
|
||||
assert_eq!(
|
||||
saved[0].team_id, None,
|
||||
"the removed persona's instance is detached from the team"
|
||||
);
|
||||
}
|
||||
|
||||
/// An inbound edit that omits `persona_ids` (a pre-always-publish client)
|
||||
/// preserves local membership, so the delta is empty and no instance is
|
||||
/// re-pointed — a metadata-only inbound edit must not disturb bindings.
|
||||
#[test]
|
||||
fn inbound_team_omitted_roster_leaves_bindings_untouched() {
|
||||
let mut teams = vec![local_team()];
|
||||
teams[0].persona_ids = vec!["p-a".to_string()];
|
||||
let existing = vec![team_instance('a', "p-a", None)];
|
||||
let saved = RefCell::new(None);
|
||||
|
||||
commit_inbound_team(
|
||||
&mut teams,
|
||||
TEAM_ID.to_string(),
|
||||
team_content_omitting_optional_fields("Renamed"),
|
||||
|_| Ok(()),
|
||||
|| Ok(existing.clone()),
|
||||
|records| {
|
||||
*saved.borrow_mut() = Some(records.to_vec());
|
||||
Ok(())
|
||||
},
|
||||
)
|
||||
.expect("inbound metadata-only edit succeeds");
|
||||
|
||||
assert!(
|
||||
saved.borrow().is_none(),
|
||||
"an empty membership delta writes nothing to the agent store"
|
||||
);
|
||||
}
|
||||
|
||||
/// 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.
|
||||
#[test]
|
||||
fn inbound_team_swallows_agent_store_failure() {
|
||||
let mut teams = vec![local_team()];
|
||||
teams[0].persona_ids = vec![];
|
||||
commit_inbound_team(
|
||||
&mut teams,
|
||||
TEAM_ID.to_string(),
|
||||
TeamEventContent {
|
||||
name: "Team".to_string(),
|
||||
description: None,
|
||||
instructions: None,
|
||||
persona_ids: Some(vec!["p-added".to_string()]),
|
||||
},
|
||||
|_| Ok(()),
|
||||
|| Err("agent store unreadable".to_string()),
|
||||
|_| Ok(()),
|
||||
)
|
||||
.expect("inbound reconcile swallows secondary-store failure");
|
||||
}
|
||||
|
||||
/// A `persist_teams` error propagates — the authoritative team write failing is
|
||||
/// a real reconcile failure, unlike best-effort agent IO.
|
||||
#[test]
|
||||
fn inbound_team_propagates_persist_teams_error() {
|
||||
let mut teams = vec![local_team()];
|
||||
let err = commit_inbound_team(
|
||||
&mut teams,
|
||||
TEAM_ID.to_string(),
|
||||
team_content("Team"),
|
||||
|_| Err("disk full".to_string()),
|
||||
|| Ok(vec![]),
|
||||
|_| Ok(()),
|
||||
)
|
||||
.expect_err("a failed team persist must propagate");
|
||||
assert_eq!(err, "disk full");
|
||||
}
|
||||
|
||||
// ── Tombstone (kind:5) consume ────────────────────────────────────────────
|
||||
|
||||
fn deletion_event(coord: &str) -> nostr::Event {
|
||||
|
||||
@@ -4,8 +4,9 @@ use uuid::Uuid;
|
||||
use crate::{
|
||||
app_state::AppState,
|
||||
managed_agents::{
|
||||
delete_team_with_cascade, ensure_persona_ids_are_active, load_personas, load_teams,
|
||||
save_teams, try_regenerate_nest, CreateTeamRequest, TeamRecord, UpdateTeamRequest,
|
||||
delete_team_with_cascade, ensure_persona_ids_are_active, load_managed_agents,
|
||||
load_personas, load_teams, save_managed_agents, save_teams, try_regenerate_nest,
|
||||
CreateTeamRequest, TeamRecord, UpdateTeamRequest,
|
||||
},
|
||||
util::now_iso,
|
||||
};
|
||||
@@ -25,6 +26,174 @@ fn trim_optional(value: Option<String>) -> Option<String> {
|
||||
})
|
||||
}
|
||||
|
||||
/// 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).
|
||||
///
|
||||
/// `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.
|
||||
pub(in crate::commands) fn propagate_membership_best_effort(
|
||||
team_id: &str,
|
||||
previous_persona_ids: &[String],
|
||||
current_persona_ids: &[String],
|
||||
load_agents: impl FnOnce() -> Result<Vec<crate::managed_agents::ManagedAgentRecord>, 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 {
|
||||
eprintln!("buzz-desktop: team-membership-propagate: {e}");
|
||||
}
|
||||
}
|
||||
|
||||
/// 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.
|
||||
fn commit_team_create(
|
||||
teams: &mut Vec<TeamRecord>,
|
||||
team: TeamRecord,
|
||||
persist_teams: impl FnOnce(&[TeamRecord]) -> Result<(), String>,
|
||||
load_agents: impl FnOnce() -> Result<Vec<crate::managed_agents::ManagedAgentRecord>, String>,
|
||||
save_agents: impl FnOnce(&[crate::managed_agents::ManagedAgentRecord]) -> Result<(), String>,
|
||||
) -> Result<TeamRecord, String> {
|
||||
teams.push(team.clone());
|
||||
persist_teams(teams)?;
|
||||
propagate_membership_best_effort(&team.id, &[], &team.persona_ids, load_agents, save_agents);
|
||||
Ok(team)
|
||||
}
|
||||
|
||||
/// 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
|
||||
/// 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.
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
fn commit_team_update(
|
||||
teams: &mut [TeamRecord],
|
||||
id: &str,
|
||||
name: String,
|
||||
description: Option<String>,
|
||||
instructions: Option<String>,
|
||||
persona_ids: Vec<String>,
|
||||
now: String,
|
||||
persist_teams: impl FnOnce(&[TeamRecord]) -> Result<(), String>,
|
||||
load_agents: impl FnOnce() -> Result<Vec<crate::managed_agents::ManagedAgentRecord>, String>,
|
||||
save_agents: impl FnOnce(&[crate::managed_agents::ManagedAgentRecord]) -> Result<(), String>,
|
||||
) -> Result<TeamRecord, String> {
|
||||
let team = teams
|
||||
.iter_mut()
|
||||
.find(|record| record.id == id)
|
||||
.ok_or_else(|| format!("team {id} not found"))?;
|
||||
|
||||
// Capture the pre-edit roster before mutation: the propagation delta
|
||||
// (added → backfill, removed → detach) is computed against it.
|
||||
let previous_persona_ids = team.persona_ids.clone();
|
||||
team.name = name;
|
||||
team.description = description;
|
||||
team.instructions = instructions;
|
||||
team.persona_ids = persona_ids;
|
||||
team.updated_at = now;
|
||||
|
||||
let updated = team.clone();
|
||||
persist_teams(teams)?;
|
||||
propagate_membership_best_effort(
|
||||
&updated.id,
|
||||
&previous_persona_ids,
|
||||
&updated.persona_ids,
|
||||
load_agents,
|
||||
save_agents,
|
||||
);
|
||||
Ok(updated)
|
||||
}
|
||||
|
||||
/// Pure core of the membership propagation: apply the roster delta to `records`
|
||||
/// in place and report whether anything changed. Decoupled from the store IO so
|
||||
/// the binding rules are unit-testable.
|
||||
///
|
||||
/// Two directions, keyed on the delta between the pre-edit and post-edit
|
||||
/// rosters:
|
||||
///
|
||||
/// - **Added** (`current` but not `previous`): backfill `team_id` on the
|
||||
/// persona's *unbound* instances, so an added persona spawns with the team's
|
||||
/// instructions (`spawn_snapshot::effective_team_instructions` keys on
|
||||
/// `record.team_id`). Only an unset field is set — a shared persona keeps an
|
||||
/// existing binding — and an explicit add is legitimate binding evidence even
|
||||
/// when the persona belongs to several teams.
|
||||
/// - **Removed** (`previous` but not `current`): clear `team_id` on instances
|
||||
/// bound to *this* team, so a "keep agents" removal stops feeding a kept
|
||||
/// instance the instructions of a team it no longer belongs to. Bindings to
|
||||
/// other teams are untouched.
|
||||
///
|
||||
/// Delta-scoping is what keeps a metadata-only edit inert: with no roster
|
||||
/// change both sets are empty and no instance is re-pointed — a shared unbound
|
||||
/// persona is not silently bound to whichever team was last edited. `create`
|
||||
/// has no prior roster, so it passes an empty `previous` and the whole roster is
|
||||
/// "added" (the pre-fix whole-roster backfill). A persona both removed and
|
||||
/// re-added in one edit appears in neither set (set difference, not
|
||||
/// operation order), so its binding is left as-is.
|
||||
fn apply_team_membership_delta(
|
||||
records: &mut [crate::managed_agents::ManagedAgentRecord],
|
||||
team_id: &str,
|
||||
previous_persona_ids: &[String],
|
||||
current_persona_ids: &[String],
|
||||
) -> bool {
|
||||
let added: Vec<&str> = current_persona_ids
|
||||
.iter()
|
||||
.filter(|id| !previous_persona_ids.iter().any(|p| p == *id))
|
||||
.map(String::as_str)
|
||||
.collect();
|
||||
let removed: Vec<&str> = previous_persona_ids
|
||||
.iter()
|
||||
.filter(|id| !current_persona_ids.iter().any(|p| p == *id))
|
||||
.map(String::as_str)
|
||||
.collect();
|
||||
if added.is_empty() && removed.is_empty() {
|
||||
return false;
|
||||
}
|
||||
|
||||
let mut changed = false;
|
||||
for record in records.iter_mut() {
|
||||
if record.pubkey.is_empty() {
|
||||
continue;
|
||||
}
|
||||
let Some(persona_id) = record.persona_id.as_deref() else {
|
||||
continue;
|
||||
};
|
||||
if record.team_id.is_none() && added.contains(&persona_id) {
|
||||
record.team_id = Some(team_id.to_string());
|
||||
changed = true;
|
||||
} else if record.team_id.as_deref() == Some(team_id) && removed.contains(&persona_id) {
|
||||
record.team_id = None;
|
||||
changed = true;
|
||||
}
|
||||
}
|
||||
changed
|
||||
}
|
||||
|
||||
/// Retain a freshly authored team event in the local store, flagged for relay
|
||||
/// sync. Called inside a command's `managed_agents_store_lock`-held body after
|
||||
/// `save_teams`; the background flush loop publishes it out-of-band.
|
||||
@@ -171,8 +340,13 @@ pub async fn create_team(input: CreateTeamRequest, app: AppHandle) -> Result<Tea
|
||||
created_at: now.clone(),
|
||||
updated_at: now,
|
||||
};
|
||||
teams.push(team.clone());
|
||||
save_teams(&app, &teams)?;
|
||||
let team = commit_team_create(
|
||||
&mut teams,
|
||||
team,
|
||||
|teams| save_teams(&app, teams),
|
||||
|| load_managed_agents(&app),
|
||||
|records| save_managed_agents(&app, records),
|
||||
)?;
|
||||
// Created teams are always non-builtin; publish to the relay.
|
||||
retain_team_pending(&app, &state, &team);
|
||||
Ok(team)
|
||||
@@ -197,19 +371,18 @@ pub async fn update_team(input: UpdateTeamRequest, app: AppHandle) -> Result<Tea
|
||||
let personas = load_personas(&app)?;
|
||||
ensure_persona_ids_are_active(&personas, &input.persona_ids)?;
|
||||
let mut teams = load_teams(&app)?;
|
||||
let team = teams
|
||||
.iter_mut()
|
||||
.find(|record| record.id == input.id)
|
||||
.ok_or_else(|| format!("team {} not found", input.id))?;
|
||||
|
||||
team.name = name;
|
||||
team.description = description;
|
||||
team.instructions = instructions;
|
||||
team.persona_ids = input.persona_ids;
|
||||
team.updated_at = now_iso();
|
||||
|
||||
let updated = team.clone();
|
||||
save_teams(&app, &teams)?;
|
||||
let updated = commit_team_update(
|
||||
&mut teams,
|
||||
&input.id,
|
||||
name,
|
||||
description,
|
||||
instructions,
|
||||
input.persona_ids,
|
||||
now_iso(),
|
||||
|teams| save_teams(&app, teams),
|
||||
|| load_managed_agents(&app),
|
||||
|records| save_managed_agents(&app, records),
|
||||
)?;
|
||||
// Built-in teams are not owner-authored — never publish them.
|
||||
if !updated.is_builtin {
|
||||
retain_team_pending(&app, &state, &updated);
|
||||
@@ -220,6 +393,265 @@ pub async fn update_team(input: UpdateTeamRequest, app: AppHandle) -> Result<Tea
|
||||
.map_err(|e| format!("spawn_blocking failed: {e}"))?
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::{apply_team_membership_delta, commit_team_create, commit_team_update};
|
||||
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::<ManagedAgentRecord>(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<String> {
|
||||
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<Vec<ManagedAgentRecord>>,
|
||||
}
|
||||
|
||||
/// 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<TeamRecord> = 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: both commits still return the persisted team. Otherwise a UI
|
||||
/// retry of a create whose team already landed would mint a duplicate.
|
||||
#[test]
|
||||
fn commit_returns_ok_when_agent_save_fails() {
|
||||
let mut teams: Vec<TeamRecord> = 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");
|
||||
|
||||
let mut teams = vec![team("team-a", &["duncan"])];
|
||||
let updated = commit_team_update(
|
||||
&mut teams,
|
||||
"team-a",
|
||||
"team-a".to_string(),
|
||||
None,
|
||||
None,
|
||||
ids(&[]),
|
||||
"2026-02-02T00:00:00Z".to_string(),
|
||||
|_| Ok(()),
|
||||
|| Err("agent store unreadable".to_string()),
|
||||
|_| Ok(()),
|
||||
)
|
||||
.expect("update swallows secondary-store failure");
|
||||
assert_eq!(updated.persona_ids, Vec::<String>::new());
|
||||
}
|
||||
}
|
||||
|
||||
#[tauri::command]
|
||||
pub async fn delete_team(id: String, app: AppHandle) -> Result<(), String> {
|
||||
use tauri::Manager;
|
||||
|
||||
@@ -233,16 +233,39 @@ pub async fn apply_workspace(
|
||||
// Adopt whatever the pre-scoping release left queued in the global
|
||||
// retention database BEFORE the scoped reconcile and flush run, so
|
||||
// stranded tombstones and archive requests publish on this boot
|
||||
// instead of being abandoned by the storage cutover.
|
||||
// instead of being abandoned by the storage cutover. Best-effort:
|
||||
// it is not a prerequisite for the superseding head — the team leg
|
||||
// below builds the repaired roster's head fresh from disk with a
|
||||
// monotonic `created_at` regardless of what the legacy copy left.
|
||||
migrate_legacy_retention_into(&restore_app, &scope);
|
||||
crate::event_sync::spawn_event_sync(
|
||||
// Await the reconcile to completion — do NOT spawn it — and
|
||||
// propagate its failure. The boot migration may have repaired team
|
||||
// membership on disk; the frontend starts inbound history replay
|
||||
// the moment `useCommunityInit` observes the applied workspace, and
|
||||
// an old relay team head could otherwise win that race and overwrite
|
||||
// the repaired `persona_ids`. The team leg is fatal (see
|
||||
// `run_event_sync`): only its success durably retains the corrected
|
||||
// head with a superseding `monotonic_created_at`, so
|
||||
// `retain_inbound_event`'s equal/older guard rejects the stale head.
|
||||
// On failure we return `Err` — the command reports failure,
|
||||
// `useCommunityInit` never exposes the community, and inbound replay
|
||||
// never starts against an un-superseded disk state.
|
||||
crate::event_sync::run_event_sync_blocking(
|
||||
restore_app.clone(),
|
||||
scope.owner_keys,
|
||||
scope.db_path,
|
||||
)
|
||||
.await?;
|
||||
}
|
||||
Err(error) => {
|
||||
eprintln!("buzz-desktop: scoped event-sync unavailable after workspace apply: {error}");
|
||||
// Scope resolution is a prerequisite for establishing the
|
||||
// superseding head, so its failure is fatal for the same reason:
|
||||
// without a scope we cannot retain the repaired roster ahead of an
|
||||
// inbound replay. Fail the command rather than silently opening the
|
||||
// inbound lane.
|
||||
return Err(format!(
|
||||
"scoped event-sync unavailable after workspace apply: {error}"
|
||||
));
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -13,32 +13,44 @@ use std::path::Path;
|
||||
/// `sync_team_personas` wrote in [`crate::migration::run_boot_migrations`]
|
||||
/// (see its `# Ordering` guard). Event signing needs the resolved owner keys,
|
||||
/// so this runs after identity resolution, not in the boot migrations.
|
||||
pub fn run_event_sync(app: &tauri::AppHandle, owner_keys: &nostr::Keys, db_path: &Path) {
|
||||
pub fn run_event_sync(
|
||||
app: &tauri::AppHandle,
|
||||
owner_keys: &nostr::Keys,
|
||||
db_path: &Path,
|
||||
) -> Result<(), String> {
|
||||
// Persona and agent legs stay best-effort: they log and swallow, and their
|
||||
// failure does not undo the boot team-membership repair. The team leg is
|
||||
// fatal — it establishes the superseding local head (a monotonic
|
||||
// `created_at`) that lets `retain_inbound_event`'s equal/older guard reject
|
||||
// a stale relay roster. If it fails, the caller must not let the frontend
|
||||
// expose the community and start inbound replay against an un-superseded
|
||||
// disk state.
|
||||
migrate_personas_to_events(app, owner_keys, db_path);
|
||||
migrate_teams_to_events(app, owner_keys, db_path);
|
||||
migrate_teams_to_events(app, owner_keys, db_path)?;
|
||||
crate::managed_agents::reconcile::reconcile_agents_to_events(app, owner_keys, db_path);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Spawn the best-effort event reconcile off the synchronous Tauri setup path.
|
||||
/// Run the scoped event reconcile to completion on the blocking pool.
|
||||
///
|
||||
/// The owner keys are cloned before spawning so the task never touches the
|
||||
/// `AppState::keys` mutex. The reconcile itself is still synchronous JSON,
|
||||
/// SQLite, and signing work, so it runs on the blocking pool rather than an
|
||||
/// async worker.
|
||||
pub fn spawn_event_sync(
|
||||
/// Callers that must not let downstream work observe a not-yet-retained disk
|
||||
/// state (e.g. `apply_workspace` before the frontend can start inbound history
|
||||
/// replay) await this so the repaired local heads are durably retained — with a
|
||||
/// superseding `monotonic_created_at` — before an old relay head can race in.
|
||||
/// The owner keys are moved in so the task never touches the `AppState::keys`
|
||||
/// mutex; the reconcile itself is synchronous JSON/SQLite/signing work, so it
|
||||
/// runs on the blocking pool rather than an async worker.
|
||||
///
|
||||
/// Returns `Err` if the task fails to join or the fatal team leg errors, so the
|
||||
/// caller can withhold community exposure until the superseding head is durable.
|
||||
pub async fn run_event_sync_blocking(
|
||||
app: tauri::AppHandle,
|
||||
owner_keys: nostr::Keys,
|
||||
db_path: std::path::PathBuf,
|
||||
) {
|
||||
tauri::async_runtime::spawn(async move {
|
||||
if let Err(e) = tauri::async_runtime::spawn_blocking(move || {
|
||||
run_event_sync(&app, &owner_keys, &db_path);
|
||||
})
|
||||
) -> Result<(), String> {
|
||||
tauri::async_runtime::spawn_blocking(move || run_event_sync(&app, &owner_keys, &db_path))
|
||||
.await
|
||||
{
|
||||
eprintln!("buzz-desktop: event-sync: spawn_blocking failed: {e}");
|
||||
}
|
||||
});
|
||||
.map_err(|e| format!("event-sync: spawn_blocking failed: {e}"))?
|
||||
}
|
||||
|
||||
/// Reconcile `personas.json` into the persona-event retention store.
|
||||
@@ -219,21 +231,23 @@ fn migrate_personas_in_dir_at(
|
||||
///
|
||||
/// Must run after the persisted identity is resolved (it signs each event with
|
||||
/// the owner's keys).
|
||||
pub fn migrate_teams_to_events(app: &tauri::AppHandle, keys: &nostr::Keys, db_path: &Path) {
|
||||
pub fn migrate_teams_to_events(
|
||||
app: &tauri::AppHandle,
|
||||
keys: &nostr::Keys,
|
||||
db_path: &Path,
|
||||
) -> Result<(), String> {
|
||||
use crate::managed_agents::managed_agents_base_dir;
|
||||
|
||||
let Ok(base_dir) = managed_agents_base_dir(app) else {
|
||||
return;
|
||||
};
|
||||
let base_dir = managed_agents_base_dir(app)
|
||||
.map_err(|e| format!("team-event-migration: base dir unavailable: {e}"))?;
|
||||
|
||||
match migrate_teams_in_dir_at(&base_dir, keys, db_path) {
|
||||
Ok(0) => {}
|
||||
Ok(0) => Ok(()),
|
||||
Ok(migrated) => {
|
||||
eprintln!("buzz-desktop: team-event-migration: {migrated} teams migrated to retention");
|
||||
Ok(())
|
||||
}
|
||||
Err(e) => {
|
||||
eprintln!("buzz-desktop: team-event-migration: {e}");
|
||||
}
|
||||
Err(e) => Err(format!("team-event-migration: {e}")),
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -133,3 +133,132 @@ fn migrate_teams_no_file_is_noop() {
|
||||
let keys = nostr::Keys::generate();
|
||||
assert_eq!(migrate_teams_in_dir(base.path(), &keys).unwrap(), 0);
|
||||
}
|
||||
|
||||
/// Error-contract for the fatal team leg. `run_event_sync` propagates a team
|
||||
/// leg failure via `?`, and `apply_workspace` returns that `Err` so the
|
||||
/// frontend never exposes the community against an un-superseded disk state.
|
||||
/// This proves the leg genuinely surfaces failure (rather than logging and
|
||||
/// swallowing) on an unreadable store — the precondition that made the
|
||||
/// propagation load-bearing.
|
||||
#[test]
|
||||
fn migrate_teams_surfaces_error_on_unparseable_store() {
|
||||
let base = tempfile::tempdir().unwrap();
|
||||
std::fs::write(base.path().join("teams.json"), "{ not valid json").unwrap();
|
||||
let keys = nostr::Keys::generate();
|
||||
assert!(migrate_teams_in_dir(base.path(), &keys).is_err());
|
||||
}
|
||||
|
||||
/// Build a signed inbound team head at an explicit `created_at`, mirroring a
|
||||
/// relay replay of a stale, pre-namespacing roster.
|
||||
fn stale_inbound_head(
|
||||
keys: &nostr::Keys,
|
||||
id: &str,
|
||||
bare_persona_ids: &[&str],
|
||||
created_at: i64,
|
||||
) -> crate::managed_agents::retention::RetainedEvent {
|
||||
use crate::managed_agents::{team_events::build_team_event, TeamRecord};
|
||||
use buzz_core_pkg::kind::KIND_TEAM;
|
||||
use nostr::JsonUtil;
|
||||
|
||||
let record = TeamRecord {
|
||||
id: id.to_string(),
|
||||
name: "Sietch Tabr".to_string(),
|
||||
description: None,
|
||||
instructions: None,
|
||||
persona_ids: bare_persona_ids.iter().map(|s| s.to_string()).collect(),
|
||||
is_builtin: false,
|
||||
source_dir: None,
|
||||
is_symlink: false,
|
||||
symlink_target: None,
|
||||
version: None,
|
||||
created_at: "2025-01-01T00:00:00Z".to_string(),
|
||||
updated_at: "2025-01-01T00:00:00Z".to_string(),
|
||||
};
|
||||
let event = build_team_event(&record)
|
||||
.unwrap()
|
||||
.custom_created_at(nostr::Timestamp::from(created_at as u64))
|
||||
.sign_with_keys(keys)
|
||||
.unwrap();
|
||||
crate::managed_agents::retention::RetainedEvent {
|
||||
kind: KIND_TEAM,
|
||||
pubkey: keys.public_key().to_hex(),
|
||||
d_tag: id.to_string(),
|
||||
content: event.content.to_string(),
|
||||
created_at: event.created_at.as_secs() as i64,
|
||||
raw_event: event.as_json(),
|
||||
pending_sync: false,
|
||||
}
|
||||
}
|
||||
|
||||
/// Finding-1 retention-precedence guarantee. This proves the *mechanic* the
|
||||
/// awaited-reconcile ordering relies on — it does not itself exercise
|
||||
/// `apply_workspace` (an `AppHandle`-level path). Given the boot reconcile has
|
||||
/// retained the repaired namespaced roster with a monotonic `created_at`
|
||||
/// (reconcile-first), a stale relay head replayed afterward is older, so
|
||||
/// `retain_inbound_event` skips it and the repaired roster stays. The
|
||||
/// inbound-first lane is the counterfactual the ordering closes: with no
|
||||
/// repaired head retained yet, the very same stale head is applied and restores
|
||||
/// bare membership. Retention order is the only difference between the lanes;
|
||||
/// `apply_workspace` awaiting the reconcile (see `commands/workspace.rs`) is
|
||||
/// what forces the reconcile-first order in production.
|
||||
#[test]
|
||||
fn reconcile_first_makes_stale_inbound_team_head_lose() {
|
||||
use crate::managed_agents::retention::{
|
||||
get_retained_event, open_retention_db, retain_inbound_event, InboundOutcome,
|
||||
};
|
||||
use buzz_core_pkg::kind::KIND_TEAM;
|
||||
|
||||
let keys = nostr::Keys::generate();
|
||||
let pubkey = keys.public_key().to_hex();
|
||||
let repaired = serde_json::json!([{
|
||||
"id": "sietch-tabr",
|
||||
"name": "Sietch Tabr",
|
||||
"persona_ids": ["sietch-tabr:thufir", "sietch-tabr:paul", "sietch-tabr:duncan"],
|
||||
"is_builtin": false,
|
||||
"created_at": "2025-01-01T00:00:00Z",
|
||||
"updated_at": "2025-01-01T00:00:00Z"
|
||||
}]);
|
||||
let bare = ["thufir", "paul", "duncan"];
|
||||
|
||||
// Reconcile-first lane (the fix): the awaited boot reconcile retains the
|
||||
// repaired namespaced roster with a monotonic `created_at`; a stale relay
|
||||
// head replayed afterward is older, so `retain_inbound_event` skips it and
|
||||
// the retained roster stays repaired.
|
||||
let ordered = tempfile::tempdir().unwrap();
|
||||
let ordered_db = ordered.path().join("retention.db");
|
||||
write_base_teams(ordered.path(), &repaired);
|
||||
assert_eq!(
|
||||
migrate_teams_in_dir_at(ordered.path(), &keys, &ordered_db).unwrap(),
|
||||
1
|
||||
);
|
||||
let conn = open_retention_db(&ordered_db).unwrap();
|
||||
let repaired_head = get_retained_event(&conn, KIND_TEAM, &pubkey, "sietch-tabr")
|
||||
.unwrap()
|
||||
.unwrap();
|
||||
let stale = stale_inbound_head(&keys, "sietch-tabr", &bare, repaired_head.created_at - 1);
|
||||
assert_eq!(
|
||||
retain_inbound_event(&conn, &stale).unwrap(),
|
||||
InboundOutcome::Skipped
|
||||
);
|
||||
let head = get_retained_event(&conn, KIND_TEAM, &pubkey, "sietch-tabr")
|
||||
.unwrap()
|
||||
.unwrap();
|
||||
assert!(head.content.contains("sietch-tabr:thufir"));
|
||||
assert!(!head.content.contains("\"thufir\""));
|
||||
|
||||
// Inbound-first lane (the race the fix closes): with no repaired head
|
||||
// retained yet, the very same stale relay head is applied, restoring the
|
||||
// bare pre-namespacing roster. Ordering is the only difference.
|
||||
let raced = tempfile::tempdir().unwrap();
|
||||
let raced_db = raced.path().join("retention.db");
|
||||
let raced_conn = open_retention_db(&raced_db).unwrap();
|
||||
let stale = stale_inbound_head(&keys, "sietch-tabr", &bare, repaired_head.created_at - 1);
|
||||
assert_eq!(
|
||||
retain_inbound_event(&raced_conn, &stale).unwrap(),
|
||||
InboundOutcome::Applied
|
||||
);
|
||||
let head = get_retained_event(&raced_conn, KIND_TEAM, &pubkey, "sietch-tabr")
|
||||
.unwrap()
|
||||
.unwrap();
|
||||
assert!(head.content.contains("\"thufir\""));
|
||||
}
|
||||
|
||||
@@ -39,6 +39,7 @@ pub(crate) mod spawn_snapshot;
|
||||
pub(crate) mod storage;
|
||||
pub(crate) mod team_events;
|
||||
mod team_repair;
|
||||
pub(crate) use team_repair::team_persona_key;
|
||||
mod teams;
|
||||
mod types;
|
||||
|
||||
|
||||
@@ -149,8 +149,7 @@ fn run_boot_migrations_inner(app: &tauri::AppHandle, reset_completed: bool) {
|
||||
// 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 the gate + migration run through the same
|
||||
// code path that the behavioral test exercises.
|
||||
// Uses the composed helper so gate + migration share 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);
|
||||
}
|
||||
@@ -181,11 +180,12 @@ fn run_boot_migrations_inner(app: &tauri::AppHandle, reset_completed: bool) {
|
||||
strip_baked_team_instructions(app);
|
||||
refresh_builtin_agent_avatars(app);
|
||||
// B5: manufacture definitions for standalone agents AFTER the fold (so
|
||||
// pre-existing definition slugs are present for collision checks) and
|
||||
// before event sync republishes — the backfilled link is what flips the
|
||||
// 30177 projection to its slim shape.
|
||||
// pre-existing definition slugs exist for collision checks) and before event
|
||||
// sync republishes — the backfilled link flips the 30177 projection.
|
||||
backfill_standalone_agents(app);
|
||||
detach_directory_backed_teams(app);
|
||||
// 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);
|
||||
reconcile_provider_mcp_commands(app);
|
||||
reconcile_databricks_v1_to_v2(app);
|
||||
materialize_agent_runtimes(app);
|
||||
@@ -1373,8 +1373,8 @@ use fold::load_persona_runtimes;
|
||||
mod backfill;
|
||||
pub use backfill::backfill_standalone_agents;
|
||||
mod detach;
|
||||
pub use detach::detach_directory_backed_teams;
|
||||
mod pollen;
|
||||
mod team_membership;
|
||||
pub(crate) use pollen::*;
|
||||
mod team_suffix;
|
||||
pub use team_suffix::strip_baked_team_instructions;
|
||||
|
||||
@@ -9,10 +9,12 @@ use crate::managed_agents::{ManagedAgentRecord, TeamRecord};
|
||||
/// Lift pack instructions into `TeamRecord.instructions` and detach
|
||||
/// directory-backed teams from their source directories.
|
||||
///
|
||||
/// Runs on app launch if any `TeamRecord` still has `source_dir` set.
|
||||
/// Both output files are written atomically (temp-file + rename), so a crash
|
||||
/// mid-write leaves the previous version intact and the migration can safely
|
||||
/// retry on next boot.
|
||||
/// Core logic, decoupled from the Tauri `AppHandle` for testing.
|
||||
///
|
||||
/// Runs on app launch (gated on a clean team-membership repair) if any
|
||||
/// `TeamRecord` still has `source_dir` set. Both output files are written
|
||||
/// atomically (temp-file + rename), so a crash mid-write leaves the previous
|
||||
/// version intact and the migration can safely retry on next boot.
|
||||
///
|
||||
/// Steps (written last so the idempotency gate stays open until both files
|
||||
/// are committed):
|
||||
@@ -24,18 +26,6 @@ use crate::managed_agents::{ManagedAgentRecord, TeamRecord};
|
||||
/// `instructions` if the field is not already set.
|
||||
/// 4. Clear `source_dir`, `is_symlink`, `symlink_target`, `version` on each
|
||||
/// directory-backed `TeamRecord`.
|
||||
pub fn detach_directory_backed_teams(app: &tauri::AppHandle) {
|
||||
let Ok(base_dir) = crate::managed_agents::managed_agents_base_dir(app) else {
|
||||
return;
|
||||
};
|
||||
match detach_directory_backed_teams_in_dir(&base_dir) {
|
||||
Ok(0) => {}
|
||||
Ok(n) => eprintln!("buzz-desktop: detach-dir-teams: detached {n} directory-backed team(s)"),
|
||||
Err(e) => eprintln!("buzz-desktop: detach-dir-teams: {e}"),
|
||||
}
|
||||
}
|
||||
|
||||
/// Core logic, decoupled from the Tauri `AppHandle` for testing.
|
||||
///
|
||||
/// `base_dir` is the managed-agents base directory (`<AppDataDir>/agents/`).
|
||||
/// Returns the number of teams detached (0 = nothing to do).
|
||||
|
||||
@@ -0,0 +1,353 @@
|
||||
//! Repair team↔member links that a membership edit failed to propagate.
|
||||
//!
|
||||
//! Two independent defects, both rooted in a team-membership change not
|
||||
//! reaching the records that depend on it, are healed in one pass over
|
||||
//! `teams.json` + `managed-agents.json`:
|
||||
//!
|
||||
//! 1. **Stale `persona_ids`.** Team records written before persona ids were
|
||||
//! namespaced hold bare slugs (`thufir`) instead of the namespaced id
|
||||
//! (`sietch-tabr:thufir`). Nothing rewrites them, and the interactive save
|
||||
//! path (`ensure_persona_ids_are_active`) *drops* an id it cannot resolve —
|
||||
//! silently shrinking the team. This migration rewrites a stale id to the
|
||||
//! persona it names whenever that persona is unambiguous, and — unlike the
|
||||
//! save path — never drops one it cannot resolve.
|
||||
//!
|
||||
//! 2. **Orphaned or stale instance `team_id`.** Team instructions are injected
|
||||
//! at spawn by matching `record.team_id`
|
||||
//! (`spawn_snapshot::effective_team_instructions`), so an instance's binding
|
||||
//! must track its persona's membership. Two ways it drifts: adding a persona
|
||||
//! to a team does not backfill `team_id` on that persona's already-running
|
||||
//! instances (a member in the roster but not in behavior), and removing a
|
||||
//! persona while keeping its agents leaves the binding pointing at a team
|
||||
//! that no longer lists it (still drawing that team's instructions). This
|
||||
//! backfills an unset binding and heals a stale one — always on the same
|
||||
//! single-team evidence rule, never guessing across teams.
|
||||
//!
|
||||
//! The stale-id rewrite is strictly additive (rewrite-or-leave); the binding
|
||||
//! repair converges to a fixed point (bound-to-a-listing-team or unbound), so a
|
||||
//! second boot is a clean no-op either way. Runs BEFORE
|
||||
//! `detach_directory_backed_teams` so a not-yet-detached directory-backed team
|
||||
//! can still be scoped by its `source_dir`, and before any UI save can drop an
|
||||
//! unresolvable id.
|
||||
|
||||
use std::collections::HashMap;
|
||||
use std::path::Path;
|
||||
|
||||
use crate::managed_agents::{team_persona_key, ManagedAgentRecord, TeamRecord};
|
||||
|
||||
/// Repair stale team `persona_ids`/instance `team_id`, then detach
|
||||
/// directory-backed teams — but only when the repair succeeded.
|
||||
///
|
||||
/// `repair` clears no `source_dir`; the downstream detach does. A stale bare
|
||||
/// slug shared across source teams is disambiguated by `source_dir`, so if
|
||||
/// repair fails (its backup or write errored) and detach still ran, the next
|
||||
/// boot would see only ambiguous candidates and the original membership-loss
|
||||
/// path recurs. Gating detach on a clean repair preserves `source_dir` as retry
|
||||
/// evidence for that boot; the next boot retries repair and, once clean,
|
||||
/// detaches.
|
||||
pub(super) fn repair_then_detach_teams(app: &tauri::AppHandle) {
|
||||
let Ok(base_dir) = crate::managed_agents::managed_agents_base_dir(app) else {
|
||||
return;
|
||||
};
|
||||
orchestrate_repair_then_detach(
|
||||
|| repair_team_membership_in_dir(&base_dir),
|
||||
|| super::detach::detach_directory_backed_teams_in_dir(&base_dir),
|
||||
);
|
||||
}
|
||||
|
||||
/// 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.
|
||||
fn orchestrate_repair_then_detach(
|
||||
repair: impl FnOnce() -> Result<usize, String>,
|
||||
detach: impl FnOnce() -> Result<usize, String>,
|
||||
) {
|
||||
match repair() {
|
||||
Ok(repaired) => {
|
||||
if repaired > 0 {
|
||||
eprintln!("buzz-desktop: team-membership-repair: repaired {repaired} record(s)");
|
||||
}
|
||||
match detach() {
|
||||
Ok(0) => {}
|
||||
Ok(n) => {
|
||||
eprintln!(
|
||||
"buzz-desktop: detach-dir-teams: detached {n} directory-backed team(s)"
|
||||
)
|
||||
}
|
||||
Err(e) => eprintln!("buzz-desktop: detach-dir-teams: {e}"),
|
||||
}
|
||||
}
|
||||
Err(e) => eprintln!(
|
||||
"buzz-desktop: team-membership-repair: {e} — skipping directory-backed detach this \
|
||||
boot to preserve source_dir for a clean-repair retry"
|
||||
),
|
||||
}
|
||||
}
|
||||
|
||||
/// Core logic, decoupled from the Tauri `AppHandle` for testing.
|
||||
///
|
||||
/// `base_dir` is the managed-agents base directory (`<AppDataDir>/agents/`).
|
||||
/// Returns the number of records changed across both files (0 = nothing to do,
|
||||
/// nothing written, so a re-run is a clean no-op).
|
||||
pub(super) fn repair_team_membership_in_dir(base_dir: &Path) -> Result<usize, String> {
|
||||
let teams_path = base_dir.join("teams.json");
|
||||
let agents_path = base_dir.join("managed-agents.json");
|
||||
|
||||
// Definitions and teams both live in these two files; without either there
|
||||
// is nothing to link.
|
||||
if !teams_path.exists() || !agents_path.exists() {
|
||||
return Ok(0);
|
||||
}
|
||||
|
||||
let teams_content = std::fs::read_to_string(&teams_path)
|
||||
.map_err(|e| format!("failed to read teams.json: {e}"))?;
|
||||
let mut teams: Vec<TeamRecord> = serde_json::from_str(&teams_content)
|
||||
.map_err(|e| format!("failed to parse teams.json: {e}"))?;
|
||||
|
||||
let agents_content = std::fs::read_to_string(&agents_path)
|
||||
.map_err(|e| format!("failed to read managed-agents.json: {e}"))?;
|
||||
let mut agents: Vec<ManagedAgentRecord> = serde_json::from_str(&agents_content)
|
||||
.map_err(|e| format!("failed to parse managed-agents.json: {e}"))?;
|
||||
|
||||
let rewrites = rewrite_stale_persona_ids(&mut teams, &agents);
|
||||
let backfills = backfill_instance_team_ids(&teams, &mut agents);
|
||||
|
||||
if rewrites == 0 && backfills == 0 {
|
||||
return Ok(0);
|
||||
}
|
||||
|
||||
// Pre-migration backups, both taken BEFORE either live store write: the
|
||||
// stated contract is a full recovery pair even if a crash lands between the
|
||||
// two writes, so neither store may be rewritten until both pristine backups
|
||||
// exist. A stale bare slug shared across source teams is disambiguated by
|
||||
// `source_dir`, which the downstream detach clears — so the pristine
|
||||
// pre-repair `teams.json` is the evidence a retry needs. Each backup is
|
||||
// created once (create-new), so a re-run after a partial failure never
|
||||
// overwrites the pristine copy with a half-migrated snapshot.
|
||||
if rewrites > 0 {
|
||||
let bak = crate::util::resolved_backup_path(
|
||||
&teams_path,
|
||||
"teams.json.pre-team-membership-repair.bak",
|
||||
);
|
||||
crate::util::create_restricted_backup_once(&bak, teams_content.as_bytes())
|
||||
.map_err(|e| format!("failed to write teams.json backup: {e}"))?;
|
||||
}
|
||||
if backfills > 0 {
|
||||
let bak = crate::util::resolved_backup_path(
|
||||
&agents_path,
|
||||
"managed-agents.json.pre-team-membership-repair.bak",
|
||||
);
|
||||
crate::util::create_restricted_backup_once(&bak, agents_content.as_bytes())
|
||||
.map_err(|e| format!("failed to write managed-agents.json backup: {e}"))?;
|
||||
}
|
||||
|
||||
if rewrites > 0 {
|
||||
let payload = serde_json::to_vec_pretty(&teams)
|
||||
.map_err(|e| format!("failed to serialize teams.json: {e}"))?;
|
||||
crate::managed_agents::atomic_write_json(&teams_path, &payload)?;
|
||||
}
|
||||
|
||||
if backfills > 0 {
|
||||
// Restricted: this store can carry plaintext agent nsecs on a
|
||||
// keyringless host (SECURITY.md:90).
|
||||
let payload = serde_json::to_vec_pretty(&agents)
|
||||
.map_err(|e| format!("failed to serialize managed-agents.json: {e}"))?;
|
||||
crate::managed_agents::atomic_write_json_restricted(&agents_path, &payload)?;
|
||||
}
|
||||
|
||||
Ok(rewrites + backfills)
|
||||
}
|
||||
|
||||
/// Set of persona ids that resolve to a definition — the definition records are
|
||||
/// the key-less unified-store entries (`pubkey == ""`); their `slug` is the id
|
||||
/// a team references.
|
||||
fn resolvable_ids(agents: &[ManagedAgentRecord]) -> Vec<&str> {
|
||||
agents
|
||||
.iter()
|
||||
.filter(|r| r.pubkey.is_empty())
|
||||
.filter_map(|r| r.slug.as_deref())
|
||||
.collect()
|
||||
}
|
||||
|
||||
/// Rewrite each team's stale `persona_ids` to the persona they name, when
|
||||
/// unambiguous. Returns the number of ids rewritten.
|
||||
///
|
||||
/// An id is *stale* when no definition slug equals it. Its repair target is the
|
||||
/// definition whose `source_team_persona_slug` equals the stale id — i.e. the
|
||||
/// bare slug is the pre-namespacing form of that persona's namespaced slug. The
|
||||
/// rewrite happens only when exactly one such definition exists (optionally
|
||||
/// scoped to the team's source team); zero or many candidates leave the id
|
||||
/// untouched, which is strictly safer than the save path that drops it.
|
||||
fn rewrite_stale_persona_ids(teams: &mut [TeamRecord], agents: &[ManagedAgentRecord]) -> usize {
|
||||
let resolvable = resolvable_ids(agents);
|
||||
let definitions: Vec<&ManagedAgentRecord> =
|
||||
agents.iter().filter(|r| r.pubkey.is_empty()).collect();
|
||||
|
||||
let mut rewritten = 0usize;
|
||||
for team in teams.iter_mut() {
|
||||
// Scope candidate personas to this team's source team when derivable:
|
||||
// a directory-backed team keys off its source_dir name; a detached team
|
||||
// keys off the unique source_team of its already-resolvable members.
|
||||
let scope = team_source_scope(team, &definitions);
|
||||
for id in team.persona_ids.iter_mut() {
|
||||
if resolvable.contains(&id.as_str()) {
|
||||
continue;
|
||||
}
|
||||
let candidates: Vec<&&ManagedAgentRecord> = definitions
|
||||
.iter()
|
||||
.filter(|d| d.source_team_persona_slug.as_deref() == Some(id.as_str()))
|
||||
.filter(|d| match scope.as_deref() {
|
||||
Some(team_key) => d.source_team.as_deref() == Some(team_key),
|
||||
None => true,
|
||||
})
|
||||
.collect();
|
||||
let [only] = candidates.as_slice() else {
|
||||
eprintln!(
|
||||
"buzz-desktop: team-membership-repair: team {:?}: leaving unresolvable \
|
||||
persona id {:?} ({} candidate(s))",
|
||||
team.id,
|
||||
id,
|
||||
candidates.len()
|
||||
);
|
||||
continue;
|
||||
};
|
||||
if let Some(slug) = only.slug.as_deref() {
|
||||
*id = slug.to_string();
|
||||
rewritten += 1;
|
||||
}
|
||||
}
|
||||
}
|
||||
rewritten
|
||||
}
|
||||
|
||||
/// The source-team key that scopes a team's persona candidates, or `None` when
|
||||
/// it cannot be derived (matching then falls back to a global unique slug).
|
||||
///
|
||||
/// Directory-backed teams use `team_persona_key` (the pack manifest id). A
|
||||
/// detached team (`source_dir` cleared) has no such key, so we infer it from
|
||||
/// the unique `source_team` among its members that already resolve.
|
||||
fn team_source_scope(team: &TeamRecord, definitions: &[&ManagedAgentRecord]) -> Option<String> {
|
||||
if team.source_dir.is_some() {
|
||||
return Some(team_persona_key(team).to_string());
|
||||
}
|
||||
let mut source_teams: Vec<&str> = team
|
||||
.persona_ids
|
||||
.iter()
|
||||
.filter_map(|id| {
|
||||
definitions
|
||||
.iter()
|
||||
.find(|d| d.slug.as_deref() == Some(id.as_str()))
|
||||
.and_then(|d| d.source_team.as_deref())
|
||||
})
|
||||
.collect();
|
||||
source_teams.sort_unstable();
|
||||
source_teams.dedup();
|
||||
match source_teams.as_slice() {
|
||||
[only] => Some((*only).to_string()),
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
|
||||
/// Repair instance `team_id` against the current rosters. Returns the number of
|
||||
/// instances changed.
|
||||
///
|
||||
/// Two directions, both conservative and evidence-gated:
|
||||
///
|
||||
/// - **Unbound → bound (backfill).** An instance whose persona is a team member
|
||||
/// but whose own `team_id` is unset is bound to that team, so it spawns with
|
||||
/// the team's instructions. Only when the persona belongs to *exactly one*
|
||||
/// team — a persona spanning several teams has no evidence selecting one
|
||||
/// (JSON team order is not ownership), so it is left unbound and logged.
|
||||
/// - **Stale binding → cleared or re-pointed.** An instance bound to a team
|
||||
/// whose roster no longer lists its persona (a "keep agents" removal left the
|
||||
/// binding behind, so the kept instance keeps drawing that team's
|
||||
/// instructions at spawn) is healed: re-pointed when the persona now belongs
|
||||
/// to exactly one *other* team (same single-evidence rule), otherwise unbound
|
||||
/// and logged. A binding whose team still lists the persona is authoritative
|
||||
/// and never touched.
|
||||
///
|
||||
/// Idempotent: after a repair every instance is either bound to a team that
|
||||
/// lists it or unbound with no single-team evidence, so a second pass is a
|
||||
/// no-op.
|
||||
fn backfill_instance_team_ids(teams: &[TeamRecord], agents: &mut [ManagedAgentRecord]) -> usize {
|
||||
// persona_id → the sole team referencing it, or None once a *distinct*
|
||||
// second team is seen (ambiguous → never used as binding evidence). A
|
||||
// persona listed twice within one team is not ambiguity — duplicates are
|
||||
// not prohibited at the storage boundary (`ensure_persona_ids_are_active`
|
||||
// checks existence only; create/update/inbound persist the vector
|
||||
// unchanged), so poisoning on a same-team repeat would strand a
|
||||
// legitimately single-team instance.
|
||||
let mut persona_to_team: HashMap<&str, Option<&str>> = HashMap::new();
|
||||
// Team ids that exist in the store, and the (team_id, persona_id) pairs they
|
||||
// list. A binding is *stale* only when its team still exists but no longer
|
||||
// lists the persona — a binding to an absent team is left alone (it already
|
||||
// degrades to no instructions via `effective_team_instructions`, and a
|
||||
// deleted team is not this repair's concern).
|
||||
let mut team_ids: std::collections::HashSet<&str> = std::collections::HashSet::new();
|
||||
let mut membership: std::collections::HashSet<(&str, &str)> = std::collections::HashSet::new();
|
||||
for team in teams {
|
||||
team_ids.insert(team.id.as_str());
|
||||
for persona_id in &team.persona_ids {
|
||||
membership.insert((team.id.as_str(), persona_id.as_str()));
|
||||
persona_to_team
|
||||
.entry(persona_id.as_str())
|
||||
.and_modify(|slot| {
|
||||
if slot.is_some_and(|seen| seen != team.id.as_str()) {
|
||||
*slot = None;
|
||||
}
|
||||
})
|
||||
.or_insert(Some(team.id.as_str()));
|
||||
}
|
||||
}
|
||||
|
||||
let mut repaired = 0usize;
|
||||
for agent in agents.iter_mut() {
|
||||
if agent.pubkey.is_empty() {
|
||||
continue;
|
||||
}
|
||||
let Some(persona_id) = agent.persona_id.as_deref() else {
|
||||
continue;
|
||||
};
|
||||
match agent.team_id.as_deref() {
|
||||
// Live binding, or a binding to an absent team: leave it. A binding
|
||||
// is only stale when its team exists and dropped the persona.
|
||||
Some(bound)
|
||||
if !team_ids.contains(bound) || membership.contains(&(bound, persona_id)) => {}
|
||||
// Stale binding: the still-present bound team dropped this persona.
|
||||
// Re-point on single-team evidence, else unbind — never guess.
|
||||
Some(_) => match persona_to_team.get(persona_id) {
|
||||
Some(Some(team_id)) => {
|
||||
agent.team_id = Some((*team_id).to_string());
|
||||
repaired += 1;
|
||||
}
|
||||
_ => {
|
||||
eprintln!(
|
||||
"buzz-desktop: team-membership-repair: unbinding instance {:?} — persona \
|
||||
{persona_id:?} left its team's roster with no single-team successor",
|
||||
agent.pubkey
|
||||
);
|
||||
agent.team_id = None;
|
||||
repaired += 1;
|
||||
}
|
||||
},
|
||||
// Unbound: backfill on single-team evidence.
|
||||
None => match persona_to_team.get(persona_id) {
|
||||
Some(Some(team_id)) => {
|
||||
agent.team_id = Some((*team_id).to_string());
|
||||
repaired += 1;
|
||||
}
|
||||
Some(None) => eprintln!(
|
||||
"buzz-desktop: team-membership-repair: leaving instance {:?} unbound — persona \
|
||||
{persona_id:?} spans multiple teams",
|
||||
agent.pubkey
|
||||
),
|
||||
None => {}
|
||||
},
|
||||
}
|
||||
}
|
||||
repaired
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "team_membership_tests.rs"]
|
||||
mod tests;
|
||||
@@ -0,0 +1,625 @@
|
||||
use super::repair_team_membership_in_dir;
|
||||
use crate::migration::test_support::{
|
||||
read_agents_json, read_teams_json, write_agents_json, write_teams_json,
|
||||
};
|
||||
use std::path::{Path, PathBuf};
|
||||
|
||||
fn base(dir: &Path) -> PathBuf {
|
||||
dir.join("agents")
|
||||
}
|
||||
|
||||
/// A key-less definition record: `pubkey == ""`, persona id == `slug`.
|
||||
/// `source_team` is the manifest id; `source_team_persona_slug` is the
|
||||
/// pre-namespacing bare slug a stale team id would carry.
|
||||
fn definition(slug: &str, source_team: &str, bare_slug: &str) -> serde_json::Value {
|
||||
serde_json::json!({
|
||||
"name": slug,
|
||||
"pubkey": "",
|
||||
"relay_url": "ws://localhost:3000",
|
||||
"acp_command": "buzz-acp",
|
||||
"agent_command": "goose",
|
||||
"agent_args": [],
|
||||
"mcp_command": "",
|
||||
"turn_timeout_seconds": 320,
|
||||
"parallelism": 4,
|
||||
"system_prompt": "prompt",
|
||||
"model": "gpt-x",
|
||||
"provider": "openai",
|
||||
"env_vars": {},
|
||||
"start_on_app_launch": true,
|
||||
"created_at": "2026-01-01T00:00:00Z",
|
||||
"updated_at": "2026-01-01T00:00:00Z",
|
||||
"slug": slug,
|
||||
"source_team": source_team,
|
||||
"source_team_persona_slug": bare_slug,
|
||||
})
|
||||
}
|
||||
|
||||
/// A standalone definition with no team provenance (persona id == slug).
|
||||
fn standalone_definition(slug: &str) -> serde_json::Value {
|
||||
serde_json::json!({
|
||||
"name": slug,
|
||||
"pubkey": "",
|
||||
"relay_url": "ws://localhost:3000",
|
||||
"acp_command": "buzz-acp",
|
||||
"agent_command": "goose",
|
||||
"agent_args": [],
|
||||
"mcp_command": "",
|
||||
"turn_timeout_seconds": 320,
|
||||
"parallelism": 4,
|
||||
"system_prompt": "prompt",
|
||||
"model": "gpt-x",
|
||||
"provider": "openai",
|
||||
"env_vars": {},
|
||||
"start_on_app_launch": true,
|
||||
"created_at": "2026-01-01T00:00:00Z",
|
||||
"updated_at": "2026-01-01T00:00:00Z",
|
||||
"slug": slug,
|
||||
})
|
||||
}
|
||||
|
||||
/// A running instance record: `pubkey` set, linked to a persona by `persona_id`.
|
||||
fn instance(pubkey_seed: char, persona_id: &str, team_id: Option<&str>) -> serde_json::Value {
|
||||
let mut record = serde_json::json!({
|
||||
"name": persona_id,
|
||||
"pubkey": pubkey_seed.to_string().repeat(64),
|
||||
"relay_url": "ws://localhost:3000",
|
||||
"acp_command": "buzz-acp",
|
||||
"agent_command": "goose",
|
||||
"agent_args": [],
|
||||
"mcp_command": "",
|
||||
"turn_timeout_seconds": 320,
|
||||
"parallelism": 4,
|
||||
"system_prompt": "prompt",
|
||||
"model": "gpt-x",
|
||||
"provider": "openai",
|
||||
"env_vars": {},
|
||||
"start_on_app_launch": true,
|
||||
"created_at": "2026-01-01T00:00:00Z",
|
||||
"updated_at": "2026-01-01T00:00:00Z",
|
||||
"persona_id": persona_id,
|
||||
});
|
||||
record["team_id"] = match team_id {
|
||||
Some(id) => serde_json::json!(id),
|
||||
None => serde_json::Value::Null,
|
||||
};
|
||||
record
|
||||
}
|
||||
|
||||
fn team(id: &str, persona_ids: &[&str]) -> serde_json::Value {
|
||||
serde_json::json!({
|
||||
"id": id,
|
||||
"name": "Sietch Tabr",
|
||||
"description": null,
|
||||
"persona_ids": persona_ids,
|
||||
"created_at": "2026-01-01T00:00:00Z",
|
||||
"updated_at": "2026-01-01T00:00:00Z",
|
||||
})
|
||||
}
|
||||
|
||||
fn team_persona_ids(dir: &Path, id: &str) -> Vec<String> {
|
||||
read_teams_json(dir)
|
||||
.into_iter()
|
||||
.find(|t| t["id"] == id)
|
||||
.unwrap()["persona_ids"]
|
||||
.as_array()
|
||||
.unwrap()
|
||||
.iter()
|
||||
.map(|v| v.as_str().unwrap().to_string())
|
||||
.collect()
|
||||
}
|
||||
|
||||
fn instance_team_id(dir: &Path, pubkey_seed: char) -> Option<String> {
|
||||
read_agents_json(dir)
|
||||
.into_iter()
|
||||
.find(|r| r["pubkey"].as_str() == Some(&pubkey_seed.to_string().repeat(64)))
|
||||
.unwrap()["team_id"]
|
||||
.as_str()
|
||||
.map(str::to_string)
|
||||
}
|
||||
|
||||
const TEAM_ID: &str = "ab5c038c-1b12-46e2-8283-d6f7c0606fce";
|
||||
const ST: &str = "com.wpfleger.sietch-tabr";
|
||||
|
||||
/// Will's pre-fix store: the team holds four bare pre-namespacing ids plus one
|
||||
/// resolvable standalone id. Each bare id names exactly one team persona, so
|
||||
/// all four are rewritten to their namespaced slug and the standalone id is
|
||||
/// left untouched — the class the save path silently drops.
|
||||
#[test]
|
||||
fn rewrites_bare_ids_to_namespaced_slugs() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
write_teams_json(
|
||||
dir.path(),
|
||||
&serde_json::json!([team(
|
||||
TEAM_ID,
|
||||
&["369695d6", "thufir", "paul", "duncan", "alia"]
|
||||
)]),
|
||||
);
|
||||
write_agents_json(
|
||||
dir.path(),
|
||||
&serde_json::json!([
|
||||
standalone_definition("369695d6"),
|
||||
definition("sietch-tabr:thufir", ST, "thufir"),
|
||||
definition("sietch-tabr:paul", ST, "paul"),
|
||||
definition("sietch-tabr:duncan", ST, "duncan"),
|
||||
definition("sietch-tabr:alia", ST, "alia"),
|
||||
]),
|
||||
);
|
||||
|
||||
assert_eq!(repair_team_membership_in_dir(&base(dir.path())).unwrap(), 4);
|
||||
assert_eq!(
|
||||
team_persona_ids(dir.path(), TEAM_ID),
|
||||
vec![
|
||||
"369695d6",
|
||||
"sietch-tabr:thufir",
|
||||
"sietch-tabr:paul",
|
||||
"sietch-tabr:duncan",
|
||||
"sietch-tabr:alia",
|
||||
]
|
||||
);
|
||||
}
|
||||
|
||||
/// A directory-backed team scopes candidates by its `source_dir` name (the pack
|
||||
/// manifest id), so a bare slug that appears under two different source teams is
|
||||
/// disambiguated to the one this team is sourced from.
|
||||
#[test]
|
||||
fn scopes_candidates_by_source_dir_for_directory_backed_team() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let mut t = team(TEAM_ID, &["thufir"]);
|
||||
t["source_dir"] = serde_json::json!(format!("/packs/{ST}"));
|
||||
write_teams_json(dir.path(), &serde_json::json!([t]));
|
||||
write_agents_json(
|
||||
dir.path(),
|
||||
&serde_json::json!([
|
||||
definition("sietch-tabr:thufir", ST, "thufir"),
|
||||
// A collision: a different team also has a persona whose bare slug
|
||||
// is "thufir". Without source scoping this would be ambiguous.
|
||||
definition("other:thufir", "com.other.pack", "thufir"),
|
||||
]),
|
||||
);
|
||||
|
||||
assert_eq!(repair_team_membership_in_dir(&base(dir.path())).unwrap(), 1);
|
||||
assert_eq!(
|
||||
team_persona_ids(dir.path(), TEAM_ID),
|
||||
vec!["sietch-tabr:thufir"]
|
||||
);
|
||||
}
|
||||
|
||||
/// A bare id that names two personas with no usable scope is ambiguous: the
|
||||
/// migration leaves it in place (strictly safer than the save path, which drops
|
||||
/// it) and the file is not rewritten.
|
||||
#[test]
|
||||
fn leaves_ambiguous_id_in_place_without_writing() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
// Detached team (no source_dir) with a single stale member => no resolvable
|
||||
// sibling to infer a source-team scope from.
|
||||
write_teams_json(dir.path(), &serde_json::json!([team(TEAM_ID, &["thufir"])]));
|
||||
write_agents_json(
|
||||
dir.path(),
|
||||
&serde_json::json!([
|
||||
definition("sietch-tabr:thufir", ST, "thufir"),
|
||||
definition("other:thufir", "com.other.pack", "thufir"),
|
||||
]),
|
||||
);
|
||||
let before = std::fs::read_to_string(base(dir.path()).join("teams.json")).unwrap();
|
||||
|
||||
assert_eq!(repair_team_membership_in_dir(&base(dir.path())).unwrap(), 0);
|
||||
assert_eq!(team_persona_ids(dir.path(), TEAM_ID), vec!["thufir"]);
|
||||
assert_eq!(
|
||||
std::fs::read_to_string(base(dir.path()).join("teams.json")).unwrap(),
|
||||
before,
|
||||
"an ambiguous-only store is never rewritten"
|
||||
);
|
||||
assert!(
|
||||
!base(dir.path())
|
||||
.join("teams.json.pre-team-membership-repair.bak")
|
||||
.exists(),
|
||||
"no backup when nothing is repaired"
|
||||
);
|
||||
}
|
||||
|
||||
/// A detached team infers its source-team scope from the unique `source_team`
|
||||
/// among its already-resolvable members, so a bare id is disambiguated even
|
||||
/// without a `source_dir`.
|
||||
#[test]
|
||||
fn infers_scope_from_resolvable_siblings_when_detached() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
write_teams_json(
|
||||
dir.path(),
|
||||
&serde_json::json!([team(TEAM_ID, &["sietch-tabr:paul", "thufir"])]),
|
||||
);
|
||||
write_agents_json(
|
||||
dir.path(),
|
||||
&serde_json::json!([
|
||||
definition("sietch-tabr:paul", ST, "paul"),
|
||||
definition("sietch-tabr:thufir", ST, "thufir"),
|
||||
definition("other:thufir", "com.other.pack", "thufir"),
|
||||
]),
|
||||
);
|
||||
|
||||
assert_eq!(repair_team_membership_in_dir(&base(dir.path())).unwrap(), 1);
|
||||
assert_eq!(
|
||||
team_persona_ids(dir.path(), TEAM_ID),
|
||||
vec!["sietch-tabr:paul", "sietch-tabr:thufir"]
|
||||
);
|
||||
}
|
||||
|
||||
/// Backfill sets `team_id` on an instance whose persona is a team member but
|
||||
/// whose own `team_id` is null (the Gurney case), and leaves an already-bound
|
||||
/// instance untouched (a persona shared across teams keeps its binding).
|
||||
#[test]
|
||||
fn backfills_null_team_id_but_never_re_points_a_bound_instance() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
write_teams_json(
|
||||
dir.path(),
|
||||
&serde_json::json!([team(TEAM_ID, &["sietch-tabr:gurney", "sietch-tabr:hayt"])]),
|
||||
);
|
||||
write_agents_json(
|
||||
dir.path(),
|
||||
&serde_json::json!([
|
||||
definition("sietch-tabr:gurney", ST, "gurney"),
|
||||
definition("sietch-tabr:hayt", ST, "hayt"),
|
||||
instance('g', "sietch-tabr:gurney", None),
|
||||
instance('h', "sietch-tabr:hayt", Some("other-team")),
|
||||
]),
|
||||
);
|
||||
|
||||
assert_eq!(repair_team_membership_in_dir(&base(dir.path())).unwrap(), 1);
|
||||
assert_eq!(instance_team_id(dir.path(), 'g').as_deref(), Some(TEAM_ID));
|
||||
assert_eq!(
|
||||
instance_team_id(dir.path(), 'h').as_deref(),
|
||||
Some("other-team"),
|
||||
"an already-bound instance is never re-pointed"
|
||||
);
|
||||
}
|
||||
|
||||
/// A legacy unbound instance whose persona belongs to *two* teams is left
|
||||
/// unbound: JSON team order is not ownership evidence, and the product permits
|
||||
/// one persona under multiple teams with distinct instructions. Its team
|
||||
/// sibling — a persona in only one team — is still backfilled in the same pass.
|
||||
#[test]
|
||||
fn leaves_unbound_instance_of_a_multi_team_persona_unbound() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
write_teams_json(
|
||||
dir.path(),
|
||||
&serde_json::json!([
|
||||
team(TEAM_ID, &["sietch-tabr:duncan", "sietch-tabr:paul"]),
|
||||
team("other-team", &["sietch-tabr:duncan"]),
|
||||
]),
|
||||
);
|
||||
write_agents_json(
|
||||
dir.path(),
|
||||
&serde_json::json!([
|
||||
definition("sietch-tabr:duncan", ST, "duncan"),
|
||||
definition("sietch-tabr:paul", ST, "paul"),
|
||||
instance('d', "sietch-tabr:duncan", None),
|
||||
instance('p', "sietch-tabr:paul", None),
|
||||
]),
|
||||
);
|
||||
|
||||
// Only Paul (single-team) is backfilled; Duncan (two teams) stays unbound.
|
||||
assert_eq!(repair_team_membership_in_dir(&base(dir.path())).unwrap(), 1);
|
||||
assert_eq!(instance_team_id(dir.path(), 'd'), None);
|
||||
assert_eq!(instance_team_id(dir.path(), 'p').as_deref(), Some(TEAM_ID));
|
||||
}
|
||||
|
||||
/// A persona listed twice within a *single* team is not ambiguity — the storage
|
||||
/// boundary does not dedupe `persona_ids`. Its unbound instance is still bound
|
||||
/// to that one team; only a *distinct* second team poisons the entry.
|
||||
#[test]
|
||||
fn same_team_duplicate_persona_id_still_backfills() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
write_teams_json(
|
||||
dir.path(),
|
||||
&serde_json::json!([team(TEAM_ID, &["sietch-tabr:duncan", "sietch-tabr:duncan"])]),
|
||||
);
|
||||
write_agents_json(
|
||||
dir.path(),
|
||||
&serde_json::json!([
|
||||
definition("sietch-tabr:duncan", ST, "duncan"),
|
||||
instance('d', "sietch-tabr:duncan", None),
|
||||
]),
|
||||
);
|
||||
|
||||
assert_eq!(repair_team_membership_in_dir(&base(dir.path())).unwrap(), 1);
|
||||
assert_eq!(instance_team_id(dir.path(), 'd').as_deref(), Some(TEAM_ID));
|
||||
}
|
||||
|
||||
/// A stale binding — the bound team no longer lists the instance's persona (a
|
||||
/// "keep agents" removal left it behind) — is cleared when no other single team
|
||||
/// claims the persona, so the kept instance stops drawing that team's
|
||||
/// instructions at spawn.
|
||||
#[test]
|
||||
fn clears_stale_binding_when_persona_left_its_team() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
// The team no longer lists gurney; the instance is still bound to it.
|
||||
write_teams_json(
|
||||
dir.path(),
|
||||
&serde_json::json!([team(TEAM_ID, &["sietch-tabr:paul"])]),
|
||||
);
|
||||
write_agents_json(
|
||||
dir.path(),
|
||||
&serde_json::json!([
|
||||
definition("sietch-tabr:gurney", ST, "gurney"),
|
||||
definition("sietch-tabr:paul", ST, "paul"),
|
||||
instance('g', "sietch-tabr:gurney", Some(TEAM_ID)),
|
||||
]),
|
||||
);
|
||||
|
||||
assert_eq!(repair_team_membership_in_dir(&base(dir.path())).unwrap(), 1);
|
||||
assert_eq!(instance_team_id(dir.path(), 'g'), None);
|
||||
}
|
||||
|
||||
/// A stale binding is *re-pointed* — not merely cleared — when the persona now
|
||||
/// belongs to exactly one other team, matching the single-evidence backfill
|
||||
/// rule.
|
||||
#[test]
|
||||
fn repoints_stale_binding_to_the_sole_successor_team() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
// gurney left TEAM_ID but is the sole member of other-team.
|
||||
write_teams_json(
|
||||
dir.path(),
|
||||
&serde_json::json!([
|
||||
team(TEAM_ID, &["sietch-tabr:paul"]),
|
||||
team("other-team", &["sietch-tabr:gurney"]),
|
||||
]),
|
||||
);
|
||||
write_agents_json(
|
||||
dir.path(),
|
||||
&serde_json::json!([
|
||||
definition("sietch-tabr:gurney", ST, "gurney"),
|
||||
definition("sietch-tabr:paul", ST, "paul"),
|
||||
instance('g', "sietch-tabr:gurney", Some(TEAM_ID)),
|
||||
]),
|
||||
);
|
||||
|
||||
assert_eq!(repair_team_membership_in_dir(&base(dir.path())).unwrap(), 1);
|
||||
assert_eq!(
|
||||
instance_team_id(dir.path(), 'g').as_deref(),
|
||||
Some("other-team")
|
||||
);
|
||||
}
|
||||
|
||||
/// A binding whose team still lists the persona is authoritative — a repair pass
|
||||
/// leaves it untouched even when that persona also belongs to another team.
|
||||
#[test]
|
||||
fn leaves_live_binding_untouched_for_multi_team_persona() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
write_teams_json(
|
||||
dir.path(),
|
||||
&serde_json::json!([
|
||||
team(TEAM_ID, &["sietch-tabr:duncan"]),
|
||||
team("other-team", &["sietch-tabr:duncan"]),
|
||||
]),
|
||||
);
|
||||
write_agents_json(
|
||||
dir.path(),
|
||||
&serde_json::json!([
|
||||
definition("sietch-tabr:duncan", ST, "duncan"),
|
||||
instance('d', "sietch-tabr:duncan", Some(TEAM_ID)),
|
||||
]),
|
||||
);
|
||||
|
||||
assert_eq!(repair_team_membership_in_dir(&base(dir.path())).unwrap(), 0);
|
||||
assert_eq!(instance_team_id(dir.path(), 'd').as_deref(), Some(TEAM_ID));
|
||||
}
|
||||
|
||||
/// A store that needs no repair is a clean no-op: `Ok(0)`, no write, no backup.
|
||||
#[test]
|
||||
fn clean_store_is_a_no_op() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
write_teams_json(
|
||||
dir.path(),
|
||||
&serde_json::json!([team(TEAM_ID, &["sietch-tabr:paul"])]),
|
||||
);
|
||||
write_agents_json(
|
||||
dir.path(),
|
||||
&serde_json::json!([
|
||||
definition("sietch-tabr:paul", ST, "paul"),
|
||||
instance('p', "sietch-tabr:paul", Some(TEAM_ID)),
|
||||
]),
|
||||
);
|
||||
let teams_before = std::fs::read_to_string(base(dir.path()).join("teams.json")).unwrap();
|
||||
let agents_before =
|
||||
std::fs::read_to_string(base(dir.path()).join("managed-agents.json")).unwrap();
|
||||
|
||||
assert_eq!(repair_team_membership_in_dir(&base(dir.path())).unwrap(), 0);
|
||||
assert_eq!(
|
||||
std::fs::read_to_string(base(dir.path()).join("teams.json")).unwrap(),
|
||||
teams_before
|
||||
);
|
||||
assert_eq!(
|
||||
std::fs::read_to_string(base(dir.path()).join("managed-agents.json")).unwrap(),
|
||||
agents_before
|
||||
);
|
||||
}
|
||||
|
||||
/// The full repair is idempotent: a second boot over the already-repaired store
|
||||
/// finds nothing to do and does not write.
|
||||
#[test]
|
||||
fn second_run_is_a_no_op() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
write_teams_json(dir.path(), &serde_json::json!([team(TEAM_ID, &["thufir"])]));
|
||||
write_agents_json(
|
||||
dir.path(),
|
||||
&serde_json::json!([
|
||||
definition("sietch-tabr:thufir", ST, "thufir"),
|
||||
instance('t', "sietch-tabr:thufir", None),
|
||||
]),
|
||||
);
|
||||
|
||||
assert_eq!(repair_team_membership_in_dir(&base(dir.path())).unwrap(), 2);
|
||||
let teams_after = std::fs::read_to_string(base(dir.path()).join("teams.json")).unwrap();
|
||||
let agents_after =
|
||||
std::fs::read_to_string(base(dir.path()).join("managed-agents.json")).unwrap();
|
||||
|
||||
assert_eq!(
|
||||
repair_team_membership_in_dir(&base(dir.path())).unwrap(),
|
||||
0,
|
||||
"second run finds nothing"
|
||||
);
|
||||
assert_eq!(
|
||||
std::fs::read_to_string(base(dir.path()).join("teams.json")).unwrap(),
|
||||
teams_after
|
||||
);
|
||||
assert_eq!(
|
||||
std::fs::read_to_string(base(dir.path()).join("managed-agents.json")).unwrap(),
|
||||
agents_after
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn missing_store_is_a_no_op() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
std::fs::create_dir_all(base(dir.path())).unwrap();
|
||||
assert_eq!(repair_team_membership_in_dir(&base(dir.path())).unwrap(), 0);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn unparseable_store_errors_without_writing() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
std::fs::create_dir_all(base(dir.path())).unwrap();
|
||||
let teams_path = base(dir.path()).join("teams.json");
|
||||
std::fs::write(&teams_path, "{ not json").unwrap();
|
||||
write_agents_json(dir.path(), &serde_json::json!([]));
|
||||
|
||||
let err = repair_team_membership_in_dir(&base(dir.path())).unwrap_err();
|
||||
assert!(err.contains("failed to parse"), "unexpected error: {err}");
|
||||
assert_eq!(
|
||||
std::fs::read_to_string(&teams_path).unwrap(),
|
||||
"{ not json",
|
||||
"a corrupt store is left for manual recovery"
|
||||
);
|
||||
}
|
||||
|
||||
/// The teams.json backup captures the pre-migration bytes and is written once.
|
||||
#[test]
|
||||
fn writes_teams_backup_once() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
write_teams_json(dir.path(), &serde_json::json!([team(TEAM_ID, &["thufir"])]));
|
||||
write_agents_json(
|
||||
dir.path(),
|
||||
&serde_json::json!([definition("sietch-tabr:thufir", ST, "thufir")]),
|
||||
);
|
||||
let bak = base(dir.path()).join("teams.json.pre-team-membership-repair.bak");
|
||||
|
||||
assert_eq!(repair_team_membership_in_dir(&base(dir.path())).unwrap(), 1);
|
||||
let bak_content = std::fs::read_to_string(&bak).unwrap();
|
||||
assert!(
|
||||
bak_content.contains("\"thufir\""),
|
||||
"backup holds the pre-migration stale id"
|
||||
);
|
||||
}
|
||||
|
||||
/// Both pristine backups are created BEFORE either live store is rewritten, so
|
||||
/// a crash between the two writes still leaves a full recovery pair (Carl's
|
||||
/// backup-contract finding). A stale bare slug on the team (drives the teams
|
||||
/// rewrite) plus an unbound instance (drives the agents backfill) exercises
|
||||
/// both stores; each backup must hold the pre-migration bytes.
|
||||
#[test]
|
||||
fn both_backups_precede_either_live_write() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
write_teams_json(dir.path(), &serde_json::json!([team(TEAM_ID, &["thufir"])]));
|
||||
write_agents_json(
|
||||
dir.path(),
|
||||
&serde_json::json!([
|
||||
definition("sietch-tabr:thufir", ST, "thufir"),
|
||||
instance('t', "sietch-tabr:thufir", None),
|
||||
]),
|
||||
);
|
||||
let teams_bak = base(dir.path()).join("teams.json.pre-team-membership-repair.bak");
|
||||
let agents_bak = base(dir.path()).join("managed-agents.json.pre-team-membership-repair.bak");
|
||||
|
||||
assert_eq!(repair_team_membership_in_dir(&base(dir.path())).unwrap(), 2);
|
||||
|
||||
// teams.json backup holds the stale bare slug (pre-rewrite bytes).
|
||||
let teams_bak_content = std::fs::read_to_string(&teams_bak).unwrap();
|
||||
assert!(
|
||||
teams_bak_content.contains("\"thufir\"")
|
||||
&& !teams_bak_content.contains("sietch-tabr:thufir"),
|
||||
"teams backup captures pre-rewrite bytes"
|
||||
);
|
||||
// managed-agents.json backup holds the null binding (pre-backfill bytes).
|
||||
let agents_bak_content = std::fs::read_to_string(&agents_bak).unwrap();
|
||||
assert!(
|
||||
agents_bak_content.contains("\"team_id\": null"),
|
||||
"agents backup captures pre-backfill bytes"
|
||||
);
|
||||
}
|
||||
|
||||
// ── repair→detach orchestration gate (Carl's finding #2) ──────────────────
|
||||
|
||||
use super::orchestrate_repair_then_detach;
|
||||
use std::cell::Cell;
|
||||
|
||||
/// A failed repair must SKIP the directory-backed detach: detach clears
|
||||
/// `source_dir`, the disambiguating evidence a clean-repair retry needs, so
|
||||
/// running it after a repair error would let the original membership-loss path
|
||||
/// recur on the next boot.
|
||||
#[test]
|
||||
fn failed_repair_skips_detach() {
|
||||
let detach_ran = Cell::new(false);
|
||||
orchestrate_repair_then_detach(
|
||||
|| Err("repair write failed".to_string()),
|
||||
|| {
|
||||
detach_ran.set(true);
|
||||
Ok(0)
|
||||
},
|
||||
);
|
||||
assert!(
|
||||
!detach_ran.get(),
|
||||
"detach must not run when repair failed — source_dir is preserved for retry"
|
||||
);
|
||||
}
|
||||
|
||||
/// A successful repair runs detach, whether or not the repair changed anything
|
||||
/// (a clean-store boot with directory-backed teams still needs detaching).
|
||||
#[test]
|
||||
fn successful_repair_runs_detach() {
|
||||
let detach_ran = Cell::new(false);
|
||||
orchestrate_repair_then_detach(
|
||||
|| Ok(0),
|
||||
|| {
|
||||
detach_ran.set(true);
|
||||
Ok(1)
|
||||
},
|
||||
);
|
||||
assert!(
|
||||
detach_ran.get(),
|
||||
"detach runs after a clean repair even when repair changed nothing"
|
||||
);
|
||||
}
|
||||
|
||||
/// End-to-end discriminating proof: a failed repair must leave a
|
||||
/// directory-backed team's `source_dir` intact, because the gate skips the real
|
||||
/// detach op that would otherwise clear it. The store here is fully valid — so
|
||||
/// detach WOULD succeed and strip `source_dir` if the gate let it run — which
|
||||
/// is what makes this catch a gate that runs detach unconditionally.
|
||||
#[test]
|
||||
fn failed_repair_preserves_source_dir_against_real_detach() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let base_dir = base(dir.path());
|
||||
let mut t = team(TEAM_ID, &["sietch-tabr:thufir"]);
|
||||
t["source_dir"] = serde_json::json!(format!("/packs/{ST}"));
|
||||
write_teams_json(dir.path(), &serde_json::json!([t]));
|
||||
write_agents_json(
|
||||
dir.path(),
|
||||
&serde_json::json!([definition("sietch-tabr:thufir", ST, "thufir")]),
|
||||
);
|
||||
|
||||
orchestrate_repair_then_detach(
|
||||
|| Err("repair backup write failed".to_string()),
|
||||
|| super::super::detach::detach_directory_backed_teams_in_dir(&base_dir),
|
||||
);
|
||||
|
||||
let source_dir = read_teams_json(dir.path())
|
||||
.into_iter()
|
||||
.find(|t| t["id"] == TEAM_ID)
|
||||
.unwrap()["source_dir"]
|
||||
.clone();
|
||||
assert_eq!(
|
||||
source_dir,
|
||||
serde_json::json!(format!("/packs/{ST}")),
|
||||
"a failed repair must preserve source_dir — detach never ran to clear it"
|
||||
);
|
||||
}
|
||||
@@ -29,3 +29,17 @@ pub(crate) fn read_personas_json(dir: &Path) -> Vec<serde_json::Value> {
|
||||
let content = std::fs::read_to_string(dir.join("agents/personas.json")).unwrap();
|
||||
serde_json::from_str(&content).unwrap()
|
||||
}
|
||||
|
||||
pub(crate) fn write_teams_json(dir: &Path, records: &serde_json::Value) {
|
||||
std::fs::create_dir_all(dir.join("agents")).unwrap();
|
||||
std::fs::write(
|
||||
dir.join("agents/teams.json"),
|
||||
serde_json::to_vec_pretty(records).unwrap(),
|
||||
)
|
||||
.unwrap();
|
||||
}
|
||||
|
||||
pub(crate) fn read_teams_json(dir: &Path) -> Vec<serde_json::Value> {
|
||||
let content = std::fs::read_to_string(dir.join("agents/teams.json")).unwrap();
|
||||
serde_json::from_str(&content).unwrap()
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user