jerry/buzz
1
0
Fork 0
mirror of https://github.com/block/buzz.git synced 2026-11-09 02:41:57 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: prevent agent settings from being silently deleted on channel deletion (#752)

Browse Source 
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Wes
2026-05-26 16:09:09 -07:00
committed by GitHub
parent fbbb2d1d6e
commit ab71ade9e7
7 changed files with 21 additions and 424 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
desktop/src-tauri/src/lib.rs
View File

@@ -4,7 +4,6 @@ mod events;
mod huddle;
mod managed_agents;
mod media_proxy;
mod migration;
mod models;
pub mod nostr_convert;
mod prevent_sleep;
@@ -352,11 +351,6 @@ pub fn run() {
let app_handle = app.handle().clone();
let shutdown_started = Arc::clone(&restore_shutdown_started);
// Migrate data from the legacy `com.wesb.sprout` directory before
// resolving identity, so the persisted key is available at the new
// path on first launch after the identifier change.
migration::migrate_legacy_data_dir(&app_handle);
// Resolve persisted identity key (env var → file → generate+save).
// This is fatal — the app should not start with an ephemeral identity
// that will be lost on restart, as that silently breaks channel

10
desktop/src-tauri/src/managed_agents/storage.rs
View File

@@ -55,7 +55,15 @@ pub fn save_managed_agents(app: &AppHandle, records: &[ManagedAgentRecord]) -> R
let path = managed_agents_store_path(app)?;
let payload = serde_json::to_vec_pretty(&sorted)
.map_err(|error| format!("failed to serialize agent store: {error}"))?;
fs::write(&path, payload).map_err(|error| format!("failed to write agent store: {error}"))
// Atomic write: write to a temp file then rename. This prevents partial
// writes from corrupting the store if the process crashes mid-write.
// rename() is atomic on the same filesystem on both macOS and Linux.
let tmp_path = path.with_extension("json.tmp");
fs::write(&tmp_path, &payload)
.map_err(|error| format!("failed to write temp agent store: {error}"))?;
fs::rename(&tmp_path, &path)
.map_err(|error| format!("failed to rename temp agent store: {error}"))
}
/// Maximum log file size before rotation (10 MB).

286
desktop/src-tauri/src/migration.rs
View File

@@ -1,286 +0,0 @@
//! Per-file migration from the legacy `com.wesb.sprout` app data directory
//! to the current `xyz.block.sprout.app` directory.
//!
//! On each launch, for every known file in [`LEGACY_FILES`]:
//! - If the file **already exists** at the new path → skip (it's its own guard).
//! - If the file exists at the old path but not the new → copy it over.
//!
//! Each `std::fs::copy` is atomic per-file, so there is no partial-migration
//! problem and no sentinel file is needed.
//!
//! The legacy directory is intentionally **not** deleted — users can clean it
//! up manually once they're satisfied everything works.
//!
//! Errors are logged but never fatal; the app must still start even if
//! individual file copies fail.
//!
//! **Note on dev/prod side-by-side:** Both the production build
//! (`xyz.block.sprout.app`) and the dev build (`xyz.block.sprout.app.dev`)
//! will attempt to migrate from the same legacy `com.wesb.sprout` directory,
//! resulting in duplicated identity keys. To avoid this, set the
//! `SPROUT_PRIVATE_KEY` env var when running the dev build — this bypasses
//! file-based identity resolution entirely.
use std::path::{Path, PathBuf};
use tauri::Manager;
const LEGACY_DATA_DIR_NAME: &str = "com.wesb.sprout";
/// Known files to migrate from the legacy data directory.
///
/// Agent logs and `.window-state.json` are excluded — logs are ephemeral and
/// the window-state plugin recreates its file automatically.
const LEGACY_FILES: &[&str] = &[
"identity.key",
"agents/managed-agents.json",
"agents/personas.json",
"agents/teams.json",
];
/// Compute the legacy `com.wesb.sprout` data directory path by replacing the
/// last component of the current app data directory.
fn legacy_data_dir(current: &Path) -> Option<PathBuf> {
current.parent().map(|p| p.join(LEGACY_DATA_DIR_NAME))
}
/// Copy a single file from `old_dir/rel` to `new_dir/rel`, creating parent
/// directories as needed. Skips the file if it already exists at the
/// destination.
///
/// Returns `true` if the file was copied, `false` if skipped or missing.
fn migrate_file(old_dir: &Path, new_dir: &Path, rel: &str) -> bool {
let src = old_dir.join(rel);
let dst = new_dir.join(rel);
if dst.exists() {
return false; // Already present — nothing to do.
}
if !src.exists() {
return false; // Nothing to migrate.
}
// Ensure parent directories exist (e.g. `agents/`).
if let Some(parent) = dst.parent() {
if let Err(e) = std::fs::create_dir_all(parent) {
eprintln!(
"sprout-desktop: migration: failed to create {}: {e}",
parent.display()
);
return false;
}
}
match std::fs::copy(&src, &dst) {
Ok(_) => {
eprintln!("sprout-desktop: migration: copied {rel}");
true
}
Err(e) => {
eprintln!("sprout-desktop: migration: failed to copy {rel}: {e}");
false
}
}
}
/// Migrate known files from the legacy `com.wesb.sprout` app data directory
/// to the current directory.
///
/// Called in `setup()` **before** `resolve_persisted_identity` so the persisted
/// key is available at the new path on first launch after the identifier change.
pub fn migrate_legacy_data_dir(app: &tauri::AppHandle) {
let current_dir = match app.path().app_data_dir() {
Ok(dir) => dir,
Err(e) => {
eprintln!("sprout-desktop: migration: cannot resolve app data dir: {e}");
return;
}
};
let old_dir = match legacy_data_dir(&current_dir) {
Some(dir) => dir,
None => {
eprintln!("sprout-desktop: migration: cannot compute legacy data dir (no parent)");
return;
}
};
if !old_dir.exists() {
return; // Nothing to migrate.
}
let mut copied = 0u32;
for rel in LEGACY_FILES {
if migrate_file(&old_dir, &current_dir, rel) {
copied += 1;
}
}
if copied > 0 {
eprintln!("sprout-desktop: migration: {copied} file(s) migrated from legacy data dir");
}
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn legacy_data_dir_replaces_last_component() {
let current = PathBuf::from("/Users/me/Library/Application Support/xyz.block.sprout.app");
let legacy = legacy_data_dir(&current).unwrap();
assert_eq!(
legacy,
PathBuf::from("/Users/me/Library/Application Support/com.wesb.sprout")
);
}
#[test]
fn legacy_data_dir_returns_none_for_root() {
let current = PathBuf::from("/");
let _ = legacy_data_dir(&current);
}
/// Helper: create a temp dir structure mimicking the old `com.wesb.sprout`
/// layout and return `(parent_dir, old_dir, new_dir)`.
fn setup_legacy_layout() -> (tempfile::TempDir, PathBuf, PathBuf) {
let parent = tempfile::tempdir().unwrap();
let old_dir = parent.path().join(LEGACY_DATA_DIR_NAME);
let new_dir = parent.path().join("xyz.block.sprout.app");
std::fs::create_dir_all(old_dir.join("agents")).unwrap();
std::fs::write(old_dir.join("identity.key"), "nsec1-fake-key-data").unwrap();
std::fs::write(
old_dir.join("agents/managed-agents.json"),
r#"[{"id":"a1"}]"#,
)
.unwrap();
std::fs::write(old_dir.join("agents/personas.json"), "[]").unwrap();
std::fs::write(old_dir.join("agents/teams.json"), "[]").unwrap();
(parent, old_dir, new_dir)
}
#[test]
fn migrate_file_copies_when_missing_at_dest() {
let (_parent, old_dir, new_dir) = setup_legacy_layout();
assert!(migrate_file(&old_dir, &new_dir, "identity.key"));
assert_eq!(
std::fs::read_to_string(new_dir.join("identity.key")).unwrap(),
"nsec1-fake-key-data"
);
}
#[test]
fn migrate_file_creates_parent_dirs() {
let (_parent, old_dir, new_dir) = setup_legacy_layout();
// agents/ doesn't exist in new_dir yet.
assert!(migrate_file(&old_dir, &new_dir, "agents/teams.json"));
assert_eq!(
std::fs::read_to_string(new_dir.join("agents/teams.json")).unwrap(),
"[]"
);
}
#[test]
fn migrate_file_skips_when_dest_exists() {
let (_parent, old_dir, new_dir) = setup_legacy_layout();
// Pre-create the file at the new location with different content.
std::fs::create_dir_all(&new_dir).unwrap();
std::fs::write(new_dir.join("identity.key"), "nsec1-new-key").unwrap();
// Should skip — returns false.
assert!(!migrate_file(&old_dir, &new_dir, "identity.key"));
// Original content preserved.
assert_eq!(
std::fs::read_to_string(new_dir.join("identity.key")).unwrap(),
"nsec1-new-key"
);
}
#[test]
fn migrate_file_skips_when_source_missing() {
let (_parent, old_dir, new_dir) = setup_legacy_layout();
// File that doesn't exist in old dir.
assert!(!migrate_file(&old_dir, &new_dir, "nonexistent.json"));
assert!(!new_dir.join("nonexistent.json").exists());
}
#[test]
fn migrate_all_known_files() {
let (_parent, old_dir, new_dir) = setup_legacy_layout();
let mut copied = 0u32;
for rel in LEGACY_FILES {
if migrate_file(&old_dir, &new_dir, rel) {
copied += 1;
}
}
assert_eq!(copied, 4);
assert_eq!(
std::fs::read_to_string(new_dir.join("identity.key")).unwrap(),
"nsec1-fake-key-data"
);
assert_eq!(
std::fs::read_to_string(new_dir.join("agents/managed-agents.json")).unwrap(),
r#"[{"id":"a1"}]"#
);
assert_eq!(
std::fs::read_to_string(new_dir.join("agents/personas.json")).unwrap(),
"[]"
);
assert_eq!(
std::fs::read_to_string(new_dir.join("agents/teams.json")).unwrap(),
"[]"
);
// Old directory must still exist.
assert!(old_dir.exists());
}
#[test]
fn migrate_is_idempotent() {
let (_parent, old_dir, new_dir) = setup_legacy_layout();
// First pass: copies everything.
let first_pass: u32 = LEGACY_FILES
.iter()
.map(|rel| u32::from(migrate_file(&old_dir, &new_dir, rel)))
.sum();
assert_eq!(first_pass, 4);
// Second pass: skips everything (all files already exist).
let second_pass: u32 = LEGACY_FILES
.iter()
.map(|rel| u32::from(migrate_file(&old_dir, &new_dir, rel)))
.sum();
assert_eq!(second_pass, 0);
}
#[test]
fn migrate_partial_only_copies_missing() {
let (_parent, old_dir, new_dir) = setup_legacy_layout();
// Pre-create identity.key in new dir — only the other 3 should copy.
std::fs::create_dir_all(&new_dir).unwrap();
std::fs::write(new_dir.join("identity.key"), "nsec1-already-here").unwrap();
let copied: u32 = LEGACY_FILES
.iter()
.map(|rel| u32::from(migrate_file(&old_dir, &new_dir, rel)))
.sum();
assert_eq!(copied, 3);
// identity.key should be untouched.
assert_eq!(
std::fs::read_to_string(new_dir.join("identity.key")).unwrap(),
"nsec1-already-here"
);
}
}

77
desktop/src/features/channels/cleanupChannelAgents.ts
View File

@@ -1,77 +0,0 @@
/**
* Best-effort cleanup of channel-scoped managed agents.
*
* Each agent added via the "Add agents" dialog is a dedicated managed-agent
* record. If that agent is no longer present in any channel, the managed-agent
* record should be removed as well.
*/
import {
deleteManagedAgent,
getChannelMembers,
listManagedAgents,
listRelayAgents,
} from "@/shared/api/tauri";
async function cleanupManagedAgentsByPubkey(
pubkeys: readonly string[],
options?: { ignoreChannelId?: string },
): Promise<void> {
const normalizedPubkeys = new Set(
pubkeys
.map((pubkey) => pubkey.trim().toLowerCase())
.filter((pubkey) => pubkey.length > 0),
);
if (normalizedPubkeys.size === 0) {
return;
}
const [managedAgents, relayAgents] = await Promise.all([
listManagedAgents(),
listRelayAgents(),
]);
const agentsToDelete = managedAgents.filter((agent) =>
normalizedPubkeys.has(agent.pubkey.toLowerCase()),
);
// Delete orphaned agents (best-effort — don't block channel deletion).
await Promise.allSettled(
agentsToDelete
.filter((agent) => {
const relayAgent = relayAgents.find(
(candidate) =>
candidate.pubkey.toLowerCase() === agent.pubkey.toLowerCase(),
);
if (!relayAgent) {
// Not found in relay — safe to delete.
return true;
}
const activeChannelIds = relayAgent.channelIds.filter(
(channelId) => channelId !== options?.ignoreChannelId,
);
return activeChannelIds.length === 0;
})
.map((agent) => deleteManagedAgent(agent.pubkey)),
);
}
export async function cleanupManagedAgentIfOrphaned(
pubkey: string,
channelId?: string,
): Promise<void> {
await cleanupManagedAgentsByPubkey([pubkey], {
ignoreChannelId: channelId,
});
}
export async function cleanupChannelAgents(channelId: string): Promise<void> {
const members = await getChannelMembers(channelId);
await cleanupManagedAgentsByPubkey(
members.map((member) => member.pubkey),
{
ignoreChannelId: channelId,
},
);
}

26
desktop/src/features/channels/hooks.ts
View File

@@ -21,10 +21,6 @@ import {
unarchiveChannel,
updateChannel,
} from "@/shared/api/tauri";
import {
cleanupChannelAgents,
cleanupManagedAgentIfOrphaned,
} from "@/features/channels/cleanupChannelAgents";
import type {
AddChannelMembersInput,
Channel,
@@ -335,13 +331,6 @@ export function useDeleteChannelMutation(channelId: string | null) {
throw new Error("No channel selected.");
}
// Best-effort cleanup of managed agents scoped to this channel.
try {
await cleanupChannelAgents(channelId);
} catch (error) {
console.warn("Failed to clean up managed agents:", error);
}
await deleteChannel(channelId);
},
onSuccess: () => {
@@ -386,19 +375,6 @@ export function useAddChannelMembersMutation(channelId: string | null) {
});
}
export async function removeChannelMemberWithManagedAgentCleanup(
channelId: string,
pubkey: string,
) {
await removeChannelMember(channelId, pubkey);
try {
await cleanupManagedAgentIfOrphaned(pubkey, channelId);
} catch (error) {
console.warn("Failed to clean up managed agent:", error);
}
}
export function useRemoveChannelMemberMutation(channelId: string | null) {
const queryClient = useQueryClient();
@@ -408,7 +384,7 @@ export function useRemoveChannelMemberMutation(channelId: string | null) {
throw new Error("No channel selected.");
}
await removeChannelMemberWithManagedAgentCleanup(channelId, pubkey);
await removeChannelMember(channelId, pubkey);
},
onSettled: async () => {
await Promise.all([

4
desktop/src/features/channels/ui/useMembersSidebarActions.ts
View File

@@ -13,9 +13,9 @@ import {
} from "@/features/agents/lib/managedAgentControlActions";
import {
channelsQueryKey,
removeChannelMemberWithManagedAgentCleanup,
useRemoveChannelMemberMutation,
} from "@/features/channels/hooks";
import { removeChannelMember } from "@/shared/api/tauri";
import type { ChannelMember, ManagedAgent } from "@/shared/api/types";
type UseMembersSidebarActionsOptions = {
@@ -242,7 +242,7 @@ export function useMembersSidebarActions({
throw new Error("No channel selected.");
}
await removeChannelMemberWithManagedAgentCleanup(channelId, pubkey);
await removeChannelMember(channelId, pubkey);
}
async function invalidateSidebarQueries() {

36
desktop/tests/e2e/channels.spec.ts
View File

@@ -872,7 +872,7 @@ test("open-channel members can add agents from the header", async ({
await expect(page.getByTestId("add-channel-bot-dialog-footer")).toBeVisible();
});
test("removing a channel-scoped agent also cleans up the managed agent record", async ({
test("removing a channel-scoped agent preserves the managed agent record", async ({
page,
}) => {
const agentName = `cleanup-agent-${Date.now()}`;
@@ -893,10 +893,7 @@ test("removing a channel-scoped agent also cleans up the managed agent record",
await expect(page.getByTestId("members-sidebar")).not.toBeVisible();
await page.getByTestId("open-agents-view").click();
await expect(page.getByTestId(`managed-agent-${agentPubkey}`)).toHaveCount(0);
const commands = await readCommandLog(page);
expect(commands).toContain("delete_managed_agent");
await expect(page.getByTestId(`managed-agent-${agentPubkey}`)).toHaveCount(1);
});
test("members sidebar can respawn a stopped managed bot", async ({ page }) => {
@@ -947,7 +944,7 @@ test("members sidebar can respawn a stopped managed bot", async ({ page }) => {
).toBe(baselineStopCount + 1);
});
test("members sidebar supports bulk remove for managed bots", async ({
test("members sidebar supports bulk remove for managed bots from channel", async ({
page,
}) => {
const firstAgentName = `sidebar-remove-a-${Date.now()}`;
@@ -985,21 +982,18 @@ test("members sidebar supports bulk remove for managed bots", async ({
await page.getByTestId("open-agents-view").click();
await expect(
page.getByTestId(`managed-agent-${firstAgentPubkey}`),
).toHaveCount(0);
).toHaveCount(1);
await expect(
page.getByTestId(`managed-agent-${secondAgentPubkey}`),
).toHaveCount(0);
).toHaveCount(1);
const commands = await readCommandLog(page);
expect(
commands.filter((command) => command === "remove_channel_member"),
).toHaveLength(2);
expect(
commands.filter((command) => command === "delete_managed_agent"),
).toHaveLength(2);
});
test("removing a multi-channel managed bot keeps its record until it is orphaned", async ({
test("removing a multi-channel managed bot preserves its record after removal from all channels", async ({
page,
}) => {
const agentName = `multi-channel-agent-${Date.now()}`;
@@ -1035,9 +1029,6 @@ test("removing a multi-channel managed bot keeps its record until it is orphaned
const baselineRemoves = baseline.filter(
(c) => c === "remove_channel_member",
).length;
const baselineDeletes = baseline.filter(
(c) => c === "delete_managed_agent",
).length;
await openMembersSidebar(page, "general");
await openMemberMenu(page, agentPubkey);
@@ -1052,16 +1043,11 @@ test("removing a multi-channel managed bot keeps its record until it is orphaned
await expect(page.getByTestId(`managed-agent-${agentPubkey}`)).toHaveCount(1);
let commands = await readCommandLog(page);
// First removal: 1 remove_channel_member, bot still in second channel
// so no delete_managed_agent yet.
// First removal: 1 remove_channel_member, agent record preserved.
expect(
commands.filter((c) => c === "remove_channel_member").length -
baselineRemoves,
).toBe(1);
expect(
commands.filter((c) => c === "delete_managed_agent").length -
baselineDeletes,
).toBe(0);
await openMembersSidebar(page, secondChannelName);
await openMemberMenu(page, agentPubkey);
@@ -1073,18 +1059,14 @@ test("removing a multi-channel managed bot keeps its record until it is orphaned
await expect(page.getByTestId("members-sidebar")).not.toBeVisible();
await page.getByTestId("open-agents-view").click();
await expect(page.getByTestId(`managed-agent-${agentPubkey}`)).toHaveCount(0);
await expect(page.getByTestId(`managed-agent-${agentPubkey}`)).toHaveCount(1);
commands = await readCommandLog(page);
// Second removal: bot is now orphaned, so cleanup deletes the managed agent.
// Second removal: agent is preserved even after removal from all channels.
expect(
commands.filter((c) => c === "remove_channel_member").length -
baselineRemoves,
).toBe(2);
expect(
commands.filter((c) => c === "delete_managed_agent").length -
baselineDeletes,
).toBe(1);
});
test("bulk remove stays hidden when row-level remove is not allowed", async ({